D19102: Fix extraction of aspect ratio property

[Alexander Stippich]

astippich marked an inline comment as done.
astippich added inline comments.

INLINE COMMENTS

> bruns wrote in formatstrings.cpp:157
> Thats obviously just avoiding the rules. You can ask for an exception anyway.

The intention was to land the fix as soon as possible while complying to the 
rules, and fixing the i18n in the following frameworks release.

REPOSITORY
  R286 KFileMetaData

BRANCH
  ffmpeg_aspectratio

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

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

D19107: Write valid UTF8 characters without escaping.

[Thiago Macieira]

thiago added a comment.


  Much better, thanks.

REPOSITORY
  R237 KConfig

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

To: vandenoever, dfaure, arichardson, apol, #frameworks, thiago
Cc: rapiteanu, kde-frameworks-devel, michaelh, ngraham, bruns

D19035: Help build on Windows with usage of CMake Targets

[Christoph Cullmann]

cullmann added a reviewer: vkrause.
cullmann added a comment.


  I think I let Volker decide that, for non-cross compile stuff this is more or 
less a nop and I never do cross compiling ;)

REPOSITORY
  R216 Syntax Highlighting

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

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

D19163: Make it possible to show all spaces in the document

[Christoph Cullmann]

cullmann added a comment.


  I like the feature, but for the implementation: Could we alter the config to 
itself hold the enum?
  An other thing: why was the non-printable regex altered?

REPOSITORY
  R39 KTextEditor

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

To: apol, #kate
Cc: cullmann, ngraham, kwrite-devel, kde-frameworks-devel, gennad, michaelh, 
bruns, demsking, sars, dhaumann

D19001: katetextbuffer: refactor TextBuffer::save() to better separate code paths

[Christoph Cullmann]

cullmann added a comment.


  If you pass me your author email, I will push that.

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

To: mgerstner, dhaumann, cullmann, #ktexteditor, chinmoyr, fvogt
Cc: kwrite-devel, kde-frameworks-devel, gennad, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D19001: katetextbuffer: refactor TextBuffer::save() to better separate code paths

[Christoph Cullmann]

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


  Thanks, other things looks reasonable to me.

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

To: mgerstner, dhaumann, cullmann, #ktexteditor, chinmoyr, fvogt
Cc: kwrite-devel, kde-frameworks-devel, gennad, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

KDE CI: Frameworks » plasma-framework » kf5-qt5 SUSEQt5.12 - Build # 15 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/plasma-framework/job/kf5-qt5%20SUSEQt5.12/15/
 Project:
kf5-qt5 SUSEQt5.12
 Date of build:
Wed, 20 Feb 2019 04:52:43 +
 Build duration:
42 min and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yaml
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot Failed: 6 test(s), Passed: 8 test(s), Skipped: 0 test(s), Total: 14 test(s)Failed: projectroot.autotests.dialognativetestFailed: projectroot.autotests.plasma_configmodeltestFailed: projectroot.autotests.plasma_fallbackpackagetestFailed: projectroot.autotests.plasma_iconitemtestFailed: projectroot.autotests.plasma_packagestructuretestFailed: projectroot.autotests.plasma_storagetest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report33%
(6/18)36%
(45/126)36%
(45/126)27%
(3608/13330)18%
(1818/9850)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests86%
(12/14)86%
(12/14)55%
(612/1117)29%
(315/1086)src.declarativeimports.calendar0%
(0/6)0%
(0/6)0%
(0/464)0%
(0/243)src.declarativeimports.core31%
(5/16)31%
(5/16)13%
(299/2253)7%
(96/1458)src.declarativeimports.plasmacomponents0%
(0/6)0%
(0/6)0%
(0/518)0%
(0/207)src.declarativeimports.plasmaextracomponents0%
(0/3)0%
(0/3)0%
(0/42)0%
(0/22)src.declarativeimports.platformcomponents0%
(0/3)0%
(0/3)0%
(0/58)0%
(0/14)src.declarativeimports.platformcomponents.utils0%
(0/2)0%
(0/2)0%
(0/14)0%
(0/2)src.plasma64%
(14/22)64%
(14/22)40%
(1415/3501)28%
(792/2827)src.plasma.packagestructure0%
(0/7)0%
(0/7)0%
(0/134)0%
(0/12)src.plasma.private50%
(9/18)50%
(9/18)43%
(673/1570)29%
(300/1032)src.plasma.scripting0%
(0/3)0%
(0/3)0%
(0/162)0%
(0/128)src.plasmapkg0%
(0/1)0%
(0/1)0%
(0/45)0%
(0/40)src.plasmaquick33%
(4/12)33%
(4/12)29%
(578/2013)18%
(310/1713)src.plasmaquick.private50%
(1/2)50%
(1/2)29%
(31/106)36%
(5/14)src.scriptengines.qml.plasmoid0%
(0/6)0%
(0/6)0%
(0/1178)0%
(0/1028)tests.dpi0%
(0/2)0%
   

KDE CI: Frameworks » plasma-framework » kf5-qt5 SUSEQt5.10 - Build # 23 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/plasma-framework/job/kf5-qt5%20SUSEQt5.10/23/
 Project:
kf5-qt5 SUSEQt5.10
 Date of build:
Wed, 20 Feb 2019 04:52:43 +
 Build duration:
22 min and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yaml
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot Failed: 6 test(s), Passed: 8 test(s), Skipped: 0 test(s), Total: 14 test(s)Failed: projectroot.autotests.dialognativetestFailed: projectroot.autotests.plasma_configmodeltestFailed: projectroot.autotests.plasma_fallbackpackagetestFailed: projectroot.autotests.plasma_iconitemtestFailed: projectroot.autotests.plasma_packagestructuretestFailed: projectroot.autotests.plasma_storagetest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report33%
(6/18)36%
(45/126)36%
(45/126)27%
(3606/13330)18%
(1817/9850)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests86%
(12/14)86%
(12/14)55%
(610/1117)29%
(315/1086)src.declarativeimports.calendar0%
(0/6)0%
(0/6)0%
(0/464)0%
(0/243)src.declarativeimports.core31%
(5/16)31%
(5/16)13%
(299/2253)7%
(96/1458)src.declarativeimports.plasmacomponents0%
(0/6)0%
(0/6)0%
(0/518)0%
(0/207)src.declarativeimports.plasmaextracomponents0%
(0/3)0%
(0/3)0%
(0/42)0%
(0/22)src.declarativeimports.platformcomponents0%
(0/3)0%
(0/3)0%
(0/58)0%
(0/14)src.declarativeimports.platformcomponents.utils0%
(0/2)0%
(0/2)0%
(0/14)0%
(0/2)src.plasma64%
(14/22)64%
(14/22)40%
(1415/3501)28%
(792/2827)src.plasma.packagestructure0%
(0/7)0%
(0/7)0%
(0/134)0%
(0/12)src.plasma.private50%
(9/18)50%
(9/18)43%
(673/1570)29%
(299/1032)src.plasma.scripting0%
(0/3)0%
(0/3)0%
(0/162)0%
(0/128)src.plasmapkg0%
(0/1)0%
(0/1)0%
(0/45)0%
(0/40)src.plasmaquick33%
(4/12)33%
(4/12)29%
(578/2013)18%
(310/1713)src.plasmaquick.private50%
(1/2)50%
(1/2)29%
(31/106)36%
(5/14)src.scriptengines.qml.plasmoid0%
(0/6)0%
(0/6)0%
(0/1178)0%
(0/1028)tests.dpi0%
(0/2)0%
   

KDE CI: Frameworks » plasma-framework » kf5-qt5 FreeBSDQt5.12 - Build # 27 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/plasma-framework/job/kf5-qt5%20FreeBSDQt5.12/27/
 Project:
kf5-qt5 FreeBSDQt5.12
 Date of build:
Wed, 20 Feb 2019 04:52:43 +
 Build duration:
3 min 52 sec and counting
   JUnit Tests
  Name: projectroot Failed: 6 test(s), Passed: 8 test(s), Skipped: 0 test(s), Total: 14 test(s)Failed: projectroot.autotests.dialognativetestFailed: projectroot.autotests.plasma_configmodeltestFailed: projectroot.autotests.plasma_fallbackpackagetestFailed: projectroot.autotests.plasma_iconitemtestFailed: projectroot.autotests.plasma_packagestructuretestFailed: projectroot.autotests.plasma_storagetest

D18986: [breeze-icons] Add video camera icons

[Krešimir Čohar]

rooty added a comment.


  In D18986#415677 , @trickyricky26 
wrote:
  
  > I will make some minor changes to the 64px design.
  
  
  Sure thing, sorry about being lazy and not updating the screenshots 

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg, hein, ndavis, rooty
Cc: filipf, trickyricky26, ngraham, kde-frameworks-devel, michaelh, bruns

D19092: Add bison minimum version of 2.4.1 due to %code

[Kurt Hindenburg]

hindenburg added a reviewer: Frameworks.

REPOSITORY
  R309 KService

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

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

D19163: Make it possible to show all spaces in the document

[Nathaniel Graham]

ngraham added a comment.


  I have wanted this for so long!

REPOSITORY
  R39 KTextEditor

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

To: apol, #kate
Cc: ngraham, kwrite-devel, kde-frameworks-devel, gennad, michaelh, bruns, 
demsking, cullmann, sars, dhaumann

D19163: Make it possible to show all spaces in the document

[Aleix Pol Gonzalez]

apol added a comment.


  F6624272: kate-spaces.png 

REPOSITORY
  R39 KTextEditor

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

To: apol, #kate
Cc: kwrite-devel, kde-frameworks-devel, gennad, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D18853: Fix sharing files with spaces or quotes in names via Telegram

[Mikhail Rudenko]

mikhailru updated this revision to Diff 52123.
mikhailru added a comment.


  @apol, thanks for a nice suggestion!
  I somehow missed KShell::quoteArg. :)

REPOSITORY
  R495 Purpose Library

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18853?vs=51187=52123

BRANCH
  telegram-sharing-fix (branched from master)

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

AFFECTED FILES
  src/plugins/telegram/telegramplugin.cpp

To: mikhailru, apol
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D19101: Increase precision of frame rate property

[Stefan Brüns]

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

REPOSITORY
  R286 KFileMetaData

BRANCH
  ffmpeg_framerate

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

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

D19102: Fix extraction of aspect ratio property

[Stefan Brüns]

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

INLINE COMMENTS

> astippich wrote in formatstrings.cpp:157
> That was actually done deliberately to get this in for Kf 5.56 since there is 
> string freeze. But if concatenation is a no go, this has to wait

Thats obviously just avoiding the rules. You can ask for an exception anyway.

REPOSITORY
  R286 KFileMetaData

BRANCH
  ffmpeg_aspectratio

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

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

D19087: Add standalone conversion functions for PropertyMap to Json and vice versa

[Stefan Brüns]

bruns added a comment.


  In D19087#415703 , @astippich 
wrote:
  
  > The stringlist case is still not handled.
  
  
  Thats the reason I have not marked the comment as done ...
  
  > And a test case for multiple ints (2 ReleaseYear properties) will also 
likely fail. When going with JSON, I think this has to be done by always 
inserting values as an array (even if there is only one value), and removing 
this during deserialization. Stringlist will then be an array in an array in 
JSON. That will ensure that the map is of the same structure as it was put in, 
and this should also be possible to do in a backwards compatible way.
  
  
  
  > What do you think of using QDataStream here? This would require a db 
version bump, of course.
  
  Not too much:
  
  > Versioning
  > 
  > QDataStream's binary format has evolved since Qt 1.0, and is likely to 
continue evolving to reflect changes done in Qt. When inputting or outputting 
complex types, it's very important to make sure that the same version of the 
stream (version()) is used for reading and writing. If you need both forward 
and backward compatibility, you can hardcode the version number in the 
application:

REPOSITORY
  R293 Baloo

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

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

D19168: Fix crash in Dolphin when dropping trashed file in trash

[David Faure]

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


  Looks good, just some minor improvement suggestions for the unittest.

INLINE COMMENTS

> dropjobtest.cpp:361
> +KIO::CopyJob *copyJob = KIO::move(QUrl::fromLocalFile(m_srcFile), 
> QUrl(QStringLiteral("trash:/")));
> +QSignalSpy copyingDoneSpy(copyJob, 
> SIGNAL(copyingDone(KIO::Job*,QUrl,QUrl,QDateTime,bool,bool)));
> +QVERIFY(copyJob->exec());

Would be simpler with the PMF connect syntax ;)

  QSignalSpy copyingDoneSpy(copyJob, ::CopyJob::copyingDone);

> dropjobtest.cpp:376
> +// Then an error should be reported and no files action should occur
> +QVERIFY2(job->exec() == false, qPrintable(job->errorString()));
> +QVERIFY(job->error() == KIO::ERR_DROP_ON_ITSELF);

If exec() returns true, there won't be an errorString. So this line can be 
simplified to

  QVERIFY(!job->exec());

> dropjobtest.cpp:377
> +QVERIFY2(job->exec() == false, qPrintable(job->errorString()));
> +QVERIFY(job->error() == KIO::ERR_DROP_ON_ITSELF);
> +}

QVERIFY(a==b) should always be QCOMPARE(a, b) instead, to see both values in 
case of a failure.

REPOSITORY
  R241 KIO

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

To: hallas, #frameworks, elvisangelaccio, ngraham, dfaure
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D19170: Fix crash while moving files

[David Faure]

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


  If some code is deleting this job from a slot connected to it, that code 
needs to be fixed. This hack isn't a fix, it will only create more problems..
  
  E.g. if some other bad code runs the event loop in the slot connected to 
finished, it will now delete the job, and we'll get the exact same crash but on 
`if (emitResult)` this time.
  
  This is a big no from me. The emitter of a signal MUST NOT be deleted in 
slots connected to it.
  
  The bug needs proper analysis.

REPOSITORY
  R244 KCoreAddons

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

To: hallas, #frameworks, elvisangelaccio, dfaure
Cc: dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D19109: [Extractor] Add metadata to extractors

[Stefan Brüns]

bruns added a comment.


  In D19109#415710 , @astippich 
wrote:
  
  > In D19109#414968 , @bruns wrote:
  >
  > > In D19109#414758 , @astippich 
wrote:
  > >
  > > > A few general remarks:
  > > >
  > > > - I really do not like that there are two lists of supported mimetypes 
now which have to be kept in sync
  > >
  > >
  > > I think this is trivial enough. Also this is covered by the unit test.
  >
  >
  > My fear is that it is easily forgotten, but I did not see the autotest. 
Still, do you think it is feasible to generate the mimetype stringlist from the 
JSON data to remove the duplication?
  
  
  These are not completely duplicate - e.g. the officeextractor (pre-2007) uses 
runtime detection of some binary helpers. If these are not found, the list 
returned by the plugin is empty. The plugin has no direct access to its 
metadata, as it is only available from the loader and there is no possibility 
to pass it back, so it can not default to it.
  
  >>> - Do we really need versioning per mimetype? IMHO it is sufficient to 
have a version number per extractor. From my experience, fixing an extractor 
usually impacts all its supported mimetypes, and rarily affects only one 
mimetype.
  >> 
  >> Past experience tells otherwise. There have been feature extensions and 
bugfixes for specific mimetypes, just look at your own commits
  >> 
  >> - "fix ape disc number extraction"
  >> - "implement more tags for asf metadata"
  >> - ...
  >> 
  >>   I want to reduce reindexing as much as possible.
  > 
  > And I can give you examples where this was not the case :).
  
  ... which does not **prohibit** bumping the version for **all** affected 
encoders. Also, there is nothing disallowing to skip versions, e.g. if 
"foo/bar" is 2.1, and "foo/baz" is 1.3, and both get a major bump, both can be 
set to 3.0.
  
  This is also only the case because TagLibExtractor was stupidly written 
(which D18826  fixes). The other extractors 
do not have that many special codepath.
  
  > Well, I find it cumbersome to implement this fine-grained control, but 
otherwise people will probably yell because of high cpu usage...
  >  At least, I would like to group duplicated mimetypes such as audio/wav and 
audio/x-wav, but that is not possible with JSON, is it?
  
  You can reorder any aliasing mimetypes.
  
  Another question is, why do we have "audio/wav" and "audio/x-wav" in the 
first place? Are there really files where one type is a reported for one file, 
and the other for other files? Wouldn't it be better to just have the canonical 
type? At least on my computer, shared-mime-info only has audio/x-wav, listing 
audio/wav and audio/vnd.wave as aliases. Aliases should never be returned by 
QMimeDatabase.

REPOSITORY
  R286 KFileMetaData

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

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

D19168: Fix crash in Dolphin when dropping trashed file in trash

[Dominik Haumann]

dhaumann added a reviewer: dfaure.

REPOSITORY
  R241 KIO

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

To: hallas, #frameworks, elvisangelaccio, ngraham, dfaure
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D19170: Fix crash while moving files

[Dominik Haumann]

dhaumann added a reviewer: dfaure.

REPOSITORY
  R244 KCoreAddons

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

To: hallas, #frameworks, elvisangelaccio, dfaure
Cc: dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D19098: Add PropertyMap to QVariantMap conversion function using names

[Alexander Stippich]

astippich added a comment.


  In D19098#414973 , @bruns wrote:
  
  > In D19098#414729 , @astippich 
wrote:
  >
  > > It already does at two different places, because it fuses different 
information into a single QMap later on (xattr, file size etc...) 
  > >  
https://phabricator.kde.org/source/baloo-widgets/browse/master/src/extractor.cpp$65
  > >  
https://phabricator.kde.org/source/baloo-widgets/browse/master/src/filefetchjob.cpp$62
  >
  >
  > This can be done by using a KFM::PropertyMap directly, and adding property 
types for the UserMetaData (tags, comment, rating). Note, the strings returned 
by PropertyInfo::name() are not shared ...
  
  
  It is not only xattr, also everything from kfileitems{group,size,owner...}. 
Adding these as property with no users in KFileMetaData does not seem clean.
  Also, you would have to construct the properties from the name. Why not use 
the names directly then? Changing everything to a PropertyMap requires a 
rewrite of large parts, and I certainly will not rewrite baloo-widgets right 
now.
  This is just for a small cleanup.

REPOSITORY
  R286 KFileMetaData

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

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

D19109: [Extractor] Add metadata to extractors

[Alexander Stippich]

astippich added a comment.


  In D19109#414968 , @bruns wrote:
  
  > In D19109#414758 , @astippich 
wrote:
  >
  > > A few general remarks:
  > >
  > > - I really do not like that there are two lists of supported mimetypes 
now which have to be kept in sync
  >
  >
  > I think this is trivial enough. Also this is covered by the unit test.
  
  
  My fear is that it is easily forgotten, but I did not see the autotest. 
Still, do you think it is feasible to generate the mimetype stringlist from the 
JSON data to remove the duplication?
  
  >> - Do we really need versioning per mimetype? IMHO it is sufficient to have 
a version number per extractor. From my experience, fixing an extractor usually 
impacts all its supported mimetypes, and rarily affects only one mimetype.
  > 
  > Past experience tells otherwise. There have been feature extensions and 
bugfixes for specific mimetypes, just look at your own commits
  > 
  > - "fix ape disc number extraction"
  > - "implement more tags for asf metadata"
  > - ...
  > 
  >   I want to reduce reindexing as much as possible.
  
  And I can give you examples where this was not the case :). This is also only 
the case because TagLibExtractor was stupidly written (which D18826 
 fixes). The other extractors do not have 
that many special codepath.
  Well, I find it cumbersome to implement this fine-grained control, but 
otherwise people will probably yell because of high cpu usage...
  At least, I would like to group duplicated mimetypes such as audio/wav and 
audio/x-wav, but that is not possible with JSON, is it?
  
  >>   Also, this makes the list hard to maintain, also regarding file types 
which have multiple mime types, e.g. audio/wav and audio/x-wav
  >> 
  >> - Do we need an x.y version? I think a single integer is enough or what do 
you have in mind?
  > 
  > Changes only affecting failed files are minor versions, changes affecting 
already indexed files (i.e. support for new properties) get a new major version.
  > 
  >> - I prefer to directly construct the qvariantmap in the extractors, and 
re-use the mimetype list which is already available.
  > 
  > Requires changing the plugin interface. Does not allow to query extractor 
properties without fully loading the plugin (which is expensive). Read 
https://vizzzion.org/blog/2013/08/ "K_PLUGIN_FACTORY_WITH_JSON or where is the 
metadata?"
  
  Thanks.

REPOSITORY
  R286 KFileMetaData

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

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

D19170: Fix crash while moving files

[Dominik Haumann]

dhaumann added inline comments.

INLINE COMMENTS

> kjob.cpp:105
>  
> +if (isAutoDelete()) {
> +deleteLater();

Alternatively to moving the code, you could also use a `QPointer 
that(this);` to monitor whether the QObject is still existing. Then, you'd have 
to call

  if (that && isAutoDelete()) {...}

What I wonder is if calling deleteLater() earlier, would it be possible to 
somehow change order of execution?
Reasoning: deleteLater() goes through the event queue, but the signals & slots 
as well go through the event queue in case of threaded execution or when using 
Qt::QueuedConnection. So this possibly changes the order of events.

Someone who knows the code and how this internally is processed definitely 
needs to have a look at this :-)

REPOSITORY
  R244 KCoreAddons

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

To: hallas, #frameworks, elvisangelaccio
Cc: dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D19087: Add standalone conversion functions for PropertyMap to Json and vice versa

[Alexander Stippich]

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


  The stringlist case is still not handled. And a test case for multiple ints 
(2 ReleaseYear properties) will also likely fail. When going with JSON, I think 
this has to be done by always inserting values as an array (even if there is 
only one value), and removing this during deserialization. Stringlist will then 
be an array in an array in JSON. That will ensure that the map is of the same 
structure as it was put in, and this should also be possible to do in a 
backwards compatible way.
  
  What do you think of using QDataStream here? This would require a db version 
bump, of course.

INLINE COMMENTS

> astippich wrote in propertydata.cpp:83
> I think that will break all properties containing doubles, of which there are 
> a few.

Now ints are converted to doubles, which is not fatal, but IMHO also not very 
clean either. But since JSON only knows doubles, this is what we get. Only way 
out in this case is to query PropertyInfo::valueType, and do the conversion 
based on this.

REPOSITORY
  R293 Baloo

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

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

D19161: Use QTextFormat::TextUnderlineStyle instead of QTextFormat::FontUnderline

[Dominik Haumann]

dhaumann added a comment.


  Btw, searching in lxr.kde.org for FontUnderline reveals some more hits: 
https://lxr.kde.org/ident?_i=FontUnderline&_remember=1

REPOSITORY
  R39 KTextEditor

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

To: ahmadsamir, #ktexteditor, cullmann, dhaumann
Cc: kde-frameworks-devel, kwrite-devel, gennad, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D19161: Use QTextFormat::TextUnderlineStyle instead of QTextFormat::FontUnderline

[Dominik Haumann]

dhaumann added a comment.


  This patch looks good to me, even though I cannot reproduce the issue 
following the steps in https://bugs.kde.org/show_bug.cgi?id=399278.
  
  But yes, the Qt documentation says to use TextUnderlineStyle instead of 
FontUnderline. The implementation of fontUnderline() indeed also first tries to 
use TextUnderlineStyle, see: 
https://code.woboq.org/qt5/qtbase/src/gui/text/qtextformat.cpp.html#_ZNK15QTextCharFormat13fontUnderlineEv
  
  So from my side this change is OK, but probably before committing this should 
be tested again.

REPOSITORY
  R39 KTextEditor

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

To: ahmadsamir, #ktexteditor, cullmann, dhaumann
Cc: kde-frameworks-devel, kwrite-devel, gennad, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D19102: Fix extraction of aspect ratio property

[Alexander Stippich]

astippich marked an inline comment as done.
astippich added inline comments.

INLINE COMMENTS

> bruns wrote in formatstrings.cpp:157
> Proper i18n

That was actually done deliberately to get this in for Kf 5.56 since there is 
string freeze. But if concatenation is a no go, this has to wait

REPOSITORY
  R286 KFileMetaData

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

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

D19102: Fix extraction of aspect ratio property

[Alexander Stippich]

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


  - do not use QString::number

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19102?vs=52104=52114

BRANCH
  ffmpeg_aspectratio

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

AFFECTED FILES
  autotests/ffmpegextractortest.cpp
  autotests/propertyinfotest.cpp
  src/extractors/ffmpegextractor.cpp
  src/formatstrings.cpp
  src/formatstrings_p.h
  src/propertyinfo.cpp

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

D18986: [breeze-icons] Add video camera icons

[TrickyRicky]

trickyricky26 commandeered this revision.
trickyricky26 added a reviewer: rooty.
trickyricky26 added a comment.


  I will make some minor changes to the 64px design.

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg, hein, ndavis, rooty
Cc: filipf, trickyricky26, ngraham, kde-frameworks-devel, michaelh, bruns

D19101: Increase precision of frame rate property

[Alexander Stippich]

astippich marked 2 inline comments as done.

REPOSITORY
  R286 KFileMetaData

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

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

D19101: Increase precision of frame rate property

[Alexander Stippich]

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


  - do not use QString::number

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19101?vs=52103=52109

BRANCH
  ffmpeg_framerate

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

AFFECTED FILES
  autotests/ffmpegextractortest.cpp
  autotests/propertyinfotest.cpp
  src/extractors/ffmpegextractor.cpp
  src/formatstrings.cpp
  src/propertyinfo.cpp

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

D19170: Fix crash while moving files

[David Hallas]

hallas created this revision.
hallas added reviewers: Frameworks, elvisangelaccio.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
hallas requested review of this revision.

REVISION SUMMARY
  Fix crash while moving files. The backtrace points towards the KJob
  being deleted after emitting the result signal causing the subsequent
  call to isAutoDelete to crash. Instead simply call isAutoDelete and
  potentially deleteLater prior to emitting the result signal.
  
  BUG: 368287

TEST PLAN
  I haven't been able to reproduce the issue so this commit should be
  seen as a test commit to verify if this is indeed the root cause.

REPOSITORY
  R244 KCoreAddons

BRANCH
  fix_crash_on_while_moving_files (branched from master)

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

AFFECTED FILES
  src/lib/jobs/kjob.cpp

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

D19168: Fix crash in Dolphin when dropping trashed file in trash

[David Hallas]

hallas added reviewers: elvisangelaccio, ngraham.

REPOSITORY
  R241 KIO

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

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

D19168: Fix crash in Dolphin when dropping trashed file in trash

[David Hallas]

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

REVISION SUMMARY
  Fix crash in Dolphin when dropping trashed file in trash. The actual
  crash happens because of an assertion failure in kcoredirlister_p.h:308
  and this is triggered from kcoredirlister.cpp:995. What actually happens
  is that the dropjob determines that it should perform a move action
  which ends up being a rename operation for kio_trash. But it ends up
  moving the file to itself and this triggers the above crash. The
  solution is to error out in the dropjob with a KIO::ERR_DROP_ON_ITSELF
  error so that the user can see that it doesn't make sense to drop a
  file from the trash inside the trash again.
  
  BUG: 378051

TEST PLAN
  Put a file in trash
  Drag and drop the file to the trash

REPOSITORY
  R241 KIO

BRANCH
  fix_crash_when_dropping_trashed_file_in_trash (branched from master)

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

AFFECTED FILES
  autotests/dropjobtest.cpp
  src/widgets/dropjob.cpp

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

KDE CI: Frameworks » kfilemetadata » kf5-qt5 SUSEQt5.12 - Build # 21 - Still unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kfilemetadata/job/kf5-qt5%20SUSEQt5.12/21/
 Project:
kf5-qt5 SUSEQt5.12
 Date of build:
Tue, 19 Feb 2019 19:52:18 +
 Build duration:
1 min 54 sec and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yaml
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot Failed: 1 test(s), Passed: 16 test(s), Skipped: 0 test(s), Total: 17 test(s)Failed: projectroot.autotests.taglibextractortest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report80%
(4/5)88%
(50/57)88%
(50/57)83%
(3352/4024)48%
(4905/10309)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(17/17)100%
(17/17)98%
(916/935)49%
(2036/4131)src76%
(19/25)76%
(19/25)80%
(1151/1441)43%
(482/1114)src.extractors100%
(13/13)100%
(13/13)79%
(1227/1563)47%
(2316/4903)src.writers100%
(1/1)100%
(1/1)98%
(58/59)48%
(71/149)tests0%
(0/1)0%
(0/1)0%
(0/26)0%
(0/12)

KDE CI: Frameworks » kfilemetadata » kf5-qt5 SUSEQt5.10 - Build # 26 - Still unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kfilemetadata/job/kf5-qt5%20SUSEQt5.10/26/
 Project:
kf5-qt5 SUSEQt5.10
 Date of build:
Tue, 19 Feb 2019 19:52:18 +
 Build duration:
2 min 1 sec and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yaml
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot Failed: 1 test(s), Passed: 16 test(s), Skipped: 0 test(s), Total: 17 test(s)Failed: projectroot.autotests.taglibextractortest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report80%
(4/5)88%
(50/57)88%
(50/57)83%
(3352/4024)48%
(4905/10309)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(17/17)100%
(17/17)98%
(916/935)49%
(2036/4131)src76%
(19/25)76%
(19/25)80%
(1151/1441)43%
(482/1114)src.extractors100%
(13/13)100%
(13/13)79%
(1227/1563)47%
(2316/4903)src.writers100%
(1/1)100%
(1/1)98%
(58/59)48%
(71/149)tests0%
(0/1)0%
(0/1)0%
(0/26)0%
(0/12)

D19102: Fix extraction of aspect ratio property

[Stefan Brüns]

bruns requested changes to this revision.
bruns added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> formatstrings.cpp:157
> +{
> +return QString(QString::number(round(value.toDouble() * 100) / 100) + 
> QStringLiteral(":1"));
> +}

Proper i18n

REPOSITORY
  R286 KFileMetaData

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

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

D19101: Increase precision of frame rate property

[Stefan Brüns]

bruns requested changes to this revision.
bruns added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> formatstrings.cpp:152
>  {
> -return QString(value.toString() + i18nc("Symbol of frames per second, 
> with space", " fps"));
> +return QString(QString::number(round(value.toDouble() * 100) / 100) + 
> i18nc("Symbol of frames per second, with space", " fps"));
>  }

No concatenation please, see
https://techbase.kde.org/Development/Tutorials/Localization/i18n_Krazy#Number_Formatting

REPOSITORY
  R286 KFileMetaData

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

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

D19102: Fix extraction of aspect ratio property

[Alexander Stippich]

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


  - rebase
  - use exact value for test

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19102?vs=51918=52104

BRANCH
  ffmpeg_aspectratio

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

AFFECTED FILES
  autotests/ffmpegextractortest.cpp
  autotests/propertyinfotest.cpp
  src/extractors/ffmpegextractor.cpp
  src/formatstrings.cpp
  src/formatstrings_p.h
  src/propertyinfo.cpp

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

D19101: Increase precision of frame rate property

[Alexander Stippich]

astippich removed a dependency: D19100: Cleanup ffmpeg extractor.

REPOSITORY
  R286 KFileMetaData

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

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

D19100: Cleanup ffmpeg extractor

[Alexander Stippich]

astippich removed a dependent revision: D19101: Increase precision of frame 
rate property.

REPOSITORY
  R286 KFileMetaData

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

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

D19101: Increase precision of frame rate property

[Alexander Stippich]

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


  - rebase on master
  - use exact value in test

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19101?vs=51917=52103

BRANCH
  ffmpeg_framerate

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

AFFECTED FILES
  autotests/ffmpegextractortest.cpp
  autotests/propertyinfotest.cpp
  src/extractors/ffmpegextractor.cpp
  src/formatstrings.cpp
  src/propertyinfo.cpp

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

KDE CI: Frameworks » kfilemetadata » kf5-qt5 SUSEQt5.10 - Build # 25 - Still Failing!

[CI System]

BUILD FAILURE
 Build URL
https://build.kde.org/job/Frameworks/job/kfilemetadata/job/kf5-qt5%20SUSEQt5.10/25/
 Project:
kf5-qt5 SUSEQt5.10
 Date of build:
Tue, 19 Feb 2019 19:21:03 +
 Build duration:
3 min 55 sec and counting
   CONSOLE OUTPUT
  [...truncated 565 lines...][2019-02-19T19:24:21.487Z]  ^[2019-02-19T19:24:21.487Z] In file included from /home/jenkins/workspace/Frameworks/kfilemetadata/kf5-qt5 SUSEQt5.10/src/extractors/popplerextractor.h:25,[2019-02-19T19:24:21.487Z]  from /home/jenkins/workspace/Frameworks/kfilemetadata/kf5-qt5 SUSEQt5.10/src/extractors/popplerextractor.cpp:22:[2019-02-19T19:24:21.487Z] /usr/include/poppler/qt5/poppler-qt5.h:1965:52: note: declared here[2019-02-19T19:24:21.487Z]  POPPLER_QT5_EXPORT Q_DECL_DEPRECATED QDateTime convertDate( char *dateString );[2019-02-19T19:24:21.487Z] ^~~[2019-02-19T19:24:21.988Z] [ 48%] Building CXX object autotests/CMakeFiles/popplerextractortest.dir/popplerextractortest_autogen/mocs_compilation.cpp.o[2019-02-19T19:24:23.664Z] [ 49%] Linking CXX executable ../bin/popplerextractortest[2019-02-19T19:24:24.164Z] [ 49%] Built target popplerextractortest[2019-02-19T19:24:24.164Z] Scanning dependencies of target usermetadatawritertest[2019-02-19T19:24:24.164Z] [ 50%] Building CXX object autotests/CMakeFiles/usermetadatawritertest.dir/usermetadatawritertest.cpp.o[2019-02-19T19:24:26.509Z] [ 51%] Building CXX object autotests/CMakeFiles/usermetadatawritertest.dir/__/src/usermetadata.cpp.o[2019-02-19T19:24:28.172Z] [ 51%] Building CXX object autotests/CMakeFiles/usermetadatawritertest.dir/usermetadatawritertest_autogen/mocs_compilation.cpp.o[2019-02-19T19:24:28.673Z] [ 52%] Linking CXX executable ../bin/usermetadatawritertest[2019-02-19T19:24:29.174Z] [ 52%] Built target usermetadatawritertest[2019-02-19T19:24:29.174Z] Scanning dependencies of target propertyinfotest[2019-02-19T19:24:29.174Z] Scanning dependencies of target kfilemetadata_taglibwriter[2019-02-19T19:24:29.174Z] [ 53%] Building CXX object autotests/CMakeFiles/propertyinfotest.dir/propertyinfotest.cpp.o[2019-02-19T19:24:29.174Z] [ 53%] Building CXX object src/writers/CMakeFiles/kfilemetadata_taglibwriter.dir/taglibwriter.cpp.o[2019-02-19T19:24:30.239Z] Scanning dependencies of target epubextractortest[2019-02-19T19:24:30.755Z] [ 53%] Building CXX object autotests/CMakeFiles/epubextractortest.dir/epubextractortest.cpp.o[2019-02-19T19:24:30.755Z] Scanning dependencies of target kfilemetadata_officeextractor[2019-02-19T19:24:30.755Z] [ 54%] Building CXX object src/extractors/CMakeFiles/kfilemetadata_officeextractor.dir/officeextractor.cpp.o[2019-02-19T19:24:31.849Z] [ 55%] Building CXX object src/writers/CMakeFiles/kfilemetadata_taglibwriter.dir/kfilemetadata_taglibwriter_autogen/mocs_compilation.cpp.o[2019-02-19T19:24:31.849Z] Scanning dependencies of target kfilemetadata_odfextractor[2019-02-19T19:24:31.849Z] [ 56%] Building CXX object src/extractors/CMakeFiles/kfilemetadata_odfextractor.dir/odfextractor.cpp.o[2019-02-19T19:24:32.939Z] Scanning dependencies of target kfilemetadata_postscriptdscextractor[2019-02-19T19:24:32.939Z] [ 57%] Building CXX object src/extractors/CMakeFiles/kfilemetadata_postscriptdscextractor.dir/postscriptdscextractor.cpp.o[2019-02-19T19:24:33.450Z] [ 57%] Building CXX object src/extractors/CMakeFiles/kfilemetadata_officeextractor.dir/kfilemetadata_officeextractor_autogen/mocs_compilation.cpp.o[2019-02-19T19:24:35.142Z] [ 57%] Building CXX object autotests/CMakeFiles/propertyinfotest.dir/propertyinfotest_autogen/mocs_compilation.cpp.o[2019-02-19T19:24:35.142Z] [ 58%] Linking CXX shared module ../../bin/kfilemetadata_taglibwriter.so[2019-02-19T19:24:35.672Z] [ 59%] Building CXX object autotests/CMakeFiles/epubextractortest.dir/__/src/extractors/epubextractor.cpp.o[2019-02-19T19:24:35.672Z] [ 59%] Built target kfilemetadata_taglibwriter[2019-02-19T19:24:36.761Z] [ 60%] Building CXX object src/extractors/CMakeFiles/kfilemetadata_postscriptdscextractor.dir/__/kfilemetadata_debug.cpp.o[2019-02-19T19:24:36.761Z] [ 61%] Linking CXX shared module ../../bin/kf5/kfilemetadata/kfilemetadata_officeextractor.so[2019-02-19T19:24:36.761Z] [ 61%] Building CXX object src/extractors/CMakeFiles/kfilemetadata_odfextractor.dir/kfilemetadata_odfextractor_autogen/mocs_compilation.cpp.o[2019-02-19T19:24:37.262Z] [ 61%] Built target kfilemetadata_officeextractor[2019-02-19T19:24:37.767Z] [ 62%] Linking CXX executable ../bin/propertyinfotest[2019-02-19T19:24:38.269Z] [ 62%] Building CXX object autotests/CMakeFiles/epubextractortest.dir/epubextractortest_autogen/mocs_compilation.cpp.o[2019-02-19T19:24:38.269Z] [ 62%] Built target propertyinfotest[2019-02-19T19:24:38.269Z] [ 62%] Building CXX object 

KDE CI: Frameworks » kfilemetadata » kf5-qt5 SUSEQt5.12 - Build # 20 - Still Failing!

[CI System]

BUILD FAILURE
 Build URL
https://build.kde.org/job/Frameworks/job/kfilemetadata/job/kf5-qt5%20SUSEQt5.12/20/
 Project:
kf5-qt5 SUSEQt5.12
 Date of build:
Tue, 19 Feb 2019 19:20:48 +
 Build duration:
3 min 8 sec and counting
   CONSOLE OUTPUT
  [...truncated 565 lines...][2019-02-19T19:23:24.939Z] [ 52%] Building CXX object src/writers/CMakeFiles/kfilemetadata_taglibwriter.dir/kfilemetadata_taglibwriter_autogen/mocs_compilation.cpp.o[2019-02-19T19:23:25.440Z] [ 53%] Linking CXX shared module ../../bin/kf5/kfilemetadata/kfilemetadata_officeextractor.so[2019-02-19T19:23:25.440Z] [ 53%] Built target kfilemetadata_officeextractor[2019-02-19T19:23:25.440Z] Scanning dependencies of target externalwritertest[2019-02-19T19:23:25.440Z] [ 54%] Building CXX object autotests/CMakeFiles/externalwritertest.dir/externalwritertest.cpp.o[2019-02-19T19:23:26.491Z] [ 55%] Linking CXX shared module ../../bin/kfilemetadata_taglibwriter.so[2019-02-19T19:23:26.491Z] [ 55%] Built target kfilemetadata_taglibwriter[2019-02-19T19:23:26.491Z] Scanning dependencies of target popplerextractortest[2019-02-19T19:23:26.491Z] [ 55%] Building CXX object autotests/CMakeFiles/popplerextractortest.dir/popplerextractortest.cpp.o[2019-02-19T19:23:27.552Z] [ 55%] Building CXX object autotests/CMakeFiles/externalwritertest.dir/__/src/externalwriter.cpp.o[2019-02-19T19:23:28.607Z] [ 56%] Building CXX object autotests/CMakeFiles/popplerextractortest.dir/__/src/extractors/popplerextractor.cpp.o[2019-02-19T19:23:29.108Z] [ 57%] Building CXX object autotests/CMakeFiles/externalwritertest.dir/kfilemetadata_debug.cpp.o[2019-02-19T19:23:29.610Z] /home/jenkins/workspace/Frameworks/kfilemetadata/kf5-qt5 SUSEQt5.12/src/extractors/popplerextractor.cpp: In member function ���virtual void KFileMetaData::PopplerExtractor::extract(KFileMetaData::ExtractionResult*)���:[2019-02-19T19:23:29.610Z] /home/jenkins/workspace/Frameworks/kfilemetadata/kf5-qt5 SUSEQt5.12/src/extractors/popplerextractor.cpp:79:77: warning: ���QDateTime Poppler::convertDate(char*)��� is deprecated [-Wdeprecated-declarations][2019-02-19T19:23:29.610Z]  result->add(Property::CreationDate, Poppler::convertDate(utf8.data()));[2019-02-19T19:23:29.610Z]  ^[2019-02-19T19:23:29.610Z] In file included from /home/jenkins/workspace/Frameworks/kfilemetadata/kf5-qt5 SUSEQt5.12/src/extractors/popplerextractor.h:25,[2019-02-19T19:23:29.610Z]  from /home/jenkins/workspace/Frameworks/kfilemetadata/kf5-qt5 SUSEQt5.12/src/extractors/popplerextractor.cpp:22:[2019-02-19T19:23:29.611Z] /usr/include/poppler/qt5/poppler-qt5.h:1965:52: note: declared here[2019-02-19T19:23:29.611Z]  POPPLER_QT5_EXPORT Q_DECL_DEPRECATED QDateTime convertDate( char *dateString );[2019-02-19T19:23:29.611Z] ^~~[2019-02-19T19:23:29.611Z] /home/jenkins/workspace/Frameworks/kfilemetadata/kf5-qt5 SUSEQt5.12/src/extractors/popplerextractor.cpp:79:77: warning: ���QDateTime Poppler::convertDate(char*)��� is deprecated [-Wdeprecated-declarations][2019-02-19T19:23:29.611Z]  result->add(Property::CreationDate, Poppler::convertDate(utf8.data()));[2019-02-19T19:23:29.611Z]  ^[2019-02-19T19:23:29.611Z] In file included from /home/jenkins/workspace/Frameworks/kfilemetadata/kf5-qt5 SUSEQt5.12/src/extractors/popplerextractor.h:25,[2019-02-19T19:23:29.611Z]  from /home/jenkins/workspace/Frameworks/kfilemetadata/kf5-qt5 SUSEQt5.12/src/extractors/popplerextractor.cpp:22:[2019-02-19T19:23:29.611Z] /usr/include/poppler/qt5/poppler-qt5.h:1965:52: note: declared here[2019-02-19T19:23:29.611Z]  POPPLER_QT5_EXPORT Q_DECL_DEPRECATED QDateTime convertDate( char *dateString );[2019-02-19T19:23:29.611Z] ^~~[2019-02-19T19:23:30.664Z] [ 57%] Building CXX object autotests/CMakeFiles/externalwritertest.dir/externalwritertest_autogen/mocs_compilation.cpp.o[2019-02-19T19:23:30.664Z] [ 57%] Building CXX object autotests/CMakeFiles/popplerextractortest.dir/popplerextractortest_autogen/mocs_compilation.cpp.o[2019-02-19T19:23:31.725Z] [ 58%] Linking CXX executable ../bin/externalwritertest[2019-02-19T19:23:32.227Z] [ 58%] Built target externalwritertest[2019-02-19T19:23:32.228Z] Scanning dependencies of target usermetadatawritertest[2019-02-19T19:23:32.228Z] [ 59%] Building CXX object autotests/CMakeFiles/usermetadatawritertest.dir/usermetadatawritertest.cpp.o[2019-02-19T19:23:32.228Z] [ 60%] Linking CXX executable ../bin/popplerextractortest[2019-02-19T19:23:32.730Z] [ 60%] Built target popplerextractortest[2019-02-19T19:23:32.730Z] Scanning dependencies of target propertyinfotest[2019-02-19T19:23:32.730Z] [ 61%] Building CXX object 

KDE CI: Frameworks » kfilemetadata » kf5-qt5 SUSEQt5.10 - Build # 24 - Failure!

[CI System]

BUILD FAILURE
 Build URL
https://build.kde.org/job/Frameworks/job/kfilemetadata/job/kf5-qt5%20SUSEQt5.10/24/
 Project:
kf5-qt5 SUSEQt5.10
 Date of build:
Tue, 19 Feb 2019 19:17:17 +
 Build duration:
3 min 44 sec and counting
   CONSOLE OUTPUT
  [...truncated 571 lines...][2019-02-19T19:20:34.399Z] [ 48%] Building CXX object autotests/CMakeFiles/popplerextractortest.dir/popplerextractortest_autogen/mocs_compilation.cpp.o[2019-02-19T19:20:34.916Z] [ 49%] Linking CXX shared module ../../bin/kf5/kfilemetadata/kfilemetadata_officeextractor.so[2019-02-19T19:20:34.916Z] [ 49%] Building CXX object autotests/CMakeFiles/externalwritertest.dir/externalwritertest_autogen/mocs_compilation.cpp.o[2019-02-19T19:20:34.916Z] [ 50%] Building CXX object src/extractors/CMakeFiles/kfilemetadata_postscriptdscextractor.dir/__/kfilemetadata_debug.cpp.o[2019-02-19T19:20:35.421Z] [ 50%] Built target kfilemetadata_officeextractor[2019-02-19T19:20:35.923Z] [ 51%] Building CXX object src/extractors/CMakeFiles/kfilemetadata_plaintextextractor.dir/kfilemetadata_plaintextextractor_autogen/mocs_compilation.cpp.o[2019-02-19T19:20:35.923Z] [ 52%] Linking CXX shared module ../../bin/kf5/kfilemetadata/kfilemetadata_odfextractor.so[2019-02-19T19:20:36.426Z] [ 52%] Built target kfilemetadata_odfextractor[2019-02-19T19:20:36.928Z] [ 53%] Linking CXX executable ../bin/externalwritertest[2019-02-19T19:20:36.928Z] [ 53%] Building CXX object src/extractors/CMakeFiles/kfilemetadata_postscriptdscextractor.dir/kfilemetadata_postscriptdscextractor_autogen/mocs_compilation.cpp.o[2019-02-19T19:20:36.928Z] [ 54%] Linking CXX executable ../bin/popplerextractortest[2019-02-19T19:20:37.427Z] [ 54%] Built target externalwritertest[2019-02-19T19:20:37.427Z] [ 54%] Built target popplerextractortest[2019-02-19T19:20:37.937Z] [ 54%] Linking CXX shared module ../../bin/kf5/kfilemetadata/kfilemetadata_plaintextextractor.so[2019-02-19T19:20:37.937Z] [ 54%] Built target kfilemetadata_plaintextextractor[2019-02-19T19:20:38.778Z] [ 55%] Linking CXX shared module ../../bin/kf5/kfilemetadata/kfilemetadata_postscriptdscextractor.so[2019-02-19T19:20:38.778Z] [ 55%] Built target kfilemetadata_postscriptdscextractor[2019-02-19T19:20:38.778Z] Scanning dependencies of target exiv2extractortest[2019-02-19T19:20:38.778Z] [ 55%] Building CXX object autotests/CMakeFiles/exiv2extractortest.dir/exiv2extractortest.cpp.o[2019-02-19T19:20:41.114Z] [ 56%] Building CXX object autotests/CMakeFiles/exiv2extractortest.dir/__/src/extractors/exiv2extractor.cpp.o[2019-02-19T19:20:42.770Z] /home/jenkins/workspace/Frameworks/kfilemetadata/kf5-qt5 SUSEQt5.10/src/extractors/exiv2extractor.cpp: In constructor ���std::auto_ptr<  >::auto_ptr(std::auto_ptr<  >::element_type*) [with _Tp = Exiv2::Image; std::auto_ptr<  >::element_type = Exiv2::Image]���:[2019-02-19T19:20:42.770Z] /home/jenkins/workspace/Frameworks/kfilemetadata/kf5-qt5 SUSEQt5.10/src/extractors/exiv2extractor.cpp:152:27: warning: zero as null pointer constant [-Wzero-as-null-pointer-constant][2019-02-19T19:20:42.770Z]  Exiv2::Image::AutoPtr image;[2019-02-19T19:20:42.770Z]^[2019-02-19T19:20:43.271Z] [ 56%] Building CXX object autotests/CMakeFiles/exiv2extractortest.dir/exiv2extractortest_autogen/mocs_compilation.cpp.o[2019-02-19T19:20:45.614Z] Scanning dependencies of target postscriptdscextractortest[2019-02-19T19:20:45.614Z] [ 56%] Building CXX object autotests/CMakeFiles/postscriptdscextractortest.dir/postscriptdscextractortest.cpp.o[2019-02-19T19:20:45.614Z] Scanning dependencies of target officeextractortest[2019-02-19T19:20:45.614Z] [ 56%] Building CXX object autotests/CMakeFiles/officeextractortest.dir/office2007extractortest.cpp.o[2019-02-19T19:20:46.115Z] [ 57%] Linking CXX executable ../bin/exiv2extractortest[2019-02-19T19:20:46.617Z] [ 57%] Built target exiv2extractortest[2019-02-19T19:20:46.617Z] Scanning dependencies of target epubextractortest[2019-02-19T19:20:46.617Z] [ 57%] Building CXX object autotests/CMakeFiles/epubextractortest.dir/epubextractortest.cpp.o[2019-02-19T19:20:46.617Z] [ 58%] Building CXX object autotests/CMakeFiles/officeextractortest.dir/__/src/extractors/office2007extractor.cpp.o[2019-02-19T19:20:46.617Z] Scanning dependencies of target propertyinfotest[2019-02-19T19:20:46.617Z] [ 59%] Building CXX object autotests/CMakeFiles/propertyinfotest.dir/propertyinfotest.cpp.o[2019-02-19T19:20:47.671Z] Scanning dependencies of target usermetadatawritertest[2019-02-19T19:20:48.176Z] [ 60%] Building CXX object autotests/CMakeFiles/usermetadatawritertest.dir/usermetadatawritertest.cpp.o[2019-02-19T19:20:48.176Z] [ 61%] Building CXX object autotests/CMakeFiles/postscriptdscextractortest.dir/__/src/extractors/postscriptdscextractor.cpp.o[2019-02-19T19:20:48.677Z] [ 61%] Building CXX object autotests/CMakeFiles/officeextractortest.dir/officeextractortest_autogen/mocs_compilation.cpp.o[2019-02-19T19:20:49.732Z] [ 62%] Building CXX object 

KDE CI: Frameworks » kfilemetadata » kf5-qt5 SUSEQt5.12 - Build # 19 - Failure!

[CI System]

BUILD FAILURE
 Build URL
https://build.kde.org/job/Frameworks/job/kfilemetadata/job/kf5-qt5%20SUSEQt5.12/19/
 Project:
kf5-qt5 SUSEQt5.12
 Date of build:
Tue, 19 Feb 2019 19:17:17 +
 Build duration:
3 min 29 sec and counting
   CONSOLE OUTPUT
  [...truncated 565 lines...][2019-02-19T19:20:14.365Z] [ 52%] Building CXX object src/writers/CMakeFiles/kfilemetadata_taglibwriter.dir/kfilemetadata_taglibwriter_autogen/mocs_compilation.cpp.o[2019-02-19T19:20:15.441Z] [ 53%] Linking CXX shared module ../../bin/kf5/kfilemetadata/kfilemetadata_officeextractor.so[2019-02-19T19:20:15.441Z] [ 53%] Built target kfilemetadata_officeextractor[2019-02-19T19:20:15.441Z] Scanning dependencies of target externalwritertest[2019-02-19T19:20:15.441Z] [ 54%] Building CXX object autotests/CMakeFiles/externalwritertest.dir/externalwritertest.cpp.o[2019-02-19T19:20:15.954Z] [ 55%] Linking CXX shared module ../../bin/kfilemetadata_taglibwriter.so[2019-02-19T19:20:15.954Z] [ 55%] Built target kfilemetadata_taglibwriter[2019-02-19T19:20:16.467Z] Scanning dependencies of target popplerextractortest[2019-02-19T19:20:16.467Z] [ 55%] Building CXX object autotests/CMakeFiles/popplerextractortest.dir/popplerextractortest.cpp.o[2019-02-19T19:20:17.543Z] [ 55%] Building CXX object autotests/CMakeFiles/externalwritertest.dir/__/src/externalwriter.cpp.o[2019-02-19T19:20:18.616Z] [ 56%] Building CXX object autotests/CMakeFiles/popplerextractortest.dir/__/src/extractors/popplerextractor.cpp.o[2019-02-19T19:20:19.127Z] [ 57%] Building CXX object autotests/CMakeFiles/externalwritertest.dir/kfilemetadata_debug.cpp.o[2019-02-19T19:20:19.641Z] /home/jenkins/workspace/Frameworks/kfilemetadata/kf5-qt5 SUSEQt5.12/src/extractors/popplerextractor.cpp: In member function ���virtual void KFileMetaData::PopplerExtractor::extract(KFileMetaData::ExtractionResult*)���:[2019-02-19T19:20:19.641Z] /home/jenkins/workspace/Frameworks/kfilemetadata/kf5-qt5 SUSEQt5.12/src/extractors/popplerextractor.cpp:79:77: warning: ���QDateTime Poppler::convertDate(char*)��� is deprecated [-Wdeprecated-declarations][2019-02-19T19:20:19.641Z]  result->add(Property::CreationDate, Poppler::convertDate(utf8.data()));[2019-02-19T19:20:19.641Z]  ^[2019-02-19T19:20:19.641Z] In file included from /home/jenkins/workspace/Frameworks/kfilemetadata/kf5-qt5 SUSEQt5.12/src/extractors/popplerextractor.h:25,[2019-02-19T19:20:19.641Z]  from /home/jenkins/workspace/Frameworks/kfilemetadata/kf5-qt5 SUSEQt5.12/src/extractors/popplerextractor.cpp:22:[2019-02-19T19:20:19.641Z] /usr/include/poppler/qt5/poppler-qt5.h:1965:52: note: declared here[2019-02-19T19:20:19.641Z]  POPPLER_QT5_EXPORT Q_DECL_DEPRECATED QDateTime convertDate( char *dateString );[2019-02-19T19:20:19.641Z] ^~~[2019-02-19T19:20:19.641Z] /home/jenkins/workspace/Frameworks/kfilemetadata/kf5-qt5 SUSEQt5.12/src/extractors/popplerextractor.cpp:79:77: warning: ���QDateTime Poppler::convertDate(char*)��� is deprecated [-Wdeprecated-declarations][2019-02-19T19:20:19.641Z]  result->add(Property::CreationDate, Poppler::convertDate(utf8.data()));[2019-02-19T19:20:19.641Z]  ^[2019-02-19T19:20:19.641Z] In file included from /home/jenkins/workspace/Frameworks/kfilemetadata/kf5-qt5 SUSEQt5.12/src/extractors/popplerextractor.h:25,[2019-02-19T19:20:19.641Z]  from /home/jenkins/workspace/Frameworks/kfilemetadata/kf5-qt5 SUSEQt5.12/src/extractors/popplerextractor.cpp:22:[2019-02-19T19:20:19.641Z] /usr/include/poppler/qt5/poppler-qt5.h:1965:52: note: declared here[2019-02-19T19:20:19.641Z]  POPPLER_QT5_EXPORT Q_DECL_DEPRECATED QDateTime convertDate( char *dateString );[2019-02-19T19:20:19.641Z] ^~~[2019-02-19T19:20:20.153Z] [ 57%] Building CXX object autotests/CMakeFiles/popplerextractortest.dir/popplerextractortest_autogen/mocs_compilation.cpp.o[2019-02-19T19:20:20.666Z] [ 57%] Building CXX object autotests/CMakeFiles/externalwritertest.dir/externalwritertest_autogen/mocs_compilation.cpp.o[2019-02-19T19:20:21.745Z] [ 58%] Linking CXX executable ../bin/popplerextractortest[2019-02-19T19:20:22.257Z] [ 59%] Linking CXX executable ../bin/externalwritertest[2019-02-19T19:20:22.257Z] [ 59%] Built target externalwritertest[2019-02-19T19:20:22.257Z] Scanning dependencies of target usermetadatawritertest[2019-02-19T19:20:22.257Z] [ 60%] Building CXX object autotests/CMakeFiles/usermetadatawritertest.dir/usermetadatawritertest.cpp.o[2019-02-19T19:20:22.770Z] [ 60%] Built target popplerextractortest[2019-02-19T19:20:22.770Z] Scanning dependencies of target propertyinfotest[2019-02-19T19:20:22.770Z] [ 61%] Building CXX object 

D19100: Cleanup ffmpeg extractor

[Alexander Stippich]

This revision was automatically updated to reflect the committed changes.
Closed by commit R286:96b08a1a601f: Cleanup ffmpeg extractor (authored by 
astippich).

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19100?vs=51916=52102

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

AFFECTED FILES
  src/extractors/ffmpegextractor.cpp

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

D19099: Add a test for ffmpeg extractor

[Alexander Stippich]

This revision was automatically updated to reflect the committed changes.
astippich marked an inline comment as done.
Closed by commit R286:f4c72f517911: Add a test for ffmpeg extractor (authored 
by astippich).

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19099?vs=51990=52101

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/ffmpegextractortest.cpp
  autotests/ffmpegextractortest.h
  autotests/samplefiles/test.webm

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

D19107: Write valid UTF8 characters without escaping.

[Jos van den Oever]

vandenoever added inline comments.

INLINE COMMENTS

> thiago wrote in kconfigini.cpp:683
> This function does operate properly to find valid syntax UTF-8 sequences, but 
> it is neither catching overlong sequences nor UTF-8 content above U+10 
> (UTF-8 can encode 0x11000 in 4 bytes).
> 
> See 
> https://code.woboq.org/qt5/qtbase/tests/auto/corelib/codecs/utf8/utf8data.cpp.html#_Z19loadInvalidUtf8Rowsv
>  for potential UTF-8 pitfalls.

Thanks for the unicode explanation. I've added the checks for out of range and 
overlong now.

REPOSITORY
  R237 KConfig

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

To: vandenoever, dfaure, arichardson, apol, #frameworks, thiago
Cc: rapiteanu, kde-frameworks-devel, michaelh, ngraham, bruns

D19107: Write valid UTF8 characters without escaping.

[Jos van den Oever]

vandenoever updated this revision to Diff 52096.
vandenoever added a comment.


  Clean up test by using QTest data.

REPOSITORY
  R237 KConfig

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19107?vs=52091=52096

BRANCH
  utf8

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

AFFECTED FILES
  autotests/kconfigtest.cpp
  autotests/kconfigtest.h
  src/core/kconfigini.cpp

To: vandenoever, dfaure, arichardson, apol, #frameworks, thiago
Cc: rapiteanu, kde-frameworks-devel, michaelh, ngraham, bruns

D19107: Write valid UTF8 characters without escaping.

[Jos van den Oever]

vandenoever updated this revision to Diff 52091.
vandenoever added a comment.


  Remove duplicate if statement.

REPOSITORY
  R237 KConfig

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19107?vs=52090=52091

BRANCH
  utf8

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

AFFECTED FILES
  autotests/kconfigtest.cpp
  autotests/kconfigtest.h
  src/core/kconfigini.cpp

To: vandenoever, dfaure, arichardson, apol, #frameworks, thiago
Cc: rapiteanu, kde-frameworks-devel, michaelh, ngraham, bruns

D19107: Write valid UTF8 characters without escaping.

[Jos van den Oever]

vandenoever updated this revision to Diff 52090.
vandenoever added a comment.


  Added tests and code fixes to deal with overlong sequences and content 
  above U+10.

REPOSITORY
  R237 KConfig

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19107?vs=52015=52090

BRANCH
  utf8

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

AFFECTED FILES
  autotests/kconfigtest.cpp
  autotests/kconfigtest.h
  src/core/kconfigini.cpp

To: vandenoever, dfaure, arichardson, apol, #frameworks, thiago
Cc: rapiteanu, kde-frameworks-devel, michaelh, ngraham, bruns

D19107: Write valid UTF8 characters without escaping.

[Jos van den Oever]

vandenoever added inline comments.

INLINE COMMENTS

> thiago wrote in kconfigini.cpp:683
> This function does operate properly to find valid syntax UTF-8 sequences, but 
> it is neither catching overlong sequences nor UTF-8 content above U+10 
> (UTF-8 can encode 0x11000 in 4 bytes).
> 
> See 
> https://code.woboq.org/qt5/qtbase/tests/auto/corelib/codecs/utf8/utf8data.cpp.html#_Z19loadInvalidUtf8Rowsv
>  for potential UTF-8 pitfalls.

My understanding of overlong sequences was wrong. I understand now what is 
meant from your link.

REPOSITORY
  R237 KConfig

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

To: vandenoever, dfaure, arichardson, apol, #frameworks, thiago
Cc: rapiteanu, kde-frameworks-devel, michaelh, ngraham, bruns

D19107: Write valid UTF8 characters without escaping.

[Thiago Macieira]

thiago added inline comments.

INLINE COMMENTS

> vandenoever wrote in kconfigini.cpp:683
> A check for U+10 > value is needed.
> Overlong sequences are caught on line 696 (count < 4).

That's not what an overlong sequence is. You can produce 2-, 3- and 4-byte 
overlong sequences. See the examples in the Qt test, like 0xc0 0x80.

REPOSITORY
  R237 KConfig

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

To: vandenoever, dfaure, arichardson, apol, #frameworks, thiago
Cc: rapiteanu, kde-frameworks-devel, michaelh, ngraham, bruns

D19107: Write valid UTF8 characters without escaping.

[Jos van den Oever]

vandenoever added inline comments.

INLINE COMMENTS

> thiago wrote in kconfigini.cpp:683
> This function does operate properly to find valid syntax UTF-8 sequences, but 
> it is neither catching overlong sequences nor UTF-8 content above U+10 
> (UTF-8 can encode 0x11000 in 4 bytes).
> 
> See 
> https://code.woboq.org/qt5/qtbase/tests/auto/corelib/codecs/utf8/utf8data.cpp.html#_Z19loadInvalidUtf8Rowsv
>  for potential UTF-8 pitfalls.

A check for U+10 > value is needed.
Overlong sequences are caught on line 696 (count < 4).

REPOSITORY
  R237 KConfig

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

To: vandenoever, dfaure, arichardson, apol, #frameworks, thiago
Cc: rapiteanu, kde-frameworks-devel, michaelh, ngraham, bruns

D19163: Make it possible to show all spaces in the document

[Aleix Pol Gonzalez]

apol created this revision.
apol added a reviewer: Kate.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
apol requested review of this revision.

REVISION SUMMARY
  It's a feature offered by several editors. Now we allow to render none, only 
trailing and all of them.
  
  BUG: 381267
  BUG: 342811

TEST PLAN
  Tested locally

REPOSITORY
  R39 KTextEditor

BRANCH
  master

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

AFFECTED FILES
  src/dialogs/katedialogs.cpp
  src/dialogs/textareaappearanceconfigwidget.ui
  src/render/katerenderer.cpp
  src/render/katerenderer.h
  src/utils/kateconfig.cpp
  src/utils/kateconfig.h

To: apol, #kate
Cc: kwrite-devel, kde-frameworks-devel, gennad, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D19107: Write valid UTF8 characters without escaping.

[Thiago Macieira]

thiago added inline comments.

INLINE COMMENTS

> kconfigini.cpp:683
> +// When an additional byte leads to an invalid character, return 
> false.
> +bool addByte(unsigned char b) {
> +if (count == 0) {

This function does operate properly to find valid syntax UTF-8 sequences, but 
it is neither catching overlong sequences nor UTF-8 content above U+10 
(UTF-8 can encode 0x11000 in 4 bytes).

See 
https://code.woboq.org/qt5/qtbase/tests/auto/corelib/codecs/utf8/utf8data.cpp.html#_Z19loadInvalidUtf8Rowsv
 for potential UTF-8 pitfalls.

REPOSITORY
  R237 KConfig

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

To: vandenoever, dfaure, arichardson, apol, #frameworks, thiago
Cc: rapiteanu, kde-frameworks-devel, michaelh, ngraham, bruns

D19161: Use QTextFormat::TextUnderlineStyle instead of QTextFormat::FontUnderline

[Ahmad Samir]

ahmadsamir created this revision.
ahmadsamir added reviewers: KTextEditor, cullmann, dhaumann.
Herald added projects: Kate, Frameworks.
ahmadsamir requested review of this revision.

REVISION SUMMARY
  QTextFormat::FontUnderline has been deprecated upstream for a long time.
  
  It seems that for some time setting the underline attribute for syntax
  highlighting has been broken, i.e. a user can't set it via the config
  widget and setKateExtendedAttributeList always sets it to false. The
  latter meant that when the user applied any changes that would write
  katesyntaxhighlightingrc, the underline attribute for syntax HL would
  unconditionally be set to false which leads to some weird situations,
  (e.g. bug 399278 where un/setting "highlight trailing spaces" causes
  katesyntaxhighlightingrc to get written to disk which leads to
  Markdown:linebreak losing its underline attribute for good).
  
  BUG: 399278
  FIXED-IN: 5.57

REPOSITORY
  R39 KTextEditor

BRANCH
  syntax-hl (branched from master)

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

AFFECTED FILES
  src/schema/katestyletreewidget.cpp
  src/syntax/katehighlight.cpp
  src/syntax/katesyntaxmanager.cpp

To: ahmadsamir, #ktexteditor, cullmann, dhaumann
Cc: kde-frameworks-devel, kwrite-devel, gennad, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D19131: [RFC] DocumentPrivate: Fix "auto-rewrap" in static wrap mode

[loh tar]

loh.tar updated this revision to Diff 52072.
loh.tar edited the summary of this revision.
loh.tar added a comment.


  Fix mentioned misplaced cursor.
  The reason was a special treatment for past-end-of-line cursors. The fix 
changed/fixed also that now the cursor is moved by one not two positions in 
this case.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19131?vs=52006=52072

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

AFFECTED FILES
  src/document/katedocument.cpp
  src/document/katedocument.h

To: loh.tar, cullmann, #ktexteditor
Cc: kwrite-devel, kde-frameworks-devel, #ktexteditor, gennad, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D19156: port to findcanberra from ECM

[Harald Sitter]

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

REVISION SUMMARY
  also move to imported target available from that finder

TEST PLAN
  it builds!

REPOSITORY
  R289 KNotifications

BRANCH
  master

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

AFFECTED FILES
  CMakeLists.txt
  cmake/modules/FindCanberra.cmake
  src/CMakeLists.txt

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

D18793: Handle text completion with block selection mode

[Milian Wolff]

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


  we override execution in our own completion models, so this patch will only 
change the behavior for the builtin word and keyword completion models in 
ktexteditor I believe
  
  that said, I think it makes sense to insert the word everywhere in block 
selection, it shouldn't be different from typing text.
  
  so +1 for the idea, but -1 on the actual implementation:
  
  - we need to have a unit test for this new behavior
  - we should introduce new helper API to make it easier to opt-in to this new 
behavior and reduce the if/else depth. This would also make it easier for us in 
KDevelop to change our behavior accordingly. I believe the code completion 
execution code should basically be agnostic to the block selection mode. I.e. 
instead of the proposed
  
if (completeBlockSelection) {
removeText
typeChars
} else {
replaceText
}
  
  it should always just call "replaceText" with the the block selection range 
and then internal API should duplicate the text, if the selection is a block 
selection
  
  as-is, this patch adds too much code duplication

REPOSITORY
  R39 KTextEditor

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

To: ahmadsamir, #ktexteditor, cullmann, dhaumann, #kdevelop, mwolff
Cc: mwolff, kde-frameworks-devel, kwrite-devel, #ktexteditor, gennad, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D19075: Fix FindEGL

[Tobias C. Berner]

This revision was automatically updated to reflect the committed changes.
Closed by commit R240:7c64db956829: Fix FindEGL (authored by tcberner).

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19075?vs=51847=52057

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

AFFECTED FILES
  find-modules/FindEGL.cmake

To: tcberner, #automotive, hausmann, #freebsd, apol
Cc: bcooksley, kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D18952: new find module for Canberra

[Harald Sitter]

sitter updated this revision to Diff 52042.
sitter added a comment.


  explicitly set FOUND_VAR so it is camelcase too

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18952?vs=51976=52042

BRANCH
  master

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

AFFECTED FILES
  docs/find-module/FindCanberra.rst
  find-modules/FindCanberra.cmake

To: sitter, cgiboudeaux
Cc: aacid, apol, kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D18952: new find module for Canberra

[Harald Sitter]

This revision was automatically updated to reflect the committed changes.
Closed by commit R240:50e8dd7b2d00: new find module for Canberra (authored by 
sitter).

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18952?vs=52042=52043

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

AFFECTED FILES
  docs/find-module/FindCanberra.rst
  find-modules/FindCanberra.cmake

To: sitter, cgiboudeaux
Cc: aacid, apol, kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D18952: new find module for Canberra

[Harald Sitter]

sitter marked an inline comment as done.

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

To: sitter, cgiboudeaux
Cc: aacid, apol, kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D18883: Add PDF thumbnailer

[Kai Uwe Broulik]

broulik added a comment.


  Perhaps but it's only recently become an issue since kioslaves now support 
KCrash. Previously such thumbnailer crashes just went unnoticed.

REPOSITORY
  R320 KIO Extras

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

To: broulik, dfaure, aacid, jtamate
Cc: bruns, dhaumann, ngraham, pino, ltoscano, kde-frameworks-devel, kfm-devel, 
alexde, feverfew, michaelh, spoorun, navarromorales, firef, andrebarros, 
emmanuelp, mikesomov

D18952: new find module for Canberra

[Christophe Giboudeaux]

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


  Thanks! just a little thing to fix before pushing

INLINE COMMENTS

> FindCanberra.cmake:75
> +find_package_handle_standard_args(Canberra
> +REQUIRED_VARS
> +Canberra_LIBRARIES

Also add

  FOUND_VAR Canberra_FOUND

The default for CMake 2.8.12 which ECM requires is '_FOUND'.

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

To: sitter, cgiboudeaux
Cc: aacid, apol, kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns