D16579: Musepack disk number field name is DISC.

[Alexander Stippich]

astippich added a comment.


  In D16579#352147 , @smithjd wrote:
  
  > In D16579#351910 , @astippich 
wrote:
  >
  > > The ape tag tests fail with this patch, but the test is actually wrong in 
that regard. It tests for an empty disc number, which I haven't noticed before.
  > >  I've found references to both DISCNUMBER and DISC, so the safest way is 
probably to check both.
  > >  So please query both tags like it is already done for the album artist 
and adjust the taglibextractortest.
  >
  >
  > DISCNUMBER doesn't seem to be a valid (or widely used) field for APEv2.
  >
  > More (Picard) information: https://picard.musicbrainz.org/docs/mappings
  >
  > Also it appears Picard, Puddletag and ffmpeg use 'Album Artist', not 
'ALBUMARTIST'. And the link I provided 
(https://wiki.hydrogenaud.io/index.php?title=Tag_Mapping) also lists 'Album 
Artist' not 'ALBUMARTIST'. The unit test file has an 'ALBUMARTIST' field that 
should be changed to 'Album Artist'.
  
  
  Yes, maybe it's not widely used, but it is used. Kodi for example supports 
both reading from DISCNUMBER and DISC. If you use kid3 to edit the metadata of 
ape tags, the standard behavior is to actually to write to DISCNUMBER (and 
similar to ALBUMARTIST).
  One thing I've learned when I digged into metadata of audio files is that 
there is no standard, and KFileMetaData should handle as much cases as 
possible. Since it is easy to query both, please add it.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16579

To: smithjd, astippich
Cc: astippich, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, spoorun, 
ngraham, bruns, abrahams

D12820: Add KWayland virtual desktop protocol

[Marco Martin]

This revision was automatically updated to reflect the committed changes.
Closed by commit R127:d0db5da57563: Add KWayland virtual desktop protocol 
(authored by mart).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D12820?vs=38123=44643#toc

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12820?vs=38123=44643

REVISION DETAIL
  https://phabricator.kde.org/D12820

AFFECTED FILES
  autotests/client/CMakeLists.txt
  autotests/client/test_plasma_virtual_desktop.cpp
  src/client/CMakeLists.txt
  src/client/plasmavirtualdesktop.cpp
  src/client/plasmavirtualdesktop.h
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/protocols/plasma-virtual-desktop.xml
  src/client/protocols/plasma-window-management.xml
  src/client/registry.cpp
  src/client/registry.h
  src/server/CMakeLists.txt
  src/server/display.cpp
  src/server/display.h
  src/server/plasmavirtualdesktop_interface.cpp
  src/server/plasmavirtualdesktop_interface.h
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h
  src/tools/mapping.txt

To: mart, #kwin, #plasma, graesslin, hein, davidedmundson
Cc: davidedmundson, zzag, bshah, romangg, kde-frameworks-devel, michaelh, 
ngraham, bruns

D16490: [KFileMetaData] Add unittest for XML extractor

[Alexander Stippich]

astippich added a comment.


  In D16490#352109 , @bruns wrote:
  
  > In D16490#351935 , @astippich 
wrote:
  >
  > > In D16490#351799 , @bruns 
wrote:
  > >
  > > > In D16490#351662 , 
@astippich wrote:
  > > >
  > > > > Only one minor thing: please also check that the mimetype is in the 
list of supported mimetypes
  > > >
  > > >
  > > > This can actually happen and is completely valid, due to mimetype 
inheritance.
  > > >
  > > > So the check would be `for supported in supportedMimetypes { if 
QMimeType(input->mimeType()).inherits(supported) return true; }; return false`. 
But this is already done from the calling code ...
  > >
  > >
  > > Hmmm, I don't understand. When I change the code to return an empty 
stringlist of supported mimetypes for the xmlextractor, the tests still pass.
  > >  This should imho be covered by the tests.
  >
  >
  > This is one level above these tests. The surrounding code ensures the right 
extractor is called for each file, see 
`ExtractorCollection::fetchExtractors(...)`.
  
  
  Right, and if e.g. the list of supported mimetypes is empty, the 
corresponding extractor will never be selected because ExtractorCollection 
doesn't know that the mimetype is supported by this extractor.
  Hence we should ensure and test imho that the list of supported mimetypes 
provided to the ExtractorCollection is correct for this extractor. I'm not 
calling for testing that the right extractor is selected.
  
  > These are unit tests. The test itself is responsible to call an extractor 
with a suitable file and a matching mimetype.
  > 
  > What you are calling for are system tests.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16490

To: bruns, #frameworks, astippich
Cc: lbeltrame, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D16490: [KFileMetaData] Add unittest for XML extractor

[Stefan Brüns]

bruns added a comment.


  In D16490#352199 , @astippich 
wrote:
  
  > In D16490#352109 , @bruns wrote:
  >
  > > In D16490#351935 , @astippich 
wrote:
  > >
  > > > In D16490#351799 , @bruns 
wrote:
  > > >
  > > > > In D16490#351662 , 
@astippich wrote:
  > > > >
  > > > > > Only one minor thing: please also check that the mimetype is in the 
list of supported mimetypes
  > > > >
  > > > >
  > > > > This can actually happen and is completely valid, due to mimetype 
inheritance.
  > > > >
  > > > > So the check would be `for supported in supportedMimetypes { if 
QMimeType(input->mimeType()).inherits(supported) return true; }; return false`. 
But this is already done from the calling code ...
  > > >
  > > >
  > > > Hmmm, I don't understand. When I change the code to return an empty 
stringlist of supported mimetypes for the xmlextractor, the tests still pass.
  > > >  This should imho be covered by the tests.
  > >
  > >
  > > This is one level above these tests. The surrounding code ensures the 
right extractor is called for each file, see 
`ExtractorCollection::fetchExtractors(...)`.
  >
  >
  > Right, and if e.g. the list of supported mimetypes is empty, the 
corresponding extractor will never be selected because ExtractorCollection 
doesn't know that the mimetype is supported by this extractor.
  >  Hence we should ensure and test imho that the list of supported mimetypes 
provided to the ExtractorCollection is correct for this extractor. I'm not 
calling for testing that the right extractor is selected.
  
  
  The unit tests do not use ExtractorCollection, because they test the 
extractors, not ExtractorCollection. The extractor unit tests explicitly pass 
the mime type to the extractor.
  
  We don't want to double the checks.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16490

To: bruns, #frameworks, astippich
Cc: lbeltrame, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

KDE CI: Frameworks » kwayland » kf5-qt5 SUSEQt5.9 - Build # 68 - Failure!

[CI System]

BUILD FAILURE
 Build URL
https://build.kde.org/job/Frameworks/job/kwayland/job/kf5-qt5%20SUSEQt5.9/68/
 Project:
kf5-qt5 SUSEQt5.9
 Date of build:
Thu, 01 Nov 2018 15:34:57 +
 Build duration:
7 min 12 sec and counting
   CONSOLE OUTPUT
  [...truncated 532.94 KB...][ 82%] Linking CXX executable ../bin/panelTest[ 82%] Built target panelTestScanning dependencies of target shadowTest[ 82%] Building CXX object tests/CMakeFiles/shadowTest.dir/shadowtest.cpp.o[ 82%] Built target testQtSurfaceExtensionScanning dependencies of target dpmsTest[ 82%] Building CXX object tests/CMakeFiles/dpmsTest.dir/dpmstest.cpp.o[ 82%] Building CXX object tests/CMakeFiles/shadowTest.dir/shadowTest_autogen/mocs_compilation.cpp.o[ 83%] Linking CXX executable ../bin/shadowTest[ 83%] Building CXX object tests/CMakeFiles/dpmsTest.dir/dpmsTest_autogen/mocs_compilation.cpp.o[ 83%] Built target shadowTestScanning dependencies of target plasmasurface-test[ 83%] Building CXX object tests/CMakeFiles/plasmasurface-test.dir/plasmasurfacetest.cpp.o[ 83%] Linking CXX executable ../bin/dpmsTest[ 83%] Built target dpmsTestScanning dependencies of target xdg-test[ 83%] Building CXX object tests/CMakeFiles/xdg-test.dir/xdgtest.cpp.o[ 83%] Building CXX object tests/CMakeFiles/plasmasurface-test.dir/plasmasurface-test_autogen/mocs_compilation.cpp.o[ 83%] Linking CXX executable ../bin/plasmasurface-test[ 83%] Built target plasmasurface-test[ 83%] Building CXX object tests/CMakeFiles/xdg-test.dir/xdg-test_autogen/mocs_compilation.cpp.oScanning dependencies of target pasteClient[ 83%] Building CXX object tests/CMakeFiles/pasteClient.dir/pasteclient.cpp.o[ 84%] Linking CXX executable ../bin/xdg-test[ 84%] Built target xdg-testScanning dependencies of target copyClient[ 84%] Building CXX object tests/CMakeFiles/copyClient.dir/copyclient.cpp.o[ 84%] Building CXX object tests/CMakeFiles/pasteClient.dir/pasteClient_autogen/mocs_compilation.cpp.o[ 85%] Linking CXX executable ../bin/pasteClient[ 85%] Built target pasteClientScanning dependencies of target touchClientTest[ 86%] Building CXX object tests/CMakeFiles/touchClientTest.dir/touchclienttest.cpp.o[ 86%] Building CXX object tests/CMakeFiles/copyClient.dir/copyClient_autogen/mocs_compilation.cpp.o[ 86%] Linking CXX executable ../bin/copyClient[ 86%] Built target copyClientScanning dependencies of target testServer[ 86%] Building CXX object tests/CMakeFiles/testServer.dir/waylandservertest.cpp.o[ 86%] Building CXX object tests/CMakeFiles/touchClientTest.dir/touchClientTest_autogen/mocs_compilation.cpp.o[ 87%] Building CXX object tests/CMakeFiles/testServer.dir/testServer_autogen/mocs_compilation.cpp.o[ 87%] Linking CXX executable ../bin/testServer[ 87%] Built target testServerScanning dependencies of target testRenderingServer[ 87%] Building CXX object tests/CMakeFiles/testRenderingServer.dir/renderingservertest.cpp.o[ 87%] Linking CXX executable ../bin/touchClientTest[ 87%] Built target touchClientTestScanning dependencies of target qtwayland-integration-test[ 88%] Building CXX object tests/CMakeFiles/qtwayland-integration-test.dir/qtwaylandintegrationtest.cpp.o[ 88%] Building CXX object tests/CMakeFiles/qtwayland-integration-test.dir/qtwayland-integration-test_autogen/mocs_compilation.cpp.o[ 88%] Building CXX object tests/CMakeFiles/testRenderingServer.dir/testRenderingServer_autogen/mocs_compilation.cpp.o[ 88%] Linking CXX executable ../bin/testRenderingServer[ 88%] Linking CXX executable ../bin/qtwayland-integration-test[ 88%] Built target testRenderingServerScanning dependencies of target subsurface-test[ 88%] Building CXX object tests/CMakeFiles/subsurface-test.dir/subsurfacetest.cpp.o[ 88%] Built target qtwayland-integration-testScanning dependencies of target xdgforeign-test[ 88%] Building CXX object tests/CMakeFiles/xdgforeign-test.dir/xdgforeigntest.cpp.o/home/jenkins/workspace/Frameworks/kwayland/kf5-qt5 SUSEQt5.9/tests/xdgforeigntest.cpp: In lambda function:/home/jenkins/workspace/Frameworks/kwayland/kf5-qt5 SUSEQt5.9/tests/xdgforeigntest.cpp:146:18: warning: unused variable ���parentDeco��� [-Wunused-variable] auto parentDeco = m_decoration->create(m_surface, this);  ^~/home/jenkins/workspace/Frameworks/kwayland/kf5-qt5 SUSEQt5.9/tests/xdgforeigntest.cpp:153:18: warning: unused variable ���childDeco��� [-Wunused-variable] auto childDeco = m_decoration->create(m_childSurface, this);  ^[ 88%] Building CXX object tests/CMakeFiles/subsurface-test.dir/subsurface-test_autogen/mocs_compilation.cpp.o[ 88%] Linking CXX executable ../bin/subsurface-test[ 88%] Building CXX object tests/CMakeFiles/xdgforeign-test.dir/xdgforeign-test_autogen/mocs_compilation.cpp.o[ 88%] Linking CXX executable ../bin/xdgforeign-test[ 88%] Built target subsurface-test[ 88%] Built target xdgforeign-testScanning dependencies of target org-kde-kf5-kwayland-testserver[ 89%] Building CXX object 

D16489: [KFileMetaData] Add extractor for generic XML and SVG

[Stefan Brüns]

bruns updated this revision to Diff 44645.
bruns marked an inline comment as done.
bruns added a comment.


  Reorder mimetype check to use SVG(+XML) special case if applicable and use
  generic implementation for all other mimetypes inheriting from
  application/xml.
  Use static QStringList for mimetypes.

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16489?vs=44374=44645

REVISION DETAIL
  https://phabricator.kde.org/D16489

AFFECTED FILES
  src/extractors/CMakeLists.txt
  src/extractors/xmlextractor.cpp
  src/extractors/xmlextractor.h

To: bruns, #frameworks, astippich
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16570: Remove unused variables

[Christoph Cullmann]

cullmann accepted this revision.
cullmann added a comment.
This revision is now accepted and ready to land.


  Looks reasonable, away with the vars. Thanks!

REPOSITORY
  R39 KTextEditor

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D16570

To: aacid, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, michaelh, ngraham, bruns, 
demsking, sars, dhaumann

D16554: move supported mimetypes to static string list

[Alexander Stippich]

astippich updated this revision to Diff 44612.
astippich added a comment.


  - coding style fixes
  - use anonymous namespace

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16554?vs=44572=44612

BRANCH
  mimetype_lists

REVISION DETAIL
  https://phabricator.kde.org/D16554

AFFECTED FILES
  src/extractors/epubextractor.cpp
  src/extractors/epubextractor.h
  src/extractors/exiv2extractor.cpp
  src/extractors/exiv2extractor.h
  src/extractors/ffmpegextractor.cpp
  src/extractors/ffmpegextractor.h
  src/extractors/odfextractor.cpp
  src/extractors/odfextractor.h
  src/extractors/office2007extractor.cpp
  src/extractors/office2007extractor.h
  src/extractors/plaintextextractor.cpp
  src/extractors/plaintextextractor.h
  src/extractors/poextractor.cpp
  src/extractors/poextractor.h
  src/extractors/popplerextractor.cpp
  src/extractors/popplerextractor.h
  src/extractors/taglibextractor.cpp
  src/extractors/taglibextractor.h
  src/writers/taglibwriter.cpp
  src/writers/taglibwriter.h

To: astippich, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16583: use qlatin1 for embedded image extractor

[Alexander Stippich]

astippich created this revision.
astippich added reviewers: mgallien, bruns.
Herald added projects: Frameworks, Baloo.
Herald added subscribers: Baloo, kde-frameworks-devel.
astippich requested review of this revision.

REVISION SUMMARY
  use qlatin1string where applicable instead
  of qstringliteral

REPOSITORY
  R286 KFileMetaData

BRANCH
  embedded_image_latin1

REVISION DETAIL
  https://phabricator.kde.org/D16583

AFFECTED FILES
  src/embeddedimagedata.cpp

To: astippich, mgallien, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16579: Musepack disk number field name is DISC.

[Alexander Stippich]

astippich requested changes to this revision.
astippich added a comment.
This revision now requires changes to proceed.


  The ape tag tests fail with this patch, but the test is actually wrong in 
that regard. It tests for an empty disc number, which I haven't noticed before.
  I've found references to both DISCNUMBER and DISC, so the safest way is 
probably to check both.
  So please query both tags like it is already done for the album artist and 
adjust the taglibextractortest.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16579

To: smithjd, astippich
Cc: astippich, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, spoorun, 
ngraham, bruns, abrahams

D16570: Remove unused variables

[Albert Astals Cid]

aacid closed this revision.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D16570

To: aacid, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, michaelh, ngraham, bruns, 
demsking, sars, dhaumann

D16585: simplify handling of id3 tags

[Alexander Stippich]

astippich created this revision.
astippich added reviewers: mgallien, bruns.
Herald added projects: Frameworks, Baloo.
Herald added subscribers: Baloo, kde-frameworks-devel.
astippich requested review of this revision.

REVISION SUMMARY
  remove unrequired checks and use range-based loop

REPOSITORY
  R286 KFileMetaData

BRANCH
  simplify_id3

REVISION DETAIL
  https://phabricator.kde.org/D16585

AFFECTED FILES
  src/extractors/taglibextractor.cpp

To: astippich, mgallien, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16554: move supported mimetypes to static string list

[Alexander Stippich]

astippich updated this revision to Diff 44613.
astippich added a comment.


  - adjust taglibwriter

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16554?vs=44612=44613

BRANCH
  mimetype_lists

REVISION DETAIL
  https://phabricator.kde.org/D16554

AFFECTED FILES
  src/extractors/epubextractor.cpp
  src/extractors/epubextractor.h
  src/extractors/exiv2extractor.cpp
  src/extractors/exiv2extractor.h
  src/extractors/ffmpegextractor.cpp
  src/extractors/ffmpegextractor.h
  src/extractors/odfextractor.cpp
  src/extractors/odfextractor.h
  src/extractors/office2007extractor.cpp
  src/extractors/office2007extractor.h
  src/extractors/plaintextextractor.cpp
  src/extractors/plaintextextractor.h
  src/extractors/poextractor.cpp
  src/extractors/poextractor.h
  src/extractors/popplerextractor.cpp
  src/extractors/popplerextractor.h
  src/extractors/taglibextractor.cpp
  src/extractors/taglibextractor.h
  src/writers/taglibwriter.cpp
  src/writers/taglibwriter.h

To: astippich, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16563: Remove unused variables

[Albert Astals Cid]

aacid updated this revision to Diff 44617.
aacid added a comment.


  actually use the variables

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16563?vs=44586=44617

BRANCH
  arcpatch-D16563

REVISION DETAIL
  https://phabricator.kde.org/D16563

AFFECTED FILES
  src/engine/transaction.cpp

To: aacid, #baloo, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16563: Actually use fileNameTerms and xAttrTerms

[Albert Astals Cid]

aacid updated this revision to Diff 44618.
aacid retitled this revision from "Remove unused variables" to "Actually use 
fileNameTerms and xAttrTerms".
aacid added a comment.


  update summary

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16563?vs=44617=44618

BRANCH
  arcpatch-D16563

REVISION DETAIL
  https://phabricator.kde.org/D16563

AFFECTED FILES
  src/engine/transaction.cpp

To: aacid, #baloo, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16563: Actually use fileNameTerms and xAttrTerms

[Albert Astals Cid]

aacid edited the summary of this revision.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D16563

To: aacid, #baloo, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16490: [KFileMetaData] Add unittest for XML extractor

[Alexander Stippich]

astippich added a comment.


  In D16490#351799 , @bruns wrote:
  
  > In D16490#351662 , @astippich 
wrote:
  >
  > > Only one minor thing: please also check that the mimetype is in the list 
of supported mimetypes
  >
  >
  > This can actually happen and is completely valid, due to mimetype 
inheritance.
  >
  > So the check would be `for supported in supportedMimetypes { if 
QMimeType(input->mimeType()).inherits(supported) return true; }; return false`. 
But this is already done from the calling code ...
  
  
  Hmmm, I don't understand. When I change the code to return an empty 
stringlist of supported mimetypes for the xmlextractor, the tests still pass.
  This should imho be covered by the tests.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16490

To: bruns, #frameworks, astippich
Cc: lbeltrame, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D16489: [KFileMetaData] Add extractor for generic XML and SVG

[Alexander Stippich]

astippich requested changes to this revision.
This revision now requires changes to proceed.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16489

To: bruns, #frameworks, astippich
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16588: Change package manager icons to emblems

[Noah Davis]

ndavis edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

REVISION DETAIL
  https://phabricator.kde.org/D16588

To: ndavis, #vdg
Cc: #vdg, kde-frameworks-devel, michaelh, ngraham, bruns

D16490: [KFileMetaData] Add unittest for XML extractor

[Stefan Brüns]

bruns added a comment.


  In D16490#351935 , @astippich 
wrote:
  
  > In D16490#351799 , @bruns wrote:
  >
  > > In D16490#351662 , @astippich 
wrote:
  > >
  > > > Only one minor thing: please also check that the mimetype is in the 
list of supported mimetypes
  > >
  > >
  > > This can actually happen and is completely valid, due to mimetype 
inheritance.
  > >
  > > So the check would be `for supported in supportedMimetypes { if 
QMimeType(input->mimeType()).inherits(supported) return true; }; return false`. 
But this is already done from the calling code ...
  >
  >
  > Hmmm, I don't understand. When I change the code to return an empty 
stringlist of supported mimetypes for the xmlextractor, the tests still pass.
  >  This should imho be covered by the tests.
  
  
  This is one level above these tests. The surrounding code ensures the right 
extractor is called for each file, see 
`ExtractorCollection::fetchExtractors(...)`.
  
  These are unit tests. The test itself is responsible to call an extractor 
with a suitable file and a matching mimetype.
  
  What you are calling for are system tests.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16490

To: bruns, #frameworks, astippich
Cc: lbeltrame, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D16554: move supported mimetypes to static string list

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> epubextractor.cpp:44
>  {
> +const QStringList EPubExtractor::mMimetypes = {
> +QStringLiteral("application/epub+zip"),

The idea behind using anonymous namespaces is to neither "pollute" the class 
namespace nor the global namespace. Remove the "EPubExtractor::" qualifier, see 
`fetchMetadata`.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16554

To: astippich, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16588: Change package manager icons to emblems

[Noah Davis]

ndavis created this revision.
ndavis added a reviewer: VDG.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
ndavis requested review of this revision.

REVISION SUMMARY
  The old package-* icons were hard to read and straddled the line between 
being 
  symbols for actions and package properties. I have changed them to emblems 
and 
  change the style to match. The new style should be much easier to read. I have
  Also dropped the package-supported icon since each distro may prefer to use 
  their own logo. Currently, only Ubuntu uses that icon and it uses its logo.

REPOSITORY
  R266 Breeze Icons

BRANCH
  package-manager-icons (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D16588

AFFECTED FILES
  icons-dark/actions/16/installed.svg
  icons-dark/actions/16/newer.svg
  icons-dark/actions/16/noninstalled.svg
  icons-dark/actions/16/outdated.svg
  icons-dark/actions/16/package-available-locked.svg
  icons-dark/actions/16/package-available.svg
  icons-dark/actions/16/package-broken.svg
  icons-dark/actions/16/package-downgrade.svg
  icons-dark/actions/16/package-install.svg
  icons-dark/actions/16/package-installed-locked.svg
  icons-dark/actions/16/package-installed-outdated.svg
  icons-dark/actions/16/package-installed-updated.svg
  icons-dark/actions/16/package-new.svg
  icons-dark/actions/16/package-purge.svg
  icons-dark/actions/16/package-reinstall.svg
  icons-dark/actions/16/package-remove.svg
  icons-dark/actions/16/package-supported.svg
  icons-dark/actions/16/package-upgrade.svg
  icons-dark/emblems/16/installed.svg
  icons-dark/emblems/16/newer.svg
  icons-dark/emblems/16/noninstalled.svg
  icons-dark/emblems/16/outdated.svg
  icons-dark/emblems/16/package-available-locked.svg
  icons-dark/emblems/16/package-available.svg
  icons-dark/emblems/16/package-broken.svg
  icons-dark/emblems/16/package-downgrade.svg
  icons-dark/emblems/16/package-install-auto.svg
  icons-dark/emblems/16/package-install.svg
  icons-dark/emblems/16/package-installed-locked.svg
  icons-dark/emblems/16/package-installed-outdated.svg
  icons-dark/emblems/16/package-installed-updated.svg
  icons-dark/emblems/16/package-new.svg
  icons-dark/emblems/16/package-purge.svg
  icons-dark/emblems/16/package-reinstall.svg
  icons-dark/emblems/16/package-remove-auto.svg
  icons-dark/emblems/16/package-remove.svg
  icons-dark/emblems/16/package-upgrade-auto.svg
  icons-dark/emblems/16/package-upgrade.svg
  icons-dark/emblems/16/unrequired.svg
  icons/actions/16/installed.svg
  icons/actions/16/newer.svg
  icons/actions/16/noninstalled.svg
  icons/actions/16/outdated.svg
  icons/actions/16/package-available-locked.svg
  icons/actions/16/package-available.svg
  icons/actions/16/package-broken.svg
  icons/actions/16/package-downgrade.svg
  icons/actions/16/package-install.svg
  icons/actions/16/package-installed-locked.svg
  icons/actions/16/package-installed-outdated.svg
  icons/actions/16/package-installed-updated.svg
  icons/actions/16/package-new.svg
  icons/actions/16/package-purge.svg
  icons/actions/16/package-reinstall.svg
  icons/actions/16/package-remove.svg
  icons/actions/16/package-supported.svg
  icons/actions/16/package-upgrade.svg
  icons/actions/16/unrequired.svg
  icons/emblems/16/installed.svg
  icons/emblems/16/newer.svg
  icons/emblems/16/noninstalled.svg
  icons/emblems/16/outdated.svg
  icons/emblems/16/package-available-locked.svg
  icons/emblems/16/package-available.svg
  icons/emblems/16/package-broken.svg
  icons/emblems/16/package-downgrade.svg
  icons/emblems/16/package-install-auto.svg
  icons/emblems/16/package-install.svg
  icons/emblems/16/package-installed-locked.svg
  icons/emblems/16/package-installed-outdated.svg
  icons/emblems/16/package-installed-updated.svg
  icons/emblems/16/package-new.svg
  icons/emblems/16/package-purge.svg
  icons/emblems/16/package-reinstall.svg
  icons/emblems/16/package-remove-auto.svg
  icons/emblems/16/package-remove.svg
  icons/emblems/16/package-upgrade-auto.svg
  icons/emblems/16/package-upgrade.svg
  icons/emblems/16/unrequired.svg

To: ndavis, #vdg
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16588: Change package manager icons to emblems

[Noah Davis]

ndavis added a subscriber: VDG.

REPOSITORY
  R266 Breeze Icons

REVISION DETAIL
  https://phabricator.kde.org/D16588

To: ndavis, #vdg
Cc: #vdg, kde-frameworks-devel, michaelh, ngraham, bruns

D16588: Change package manager icons to emblems

[Noah Davis]

ndavis edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

REVISION DETAIL
  https://phabricator.kde.org/D16588

To: ndavis, #vdg
Cc: #vdg, kde-frameworks-devel, michaelh, ngraham, bruns

D16563: Actually use fileNameTerms and xAttrTerms

[Stefan Brüns]

bruns accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R293 Baloo

BRANCH
  arcpatch-D16563

REVISION DETAIL
  https://phabricator.kde.org/D16563

To: aacid, #baloo, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

KDE CI: Frameworks » kwayland » kf5-qt5 SUSEQt5.10 - Build # 95 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kwayland/job/kf5-qt5%20SUSEQt5.10/95/
 Project:
kf5-qt5 SUSEQt5.10
 Date of build:
Thu, 01 Nov 2018 15:34:57 +
 Build duration:
14 min and counting
   JUnit Tests
  Name: (root) Failed: 3 test(s), Passed: 43 test(s), Skipped: 0 test(s), Total: 46 test(s)Failed: TestSuite.kwayland-testDataDeviceFailed: TestSuite.kwayland-testPlasmaVirtualDesktopFailed: TestSuite.kwayland-testPlasmaWindowModel
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report63%
(5/8)91%
(234/257)91%
(234/257)85%
(24795/29126)52%
(9846/18779)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests.client98%
(41/42)98%
(41/42)97%
(11679/12056)49%
(6205/12589)autotests.server100%
(5/5)100%
(5/5)99%
(353/356)49%
(169/344)src.client97%
(71/73)97%
(71/73)83%
(5733/6934)64%
(1506/2370)src.compat100%
(2/2)100%
(2/2)100%
(81/81)100%
(0/0)src.server99%
(115/116)99%
(115/116)85%
(6949/8190)64%
(1966/3061)src.tools0%
(0/2)0%
(0/2)0%
(0/693)0%
(0/272)src.tools.testserver0%
(0/3)0%
(0/3)0%
(0/69)0%
(0/10)tests0%
(0/14)0%
(0/14)0%
(0/747)0%
(0/133)

D16415: Creating new syntax highlighting file for Job Control Language (JCL)

[Christoph Cullmann]

cullmann accepted this revision.
cullmann added a comment.


  nice, thanks, will merge this!

REPOSITORY
  R216 Syntax Highlighting

REVISION DETAIL
  https://phabricator.kde.org/D16415

To: phily, #framework_syntax_highlighting, dhaumann, cullmann
Cc: cullmann, dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, ngraham, 
bruns, demsking, sars

D16416: z/OS CLIST file syntax highlighting

[Christoph Cullmann]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R216:185b54827ab0: z/OS CLIST file syntax highlighting 
(authored by phily, committed by cullmann).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D16416?vs=44278=44649#toc

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16416?vs=44278=44649

REVISION DETAIL
  https://phabricator.kde.org/D16416

AFFECTED FILES
  autotests/folding/test.clist.fold
  autotests/html/test.clist.html
  autotests/input/test.clist
  autotests/reference/test.clist.ref
  data/syntax/clist.xml

To: phily, #framework_syntax_highlighting, dhaumann, cullmann
Cc: cullmann, dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, ngraham, 
bruns, demsking, sars

D16415: Creating new syntax highlighting file for Job Control Language (JCL)

[Christoph Cullmann]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R216:a2032c52bf1b: Creating new syntax highlighting file for 
Job Control Language (JCL) (authored by phily, committed by cullmann).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D16415?vs=44457=44648#toc

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16415?vs=44457=44648

REVISION DETAIL
  https://phabricator.kde.org/D16415

AFFECTED FILES
  autotests/folding/test.jcl.fold
  autotests/html/test.jcl.html
  autotests/input/test.jcl
  autotests/reference/test.jcl.ref
  data/syntax/jcl.xml

To: phily, #framework_syntax_highlighting, dhaumann, cullmann
Cc: cullmann, dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, ngraham, 
bruns, demsking, sars

D16579: Musepack disk number field name is DISC.

[Stefan Brüns]

bruns requested changes to this revision.
bruns added a comment.
This revision now requires changes to proceed.


  In D16579#352218 , @smithjd wrote:
  
  > In D16579#352191 , @astippich 
wrote:
  >
  > > Yes, maybe it's not widely used, but it is used. Kodi for example 
supports both reading from DISCNUMBER and DISC. If you use kid3 to edit the 
metadata of ape tags, the standard behavior is to actually to write to 
DISCNUMBER (and similar to ALBUMARTIST).
  > >  One thing I've learned when I digged into metadata of audio files is 
that there is no standard, and KFileMetaData should handle as much cases as 
possible. Since it is easy to query both, please add it.
  >
  >
  > kid3 allows arbitrary field names and the APE tag field names for DISC and 
ALBUM ARTIST aren't there by default. The corresponding tags for id3 are there 
and can be applied to APE tags, but they aren't widely used as valid APE fields.
  
  
  Even if not used widely, they are still used. There is no drawback supporting 
both, save a few lines of code.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16579

To: smithjd, astippich, bruns
Cc: bruns, astippich, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
spoorun, ngraham, abrahams

D16490: [XmlExtractor] Add unittest for XML extractor

[Alexander Stippich]

astippich added a comment.


  I understand that ExtractorCollection is not used, and that the extractor 
gets the mimetype directly set in the test.
  But the list retrieved via XmlExtractor::mimetypes() is part of the extractor 
itself, isn't it? So imho that should be tested here.
  All I'm asking for is something similar to 
https://phabricator.kde.org/source/kfilemetadata/browse/master/autotests/embeddedimagedatatest.cpp$48

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16490

To: bruns, #frameworks, astippich
Cc: lbeltrame, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D16579: Musepack disk number field name is DISC.

[James Smith]

smithjd added a comment.


  I would also argue that accepting values from tag field names that have 
identically-purposed, widely-acceptable alternatives is irresponsible. Changing 
your tags to meet the standard then is a more viable course of action.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16579

To: smithjd, astippich, bruns
Cc: bruns, astippich, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
spoorun, ngraham, abrahams

D16489: [KFileMetaData] Add extractor for generic XML and SVG

[Stefan Brüns]

bruns marked 2 inline comments as done.
bruns added inline comments.

INLINE COMMENTS

> astippich wrote in xmlextractor.cpp:24
> seems unused

qCDebug(...) ...

> xmlextractor.cpp:48
> +} else if (node.localName() == QLatin1String("text")) {
> +qCDebug(KFILEMETADATA_LOG) << node.text();
> +result->append(node.text());

^ here

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16489

To: bruns, #frameworks, astippich
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16591: [XmlExtractor] Use QXmlStreamReader for better performance

[Stefan Brüns]

bruns created this revision.
bruns added reviewers: Frameworks, astippich.
Herald added projects: Frameworks, Baloo.
Herald added subscribers: Baloo, kde-frameworks-devel.
bruns requested review of this revision.

REVISION SUMMARY
  At least in the benchmark, QXmlStreamReader is more than twice as fast
  as QDomDocument.

TEST PLAN
  Before/after:
  RESULT : XmlExtractorTests::benchMarkXmlExtractor():
  
810 msecs per iteration (total: 810, iterations: 1)
  
  RESULT : XmlExtractorTests::benchMarkXmlExtractor():
  
288 msecs per iteration (total: 288, iterations: 1)

REPOSITORY
  R286 KFileMetaData

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D16591

AFFECTED FILES
  autotests/xmlextractortest.cpp
  src/extractors/xmlextractor.cpp

To: bruns, #frameworks, astippich
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16563: Actually use fileNameTerms and xAttrTerms

[Albert Astals Cid]

aacid closed this revision.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D16563

To: aacid, #baloo, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16554: move supported mimetypes to static string list

[Alexander Stippich]

astippich added inline comments.

INLINE COMMENTS

> bruns wrote in epubextractor.cpp:44
> The idea behind using anonymous namespaces is to neither "pollute" the class 
> namespace nor the global namespace. Remove the "EPubExtractor::" qualifier, 
> see `fetchMetadata`.

All the stringlists are currently implemented as a private static member of the 
class... would you like to change that to a local static variable?

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16554

To: astippich, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16415: Creating new syntax highlighting file for Job Control Language (JCL)

[Christoph Cullmann]

cullmann added a comment.


  Merged ;=)
  
  Git commit a2032c52bf1b5ced774c2f58994aaeb4d233f25a by Christoph Cullmann, on 
behalf of Phil Young.
  Committed on 01/11/2018 at 15:54.
  Pushed by cullmann into branch 'master'.
  
  Creating new syntax highlighting file for Job Control Language (JCL)
  
  Differential Revision: https://phabricator.kde.org/D16415
  
  A  +43   -0autotests/folding/test.jcl.fold
  A  +50   -0autotests/html/test.jcl.html
  A  +43   -0autotests/input/test.jcl
  A  +43   -0autotests/reference/test.jcl.ref
  A  +170  -0data/syntax/jcl.xml
  
  
https://commits.kde.org/syntax-highlighting/a2032c52bf1b5ced774c2f58994aaeb4d233f25a

REPOSITORY
  R216 Syntax Highlighting

REVISION DETAIL
  https://phabricator.kde.org/D16415

To: phily, #framework_syntax_highlighting, dhaumann, cullmann
Cc: cullmann, dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, ngraham, 
bruns, demsking, sars

D16579: Musepack disk number field name is DISC.

[James Smith]

smithjd added a comment.


  In D16579#352191 , @astippich 
wrote:
  
  > In D16579#352147 , @smithjd 
wrote:
  >
  > > In D16579#351910 , @astippich 
wrote:
  > >
  > > > The ape tag tests fail with this patch, but the test is actually wrong 
in that regard. It tests for an empty disc number, which I haven't noticed 
before.
  > > >  I've found references to both DISCNUMBER and DISC, so the safest way 
is probably to check both.
  > > >  So please query both tags like it is already done for the album artist 
and adjust the taglibextractortest.
  > >
  > >
  > > DISCNUMBER doesn't seem to be a valid (or widely used) field for APEv2.
  > >
  > > More (Picard) information: https://picard.musicbrainz.org/docs/mappings
  > >
  > > Also it appears Picard, Puddletag and ffmpeg use 'Album Artist', not 
'ALBUMARTIST'. And the link I provided 
(https://wiki.hydrogenaud.io/index.php?title=Tag_Mapping) also lists 'Album 
Artist' not 'ALBUMARTIST'. The unit test file has an 'ALBUMARTIST' field that 
should be changed to 'Album Artist'.
  >
  >
  > Yes, maybe it's not widely used, but it is used. Kodi for example supports 
both reading from DISCNUMBER and DISC. If you use kid3 to edit the metadata of 
ape tags, the standard behavior is to actually to write to DISCNUMBER (and 
similar to ALBUMARTIST).
  >  One thing I've learned when I digged into metadata of audio files is that 
there is no standard, and KFileMetaData should handle as much cases as 
possible. Since it is easy to query both, please add it.
  
  
  kid3 allows arbitrary field names and the APE tag field names for DISC and 
ALBUM ARTIST aren't there by default. The corresponding tags for id3 are there 
and can be applied to APE tags, but they aren't widely used as valid APE fields.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16579

To: smithjd, astippich
Cc: astippich, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, spoorun, 
ngraham, bruns, abrahams

D16591: [XmlExtractor] Use QXmlStreamReader for better performance

[Stefan Brüns]

bruns added a dependency: D16490: [XmlExtractor] Add unittest for XML extractor.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16591

To: bruns, #frameworks, astippich
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16490: [XmlExtractor] Add unittest for XML extractor

[Stefan Brüns]

bruns added a dependent revision: D16591: [XmlExtractor] Use QXmlStreamReader 
for better performance.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16490

To: bruns, #frameworks, astippich
Cc: lbeltrame, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D16490: [XmlExtractor] Add unittest for XML extractor

[Stefan Brüns]

bruns retitled this revision from "[KFileMetaData] Add unittest for XML 
extractor" to "[XmlExtractor] Add unittest for XML extractor".

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16490

To: bruns, #frameworks, astippich
Cc: lbeltrame, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D16489: [KFileMetaData] Add extractor for generic XML and SVG

[Alexander Stippich]

astippich accepted this revision.
astippich added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> bruns wrote in xmlextractor.cpp:24
> qCDebug(...) ...

I think that gets pulled in via "kfilemetadata_debug.h", but nevermind

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16489

To: bruns, #frameworks, astippich
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16490: [XmlExtractor] Add unittest for XML extractor

[Stefan Brüns]

bruns added a comment.


  In D16490#352244 , @astippich 
wrote:
  
  > I understand that ExtractorCollection is not used, and that the extractor 
gets the mimetype directly set in the test.
  >  But the list retrieved via XmlExtractor::mimetypes() is part of the 
extractor itself, isn't it? So imho that should be tested here.
  >  All I'm asking for is something similar to 
https://phabricator.kde.org/source/kfilemetadata/browse/master/autotests/embeddedimagedatatest.cpp$48
  
  
  For very specialized media types, this works out ok, but this is not correct 
in general:
  
  Assume you have a filetype which is a specialization of some other, supported 
mime type. This specialization is not in the mimetypes list, but can still be 
handled by the extractor.
  
  For XML, you will not be able to add every  specialized mimetype. SVG, which 
is a specialization, is listed specifically because it has specific supporting 
code, but every other XML variant can be handled by the generic code.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16490

To: bruns, #frameworks, astippich
Cc: lbeltrame, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D16579: Musepack disk number field name is DISC.

[Alexander Stippich]

astippich added a comment.


  In D16579#352255 , @smithjd wrote:
  
  > I would also argue that accepting values from tag field names that have 
identically-purposed, widely-acceptable alternatives is irresponsible. Changing 
your tags to meet the standard then is a more viable course of action.
  
  
  In an ideal world, yes of course. But it is just not going to happen that 
every user changes its tags according to the "standard" because KFileMetaData 
decided to stick to it. Instead, we get complaints or bug reports about it.
  Think about it from an end-user perspective, who doesn't know anything about 
the specific tags, and just wants to edit the tags of a music file. One opens 
kid3, clicks the 'Add' button, starts typing 'disc...' and kid3 then proposes 
'Disc Number'. The user uses it and wonders why it doesn't show up in Dolphin 
afterwards and complains.
  
  You can also argue that it's actually not a standard, it's an agreement. Both 
ape and vorbis tags allow to write arbitrary tags to the file. From that 
perspective, both are actually valid entries. Since it really is a matter of 3 
lines of code, please just add it.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16579

To: smithjd, astippich, bruns
Cc: bruns, astippich, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
spoorun, ngraham, abrahams

D16490: [XmlExtractor] Add unittest for XML extractor

[Alexander Stippich]

astippich accepted this revision.
astippich added a comment.
This revision is now accepted and ready to land.


  Alright, you convinced me.

REPOSITORY
  R286 KFileMetaData

BRANCH
  xml_extractor

REVISION DETAIL
  https://phabricator.kde.org/D16490

To: bruns, #frameworks, astippich
Cc: lbeltrame, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D16579: Musepack disk number field name is DISC.

[James Smith]

smithjd added a comment.


  In D16579#351910 , @astippich 
wrote:
  
  > The ape tag tests fail with this patch, but the test is actually wrong in 
that regard. It tests for an empty disc number, which I haven't noticed before.
  >  I've found references to both DISCNUMBER and DISC, so the safest way is 
probably to check both.
  >  So please query both tags like it is already done for the album artist and 
adjust the taglibextractortest.
  
  
  DISCNUMBER doesn't seem to be a valid (or widely used) field for APEv2.
  
  More (Picard) information: https://picard.musicbrainz.org/docs/mappings
  
  Also it appears Picard, Puddletag and ffmpeg use 'Album Artist', not 
'ALBUMARTIST'. And the link I provided 
(https://wiki.hydrogenaud.io/index.php?title=Tag_Mapping) also lists 'Album 
Artist' not 'ALBUMARTIST'. The unit test file has an 'ALBUMARTIST' field that 
should be changed to 'Album Artist'.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16579

To: smithjd, astippich
Cc: astippich, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, spoorun, 
ngraham, bruns, abrahams

D16579: Musepack disk number field name is DISC.

[James Smith]

smithjd updated this revision to Diff 44638.
smithjd added a comment.


  - Change the unit test to check if the APE 'disc' value is correct.

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16579?vs=44607=44638

BRANCH
  master-musepackFixes (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D16579

AFFECTED FILES
  autotests/taglibextractortest.cpp
  src/extractors/taglibextractor.cpp

To: smithjd, astippich
Cc: astippich, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, spoorun, 
ngraham, bruns, abrahams

D16579: Musepack disk number field name is DISC.

[Stefan Brüns]

bruns added a comment.


  In D16579#352406 , @smithjd wrote:
  
  > In D16579#352282 , @bruns wrote:
  >
  > > In D16579#352279 , @smithjd 
wrote:
  > >
  > > > I would instead recommend a tag editor that properly tags APE files, 
such as puddletag. APE-using formats are less mainstream than id3 using 
formats. Users with APE-using formats usually know WHY they use their format of 
choice, and will know there are potential support shortfalls such as 
non-compliant tagging software, or software that allows to circumvent the 
accepted tagging standard, and will know how to mitigate or avoid such issues. 
I must also point out that stricter tag field adherence is better for APE-using 
formats in particular, and is better for all tag-using formats in general.
  > >
  > >
  > > Good luck "educating" every APE user, they will really like to be told 
they should do what you consider right.
  >
  >
  > ...and we should probably rip out *EVERY* other format except for mp3 with 
its larger installed base and because it's standard fields are more supported 
by other software.
  >
  > This is a *fix* for APE tag support, not an argument over why there are 
*de-facto* standard tag fields or why certain users can screw up with powerful 
software, or why standardization is a problem and goal, especially in a tag 
format that permits arbitrary field names. There are 2 sources attached to this 
review and anecdotal evidence as to what a number of tag programs can or do 
write into APE tags. This is enough for an educated guess as to a broad 
consensus on standardized APE field names.
  >
  > I'm also not in favour of compatibility with alternatives to the de-facto 
APE field names. We should not be encouraging alternatives to well-established, 
documented field names. It's also potential burden on the writers of other 
software and also on the users of APE tags, and detrimental to the APE-using 
format's ecosystem.
  
  
  Ever heard of:
  
  > Be conservative in what you do, be liberal in what you accept from others
  
  You deliberately break other users files. The only consensus you have is with 
yourself.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16579

To: smithjd, astippich, bruns
Cc: bruns, astippich, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
spoorun, ngraham, abrahams

D16554: move supported mimetypes to static string list

[Stefan Brüns]

bruns accepted this revision.
bruns added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> taglibextractor.cpp:71
> +QStringLiteral("audio/x-vorbis+ogg"),
> +QStringLiteral("audio/wav"),
> +QStringLiteral("audio/x-wavpack"),

w < x

REPOSITORY
  R286 KFileMetaData

BRANCH
  mimetype_lists

REVISION DETAIL
  https://phabricator.kde.org/D16554

To: astippich, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16594: Add context to kcmodule connection to lambdas

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> kcmoduleqml.cpp:74
>  setButtons((KCModule::Buttons)(int)d->configModule->buttons());
> -connect(configModule, ::ConfigModule::buttonsChanged, [=] {
> +connect(configModule, ::ConfigModule::buttonsChanged, this, 
> [=] {
>  setButtons((KCModule::Buttons)(int)d->configModule->buttons());

`this` is desctructed after `d` has been deleted, so the signal may be still 
connected after d is deleted.

Does `disconnect(configModule, 0, this, 0)` work, when called in the destructor?

REPOSITORY
  R295 KCMUtils

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D16594

To: davidedmundson, #plasma, broulik
Cc: bruns, kde-frameworks-devel, michaelh, ngraham

D16579: Musepack disk number field name is DISC.

[James Smith]

smithjd added a comment.


  In D16579#352282 , @bruns wrote:
  
  > In D16579#352279 , @smithjd 
wrote:
  >
  > > I would instead recommend a tag editor that properly tags APE files, such 
as puddletag. APE-using formats are less mainstream than id3 using formats. 
Users with APE-using formats usually know WHY they use their format of choice, 
and will know there are potential support shortfalls such as non-compliant 
tagging software, or software that allows to circumvent the accepted tagging 
standard, and will know how to mitigate or avoid such issues. I must also point 
out that stricter tag field adherence is better for APE-using formats in 
particular, and is better for all tag-using formats in general.
  >
  >
  > Good luck "educating" every APE user, they will really like to be told they 
should do what you consider right.
  
  
  ...and we should probably rip out *EVERY* other format except for mp3 with 
its larger installed base and because it's standard fields are more supported 
by other software.
  
  This is a *fix* for APE tag support, not an argument over why there are 
*de-facto* standard tag fields or why certain users can screw up with powerful 
software, or why standardization is a problem and goal, especially in a tag 
format that permits arbitrary field names. There are 2 sources attached to this 
review and anecdotal evidence as to what a number of tag programs can or do 
write into APE tags. This is enough for an educated guess as to a broad 
consensus on standardized APE field names.
  
  I'm also not in favour of compatibility with alternatives to the de-facto APE 
field names. We should not be encouraging alternatives to well-established, 
documented field names. It's also potential burden on the writers of other 
software and also on the users of APE tags, and detrimental to the APE-using 
format's ecosystem.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16579

To: smithjd, astippich, bruns
Cc: bruns, astippich, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
spoorun, ngraham, abrahams

D16583: use qlatin1 for embedded image extractor

[Stefan Brüns]

bruns added a comment.


  Please use proper Capitalization in the Summary.

INLINE COMMENTS

> embeddedimagedata.cpp:175
>  TagLib::List lstPic;
> -if (mimeType == QStringLiteral("audio/ogg") || mimeType == 
> QStringLiteral("audio/x-vorbis+ogg")) {
> +if (mimeType == QLatin1String("audio/ogg") || mimeType == 
> QLatin1String("audio/x-vorbis+ogg")) {
>  TagLib::Ogg::Vorbis::File oggFile(, true);

can you add a line break here and below? Also add parentheses, like for 
"audio/mpeg" above.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16583

To: astippich, mgallien, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16560: create a separate test file for embedded images test

[Stefan Brüns]

bruns accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R286 KFileMetaData

BRANCH
  cover_jpg

REVISION DETAIL
  https://phabricator.kde.org/D16560

To: astippich, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16554: move supported mimetypes to static string list

[Alexander Stippich]

astippich updated this revision to Diff 44661.
astippich added a comment.


  - use local static variable

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16554?vs=44613=44661

BRANCH
  mimetype_lists

REVISION DETAIL
  https://phabricator.kde.org/D16554

AFFECTED FILES
  src/extractors/epubextractor.cpp
  src/extractors/exiv2extractor.cpp
  src/extractors/ffmpegextractor.cpp
  src/extractors/odfextractor.cpp
  src/extractors/office2007extractor.cpp
  src/extractors/plaintextextractor.cpp
  src/extractors/poextractor.cpp
  src/extractors/popplerextractor.cpp
  src/extractors/taglibextractor.cpp
  src/writers/taglibwriter.cpp

To: astippich, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16585: simplify handling of id3 tags

[Stefan Brüns]

bruns accepted this revision.
bruns added a comment.
This revision is now accepted and ready to land.


  A definitive improvement ...

REPOSITORY
  R286 KFileMetaData

BRANCH
  simplify_id3

REVISION DETAIL
  https://phabricator.kde.org/D16585

To: astippich, mgallien, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16594: Add context to kcmodule connection to lambdas

[Kai Uwe Broulik]

broulik accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R295 KCMUtils

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D16594

To: davidedmundson, #plasma, broulik
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16594: Add context to kcmodule connection to lambdas

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> bruns wrote in kcmoduleqml.cpp:74
> `this` is desctructed after `d` has been deleted, so the signal may be still 
> connected after d is deleted.
> 
> Does `disconnect(configModule, 0, this, 0)` work, when called in the 
> destructor?

Technically yes, but there's no sane scope for a code path where configModule 
is emitting anything between deleting d and this.

REPOSITORY
  R295 KCMUtils

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D16594

To: davidedmundson, #plasma, broulik
Cc: bruns, kde-frameworks-devel, michaelh, ngraham

D16249: Warn user before copy/move job if the file size exceeds the maximum possible file size in FAT32 file system(4 GB)

[Nathaniel Graham]

ngraham added a comment.


  "Format with destination drive to a filesystem type which" -> "Reformat the 
destination drive to use a filesystem that"

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D16249

To: shubham, ngraham, elvisangelaccio, #frameworks, bruns
Cc: cfeck, bruns, kde-frameworks-devel, michaelh, ngraham

D16588: Change package manager icons to emblems

[Nathaniel Graham]

ngraham added a comment.


  These are pretty darn good. They no longer have package-related iconography, 
but maybe that's okay and they never needed it in the first place. I can see 
how it would actually get in the way given how these icons are actually used.
  
  These icons are also used by Apper, which is a distro-agnostic KDE app you 
can try out to see how they look there.
  
  I'm inclined to give 'em a thumbs up, but let's see what other #VDG 
 folks think as well!

REPOSITORY
  R266 Breeze Icons

REVISION DETAIL
  https://phabricator.kde.org/D16588

To: ndavis, #vdg
Cc: ngraham, #vdg, kde-frameworks-devel, michaelh, bruns

D12045: Clean up existing documentation

[Stefan Brüns]

bruns added a comment.
Herald edited subscribers, added: kde-frameworks-devel; removed: Frameworks.


  I think these changes are a sufficient improvement to push it as is, we can 
do more fixups later.
  
  @ngraham - your opinion?

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D12045

To: michaelh, #baloo
Cc: kde-frameworks-devel, ngraham, bruns, ashaposhnikov, michaelh, astippich, 
spoorun, abrahams, #frameworks

D16249: Warn user before copy/move job if the file size exceeds the maximum possible file size in FAT32 file system(4 GB)

[Shubham]

shubham updated this revision to Diff 44688.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16249?vs=43938=44688

REVISION DETAIL
  https://phabricator.kde.org/D16249

AFFECTED FILES
  src/core/copyjob.cpp
  src/core/global.h
  src/core/job_error.cpp

To: shubham, ngraham, elvisangelaccio, #frameworks, bruns
Cc: cfeck, bruns, kde-frameworks-devel, michaelh, ngraham

D16588: Change package manager icons to emblems

[Noah Davis]

ndavis edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

REVISION DETAIL
  https://phabricator.kde.org/D16588

To: ndavis, #vdg
Cc: #vdg, kde-frameworks-devel, michaelh, ngraham, bruns

D12045: Clean up existing documentation

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  Yeah, go for it. Such a shame that Michael disappeared.

REPOSITORY
  R293 Baloo

BRANCH
  doc-cleanup

REVISION DETAIL
  https://phabricator.kde.org/D12045

To: michaelh, #baloo, ngraham
Cc: kde-frameworks-devel, ngraham, bruns, ashaposhnikov, michaelh, astippich, 
spoorun, abrahams

D16498: [KFileMetaData] Add extractor for DSC conforming (Encapsulated) Postscript

[Stefan Brüns]

bruns added a comment.


  @pino
  
  Please answer why you consider running a full blown postscript interpreter in 
an uncontrolled environment (no sandboxing, runs without user interaction) is 
better than 20 code lines of trivial text parsing.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16498

To: bruns, #frameworks, #baloo, astippich, ngraham, poboiko, pino
Cc: pino, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16554: move supported mimetypes to static string list

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> astippich wrote in epubextractor.cpp:44
> All the stringlists are currently implemented as a private static member of 
> the class... would you like to change that to a local static variable?

I consider this cleaner, yes.
We don't expose static helper functions, either.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16554

To: astippich, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16465: [KSambaShare] Make "net usershare info" parser testable

[Stefan Brüns]

bruns added a comment.


  Ping!

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D16465

To: bruns, #frameworks, broulik
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16404: [Bookmarks Runner] Cleanup tests CMakeList

[Stefan Brüns]

bruns added a comment.


  Ping!

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D16404

To: bruns, #frameworks
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16249: Warn user before copy/move job if the file size exceeds the maximum possible file size in FAT32 file system(4 GB)

[Stefan Brüns]

bruns added a comment.


  Please update the diff.

INLINE COMMENTS

> bruns wrote in job_error.cpp:1079
> "Format **with**"
> "filesystem type" or just "filesystem"
> either "a file" or better "files"

"transeferred"

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D16249

To: shubham, ngraham, elvisangelaccio, #frameworks
Cc: cfeck, bruns, kde-frameworks-devel, michaelh, ngraham

D16249: Warn user before copy/move job if the file size exceeds the maximum possible file size in FAT32 file system(4 GB)

[Stefan Brüns]

bruns requested changes to this revision.
This revision now requires changes to proceed.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D16249

To: shubham, ngraham, elvisangelaccio, #frameworks, bruns
Cc: cfeck, bruns, kde-frameworks-devel, michaelh, ngraham

D16490: [XmlExtractor] Add unittest for XML extractor

[Alexander Stippich]

astippich added a comment.


  This causes the Qt5.9 build to fail
  
  
https://build.kde.org/job/Frameworks/job/kfilemetadata/job/kf5-qt5%20SUSEQt5.9/workflow-stage/

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16490

To: bruns, #frameworks, astippich
Cc: lbeltrame, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D16498: [KFileMetaData] Add extractor for DSC conforming (Encapsulated) Postscript

[Pino Toscano]

pino added a comment.


  In D16498#352314 , @bruns wrote:
  
  > Please answer why you consider running a full blown postscript interpreter 
in an uncontrolled environment (no sandboxing, runs without user interaction) 
is better than 20 code lines of trivial text parsing.
  
  
  Please be patient, I've got other things that currently take my time, and I 
need to properly get some data to satisfty your questions.
  
  In the meanwhile, two suggestions:
  
  - please try to see also other people's point of point, and not just yours; 
if I suggested something, then it's because, according to my 
knowledge/experience (which includes also maintaining okular in the past, FYI), 
I deem it the optimal way
  - when asking for people's opinion, again, try to use a more friendly 
attitude; that will certainly help getting my attention faster, rather than 
feeling attacked because I'm expressing //technical// doubts on this solution

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16498

To: bruns, #frameworks, #baloo, astippich, ngraham, poboiko, pino
Cc: pino, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16498: [KFileMetaData] Add extractor for DSC conforming (Encapsulated) Postscript

[Stefan Brüns]

bruns added a comment.


  In D16498#352323 , @pino wrote:
  
  > In D16498#352314 , @bruns wrote:
  >
  > > Please answer why you consider running a full blown postscript 
interpreter in an uncontrolled environment (no sandboxing, runs without user 
interaction) is better than 20 code lines of trivial text parsing.
  >
  >
  > Please be patient, I've got other things that currently take my time, and I 
need to properly get some data to satisfty your questions.
  
  
  Thats completely fine for me, if you need ore time, just say so.
  
  > In the meanwhile, two suggestions:
  > 
  > - please try to see also other people's point of point, and not just yours; 
if I suggested something, then it's because, according to my 
knowledge/experience (which includes also maintaining okular in the past, FYI), 
I deem it the optimal way
  > - when asking for people's opinion, again, try to use a more friendly 
attitude; that will certainly help getting my attention faster, rather than 
feeling attacked because I'm expressing //technical// doubts on this solution
  
  Not answering can also be considered impolite. I am trying to solve a real 
problem here. Postscript files currently pose a significant problem for baloo. 
I listed //technical// reasons why libspectre/gs is **not** a good solution.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16498

To: bruns, #frameworks, #baloo, astippich, ngraham, poboiko, pino
Cc: pino, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16594: Add context to kcmodule connection to lambdas

[David Edmundson]

davidedmundson created this revision.
davidedmundson added a reviewer: Plasma.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
davidedmundson requested review of this revision.

REVISION SUMMARY
  configModule outlives KCModuleQML.
  
  BUG: 397894

TEST PLAN
  Compiled

REPOSITORY
  R295 KCMUtils

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D16594

AFFECTED FILES
  src/kcmoduleqml.cpp

To: davidedmundson, #plasma
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16579: Musepack disk number field name is DISC.

[James Smith]

smithjd added a comment.


  I would instead recommend a tag editor that properly tags APE files, such as 
puddletag. APE-using formats are less mainstream than id3 using formats. Users 
with APE-using formats usually know WHY they use their format of choice, and 
will know there are potential support shortfalls such as non-compliant tagging 
software, or software that allows to circumvent the accepted tagging standard, 
and will know how to mitigate or avoid such issues. I must also point out that 
stricter tag field adherence is better for APE-using formats in particular, and 
is better for all tag-using formats in general.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16579

To: smithjd, astippich, bruns
Cc: bruns, astippich, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
spoorun, ngraham, abrahams

D16490: [XmlExtractor] Add unittest for XML extractor

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R286:dbb34ea5039c: [XmlExtractor] Add unittest for XML 
extractor (authored by bruns).

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16490?vs=44409=44656

REVISION DETAIL
  https://phabricator.kde.org/D16490

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/samplefiles/test_with_metadata.svg
  autotests/xmlextractortest.cpp
  autotests/xmlextractortest.h

To: bruns, #frameworks, astippich
Cc: lbeltrame, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D16592: [ExtractorCollection] Add unit-test to verify only specific extractor is used

[Stefan Brüns]

bruns created this revision.
bruns added reviewers: Frameworks, astippich.
Herald added projects: Frameworks, Baloo.
Herald added subscribers: Baloo, kde-frameworks-devel.
bruns requested review of this revision.

REVISION SUMMARY
  If there is a direct match, only the matching extractor plugin should be used

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16592

AFFECTED FILES
  autotests/extractorcollectiontest.cpp

To: bruns, #frameworks, astippich
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16592: [ExtractorCollection] Add unit-test to verify only specific extractor is used

[Stefan Brüns]

bruns added a dependent revision: D16593: [ExtractorCollection] Use only best 
matching extractor plugin.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16592

To: bruns, #frameworks, astippich
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16593: [ExtractorCollection] Use only best matching extractor plugin

[Stefan Brüns]

bruns created this revision.
bruns added reviewers: Frameworks, astippich.
Herald added projects: Frameworks, Baloo.
Herald added subscribers: Baloo, kde-frameworks-devel.
bruns requested review of this revision.

REVISION SUMMARY
  If there is a direct match for a mime type, only the matching plugin(s)
  is (are) returned, inheritance is not considered. Otherwise, all matches
  for all mime ancestors are returned, even if one plugin is a much better
  match than any other.
  
  Up until now, the only extractor selected by inheritance was the plain
  text extractor, so there was one fuzzy matched plugin at most. With the
  XML extractor, the XML extractor should be preferred over the plain test
  extractor.
  
  Depends on D16592 

REPOSITORY
  R286 KFileMetaData

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D16593

AFFECTED FILES
  autotests/extractorcollectiontest.cpp
  src/extractorcollection.cpp
  src/extractorcollection.h

To: bruns, #frameworks, astippich
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16579: Musepack disk number field name is DISC.

[Stefan Brüns]

bruns added a comment.


  In D16579#352279 , @smithjd wrote:
  
  > I would instead recommend a tag editor that properly tags APE files, such 
as puddletag. APE-using formats are less mainstream than id3 using formats. 
Users with APE-using formats usually know WHY they use their format of choice, 
and will know there are potential support shortfalls such as non-compliant 
tagging software, or software that allows to circumvent the accepted tagging 
standard, and will know how to mitigate or avoid such issues. I must also point 
out that stricter tag field adherence is better for APE-using formats in 
particular, and is better for all tag-using formats in general.
  
  
  Good luck "educating" every APE user, they will really like to be told they 
should do what you consider right.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16579

To: smithjd, astippich, bruns
Cc: bruns, astippich, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
spoorun, ngraham, abrahams

D16489: [KFileMetaData] Add extractor for generic XML and SVG

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R286:9c58ae5b4800: [KFileMetaData] Add extractor for generic 
XML and SVG (authored by bruns).

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16489?vs=44645=44655

REVISION DETAIL
  https://phabricator.kde.org/D16489

AFFECTED FILES
  src/extractors/CMakeLists.txt
  src/extractors/xmlextractor.cpp
  src/extractors/xmlextractor.h

To: bruns, #frameworks, astippich
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams