Re: Review Request 129299: Warn on startup about ambiguous shortcuts (with an exception for Shift+Delete)

[Aleix Pol Gonzalez]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129299/#review100456
---



How about putting it in QDebug?
Message boxes could make us all miserable.


src/kxmlguiwindow.cpp (line 313)


How come this is the _only_ exception?


- Aleix Pol Gonzalez


On Oct. 31, 2016, 8:18 p.m., Albert Astals Cid wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129299/
> ---
> 
> (Updated Oct. 31, 2016, 8:18 p.m.)
> 
> 
> Review request for KDE Frameworks, David Faure and Elvis Angelaccio.
> 
> 
> Repository: kxmlgui
> 
> 
> Description
> ---
> 
> Add a warning at the createGui stage about ambiguous shortcuts being found in 
> the same action collection.
> 
> This is usually a developer issue, but the error message about ambiguity will 
> only show up when someone tries to use the shortcut, so it is relatively easy 
> to miss if you do not try all your actions via a shortcut.
> 
> Also if the involved shortcut is one of the non primary shortcuts of 
> edit_cut, just give it away, since it's usually Shift+Delete being fought 
> over.
> 
> 
> Diffs
> -
> 
>   src/kxmlguiwindow.cpp 519fb26 
> 
> Diff: https://git.reviewboard.kde.org/r/129299/diff/
> 
> 
> Testing
> ---
> 
> gwenview now defaults to Shift+Delete being "Hard delete" and not "Cut", if 
> you remove the 
>   if (action == editCutAction || existingShortcutAction == editCutAction) {
> part, you get warning about the actions involved
> 
> 
> Thanks,
> 
> Albert Astals Cid
> 
>

Re: Review Request 129298: RFC: supporting dependencies on KPackage

[Aleix Pol Gonzalez]

> On Oct. 31, 2016, 6:19 p.m., Marco Martin wrote:
> > autotests/data/testpackagesdep/metadata.json, line 14
> > 
> >
> > if kns ends up using ids, maybe the server should be specified as well, 
> > as the id would be server-dependent?

I'm not sure, either way we need changes in the KNS API, as the current search 
in place won't work. I'd prefer if we could use rdn-like notation on kns.

I don't think it should be server-dependent. If anything, if the user changes 
the contents server, it might not find the component.


> On Oct. 31, 2016, 6:19 p.m., Marco Martin wrote:
> > src/kpackage/private/packagejobthread.cpp, line 436
> > 
> >
> > would this need a local database of installed packages/dependencies?

No. I don't want a KPackage db. Both the distro and KNS have some of this stuff 
in place.
That said, I'm not sure how I expect it to work, some research there would be 
welcome.


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129298/#review100442
---


On Oct. 31, 2016, 6:09 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129298/
> ---
> 
> (Updated Oct. 31, 2016, 6:09 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma, Dan Leinir Turthra Jensen, and 
> Marco Martin.
> 
> 
> Repository: kpackage
> 
> 
> Description
> ---
> 
> Makes it possible to specify components to be installed together with a 
> KPackage. They will be specified by a url, we'll have handlers for any kind, 
> making reasonably extensible and doesn't tie us to a technology.
> 
> In this repository I created two Proof of Concept of such handlers, one for 
> the appstream urls and one for KNS: kde:scratch/apol/kpackage-install-helpers
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 395b16e 
>   autotests/data/testpackagesdep/contents/ui/main.qml PRE-CREATION 
>   autotests/data/testpackagesdep/metadata.json PRE-CREATION 
>   autotests/data/testpackagesdep/testpackagesdep.testappdataxml PRE-CREATION 
>   src/kpackage/config-package.h.cmake 61f2f0c 
>   src/kpackage/private/packagejobthread.cpp 0a0cc01 
>   src/kpackage/private/packagejobthread_p.h 1babf0b 
> 
> Diff: https://git.reviewboard.kde.org/r/129298/diff/
> 
> 
> Testing
> ---
> 
> None. just builds. It's a proof of concept, not even the test I added was 
> tested, it was just to display what it could look like.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

Jenkins-kde-ci: knotifyconfig master kf5-qt5 » Linux,gcc - Build # 247 - Failure!

[no-reply]

GENERAL INFO

BUILD FAILURE
Build URL: 
https://build.kde.org/job/knotifyconfig%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/247/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 31 Oct 2016 21:16:36 +
Build duration: 1 min 4 sec

CHANGE SET
No changes

Jenkins-kde-ci: kio master kf5-qt5 » Linux,gcc - Build # 254 - Still Unstable!

[no-reply]

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/kio%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/254/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 31 Oct 2016 20:57:58 +
Build duration: 14 min

CHANGE SET
Revision a2aef4c9b3d43ffc45019f6cccb40721e0594aae by cfeck: (Removed duplicate 
clear button)
  change: edit src/widgets/kurlrequesterdialog.cpp
  change: edit src/widgets/kurlrequesterdialog.h


JUNIT RESULTS

Name: (root) Failed: 1 test(s), Passed: 51 test(s), Skipped: 0 test(s), Total: 
52 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesmodeltest

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 21/21 (100%)FILES 270/340 (79%)CLASSES 270/340 (79%)LINE 28918/51406 
(56%)CONDITIONAL 15618/38129 (41%)

By packages
  
autotests
FILES 66/66 (100%)CLASSES 66/66 (100%)LINE 7542/8128 
(93%)CONDITIONAL 4023/8518 (47%)
autotests.http
FILES 9/9 (100%)CLASSES 9/9 (100%)LINE 543/544 
(100%)CONDITIONAL 200/336 (60%)
autotests.kcookiejar
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 179/198 (90%)CONDITIONAL 
60/90 (67%)
src.core
FILES 96/116 (83%)CLASSES 96/116 (83%)LINE 7787/14137 
(55%)CONDITIONAL 4256/9067 (47%)
src.core.kssl
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 35/93 (38%)CONDITIONAL 
3/6 (50%)
src.filewidgets
FILES 25/36 (69%)CLASSES 25/36 (69%)LINE 3327/7561 
(44%)CONDITIONAL 1220/4383 (28%)
src.gui
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 104/110 (95%)CONDITIONAL 
46/72 (64%)
src.ioslaves.file
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 443/841 (53%)CONDITIONAL 
329/725 (45%)
src.ioslaves.http
FILES 8/8 (100%)CLASSES 8/8 (100%)LINE 1763/3783 
(47%)CONDITIONAL 1259/3434 (37%)
src.ioslaves.http.kcookiejar
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 620/781 (79%)CONDITIONAL 
602/831 (72%)
src.ioslaves.trash
FILES 7/9 (78%)CLASSES 7/9 (78%)LINE 713/1155 (62%)CONDITIONAL 
375/753 (50%)
src.ioslaves.trash.tests
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 686/764 (90%)CONDITIONAL 
445/936 (48%)
src.kioslave
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 14/27 (52%)CONDITIONAL 
5/10 (50%)
src.kntlm
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 373/385 (97%)CONDITIONAL 
111/138 (80%)
src.kpasswdserver
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 377/594 (63%)CONDITIONAL 
280/580 (48%)
src.kpasswdserver.autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 283/286 (99%)CONDITIONAL 
144/256 (56%)
src.urifilters.fixhost
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 25/34 (74%)CONDITIONAL 
36/54 (67%)
src.urifilters.ikws
FILES 5/10 (50%)CLASSES 5/10 (50%)LINE 240/725 (33%)CONDITIONAL 
146/542 (27%)
src.urifilters.localdomain
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 19/26 (73%)CONDITIONAL 
14/22 (64%)
src.urifilters.shorturi
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 239/268 (89%)CONDITIONAL 
333/414 (80%)
src.widgets
FILES 32/64 (50%)CLASSES 32/64 (50%)LINE 3606/10966 
(33%)CONDITIONAL 1731/6962 (25%)

Jenkins-kde-ci: kio master stable-kf5-qt5 » Linux,gcc - Build # 256 - Still Unstable!

[no-reply]

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/kio%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/256/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 31 Oct 2016 20:57:58 +
Build duration: 8 min 37 sec

CHANGE SET
Revision a2aef4c9b3d43ffc45019f6cccb40721e0594aae by cfeck: (Removed duplicate 
clear button)
  change: edit src/widgets/kurlrequesterdialog.cpp
  change: edit src/widgets/kurlrequesterdialog.h


JUNIT RESULTS

Name: (root) Failed: 2 test(s), Passed: 50 test(s), Skipped: 0 test(s), Total: 
52 test(s)Failed: TestSuite.kiocore-threadtestFailed: 
TestSuite.kiofilewidgets-kfileplacesmodeltest

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 21/21 (100%)FILES 269/339 (79%)CLASSES 269/339 (79%)LINE 28884/51371 
(56%)CONDITIONAL 15602/38107 (41%)

By packages
  
autotests
FILES 65/65 (100%)CLASSES 65/65 (100%)LINE 7507/8093 
(93%)CONDITIONAL 4009/8496 (47%)
autotests.http
FILES 9/9 (100%)CLASSES 9/9 (100%)LINE 543/544 
(100%)CONDITIONAL 200/336 (60%)
autotests.kcookiejar
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 179/198 (90%)CONDITIONAL 
60/90 (67%)
src.core
FILES 96/116 (83%)CLASSES 96/116 (83%)LINE 7778/14137 
(55%)CONDITIONAL 4249/9067 (47%)
src.core.kssl
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 35/93 (38%)CONDITIONAL 
3/6 (50%)
src.filewidgets
FILES 25/36 (69%)CLASSES 25/36 (69%)LINE 3329/7561 
(44%)CONDITIONAL 1222/4383 (28%)
src.gui
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 104/110 (95%)CONDITIONAL 
46/72 (64%)
src.ioslaves.file
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 443/841 (53%)CONDITIONAL 
329/725 (45%)
src.ioslaves.http
FILES 8/8 (100%)CLASSES 8/8 (100%)LINE 1763/3783 
(47%)CONDITIONAL 1259/3434 (37%)
src.ioslaves.http.kcookiejar
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 620/781 (79%)CONDITIONAL 
602/831 (72%)
src.ioslaves.trash
FILES 7/9 (78%)CLASSES 7/9 (78%)LINE 713/1155 (62%)CONDITIONAL 
375/753 (50%)
src.ioslaves.trash.tests
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 686/764 (90%)CONDITIONAL 
445/936 (48%)
src.kioslave
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 14/27 (52%)CONDITIONAL 
5/10 (50%)
src.kntlm
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 373/385 (97%)CONDITIONAL 
111/138 (80%)
src.kpasswdserver
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 377/594 (63%)CONDITIONAL 
280/580 (48%)
src.kpasswdserver.autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 283/286 (99%)CONDITIONAL 
144/256 (56%)
src.urifilters.fixhost
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 25/34 (74%)CONDITIONAL 
36/54 (67%)
src.urifilters.ikws
FILES 5/10 (50%)CLASSES 5/10 (50%)LINE 240/725 (33%)CONDITIONAL 
146/542 (27%)
src.urifilters.localdomain
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 19/26 (73%)CONDITIONAL 
14/22 (64%)
src.urifilters.shorturi
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 239/268 (89%)CONDITIONAL 
333/414 (80%)
src.widgets
FILES 32/64 (50%)CLASSES 32/64 (50%)LINE 3614/10966 
(33%)CONDITIONAL 1734/6962 (25%)

Jenkins-kde-ci: kio master kf5-qt5 » Linux,gcc - Build # 253 - Unstable!

[no-reply]

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/kio%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/253/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 31 Oct 2016 20:38:18 +
Build duration: 12 min

CHANGE SET
Revision af0105c22ec80e411bf226e77f40987464e478d3 by Antonio Rojas: (Fix 
editing autostart entries when /usr/local/share/applications doesn#039;t)
  change: edit src/widgets/kpropertiesdialog.cpp


JUNIT RESULTS

Name: (root) Failed: 1 test(s), Passed: 51 test(s), Skipped: 0 test(s), Total: 
52 test(s)Failed: TestSuite.kiocore-ktcpsockettest

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 21/21 (100%)FILES 270/340 (79%)CLASSES 270/340 (79%)LINE 29207/51414 
(57%)CONDITIONAL 15953/38129 (42%)

By packages
  
autotests
FILES 66/66 (100%)CLASSES 66/66 (100%)LINE 7799/8128 
(96%)CONDITIONAL 4357/8518 (51%)
autotests.http
FILES 9/9 (100%)CLASSES 9/9 (100%)LINE 543/544 
(100%)CONDITIONAL 200/336 (60%)
autotests.kcookiejar
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 179/198 (90%)CONDITIONAL 
60/90 (67%)
src.core
FILES 96/116 (83%)CLASSES 96/116 (83%)LINE 7803/14137 
(55%)CONDITIONAL 4263/9067 (47%)
src.core.kssl
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 35/93 (38%)CONDITIONAL 
3/6 (50%)
src.filewidgets
FILES 25/36 (69%)CLASSES 25/36 (69%)LINE 3343/7561 
(44%)CONDITIONAL 1212/4383 (28%)
src.gui
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 104/110 (95%)CONDITIONAL 
46/72 (64%)
src.ioslaves.file
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 443/841 (53%)CONDITIONAL 
329/725 (45%)
src.ioslaves.http
FILES 8/8 (100%)CLASSES 8/8 (100%)LINE 1763/3783 
(47%)CONDITIONAL 1259/3434 (37%)
src.ioslaves.http.kcookiejar
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 620/781 (79%)CONDITIONAL 
602/831 (72%)
src.ioslaves.trash
FILES 7/9 (78%)CLASSES 7/9 (78%)LINE 713/1155 (62%)CONDITIONAL 
375/753 (50%)
src.ioslaves.trash.tests
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 686/764 (90%)CONDITIONAL 
445/936 (48%)
src.kioslave
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 14/27 (52%)CONDITIONAL 
5/10 (50%)
src.kntlm
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 373/385 (97%)CONDITIONAL 
111/138 (80%)
src.kpasswdserver
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 377/594 (63%)CONDITIONAL 
280/580 (48%)
src.kpasswdserver.autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 283/286 (99%)CONDITIONAL 
146/256 (57%)
src.urifilters.fixhost
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 25/34 (74%)CONDITIONAL 
36/54 (67%)
src.urifilters.ikws
FILES 5/10 (50%)CLASSES 5/10 (50%)LINE 240/725 (33%)CONDITIONAL 
146/542 (27%)
src.urifilters.localdomain
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 19/26 (73%)CONDITIONAL 
14/22 (64%)
src.urifilters.shorturi
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 239/268 (89%)CONDITIONAL 
333/414 (80%)
src.widgets
FILES 32/64 (50%)CLASSES 32/64 (50%)LINE 3606/10974 
(33%)CONDITIONAL 1731/6962 (25%)

Jenkins-kde-ci: kio master stable-kf5-qt5 » Linux,gcc - Build # 255 - Still Unstable!

[no-reply]

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/kio%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/255/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 31 Oct 2016 20:38:18 +
Build duration: 6 min 56 sec

CHANGE SET
Revision af0105c22ec80e411bf226e77f40987464e478d3 by Antonio Rojas: (Fix 
editing autostart entries when /usr/local/share/applications doesn#039;t)
  change: edit src/widgets/kpropertiesdialog.cpp


JUNIT RESULTS

Name: (root) Failed: 1 test(s), Passed: 51 test(s), Skipped: 0 test(s), Total: 
52 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesmodeltest

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 21/21 (100%)FILES 270/340 (79%)CLASSES 270/340 (79%)LINE 28916/51414 
(56%)CONDITIONAL 15617/38129 (41%)

By packages
  
autotests
FILES 66/66 (100%)CLASSES 66/66 (100%)LINE 7539/8128 
(93%)CONDITIONAL 4022/8518 (47%)
autotests.http
FILES 9/9 (100%)CLASSES 9/9 (100%)LINE 543/544 
(100%)CONDITIONAL 200/336 (60%)
autotests.kcookiejar
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 179/198 (90%)CONDITIONAL 
60/90 (67%)
src.core
FILES 96/116 (83%)CLASSES 96/116 (83%)LINE 7778/14137 
(55%)CONDITIONAL 4251/9067 (47%)
src.core.kssl
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 35/93 (38%)CONDITIONAL 
3/6 (50%)
src.filewidgets
FILES 25/36 (69%)CLASSES 25/36 (69%)LINE 3329/7561 
(44%)CONDITIONAL 1222/4383 (28%)
src.gui
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 104/110 (95%)CONDITIONAL 
46/72 (64%)
src.ioslaves.file
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 443/841 (53%)CONDITIONAL 
329/725 (45%)
src.ioslaves.http
FILES 8/8 (100%)CLASSES 8/8 (100%)LINE 1763/3783 
(47%)CONDITIONAL 1259/3434 (37%)
src.ioslaves.http.kcookiejar
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 620/781 (79%)CONDITIONAL 
602/831 (72%)
src.ioslaves.trash
FILES 7/9 (78%)CLASSES 7/9 (78%)LINE 713/1155 (62%)CONDITIONAL 
375/753 (50%)
src.ioslaves.trash.tests
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 686/764 (90%)CONDITIONAL 
445/936 (48%)
src.kioslave
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 14/27 (52%)CONDITIONAL 
5/10 (50%)
src.kntlm
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 373/385 (97%)CONDITIONAL 
111/138 (80%)
src.kpasswdserver
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 377/594 (63%)CONDITIONAL 
280/580 (48%)
src.kpasswdserver.autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 283/286 (99%)CONDITIONAL 
144/256 (56%)
src.urifilters.fixhost
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 25/34 (74%)CONDITIONAL 
36/54 (67%)
src.urifilters.ikws
FILES 5/10 (50%)CLASSES 5/10 (50%)LINE 240/725 (33%)CONDITIONAL 
146/542 (27%)
src.urifilters.localdomain
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 19/26 (73%)CONDITIONAL 
14/22 (64%)
src.urifilters.shorturi
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 239/268 (89%)CONDITIONAL 
333/414 (80%)
src.widgets
FILES 32/64 (50%)CLASSES 32/64 (50%)LINE 3614/10974 
(33%)CONDITIONAL 1734/6962 (25%)

Re: Review Request 129280: Fix editing autostart items

[Antonio Rojas]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129280/
---

(Updated Oct. 31, 2016, 8:37 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and David Faure.


Changes
---

Submitted with commit af0105c22ec80e411bf226e77f40987464e478d3 by Antonio Rojas 
to branch master.


Bugs: 371194
https://bugs.kde.org/show_bug.cgi?id=371194


Repository: kio


Description
---

QFileInfo.canonicalFilePath() returns an empty string for non-existant 
directories, which matches any .desktop file path and makes KPropertiesDialog 
try to save it to the wrong location. Make sure that the dir exists before 
trying to match it in the .desktop file path.


Diffs
-

  src/widgets/kpropertiesdialog.cpp b570a85 

Diff: https://git.reviewboard.kde.org/r/129280/diff/


Testing
---

Autostart items can be edited and saved again from the systemsettings autostart 
KCM.


Thanks,

Antonio Rojas

Re: Review Request 129299: Warn on startup about ambiguous shortcuts (with an exception for Shift+Delete)

[Albert Astals Cid]

> On Oct. 31, 2016, 6:50 p.m., Elvis Angelaccio wrote:
> > src/kxmlguiwindow.cpp, line 327
> > 
> >
> > I think it would be better if this were not blocking (i.e. creating the 
> > QMessageBox manually and showing it with `open()`).
> > 
> > Otherwise the users get a messagebox instead of the main window of the 
> > app they started.

I don't know, ideally it should only be only enabled for developer builds but 
we do not have such flag, right?


- Albert


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129299/#review100448
---


On Oct. 31, 2016, 7:18 p.m., Albert Astals Cid wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129299/
> ---
> 
> (Updated Oct. 31, 2016, 7:18 p.m.)
> 
> 
> Review request for KDE Frameworks, David Faure and Elvis Angelaccio.
> 
> 
> Repository: kxmlgui
> 
> 
> Description
> ---
> 
> Add a warning at the createGui stage about ambiguous shortcuts being found in 
> the same action collection.
> 
> This is usually a developer issue, but the error message about ambiguity will 
> only show up when someone tries to use the shortcut, so it is relatively easy 
> to miss if you do not try all your actions via a shortcut.
> 
> Also if the involved shortcut is one of the non primary shortcuts of 
> edit_cut, just give it away, since it's usually Shift+Delete being fought 
> over.
> 
> 
> Diffs
> -
> 
>   src/kxmlguiwindow.cpp 519fb26 
> 
> Diff: https://git.reviewboard.kde.org/r/129299/diff/
> 
> 
> Testing
> ---
> 
> gwenview now defaults to Shift+Delete being "Hard delete" and not "Cut", if 
> you remove the 
>   if (action == editCutAction || existingShortcutAction == editCutAction) {
> part, you get warning about the actions involved
> 
> 
> Thanks,
> 
> Albert Astals Cid
> 
>

Re: Review Request 129299: Warn on startup about ambiguous shortcuts (with an exception for Shift+Delete)

[Albert Astals Cid]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129299/
---

(Updated Oct. 31, 2016, 7:18 p.m.)


Review request for KDE Frameworks, David Faure and Elvis Angelaccio.


Repository: kxmlgui


Description
---

Add a warning at the createGui stage about ambiguous shortcuts being found in 
the same action collection.

This is usually a developer issue, but the error message about ambiguity will 
only show up when someone tries to use the shortcut, so it is relatively easy 
to miss if you do not try all your actions via a shortcut.

Also if the involved shortcut is one of the non primary shortcuts of edit_cut, 
just give it away, since it's usually Shift+Delete being fought over.


Diffs (updated)
-

  src/kxmlguiwindow.cpp 519fb26 

Diff: https://git.reviewboard.kde.org/r/129299/diff/


Testing
---

gwenview now defaults to Shift+Delete being "Hard delete" and not "Cut", if you 
remove the 
  if (action == editCutAction || existingShortcutAction == editCutAction) {
part, you get warning about the actions involved


Thanks,

Albert Astals Cid

Re: Review Request 129299: Warn on startup about ambiguous shortcuts (with an exception for Shift+Delete)

[Albert Astals Cid]

> On Oct. 31, 2016, 6:50 p.m., Elvis Angelaccio wrote:
> > +1, fixes the Gwenview case. I get a warning with Dolphin, but it looks 
> > unrelated: http://i.imgur.com/ZF454gn.png

No, that's actually a problem with my code being too strict, dolphin alternates 
the enabledness of the actions that share the shortcut so that is fine, i've 
made it so that my code only checks for enable actions sharing shortcuts (of 
course it does not re-check each time the enabled status changes so the app at 
some point may break it, but i'm hoping that if someone has been smart enough 
to play with enable/disable actions they checked the shortcuts don't collide)


- Albert


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129299/#review100448
---


On Oct. 31, 2016, 5:24 p.m., Albert Astals Cid wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129299/
> ---
> 
> (Updated Oct. 31, 2016, 5:24 p.m.)
> 
> 
> Review request for KDE Frameworks, David Faure and Elvis Angelaccio.
> 
> 
> Repository: kxmlgui
> 
> 
> Description
> ---
> 
> Add a warning at the createGui stage about ambiguous shortcuts being found in 
> the same action collection.
> 
> This is usually a developer issue, but the error message about ambiguity will 
> only show up when someone tries to use the shortcut, so it is relatively easy 
> to miss if you do not try all your actions via a shortcut.
> 
> Also if the involved shortcut is one of the non primary shortcuts of 
> edit_cut, just give it away, since it's usually Shift+Delete being fought 
> over.
> 
> 
> Diffs
> -
> 
>   src/kxmlguiwindow.cpp 519fb26 
> 
> Diff: https://git.reviewboard.kde.org/r/129299/diff/
> 
> 
> Testing
> ---
> 
> gwenview now defaults to Shift+Delete being "Hard delete" and not "Cut", if 
> you remove the 
>   if (action == editCutAction || existingShortcutAction == editCutAction) {
> part, you get warning about the actions involved
> 
> 
> Thanks,
> 
> Albert Astals Cid
> 
>

Re: Review Request 129299: Warn on startup about ambiguous shortcuts (with an exception for Shift+Delete)

[Elvis Angelaccio]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129299/#review100448
---



+1, fixes the Gwenview case. I get a warning with Dolphin, but it looks 
unrelated: http://i.imgur.com/ZF454gn.png


src/kxmlguiwindow.cpp (line 327)


I think it would be better if this were not blocking (i.e. creating the 
QMessageBox manually and showing it with `open()`).

Otherwise the users get a messagebox instead of the main window of the app 
they started.


- Elvis Angelaccio


On Oct. 31, 2016, 5:24 p.m., Albert Astals Cid wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129299/
> ---
> 
> (Updated Oct. 31, 2016, 5:24 p.m.)
> 
> 
> Review request for KDE Frameworks, David Faure and Elvis Angelaccio.
> 
> 
> Repository: kxmlgui
> 
> 
> Description
> ---
> 
> Add a warning at the createGui stage about ambiguous shortcuts being found in 
> the same action collection.
> 
> This is usually a developer issue, but the error message about ambiguity will 
> only show up when someone tries to use the shortcut, so it is relatively easy 
> to miss if you do not try all your actions via a shortcut.
> 
> Also if the involved shortcut is one of the non primary shortcuts of 
> edit_cut, just give it away, since it's usually Shift+Delete being fought 
> over.
> 
> 
> Diffs
> -
> 
>   src/kxmlguiwindow.cpp 519fb26 
> 
> Diff: https://git.reviewboard.kde.org/r/129299/diff/
> 
> 
> Testing
> ---
> 
> gwenview now defaults to Shift+Delete being "Hard delete" and not "Cut", if 
> you remove the 
>   if (action == editCutAction || existingShortcutAction == editCutAction) {
> part, you get warning about the actions involved
> 
> 
> Thanks,
> 
> Albert Astals Cid
> 
>

Review Request 129299: Warn on startup about ambiguous shortcuts (with an exception for Shift+Delete)

[Albert Astals Cid]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129299/
---

Review request for KDE Frameworks, David Faure and Elvis Angelaccio.


Repository: kxmlgui


Description
---

Add a warning at the createGui stage about ambiguous shortcuts being found in 
the same action collection.

This is usually a developer issue, but the error message about ambiguity will 
only show up when someone tries to use the shortcut, so it is relatively easy 
to miss if you do not try all your actions via a shortcut.

Also if the involved shortcut is one of the non primary shortcuts of edit_cut, 
just give it away, since it's usually Shift+Delete being fought over.


Diffs
-

  src/kxmlguiwindow.cpp 519fb26 

Diff: https://git.reviewboard.kde.org/r/129299/diff/


Testing
---

gwenview now defaults to Shift+Delete being "Hard delete" and not "Cut", if you 
remove the 
  if (action == editCutAction || existingShortcutAction == editCutAction) {
part, you get warning about the actions involved


Thanks,

Albert Astals Cid

Re: Review Request 129251: Remove Shift+Del as secondary shortcut for Cut

[Albert Astals Cid]

> On Oct. 30, 2016, 10:27 a.m., David Faure wrote:
> > Right. We have Shift+Del as an alternate shortcut for cut because when I 
> > switched from Windows to Linux (last millenium!), I had taken the habit to 
> > use Ctrl+Insert, Shift+Del, and Shift+Insert for copy/cut/paste (these are 
> > certainly more logical than Ctrl+C/X/V...).
> > 
> > Indeed this shortcut doesn't work for file managers, who have to get rid of 
> > it. But the majority of apps are not file managers, so the current 
> > situation seems fine to me (file managers remove the shortcut, everyone 
> > else gets it). => I vote for discard.
> 
> Mark Gaiser wrote:
> What would be your shortcut of choice for permanently deleting files?
> Meta + Delete?
> or just nothing and force the user to clear the trash to permanently 
> delete files?
> 
> David Faure wrote:
> IMHO permanent deletion is Shift+Delete, in filemanagers -- the two 
> alternatives you offer seem extremely bad to me ;)
> 
> Permanent deletion of emails is Shift+Delete too (and there cut isn't 
> supported), so that's consistent.
> 
> Even while I was using Shift+Delete as my preferred shortcut for cut (bad 
> laptop keyboards forced me to switch away from that), it never bothered me 
> that it would delete and not cut in filemanagers. Maybe because I never used 
> cut very much in filemanagers, I admit.
> I suppose this forces one to learn Ctrl+X for cutting in dolphin.
> 
> All this being said, I can very well imagine that not many people use 
> Ctrl+Insert, Shift+Del, and Shift+Insert... I don't have a strong case if I'm 
> not using that myself anymore. It just seems harmless to support this in apps 
> where it doesn't conflict with anything else, and to give priority to other 
> shortcuts in apps where they do conflict (i.e. permanent deletion with an 
> easy shortcut has priority over the 
> less-commonly-used-alternate-shortcut-for-cut).

My suggestion at https://git.reviewboard.kde.org/r/129299/


- Albert


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129251/#review100392
---


On Oct. 24, 2016, 10:47 a.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129251/
> ---
> 
> (Updated Oct. 24, 2016, 10:47 a.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and Matthew Dawson.
> 
> 
> Bugs: 357747
> https://bugs.kde.org/show_bug.cgi?id=357747
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> This patch removes Shift+Del as secondary shortcut for the Cut action. This 
> shortcut was set back in 2001.
> 
> Reasons for removing it:
> 
> * The expected standard behavior for this shortcut is "Permanently delete"
> * For the reason above, it is also set as primary shortcut for the 
> `DeleteFile` action. This causes conflicts in applications.
> * For the reason above, many applications (e.g. Dolphin or Digikam) already 
> resolve this conflict on their own.
> 
> Credits to Jan for the investigation: 
> https://bugs.kde.org/show_bug.cgi?id=347373#c2
> 
> 
> Diffs
> -
> 
>   src/gui/kstandardshortcut.cpp 92eb091382c7ab2110240cef21f29268be787250 
> 
> Diff: https://git.reviewboard.kde.org/r/129251/diff/
> 
> 
> Testing
> ---
> 
> Using Shift+Del in Gwenview now works as expected.
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

Re: Review Request 129298: RFC: supporting dependencies on KPackage

[Marco Martin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129298/#review100442
---



yep, that's pretty much what i had in mind.
it's early stage, but if someone doesn't have significant issues with it, i 
think is a good direction to go


autotests/data/testpackagesdep/metadata.json (line 14)


if kns ends up using ids, maybe the server should be specified as well, as 
the id would be server-dependent?



src/kpackage/private/packagejobthread.cpp (line 436)


would this need a local database of installed packages/dependencies?


- Marco Martin


On Oct. 31, 2016, 5:09 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129298/
> ---
> 
> (Updated Oct. 31, 2016, 5:09 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma, Dan Leinir Turthra Jensen, and 
> Marco Martin.
> 
> 
> Repository: kpackage
> 
> 
> Description
> ---
> 
> Makes it possible to specify components to be installed together with a 
> KPackage. They will be specified by a url, we'll have handlers for any kind, 
> making reasonably extensible and doesn't tie us to a technology.
> 
> In this repository I created two Proof of Concept of such handlers, one for 
> the appstream urls and one for KNS: kde:scratch/apol/kpackage-install-helpers
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 395b16e 
>   autotests/data/testpackagesdep/contents/ui/main.qml PRE-CREATION 
>   autotests/data/testpackagesdep/metadata.json PRE-CREATION 
>   autotests/data/testpackagesdep/testpackagesdep.testappdataxml PRE-CREATION 
>   src/kpackage/config-package.h.cmake 61f2f0c 
>   src/kpackage/private/packagejobthread.cpp 0a0cc01 
>   src/kpackage/private/packagejobthread_p.h 1babf0b 
> 
> Diff: https://git.reviewboard.kde.org/r/129298/diff/
> 
> 
> Testing
> ---
> 
> None. just builds. It's a proof of concept, not even the test I added was 
> tested, it was just to display what it could look like.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

Review Request 129298: RFC: supporting dependencies on KPackage

[Aleix Pol Gonzalez]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129298/
---

Review request for KDE Frameworks and Plasma.


Repository: kpackage


Description
---

Makes it possible to specify components to be installed together with a 
KPackage. They will be specified by a url, we'll have handlers for any kind, 
making reasonably extensible and doesn't tie us to a technology.

In this repository I created two Proof of Concept of such handlers, one for the 
appstream urls and one for KNS: kde:scratch/apol/kpackage-install-helpers


Diffs
-

  autotests/CMakeLists.txt 395b16e 
  autotests/data/testpackagesdep/contents/ui/main.qml PRE-CREATION 
  autotests/data/testpackagesdep/metadata.json PRE-CREATION 
  autotests/data/testpackagesdep/testpackagesdep.testappdataxml PRE-CREATION 
  src/kpackage/config-package.h.cmake 61f2f0c 
  src/kpackage/private/packagejobthread.cpp 0a0cc01 
  src/kpackage/private/packagejobthread_p.h 1babf0b 

Diff: https://git.reviewboard.kde.org/r/129298/diff/


Testing
---

None. just builds. It's a proof of concept, not even the test I added was 
tested, it was just to display what it could look like.


Thanks,

Aleix Pol Gonzalez

Jenkins-kde-ci: frameworkintegration master stable-kf5-qt5 » Linux,gcc - Build # 269 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/frameworkintegration%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/269/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 31 Oct 2016 16:52:39 +
Build duration: 56 sec

CHANGE SET
No changes


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 2 test(s), Skipped: 0 test(s), Total: 2 
test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 2/2 (100%)FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 72/205 
(35%)CONDITIONAL 34/172 (20%)

By packages
  
autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 25/27 (93%)CONDITIONAL 
5/10 (50%)
src.kstyle
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 47/178 (26%)CONDITIONAL 
29/162 (18%)

Jenkins-kde-ci: frameworkintegration master stable-kf5-qt5 » Linux,gcc - Build # 269 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/frameworkintegration%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/269/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 31 Oct 2016 16:52:39 +
Build duration: 56 sec

CHANGE SET
No changes


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 2 test(s), Skipped: 0 test(s), Total: 2 
test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 2/2 (100%)FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 72/205 
(35%)CONDITIONAL 34/172 (20%)

By packages
  
autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 25/27 (93%)CONDITIONAL 
5/10 (50%)
src.kstyle
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 47/178 (26%)CONDITIONAL 
29/162 (18%)

Jenkins-kde-ci: knotifyconfig master stable-kf5-qt5 » Linux,gcc - Build # 250 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/knotifyconfig%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/250/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 31 Oct 2016 16:50:49 +
Build duration: 1 min 1 sec

CHANGE SET
No changes


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 
test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  

By packages
  

Jenkins-kde-ci: knotifyconfig master stable-kf5-qt5 » Linux,gcc - Build # 250 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/knotifyconfig%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/250/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 31 Oct 2016 16:50:49 +
Build duration: 1 min 1 sec

CHANGE SET
No changes


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 
test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  

By packages
  

Jenkins-kde-ci: frameworkintegration master stable-kf5-qt5 » Linux,gcc - Build # 268 - Failure!

[no-reply]

GENERAL INFO

BUILD FAILURE
Build URL: 
https://build.kde.org/job/frameworkintegration%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/268/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 31 Oct 2016 16:49:34 +
Build duration: 11 sec

CHANGE SET
No changes

Jenkins-kde-ci: knotifyconfig master stable-kf5-qt5 » Linux,gcc - Build # 249 - Failure!

[no-reply]

GENERAL INFO

BUILD FAILURE
Build URL: 
https://build.kde.org/job/knotifyconfig%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/249/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 31 Oct 2016 16:49:35 +
Build duration: 1 min 8 sec

CHANGE SET
No changes

Jenkins-kde-ci: kio master stable-kf5-qt5 » Linux,gcc - Build # 254 - Still Unstable!

[no-reply]

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/kio%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/254/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 31 Oct 2016 16:35:36 +
Build duration: 12 min

CHANGE SET
Revision b4fe6cdd7f76d280e77d7ee0da52ed975f65950d by kde: ([KOpenWithDialog] 
Hide TreeView header)
  change: edit src/widgets/kopenwithdialog.cpp


JUNIT RESULTS

Name: (root) Failed: 1 test(s), Passed: 51 test(s), Skipped: 0 test(s), Total: 
52 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesmodeltest

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 21/21 (100%)FILES 270/340 (79%)CLASSES 270/340 (79%)LINE 28911/51413 
(56%)CONDITIONAL 15601/38121 (41%)

By packages
  
autotests
FILES 66/66 (100%)CLASSES 66/66 (100%)LINE 7542/8128 
(93%)CONDITIONAL 4023/8518 (47%)
autotests.http
FILES 9/9 (100%)CLASSES 9/9 (100%)LINE 543/544 
(100%)CONDITIONAL 200/336 (60%)
autotests.kcookiejar
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 179/198 (90%)CONDITIONAL 
60/90 (67%)
src.core
FILES 96/116 (83%)CLASSES 96/116 (83%)LINE 7778/14137 
(55%)CONDITIONAL 4245/9067 (47%)
src.core.kssl
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 35/93 (38%)CONDITIONAL 
3/6 (50%)
src.filewidgets
FILES 25/36 (69%)CLASSES 25/36 (69%)LINE 3322/7561 
(44%)CONDITIONAL 1218/4383 (28%)
src.gui
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 104/110 (95%)CONDITIONAL 
46/72 (64%)
src.ioslaves.file
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 443/841 (53%)CONDITIONAL 
329/725 (45%)
src.ioslaves.http
FILES 8/8 (100%)CLASSES 8/8 (100%)LINE 1763/3783 
(47%)CONDITIONAL 1260/3434 (37%)
src.ioslaves.http.kcookiejar
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 620/781 (79%)CONDITIONAL 
602/831 (72%)
src.ioslaves.trash
FILES 7/9 (78%)CLASSES 7/9 (78%)LINE 713/1155 (62%)CONDITIONAL 
375/753 (50%)
src.ioslaves.trash.tests
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 686/764 (90%)CONDITIONAL 
445/936 (48%)
src.kioslave
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 14/27 (52%)CONDITIONAL 
5/10 (50%)
src.kntlm
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 373/385 (97%)CONDITIONAL 
111/138 (80%)
src.kpasswdserver
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 377/594 (63%)CONDITIONAL 
280/580 (48%)
src.kpasswdserver.autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 283/286 (99%)CONDITIONAL 
146/256 (57%)
src.urifilters.fixhost
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 25/34 (74%)CONDITIONAL 
36/54 (67%)
src.urifilters.ikws
FILES 5/10 (50%)CLASSES 5/10 (50%)LINE 240/725 (33%)CONDITIONAL 
146/542 (27%)
src.urifilters.localdomain
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 19/26 (73%)CONDITIONAL 
14/22 (64%)
src.urifilters.shorturi
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 239/268 (89%)CONDITIONAL 
333/414 (80%)
src.widgets
FILES 32/64 (50%)CLASSES 32/64 (50%)LINE 3613/10973 
(33%)CONDITIONAL 1724/6954 (25%)

Jenkins-kde-ci: kross master kf5-qt5 » Linux,gcc - Build # 251 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kross%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/251/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 31 Oct 2016 16:37:50 +
Build duration: 4 min 56 sec

CHANGE SET
No changes


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 2 test(s), Skipped: 0 test(s), Total: 2 
test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 2/2 (100%)FILES 2/13 (15%)CLASSES 2/13 (15%)LINE 132/947 
(14%)CONDITIONAL 66/564 (12%)

By packages
  
autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 56/56 (100%)CONDITIONAL 
33/50 (66%)
src.core
FILES 1/12 (8%)CLASSES 1/12 (8%)LINE 76/891 (9%)CONDITIONAL 
33/514 (6%)

Jenkins-kde-ci: kactivities master kf5-qt5 » Linux,gcc - Build # 268 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kactivities%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/268/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 31 Oct 2016 16:37:50 +
Build duration: 4 min 42 sec

CHANGE SET
No changes


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 
test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  

By packages
  

Jenkins-kde-ci: kactivities master kf5-qt5 » Linux,gcc - Build # 268 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kactivities%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/268/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 31 Oct 2016 16:37:50 +
Build duration: 4 min 42 sec

CHANGE SET
No changes


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 
test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  

By packages
  

Jenkins-kde-ci: kactivities master kf5-qt5 » Linux,gcc - Build # 267 - Failure!

[no-reply]

GENERAL INFO

BUILD FAILURE
Build URL: 
https://build.kde.org/job/kactivities%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/267/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 31 Oct 2016 16:34:06 +
Build duration: 2 min 31 sec

CHANGE SET
No changes

Jenkins-kde-ci: kross master kf5-qt5 » Linux,gcc - Build # 250 - Failure!

[no-reply]

GENERAL INFO

BUILD FAILURE
Build URL: 
https://build.kde.org/job/kross%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/250/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 31 Oct 2016 16:34:06 +
Build duration: 2 min 2 sec

CHANGE SET
No changes

Jenkins-kde-ci: kio master stable-kf5-qt5 » Linux,gcc - Build # 253 - Still Unstable!

[no-reply]

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/kio%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/253/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 31 Oct 2016 16:21:08 +
Build duration: 11 min

CHANGE SET
No changes


JUNIT RESULTS

Name: (root) Failed: 1 test(s), Passed: 51 test(s), Skipped: 0 test(s), Total: 
52 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesmodeltest

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 21/21 (100%)FILES 270/340 (79%)CLASSES 270/340 (79%)LINE 28907/51412 
(56%)CONDITIONAL 15601/38121 (41%)

By packages
  
autotests
FILES 66/66 (100%)CLASSES 66/66 (100%)LINE 7542/8128 
(93%)CONDITIONAL 4023/8518 (47%)
autotests.http
FILES 9/9 (100%)CLASSES 9/9 (100%)LINE 543/544 
(100%)CONDITIONAL 200/336 (60%)
autotests.kcookiejar
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 179/198 (90%)CONDITIONAL 
60/90 (67%)
src.core
FILES 96/116 (83%)CLASSES 96/116 (83%)LINE 7766/14137 
(55%)CONDITIONAL 4243/9067 (47%)
src.core.kssl
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 35/93 (38%)CONDITIONAL 
3/6 (50%)
src.filewidgets
FILES 25/36 (69%)CLASSES 25/36 (69%)LINE 3325/7561 
(44%)CONDITIONAL 1218/4383 (28%)
src.gui
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 104/110 (95%)CONDITIONAL 
46/72 (64%)
src.ioslaves.file
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 443/841 (53%)CONDITIONAL 
329/725 (45%)
src.ioslaves.http
FILES 8/8 (100%)CLASSES 8/8 (100%)LINE 1763/3783 
(47%)CONDITIONAL 1259/3434 (37%)
src.ioslaves.http.kcookiejar
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 620/781 (79%)CONDITIONAL 
602/831 (72%)
src.ioslaves.trash
FILES 7/9 (78%)CLASSES 7/9 (78%)LINE 713/1155 (62%)CONDITIONAL 
375/753 (50%)
src.ioslaves.trash.tests
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 686/764 (90%)CONDITIONAL 
445/936 (48%)
src.kioslave
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 14/27 (52%)CONDITIONAL 
5/10 (50%)
src.kntlm
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 373/385 (97%)CONDITIONAL 
111/138 (80%)
src.kpasswdserver
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 382/594 (64%)CONDITIONAL 
285/580 (49%)
src.kpasswdserver.autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 283/286 (99%)CONDITIONAL 
144/256 (56%)
src.urifilters.fixhost
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 25/34 (74%)CONDITIONAL 
36/54 (67%)
src.urifilters.ikws
FILES 5/10 (50%)CLASSES 5/10 (50%)LINE 240/725 (33%)CONDITIONAL 
146/542 (27%)
src.urifilters.localdomain
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 19/26 (73%)CONDITIONAL 
14/22 (64%)
src.urifilters.shorturi
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 239/268 (89%)CONDITIONAL 
333/414 (80%)
src.widgets
FILES 32/64 (50%)CLASSES 32/64 (50%)LINE 3613/10972 
(33%)CONDITIONAL 1724/6954 (25%)

Re: Review Request 129271: [KOpenWithDialog] Hide TreeView header

[Kai Uwe Broulik]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129271/
---

(Updated Oct. 31, 2016, 4:26 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and KDE Usability.


Changes
---

Submitted with commit b4fe6cdd7f76d280e77d7ee0da52ed975f65950d by Kai Uwe 
Broulik to branch master.


Repository: kio


Description
---

It's not like you could click it to change the sort order anyway.


Diffs
-

  src/widgets/kopenwithdialog.cpp 6c6e500 

Diff: https://git.reviewboard.kde.org/r/129271/diff/


Testing
---

Moar space.


File Attachments


Without "Known Applications" header
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/10/28/bc73e808-eaf2-4491-8544-d15fa6041b72__Screenshot_20161028_122709.png


Thanks,

Kai Uwe Broulik

Re: Review Request 129205: [kcoredirlister] Ability to watch files changes

[Anthony Fieroni]

> On Oct. 31, 2016, 12:38 a.m., David Faure wrote:
> > src/core/kcoredirlister.h, line 429
> > 
> >
> > You can't add virtual methods to a public class, this is not binary 
> > compatible.
> 
> Anthony Fieroni wrote:
> That because it's added at last position in public scope, i wasn't sure 
> to add it here or even after qsignals.
> 
> David Faure wrote:
> No no. Adding a virtual method is binary incompatible, where-ever you put 
> it. (it changes the size of the vtable).
> Please read 
> https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B
> (but anyway for this particular issue I'm recommending a kded module so 
> this is moot anyway)

I'm readed it, i'm in wrong, forgot for vtable size changes, you're right.


- Anthony


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129205/#review100411
---


On Oct. 24, 2016, 10:05 a.m., Anthony Fieroni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129205/
> ---
> 
> (Updated Oct. 24, 2016, 10:05 a.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> David, i will discard review if you don't like it, cause watching files 
> changes can be *really* expensive. I try to:
> 1. to not break abi compability
> 2. to extend filenamesearch with this option
> 3. to fix https://git.reviewboard.kde.org/r/129141/
> 
> 
> Diffs
> -
> 
>   src/core/kcoredirlister.h e6ba2ac 
>   src/core/kcoredirlister.cpp 508516e 
>   src/core/kcoredirlister_p.h 9a3cc7b 
>   tests/kdirlistertest_gui.h 8316b20 
>   tests/kdirlistertest_gui.cpp 11e89cf 
> 
> Diff: https://git.reviewboard.kde.org/r/129205/diff/
> 
> 
> Testing
> ---
> 
> For 3. i still can't figure out why in filenamesearch signal for delete 
> item(s) is not triggered.
> 1. Search for file (by name) in dolphin
> 2. When appear in view delete him
> 3. Signal ItemsDeleted is not triggered, file stays in the view even if new 
> search is performed and reload is needed. The cache look good, tests pass, 
> works but in filenamesearch.
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

Re: Review Request 129205: [kcoredirlister] Ability to watch files changes

[Anthony Fieroni]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129205/
---

(Updated Oct. 31, 2016, 5:53 p.m.)


Status
--

This change has been discarded.


Review request for KDE Frameworks and David Faure.


Repository: kio


Description
---

David, i will discard review if you don't like it, cause watching files changes 
can be *really* expensive. I try to:
1. to not break abi compability
2. to extend filenamesearch with this option
3. to fix https://git.reviewboard.kde.org/r/129141/


Diffs
-

  src/core/kcoredirlister.h e6ba2ac 
  src/core/kcoredirlister.cpp 508516e 
  src/core/kcoredirlister_p.h 9a3cc7b 
  tests/kdirlistertest_gui.h 8316b20 
  tests/kdirlistertest_gui.cpp 11e89cf 

Diff: https://git.reviewboard.kde.org/r/129205/diff/


Testing
---

For 3. i still can't figure out why in filenamesearch signal for delete item(s) 
is not triggered.
1. Search for file (by name) in dolphin
2. When appear in view delete him
3. Signal ItemsDeleted is not triggered, file stays in the view even if new 
search is performed and reload is needed. The cache look good, tests pass, 
works but in filenamesearch.


Thanks,

Anthony Fieroni

Jenkins-kde-ci: plasma-framework master stable-kf5-qt5 » Linux,NoX11,gcc - Build # 234 - Unstable!

[no-reply]

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/plasma-framework%20master%20stable-kf5-qt5/PLATFORM=Linux,Variation=NoX11,compiler=gcc/234/
Project: PLATFORM=Linux,Variation=NoX11,compiler=gcc
Date of build: Mon, 31 Oct 2016 13:46:05 +
Build duration: 2 min 36 sec

CHANGE SET
Revision 07ae9bbc4608700f7fd1fb6c2fba274923b9b794 by Marco Martin: (set context 
proeprties before reloading the qml)
  change: edit src/scriptengines/qml/plasmoid/wallpaperinterface.cpp


JUNIT RESULTS

Name: (root) Failed: 1 test(s), Passed: 14 test(s), Skipped: 0 test(s), Total: 
15 test(s)Failed: TestSuite.plasma-dialogstatetest

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 9/9 (100%)FILES 78/115 (68%)CLASSES 78/115 (68%)LINE 4448/11228 
(40%)CONDITIONAL 2334/8460 (28%)

By packages
  
autotests
FILES 26/26 (100%)CLASSES 26/26 (100%)LINE 910/979 
(93%)CONDITIONAL 580/1162 (50%)
src.declarativeimports.core
FILES 10/18 (56%)CLASSES 10/18 (56%)LINE 561/1880 
(30%)CONDITIONAL 217/1152 (19%)
src.plasma
FILES 14/20 (70%)CLASSES 14/20 (70%)LINE 1658/3608 
(46%)CONDITIONAL 953/2705 (35%)
src.plasma.packagestructure
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 51/65 (78%)CONDITIONAL 
7/22 (32%)
src.plasma.private
FILES 13/22 (59%)CLASSES 13/22 (59%)LINE 868/1567 
(55%)CONDITIONAL 405/1022 (40%)
src.plasma.scripting
FILES 2/3 (67%)CLASSES 2/3 (67%)LINE 36/180 (20%)CONDITIONAL 
14/106 (13%)
src.plasmaquick
FILES 6/12 (50%)CLASSES 6/12 (50%)LINE 322/1700 
(19%)CONDITIONAL 151/1291 (12%)
src.plasmaquick.private
FILES 1/3 (33%)CLASSES 1/3 (33%)LINE 31/113 (27%)CONDITIONAL 
6/22 (27%)
src.scriptengines.qml.plasmoid
FILES 2/7 (29%)CLASSES 2/7 (29%)LINE 11/1136 (1%)CONDITIONAL 
1/978 (0%)

Re: Review Request 129296: Drop unused KConfig dependency

[Aleix Pol Gonzalez]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129296/
---

(Updated Oct. 31, 2016, 1:02 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Plasma.


Changes
---

Submitted with commit 18776a603499ff1c6874e0c8aa6b9e44e2f1bdc7 by Aleix Pol to 
branch master.


Repository: kpackage


Description
---

The only place KConfig was used was an unused function: `QVariantMap 
convert(const QString )`


Diffs
-

  CMakeLists.txt 83bb9a4 
  autotests/CMakeLists.txt ca839ce 
  autotests/packagestructuretest.cpp 565c7f4 
  autotests/querytest.cpp 403364f 
  src/kpackage/CMakeLists.txt f6668c7 
  src/kpackage/package.cpp c139031 
  src/kpackage/packageloader.cpp 264f238 
  src/kpackage/private/packagejobthread.cpp ec41cdd 
  src/kpackage/private/packages.cpp 0024058 
  src/kpackagetool/CMakeLists.txt 4977571 
  src/kpackagetool/kpackagetool.cpp e98bbe0 

Diff: https://git.reviewboard.kde.org/r/129296/diff/


Testing
---

Compiles, tests still pass, everything makes sense.


Thanks,

Aleix Pol Gonzalez

Re: Review Request 129296: Drop unused KConfig dependency

[Marco Martin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129296/#review100435
---


Ship it!




Ship It!

- Marco Martin


On Oct. 31, 2016, 12:52 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129296/
> ---
> 
> (Updated Oct. 31, 2016, 12:52 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kpackage
> 
> 
> Description
> ---
> 
> The only place KConfig was used was an unused function: `QVariantMap 
> convert(const QString )`
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 83bb9a4 
>   autotests/CMakeLists.txt ca839ce 
>   autotests/packagestructuretest.cpp 565c7f4 
>   autotests/querytest.cpp 403364f 
>   src/kpackage/CMakeLists.txt f6668c7 
>   src/kpackage/package.cpp c139031 
>   src/kpackage/packageloader.cpp 264f238 
>   src/kpackage/private/packagejobthread.cpp ec41cdd 
>   src/kpackage/private/packages.cpp 0024058 
>   src/kpackagetool/CMakeLists.txt 4977571 
>   src/kpackagetool/kpackagetool.cpp e98bbe0 
> 
> Diff: https://git.reviewboard.kde.org/r/129296/diff/
> 
> 
> Testing
> ---
> 
> Compiles, tests still pass, everything makes sense.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

Review Request 129296: Drop unused KConfig dependency

[Aleix Pol Gonzalez]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129296/
---

Review request for KDE Frameworks and Plasma.


Repository: kpackage


Description
---

The only place KConfig was used was an unused function: `QVariantMap 
convert(const QString )`


Diffs
-

  CMakeLists.txt 83bb9a4 
  autotests/CMakeLists.txt ca839ce 
  autotests/packagestructuretest.cpp 565c7f4 
  autotests/querytest.cpp 403364f 
  src/kpackage/CMakeLists.txt f6668c7 
  src/kpackage/package.cpp c139031 
  src/kpackage/packageloader.cpp 264f238 
  src/kpackage/private/packagejobthread.cpp ec41cdd 
  src/kpackage/private/packages.cpp 0024058 
  src/kpackagetool/CMakeLists.txt 4977571 
  src/kpackagetool/kpackagetool.cpp e98bbe0 

Diff: https://git.reviewboard.kde.org/r/129296/diff/


Testing
---

Compiles, tests still pass, everything makes sense.


Thanks,

Aleix Pol Gonzalez

Re: Review Request 129205: [kcoredirlister] Ability to watch files changes

[Albert Astals Cid]

> On Oct. 30, 2016, 10:38 p.m., David Faure wrote:
> > KCoreDirLister is complex enough, I'd rather not add features to it.
> > 
> > Let's take a step back.
> > 
> > This looks like an already solved problem to me, if I understand it 
> > correctly:
> >  - a kioslave creates a virtual filesystem on top of the local file system 
> > (there are many doing that: kio_desktop, kio_stash, 3 baloo ones 
> > (baloosearch, timeline, tags) in fact there are at least 21 (those with 
> > Class=:local) but they don't all care about deletion, e.g. because readonly)
> >  - the user deletes/renames a local file (or even adds a file, that should 
> > be added to the virtual filesystem)
> >  - somehow (see below) the app (via KCoreDirLister) needs to be notified 
> > using a URL of that virtual filesystem (not just file:///).
> > right?
> > 
> > The way this has always been done, where implemented (kio_desktop, 
> > kio_stash, and in the past some others) is the following
> >  - because kioslaves are short-lived, they can't do the watching (this 
> > replies to Mark's suggestion number 1)
> >  - a kded module is created, to do the watching using KDirWatch - of only 
> > those files that the kioslave is listing
> >  - the kded module emits KDirNotify DBus signals when a file is 
> > removed/renamed, using the kioslave's URL scheme.
> > 
> > You can see these existing kded modules here:
> > - for desktop: plasma-workspace/kioslave/desktop/desktopnotifier.cpp
> > - for baloo: frameworks/baloo/src/kioslaves/kded/
> > - for stash: playground/utils/kio-stash/src/iodaemon/
> > 
> > It seems to me that simply filenamesearch needs its own similar kded 
> > module. I don't see any in kio-extras/filenamesearch.
> 
> Albert Astals Cid wrote:
> Given we seem to have "a few" of those kded modules, is there something 
> we can do to help people making them and not having the same trouble over and 
> over again?
> 
> David Faure wrote:
> What do you mean? Documentation, yes. Shared code? I'm not sure. We're 
> talking about one class with one constructor and 2 or 3 methods, and what to 
> react to, depends on the kioslave. And how to react (converting from file:// 
> to custom://) depends on the kioslave too.
> 
> (BTW use desktopnotifier or kio-stash as example, baloo is a bit 
> different because it's listening to another daemon (so it didn't really need 
> a kded module in the first place)

Some documentation i guess, this is not the first time this question has come 
up, and i guess people just don't know what to do to update stuff when slaves 
are short lived and stateless.


- Albert


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129205/#review100411
---


On Oct. 24, 2016, 7:05 a.m., Anthony Fieroni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129205/
> ---
> 
> (Updated Oct. 24, 2016, 7:05 a.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> David, i will discard review if you don't like it, cause watching files 
> changes can be *really* expensive. I try to:
> 1. to not break abi compability
> 2. to extend filenamesearch with this option
> 3. to fix https://git.reviewboard.kde.org/r/129141/
> 
> 
> Diffs
> -
> 
>   src/core/kcoredirlister.h e6ba2ac 
>   src/core/kcoredirlister.cpp 508516e 
>   src/core/kcoredirlister_p.h 9a3cc7b 
>   tests/kdirlistertest_gui.h 8316b20 
>   tests/kdirlistertest_gui.cpp 11e89cf 
> 
> Diff: https://git.reviewboard.kde.org/r/129205/diff/
> 
> 
> Testing
> ---
> 
> For 3. i still can't figure out why in filenamesearch signal for delete 
> item(s) is not triggered.
> 1. Search for file (by name) in dolphin
> 2. When appear in view delete him
> 3. Signal ItemsDeleted is not triggered, file stays in the view even if new 
> search is performed and reload is needed. The cache look good, tests pass, 
> works but in filenamesearch.
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

Re: Review Request 129103: Generate metadata.json file at build time

[Aleix Pol Gonzalez]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129103/
---

(Updated Oct. 31, 2016, 11:10 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Plasma.


Changes
---

Submitted with commit 5ce8fe3462c2d40dcf8dea07bb760f918289df7c by Aleix Pol to 
branch master.


Repository: kpackage


Description
---

In plasma-framework internally we use a QJsonObject for reading the metadata. 
Providing the metadata in desktop only makes the system have to parse 
metadata.desktop and then convert it internally into a QJsonObject. 
Furthermore, to do so we use a desktop file parser that is not very performant 
(at least in comparison to KConfig's).

As part of #129102, we made sure that plasma supports `metadata.json`. This 
patch leverages it moving the weight of converting desktop to json at compile 
time.

It could even make sense to convert to binary json as well, I wonder whether 
that would improve at all.


Diffs
-

  KF5PackageMacros.cmake 335fc81 

Diff: https://git.reviewboard.kde.org/r/129103/diff/


Testing
---

Ran RR 129102 without any metdata.desktop files on my system.

Nope, hold the ship its, still needs work.


Thanks,

Aleix Pol Gonzalez

Re: Review Request 129279: Make sure we're looking both for json and desktop metadata

[Marco Martin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129279/#review100429
---


Ship it!




Ship It!

- Marco Martin


On Oct. 28, 2016, 5:30 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129279/
> ---
> 
> (Updated Oct. 28, 2016, 5:30 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kpackage
> 
> 
> Description
> ---
> 
> We shouldn't assume metadata is in metadata.desktop
> 
> 
> Diffs
> -
> 
>   src/kpackage/package.cpp 274185d 
> 
> Diff: https://git.reviewboard.kde.org/r/129279/diff/
> 
> 
> Testing
> ---
> 
> Tests pass, all plasma desktop loads properly, even if I remove 
> `.config/plasma-org.kde.plasma.desktop-appletsrc`
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

Re: Review Request 129278: Don't reparse the metadata file if it's already loaded

[Aleix Pol Gonzalez]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129278/
---

(Updated Oct. 31, 2016, 12:05 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Plasma.


Changes
---

Submitted with commit 6d47160367f94bda8a546ca3b3d2037a82bbb938 by Aleix Pol to 
branch master.


Repository: plasma-framework


Description
---

By doing so, we don't tie ourselves to the desktop format as well as we 
leverage cached data.


Diffs
-

  src/plasma/package.h 6687ed0 
  src/plasma/package.cpp ada6a80 
  src/plasma/packagestructure/packages.cpp a138dd9 
  src/scriptengines/qml/plasmoid/wallpaperinterface.cpp 9ef2d03 

Diff: https://git.reviewboard.kde.org/r/129278/diff/


Testing
---

Tests pass, all plasma desktop loads properly, even if I remove 
`.config/plasma-org.kde.plasma.desktop-appletsrc`


Thanks,

Aleix Pol Gonzalez

Re: Review Request 129278: Don't reparse the metadata file if it's already loaded

[Marco Martin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129278/#review100426
---


Fix it, then Ship it!





src/plasma/package.h (line 350)


needs apidoc and @since tag


- Marco Martin


On Oct. 28, 2016, 5:33 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129278/
> ---
> 
> (Updated Oct. 28, 2016, 5:33 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> By doing so, we don't tie ourselves to the desktop format as well as we 
> leverage cached data.
> 
> 
> Diffs
> -
> 
>   src/plasma/package.h 6687ed0 
>   src/plasma/package.cpp ada6a80 
>   src/plasma/packagestructure/packages.cpp a138dd9 
>   src/scriptengines/qml/plasmoid/wallpaperinterface.cpp 9ef2d03 
> 
> Diff: https://git.reviewboard.kde.org/r/129278/diff/
> 
> 
> Testing
> ---
> 
> Tests pass, all plasma desktop loads properly, even if I remove 
> `.config/plasma-org.kde.plasma.desktop-appletsrc`
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

Re: Review Request 129244: Make UDisks2 backend compile on FreeBSD (and, possibly, other UNIXes).

[Gleb Popov]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129244/
---

(Updated Oct. 31, 2016, 11:49 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Solid.


Changes
---

Submitted with commit 9bca1fd1993665a2233e0155a1baf9973c7a4b6f by Gleb Popov to 
branch master.


Repository: solid


Description
---

This patch makes Solid UDisks2 backend compile on FreeBSD.


Diffs
-

  src/solid/devices/CMakeLists.txt e1f3f6e 
  src/solid/devices/backends/udisks2/udisksblock.h 123e204 
  src/solid/devices/backends/udisks2/udisksblock.cpp 76cfc21 
  src/solid/devices/backends/udisks2/udisksopticaldisc.h f8dd59a 
  src/solid/devices/backends/udisks2/udisksopticaldisc.cpp a9d9c6b 
  src/solid/devices/backends/udisks2/udisksstoragedrive.h be24c7f 
  src/solid/devices/backends/udisks2/udisksstoragedrive.cpp af49063 

Diff: https://git.reviewboard.kde.org/r/129244/diff/


Testing
---


Thanks,

Gleb Popov

Re: Review Request 129244: Make UDisks2 backend compile on FreeBSD (and, possibly, other UNIXes).

[Albert Astals Cid]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129244/#review100423
---


Ship it!




Ship It!

- Albert Astals Cid


On Oct. 31, 2016, 7:03 a.m., Gleb Popov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129244/
> ---
> 
> (Updated Oct. 31, 2016, 7:03 a.m.)
> 
> 
> Review request for KDE Frameworks and Solid.
> 
> 
> Repository: solid
> 
> 
> Description
> ---
> 
> This patch makes Solid UDisks2 backend compile on FreeBSD.
> 
> 
> Diffs
> -
> 
>   src/solid/devices/CMakeLists.txt e1f3f6e 
>   src/solid/devices/backends/udisks2/udisksblock.h 123e204 
>   src/solid/devices/backends/udisks2/udisksblock.cpp 76cfc21 
>   src/solid/devices/backends/udisks2/udisksopticaldisc.h f8dd59a 
>   src/solid/devices/backends/udisks2/udisksopticaldisc.cpp a9d9c6b 
>   src/solid/devices/backends/udisks2/udisksstoragedrive.h be24c7f 
>   src/solid/devices/backends/udisks2/udisksstoragedrive.cpp af49063 
> 
> Diff: https://git.reviewboard.kde.org/r/129244/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gleb Popov
> 
>

Re: Review Request 129205: [kcoredirlister] Ability to watch files changes

[David Faure]

> On Oct. 30, 2016, 10:38 p.m., David Faure wrote:
> > KCoreDirLister is complex enough, I'd rather not add features to it.
> > 
> > Let's take a step back.
> > 
> > This looks like an already solved problem to me, if I understand it 
> > correctly:
> >  - a kioslave creates a virtual filesystem on top of the local file system 
> > (there are many doing that: kio_desktop, kio_stash, 3 baloo ones 
> > (baloosearch, timeline, tags) in fact there are at least 21 (those with 
> > Class=:local) but they don't all care about deletion, e.g. because readonly)
> >  - the user deletes/renames a local file (or even adds a file, that should 
> > be added to the virtual filesystem)
> >  - somehow (see below) the app (via KCoreDirLister) needs to be notified 
> > using a URL of that virtual filesystem (not just file:///).
> > right?
> > 
> > The way this has always been done, where implemented (kio_desktop, 
> > kio_stash, and in the past some others) is the following
> >  - because kioslaves are short-lived, they can't do the watching (this 
> > replies to Mark's suggestion number 1)
> >  - a kded module is created, to do the watching using KDirWatch - of only 
> > those files that the kioslave is listing
> >  - the kded module emits KDirNotify DBus signals when a file is 
> > removed/renamed, using the kioslave's URL scheme.
> > 
> > You can see these existing kded modules here:
> > - for desktop: plasma-workspace/kioslave/desktop/desktopnotifier.cpp
> > - for baloo: frameworks/baloo/src/kioslaves/kded/
> > - for stash: playground/utils/kio-stash/src/iodaemon/
> > 
> > It seems to me that simply filenamesearch needs its own similar kded 
> > module. I don't see any in kio-extras/filenamesearch.
> 
> Albert Astals Cid wrote:
> Given we seem to have "a few" of those kded modules, is there something 
> we can do to help people making them and not having the same trouble over and 
> over again?

What do you mean? Documentation, yes. Shared code? I'm not sure. We're talking 
about one class with one constructor and 2 or 3 methods, and what to react to, 
depends on the kioslave. And how to react (converting from file:// to 
custom://) depends on the kioslave too.

(BTW use desktopnotifier or kio-stash as example, baloo is a bit different 
because it's listening to another daemon (so it didn't really need a kded 
module in the first place)


> On Oct. 30, 2016, 10:38 p.m., David Faure wrote:
> > src/core/kcoredirlister.h, line 429
> > 
> >
> > You can't add virtual methods to a public class, this is not binary 
> > compatible.
> 
> Anthony Fieroni wrote:
> That because it's added at last position in public scope, i wasn't sure 
> to add it here or even after qsignals.

No no. Adding a virtual method is binary incompatible, where-ever you put it. 
(it changes the size of the vtable).
Please read 
https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B
(but anyway for this particular issue I'm recommending a kded module so this is 
moot anyway)


- David


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129205/#review100411
---


On Oct. 24, 2016, 7:05 a.m., Anthony Fieroni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129205/
> ---
> 
> (Updated Oct. 24, 2016, 7:05 a.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> David, i will discard review if you don't like it, cause watching files 
> changes can be *really* expensive. I try to:
> 1. to not break abi compability
> 2. to extend filenamesearch with this option
> 3. to fix https://git.reviewboard.kde.org/r/129141/
> 
> 
> Diffs
> -
> 
>   src/core/kcoredirlister.h e6ba2ac 
>   src/core/kcoredirlister.cpp 508516e 
>   src/core/kcoredirlister_p.h 9a3cc7b 
>   tests/kdirlistertest_gui.h 8316b20 
>   tests/kdirlistertest_gui.cpp 11e89cf 
> 
> Diff: https://git.reviewboard.kde.org/r/129205/diff/
> 
> 
> Testing
> ---
> 
> For 3. i still can't figure out why in filenamesearch signal for delete 
> item(s) is not triggered.
> 1. Search for file (by name) in dolphin
> 2. When appear in view delete him
> 3. Signal ItemsDeleted is not triggered, file stays in the view even if new 
> search is performed and reload is needed. The cache look good, tests pass, 
> works but in filenamesearch.
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

Re: Review Request 129197: Fix tests on FreeBSD

[Gleb Popov]

> On Oct. 16, 2016, 4:12 p.m., Tobias Berner wrote:
> > I'm kind of unsure if this is right. Yes, the tests run now, but isn't the 
> > issue rather in the way kpty works (or fails to work on FreeBSD)?
> 
> Gleb Popov wrote:
> From what i've understood, this boils down to 
> `KPtyDevicePrivate::_k_canRead()` method in kptydevice.cpp. The line 284
> 
> if (!::ioctl(q->masterFd(), PTY_BYTES_AVAILABLE, (char *) ))
> 
> returns 0 in `available` and this makes method return `false`. This, in 
> turn, make `waitFor*` methods return false too.
> 
> Now you mention it, i'm also unsure if this `ioctl` behaves different on 
> Linux.
> 
> Oswald Buddenhagen wrote:
> i've been trying to make sense of the freebsd pts layer, and utterly 
> failed. the documentation is anything between abysmal and non-existing. i 
> suggest you find and invite an actual expert.
> 
> kptyprocess keeps both ends of the pty open, so whether the child process 
> is running or not should be irrelevant. apparently, it's not. maybe this is 
> somehow related to whether the tty is the process' controlling terminal.
> 
> you can try the following:
> - remove the freebsd case from the PTY_BYTES_AVAILABLE definition near 
> the top. this was introduced 2008, before freebsd 8's release, which got an 
> entirely new pts layer. it may just work with the generic code now.
> - try replacing the masterFd() with slaveFd() in the above ioctl. that 
> would be kinda broken, but at least it would be a data point.

This turned out to be a bug in Qt itself. `QProcess` unit tests are also 
failing with the same `.waitForFinished()` error. So, we'll first deal with it 
and then i'll get back on this.


- Gleb


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129197/#review100039
---


On Oct. 16, 2016, 2:44 p.m., Gleb Popov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129197/
> ---
> 
> (Updated Oct. 16, 2016, 2:44 p.m.)
> 
> 
> Review request for KDE Frameworks, Adriaan de Groot, Tobias Berner, Oswald 
> Buddenhagen, and Martin Tobias Holmedahl Sandsmark.
> 
> 
> Repository: kpty
> 
> 
> Description
> ---
> 
> Apparently, KPtyDevice can't be operated on after KPtyProcess finishes. Tweak 
> tests accordingly, so they actually test things while the process is still 
> running.
> 
> 
> Diffs
> -
> 
>   autotests/kptyprocesstest.cpp 8b0b5b0 
> 
> Diff: https://git.reviewboard.kde.org/r/129197/diff/
> 
> 
> Testing
> ---
> 
> make test on FreeBSD
> 
> 
> Thanks,
> 
> Gleb Popov
> 
>

Re: Review Request 129244: Make UDisks2 backend compile on FreeBSD (and, possibly, other UNIXes).

[Gleb Popov]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129244/
---

(Updated Oct. 31, 2016, 10:03 a.m.)


Review request for KDE Frameworks and Solid.


Repository: solid


Description
---

This patch makes Solid UDisks2 backend compile on FreeBSD.


Diffs (updated)
-

  src/solid/devices/CMakeLists.txt e1f3f6e 
  src/solid/devices/backends/udisks2/udisksblock.h 123e204 
  src/solid/devices/backends/udisks2/udisksblock.cpp 76cfc21 
  src/solid/devices/backends/udisks2/udisksopticaldisc.h f8dd59a 
  src/solid/devices/backends/udisks2/udisksopticaldisc.cpp a9d9c6b 
  src/solid/devices/backends/udisks2/udisksstoragedrive.h be24c7f 
  src/solid/devices/backends/udisks2/udisksstoragedrive.cpp af49063 

Diff: https://git.reviewboard.kde.org/r/129244/diff/


Testing
---


Thanks,

Gleb Popov