D13880: [KMoreTools] Reduce menu hierarchy

[Dominik Haumann]

dhaumann added a comment.


  I like the idea as well. +1

REPOSITORY
  R304 KNewStuff

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

To: nicolasfella, gregormi, dhaumann, ngraham
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13880: [KMoreTools] Reduce menu hierarchy

[Nicolas Fella]

nicolasfella added a comment.


  In D13880#287481 , @gregormi wrote:
  
  > +1 for your suggestions. Downside: higher implementation effort.
  
  
  I like it, too, and I am willing to put effort in it, as soon as we agree on 
a way forward.
  
  > A bit of history: When I was creating the More submenu and the Configure... 
item I was pondering a thing: I was (and still am) not so glad about those two 
items because they are seldom used but always present in the menu. Back then, I 
thought about adding a keyboard modifier (like the Shift key) which must be 
pressed to let the Configure... item appear (similar to the context menu in 
Windows Explorer where the Shift key let more advanced items appear like Run as 
admin... etc.). I discarded the idea because of its hard discoverability.
  > 
  > Another alternative would be to somehow hook the Configuration of 
KMoreTools menus in the host applications main Configure... dialog. Downside: 
configuration is far away from the actual menu.
  > 
  > It would be cool if instead of the text "Configure..." or "More" there 
would be a small gear icon that does not take the space of a whole menu item. I 
suspect this is not possible with QMenus.
  
  I think this is quite orthogonal to the hierarchy discussion.
  
  @ngraham Your approach makes total sense for Spectacle, but would not be 
appropriate for e.g. the disk utility or find menu  in Dolphin since there is 
no menu level above to embed the actions. It's no big issue, just something we 
need to take into account when implementing.
  
  As for the implementation my idea would be:
  Additionally to the full menu we expose the top-level applications/more menu 
as List of Actions/Menu. Then the calling applications can embed them in any 
way they want.
  
  With this approach this patch would still be an improvement for cases where 
the old menu is used, e.g. the Dolphin cases I described earlier

REPOSITORY
  R304 KNewStuff

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

To: nicolasfella, gregormi, dhaumann, ngraham
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13880: [KMoreTools] Reduce menu hierarchy

[gregormi]

gregormi added a comment.


  > You are the most awesome person in the world today.
  
  +1 :-)
  
  > Pro: Very flat hierarchy, code could be simplified a lot
  >  Con: Unwanted and uninstalled tools would be present all the time.
  
  Exactly my thoughts. The idea behind the More menu was to hide (a potentially 
large number) of alternative programs which are not installed and not needed 
for the user. And even if there is only one alternative not installed program 
which the user does not want to use, always showing a "Install XYZ" seems a bit 
obtrusive to me.
  
  > https://phabricator.kde.org/D13880#286937
  
  +1 for your suggestions. Downside: higher implementation effort.
  
  A bit of history: When I was creating the More submenu and the Configure... 
item I was pondering a thing: I was (and still am) not so glad about those two 
items because they are seldom used but always present in the menu. Back then, I 
thought about adding a keyboard modifier (like the Shift key) which must be 
pressed to let the Configure... item appear (similar to the context menu in 
Windows Explorer where the Shift key let more advanced items appear like Run as 
admin... etc.). I discarded the idea because of its hard discoverability.
  
  Another alternative would be to somehow hook the Configuration of KMoreTools 
menus in the host applications main Configure... dialog. Downside: 
configuration is far away from the actual menu.
  
  It would be cool if instead of the text "Configure..." or "More" there would 
be a small gear icon that does not take the space of a whole menu item. I 
suspect this is not possible with QMenus.

REPOSITORY
  R304 KNewStuff

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

To: nicolasfella, gregormi, dhaumann, ngraham
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13885: taglibextractor: Restore extracting audio props without tags existing

[Alexander Stippich]

This revision was automatically updated to reflect the committed changes.
Closed by commit R286:41c075129d46: taglibextractor: Restore extracting audio 
props without tags existing (authored by astippich).

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13885?vs=37205=37212

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

AFFECTED FILES
  autotests/samplefiles/no-meta/test.flac
  autotests/samplefiles/no-meta/test.m4a
  autotests/samplefiles/no-meta/test.mp3
  autotests/samplefiles/no-meta/test.mpc
  autotests/samplefiles/no-meta/test.ogg
  autotests/samplefiles/no-meta/test.opus
  src/extractors/taglibextractor.cpp

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

D13808: Fix KMainWindow saving incorrect widget settings

[Mladen Milinkovic]

maxrd2 added inline comments.

INLINE COMMENTS

> broulik wrote in kmainwindow_unittest.cpp:267
> `mw->deleteLater()`?

Retried with all of these... none of them causes the failure, only closing the 
window manually cause it

  QApplication::postEvent(mw, new QDeferredDeleteEvent); // qpa/window manager 
sends this one.. with spontaneous flag tho
  QApplication::postEvent(mw, new QCloseEvent); // this one does absolutely 
nothing
  QTimer::singleShot(1000, [&](){ QApplication::sendEvent(mw, new 
QDeferredDeleteEvent); });
  mw->close();
  QTimer::singleShot(1000, [&](){ mw->close(); });
  mw->deleteLater();

REPOSITORY
  R263 KXmlGui

BRANCH
  fix-window-state-save

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

To: maxrd2, #kde_applications, dfaure, elvisangelaccio, broulik, cfeck
Cc: asturmlechner, wbauer, aacid, ngraham, kde-frameworks-devel, michaelh, bruns

D13808: Fix KMainWindow saving incorrect widget settings

[Mladen Milinkovic]

maxrd2 updated this revision to Diff 37211.
maxrd2 added a comment.


  Clened up test code. Replaced isVisible() with isHidden() test to match 
KMainWindow's condition.

REPOSITORY
  R263 KXmlGui

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13808?vs=37131=37211

BRANCH
  fix-window-state-save

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

AFFECTED FILES
  autotests/kmainwindow_unittest.cpp
  autotests/kmainwindow_unittest.h
  src/kmainwindow.cpp

To: maxrd2, #kde_applications, dfaure, elvisangelaccio, broulik, cfeck
Cc: asturmlechner, wbauer, aacid, ngraham, kde-frameworks-devel, michaelh, bruns

D13906: add missing include

[Alexander Stippich]

astippich created this revision.
astippich added a reviewer: mgallien.
Restricted Application added projects: Frameworks, Baloo.
Restricted Application added subscribers: Baloo, kde-frameworks-devel.
astippich requested review of this revision.

REVISION SUMMARY
  add the include of qmap to the header file of embeddedimageextractor

REPOSITORY
  R286 KFileMetaData

BRANCH
  add_include

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

AFFECTED FILES
  src/embeddedimagedata.h

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

D13885: taglibextractor: Restore extracting audio props without tags existing

[Alexander Stippich]

astippich edited the summary of this revision.
astippich edited the test plan for this revision.

REPOSITORY
  R286 KFileMetaData

BRANCH
  fix_empty_tags

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

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

D13816: Make KItinerary work as a static library

[Emmanuel Lepage Vallee]

lepagevalleeemmanuel added a subscriber: mart.
lepagevalleeemmanuel added a comment.


  Hello,
  
  > assuming we want static library support in other libraries/frameworks too,
  
  I can't talk for everybody, but I do want it. I actually maintain a huge 
patchset to fix static frameworks for ~30 of them. Getting it merged require 
face to face support with some people. Hopefully I can fix some of "all 
frameworks need this patch" cases at Akademy. I already merged the Kirigami 
patches and there is an "official" branch for qqc2-desktop-style too.
  
  > Or does anyone have an idea how to solve this more elegantly? :)
  
  Hi, we talked about this at Randa and QtWorldSummit last year. Sorry for 
stopping working on this after I made enough progress to make it work for me. I 
was assigned to work on other things. The clean way is using:
  
  Q_IMPORT_PLUGIN(KirigamiPlugin)
  
  Like other Qt static modules, This can be semi-automated using MOC magic and 
CMake-foo. All the magic is there 
https://github.com/ring-project/ring-kde-appimage-builder/tree/gentoo_base . 
It's not very clean, but it works. I have to catch a plane later today, so I 
can't write a lengthy comment right now.
  
  Just to say, fixing static frameworks and LTO bugs reduce the size of the KF5 
based APK/AppImage by 86% and they load 6-8 time faster when PGO/FDO is 
enabled. Of course all of this is not "free". There is a non-trivial amount of 
work before @apol SDK or mine can work with random KF5 apps. Things like 
https://bugs.kde.org/show_bug.cgi?id=387820 become real crashers instead of 
unrefined behaviors. I brute force fixed all of them using horrible hacks to 
Ring-KDE could ship on Android/Plasma_Mobile/Appimage/macOS with a 13-18mb 
(with or without breeze icons) size instead of ~185-200mb (with or without 
breeze icons).
  
  See how me and @mart patched kirigami for queues how to do it "right". The 
definitions of "right" here is "exactly compatible with Qt static SDK module 
format". There is zero documentation on that topic. I reverse engineered it 
from the final Qt .a using `gcc-nm` and reading the auto-generated code `qmake` 
produces. I began to work on a blog post last year about that, but because I 
had to move on, it was never finished. It only document about 10%, read the 
code and patches for the other 90% 
https://gist.github.com/Elv13/a5805f2c979f3f86dfad0eb16d03c388

REPOSITORY
  R1003 KItinerary: Travel Reservation handling library

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

To: vkrause, #frameworks
Cc: mart, lepagevalleeemmanuel, apol, kde-pim, dvasin, rodsevich, winterz, 
vkrause, mlaurent, knauss, dvratil

D13885: taglibextractor: Restore extracting audio props without tags existing

[Matthieu Gallien]

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


  @kossebau thanks for your work on this patch
  @astippich thanks for your quick reaction and the work on this patch

REPOSITORY
  R286 KFileMetaData

BRANCH
  fix_empty_tags

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

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

D13885: taglibextractor: Restore extracting audio props without tags existing

[Friedrich W. H. Kossebau]

kossebau added a comment.


  All updated no-meta samplefiles fail for me without the patch, but as wanted 
do not fail with the patch. So seems fine to me :) @astippich  Thanks for 
picking up this patch and creating proper test files.
  +1 though only, given I am no maintainer.

REPOSITORY
  R286 KFileMetaData

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

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

D13885: taglibextractor: Restore extracting audio props without tags existing

[Alexander Stippich]

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


  -fix the no-meta test case

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13885?vs=37148=37205

BRANCH
  fix_empty_tags

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

AFFECTED FILES
  autotests/samplefiles/no-meta/test.flac
  autotests/samplefiles/no-meta/test.m4a
  autotests/samplefiles/no-meta/test.mp3
  autotests/samplefiles/no-meta/test.mpc
  autotests/samplefiles/no-meta/test.ogg
  autotests/samplefiles/no-meta/test.opus
  src/extractors/taglibextractor.cpp

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

D13885: taglibextractor: Restore extracting audio props without tags existing

[Alexander Stippich]

astippich commandeered this revision.
astippich edited reviewers, added: kossebau; removed: astippich.

REPOSITORY
  R286 KFileMetaData

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

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

D13814: Speedup sort

[Elvis Angelaccio]

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

REPOSITORY
  R318 Dolphin

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

To: jtamate, #dolphin, #frameworks, markg, elvisangelaccio
Cc: elvisangelaccio, apol, bruns, markg, kfm-devel, spoorun, navarromorales, 
firef, andrebarros, emmanuelp

KDE CI: Frameworks kio kf5-qt5 SUSEQt5.10 - Build # 307 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.10/307/
 Project:
Frameworks kio kf5-qt5 SUSEQt5.10
 Date of build:
Thu, 05 Jul 2018 17:59:50 +
 Build duration:
21 min and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 57 test(s), Skipped: 0 test(s), Total: 58 test(s)Failed: TestSuite.kiofilewidgets-kfilewidgettest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)65%
(257/395)65%
(257/395)53%
(31692/59660)38%
(16059/42564)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(54/54)100%
(54/54)95%
(8742/9173)51%
(3845/7606)autotests.http100%
(5/5)100%
(5/5)99%
(581/582)68%
(113/166)autotests.kcookiejar100%
(1/1)100%
(1/1)91%
(179/197)72%
(49/68)src100%
(1/1)100%
(1/1)86%
(6/7)67%
(4/6)src.core84%
(98/116)84%
(98/116)58%
(8325/14402)50%
(4652/9297)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)50%
(3930/7913)34%
(1589/4659)src.gui100%
(2/2)100%
(2/2)94%
(103/109)74%
(49/66)src.ioslaves.file100%
(5/5)100%
(5/5)52%
(526/1013)39%
(316/814)src.ioslaves.file.kauth0%
(0/2)0%
(0/2)0%
(0/106)0%
(0/65)src.ioslaves.ftp0%
(0/1)0%
(0/1)0%
(0/1364)0%
(0/1414)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/245)0%
(0/144)src.ioslaves.http88%
(7/8)88%
(7/8)41%
(1770/4320)35%
(1306/3700)src.ioslaves.http.kcookiejar33%
(2/6)33%
(2/6)47%
(629/1330)55%
(620/1123)src.ioslaves.remote100%
(2/2)100%
(2/2)28%
(72/257)7%
(14/212)src.ioslaves.remote.kdedmodule0%
(0/2)0%
(0/2)0%
(0/12)100%
(0/0)src.ioslaves.telnet0%
(0/1)0%
(0/1)0%
(0/43)0%
(0/30)src.ioslaves.trash56%
(5/9)56%
(5/9)52%

KDE CI: Frameworks kio kf5-qt5 SUSEQt5.9 - Build # 156 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.9/156/
 Project:
Frameworks kio kf5-qt5 SUSEQt5.9
 Date of build:
Thu, 05 Jul 2018 17:59:50 +
 Build duration:
8 min 50 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 57 test(s), Skipped: 0 test(s), Total: 58 test(s)Failed: TestSuite.kiofilewidgets-kfilewidgettest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)65%
(257/395)65%
(257/395)53%
(31706/59659)38%
(16051/42562)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(54/54)100%
(54/54)95%
(8744/9173)51%
(3845/7606)autotests.http100%
(5/5)100%
(5/5)99%
(581/582)68%
(113/166)autotests.kcookiejar100%
(1/1)100%
(1/1)91%
(179/197)72%
(49/68)src100%
(1/1)100%
(1/1)86%
(6/7)67%
(4/6)src.core84%
(98/116)84%
(98/116)58%
(8359/14402)50%
(4658/9301)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)50%
(3926/7912)34%
(1590/4659)src.gui100%
(2/2)100%
(2/2)94%
(103/109)74%
(49/66)src.ioslaves.file100%
(5/5)100%
(5/5)52%
(525/1013)39%
(315/814)src.ioslaves.file.kauth0%
(0/2)0%
(0/2)0%
(0/106)0%
(0/65)src.ioslaves.ftp0%
(0/1)0%
(0/1)0%
(0/1364)0%
(0/1414)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/245)0%
(0/144)src.ioslaves.http88%
(7/8)88%
(7/8)41%
(1775/4320)35%
(1304/3700)src.ioslaves.http.kcookiejar33%
(2/6)33%
(2/6)47%
(628/1330)55%
(619/1123)src.ioslaves.remote100%
(2/2)100%
(2/2)28%
(72/257)7%
(14/212)src.ioslaves.remote.kdedmodule0%
(0/2)0%
(0/2)0%
(0/12)100%
(0/0)src.ioslaves.telnet0%
(0/1)0%
(0/1)0%
(0/43)0%
(0/30)src.ioslaves.trash56%
(5/9)56%
(5/9)51%

D13897: Refactor udsentrytest

[Jaime Torres Amate]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:d24a61cae282: Refactor udsentrytest (authored by jtamate).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13897?vs=37183=37199

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

AFFECTED FILES
  autotests/udsentrytest.cpp

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

KDE CI: Frameworks kio kf5-qt5 SUSEQt5.10 - Build # 306 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.10/306/
 Project:
Frameworks kio kf5-qt5 SUSEQt5.10
 Date of build:
Thu, 05 Jul 2018 17:29:45 +
 Build duration:
6 min 46 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 57 test(s), Skipped: 0 test(s), Total: 58 test(s)Failed: TestSuite.kiofilewidgets-kfilewidgettest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)65%
(257/395)65%
(257/395)53%
(31690/59697)38%
(16037/42564)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(54/54)100%
(54/54)95%
(8781/9213)51%
(3843/7606)autotests.http100%
(5/5)100%
(5/5)99%
(581/582)68%
(113/166)autotests.kcookiejar100%
(1/1)100%
(1/1)91%
(179/197)72%
(49/68)src100%
(1/1)100%
(1/1)86%
(6/7)67%
(4/6)src.core84%
(98/116)84%
(98/116)58%
(8306/14400)50%
(4647/9297)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)50%
(3925/7912)34%
(1589/4659)src.gui100%
(2/2)100%
(2/2)94%
(103/109)74%
(49/66)src.ioslaves.file100%
(5/5)100%
(5/5)52%
(525/1013)39%
(315/814)src.ioslaves.file.kauth0%
(0/2)0%
(0/2)0%
(0/106)0%
(0/65)src.ioslaves.ftp0%
(0/1)0%
(0/1)0%
(0/1364)0%
(0/1414)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/245)0%
(0/144)src.ioslaves.http88%
(7/8)88%
(7/8)41%
(1775/4320)35%
(1304/3700)src.ioslaves.http.kcookiejar33%
(2/6)33%
(2/6)47%
(629/1330)55%
(620/1123)src.ioslaves.remote100%
(2/2)100%
(2/2)28%
(72/257)7%
(14/212)src.ioslaves.remote.kdedmodule0%
(0/2)0%
(0/2)0%
(0/12)100%
(0/0)src.ioslaves.telnet0%
(0/1)0%
(0/1)0%
(0/43)0%
(0/30)src.ioslaves.trash56%
(5/9)56%
(5/9)51%
  

KDE CI: Frameworks kio kf5-qt5 SUSEQt5.9 - Build # 155 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.9/155/
 Project:
Frameworks kio kf5-qt5 SUSEQt5.9
 Date of build:
Thu, 05 Jul 2018 17:29:45 +
 Build duration:
4 min 50 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 57 test(s), Skipped: 0 test(s), Total: 58 test(s)Failed: TestSuite.kiofilewidgets-kfilewidgettest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)65%
(257/395)65%
(257/395)53%
(31751/59697)38%
(16072/42562)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(54/54)100%
(54/54)95%
(8783/9213)51%
(3846/7606)autotests.http100%
(5/5)100%
(5/5)99%
(581/582)68%
(113/166)autotests.kcookiejar100%
(1/1)100%
(1/1)91%
(179/197)72%
(49/68)src100%
(1/1)100%
(1/1)86%
(6/7)67%
(4/6)src.core84%
(98/116)84%
(98/116)58%
(8364/14400)50%
(4677/9301)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)50%
(3927/7912)34%
(1591/4659)src.gui100%
(2/2)100%
(2/2)94%
(103/109)74%
(49/66)src.ioslaves.file100%
(5/5)100%
(5/5)52%
(525/1013)39%
(315/814)src.ioslaves.file.kauth0%
(0/2)0%
(0/2)0%
(0/106)0%
(0/65)src.ioslaves.ftp0%
(0/1)0%
(0/1)0%
(0/1364)0%
(0/1414)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/245)0%
(0/144)src.ioslaves.http88%
(7/8)88%
(7/8)41%
(1775/4320)35%
(1304/3700)src.ioslaves.http.kcookiejar33%
(2/6)33%
(2/6)47%
(628/1330)55%
(619/1123)src.ioslaves.remote100%
(2/2)100%
(2/2)28%
(72/257)7%
(14/212)src.ioslaves.remote.kdedmodule0%
(0/2)0%
(0/2)0%
(0/12)100%
(0/0)src.ioslaves.telnet0%
(0/1)0%
(0/1)0%
(0/43)0%
(0/30)src.ioslaves.trash56%
(5/9)56%
(5/9)51%

D13892: Restore compatibility of UDS::insert

[Jaime Torres Amate]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:8cd6be0b1c09: Restore compatibility of UDS::insert 
(authored by jtamate).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13892?vs=37170=37198

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

AFFECTED FILES
  src/core/udsentry.cpp
  src/core/udsentry.h

To: jtamate, dfaure, #frameworks, aacid
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13898: Use non deprecated fastInsert in file.cpp (first of many to come)

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> file.cpp:909
>   * and it has a default ACL, also append that. */
>  appendACLAtoms(path, entry, type);
>  }

For UDS_ACCESS, _USER, _GROUP, we follow the symlink (i.e use the corresponding 
buff), for ACL we do not?

> file.cpp:913
>  
> -notype:
>  if (details > 0) {
> +entry.fastInsert(KIO::UDSEntry::UDS_MODIFICATION_TIME, 
> buff.st_mtime);

all the fields below are pointless in case of isBrokenSymlink, buff may contain 
anything after the failed stat call.

REPOSITORY
  R241 KIO

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

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

D13898: Use non deprecated fastInsert in file.cpp (first of many to come)

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> file.cpp:864
>  while (true) {
>  ssize_t n = readlink(path.constData(), 
> linkTargetBuffer.data(), bufferSize);
>  if (n < 0 && errno != ERANGE) {

This is broken (although not new).

`buff.st_size` is the size of the target name **without** null byte.
readlink(..., .., bufferSize) thus will typically read exactly bufferSize 
bytes, thus n == bufferSize
As a result, the 'good' case in the branches below will not pass, and a 
resize(bufferSize *= 2) and another readlink will happen.

REPOSITORY
  R241 KIO

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

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

D12820: Add KWayland virtual desktop protocol

[Marco Martin]

mart updated this revision to Diff 37197.
mart added a comment.


  - ensure there are never duplicates

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12820?vs=37149=37197

BRANCH
  mart/plasmavirtualdesktop

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

AFFECTED FILES
  .arclint
  CMakeLists.txt
  autotests/client/CMakeLists.txt
  autotests/client/test_datadevice.cpp
  autotests/client/test_plasma_virtual_desktop.cpp
  autotests/client/test_plasma_window_model.cpp
  autotests/client/test_plasmashell.cpp
  autotests/client/test_wayland_registry.cpp
  autotests/client/test_wayland_surface.cpp
  autotests/client/test_wayland_windowmanagement.cpp
  autotests/client/test_xdg_output.cpp
  autotests/client/test_xdg_shell_v6.cpp
  src/client/CMakeLists.txt
  src/client/plasmashell.cpp
  src/client/plasmashell.h
  src/client/plasmavirtualdesktop.cpp
  src/client/plasmavirtualdesktop.h
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/plasmawindowmodel.cpp
  src/client/plasmawindowmodel.h
  src/client/protocols/fullscreen-shell.xml
  src/client/protocols/outputdevice.xml
  src/client/protocols/plasma-shell.xml
  src/client/protocols/plasma-virtual-desktop.xml
  src/client/protocols/plasma-window-management.xml
  src/client/protocols/xdg-output-unstable-v1.xml
  src/client/registry.cpp
  src/client/registry.h
  src/client/textinput.h
  src/client/textinput_v0.cpp
  src/client/textinput_v2.cpp
  src/client/xdgoutput.cpp
  src/client/xdgoutput.h
  src/client/xdgshell_v6.cpp
  src/server/CMakeLists.txt
  src/server/datadevice_interface.cpp
  src/server/display.cpp
  src/server/display.h
  src/server/plasmashell_interface.cpp
  src/server/plasmashell_interface.h
  src/server/plasmavirtualdesktop_interface.cpp
  src/server/plasmavirtualdesktop_interface.h
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h
  src/server/surface_interface.cpp
  src/server/xdgforeign_v2_interface.cpp
  src/server/xdgoutput_interface.cpp
  src/server/xdgoutput_interface.h
  src/tools/mapping.txt
  tests/plasmasurfacetest.cpp

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

D13782: RFC: Ignore NTFS hidden flag for root volume

[Wolfgang Bauer]

wbauer added inline comments.

INLINE COMMENTS

> file_unix.cpp:566
> +// Bug 392913: NTFS root volume is always "hidden", 
> ignore this
> +if (ep->d_type == DT_DIR) {
> +const QString fullFilePath = path + QLatin1Char('/') 
> + filename;

Changing this to 'if (ep->d_type == DT_DIR || ep->d_type == DT_UNKNOWN) {' 
makes it work in both mentioned cases (internal harddisk mounted on boot via 
fstab, and external USB stick mounted via dolphin/solid) for me.

And hidden directories on the filesystem still are hidden.

REPOSITORY
  R241 KIO

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

To: broulik, dfaure, ngraham, yurikoles, bruns
Cc: wbauer, kde-frameworks-devel, michaelh, ngraham, bruns

D13808: Fix KMainWindow saving incorrect widget settings

[Mladen Milinkovic]

maxrd2 added inline comments.

INLINE COMMENTS

> broulik wrote in kmainwindow_unittest.cpp:267
> `mw->deleteLater()`?

tried it too... will retry all of them again with isHidden()

> broulik wrote in kmainwindow_unittest.cpp:278
> `QCOMPARE(mw->m-dock->isVisible(), true)`

mmm.. sorry i was supposed to use isHidden() test here, isVisible() is not the 
opposite
will change to QCOMPARE(mw->m-dock->isHidden(), false)

REPOSITORY
  R263 KXmlGui

BRANCH
  fix-window-state-save

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

To: maxrd2, #kde_applications, dfaure, elvisangelaccio, broulik, cfeck
Cc: wbauer, aacid, ngraham, kde-frameworks-devel, michaelh, bruns

D13782: RFC: Ignore NTFS hidden flag for root volume

[Wolfgang Bauer]

wbauer added a comment.


  In D13782#287336 , @wbauer wrote:
  
  > I tried the patch and it doesn't make a difference here, the mountpoint of 
my NTFS partition is still hidden.
  >  (it's on an internal drive and mounted to /windows/C via fstab in case it 
matters)
  >
  > Additional debug output showed that ep->d_type is not DT_DIR but 0 (which 
would be DT_UNKNOWN IIANM?) in my case.
  
  
  It does seem to matter.
  I just created an NTFS partition on an USB stick, mounted it via dolphin (to 
/run/media/...) and in this case ep->d_type is indeed DT_DIR, the patch works 
as intended in this case.
  
  So maybe you should check for DT_UNKNOWN as well...

REPOSITORY
  R241 KIO

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

To: broulik, dfaure, ngraham, yurikoles, bruns
Cc: wbauer, kde-frameworks-devel, michaelh, ngraham, bruns

D13899: KMessageWidget: use theme instead of hardcoded colours

[René J . V . Bertin]

rjvbb set the repository for this revision to R236 KWidgetsAddons.

REPOSITORY
  R236 KWidgetsAddons

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

To: rjvbb, #frameworks, #vdg
Cc: aacid, broulik, kde-frameworks-devel, michaelh, crozbo, firef, ngraham, 
bruns, skadinna, aaronhoneycutt, mbohlender

D13899: KMessageWidget: use theme instead of hardcoded colours

[René J . V . Bertin]

rjvbb updated this revision to Diff 37194.
rjvbb added a comment.


  constifies KThemeSettings.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13899?vs=37188=37194

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

AFFECTED FILES
  src/CMakeLists.txt
  src/kmessagewidget.cpp
  src/kthemesettings.cpp
  src/kthemesettings_p.h

To: rjvbb, #frameworks, #vdg
Cc: aacid, broulik, kde-frameworks-devel, michaelh, crozbo, firef, ngraham, 
bruns, skadinna, aaronhoneycutt, mbohlender

D13782: RFC: Ignore NTFS hidden flag for root volume

[Wolfgang Bauer]

wbauer added a comment.


  I tried the patch and it doesn't make a difference here, the mountpoint of my 
NTFS partition is still hidden.
  (it's on an internal drive and mounted to /windows/C via fstab in case it 
matters)
  
  Additional debug output showed that ep->d_type is not DT_DIR but 0 (which 
would be DT_UNKNOWN IIANM?) in my case.

REPOSITORY
  R241 KIO

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

To: broulik, dfaure, ngraham, yurikoles, bruns
Cc: wbauer, kde-frameworks-devel, michaelh, ngraham, bruns

D13816: Make KItinerary work as a static library

[Aleix Pol Gonzalez]

apol added a subscriber: lepagevalleeemmanuel.
apol added a comment.


  +1
  
  I know @lepagevalleeemmanuel looked into this issue as well, maybe he has 
some insights.

REPOSITORY
  R1003 KItinerary: Travel Reservation handling library

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

To: vkrause, #frameworks
Cc: lepagevalleeemmanuel, apol, kde-pim, dvasin, rodsevich, winterz, vkrause, 
mlaurent, knauss, dvratil

D13899: KMessageWidget: use theme instead of hardcoded colours

[Albert Astals Cid]

aacid added a comment.


  I know the class is not exported, but having it be "const correct" is always 
nice :)

INLINE COMMENTS

> kthemesettings_p.h:66
> + */
> +bool isValid();
> +

const?

> kthemesettings_p.h:71
> + */
> +bool contains(const QString );
> +

const?

> kthemesettings_p.h:76
> + */
> +QStringList childGroups();
> +

const?

REPOSITORY
  R236 KWidgetsAddons

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

To: rjvbb, #frameworks, #vdg
Cc: aacid, broulik, kde-frameworks-devel, michaelh, crozbo, firef, ngraham, 
bruns, skadinna, aaronhoneycutt, mbohlender

D13899: KMessageWidget: use theme instead of hardcoded colours

[René J . V . Bertin]

rjvbb set the repository for this revision to R236 KWidgetsAddons.

REPOSITORY
  R236 KWidgetsAddons

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

To: rjvbb, #frameworks, #vdg
Cc: broulik, kde-frameworks-devel, michaelh, crozbo, firef, ngraham, bruns, 
skadinna, aaronhoneycutt, mbohlender

D13899: KMessageWidget: use theme instead of hardcoded colours

[René J . V . Bertin]

rjvbb updated this revision to Diff 37188.
rjvbb marked 3 inline comments as done.
rjvbb edited the summary of this revision.
rjvbb edited the test plan for this revision.
rjvbb added a comment.


  Patch updated as requested.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13899?vs=37176=37188

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

AFFECTED FILES
  src/CMakeLists.txt
  src/kmessagewidget.cpp
  src/kthemesettings.cpp
  src/kthemesettings_p.h

To: rjvbb, #frameworks, #vdg
Cc: broulik, kde-frameworks-devel, michaelh, crozbo, firef, ngraham, bruns, 
skadinna, aaronhoneycutt, mbohlender

D13899: KMessageWidget: use theme instead of hardcoded colours

[René J . V . Bertin]

rjvbb marked 15 inline comments as done.
rjvbb added inline comments.

INLINE COMMENTS

> broulik wrote in kthemesettings.cpp:30
> This doesn't cascade to system-wide settings

I aligned to what KConfig actually does: it only considers the kdeglobals file 
in the writable generic config location. I haven't seen any evidence of 
cascading further up (searching for `kdeglobals` in the code).

> broulik wrote in kthemesettings_p.h:26
> Forward-declare, include in cpp

Not possible when using a QScopedPointer ;)

> broulik wrote in kthemesettings_p.h:85
> `readRgb`

Semantically RGB is not a word but an abbreviation (so I think I used the 
proper camelCase) but you're right that Qt uses `Rgb`.

REPOSITORY
  R236 KWidgetsAddons

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

To: rjvbb, #frameworks, #vdg
Cc: broulik, kde-frameworks-devel, michaelh, crozbo, firef, ngraham, bruns, 
skadinna, aaronhoneycutt, mbohlender

D13898: Use non deprecated fastInsert in file.cpp (first of many to come)

[David Faure]

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

REPOSITORY
  R241 KIO

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

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

D13897: Refactor udsentrytest

[David Faure]

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

REPOSITORY
  R241 KIO

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

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

D13510: Add XDG WM Base support to our XDGShell API

[Roman Gilg]

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

REPOSITORY
  R127 KWayland

BRANCH
  master

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

To: davidedmundson, #kwin, romangg
Cc: romangg, zzag, kde-frameworks-devel, michaelh, ngraham, bruns

KDE CI: Frameworks breeze-icons kf5-qt5 SUSEQt5.9 - Build # 38 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20breeze-icons%20kf5-qt5%20SUSEQt5.9/38/
 Project:
Frameworks breeze-icons kf5-qt5 SUSEQt5.9
 Date of build:
Thu, 05 Jul 2018 12:08:29 +
 Build duration:
4 min 11 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 4 test(s), Skipped: 0 test(s), Total: 5 test(s)Failed: TestSuite.scalable
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(2/2)100%
(6/6)100%
(6/6)78%
(234/300)58%
(106/182)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsdefault100%
(1/1)100%
(1/1)74%
(42/57)56%
(10/18)autotests100%
(5/5)100%
(5/5)79%
(192/243)59%
(96/164)

KDE CI: Frameworks breeze-icons kf5-qt5 SUSEQt5.10 - Build # 94 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20breeze-icons%20kf5-qt5%20SUSEQt5.10/94/
 Project:
Frameworks breeze-icons kf5-qt5 SUSEQt5.10
 Date of build:
Thu, 05 Jul 2018 12:08:29 +
 Build duration:
4 min 7 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 4 test(s), Skipped: 0 test(s), Total: 5 test(s)Failed: TestSuite.scalable
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(2/2)100%
(6/6)100%
(6/6)78%
(234/300)58%
(106/182)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsdefault100%
(1/1)100%
(1/1)74%
(42/57)56%
(10/18)autotests100%
(5/5)100%
(5/5)79%
(192/243)59%
(96/164)

KDE CI: Frameworks breeze-icons kf5-qt5 FreeBSDQt5.10 - Build # 14 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20breeze-icons%20kf5-qt5%20FreeBSDQt5.10/14/
 Project:
Frameworks breeze-icons kf5-qt5 FreeBSDQt5.10
 Date of build:
Thu, 05 Jul 2018 12:08:29 +
 Build duration:
1 min 23 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 4 test(s)Failed: TestSuite.scalable

KDE CI: Frameworks breeze-icons kf5-qt5 SUSEQt5.10 - Build # 93 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20breeze-icons%20kf5-qt5%20SUSEQt5.10/93/
 Project:
Frameworks breeze-icons kf5-qt5 SUSEQt5.10
 Date of build:
Thu, 05 Jul 2018 11:56:54 +
 Build duration:
4 min 15 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 4 test(s), Skipped: 0 test(s), Total: 5 test(s)Failed: TestSuite.scalable
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(2/2)100%
(6/6)100%
(6/6)78%
(234/300)58%
(106/182)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsdefault100%
(1/1)100%
(1/1)74%
(42/57)56%
(10/18)autotests100%
(5/5)100%
(5/5)79%
(192/243)59%
(96/164)

KDE CI: Frameworks breeze-icons kf5-qt5 SUSEQt5.9 - Build # 37 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20breeze-icons%20kf5-qt5%20SUSEQt5.9/37/
 Project:
Frameworks breeze-icons kf5-qt5 SUSEQt5.9
 Date of build:
Thu, 05 Jul 2018 11:56:54 +
 Build duration:
4 min 24 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 4 test(s), Skipped: 0 test(s), Total: 5 test(s)Failed: TestSuite.scalable
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(2/2)100%
(6/6)100%
(6/6)78%
(234/300)58%
(106/182)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsdefault100%
(1/1)100%
(1/1)74%
(42/57)56%
(10/18)autotests100%
(5/5)100%
(5/5)79%
(192/243)59%
(96/164)

KDE CI: Frameworks breeze-icons kf5-qt5 FreeBSDQt5.10 - Build # 13 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20breeze-icons%20kf5-qt5%20FreeBSDQt5.10/13/
 Project:
Frameworks breeze-icons kf5-qt5 FreeBSDQt5.10
 Date of build:
Thu, 05 Jul 2018 11:56:54 +
 Build duration:
1 min 44 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 4 test(s)Failed: TestSuite.scalable

D13898: Use non deprecated fastInsert in file.cpp (first of many to come)

[Jaime Torres Amate]

jtamate updated this revision to Diff 37184.
jtamate edited the summary of this revision.
jtamate added a comment.


  Renamed isSymLink to isBrokenSymLink.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13898?vs=37178=37184

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

AFFECTED FILES
  src/ioslaves/file/file.cpp

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

D13897: Refactor udsentrytest

[Jaime Torres Amate]

jtamate updated this revision to Diff 37183.
jtamate added a comment.


  Commas at the end.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13897?vs=37172=37183

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

AFFECTED FILES
  autotests/udsentrytest.cpp

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

D13814: Speedup sort

[Jaime Torres Amate]

jtamate updated this revision to Diff 37182.
jtamate added a comment.


  Remove the friend non-exist class.

REPOSITORY
  R318 Dolphin

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13814?vs=37056=37182

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

AFFECTED FILES
  src/kitemviews/kfileitemmodel.cpp
  src/kitemviews/kfileitemmodel.h

To: jtamate, #dolphin, #frameworks, markg, elvisangelaccio
Cc: elvisangelaccio, apol, bruns, markg, kfm-devel, spoorun, navarromorales, 
firef, andrebarros, emmanuelp

D13899: KMessageWidget: use theme instead of hardcoded colours

[Kai Uwe Broulik]

broulik added a comment.


  I like the idea

INLINE COMMENTS

> kmessagewidget.cpp:172
>  QColor bgBaseColor;
> +const QPalette palette = QGuiApplication::palette();
>  

Not using the widget's palette was intentional for Konsole or something I 
recall?

> kmessagewidget.cpp:176
> +// and therefore can't depend on any other KDE Frameworks. We thus try 
> to get
> +// the colours that interest us directly from the theme definition for 
> or from the
> +// global settings store, using KThemeSettings.

*colors

> kmessagewidget.cpp:178
> +// global settings store, using KThemeSettings.
> +// It that fails we use hardcoded colours (from kcolorscheme.cpp), or 
> the highlight 
> +// colour (for Information).

*If
*colors

> kmessagewidget.cpp:200
> +const QColor windowColor = 
> settings.readRGB(QStringLiteral("BackgroundNormal"), 
> palette.window().color());
> +QColor textColor = settings.readRGB(QStringLiteral("ForegroundNormal"), 
> palette.text().color());;
>  const QColor border = bgBaseColor;

Stray semi-colon

> kthemesettings.cpp:28
> +// Check if the KColorSchemeManager was used to activate a custom theme
> +QString themePath = qApp->property("KDE_COLOR_SCHEME_PATH").toString();
> +if (themePath.isEmpty()) {

Is this explicit check really neccessary? It should set a palette on `qApp` in 
`frameworkintegratio or `KColorSchemeManager`

> kthemesettings.cpp:30
> +if (themePath.isEmpty()) {
> +themePath = QStandardPaths::locate(QStandardPaths::ConfigLocation, 
> QStringLiteral("kdeglobals"));
> +}

This doesn't cascade to system-wide settings

> kthemesettings.cpp:32
> +}
> +if (!themePath.isEmpty()) {
> +m_settings = new QSettings(themePath, QSettings::IniFormat);

return early?

> kthemesettings.cpp:35
> +}
> +if (m_settings && !initialGroup.isEmpty()){
> +if (m_settings->childGroups().contains(initialGroup)) {

Coding style, space before brace

> kthemesettings.cpp:40
> +delete m_settings;
> +m_settings = nullptr;
> +}

Maybe also add a

  bool KThemeSettings::isValid() const
  {
  return m_settings != nullptr;
  }

> kthemesettings.cpp:76
> +{
> +if (m_settings && m_settings->contains(key)) {
> +QStringList rgb = m_settings->value(key).toStringList();

This `contains(key)` check looks unneccessary:

> If the setting doesn't exist, returns defaultValue.

so you get an invalid `QVariant` which will give you an empty `QStringList`

> kthemesettings_p.h:24
> +
> +#include 
> +#include 

Forward-declare, include in cpp

> kthemesettings_p.h:25
> +#include 
> +#include 
> +#include 

Move to cpp

> kthemesettings_p.h:26
> +#include 
> +#include 
> +#include 

Forward-declare, include in cpp

> kthemesettings_p.h:85
> + */
> +QColor readRGB(const QString , QColor defaultValue = QColor());
> +

`readRgb`

> kthemesettings_p.h:88
> +private:
> +QSettings *m_settings = nullptr;
> +};

Use `QScopedPointer` to avoid having to manually delete

REPOSITORY
  R236 KWidgetsAddons

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

To: rjvbb, #frameworks, #vdg
Cc: broulik, kde-frameworks-devel, michaelh, crozbo, firef, ngraham, bruns, 
skadinna, aaronhoneycutt, mbohlender

D13897: Refactor udsentrytest

[David Faure]

dfaure added a comment.


  I'm not a big fan of the comma at the beginning of the line (without even a 
space after the comma). Why not at end of line? I guess search-replace made 
this way easier, but it doesn't really look good ;)
  Try kde-dev-scripts's uncrustify script, it might reformat all this correctly 
;)

REPOSITORY
  R241 KIO

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

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

D13898: Use non deprecated fastInsert in file.cpp (first of many to come)

[David Faure]

dfaure added inline comments.

INLINE COMMENTS

> file.cpp:880
>  // A symlink -> follow it only if details>1
>  if (details > 1 && QT_STAT(path.constData(), ) == -1) {
> +isSymLink = true;

BTW the point is that we follow the link (by filling "buff" with this QT_STAT 
call), but then we also check that call for errors: if it returned -1, then the 
link is broken.

REPOSITORY
  R241 KIO

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

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

D13898: Use non deprecated fastInsert in file.cpp (first of many to come)

[David Faure]

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


  Yes I guess broken symlinks can have acl informations themselves too, so I 
don't mind the acl code being called now and not before.

INLINE COMMENTS

> file.cpp:841
>  mode_t access;
> +bool isSymLink = false;
> +long long size = 0LL;

Call it isBrokenSymlink, it's only set to true for *broken* symlinks.

REPOSITORY
  R241 KIO

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

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

D13895: Fix off by one error in Cache::clear

[David Edmundson]

This revision was automatically updated to reflect the committed changes.
Closed by commit R159:3d81d0b314f5: Fix off by one error in Cache::clear 
(authored by davidedmundson).

REPOSITORY
  R159 KActivities Statistics

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13895?vs=37167=37180

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

AFFECTED FILES
  src/resultmodel.cpp

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

D13752: Build solidautoeject only on FreeBSD

[Kai Uwe Broulik]

broulik added a comment.


  ping

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma, #frameworks, adridg, davidedmundson, dfaure, fvogt, ervin
Cc: anthonyfieroni, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D13899: KMessageWidget: use theme instead of hardcoded colours

[René J . V . Bertin]

rjvbb added a comment.


  A few snapshots obtained with runtime theme changes (see also D13881 
)
  
  Breeze:
  F6020556: image.png 
  
  Breeze Dark:
  F6020561: image.png 
  
  Oxygen:
  F6020566: image.png 
  
  My own theme based on the palette used by Mac OS X (coloured backgrounds are 
inappropriate here):
  F6020582: image.png 

REPOSITORY
  R236 KWidgetsAddons

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

To: rjvbb, #frameworks, #vdg
Cc: kde-frameworks-devel, michaelh, crozbo, firef, ngraham, bruns, skadinna, 
aaronhoneycutt, mbohlender

D13899: KMessageWidget: use theme instead of hardcoded colours

[René J . V . Bertin]

rjvbb created this revision.
rjvbb added reviewers: Frameworks, VDG.
rjvbb added projects: Frameworks, VDG.
Restricted Application added a subscriber: kde-frameworks-devel.
rjvbb requested review of this revision.

REVISION SUMMARY
  This is a split-off of my D13777  WIP 
project (for lack of a better word).
  
  KMessageWidget used to use the highlight colour for informational messages 
and hardcoded colours for the other 3 types. This was never ideal but 
acceptable because a user-defined colour was used for the most common message 
type and commonly accepted colours for the other types (orange for warnings, 
red for errors).
  
  This patch changes that: all colours are obtained from the current theme, if 
possible. It also reverts to using the highlight colour as a fallback for 
information messages.
  
  To obtain the theme colours a utility class `KThemeSettings` is introduced 
that provides a QSettings-based interface to the colour definitions in theme 
files or the global settings store:
  
  - if an application used the `KColorSchemeManager` it will have a 
`KDE_COLOR_SCHEME_PATH` property pointing to the actual theme definition file; 
colours will thus be read from there
  - if the property is not set, the class will attempt to get the definitions 
from the global settings store
  - if that fails too, a fallback QColor value will be returned.
  
  The ctor takes a group argument for convenience, allowing to open the group 
of interest directly with all existence checking and error handling done inside 
the class.
  The `readRGB()` method is named to indicate the fact it returns an RGB 
triplet via a QColor instance.

TEST PLAN
  Works as intended.
  I know it was suggested that the convenience class be committed separately. 
That will be easy enough but I see no point in reviewing separately from its 
initial application.
  
  All colours are obtained from the theme preferentially, even the text and 
window background colours. This is for consistency but also because I've seen 
rare glitches where the background colour would "lag" in open KMessageWidget 
instances when changing the theme at runtime.
  
  There's been a rumour that the `kdeglobals` store might disappear. If  and 
when that happens this code will have to be adapted but I don't see a reason to 
stop using the file now already.

REPOSITORY
  R236 KWidgetsAddons

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

AFFECTED FILES
  src/CMakeLists.txt
  src/kmessagewidget.cpp
  src/kthemesettings.cpp
  src/kthemesettings_p.h

To: rjvbb, #frameworks, #vdg
Cc: kde-frameworks-devel, michaelh, crozbo, firef, ngraham, bruns, skadinna, 
aaronhoneycutt, mbohlender

D13808: Fix KMainWindow saving incorrect widget settings

[Kai Uwe Broulik]

broulik added inline comments.

INLINE COMMENTS

> dfaure wrote in kmainwindow_unittest.cpp:278
> Or rather `QVERIFY(mw->m_dock->isVisible())`, that's what QVERIFY is for ;-)

:D Right, I got distracted by the `==`

REPOSITORY
  R263 KXmlGui

BRANCH
  fix-window-state-save

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

To: maxrd2, #kde_applications, dfaure, elvisangelaccio, broulik, cfeck
Cc: wbauer, aacid, ngraham, kde-frameworks-devel, michaelh, bruns

D13808: Fix KMainWindow saving incorrect widget settings

[David Faure]

dfaure added inline comments.

INLINE COMMENTS

> broulik wrote in kmainwindow_unittest.cpp:278
> `QCOMPARE(mw->m-dock->isVisible(), true)`

Or rather `QVERIFY(mw->m_dock->isVisible())`, that's what QVERIFY is for ;-)

REPOSITORY
  R263 KXmlGui

BRANCH
  fix-window-state-save

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

To: maxrd2, #kde_applications, dfaure, elvisangelaccio, broulik, cfeck
Cc: wbauer, aacid, ngraham, kde-frameworks-devel, michaelh, bruns

D13808: Fix KMainWindow saving incorrect widget settings

[Kai Uwe Broulik]

broulik added inline comments.

INLINE COMMENTS

> kmainwindow_unittest.cpp:265
> +NativeMainWindow *mw = new NativeMainWindow();
> +connect(mw, ::destroyed, [&](QObject *){ el.exit(); }); // 
> quit event loop when window's gone
> +

connect(mw, ::destroyed, , ::quit);

> kmainwindow_unittest.cpp:267
> +
> +QApplication::postEvent(mw, new QDeferredDeleteEvent);
> +

`mw->deleteLater()`?

> kmainwindow_unittest.cpp:278
> +el.processEvents();
> +QVERIFY(mw->m_dock->isVisible() == true);
> +mw->m_dock->hide();

`QCOMPARE(mw->m-dock->isVisible(), true)`

REPOSITORY
  R263 KXmlGui

BRANCH
  fix-window-state-save

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

To: maxrd2, #kde_applications, dfaure, elvisangelaccio, broulik, cfeck
Cc: wbauer, aacid, ngraham, kde-frameworks-devel, michaelh, bruns

D13808: Fix KMainWindow saving incorrect widget settings

[Wolfgang Bauer]

wbauer added a comment.


  In D13808#286965 , @maxrd2 wrote:
  
  > Relative Qt Bug is here: https://bugreports.qt.io/browse/QTBUG-69277
  >  Seems it's not their bug.
  
  
  Hm.
  Interestingly, Qt Creator has similar problems with Qt 5.11.1 that are also 
fixed by reverting that commit in Qt:
  http://bugzilla.opensuse.org/show_bug.cgi?id=1099752
  (and Qt Creator also doesn't use kxmlgui AFAIK... ;-) )
  
  And the second last comment in https://codereview.qt-project.org/#/c/227398/ 
says:
  
  > This change somehow breaks QMainWindow::saveState() when it's called from 
the destructor: toolbars are not shown on the next start.
  
  Anyway, that's not really related to this review, or whether the proposed 
change makes sense...

REPOSITORY
  R263 KXmlGui

BRANCH
  fix-window-state-save

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

To: maxrd2, #kde_applications, dfaure, elvisangelaccio, broulik, cfeck
Cc: wbauer, aacid, ngraham, kde-frameworks-devel, michaelh, bruns

D13898: Use non deprecated fastInsert in file.cpp (first of many to come)

[Jaime Torres Amate]

jtamate created this revision.
jtamate added reviewers: dfaure, Frameworks.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
jtamate requested review of this revision.

REVISION SUMMARY
  Avoid the goto using local variables.
  Use fastInsert instead of insert.
  
  NOTE: In this case the comments are confusing, is it a symbolic link or a 
broken symbolic link?
  NOTE: Does make sense to have ACLs for a  (broken) symbolic link?

TEST PLAN
  The tests pass.
  Dolphin works with broken symbolic links.

REPOSITORY
  R241 KIO

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

AFFECTED FILES
  src/ioslaves/file/file.cpp

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

D13895: Fix off by one error in Cache::clear

[Kai Uwe Broulik]

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

REPOSITORY
  R159 KActivities Statistics

BRANCH
  master

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

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

D13897: Refactor udsentrytest

[Jaime Torres Amate]

jtamate created this revision.
jtamate added reviewers: dfaure, Frameworks.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
jtamate requested review of this revision.

REVISION SUMMARY
  Use c++ initialization lists instead of <<.
  Or in other words, apply 
https://www.angrycane.com.br/en/2018/06/19/speeding-up-cornercases/

TEST PLAN
  The test passes.

REPOSITORY
  R241 KIO

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

AFFECTED FILES
  autotests/udsentrytest.cpp

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

D13782: RFC: Ignore NTFS hidden flag for root volume

[Kai Uwe Broulik]

broulik added a reviewer: bruns.

REPOSITORY
  R241 KIO

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

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

KDE CI: Frameworks kwidgetsaddons kf5-qt5 WindowsMSVCQt5.10 - Build # 35 - Still unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kwidgetsaddons%20kf5-qt5%20WindowsMSVCQt5.10/35/
 Project:
Frameworks kwidgetsaddons kf5-qt5 WindowsMSVCQt5.10
 Date of build:
Thu, 05 Jul 2018 07:41:31 +
 Build duration:
10 min and counting
   JUnit Tests
  Name: (root) Failed: 3 test(s), Passed: 15 test(s), Skipped: 0 test(s), Total: 18 test(s)Failed: TestSuite.kcolorbuttontestFailed: TestSuite.kpasswordlineedittestFailed: TestSuite.ksplittercollapserbuttontest

D13884: [KMessageWidget] Update stylesheet when palette changes

[Kai Uwe Broulik]

broulik closed this revision.
broulik added a comment.


  
https://cgit.kde.org/kwidgetsaddons.git/commit/?id=ea282678c937e0e376b391560c585e835528870f

REPOSITORY
  R236 KWidgetsAddons

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

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

KDE CI: Frameworks kwidgetsaddons kf5-qt5 WindowsMSVCQt5.10 - Build # 34 - Failure!

[CI System]

Error processing tokens: Error while parsing action 'Text/ZeroOrMore/FirstOf/Token/DelimitedToken/DelimitedToken_Action3' at input position (line 1, pos 38):
${JELLY_SCRIPT,template="html_gmail"}
 ^

hudson.remoting.ChannelClosedException: Channel "unknown": Remote call on JNLP4-connect connection from aranea.kde.org/195.201.167.114:61198 failed. The channel is closing down or has closed down

D13884: [KMessageWidget] Update stylesheet when palette changes

[Kai Uwe Broulik]

broulik added inline comments.

INLINE COMMENTS

> rjvbb wrote in kmessagewidget.cpp:57
> Nitpick: shouldn't this be called `setStyleSheet` or something similar that 
> indicates a bit better that it defines the stylesheet itself?

It doesn't take any arguments, so I explicitly did not call it `set`

REPOSITORY
  R236 KWidgetsAddons

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

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

D13892: Restore compatibility of UDS::insert

[David Faure]

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

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks, aacid
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13892: Restore compatibility of UDS::insert

[Jaime Torres Amate]

jtamate updated this revision to Diff 37170.
jtamate marked 2 inline comments as done.
jtamate added a comment.


  Added KIOCORE_DEPRECATED

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13892?vs=37163=37170

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

AFFECTED FILES
  src/core/udsentry.cpp
  src/core/udsentry.h

To: jtamate, dfaure, #frameworks, aacid
Cc: kde-frameworks-devel, michaelh, ngraham, bruns