D14302: Don't block forever in ensureKdeinitRunning

2018-07-24 Thread Jaime Torres Amate
jtamate updated this revision to Diff 38372.
jtamate edited the summary of this revision.
jtamate edited the test plan for this revision.
jtamate added a comment.


  It is my first time of a backtrace that doesn't reflect the true state of a 
program. But I've learned the lesson, in case of counter intuitive backtrace, 
take a look at the dissasembler.
  Thanks everybody, specially to @thiago.

REPOSITORY
  R271 KDBusAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14302?vs=38264&id=38372

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

AFFECTED FILES
  src/kdeinitinterface.cpp

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


D14237: Make Konqi look good in HiDPI

2018-07-24 Thread Nathaniel Graham
ngraham edited the summary of this revision.

REPOSITORY
  R263 KXmlGui

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

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


D14237: Make Konqi look good in HiDPI

2018-07-24 Thread Nathaniel Graham
ngraham updated this revision to Diff 38367.
ngraham added a comment.


  Use `zopflipng` to squeeze the image down even smaller (thanks @bruns!)

REPOSITORY
  R263 KXmlGui

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14237?vs=38106&id=38367

BRANCH
  hidpi-konqi (branched from master)

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

AFFECTED FILES
  src/aboutkde.png
  src/kaboutkdedialog_p.cpp

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


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

2018-07-24 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.9/179/
 Project:
Frameworks kio kf5-qt5 SUSEQt5.9
 Date of build:
Wed, 25 Jul 2018 04:33:36 +
 Build duration:
7 min 15 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-kfileplacesviewtest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)65%
(257/395)65%
(257/395)53%
(31687/59600)38%
(16058/42566)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(54/54)100%
(54/54)95%
(8736/9166)51%
(3857/7628)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%
(8379/14359)50%
(4664/9287)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)49%
(3893/7900)34%
(1581/4655)src.gui100%
(2/2)100%
(2/2)94%
(103/109)74%
(49/66)src.ioslaves.file100%
(5/5)100%
(5/5)52%
(527/1015)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%

KDE CI: Frameworks kio kf5-qt5 FreeBSDQt5.10 - Build # 89 - Still Unstable!

2018-07-24 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20FreeBSDQt5.10/89/
 Project:
Frameworks kio kf5-qt5 FreeBSDQt5.10
 Date of build:
Wed, 25 Jul 2018 04:33:36 +
 Build duration:
6 min 35 sec and counting
   JUnit Tests
  Name: (root) Failed: 5 test(s), Passed: 52 test(s), Skipped: 0 test(s), Total: 57 test(s)Failed: TestSuite.kiocore-jobtestFailed: TestSuite.kiocore-kmountpointtestFailed: TestSuite.kiofilewidgets-kfileplacesviewtestFailed: TestSuite.kiowidgets-kdirlistertestFailed: TestSuite.kiowidgets-kdirmodeltest

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

2018-07-24 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.10/332/
 Project:
Frameworks kio kf5-qt5 SUSEQt5.10
 Date of build:
Wed, 25 Jul 2018 04:33:36 +
 Build duration:
6 min 55 sec and counting
   JUnit Tests
  Name: (root) Failed: 2 test(s), Passed: 56 test(s), Skipped: 0 test(s), Total: 58 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesviewtestFailed: TestSuite.kiofilewidgets-kfilewidgettest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)65%
(257/395)65%
(257/395)53%
(31620/59597)38%
(16041/42568)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(54/54)100%
(54/54)95%
(8736/9166)51%
(3856/7628)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%
(8296/14355)50%
(4643/9283)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)49%
(3908/7901)34%
(1586/4655)src.gui100%
(2/2)100%
(2/2)94%
(103/109)74%
(49/66)src.ioslaves.file100%
(5/5)100%
(5/5)52%
(527/1015)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%
   

D13773: [KDirOperator] Use alternating background colors for multi-column views

2018-07-24 Thread Nathaniel Graham
ngraham closed this revision.

REPOSITORY
  R241 KIO

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

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


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

2018-07-24 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.9/178/
 Project:
Frameworks kio kf5-qt5 SUSEQt5.9
 Date of build:
Wed, 25 Jul 2018 03:34:09 +
 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-kfileplacesviewtest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)65%
(257/395)65%
(257/395)53%
(31658/59593)38%
(16059/42566)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(54/54)100%
(54/54)95%
(8736/9166)51%
(3858/7628)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%
(8340/14356)50%
(4654/9287)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)49%
(3892/7897)34%
(1581/4655)src.gui100%
(2/2)100%
(2/2)94%
(103/109)74%
(49/66)src.ioslaves.file100%
(5/5)100%
(5/5)52%
(527/1015)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%
(1770/4320)35%
(1306/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)52%
  

D13773: [KDirOperator] Use alternating background colors for multi-column views

2018-07-24 Thread Andres Betts
abetts accepted this revision.
abetts added a comment.
This revision is now accepted and ready to land.


  Looks good!

REPOSITORY
  R241 KIO

BRANCH
  alternating-row-colors-for-list-style-views (branched from master)

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

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


KDE CI: Frameworks kio kf5-qt5 FreeBSDQt5.10 - Build # 88 - Still Unstable!

2018-07-24 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20FreeBSDQt5.10/88/
 Project:
Frameworks kio kf5-qt5 FreeBSDQt5.10
 Date of build:
Wed, 25 Jul 2018 03:34:09 +
 Build duration:
6 min 9 sec and counting
   JUnit Tests
  Name: (root) Failed: 5 test(s), Passed: 52 test(s), Skipped: 0 test(s), Total: 57 test(s)Failed: TestSuite.kiocore-jobtestFailed: TestSuite.kiocore-kmountpointtestFailed: TestSuite.kiofilewidgets-kfileplacesviewtestFailed: TestSuite.kiowidgets-kdirlistertestFailed: TestSuite.kiowidgets-kdirmodeltest

KDE CI: Frameworks purpose kf5-qt5 SUSEQt5.10 - Build # 90 - Unstable!

2018-07-24 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20purpose%20kf5-qt5%20SUSEQt5.10/90/
 Project:
Frameworks purpose kf5-qt5 SUSEQt5.10
 Date of build:
Wed, 25 Jul 2018 03:34:35 +
 Build duration:
4 min 52 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 2 test(s), Skipped: 0 test(s), Total: 3 test(s)Failed: TestSuite.alternativesmodeltest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report22%
(5/23)26%
(14/53)26%
(14/53)19%
(407/2151)17%
(186/1096)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(2/2)100%
(2/2)92%
(114/124)50%
(48/96)src100%
(8/8)100%
(8/8)68%
(212/312)50%
(104/210)src.externalprocess0%
(0/2)0%
(0/2)0%
(0/136)0%
(0/88)src.fileitemactionplugin0%
(0/1)0%
(0/1)0%
(0/17)0%
(0/12)src.plugins.bluetooth0%
(0/1)0%
(0/1)0%
(0/34)0%
(0/14)src.plugins.email0%
(0/1)0%
(0/1)0%
(0/55)0%
(0/20)src.plugins.imgur0%
(0/2)0%
(0/2)0%
(0/186)0%
(0/69)src.plugins.kdeconnect0%
(0/1)0%
(0/1)0%
(0/32)0%
(0/12)src.plugins.ktp-sendfile0%
(0/1)0%
(0/1)0%
(0/28)0%
(0/12)src.plugins.nextcloud0%
(0/3)0%
(0/3)0%
(0/79)0%
(0/34)src.plugins.pastebin0%
(0/1)0%
(0/1)0%
(0/54)0%
(0/29)src.plugins.phabricator0%
(0/3)0%
(0/3)0%
(0/222)0%
(0/82)src.plugins.phabricator.quick0%
(0/5)0%
(0/5)0%
(0/99)0%
(0/62)src.plugins.phabricator.tests0%
(0/1)0%
(0/1)0%
(0/60)0%
(0/28)src.plugins.reviewboard0%
(0/3)0%
(0/3)0%
(0/233)0%
(0/76)src.plugins.reviewboard.quick0%
(0/7)0%
(0/7)0%
(0/153)0%
(0/92)src.plugins.saveas100%
(1/1)100%
(1/1)57%
(29/51)64%
(28/44)src.plugins.telegram0%
(0/1)0%
(0/1)0%
(0/45)0%
  

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

2018-07-24 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.10/331/
 Project:
Frameworks kio kf5-qt5 SUSEQt5.10
 Date of build:
Wed, 25 Jul 2018 03:34:09 +
 Build duration:
5 min 2 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-kfileplacesviewtest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)65%
(257/395)65%
(257/395)53%
(31588/59593)38%
(16027/42568)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(54/54)100%
(54/54)95%
(8736/9166)51%
(3855/7628)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%
(8281/14355)50%
(4632/9283)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)49%
(3891/7897)34%
(1580/4655)src.gui100%
(2/2)100%
(2/2)94%
(103/109)74%
(49/66)src.ioslaves.file100%
(5/5)100%
(5/5)52%
(527/1015)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%

D14118: Improve "Remove this [widget]" text

2018-07-24 Thread Nathaniel Graham
ngraham added a comment.


  Now that I look at my screenshots more closely, it seems weird to have:
  
Configure Notes...
Remove this Notes widget
  
  Now I feel like they should match, and be either:
  
Configure Notes...
Remove Notes
  
  or:
  
Configure this Notes widget...
Remove this Notes widget
  
  Now I'm leaning towards the shorter version. Thoughts?

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  remove-this-widget (branched from master)

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

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


D14118: Improve "Remove this [widget]" text

2018-07-24 Thread Nathaniel Graham
ngraham marked an inline comment as done.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  remove-this-widget (branched from master)

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

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


D14118: Improve "Remove this [widget]" text

2018-07-24 Thread Nathaniel Graham
ngraham updated this revision to Diff 38365.
ngraham added a comment.


  Use consistent casing

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14118?vs=37768&id=38365

BRANCH
  remove-this-widget (branched from master)

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

AFFECTED FILES
  src/plasma/containment.cpp
  src/plasma/private/applet_p.cpp

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


D14237: Make Konqi look good in HiDPI

2018-07-24 Thread Nathaniel Graham
ngraham added a comment.


  Funny story: after multiple attempts to use Inkscape to make an SVG of this, 
all versions were much larger than the new png! I don't think that's an option. 
@broulik, does the current approach look sane to you? I wasn't able to make it 
work simply adding an `@2x` image without code changes.

REPOSITORY
  R263 KXmlGui

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

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


D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

2018-07-24 Thread Nathaniel Graham
ngraham added a comment.


  Thanks for the history! Two questions:
  
  1. Does this mean that PlasmaComponents is semi or fully deprecated or 
"legacy", and we should be porting Plasma stuff to Kirigami instead?
  2. Since there's no Kirigami `TextField`, what do we do with this patch? Is 
there any reason why we can't improve the PC3 `TextField` to match the features 
that the PC2 `TextField` has? Or should we create a Kirigami `TextField` 
instead and port current PC2/3 clients to use it?

REPOSITORY
  R242 Plasma Framework (Library)

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

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


D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

2018-07-24 Thread David Edmundson
davidedmundson added a comment.


  Story time!
  
  First we had PlasmaComponents (based on some Nokia crap and some custom 
things from late Plasma4)
  
  
  
  Then came along Plasma 5
  
  PC2 which was a continuation of above
  
  -
  
  Then QQC1 came along
  plasmastyle which was a QQC1 theme and you could write QQC1 and style as this.
  
  PC2 was ported to wrap Plasmastyle but keep it's old API compatibility with 
some stuff removed, some stuff extending QQC1.
  
  
  
  Then QQC2 came along
  
  You don't theme items you implement them with the template magic.
  
  PC3 is a QQC2 implementation. 
  If one wrote code for QQC2 it'd use the PC3 code. 
  It was made an import as at this point in time QQC2 didn't have the style 
support it does now.
  Now it's also co-installed to /QtQuick/Controls.2/Plasma exposed as a style 
  With some intention of dropping the PC3 import in the future.
  
  ---
  
  Then came along kirigami
  
  which creates new templates for widget addons  which then use the underlying 
QQC2 style.

REPOSITORY
  R242 Plasma Framework (Library)

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

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


D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

2018-07-24 Thread Nathaniel Graham
ngraham added a comment.


  Oh. Maybe I don't know the history here. Why do we have PC2 and PC3 versions 
of this control, with the PC2 version using QQC1, and the PC3 version using 
QQC2, but not having all the features of the PC2 one? And why can't we change 
the PC3 version at all?

REPOSITORY
  R242 Plasma Framework (Library)

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

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


D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

2018-07-24 Thread David Edmundson
davidedmundson added a comment.


  > Hmm
  
  QQC2 not PC2

REPOSITORY
  R242 Plasma Framework (Library)

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

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


D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

2018-07-24 Thread Nathaniel Graham
ngraham added a comment.


  In D14345#297329 , @davidedmundson 
wrote:
  
  > PC3 was previously designed to be a drop in QQC2 implementation. There is 
currently no new API.
  >  It /might/ have changed after we made plasmastyle instead. Check with 
Marco.
  
  
  Hmm, the PC3 version doesn't seem to be a drop-in replacement right now, as 
it lacks the ability to display a clear button or a little eye toggle (when 
used as a password field). If I try to use the PC3 one in KRunner, for example, 
I get the following:
  

"file:///usr/share/plasma/look-and-feel/org.kde.breeze.desktop/contents/runcommand/RunCommand.qml"
 
 "Error loading QML file.\n70: Cannot assign to non-existent property 
\"clearButtonShown\"\n"
  
  
  
  > If we were to do this (as a subclass with a new name or whatever) we 
shouldn't have the text overlap the button that this will have.
  
  Ah, good point.

REPOSITORY
  R242 Plasma Framework (Library)

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

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


D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

2018-07-24 Thread David Edmundson
davidedmundson requested changes to this revision.
davidedmundson added a comment.
This revision now requires changes to proceed.


  PC3 was previously designed to be a drop in QQC2 implementation. There is 
currently no new API.
  It /might/ have changed after we made plasmastyle instead. Check with Marco.
  
  If we were to do this (as a subclass with a new name or whatever) we 
shouldn't have the text overlap the button that this will have.

REPOSITORY
  R242 Plasma Framework (Library)

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

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


D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

2018-07-24 Thread Nathaniel Graham
ngraham edited the test plan for this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

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


D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

2018-07-24 Thread Nathaniel Graham
ngraham edited the summary of this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

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


D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

2018-07-24 Thread Nathaniel Graham
ngraham created this revision.
ngraham added reviewers: Plasma, mart, davidedmundson.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
ngraham requested review of this revision.

REVISION SUMMARY
  The PlasmaComponents2 `TextField` can draw its own Clear button, but the 
PlasmaComponents3 version can't, which impedes our ability to port code to use 
it without losing features.
  
  This patch implements the same built-in Clear button in the PlasmaComponents3 
`TextField` so that it will be a drop-in replacement for the PlasmaComponents2 
version for text fields that need a clear button.
  
  BUG: 396828
  CCBUG: 396813
  FIXED-IN: 5.49

TEST PLAN
  Tested with KRunner patched locally in `RunCommand.qml` to use the TextField 
from PlasmaComponents3 rather than PlasmaComponents2. Results:
  
  Placeholder text is now visible (See 
https://bugs.kde.org/show_bug.cgi?id=396813):
  
  There's still a clear button when you type some text (and it still works :)):

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  pc3-textfield-clear-button (branched from master)

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

AFFECTED FILES
  src/declarativeimports/plasmacomponents3/TextField.qml

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


D14341: if an applet is invalid, it has immediately UiReadyConstraint

2018-07-24 Thread Marco Martin
mart created this revision.
mart added a reviewer: Plasma.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
mart requested review of this revision.

REVISION SUMMARY
  mirroring the behavior when the applet is created by restorecontents,
  invalid applaets get immediately uireadyconstraint.
  This solves the problem of panels not appearing when the setup js contains an 
inexistent applet,
  as the panel was waiting forever for applets to finish loading, which never 
happens

TEST PLAN
  now js startup scripts with an invalid applet have panels loading correctly.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  master

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

AFFECTED FILES
  src/plasma/private/containment_p.cpp

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


D14302: Don't block forever in ensureKdeinitRunning

2018-07-24 Thread Thiago Macieira
thiago added inline comments.

INLINE COMMENTS

> kdeinitinterface.cpp:46
>  QLockFile lock(QDir::tempPath() + QLatin1Char('/') + 
> QLatin1String("startkdeinitlock"));
> -if (!lock.tryLock()) {
> +if (!lock.tryLock(timeout)) {
>  lock.lock();

This line doesn't need changing. You're getting a tryLock() failure because the 
lock file already exists. Adding a 5 second timeout is not going to change that.

> kdeinitinterface.cpp:47
> +if (!lock.tryLock(timeout)) {
>  lock.lock();
>  if 
> (dbusDaemon->isServiceRegistered(QStringLiteral("org.kde.klauncher5"))) {

The problem is here. So we failed to lock, then we try again to lock, forever. 
Why is this code doing that?

REPOSITORY
  R271 KDBusAddons

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

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


D14302: Don't block forever in ensureKdeinitRunning

2018-07-24 Thread Thiago Macieira
thiago added a comment.


  In D14302#297159 , @jtamate wrote:
  
  >   (gdb) disass
  >   Dump of assembler code for function _ZN9QLockFile7tryLockEi:
  >  0x7f54be8bc752 <+2>: mov$0x,%eax
  >   ...
  >  0x7f54be8bc76d <+29>:test   %esi,%esi
  >   ...
  >  0x7f54be8bc772 <+34>:cmovs  %eax,%esi
  >   ...
  >  0x7f54be8bc78c <+60>:movslq %esi,%rsi
  >  0x7f54be8bc78f <+63>:callq  0x7f54be962150 

  >
  
  
  This is entirely correct: in +29 it checks  parameter (in %esi) and in +34 if 
it has the sign bit set (read: is negative), moves -1 to it. Then it does a 
sign extension from 32- to 64-bit in +60, before placing the call.
  
  qMax is working properly.
  
  >   Dump of assembler code for function 
_ZN16KDEInitInterface20ensureKdeinitRunningEv:
  >   ...
  >  0x7f54c0a23ae1 <+497>:   xor%esi,%esi
  >  0x7f54c0a23ae3 <+499>:   mov%r12,%rdi
  >  0x7f54c0a23ae6 <+502>:   callq  0x7f54c0a1e960 
<_ZN9QLockFile7tryLockEi@plt>
  
  Also correct: this passed a zero as the parameter to tryLock().
  
  Now here's the interesting thing:
  
  >  0x7f54c0a23aeb <+507>:   test   %al,%al
  >  0x7f54c0a23aed <+509>:   jne0x7f54c0a23b8d 
<_ZN16KDEInitInterface20ensureKdeinitRunningEv+669>
  >  0x7f54c0a23af3 <+515>:   mov%r12,%rdi
  >  0x7f54c0a23af6 <+518>:   callq  0x7f54c0a1e9c0 
<_ZN9QLockFile4lockEv@plt>
  >   => 0x7f54c0a23afb <+523>:   mov%rbx,%rdi
  
  Note where the => is pointing to. It's not to the return from tryLock(), but 
to the return to lock(). We've been looking at the wrong line.
  
  That also means the patch doesn't make sense, because it's changing the line 
that is already working.

REPOSITORY
  R271 KDBusAddons

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

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


D14302: Don't block forever in ensureKdeinitRunning

2018-07-24 Thread Thiago Macieira
thiago added a comment.


  Quickly checking on openSUSE Tumbleweed
  
  In D14302#297185 , @lvsouza wrote:
  
  > I think I know what is happening. This line
  >
  > QDeadlineTimer timer(qMax(timeout, -1));// QDT only takes -1 as 
"forever"
  >
  > passes the result of qMax() to QDeadlineTimer's constructor. That 
constructor receives a quint64. Since qMax() is a template:
  
  
  The constructor takes a qint64, not a quint64.
  
  > inline const T &qMax(const T &a, const T &b) { return (a < b) ? b : a; }
  > 
  > it will use the type of the assigned variable (quint64 in this case) as T 
and casting -1 to INT64_MAX. Changing the line to:
  
  Again, incorrect. It will use the type T, which must be the same for both 
arguments. The timeout parameter is int and the -1 literal is int. So the 
comparison is performed in int, which should return 0.

REPOSITORY
  R271 KDBusAddons

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

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


D14302: Don't block forever in ensureKdeinitRunning

2018-07-24 Thread Lamarque Souza
lvsouza added a comment.


  I think I know what is happening. This line
  
  QDeadlineTimer timer(qMax(timeout, -1));// QDT only takes -1 as "forever"
  
  passes the result of qMax() to QDeadlineTimer's constructor. That constructor 
receives a quint64. Since qMax() is a template:
  
  inline const T &qMax(const T &a, const T &b) { return (a < b) ? b : a; }
  
  it will use the type of the assigned variable (quint64 in this case) as T and 
casting -1 to INT64_MAX. Changing the line to:
  
  QDeadlineTimer timer(qMax(timeout, qint64(-1)));
  
  should solve the problem. If it does not then this should work:
  
  qint64 maxTimeout = qMax(timeout, -1);
  QDeadlineTimer timer(maxTimeout);

REPOSITORY
  R271 KDBusAddons

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

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


D14302: Don't block forever in ensureKdeinitRunning

2018-07-24 Thread Jaime Torres Amate
jtamate added a comment.


(gdb) disass
Dump of assembler code for function _ZN9QLockFile7tryLockEi:
   0x7f54be8bc750 <+0>: push   %r15
   0x7f54be8bc752 <+2>: mov$0x,%eax
   0x7f54be8bc757 <+7>: mov$0x1,%edx
   0x7f54be8bc75c <+12>:push   %r14
   0x7f54be8bc75e <+14>:push   %r13
   0x7f54be8bc760 <+16>:push   %r12
   0x7f54be8bc762 <+18>:push   %rbp
   0x7f54be8bc763 <+19>:push   %rbx
   0x7f54be8bc764 <+20>:mov$0x64,%ebx
   0x7f54be8bc769 <+25>:sub$0x58,%rsp
   0x7f54be8bc76d <+29>:test   %esi,%esi
   0x7f54be8bc76f <+31>:mov(%rdi),%rbp
   0x7f54be8bc772 <+34>:cmovs  %eax,%esi
   0x7f54be8bc775 <+37>:lea0x20(%rsp),%r12
   0x7f54be8bc77a <+42>:lea0x30(%rsp),%r14
   0x7f54be8bc77f <+47>:lea0x10(%rsp),%r15
   0x7f54be8bc784 <+52>:lea0x18(%rsp),%r13
   0x7f54be8bc789 <+57>:mov%r12,%rdi
   0x7f54be8bc78c <+60>:movslq %esi,%rsi
   0x7f54be8bc78f <+63>:callq  0x7f54be962150 

   0x7f54be8bc794 <+68>:mov%rbp,%rdi
   0x7f54be8bc797 <+71>:callq  0x7f54be910d00 
<_ZN16QLockFilePrivate11tryLock_sysEv>
   0x7f54be8bc79c <+76>:mov%eax,0x10(%rbp)
   0x7f54be8bc79f <+79>:cmp$0x1,%eax
   0x7f54be8bc7a2 <+82>:je 0x7f54be8bc7d0 
<_ZN9QLockFile7tryLockEi+128>
   0x7f54be8bc7a4 <+84>:test   %eax,%eax
   0x7f54be8bc7a6 <+86>:je 0x7f54be8bc9a0 
<_ZN9QLockFile7tryLockEi+592>
   0x7f54be8bc7ac <+92>:cmp$0x3,%eax
   0x7f54be8bc7af <+95>:ja 0x7f54be8bc968 
<_ZN9QLockFile7tryLockEi+536>
   0x7f54be8bc7b5 <+101>:   add$0x58,%rsp
   0x7f54be8bc7b9 <+105>:   xor%eax,%eax
   0x7f54be8bc7bb <+107>:   pop%rbx
   0x7f54be8bc7bc <+108>:   pop%rbp
   0x7f54be8bc7bd <+109>:   pop%r12
   0x7f54be8bc7bf <+111>:   pop%r13
   0x7f54be8bc7c1 <+113>:   pop%r14
   0x7f54be8bc7c3 <+115>:   pop%r15
   0x7f54be8bc7c5 <+117>:   retq   
   0x7f54be8bc7c6 <+118>:   nopw   %cs:0x0(%rax,%rax,1)
   0x7f54be8bc7d0 <+128>:   cmpb   $0x0,0x14(%rbp)
   0x7f54be8bc7d4 <+132>:   jne0x7f54be8bc968 
<_ZN9QLockFile7tryLockEi+536>
   0x7f54be8bc7da <+138>:   mov%rbp,%rdi
   0x7f54be8bc7dd <+141>:   callq  0x7f54be8bc3a0 
<_ZNK16QLockFilePrivate17isApparentlyStaleEv>
   0x7f54be8bc7e2 <+146>:   test   %al,%al
   0x7f54be8bc7e4 <+148>:   je 0x7f54be8bc968 
<_ZN9QLockFile7tryLockEi+536>
   0x7f54be8bc7ea <+154>:   mov%r14,%rdi
   0x7f54be8bc7ed <+157>:   callq  0x7f54be807360 

   0x7f54be8bc7f2 <+162>:   mov%rbp,%rsi
   0x7f54be8bc7f5 <+165>:   mov%r15,%rdi
   0x7f54be8bc7f8 <+168>:   callq  0x7f54be8b2640 
<_ZN9QFileInfoC2ERK7QString>
   0x7f54be8bc7fd <+173>:   mov%r15,%rsi
   0x7f54be8bc800 <+176>:   mov%r13,%rdi
   0x7f54be8bc803 <+179>:   callq  0x7f54be8b4160 

   0x7f54be8bc808 <+184>:   mov%r13,%rsi
   0x7f54be8bc80b <+187>:   mov%r14,%rdi
   0x7f54be8bc80e <+190>:   callq  0x7f54be806e90 

   0x7f54be8bc813 <+195>:   mov%r13,%rdi
   0x7f54be8bc816 <+198>:   mov%al,0x8(%rsp)
   0x7f54be8bc81a <+202>:   callq  0x7f54be804a00 

   0x7f54be8bc81f <+207>:   mov%r15,%rdi
   0x7f54be8bc822 <+210>:   callq  0x7f54be8b43a0 

   0x7f54be8bc827 <+215>:   mov%r14,%rdi
   0x7f54be8bc82a <+218>:   callq  0x7f54be804a00 

   0x7f54be8bc82f <+223>:   movzbl 0x8(%rsp),%eax
   0x7f54be8bc834 <+228>:   test   %al,%al
   0x7f54be8bc836 <+230>:   je 0x7f54be8bc8c0 
<_ZN9QLockFile7tryLockEi+368>
   0x7f54be8bc83c <+236>:   lea0x1564dd(%rip),%rax# 
0x7f54bea12d20
   0x7f54be8bc843 <+243>:   pxor   %xmm0,%xmm0
   0x7f54be8bc847 <+247>:   movq   $0x2,0x30(%rsp)
   0x7f54be8bc850 <+256>:   mov%rax,0x48(%rsp)
   0x7f54be8bc855 <+261>:   mov0x0(%rbp),%rax
   0x7f54be8bc859 <+265>:   movups %xmm0,0x38(%rsp)
   0x7f54be8bc85e <+270>:   mov%rax,0x18(%rsp)
   0x7f54be8bc863 <+275>:   mov(%rax),%edx
   0x7f54be8bc865 <+277>:   add$0x1,%edx
   0x7f54be8bc868 <+280>:   cmp$0x1,%edx
   0x7f54be8bc86b <+283>:   ja 0x7f54be8bc9f1 
<_ZN9QLockFile7tryLockEi+673>
   0x7f54be8bc871 <+289>:   mov%r13,%rdi
   0x7f54be8bc874 <+292>:   callq  0x7f54be83d530 
   0x7f54be8bc879 <+297>:   mov%rax,%rdx
   0x7f54be8bc87c <+300>:   lea0x20a715(%rip),%rsi# 
0x7f54beac6f98
   0x7f54be8bc883 <+307>:   mov%r14,%rdi
  

D14302: Don't block forever in ensureKdeinitRunning

2018-07-24 Thread Thiago Macieira
thiago added a comment.


  > timer = {t1 = 9223372036854775807, t2 = 0, type = 1}
  
  This is indeed Forever. How did it get there?
  
  I showed in my debug session that the QDeadlineTimer is passed zero, and then 
it does initialise properly. So I'm now beginning to question the compiler. 
Specifically, this line:
  
QDeadlineTimer timer(qMax(timeout, -1));// QDT only takes -1 as 
"forever"
  
  Is it possible that the compiler miscompiled qMax and caused a value of -1 to 
be passed?
  
  Alternatively, could timeout be -1? The call site is (before your patch):
  
if (!lock.tryLock()) {
  
  Which should get the default value of 0. Could it be getting -1 somehow? 
Maybe your distribution patched Qt?
  
  Can you provide the disassembly of those two functions? Just run "disass" in 
gdb from those two frames and paste here.
  
  > In D14302#297119 , @thiago wrote:
  > 
  >> No, because your statement is incorrect. setPreciseRemainingTime **does** 
assign to t1:
  >>
  >>   t1 += secs + toSecsAndNSecs(nsecs).first;
  >>   
  > 
  > 
  > Yes, but this is assuming t1 = 0, I mean, it is not t1 = secs (not with 
+=).
  
  Wrong line. It does assign here:
  
*this = current(timerType);
  
  And even if it didn't, the value in the timer  is very specific (t1 == 
INT64_MAX and t2 == 0). The chance of getting that from uninitialised data is 
too small to be considered.

REPOSITORY
  R271 KDBusAddons

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

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


D14302: Don't block forever in ensureKdeinitRunning

2018-07-24 Thread Jaime Torres Amate
jtamate added a comment.


  In D14302#297120 , @thiago wrote:
  
  > Can you print the contents of the timer object inside tryLock()?
  
  
  
  
(gdb) info frame
Stack level 2, frame at 0x7ffdda9967c0:
 rip = 0x7f54be8bc985 in QLockFile::tryLock (io/qlockfile.cpp:274); saved 
rip = 0x7f54c0a23afb
 called by frame at 0x7ffdda996860, caller of frame at 0x7ffdda996730
 source language c++.
 Arglist at 0x7ffdda996728, args: this=, timeout=
 Locals at 0x7ffdda996728, Previous frame's sp is 0x7ffdda9967c0
 Saved registers:
  rbx at 0x7ffdda996788, rbp at 0x7ffdda996790, r12 at 0x7ffdda996798, r13 
at 0x7ffdda9967a0, r14 at 0x7ffdda9967a8, r15 at 0x7ffdda9967b0, rip at 
0x7ffdda9967b8
(gdb) info locals
remainingTime = 
d = 0x557a916a98f0
timer = {t1 = 9223372036854775807, t2 = 0, type = 1}
sleepTime = 6400
(gdb) up
#3  0x7f54c0a23afb in KDEInitInterface::ensureKdeinitRunning() () from 
/usr/lib64/libKF5DBusAddons.so.5
(gdb) info locals
No symbol table info available.
(gdb) info frame
Stack level 3, frame at 0x7ffdda996860:
 rip = 0x7f54c0a23afb in KDEInitInterface::ensureKdeinitRunning(); saved 
rip = 0x7f54c2d16ffb
 called by frame at 0x7ffdda9968a0, caller of frame at 0x7ffdda9967c0
 Arglist at 0x7ffdda9967b8, args: 
 Locals at 0x7ffdda9967b8, Previous frame's sp is 0x7ffdda996860
 Saved registers:
  rbx at 0x7ffdda996828, rbp at 0x7ffdda996830, r12 at 0x7ffdda996838, r13 
at 0x7ffdda996840, r14 at 0x7ffdda996848, r15 at 0x7ffdda996850, rip at 
0x7ffdda996858
  
  
  
  In D14302#297119 , @thiago wrote:
  
  > No, because your statement is incorrect. setPreciseRemainingTime **does** 
assign to t1:
  >
  >   t1 += secs + toSecsAndNSecs(nsecs).first;
  >   
  
  
  Yes, but this is assuming t1 = 0, I mean, it is not t1 = secs (not with 
+=).

REPOSITORY
  R271 KDBusAddons

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

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


D14302: Don't block forever in ensureKdeinitRunning

2018-07-24 Thread Thiago Macieira
thiago added a comment.


  Can you print the contents of the timer object inside tryLock()?

REPOSITORY
  R271 KDBusAddons

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

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


D14302: Don't block forever in ensureKdeinitRunning

2018-07-24 Thread Thiago Macieira
thiago added a comment.


  In D14302#296671 , @jtamate wrote:
  
  > Another hypothesis:
  >
  >   QDeadlineTimer::QDeadlineTimer(qint64 msecs, Qt::TimerType type) 
Q_DECL_NOTHROW
  >   : t2(0)
  >   {
  >   setRemainingTime(msecs, type);
  >   }
  >  
  >
  >
  > As t1 is not initialized and QDeadlineTimer::setPreciseRemainingTime only 
add to t1, it could be any value instead of 0?.
  >
  > Should I open a Qt bug?
  
  
  No, because your statement is incorrect. setPreciseRemainignTime *does* 
assign to t1:
  
*this = current(timerType);
if (QDeadlineTimerNanosecondsInT2) {
t1 += secs + toSecsAndNSecs(nsecs).first;
t2 += toSecsAndNSecs(nsecs).second;

REPOSITORY
  R271 KDBusAddons

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

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


D14308: [Exe Thumbnailer] Improve icon selection algorithm

2018-07-24 Thread Stefan BrĂ¼ns
bruns added inline comments.

INLINE COMMENTS

> icoutils_common.cpp:28
> +{
> +// compute difference of areas
> +const qreal desiredAspectRatio = (desiredHeight > 0) ? desiredWidth / 
> static_cast(desiredHeight) : 0;

Can not see an area calculation here ...

> broulik wrote in icoutils_common.cpp:33
> No idea, really.

I think the formula is a little bit to ad-hoc, and actually wrong.

We have 2 different notable cases. `desiredAspectRatio` obviously is a 
constant, this leaves us with `candidateAspectRatio` and `delta`

1. all candiates have exactly the same aspect ratio. The difference below is 
constant, thus has no influence on the sorting. Sorting is determined by 
`delta`alone (good).
2. candidates have different aspect ratios. Any candidate with a slightly 
different aspect ratio is immediately heavily penalized, and thus discarded 
(bad).

Think of e.g. a requested size of 96x96, and two candidates, 32x32 and 128x64. 
The small one has a `distance` of 128, while the second one has a distance 
25032. I don't think that's wanted here.

REPOSITORY
  R320 KIO Extras

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

To: broulik, #frameworks, dfaure, ngraham, pali, vonreth, antlarr
Cc: bruns


D14331: Avoid pixmap format test failure with serialising QIcons

2018-07-24 Thread David Edmundson
davidedmundson created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
davidedmundson requested review of this revision.

REVISION SUMMARY
  Test fails as converting an ARGB formatted pixmap with only RGB data
  gets optimised into RGB resulting in a test fail.
  
  QImage constructors can explicitly set the format.

REPOSITORY
  R127 KWayland

BRANCH
  master

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

AFFECTED FILES
  autotests/client/test_wayland_windowmanagement.cpp

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


KDE CI: Frameworks kwayland kf5-qt5 SUSEQt5.9 - Build # 45 - Fixed!

2018-07-24 Thread CI System
BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks%20kwayland%20kf5-qt5%20SUSEQt5.9/45/
 Project:
Frameworks kwayland kf5-qt5 SUSEQt5.9
 Date of build:
Tue, 24 Jul 2018 11:37:23 +
 Build duration:
12 min and counting
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 45 test(s), Skipped: 0 test(s), Total: 45 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report63%
(5/8)92%
(233/254)92%
(233/254)87%
(24412/28013)53%
(9655/18061)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests.client100%
(41/41)100%
(41/41)99%
(11455/11525)50%
(6083/12113)autotests.server100%
(5/5)100%
(5/5)99%
(353/356)49%
(169/344)src.client99%
(71/72)99%
(71/72)85%
(5680/6711)65%
(1496/2308)src.compat100%
(2/2)100%
(2/2)100%
(81/81)100%
(0/0)src.server99%
(114/115)99%
(114/115)87%
(6843/7846)66%
(1907/2883)src.tools0%
(0/2)0%
(0/2)0%
(0/693)0%
(0/272)src.tools.testserver0%
(0/3)0%
(0/3)0%
(0/69)0%
(0/10)tests0%
(0/14)0%
(0/14)0%
(0/732)0%
(0/131)

KDE CI: Frameworks kwayland kf5-qt5 SUSEQt5.10 - Build # 72 - Fixed!

2018-07-24 Thread CI System
BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks%20kwayland%20kf5-qt5%20SUSEQt5.10/72/
 Project:
Frameworks kwayland kf5-qt5 SUSEQt5.10
 Date of build:
Tue, 24 Jul 2018 11:37:23 +
 Build duration:
7 min 10 sec and counting
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 45 test(s), Skipped: 0 test(s), Total: 45 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report63%
(5/8)92%
(233/254)92%
(233/254)87%
(24410/28013)53%
(9653/18061)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests.client100%
(41/41)100%
(41/41)99%
(11456/11525)50%
(6082/12113)autotests.server100%
(5/5)100%
(5/5)99%
(353/356)49%
(169/344)src.client99%
(71/72)99%
(71/72)85%
(5677/6711)65%
(1495/2308)src.compat100%
(2/2)100%
(2/2)100%
(81/81)100%
(0/0)src.server99%
(114/115)99%
(114/115)87%
(6843/7846)66%
(1907/2883)src.tools0%
(0/2)0%
(0/2)0%
(0/693)0%
(0/272)src.tools.testserver0%
(0/3)0%
(0/3)0%
(0/69)0%
(0/10)tests0%
(0/14)0%
(0/14)0%
(0/732)0%
(0/131)

KDE CI: Frameworks kwayland kf5-qt5 FreeBSDQt5.10 - Build # 34 - Still Unstable!

2018-07-24 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kwayland%20kf5-qt5%20FreeBSDQt5.10/34/
 Project:
Frameworks kwayland kf5-qt5 FreeBSDQt5.10
 Date of build:
Tue, 24 Jul 2018 11:37:23 +
 Build duration:
5 min 50 sec and counting
   JUnit Tests
  Name: (root) Failed: 12 test(s), Passed: 28 test(s), Skipped: 0 test(s), Total: 40 test(s)Failed: TestSuite.kwayland-testCompositorFailed: TestSuite.kwayland-testDataDeviceFailed: TestSuite.kwayland-testDataSourceFailed: TestSuite.kwayland-testRegionFailed: TestSuite.kwayland-testShmPoolFailed: TestSuite.kwayland-testSubCompositorFailed: TestSuite.kwayland-testSubSurfaceFailed: TestSuite.kwayland-testWaylandConnectionThreadFailed: TestSuite.kwayland-testWaylandRegistryFailed: TestSuite.kwayland-testWaylandServerDisplayFailed: TestSuite.kwayland-testWaylandShellFailed: TestSuite.kwayland-testWaylandSurface

D14291: Cleanup RemoteAccess buffers on aboutToBeUnbound instead of object destruction

2018-07-24 Thread David Edmundson
This revision was automatically updated to reflect the committed changes.
Closed by commit R127:6aded9ccf603: Cleanup RemoteAccess buffers on 
aboutToBeUnbound instead of object destruction (authored by davidedmundson).

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14291?vs=38233&id=38328

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

AFFECTED FILES
  src/server/remote_access_interface.cpp

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


D6513: Add support for Attica tags support

2018-07-24 Thread Arjen Hiemstra
ahiemstra requested changes to this revision.
ahiemstra added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> atticaprovider.cpp:283
> +}
> +if(filterAcceptsDownloads && checker.filterAccepts(content.tags())) {
> +mCachedContent.insert(content.id(), content);

Wouldn't it be simpler to first check if the content should be considered at 
all, and then check if one of the downloads is valid? So something like:

  if(!checker.filterAccepts(content.tags()) {
continue
  }
  
  foreach download {
   if(downloadsChecker.filterAccepts(download.tags) {
 add download
 break
   }
  }

> engine.cpp:131
>  
> +d->tagFilter = group.readEntry("TagFilter").split(QStringLiteral(","));
> +if(d->tagFilter.length() == 1 && d->tagFilter.at(0).isEmpty()) {

Considering KConfig has methods to read and write lists, wouldn't it be better 
to use those?

> engine.h:190
> + * out entries marked as ghns_exclude=1. To retain this when setting a 
> custom
> + * filter, add "ghns_exclude!=1" as one of the filters.
> + *

Would it make sense to add an `addTagFilter` method or similar for convenience 
that appends instead of replaces? Or maybe make the setter a bit more 
intelligent so it is harder to forget adding the `ghns_exclude!=1` filter? 
Right now, if I need to do anything with the tag filters, I will constantly 
need to remember to add the ghns_exclude filter.

Maybe even completely separate the ghns_exclude filter from this setter and 
make a separate "also show excluded content" switch that removes the filter?

> engine.h:227
> + */
> +void setTagFilter(QStringList filter);
> +/**

`const QStringList&`

(Also applies to setDownloadTagFilter below)

> entryinternal.cpp:165
> +
> +void KNSCore::EntryInternal::setTags(const QStringList& tags)
> +{

Nitpick, but this doesn't match with the coding style of the declaration 
(`const QStringList& tags`  vs `const QStringList &tags`)

> tagsfilterchecker.cpp:43
> +public:
> +Validator(const QString& tag, const QString value) {
> +m_tag = tag;

I think you mean `const QString& value` ?

> tagsfilterchecker.cpp:98
> +QString value = filter.mid(tag.length() + 2);
> +Validator* val = validators.value(tag, nullptr);
> +if(val)

Wouldn't this be simpler?

  if(!validators.contains(tag)) {
validators[tag] = new EqualityValidator(tag, "");
  }
  validators.value(tag)->m_acceptedValues << value;

REPOSITORY
  R304 KNewStuff

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

To: leinir, #knewstuff, apol, #kde_store, whiting, ahiemstra
Cc: ahiemstra, kde-frameworks-devel, #knewstuff, michaelh, ZrenBot, ngraham, 
bruns


D6512: Add support for proposed tags addition in OCS 1.7

2018-07-24 Thread Arjen Hiemstra
ahiemstra accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R235 Attica

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

To: leinir, #knewstuff, apol, whiting, #kde_store, ahiemstra
Cc: ahiemstra, ngraham, kde-frameworks-devel, #kde_store, michaelh, ZrenBot, 
bruns, akiraohgaki, alexanderschmidt, siyuandong, ronaldv, mikesomov, starbuck, 
sebas


D6512: Add support for proposed tags addition in OCS 1.7

2018-07-24 Thread Arjen Hiemstra
ahiemstra added a comment.


  Right, so leave it to application users how to deal with that, maybe later on 
add some convenience functions. Sounds sensible. :)

REPOSITORY
  R235 Attica

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

To: leinir, #knewstuff, apol, whiting, #kde_store
Cc: ahiemstra, ngraham, kde-frameworks-devel, #kde_store, michaelh, ZrenBot, 
bruns, akiraohgaki, alexanderschmidt, siyuandong, ronaldv, mikesomov, starbuck, 
sebas


D6512: Add support for proposed tags addition in OCS 1.7

2018-07-24 Thread Dan Leinir Turthra Jensen
leinir added a comment.


  In D6512#296329 , @ahiemstra wrote:
  
  > T6133  suggests that tags are formatted 
as "group##key=value" or something similar. Wouldn't it make sense to handle 
parsing that format here as well? Or are tags more intended as exact matches?
  
  
  The group is more a convenience concept than anything else - i think it'd be 
nice to have something to do something with that, but not in this patch (the 
"something" needs a more proper definition, and i'd rather not have this 
functionality, which we need for the curating work that's being undertaken on 
the store, wait any longer than we have to...).
  
  To answer the specific question, during the matching phase the group##key is 
supposed to be considered canonical, and the groupings are more for pairing of 
similar content when putting things together. As a random sort of idea, one 
might imagine having something like a validator which matches the concept "must 
have something defined in group x", but that's why i've done the validators the 
way i have, there's nothing stopping us from putting that in when we have a 
purpose for the functionality :)

REPOSITORY
  R235 Attica

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

To: leinir, #knewstuff, apol, whiting, #kde_store
Cc: ahiemstra, ngraham, kde-frameworks-devel, #kde_store, michaelh, ZrenBot, 
bruns, akiraohgaki, alexanderschmidt, siyuandong, ronaldv, mikesomov, starbuck, 
sebas


D14308: [Exe Thumbnailer] Improve icon selection algorithm

2018-07-24 Thread Kai Uwe Broulik
broulik updated this revision to Diff 38296.
broulik added a comment.


  - Include `` just in case

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14308?vs=38271&id=38296

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

AFFECTED FILES
  thumbnail/icoutils_common.cpp

To: broulik, #frameworks, dfaure, ngraham, pali, vonreth, antlarr