D10826: Introduce DocumentId class

2018-02-27 Thread Michael Heidelbach
michaelh added inline comments.

INLINE COMMENTS

> svuorela wrote in documentid.cpp:67
> if you make operator DeviceIdAndInode explicit, you probably end up with a 
> bit better behavior.
> 
> Else the compiler happily converts a documentid to a deviceidandinode in 
> order to make various comparisons at compile time.
> 
> e.g. DocumentId id(5,5);
> bool b = true;
> if (id == b) { ... }  should compile, but probably isn't intended behavior..

This is intended. At the moment the class should behave like 
quint64(=DeviceIdAndInode)

> svuorela wrote in documentid.h:51-52
> it depends on why it is exported. If it is exported just for unit test (and 
> the header file not installed), then we don't need to have the mental and 
> code wise overhead of a d-pointer.
> 
> If we know that it will never ever need to grow, we also don't need the 
> overhead of a d-pointer. 
> So it is a big "it depends", which I was also kind of trying to ask into 
> earlier.

I have to admit, I have not understood the meaning of BALOO_ENGINE_EXPORT yet. 
The file which defines it looks like it's automatically created. I inserted it 
after trial and error. Except for testing I see no reason why it should be 
exported. I consider it as strictly internal.

In its present state this class shall serve as a replacement for the quint64 
that baloo is currently using for an id. 
Finally this class will have a payload of 24 bytes max. (64bit inode and 128bit 
uuid) and one method: `isValid()`. That at least is the plan.

This class is used a lot. What I have read about d-pointers so far indicates a 
huge performance hit when using indirection. Maybe I'm wrong.

@svuorela: I'm sorry if my answer appeared uncouth. That was not intended. 
Currently a lot of my programming is the result of trial and error rather than 
knowledge. In time the ratio, hopefully, will change in favor of the latter :)

REPOSITORY
  R293 Baloo

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

To: michaelh, adridg, #baloo, #frameworks
Cc: anthonyfieroni, svuorela, ashaposhnikov, michaelh, spoorun, nicolasfella, 
alexeymin


D10803: handle more tags in taglibextractor

2018-02-27 Thread Michael Heidelbach
michaelh added a comment.


  A thumbnail extractor might be better suited for this job.
  On my system(tumbleweed) there is a `kde-audio-thumbnail.` I don't know 
anything more about it.
  Maybe `ffmpeg-thumbnails` can do that?

REPOSITORY
  R286 KFileMetaData

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

To: astippich, mgallien
Cc: dfaure, michaelh, ngraham, #frameworks


D10899: Don't make Headings 20% transparent, to match Kirigami

2018-02-27 Thread Nathaniel Graham
ngraham created this revision.
ngraham added a reviewer: Plasma.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.
ngraham requested review of this revision.

REVISION SUMMARY
  We recently removed Kirigami Headings' slight transparency, after concluding 
that it looked inappropriate to have Headings with lighter text than their 
textual content. `PlasmaExtras.Heading` does the same thing, so we should make 
the same change here, both based on the inherent merits of the change, and also 
to maintain consistency with Kirigami.
  
  This also slightly improves matters for people who complain about poor text 
contrast throughout Plasma.

TEST PLAN
  Browsed System Settings; headings are now ever so slightly darker (or 
lighter, for users of Dark themes)
  
  Also, here are some befores-and-afters for a widget I'm working on that uses 
Headings. Before:
  
  After:
  
  It's subtle; you'll need to flip between the two with the arrow keys to see 
the difference, and may need to also zoom in ([meta] + [+] + [plus])

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  opaque-headers (branched from master)

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

AFFECTED FILES
  src/declarativeimports/plasmaextracomponents/qml/Heading.qml

To: ngraham, #plasma
Cc: #frameworks, michaelh


D10694: epubextractor: Handle multiple subjects better

2018-02-27 Thread Matthieu Gallien
mgallien requested changes to this revision.
mgallien added a comment.
This revision now requires changes to proceed.


  In D10694#214996 , @michaelh wrote:
  
  > In D10694#213712 , @mgallien 
wrote:
  >
  > > To me, it is necessary to have a test that ensures that possible existing 
clients are not affected by your change. Could you add it ?
  >
  >
  > The new behaviour will break clients. No test needed for this. E.g. 
baloo-widgets will show the label 'Subject' but no content.
  >  To make it clear: I do not want break this for clients apart from 
baloo-widgets. The question is: Who are the clients? and How to find them 
except for searching https://lxr.kde.org/? Please help me with this.
  
  
  Yesterday, I tried looking for such use and did not found them. I did try to 
use web search engine with no success. I am afraid I cannot help you but would 
be happy to hear any advice.
  
  > The current behaviour is already breaking: I have some epub files with a 
lot of `dc:subject` tags. The semicolon-concatenated string is longer than my 
monitor is wide. As a result in dolphin the tooltip does not display at all. My 
rationale was to tackle the problem at the root rather than to work around it 
by splitting the string within baloo-widgets. With regard to T8079 
 it will be the same. If data is taken from 
baloo's database that old long string will be displayed
  
  I am sure that your work is really trying to fix real problem that are 
seeing. You have my full support on your aim.
  
  I should apologize given how bad my knowledge of KFileMetaData is. It is 
already silently managing single and multiple entries. In fact, yes, the epub 
extractor is doing it wrong.
  I have also checked how baloo is handling multiple entries and it works in 
the same way than pure kfilemetadata.
  
  In the taglib extractors multiple entries are added in several call to add 
method. This is nicely handled by KFileMetaData::SimpleExtractionResult::add . 
It would be nice if you modify your patch to follow the same approach. In 
Baloo, things are a little bit different and a list is automatically created 
when multiple add are called with the same property but the result is similar 
(in baloo Result::add ).
  
  You can forget my initial objection, I was plain wrong.
  
  We may have bugs in user of the API not expecting a list when they should 
have. In my opinion, this should not block your diff. Let's fix them when we 
discover them.
  
  > According to the standard an epub can have multiple `dc:subject` entries. 
So it is reasonable to expect a string list from KFileMetaData. Plus the 
possibilities are grand with an KFileMetaData writer for epubs one could use 
the tag widget to change them. One could unify xattr tags and epub subjects in 
tags:// protocol and mayby more.

REPOSITORY
  R286 KFileMetaData

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

To: michaelh, mgallien, dfaure
Cc: #frameworks, ashaposhnikov, michaelh, spoorun, navarromorales, isidorov, 
nicolasfella, firef, andrebarros, alexeymin, emmanuelp


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

2018-02-27 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20plasma-framework%20kf5-qt5%20SUSEQt5.10/75/
 Project:
Frameworks plasma-framework kf5-qt5 SUSEQt5.10
 Date of build:
Tue, 27 Feb 2018 19:13:21 +
 Build duration:
8 min 49 sec and counting
   JUnit Tests
  Name: (root) Failed: 7 test(s), Passed: 8 test(s), Skipped: 0 test(s), Total: 15 test(s)Failed: TestSuite.dialognativetestFailed: TestSuite.plasma-configmodeltestFailed: TestSuite.plasma-dialogqmltestFailed: TestSuite.plasma-fallbackpackagetestFailed: TestSuite.plasma-iconitemtestFailed: TestSuite.plasma-packagestructuretestFailed: TestSuite.plasma-storagetest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report33%
(6/18)35%
(55/159)35%
(55/159)27%
(3560/13341)19%
(1979/10555)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests85%
(22/26)85%
(22/26)53%
(609/1139)28%
(421/1490)src.declarativeimports.calendar0%
(0/11)0%
(0/11)0%
(0/447)0%
(0/239)src.declarativeimports.core22%
(4/18)22%
(4/18)11%
(254/2229)7%
(102/1492)src.declarativeimports.plasmacomponents0%
(0/9)0%
(0/9)0%
(0/522)0%
(0/214)src.declarativeimports.plasmaextracomponents0%
(0/5)0%
(0/5)0%
(0/44)0%
(0/27)src.declarativeimports.platformcomponents0%
(0/4)0%
(0/4)0%
(0/60)0%
(0/14)src.declarativeimports.platformcomponents.utils0%
(0/2)0%
(0/2)0%
(0/15)0%
(0/4)src.plasma55%
(12/22)55%
(12/22)41%
(1442/3488)28%
(827/2913)src.plasma.packagestructure0%
(0/7)0%
(0/7)0%
(0/141)0%
(0/14)src.plasma.private46%
(11/24)46%
(11/24)42%
(671/1612)28%
(318/1121)src.plasma.scripting0%
(0/3)0%
(0/3)0%
(0/161)0%
(0/132)src.plasmapkg0%
(0/1)0%
(0/1)0%
(0/45)0%
(0/40)src.plasmaquick42%
(5/12)42%
(5/12)27%
(553/2012)17%
(306/1761)src.plasmaquick.private33%
(1/3)33%
(1/3)28%
(31/110)36%
(5/14)src.scriptengines.qml.plasmoid0%
(0/6)0%
(0/6)0%
(0/1158)0%
(0/1056)tests.dpi0%
(0/2)0%
(0/2)0%
(0/22)0%
(0/2)tests.kplugins0%
  

D10902: Don't make Titles 20% transparent either

2018-02-27 Thread Nathaniel Graham
ngraham edited the summary of this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: ngraham, #plasma, mart
Cc: #frameworks, michaelh


D10902: Don't make Titles 20% transparent either

2018-02-27 Thread Nathaniel Graham
ngraham edited the summary of this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: ngraham, #plasma, mart
Cc: #frameworks, michaelh


D10899: Don't make Headings 20% transparent, to match Kirigami

2018-02-27 Thread Nathaniel Graham
ngraham edited the summary of this revision.
ngraham edited the test plan for this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: ngraham, #plasma
Cc: #frameworks, michaelh


KDE CI: Frameworks plasma-framework kf5-qt5 FreeBSDQt5.9 - Build # 51 - Still unstable!

2018-02-27 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20plasma-framework%20kf5-qt5%20FreeBSDQt5.9/51/
 Project:
Frameworks plasma-framework kf5-qt5 FreeBSDQt5.9
 Date of build:
Tue, 27 Feb 2018 19:13:21 +
 Build duration:
4 min 48 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 13 test(s), Skipped: 0 test(s), Total: 14 test(s)Failed: TestSuite.plasma-packagestructuretest

D10902: Don't make Titles 20% transparent either

2018-02-27 Thread Nathaniel Graham
ngraham created this revision.
ngraham added reviewers: Plasma, mart.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.
ngraham requested review of this revision.

REVISION SUMMARY
  If it didn't make sense to have Headings be 20% opaque because nearby or 
child textual content would be darker, then it //really// doesn't make sense to 
have Titles be 20% transparent, because then **everything** is darker than the 
titles, which is really odd.

TEST PLAN
  Widget Explorer title, before:
  
  Widget Explorer title, after:

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  opaque-titles (branched from master)

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

AFFECTED FILES
  src/declarativeimports/plasmaextracomponents/qml/Title.qml

To: ngraham, #plasma, mart
Cc: #frameworks, michaelh


D10899: Don't make Headings 20% transparent, to match Kirigami

2018-02-27 Thread Nathaniel Graham
ngraham edited the test plan for this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: ngraham, #plasma
Cc: #frameworks, michaelh


D10899: Don't make Headings 20% transparent, to match Kirigami

2018-02-27 Thread Marco Martin
mart accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  opaque-headers (branched from master)

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

To: ngraham, #plasma, mart
Cc: #frameworks, michaelh


D10902: Don't make Titles 20% transparent either

2018-02-27 Thread Nathaniel Graham
ngraham edited the summary of this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: ngraham, #plasma, mart
Cc: #frameworks, michaelh


D10902: Don't make Titles 20% transparent either

2018-02-27 Thread Nathaniel Graham
ngraham edited the summary of this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: ngraham, #plasma, mart
Cc: #frameworks, michaelh


D10899: Don't make Headings 20% transparent, to match Kirigami

2018-02-27 Thread Nathaniel Graham
ngraham closed this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: ngraham, #plasma, mart
Cc: #frameworks, michaelh


D10902: Don't make Titles 20% transparent either

2018-02-27 Thread Nathaniel Graham
ngraham edited the test plan for this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: ngraham, #plasma, mart
Cc: #frameworks, michaelh


KDE CI: Frameworks plasma-framework kf5-qt5 SUSEQt5.7 - Build # 78 - Still Unstable!

2018-02-27 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20plasma-framework%20kf5-qt5%20SUSEQt5.7/78/
 Project:
Frameworks plasma-framework kf5-qt5 SUSEQt5.7
 Date of build:
Tue, 27 Feb 2018 19:13:21 +
 Build duration:
30 min and counting
   JUnit Tests
  Name: (root) Failed: 7 test(s), Passed: 8 test(s), Skipped: 0 test(s), Total: 15 test(s)Failed: TestSuite.dialognativetestFailed: TestSuite.plasma-configmodeltestFailed: TestSuite.plasma-dialogqmltestFailed: TestSuite.plasma-fallbackpackagetestFailed: TestSuite.plasma-iconitemtestFailed: TestSuite.plasma-packagestructuretestFailed: TestSuite.plasma-storagetest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report33%
(6/18)35%
(55/159)35%
(55/159)27%
(3559/13337)19%
(1978/10555)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests85%
(22/26)85%
(22/26)53%
(609/1139)28%
(421/1490)src.declarativeimports.calendar0%
(0/11)0%
(0/11)0%
(0/447)0%
(0/239)src.declarativeimports.core22%
(4/18)22%
(4/18)11%
(253/2225)7%
(101/1488)src.declarativeimports.plasmacomponents0%
(0/9)0%
(0/9)0%
(0/522)0%
(0/214)src.declarativeimports.plasmaextracomponents0%
(0/5)0%
(0/5)0%
(0/44)0%
(0/27)src.declarativeimports.platformcomponents0%
(0/4)0%
(0/4)0%
(0/60)0%
(0/14)src.declarativeimports.platformcomponents.utils0%
(0/2)0%
(0/2)0%
(0/15)0%
(0/4)src.plasma55%
(12/22)55%
(12/22)41%
(1442/3488)28%
(827/2917)src.plasma.packagestructure0%
(0/7)0%
(0/7)0%
(0/141)0%
(0/14)src.plasma.private46%
(11/24)46%
(11/24)42%
(671/1612)28%
(318/1121)src.plasma.scripting0%
(0/3)0%
(0/3)0%
(0/161)0%
(0/132)src.plasmapkg0%
(0/1)0%
(0/1)0%
(0/45)0%
(0/40)src.plasmaquick42%
(5/12)42%
(5/12)27%
(553/2012)17%
(306/1761)src.plasmaquick.private33%
(1/3)33%
(1/3)28%
(31/110)36%
(5/14)src.scriptengines.qml.plasmoid0%
(0/6)0%
(0/6)0%
(0/1158)0%
(0/1056)tests.dpi0%
(0/2)0%
(0/2)0%
(0/22)0%
(0/2)tests.kplugins0%
  

D10803: handle more tags in taglibextractor

2018-02-27 Thread Matthieu Gallien
mgallien added a comment.


  In D10803#214655 , @astippich 
wrote:
  
  > In D10803#213783 , @michaelh 
wrote:
  >
  > > In D10803#213767 , @astippich 
wrote:
  > >
  > > > I don't know dolphin works, but given how KFileMetadata is designed, 
the new tags should be ignored until explicit support is added in dolphin.
  > >
  > >
  > > Please try that. For unknown types baloo-widgets (responsible for 
infopanel/tooltips) falls back to `value.toString()`, no idea what will happen 
when you feed it with binary data. The cover properties return binary data, 
right?
  >
  >
  > The cover is binary data, right. And I think I was wrong. While I don't 
fully understand yet how baloo-widgets work, from what I've read all available 
properties are automatically queried. The binary data would then be converted 
to a string as you said. So adding the cover this way may not be a good 
solution. A different interface should be created for image/binary data.
  >  I think I will split this into two diffs and do the cover images 
separately.
  
  
  This is not necessary. You could use a QImage or a QPixmap. You can put them 
in a QVariant and they should be converted to an empty QString.
  As far as I understand, in Elisa, we would need to access them with 
QQuickImageProvider. All in all, well done job.
  
  I am not sure for the rating given that there is also a rating (the one used 
in Elisa) that is stored as an extended file system attribute independent from 
the file mime type. One limit of this rating is that it is specific to the 
KFileMetaData API users. Do you have an idea ?

REPOSITORY
  R286 KFileMetaData

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

To: astippich, mgallien
Cc: dfaure, michaelh, ngraham, #frameworks


D10902: Don't make Titles 20% transparent either

2018-02-27 Thread Nathaniel Graham
ngraham edited the test plan for this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: ngraham, #plasma, mart
Cc: #frameworks, michaelh


D8532: [WIP] Restrict file extractor with Seccomp

2018-02-27 Thread Oswald Buddenhagen
ossi added a comment.


  > But i'm not sure if thats feasible.
  
  launching subprocesses is no biggie; e.g., the kprocess test does it.
  if you're lucky, a few env variables will be sufficient. in the worst case, 
you'd need to chroot and do bind mounts or something like that, which is of 
course "a bit more challenging".

REPOSITORY
  R293 Baloo

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

To: davidk, apol, ossi
Cc: detlefe, ngraham, nicolasfella, #frameworks, michaelh


D10663: Remove a partially copied file if copyjob was cancelled in the middle of file copying

2018-02-27 Thread David Faure
dfaure added a comment.


  "the destination file gets deleted only if some data has been written into 
it" ... ah I see, that's a great solution indeed, I like it.
  
  About moving: if the file was fully copied, (but the source not yet deleted), 
good catch. This requires a similar solution in FileCopyJob, i.e. one layer 
below (with a similar "in progress" flag that is only true while the file copy 
is in progress, and false as soon as it's done, i.e. before the DeleteJob). I 
suggest starting with that, and then in CopyJob when killing a FileCopyJob we 
just let it do the cleanup. As you found out, CopyJob is too high-level for 
this.
  
  Pausing and doing nasty things will never be safe, I don't think we need to 
worry about that.
  
  I think all you have to do is go one more step down the stack :-)

REPOSITORY
  R241 KIO

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

To: dmitrio, #frameworks, dfaure
Cc: ngraham, anthonyfieroni, meven, #frameworks, michaelh


D10663: Remove a partially copied file if copyjob was cancelled in the middle of file copying

2018-02-27 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  @dmitrio: you can click on the "Plan Changes" action, no need to abandon this 
revision and create a new one.

REPOSITORY
  R241 KIO

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

To: dmitrio, #frameworks, dfaure
Cc: elvisangelaccio, ngraham, anthonyfieroni, meven, #frameworks, michaelh


D10824: Delete IdleSlave having temporary authorization

2018-02-27 Thread Luca Beltrame
lbeltrame added a reviewer: Frameworks.

REPOSITORY
  R303 KInit

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

To: chinmoyr, dfaure, #frameworks
Cc: #frameworks, michaelh


D10273: Create proper SocketAddress

2018-02-27 Thread Luca Beltrame
lbeltrame added a reviewer: dfaure.

REPOSITORY
  R241 KIO

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

To: chinmoyr, #frameworks, dfaure
Cc: ossi, thiago, dfaure, michaelh


D10694: epubextractor: Handle multiple subjects better

2018-02-27 Thread Michael Heidelbach
michaelh added a comment.


  In D10694#213712 , @mgallien wrote:
  
  > To me, it is necessary to have a test that ensures that possible existing 
clients are not affected by your change. Could you add it ?
  
  
  The new behaviour will break clients. No test needed for this. E.g. 
baloo-widgets will show the label 'Subject' but no content.
  To make it clear: I do not want break this for clients apart from 
baloo-widgets. The question is: Who are the clients? and How to find them 
except for searching https://lxr.kde.org/? Please help me with this.
  
  The current behaviour is already breaking: I have some epub files with a lot 
of `dc:subject` tags. The semicolon-concatenated string is longer than my 
monitor is wide. As a result in dolphin the tooltip does not display at all. My 
rationale was to tackle the problem at the root rather than to work around it 
by splitting the string within baloo-widgets. With regard to T8079 
 it will be the same. If data is taken from 
baloo's database that old long string will be displayed
  
  According to the standard an epub can have multiple `dc:subject` entries. So 
it is reasonable to expect a string list from KFileMetaData. Plus the 
possibilities are grand with an KFileMetaData writer for epubs one could use 
the tag widget to change them. One could unify xattr tags and epub subjects in 
tags:// protocol and mayby more.

REPOSITORY
  R286 KFileMetaData

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

To: michaelh, mgallien, dfaure
Cc: #frameworks, ashaposhnikov, michaelh, spoorun, navarromorales, isidorov, 
nicolasfella, firef, andrebarros, alexeymin, emmanuelp


D10857: Change qSort to std::sort in simplifiedUrlList

2018-02-27 Thread Aleix Pol Gonzalez
apol added a comment.


  I'd say the bottom line is qSort is deprecated in favor of std::sort.
  
  @jtamate make sure you are profiling release builds.

REPOSITORY
  R241 KIO

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

To: jtamate, #frameworks, dfaure
Cc: markg, apol, michaelh


D10857: Change qSort to std::sort in simplifiedUrlList

2018-02-27 Thread Mark Gaiser
markg added a comment.


  In D10857#214867 , @jtamate wrote:
  
  > In D10857#214607 , @dfaure wrote:
  >
  > > I'm not opposed to the idea, but measuring CPU usage is a rather 
misleading indicator. What if it takes 10 times longer, because it's 
progressing much more slowly? ;)
  > >
  > > Please at least measure with QElapsedTimer around the sorting (not to be 
committed, just to gather numbers about the actual performance of this from a 
user's point of view).
  > >  I'm interested in the result ;)
  >
  >
  > The results are strange. All the results are measured sorting 50.000 small 
files:
  >
  > Both Qt are the same version from opensuse.
  >
  > In an i5, why the difference is so big? recent cpu bugs?
  >  qSort in i3 
  >  274764, 276060 (with 3 directories), 365878, 424506  (without directories)
  >  std::sort in i3
  >  940, 995 (with 3 directories), 2472, 2539 (without directories)
  >
  > In AMD the results are closer, qSort wins
  >  qSort in AMD
  >  658, 726, 695, 683, 676, 666, 649, 684, 666 (without directories)
  >  std:sort in AMD
  >  843, 839, 878, 896,  925 (without directories)
  
  
  That surprises me a lot!
  I "thought" qSort was just a template or alias to std::sort, but it isn't: 
http://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/tools/qalgorithms.h#n340
  But it isn't which kinda makes my other expectations moot (qSort == 
std::sort... it isn't)
  
  So i did some tests as well.
  5 filenames sorting them (computer sorting, not the natural one) with 
std::string and QString (yes, it matters)
  Filenames, well, sorta.. Made them up in a loop :)
  
  QString version: https://p.sc2.nl/r1JTFaf_z
  qSort: ~220ms
  std::sort ~276ms
  
  std::string version: https://p.sc2.nl/HJ69Y6G_G
  qSort: ~100ms
  std::sort ~130ms
  
  Note that std::string might not be the fair comparison as it's 8 bit/char. 
QString is 16.
  My compiler (in this rare case) Visual Studio 2017 on an Intel CPU.
  
  I still think it's wise to replace all qSort, if only for it being 
deprecated. But it does suck a little that qSort beats std::sort.

REPOSITORY
  R241 KIO

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

To: jtamate, #frameworks, dfaure
Cc: markg, apol, michaelh


D10719: Highlighting for OpenSCAD

2018-02-27 Thread Julian Stirling
julianstirling updated this revision to Diff 28190.

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10719?vs=27770=28190

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

AFFECTED FILES
  autotests/input/highlight.scad
  data/syntax/openscad.xml

To: julianstirling, dhaumann
Cc: dhaumann, ngraham, #frameworks, michaelh


D10719: Highlighting for OpenSCAD

2018-02-27 Thread Julian Stirling
julianstirling added a comment.


  `git diff HEAD^^ HEAD > ../PatchCombined.patch` has hopefully done the trick.

REPOSITORY
  R216 Syntax Highlighting

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

To: julianstirling, dhaumann
Cc: dhaumann, ngraham, #frameworks, michaelh


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

2018-02-27 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20plasma-framework%20kf5-qt5%20SUSEQt5.10/76/
 Project:
Frameworks plasma-framework kf5-qt5 SUSEQt5.10
 Date of build:
Tue, 27 Feb 2018 22:26:18 +
 Build duration:
7 min 22 sec and counting
   JUnit Tests
  Name: (root) Failed: 7 test(s), Passed: 8 test(s), Skipped: 0 test(s), Total: 15 test(s)Failed: TestSuite.dialognativetestFailed: TestSuite.plasma-configmodeltestFailed: TestSuite.plasma-dialogqmltestFailed: TestSuite.plasma-fallbackpackagetestFailed: TestSuite.plasma-iconitemtestFailed: TestSuite.plasma-packagestructuretestFailed: TestSuite.plasma-storagetest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report33%
(6/18)35%
(55/159)35%
(55/159)27%
(3560/13341)19%
(1979/10555)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests85%
(22/26)85%
(22/26)53%
(609/1139)28%
(421/1490)src.declarativeimports.calendar0%
(0/11)0%
(0/11)0%
(0/447)0%
(0/239)src.declarativeimports.core22%
(4/18)22%
(4/18)11%
(254/2229)7%
(102/1492)src.declarativeimports.plasmacomponents0%
(0/9)0%
(0/9)0%
(0/522)0%
(0/214)src.declarativeimports.plasmaextracomponents0%
(0/5)0%
(0/5)0%
(0/44)0%
(0/27)src.declarativeimports.platformcomponents0%
(0/4)0%
(0/4)0%
(0/60)0%
(0/14)src.declarativeimports.platformcomponents.utils0%
(0/2)0%
(0/2)0%
(0/15)0%
(0/4)src.plasma55%
(12/22)55%
(12/22)41%
(1442/3488)28%
(827/2913)src.plasma.packagestructure0%
(0/7)0%
(0/7)0%
(0/141)0%
(0/14)src.plasma.private46%
(11/24)46%
(11/24)42%
(671/1612)28%
(318/1121)src.plasma.scripting0%
(0/3)0%
(0/3)0%
(0/161)0%
(0/132)src.plasmapkg0%
(0/1)0%
(0/1)0%
(0/45)0%
(0/40)src.plasmaquick42%
(5/12)42%
(5/12)27%
(553/2012)17%
(306/1761)src.plasmaquick.private33%
(1/3)33%
(1/3)28%
(31/110)36%
(5/14)src.scriptengines.qml.plasmoid0%
(0/6)0%
(0/6)0%
(0/1158)0%
(0/1056)tests.dpi0%
(0/2)0%
(0/2)0%
(0/22)0%
(0/2)tests.kplugins0%
  

KDE CI: Frameworks plasma-framework kf5-qt5 SUSEQt5.7 - Build # 79 - Still Unstable!

2018-02-27 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20plasma-framework%20kf5-qt5%20SUSEQt5.7/79/
 Project:
Frameworks plasma-framework kf5-qt5 SUSEQt5.7
 Date of build:
Tue, 27 Feb 2018 22:26:18 +
 Build duration:
7 min 4 sec and counting
   JUnit Tests
  Name: (root) Failed: 7 test(s), Passed: 8 test(s), Skipped: 0 test(s), Total: 15 test(s)Failed: TestSuite.dialognativetestFailed: TestSuite.plasma-configmodeltestFailed: TestSuite.plasma-dialogqmltestFailed: TestSuite.plasma-fallbackpackagetestFailed: TestSuite.plasma-iconitemtestFailed: TestSuite.plasma-packagestructuretestFailed: TestSuite.plasma-storagetest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report33%
(6/18)35%
(55/159)35%
(55/159)27%
(3559/13337)19%
(1978/10555)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests85%
(22/26)85%
(22/26)53%
(609/1139)28%
(421/1490)src.declarativeimports.calendar0%
(0/11)0%
(0/11)0%
(0/447)0%
(0/239)src.declarativeimports.core22%
(4/18)22%
(4/18)11%
(253/2225)7%
(101/1488)src.declarativeimports.plasmacomponents0%
(0/9)0%
(0/9)0%
(0/522)0%
(0/214)src.declarativeimports.plasmaextracomponents0%
(0/5)0%
(0/5)0%
(0/44)0%
(0/27)src.declarativeimports.platformcomponents0%
(0/4)0%
(0/4)0%
(0/60)0%
(0/14)src.declarativeimports.platformcomponents.utils0%
(0/2)0%
(0/2)0%
(0/15)0%
(0/4)src.plasma55%
(12/22)55%
(12/22)41%
(1442/3488)28%
(827/2917)src.plasma.packagestructure0%
(0/7)0%
(0/7)0%
(0/141)0%
(0/14)src.plasma.private46%
(11/24)46%
(11/24)42%
(671/1612)28%
(318/1121)src.plasma.scripting0%
(0/3)0%
(0/3)0%
(0/161)0%
(0/132)src.plasmapkg0%
(0/1)0%
(0/1)0%
(0/45)0%
(0/40)src.plasmaquick42%
(5/12)42%
(5/12)27%
(553/2012)17%
(306/1761)src.plasmaquick.private33%
(1/3)33%
(1/3)28%
(31/110)36%
(5/14)src.scriptengines.qml.plasmoid0%
(0/6)0%
(0/6)0%
(0/1158)0%
(0/1056)tests.dpi0%
(0/2)0%
(0/2)0%
(0/22)0%
(0/2)tests.kplugins0%
 

D10803: handle more tags in taglibextractor

2018-02-27 Thread Michael Heidelbach
michaelh added a comment.


  In D10803#215270 , @mgallien wrote:
  
  > This is not necessary. You could use a QImage or a QPixmap. You can put 
them in a QVariant and they should be converted to an empty QString.
  
  
  Please consider writing a plugin for KIO:PreviewJob (for the covers) anyway. 
It would be more widely useable. I'd like to see a/the cover/s in Dolphin too.
   :-)  We could also argue a little about covers being metadata or primary 
data. For starters I'd consider a reference to a cover as metadata. Still :-)

REPOSITORY
  R286 KFileMetaData

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

To: astippich, mgallien
Cc: dfaure, michaelh, ngraham, #frameworks


D10903: Preserve fragment when redirecting from http to https

2018-02-27 Thread Guo Ci Teo
guoci created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.
guoci requested review of this revision.

REPOSITORY
  R241 KIO

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

AFFECTED FILES
  src/ioslaves/http/http.cpp

To: guoci
Cc: #frameworks, michaelh


KDE CI: Frameworks plasma-framework kf5-qt5 FreeBSDQt5.9 - Build # 52 - Still Unstable!

2018-02-27 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20plasma-framework%20kf5-qt5%20FreeBSDQt5.9/52/
 Project:
Frameworks plasma-framework kf5-qt5 FreeBSDQt5.9
 Date of build:
Tue, 27 Feb 2018 22:26:17 +
 Build duration:
4 min 55 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 13 test(s), Skipped: 0 test(s), Total: 14 test(s)Failed: TestSuite.plasma-packagestructuretest

D8532: [WIP] Restrict file extractor with Seccomp

2018-02-27 Thread Michael Heidelbach
michaelh added a project: Baloo.
michaelh added a subscriber: Baloo.

REPOSITORY
  R293 Baloo

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

To: davidk, apol, ossi
Cc: #baloo, detlefe, ngraham, nicolasfella, #frameworks, ashaposhnikov, 
michaelh, spoorun, alexeymin


D8532: [WIP] Restrict file extractor with Seccomp

2018-02-27 Thread Michael Heidelbach
michaelh added a comment.


  I don't know Seccomp. But as far as I understood this, the same concers apply 
to the `baloo_file_temp_extractor` baloo-widgets is using. Naivly I suggest to 
implement this KFileMetadata because both executables are using it. I don't 
know if that is possible or reasonable though.

REPOSITORY
  R293 Baloo

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

To: davidk, apol, ossi
Cc: michaelh, #baloo, detlefe, ngraham, nicolasfella, #frameworks, 
ashaposhnikov, spoorun, alexeymin


D10910: Add unshadowed opaque variants for Breeze Light and Dark desktop themes

2018-02-27 Thread Mélanie Chauvel
achauvel created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.
achauvel requested review of this revision.

REVISION SUMMARY
  It seems to be quite popular since it got more than 1000 downloads on 
kde.store.org in a week, and each consists on a metadata.desktop file and a few 
symlinks.
  
  See here for technical details: https://framagit.org/ariasuni/breeze-flat

REPOSITORY
  R242 Plasma Framework (Library)

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

AFFECTED FILES
  src/desktoptheme/breeze-dark-flat/colors
  src/desktoptheme/breeze-dark-flat/dialogs
  src/desktoptheme/breeze-dark-flat/metadata.desktop
  src/desktoptheme/breeze-dark-flat/widgets
  src/desktoptheme/breeze-light-flat/colors
  src/desktoptheme/breeze-light-flat/dialogs
  src/desktoptheme/breeze-light-flat/metadata.desktop
  src/desktoptheme/breeze-light-flat/widgets

To: achauvel
Cc: #frameworks, michaelh


D10910: Add unshadowed opaque variants for Breeze Light and Dark desktop themes

2018-02-27 Thread Nathaniel Graham
ngraham added a comment.


  FWIW, Breeze shadows can now be turned off entirely (System Settings > 
Application Style > Window Decorations > Breeze > Click on the little Settings 
button in the corner > Shadows > Size: None), so the only thing this really 
adds is opaque backgrounds. Not sure it's worth it IMHO. Maybe instead we 
should let people turn it off in Breeze without modifications.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: achauvel
Cc: ngraham, #frameworks, michaelh