D15645: [WIP] Add scheme selection menu with a "System" entry.

[Shubham]

shubham added inline comments.

INLINE COMMENTS

> kcolorschememanager.cpp:39
>  
> +KActionMenu* KColorSchemeManagerPrivate::createSchemeMenu(const QIcon &icon, 
> const QString &name, const QString &selectedSchemeName, bool defaultEntry, 
> QObject *parent)
> +{

KActionMenu* -> KActionMenu  *

REPOSITORY
  R265 KConfigWidgets

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

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

KDE CI: Frameworks » kio » kf5-qt5 WindowsMSVCQt5.11 - Build # 27 - Still unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20WindowsMSVCQt5.11/27/
 Project:
kf5-qt5 WindowsMSVCQt5.11
 Date of build:
Sun, 23 Sep 2018 03:43:43 +
 Build duration:
1 hr 3 min and counting
   JUnit Tests
  Name: (root) Failed: 23 test(s), Passed: 32 test(s), Skipped: 0 test(s), Total: 55 test(s)Failed: TestSuite.kiocore-deletejobtestFailed: TestSuite.kiocore-jobtestFailed: TestSuite.kiocore-kfileitemtestFailed: TestSuite.kiocore-kmountpointtestFailed: TestSuite.kiocore-mkpathjobtestFailed: TestSuite.kiofilewidgets-kfilecopytomenutestFailed: TestSuite.kiofilewidgets-kfilecustomdialogtestFailed: TestSuite.kiofilewidgets-kfileplacesmodeltestFailed: TestSuite.kiofilewidgets-kfileplacesviewtestFailed: TestSuite.kiofilewidgets-kfilewidgettestFailed: TestSuite.kiofilewidgets-knewfilemenutestFailed: TestSuite.kiofilewidgets-kurlnavigatortestFailed: TestSuite.kiowidgets-accessmanagertestFailed: TestSuite.kiowidgets-accessmanagertest-qnamFailed: TestSuite.kiowidgets-dropjobtestFailed: TestSuite.kiowidgets-fileundomanagertestFailed: TestSuite.kiowidgets-kdirlistertestFailed: TestSuite.kiowidgets-kdirmodeltestFailed: TestSuite.kiowidgets-krununittestFailed: TestSuite.kiowidgets-kurifiltertestFailed: TestSuite.kiowidgets-kurlcompletiontestFailed: TestSuite.kiowidgets-kurlcompletiontest-nowaitFailed: TestSuite.kpasswdservertest

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

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.9/257/
 Project:
kf5-qt5 SUSEQt5.9
 Date of build:
Sun, 23 Sep 2018 03:43:42 +
 Build duration:
30 min and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 59 test(s), Skipped: 0 test(s), Total: 60 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesmodeltest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)66%
(261/397)66%
(261/397)53%
(31545/60003)36%
(15974/43904)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(56/56)100%
(56/56)91%
(8607/9499)42%
(3790/8928)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.core86%
(100/116)86%
(100/116)58%
(8369/14347)50%
(4670/9279)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)49%
(3846/7922)33%
(1530/4667)src.gui100%
(2/2)100%
(2/2)94%
(103/109)74%
(49/66)src.ioslaves.file100%
(5/5)100%
(5/5)52%
(528/1022)38%
(316/830)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/1331)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)52%
  

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

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.10/412/
 Project:
kf5-qt5 SUSEQt5.10
 Date of build:
Sun, 23 Sep 2018 03:43:42 +
 Build duration:
10 min and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 59 test(s), Skipped: 0 test(s), Total: 60 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesmodeltest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)66%
(261/397)66%
(261/397)52%
(31472/60002)36%
(15943/43906)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(56/56)100%
(56/56)91%
(8607/9499)42%
(3792/8928)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.core86%
(100/116)86%
(100/116)58%
(8307/14346)50%
(4647/9275)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)49%
(3846/7922)33%
(1530/4667)src.gui100%
(2/2)100%
(2/2)94%
(103/109)74%
(49/66)src.ioslaves.file100%
(5/5)100%
(5/5)52%
(527/1022)38%
(315/830)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%
(630/1331)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%

D15645: [WIP] Add scheme selection menu with a "System" entry.

[Nathaniel Graham]

ngraham added a comment.


  Very much +1 on the end goal!

INLINE COMMENTS

> amhndu wrote in kcolorschememanager.h:127
> `createSchemeSelectionMenuWithDefaultEntry` ?
> won't it be too big ?

I think it's okay. Pixels and characters are cheap. :)

REPOSITORY
  R265 KConfigWidgets

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

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

D15426: Avoid QByteArray::remove in AccessManagerReply::readData

[Nathaniel Graham]

ngraham added a comment.


  So this doesn't completely fix https://bugs.kde.org/show_bug.cgi?id=375765? 
More is needed?

REPOSITORY
  R241 KIO

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

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

D15615: bump required taglib version

[Sune Vuorela]

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

REPOSITORY
  R286 KFileMetaData

BRANCH
  bumb_taglib_version

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

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

D15614: remove usage of own TString to QString conversion function

[Sune Vuorela]

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

REPOSITORY
  R286 KFileMetaData

BRANCH
  taglib_extractor_string_conversion

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

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

D15583: [Balooctl] remove directory parent check

[Nathaniel Graham]

ngraham retitled this revision from "[Balooctl] fix directory parent check" to 
"[Balooctl] remove directory parent check".
ngraham edited the summary of this revision.
ngraham edited the test plan for this revision.

REPOSITORY
  R293 Baloo

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

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

D15583: [Balooctl] fix directory parent check

[Nathaniel Graham]

ngraham updated this revision to Diff 42152.
ngraham edited the test plan for this revision.
ngraham added a comment.


  Remove parent directory check entirely; it's not necessary

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15583?vs=42018&id=42152

BRANCH
  arcpatch-D15583

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

AFFECTED FILES
  src/tools/balooctl/configcommand.cpp

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

D15583: [Balooctl] fix directory parent check

[Nathaniel Graham]

ngraham added a comment.


  Oh wow, you're right. Let me fix `balooctl` to support that, then.

REPOSITORY
  R293 Baloo

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

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

D15697: Fix deletion of files from DAV

[Daniel Vrátil]

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

REVISION SUMMARY
  Always assume del() deletes a file, not a directory so don't try
  to append slash at the end of the path. Some servers don't like
  that.
  
  This partially restores change 1bdb286 
 
from Dawit.
  
  BUG: 355441
  FIXED-IN: 5.51

REPOSITORY
  R241 KIO

BRANCH
  master

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

AFFECTED FILES
  src/ioslaves/http/http.cpp

To: dvratil
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15697: Fix deletion of files from DAV

[Daniel Vrátil]

dvratil added a reviewer: dfaure.

REPOSITORY
  R241 KIO

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

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

D15644: Provide option to hide menu bar for Ksysguard

[Nathaniel Graham]

ngraham added a comment.


  In D15644#330281 , @lsartorelli 
wrote:
  
  > Hi Nathaniel, thanks for all the support.
  >  I can understand and agree with your concerns.
  >  Not sure but maybe an option could be to add the hide menu bar entry in 
the setting menu via kxmlguiwindow
  
  
  Ah but won't that menu become invisible once the menubar is disabled?
  
  > and another entry to unhide it maybe can be putted in the context menu on 
the window title bar.
  
  This would require work in the #kwin  
window manager, and I'm not sure if it would be technically possible or 
desirable.
  
  For now, maybe let's just display a warning like the one Kate shows so we 
don't get bug reports. It will be slightly annoying, but people who know better 
can turn it off. You can see how they do it here: 
https://cgit.kde.org/kate.git/tree/kate/katemainwindow.cpp#n596
  
  (bonus points if you then submit another patch for Gwenview to fix 
https://bugs.kde.org/show_bug.cgi?id=210620)
  
  Long-term, I would like to see Dolphin's approach with the Control button (or 
some variant of it) become more common.

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

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

D15644: Provide option to hide menu bar for Ksysguard

[Luca Sartorelli]

lsartorelli added a comment.


  Hi Nathaniel, thanks for all the support.
  I can understand and agree with your concerns.
  Not sure but maybe an option could be to add the hide menu bar entry in the 
setting menu via kxmlguiwindow and another entry to unhide it maybe can be 
putted in the context menu on the window title bar.
  Just an idea, that requires wider discussions and knowledges.

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

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

D15644: Provide option to hide menu bar for Ksysguard

[Luca Sartorelli]

lsartorelli retitled this revision from "Provide option to hide menu bar for 
Ksysguard - Bug 395349" to "Provide option to hide menu bar for Ksysguard".
lsartorelli edited the summary of this revision.

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

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

D15687: [Device Notifier] Avoid accessing attributes of stale UDIs

[Stefan Brüns]

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

INLINE COMMENTS

> ngraham wrote in FullRepresentation.qml:180
> Indentation

irks, tabs ...

REPOSITORY
  R120 Plasma Workspace

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

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

D15687: [Device Notifier] Avoid accessing attributes of stale UDIs

[Stefan Brüns]

bruns updated this revision to Diff 42149.
bruns added a comment.


  fix indentation

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15687?vs=42127&id=42149

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

AFFECTED FILES
  applets/devicenotifier/package/contents/ui/FullRepresentation.qml
  applets/devicenotifier/package/contents/ui/devicenotifier.qml

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

KDE CI: Frameworks » kio » kf5-qt5 WindowsMSVCQt5.11 - Build # 26 - Still unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20WindowsMSVCQt5.11/26/
 Project:
kf5-qt5 WindowsMSVCQt5.11
 Date of build:
Sat, 22 Sep 2018 16:24:11 +
 Build duration:
1 hr 29 min and counting
   JUnit Tests
  Name: (root) Failed: 24 test(s), Passed: 31 test(s), Skipped: 0 test(s), Total: 55 test(s)Failed: TestSuite.kiocore-deletejobtestFailed: TestSuite.kiocore-jobtestFailed: TestSuite.kiocore-kfileitemtestFailed: TestSuite.kiocore-kmountpointtestFailed: TestSuite.kiocore-mkpathjobtestFailed: TestSuite.kiofilewidgets-kfilecopytomenutestFailed: TestSuite.kiofilewidgets-kfilecustomdialogtestFailed: TestSuite.kiofilewidgets-kfileplacesmodeltestFailed: TestSuite.kiofilewidgets-kfileplacesviewtestFailed: TestSuite.kiofilewidgets-kfilewidgettestFailed: TestSuite.kiofilewidgets-knewfilemenutestFailed: TestSuite.kiofilewidgets-kurlnavigatortestFailed: TestSuite.kiowidgets-accessmanagertestFailed: TestSuite.kiowidgets-accessmanagertest-qnamFailed: TestSuite.kiowidgets-dropjobtestFailed: TestSuite.kiowidgets-fileundomanagertestFailed: TestSuite.kiowidgets-kdirlistertestFailed: TestSuite.kiowidgets-kdirmodeltestFailed: TestSuite.kiowidgets-kdynamicjobtrackernowidgetstestFailed: TestSuite.kiowidgets-krununittestFailed: TestSuite.kiowidgets-kurifiltertestFailed: TestSuite.kiowidgets-kurlcompletiontestFailed: TestSuite.kiowidgets-kurlcompletiontest-nowaitFailed: TestSuite.kpasswdservertest

D15583: [Balooctl] fix directory parent check

[Stefan Brüns]

bruns added a comment.


  In D15583#330119 , @ngraham wrote:
  
  > In D15583#330085 , @bruns wrote:
  >
  > > I think the whole `startsWith` is flawed - it should be possible to have 
a e.g. "/home/user/foo/bar" include when "/home/user/foo" has been excluded.
  >
  >
  > Hmm, I'm not sure how much that matches the user expectation. If a folder 
is explicitly marked as excluded, I think it's most commonly understood that 
all its sub-folders would also be excluded.
  >
  > I could see the case for allowing this behavior to be explicitly overridden 
by an advanced user who marks `~/foo/` as excluded and then later marks 
`~foo/bar/` as included, but that would be material for another patch I think.
  
  
  If I read the unit tests correctly 
(https://phabricator.kde.org/source/baloo/browse/master/autotests/unit/file/fileindexerconfigtest.cpp),
 this is already supported. You just can not do it via balooctl.

REPOSITORY
  R293 Baloo

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

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

D15566: Add QT_NO_NARROWING_CONVERSIONS_IN_CONNECT as default compile flags

[Laurent Montel]

This revision was automatically updated to reflect the committed changes.
Closed by commit R240:9b528012703a: Add QT_NO_NARROWING_CONVERSIONS_IN_CONNECT 
as default compile flags (authored by mlaurent).

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15566?vs=41793&id=42140

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

AFFECTED FILES
  kde-modules/KDEFrameworkCompilerSettings.cmake

To: mlaurent, dfaure
Cc: kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

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

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.9/256/
 Project:
kf5-qt5 SUSEQt5.9
 Date of build:
Sat, 22 Sep 2018 16:24:11 +
 Build duration:
17 min and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 59 test(s), Skipped: 0 test(s), Total: 60 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesmodeltest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)66%
(261/397)66%
(261/397)53%
(31553/60004)36%
(15971/43904)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(56/56)100%
(56/56)91%
(8607/9499)42%
(3786/8928)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.core86%
(100/116)86%
(100/116)58%
(8366/14347)50%
(4666/9279)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)49%
(3853/7923)33%
(1532/4667)src.gui100%
(2/2)100%
(2/2)94%
(103/109)74%
(49/66)src.ioslaves.file100%
(5/5)100%
(5/5)52%
(527/1022)38%
(315/830)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/1331)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)52%
  

D15426: Avoid QByteArray::remove in AccessManagerReply::readData

[Sune Vuorela]

svuorela added a comment.


  @fvogt mostly a unit test that ensures that all these 3 codepaths in slotData 
works. I'm not sure that the current unit tests ensures that. I might be wrong 
though.

REPOSITORY
  R241 KIO

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

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

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

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.10/411/
 Project:
kf5-qt5 SUSEQt5.10
 Date of build:
Sat, 22 Sep 2018 16:24:11 +
 Build duration:
5 min 54 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 59 test(s), Skipped: 0 test(s), Total: 60 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesmodeltest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)66%
(261/397)66%
(261/397)52%
(31456/60002)36%
(15932/43906)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(56/56)100%
(56/56)91%
(8607/9499)42%
(3790/8928)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.core86%
(100/116)86%
(100/116)58%
(8296/14346)50%
(4641/9275)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)49%
(3846/7922)33%
(1530/4667)src.gui100%
(2/2)100%
(2/2)94%
(103/109)74%
(49/66)src.ioslaves.file100%
(5/5)100%
(5/5)52%
(527/1022)38%
(315/830)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%
(630/1331)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%
  

D15426: Avoid QByteArray::remove in AccessManagerReply::readData

[Fabian Vogt]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:d2d52da38016: Avoid QByteArray::remove in 
AccessManagerReply::readData (authored by fvogt).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15426?vs=41476&id=42138

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

AFFECTED FILES
  src/widgets/accessmanagerreply_p.cpp
  src/widgets/accessmanagerreply_p.h

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

D15426: Avoid QByteArray::remove in AccessManagerReply::readData

[Fabian Vogt]

fvogt added a comment.


  @svuorela accessmanagertest already tests whether the AMR works in general. 
Do you mean a test which ensures there is no performance regression?

REPOSITORY
  R241 KIO

BRANCH
  master

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

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

D15426: Avoid QByteArray::remove in AccessManagerReply::readData

[Sune Vuorela]

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


  Though unit tests would be nice. It looks like something that quite easy 
could break if next author is not careful.

REPOSITORY
  R241 KIO

BRANCH
  master

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

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

D15277: [RFC] kio_mtp: Move MTP device handling from kioslave to kiod-module

[David Faure]

dfaure added inline comments.

INLINE COMMENTS

> elvisangelaccio wrote in kmtpd.json:10
> Currently the module is spawned by the ioslave on-demand. But imho it should 
> be possible to browse the device using only dbus, without having to open 
> dolphin first.
> 
> Can we try to autostart the module?

I don't understand what you mean here. How is a user supposed to "use only 
dbus"? ;-) Is this really about increasing overhead for every Plasma user (even 
those who never use mtp) for the benefit of an hypothetical command-line user?
I'm pretty sure qdbus can trigger on-demand loading too, even, since that's 
done by the dbus server, when using a dbus service file.

In case this is what you meant: please don't load the module at plasma startup. 
On demand is perfect. And it can be on demand of other apps than kioslaves, in 
case any app wants to do some MTP stuff (e.g. Amarok).

REPOSITORY
  R320 KIO Extras

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

To: akrutzler, elvisangelaccio, ltoscano, hetzenecker, dfaure
Cc: kde-frameworks-devel, kfm-devel, feverfew, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp

D15277: [RFC] kio_mtp: Move MTP device handling from kioslave to kiod-module

[David Faure]

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


  This is excellent work, thanks a lot for doing this. I just have "a few" 
minor comments... ;)

INLINE COMMENTS

> kio_mtp.cpp:107
>  {
> -qCDebug(LOG_KIO_MTP) << url.path();
> +qCDebug(LOG_KIO_MTP) << Q_FUNC_INFO << url.path();
>  

Don't use Q_FUNC_INFO in code.
Instead, set up your environment with %{function} in QT_MESSAGE_PATTERN.
For instance, mine says

'%{time h:mm:ss.zzz} %{appname}(%{pid}) %{if-category}%{category}: 
%{endif}%{if-debug}%{function}%{endif}%{if-warning}%{backtrace 
depth=3}%{endif}%{if-critical}%{backtrace 
depth=3}%{endif}%{if-fatal}%{backtrace depth=3}%{endif} %{message}'

See https://woboq.com/blog/nice-debug-output-with-qt.html for more info.

> kio_mtp.cpp:139
>  
> -getEntry(entry, device);
> -
> -listEntry(entry);
> -entry.clear();
> +for (const KMTPDeviceInterface *device : m_kmtpDaemon.devices()) {
> +listEntry(getEntry(device));

move m_kmtpDaemon.devices to a const local variable, to avoid a detach. Yes 
this is annoying.

> kio_mtp.cpp:320
> +const KMTPDeviceInterface *mtpDevice = 
> m_kmtpDaemon.deviceFromName(pathItems.first());
> +if (mtpDevice) {
> +KMTPStorageInterface *storage = 
> mtpDevice->storageFromDescription(pathItems.at(1));

and if mtpDevice is nullptr? This code doesn't seem to emit either error or 
finished, which is a bug.

Or if it can't be null, use Q_ASSERT instead of if() ;)

(Same in most other SlaveBase methods, please check that they all end up with 
either error() or finished())

> kio_mtp.cpp:338
> +});
> +connect(storage, &KMTPStorageInterface::copyFinished, &loop, 
> &QEventLoop::exit);
> +const int result = loop.exec();

Let's hope this signal is always ALWAYS emitted, including in all error cases...

> kio_mtp.h:79
> +
> +static UDSEntry getEntry(const KMTPDeviceInterface *device);
> +static UDSEntry getEntry(const KMTPStorageInterface *storage);

class-static private functions pollute the header file for no benefit, they 
might as well become file-static functions (i.e. remove the declarations from 
here, and remove the classname from the implementation, and ensure they're at 
the top of the file).

> kmtpd.cpp:59
> +// Release devices
> +for (const MTPDevice *device : m_devices) {
> +deviceRemoved(device->udi());

qAsConst(m_devices) to avoid a detach

> kmtpd.cpp:128
> +
> +MTPDevice *KMTPd::deviceFromUdi(const QString &udi)
> +{

this method could probably be const?

> kmtpd.cpp:153
> +if 
> (device.isDeviceInterface(Solid::DeviceInterface::PortableMediaPlayer)) {
> +qCDebug(LOG_KIOD_KMTPD) << "SOLID: New Device with udi=" << udi << 
> "";
> +

please clean up the "" stuff

> kmtpd.cpp:163
> +if (device) {
> +qCDebug(LOG_KIOD_KMTPD) << "SOLID: Device with udi=" << udi << " 
> removed. ";
> +

same

> kmtpd.cpp:165
> +
> +m_devices.removeAt(m_devices.indexOf(device));
> +delete device;

There's removeOne(device)

> mtpstorage.cpp:206
> +{
> +QDateTime dateTime = QDateTime::currentDateTime();
> +dateTime = dateTime.addSecs(timeToLive);

Any chance currentDateTimeUtc could be used everywhere? It's much faster than 
currentDateTime because it doesn't need to do timezone conversions (which use 
the awful process-global tzset()).

Just a thought, maybe you do need local times.

> mtpstorage.cpp:225
> +path.append(QLatin1Char('/'));
> +path.append(pathItems.at(i));
> +}

This method could be made much faster by locating the Nth slash and then using 
left(), to avoid the many reallocations due to multiple appends. Not sure it 
matters though.

> mtpstorage.cpp:256
> +
> +emit const_cast(static_cast *>(priv))->copyProgress(sent, total);
> +return LIBMTP_HANDLER_RETURN_OK;

Minor: this would be more readable with a MTPStorage * local variable, i.e. 
splitting this over two lines.

> mtpstorage.cpp:473
> +{
> +qCDebug(LOG_KIOD_KMTPD) << Q_FUNC_INFO << path;
> +

Please remove Q_FUNC_INFO everywhere.

> mtpstorage.cpp:499
> +
> +// big files take some time to copy, and this may lead into D-Bus 
> timeouts.
> +// therefore the actual copying is not done within the D-Bus method 
> itself but right after we return to the event loop

DBus timeouts are actually configurable, if you need to block until the copy is 
done, btw.

> mtpstorage.h:116
> + */
> +static uint16_t onDataPut(void *, void *priv, uint32_t sendlen, unsigned 
> char *data, uint32_t *putlen);
> +static int onDataProgress(const uint64_t sent, const uint64_t total, 
> const void * const priv);

you know what I think of private class-static methods ;-)

> kmtpstorageinterface.

D15687: [Device Notifier] Avoid accessing attributes of stale UDIs

[Nathaniel Graham]

ngraham added a comment.


  Code change looks pretty sane to me.

INLINE COMMENTS

> FullRepresentation.qml:180
>  actionToolTip: {
> + var types = model["Device Types"];
>  if (!mounted) {

Indentation

REPOSITORY
  R120 Plasma Workspace

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

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

D15583: [Balooctl] fix directory parent check

[Nathaniel Graham]

ngraham added a comment.


  In D15583#330085 , @bruns wrote:
  
  > I think the whole `startsWith` is flawed - it should be possible to have a 
e.g. "/home/user/foo/bar" include when "/home/user/foo" has been excluded.
  
  
  Hmm, I'm not sure how much that matches the user expectation. If a folder is 
explicitly marked as excluded, I think it's most commonly understood that all 
its sub-folders would also be excluded.
  
  I could see the case for allowing this behavior to be explicitly overridden 
by an advanced user who marks `~/foo/` as excluded and then later marks 
`~foo/bar/` as included, but that would be material for another patch I think.

REPOSITORY
  R293 Baloo

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

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

D15277: [RFC] kio_mtp: Move MTP device handling from kioslave to kiod-module

[Elvis Angelaccio]

elvisangelaccio added inline comments.

INLINE COMMENTS

> kmtpd.json:10
> +"X-KDE-DBus-ServiceName": "kmtpd",
> +"X-KDE-Kded-autoload": false,
> +"X-KDE-Kded-load-on-demand": true

Currently the module is spawned by the ioslave on-demand. But imho it should be 
possible to browse the device using only dbus, without having to open dolphin 
first.

Can we try to autostart the module?

REPOSITORY
  R320 KIO Extras

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

To: akrutzler, elvisangelaccio, ltoscano, hetzenecker, dfaure
Cc: kde-frameworks-devel, kfm-devel, feverfew, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp

D15277: [RFC] kio_mtp: Move MTP device handling from kioslave to kiod-module

[Elvis Angelaccio]

elvisangelaccio added a reviewer: dfaure.

REPOSITORY
  R320 KIO Extras

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

To: akrutzler, elvisangelaccio, ltoscano, hetzenecker, dfaure
Cc: kde-frameworks-devel, kfm-devel, feverfew, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp

D15277: [RFC] kio_mtp: Move MTP device handling from kioslave to kiod-module

[Elvis Angelaccio]

elvisangelaccio added a comment.
Herald added projects: Dolphin, Frameworks.
Herald added subscribers: kfm-devel, kde-frameworks-devel.


  From a quick look, the architecture is sound and respects what was discussed 
in T9390 . And indeed it works pretty well, 
awesome job @akrutzler!
  
  I'd even say this could already be shipped because it's better than what we 
currently ship. But I do have a few remarks:
  
  - It should be possible to generate the xml files using `qdbuscpp2xml`. And 
we should actually do that and copy the generated files in the `shared` folder 
(to be sure there aren't e.g. typos).
  - This diff contains some unrelated changes, which should be moved to 
different commits. For example, porting to the JSON protocol file, `#include` 
changes, etc.

REPOSITORY
  R320 KIO Extras

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

To: akrutzler, elvisangelaccio, ltoscano, hetzenecker
Cc: kde-frameworks-devel, kfm-devel, feverfew, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp

D15566: Add QT_NO_NARROWING_CONVERSIONS_IN_CONNECT as default compile flags

[David Faure]

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

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  add_QT_NO_NARROWING_CONVERSIONS_IN_CONNECT

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

To: mlaurent, dfaure
Cc: kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D15583: [Balooctl] fix directory parent check

[Stefan Brüns]

bruns added a comment.


  I think the whole `startsWith` is flawed - it should be possible to have a 
e.g. "/home/user/foo/bar" include when "/home/user/foo" has been excluded.

REPOSITORY
  R293 Baloo

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

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

D15566: Add QT_NO_NARROWING_CONVERSIONS_IN_CONNECT as default compile flags

[Laurent Montel]

mlaurent added a comment.


  Ping?

REPOSITORY
  R240 Extra CMake Modules

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

To: mlaurent, dfaure
Cc: kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D15687: [Device Notifier] Avoid accessing attributes of stale UDIs

[Stefan Brüns]

bruns updated this revision to Diff 42127.
bruns added a comment.


  add one more invalid access fix

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15687?vs=42123&id=42127

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

AFFECTED FILES
  applets/devicenotifier/package/contents/ui/FullRepresentation.qml
  applets/devicenotifier/package/contents/ui/devicenotifier.qml

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

D15687: [Device Notifier] Avoid accessing attributes of stale UDIs

[Stefan Brüns]

bruns created this revision.
bruns added a reviewer: Frameworks.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
bruns requested review of this revision.

REVISION SUMMARY
  When a Solid device is removed (e.g. a CD is ejected) the notifier tries
  to read the attributes although the Source for the UDI has just vanished.
  
  Fixes several QML error messages, i.e. "TypeError: Cannot read property
  '...' of undefined" and "Unable to assign [undefined] to QString".
  
  Apparently these errors also have the effect of items showing outdated
  state, i.e. optical media still being shown after ejecting it.
  
  CCBUG: 394348

TEST PLAN
  1. insert optical medium
  2. eject
  
  Without the changes, the item was stuck
  Now, the item is removed as soon as the medium is ejected
  
  Also, no more errors are logged for the devicenotifier

REPOSITORY
  R120 Plasma Workspace

BRANCH
  device_notifier_qml_fix

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

AFFECTED FILES
  applets/devicenotifier/package/contents/ui/FullRepresentation.qml
  applets/devicenotifier/package/contents/ui/devicenotifier.qml

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

D15686: Prevent highlighter from erasing selected text

[Simon Depiets]

sdepiets closed this revision.

REPOSITORY
  R246 Sonnet

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

To: sdepiets, #framework_syntax_highlighting, mlaurent, dfaure
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15686: Prevent highlighter from erasing selected text

[David Faure]

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

REPOSITORY
  R246 Sonnet

BRANCH
  master

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

To: sdepiets, #framework_syntax_highlighting, mlaurent, dfaure
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15686: Prevent highlighter from erasing selected text

[Simon Depiets]

sdepiets updated this revision to Diff 42121.
sdepiets added a comment.


  Coding style

REPOSITORY
  R246 Sonnet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15686?vs=42120&id=42121

BRANCH
  master

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

AFFECTED FILES
  src/ui/highlighter.cpp

To: sdepiets, #framework_syntax_highlighting, mlaurent, dfaure
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15686: Prevent highlighter from erasing selected text

[David Faure]

dfaure added a comment.


  Makes sense.

INLINE COMMENTS

> highlighter.cpp:198
> +if (cursor.hasSelection())
> +{
> +cursor.clearSelection();

coding style: join with previous line

REPOSITORY
  R246 Sonnet

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

To: sdepiets, #framework_syntax_highlighting, mlaurent, dfaure
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15686: Prevent highlighter from erasing selected text

[Simon Depiets]

sdepiets edited the summary of this revision.
sdepiets added reviewers: Framework: Syntax Highlighting, mlaurent, dfaure.

REPOSITORY
  R246 Sonnet

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

To: sdepiets, #framework_syntax_highlighting, mlaurent, dfaure
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15686: Prevent highlighter from erasing selected text

[Simon Depiets]

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

REPOSITORY
  R246 Sonnet

BRANCH
  master

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

AFFECTED FILES
  src/ui/highlighter.cpp

To: sdepiets
Cc: kde-frameworks-devel, michaelh, ngraham, bruns