Re: Review Request 125515: Preserve relative link targets when copying symlinks.

2015-10-24 Thread David Faure


> On Oct. 22, 2015, 4:32 p.m., Frank Reininghaus wrote:
> > 4096 bytes looks reasonable. I think I would still find a fail-safe 
> > solution with a dynamically increasing buffer prettier, but it's so 
> > extremely unlikely that this will ever cause problems that it's not worth 
> > arguing about it :-)
> 
> Stefan Brüns wrote:
> The needed length is available from lstat(.., ), stat.st_size.

Oh, that's perfect, we already just do an lstat() just before we need the 
size... Thanks I'll do that and push.
(I'll leave 4096 in the unittest ;)


- David


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


On Oct. 10, 2015, 3:29 p.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125515/
> ---
> 
> (Updated Oct. 10, 2015, 3:29 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 352927
> https://bugs.kde.org/show_bug.cgi?id=352927
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> BUG: 352927
> REVIEW: 125515
> Change-Id: I7d3c988da32cae9d14750c8adb9ca5af6d140572
> 
> 
> Diffs
> -
> 
>   autotests/jobtest.h ef8c3e111ec647cc59c5a9715ab3220ce1651c9e 
>   autotests/jobtest.cpp c24bcead70f78f2bec3b938819fb2fa842e937d5 
>   src/ioslaves/file/file.cpp a0a533c957628b00eff175a949685d4497c5f095 
> 
> Diff: https://git.reviewboard.kde.org/r/125515/diff/
> 
> 
> Testing
> ---
> 
> 2 unit tests added
> 
> 
> Thanks,
> 
> David Faure
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 125772: Improve search for drkonqui and keep it silent per default if not found

2015-10-24 Thread Christoph Cullmann

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

(Updated Oct. 24, 2015, 2:27 p.m.)


Review request for KDE Frameworks and David Faure.


Changes
---

Uploaded diff with Info instead of Debug :/


Repository: kcrash


Description
---

Search not only in env var + install prefix but in application binary directory 
and libexec path as configured for qt (or qt.conf).
That allows it to bundle drkonqui if wanted.
Beside, make the output of "not found" an per default hidden debug message, as 
otherwise, any application using kcrash without workspace stuff installed warns 
on start.
Sideeffect: decodeName used to decode the CMAKE var, like in other places in 
frameworks.


Diffs (updated)
-

  src/kcrash.cpp a5dbeef 

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


Testing
---

make && make test
tried out kwrite with it, got useful search paths


Thanks,

Christoph Cullmann

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 125760: Allow local embedded themes, like Qt does a default search in :/icons

2015-10-24 Thread Christoph Cullmann


> On Oct. 23, 2015, 9:46 p.m., Aleix Pol Gonzalez wrote:
> > +1 looks good to me.

Ship it from someone?


- Christoph


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


On Oct. 23, 2015, 5:28 p.m., Christoph Cullmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125760/
> ---
> 
> (Updated Oct. 23, 2015, 5:28 p.m.)
> 
> 
> Review request for KDE Frameworks, Christoph Feck and Boudewijn Rempt.
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> ---
> 
> Allow icon themes to be embedded in applications and prefer the embedded 
> ones, if e.g. Krita or Kate ships an embedded theme, that shall be found and 
> used.
> Qt does already do so, by searching in :/icons.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 707f39b 
>   autotests/kiconloader_resourcethemetest.cpp PRE-CREATION 
>   autotests/resources.qrc 5385977 
>   src/kicontheme.cpp d0ab4b9 
> 
> Diff: https://git.reviewboard.kde.org/r/125760/diff/
> 
> 
> Testing
> ---
> 
> Themes still work, if you have the right theme in :/icons it is found, in 
> contrast to before this patch.
> Still a problem, that I need to takle in a different patch: How to enforce 
> the use of some theme, Qt is able to do so via setThemeName, KIconLoader and 
> Co. ignore that.
> 
> 
> Thanks,
> 
> Christoph Cullmann
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 125760: Allow local embedded themes, like Qt does a default search in :/icons

2015-10-24 Thread Christoph Cullmann

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

(Updated Oct. 24, 2015, 1:10 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, Christoph Feck and Boudewijn Rempt.


Changes
---

Submitted with commit b10bae68c7319cbb4e4e985e139702a15df7dcb8 by Christoph 
Cullmann to branch master.


Repository: kiconthemes


Description
---

Allow icon themes to be embedded in applications and prefer the embedded ones, 
if e.g. Krita or Kate ships an embedded theme, that shall be found and used.
Qt does already do so, by searching in :/icons.


Diffs
-

  autotests/CMakeLists.txt 707f39b 
  autotests/kiconloader_resourcethemetest.cpp PRE-CREATION 
  autotests/resources.qrc 5385977 
  src/kicontheme.cpp d0ab4b9 

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


Testing
---

Themes still work, if you have the right theme in :/icons it is found, in 
contrast to before this patch.
Still a problem, that I need to takle in a different patch: How to enforce the 
use of some theme, Qt is able to do so via setThemeName, KIconLoader and Co. 
ignore that.


Thanks,

Christoph Cullmann

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 125760: Allow local embedded themes, like Qt does a default search in :/icons

2015-10-24 Thread Boudewijn Rempt

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

Ship it!


Ship It!

- Boudewijn Rempt


On Oct. 23, 2015, 5:28 p.m., Christoph Cullmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125760/
> ---
> 
> (Updated Oct. 23, 2015, 5:28 p.m.)
> 
> 
> Review request for KDE Frameworks, Christoph Feck and Boudewijn Rempt.
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> ---
> 
> Allow icon themes to be embedded in applications and prefer the embedded 
> ones, if e.g. Krita or Kate ships an embedded theme, that shall be found and 
> used.
> Qt does already do so, by searching in :/icons.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 707f39b 
>   autotests/kiconloader_resourcethemetest.cpp PRE-CREATION 
>   autotests/resources.qrc 5385977 
>   src/kicontheme.cpp d0ab4b9 
> 
> Diff: https://git.reviewboard.kde.org/r/125760/diff/
> 
> 
> Testing
> ---
> 
> Themes still work, if you have the right theme in :/icons it is found, in 
> contrast to before this patch.
> Still a problem, that I need to takle in a different patch: How to enforce 
> the use of some theme, Qt is able to do so via setThemeName, KIconLoader and 
> Co. ignore that.
> 
> 
> Thanks,
> 
> Christoph Cullmann
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 125772: Improve search for drkonqui and keep it silent per default if not found

2015-10-24 Thread Christoph Cullmann

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

Review request for KDE Frameworks and David Faure.


Repository: kcrash


Description
---

Search not only in env var + install prefix but in application binary directory 
and libexec path as configured for qt (or qt.conf).
That allows it to bundle drkonqui if wanted.
Beside, make the output of "not found" an per default hidden debug message, as 
otherwise, any application using kcrash without workspace stuff installed warns 
on start.
Sideeffect: decodeName used to decode the CMAKE var, like in other places in 
frameworks.


Diffs
-

  src/kcrash.cpp a5dbeef 

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


Testing
---

make && make test
tried out kwrite with it, got useful search paths


Thanks,

Christoph Cullmann

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: icons packages with frameworks

2015-10-24 Thread Christoph Cullmann
Hi,

works now with master branch, thanks for the reviews.

http://kate-editor.org/2015/10/24/katekwrite-on-mac-more-icons-more-plugins/

Toolbars with icons without any hacking, how cool is that ;)

Greetings
Christoph

- Am 23. Okt 2015 um 8:34 schrieb cullmann cullm...@absint.com:

> Hi,
> 
> patch for the 2) issue is there:
> 
> https://git.reviewboard.kde.org/r/125760/
> 
> Greetings
> Christoph
> 
> - Am 22. Okt 2015 um 23:15 schrieb cullmann cullm...@absint.com:
> 
>> Hi,
>> 
>> btw., to get all things working even if breeze in bundled in that way one 
>> needs
>> in addition:
>> 
>> 1) breeze to be default theme in kiconthemes or a way to set it manually
>> 2) :/icons being in default search paths in kiconsthemes, too.
>> 
>> For 2) I already have a patch, is some "setDefaultIconTheme" wanted, too, or 
>> do
>> we want
>> to switch to breeze for that in the near future anyway?
>> 
>> Didn't follow the oxygen => breeze and back switch discussion for 
>> KIconThemes in
>> detail.
>> 
>> Greetings
>> Christoph
>> 
>> - Am 21. Okt 2015 um 21:07 schrieb cullmann cullm...@absint.com:
>> 
>>> Hi,
>>> 
> So maybe this wouldn't be such a bad move after all.
 
 Agreed, we have frameworks (e.g. KIconThemes) "depending" on breeze, so it
 makes some kind of sense to ship them together.
>>> yeah, beside that, if you want to create some self-contained installers,
>>> you need breeze (or some other full iconset), too.
>>> 
>>> Therefore it is nice if one can grab the release matching the framework 
>>> release
>>> one uses.
>>> 
>>> Greetings
>>> Christoph
>>> 
>>> --
>>> - Dr.-Ing. Christoph Cullmann -
>>> AbsInt Angewandte Informatik GmbH  Email: cullm...@absint.com
>>> Science Park 1 Tel:   +49-681-38360-22
>>> 66123 Saarbrücken  Fax:   +49-681-38360-20
>>> GERMANYWWW:   http://www.AbsInt.com
>>> 
>>> Geschäftsführung: Dr.-Ing. Christian Ferdinand
>>> Eingetragen im Handelsregister des Amtsgerichts Saarbrücken, HRB 11234
>>> ___
>>> Kde-frameworks-devel mailing list
>>> Kde-frameworks-devel@kde.org
>>> https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
>> 
>> --
>> - Dr.-Ing. Christoph Cullmann -
>> AbsInt Angewandte Informatik GmbH  Email: cullm...@absint.com
>> Science Park 1 Tel:   +49-681-38360-22
>> 66123 Saarbrücken  Fax:   +49-681-38360-20
>> GERMANYWWW:   http://www.AbsInt.com
>> 
>> Geschäftsführung: Dr.-Ing. Christian Ferdinand
>> Eingetragen im Handelsregister des Amtsgerichts Saarbrücken, HRB 11234
>> ___
>> Kde-frameworks-devel mailing list
>> Kde-frameworks-devel@kde.org
>> https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
> 
> --
> - Dr.-Ing. Christoph Cullmann -
> AbsInt Angewandte Informatik GmbH  Email: cullm...@absint.com
> Science Park 1 Tel:   +49-681-38360-22
> 66123 Saarbrücken  Fax:   +49-681-38360-20
> GERMANYWWW:   http://www.AbsInt.com
> 
> Geschäftsführung: Dr.-Ing. Christian Ferdinand
> Eingetragen im Handelsregister des Amtsgerichts Saarbrücken, HRB 11234
> ___
> Kde-frameworks-devel mailing list
> Kde-frameworks-devel@kde.org
> https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

-- 
- Dr.-Ing. Christoph Cullmann -
AbsInt Angewandte Informatik GmbH  Email: cullm...@absint.com
Science Park 1 Tel:   +49-681-38360-22
66123 Saarbrücken  Fax:   +49-681-38360-20
GERMANYWWW:   http://www.AbsInt.com

Geschäftsführung: Dr.-Ing. Christian Ferdinand
Eingetragen im Handelsregister des Amtsgerichts Saarbrücken, HRB 11234
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 125772: Improve search for drkonqui and keep it silent per default if not found

2015-10-24 Thread David Faure

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

Ship it!


Ship It!

- David Faure


On Oct. 24, 2015, 2:27 p.m., Christoph Cullmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125772/
> ---
> 
> (Updated Oct. 24, 2015, 2:27 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kcrash
> 
> 
> Description
> ---
> 
> Search not only in env var + install prefix but in application binary 
> directory and libexec path as configured for qt (or qt.conf).
> That allows it to bundle drkonqui if wanted.
> Beside, make the output of "not found" an per default hidden debug message, 
> as otherwise, any application using kcrash without workspace stuff installed 
> warns on start.
> Sideeffect: decodeName used to decode the CMAKE var, like in other places in 
> frameworks.
> 
> 
> Diffs
> -
> 
>   src/kcrash.cpp a5dbeef 
> 
> Diff: https://git.reviewboard.kde.org/r/125772/diff/
> 
> 
> Testing
> ---
> 
> make && make test
> tried out kwrite with it, got useful search paths
> 
> 
> Thanks,
> 
> Christoph Cullmann
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


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

2015-10-24 Thread no-reply

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/kio%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/141/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Sat, 24 Oct 2015 18:02:50 +
Build duration: 8 min 5 sec

CHANGE SET
Revision f2c96d1550f22600f1b6dee74851c25a3f733ef4 by David Faure: (Preserve 
relative link targets when copying symlinks.)
  change: edit src/ioslaves/file/file.cpp
  change: edit autotests/jobtest.cpp
  change: edit autotests/jobtest.h
Revision 5804d7019e53bf1b551ffd4f013c4ddcbee41428 by David Faure: (Fix 
double-emit of result and missing warning when listing hits an)
  change: edit src/core/listjob.h
  change: edit src/core/listjob.cpp
  change: edit autotests/jobtest.h
  change: edit autotests/jobtest.cpp
Revision 7f47a26d92e62fcf71c20d20700f64fef87f093e by David Faure: (docu: 
recommend uiDelegate() instead of deprecated ui())
  change: edit src/core/job_base.h
  change: edit src/core/statjob.h


JUNIT RESULTS

Name: (root) Failed: 1 test(s), Passed: 46 test(s), Skipped: 0 test(s), Total: 
47 test(s)Failed: TestSuite.kiocore-threadtest

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 20/20 (100%)FILES 252/330 (76%)CLASSES 252/330 (76%)LINE 24649/49843 
(49%)CONDITIONAL 13279/20541 (65%)

By packages
  
autotests
FILES 60/60 (100%)CLASSES 60/60 (100%)LINE 6981/7198 
(97%)CONDITIONAL 3817/7021 (54%)
autotests.http
FILES 9/9 (100%)CLASSES 9/9 (100%)LINE 475/476 
(100%)CONDITIONAL 166/272 (61%)
autotests.kcookiejar
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 182/199 (91%)CONDITIONAL 
60/90 (67%)
src.core
FILES 94/116 (81%)CLASSES 94/116 (81%)LINE 7331/13945 
(53%)CONDITIONAL 3817/5307 (72%)
src.core.kssl
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 35/93 (38%)CONDITIONAL 
3/6 (50%)
src.filewidgets
FILES 20/36 (56%)CLASSES 20/36 (56%)LINE 2266/7573 
(30%)CONDITIONAL 880/1367 (64%)
src.ioslaves.file
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 430/843 (51%)CONDITIONAL 
313/459 (68%)
src.ioslaves.http
FILES 8/8 (100%)CLASSES 8/8 (100%)LINE 750/3793 
(20%)CONDITIONAL 547/678 (81%)
src.ioslaves.http.kcookiejar
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 642/804 (80%)CONDITIONAL 
600/756 (79%)
src.ioslaves.trash
FILES 7/9 (78%)CLASSES 7/9 (78%)LINE 684/1182 (58%)CONDITIONAL 
358/495 (72%)
src.ioslaves.trash.tests
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 690/768 (90%)CONDITIONAL 
431/818 (53%)
src.kioslave
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 14/26 (54%)CONDITIONAL 
5/10 (50%)
src.kntlm
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 302/388 (78%)CONDITIONAL 
86/108 (80%)
src.kpasswdserver
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 389/600 (65%)CONDITIONAL 
284/412 (69%)
src.kpasswdserver.autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 278/283 (98%)CONDITIONAL 
146/254 (57%)
src.urifilters.fixhost
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 32/35 (91%)CONDITIONAL 
42/52 (81%)
src.urifilters.ikws
FILES 5/10 (50%)CLASSES 5/10 (50%)LINE 246/741 (33%)CONDITIONAL 
145/192 (76%)
src.urifilters.localdomain
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 19/26 (73%)CONDITIONAL 
14/18 (78%)
src.urifilters.shorturi
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 225/255 (88%)CONDITIONAL 
292/358 (82%)
src.widgets
FILES 29/62 (47%)CLASSES 29/62 (47%)LINE 2678/10615 
(25%)CONDITIONAL 1273/1868 (68%)___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 125772: Improve search for drkonqui and keep it silent per default if not found

2015-10-24 Thread Christoph Cullmann

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

(Updated Oct. 24, 2015, 6:25 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and David Faure.


Changes
---

Submitted with commit c9b6823213343667b29d2f0dfdbab1a2612e8c8f by Christoph 
Cullmann to branch master.


Repository: kcrash


Description
---

Search not only in env var + install prefix but in application binary directory 
and libexec path as configured for qt (or qt.conf).
That allows it to bundle drkonqui if wanted.
Beside, make the output of "not found" an per default hidden debug message, as 
otherwise, any application using kcrash without workspace stuff installed warns 
on start.
Sideeffect: decodeName used to decode the CMAKE var, like in other places in 
frameworks.


Diffs
-

  src/kcrash.cpp a5dbeef 

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


Testing
---

make && make test
tried out kwrite with it, got useful search paths


Thanks,

Christoph Cullmann

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 125778: Allow kio .protocol files to be app local

2015-10-24 Thread Christoph Cullmann

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

Review request for KDE Frameworks and David Faure.


Repository: kio


Description
---

Allow .protocol files to be bundled with applications.
Search in app local paths first.
Side effect: protocols() result is now sorted by name and allProtocols won't 
create duplicated cache entries if .protocol files are duplicated


Diffs
-

  src/core/kprotocolinfofactory.cpp 29ba8f4 

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


Testing
---

.protocol files are now found if bundled with the application, next step: make 
kio slaves work ;)


Thanks,

Christoph Cullmann

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 125779: properly handle middle click in navigatormenu

2015-10-24 Thread Ilia Kats

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

Review request for KDE Frameworks.


Repository: kio


Description
---

previously, two clicked() events were emitted upon middle click, one with 
Qt::MidButton and one with Qt::LeftButton, resulting in e.g. Dolphin both 
opening a new tab as well as changing the location of the current tab. This 
fixes that


Diffs
-

  src/filewidgets/kurlnavigatorbutton.cpp e6aafb7 
  src/filewidgets/kurlnavigatorbutton_p.h dbb3e7b 
  src/filewidgets/kurlnavigatormenu.cpp d492c7c 
  src/filewidgets/kurlnavigatormenu_p.h 70133d1 

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


Testing
---

Opened Dolphin, tried left/right/middle click on the menu -> behaves as 
expected.


Thanks,

Ilia Kats

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 125779: properly handle middle click in navigatormenu

2015-10-24 Thread Martin Klapetek

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


Thanks for the patch!


src/filewidgets/kurlnavigatorbutton.cpp (lines 450 - 453)


The coding style requests braces around one-lines if-else too.

Also, I think this can do just

emit clicked(btn);

...no need for the if at all (just like you did below)

And I would suggest to use "button" instead of "btn", no reason to use 
abbrs.


- Martin Klapetek


On Oct. 24, 2015, 8:50 p.m., Ilia Kats wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125779/
> ---
> 
> (Updated Oct. 24, 2015, 8:50 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> previously, two clicked() events were emitted upon middle click, one with 
> Qt::MidButton and one with Qt::LeftButton, resulting in e.g. Dolphin both 
> opening a new tab as well as changing the location of the current tab. This 
> fixes that
> 
> 
> Diffs
> -
> 
>   src/filewidgets/kurlnavigatorbutton.cpp e6aafb7 
>   src/filewidgets/kurlnavigatorbutton_p.h dbb3e7b 
>   src/filewidgets/kurlnavigatormenu.cpp d492c7c 
>   src/filewidgets/kurlnavigatormenu_p.h 70133d1 
> 
> Diff: https://git.reviewboard.kde.org/r/125779/diff/
> 
> 
> Testing
> ---
> 
> Opened Dolphin, tried left/right/middle click on the menu -> behaves as 
> expected.
> 
> 
> Thanks,
> 
> Ilia Kats
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 125778: Allow kio .protocol files to be app local and local kioslave deployment

2015-10-24 Thread Christoph Cullmann

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

(Updated Oct. 24, 2015, 7:06 p.m.)


Review request for KDE Frameworks and David Faure.


Changes
---

KIO http seems to work on mac now inside an application bundle ;=)


Summary (updated)
-

Allow kio .protocol files to be app local and local kioslave deployment


Repository: kio


Description (updated)
---

Allow .protocol files to be bundled with applications.
Search in app local paths first.
Side effect: protocols() result is now sorted by name and allProtocols won't 
create duplicated cache entries if .protocol files are duplicated

kioslave helper is search locally, too.

forkSlaves check was missing in holdSlave to avoid segfault without dbus.


Diffs (updated)
-

  src/core/kprotocolinfofactory.cpp 29ba8f4 
  src/core/slave.cpp 9ce0d78 

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


Testing (updated)
---

.protocol files are now found if bundled with the application
kwrite http://www.kde.org works on mac now.


Thanks,

Christoph Cullmann

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 125762: External extractor plugin support for KFileMetaData

2015-10-24 Thread Boudhayan Gupta


> On Oct. 24, 2015, 6 p.m., Vishesh Handa wrote:
> > I still have many thoughts regarding this particular approach, and third 
> > party plugins in general. I'm trying to write them into a cohesive blob.

Do you mean the entire current extractor interface, or just this patch?

What do you mean, "write them into a cohesive blob"? Do you mean all the 
plugins would be compiled into one giant SO/DLL?

I wrote this patch because it was a 4-hour job, but if you want to overhaul the 
entire plugin API I suppose the SOC project would be a better place to do it.


> On Oct. 24, 2015, 6 p.m., Vishesh Handa wrote:
> > src/extractors/externalextractor.cpp, line 69
> > 
> >
> > Why move from the C++ for syntax to Q_FOREACH? There doesn't seem to be 
> > any advantage in this case.

I was told on IRC (IIRC, by Luca Beltrame) that we still can't use range-based 
for loops in frameworks because of compiler requirements. My initial patch used 
range-based for loops.


- Boudhayan


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


On Oct. 24, 2015, 5:49 p.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125762/
> ---
> 
> (Updated Oct. 24, 2015, 5:49 p.m.)
> 
> 
> Review request for Baloo, KDE Frameworks, Pinak Ahuja, and Vishesh Handa.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> This patch introduces support for external metadata extractors in 
> KFileMetaData
> 
> The external extractors themselves can be written in any language, provided 
> that it can be executed as a standalone executable (compiled or script with a 
> hashbang), with command line arguments, and can output data to stdout.
> 
> The extractors are executed like so:
> 
> * `extractor --mimetypes` - outputs a list of mimetypes supported by the 
> extractor, one per line.
> * `extractor filename` - outputs a json document with the metadata. The keys 
> are such that they can be directly used with PropertyInfo::fromName().
> 
> At the KFileMetaData end, an additional internal plugin (ExternalExtractor) 
> is provided that forms a conduit between external extractors and the internal 
> API. This plugin looks for executables called 
> kfilemetadata_extractor_ in /usr/bin to find external extractors, 
> and executes them with the --mimetypes arg to find the list of mimetypes each 
> extractor supports. ExternalExtractor then claims to support all of these 
> mimetypes, and then delegates to the extractor executable when doing the 
> actual extraction.
> 
> 
> Diffs
> -
> 
>   README.md 19b1a26 
>   src/extractors/CMakeLists.txt 5dd223e 
>   src/extractors/externalextractor.h PRE-CREATION 
>   src/extractors/externalextractor.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/125762/diff/
> 
> 
> Testing
> ---
> 
> Tested with the sample executable file extractor (as attched, written in 
> python) with the dump manual test in KFileMetaData. Works.
> 
> 
> File Attachments
> 
> 
> kfilemetadata_extractor_executable
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/10/23/146b657f-31d9-4117-a82f-ef966a6339d4__kfilemetadata_extractor_executable
> 
> 
> Thanks,
> 
> Boudhayan Gupta
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 125762: External extractor plugin support for KFileMetaData

2015-10-24 Thread Boudhayan Gupta

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

(Updated Oct. 24, 2015, 5:49 p.m.)


Review request for Baloo, KDE Frameworks, Pinak Ahuja, and Vishesh Handa.


Changes
---

Update diff based on offline comments from pinakahuja.

* Comment explaining methods in cpp above ctor/dtor are static
* Look at cwd, KFILEMETADATA_EXTRACTOR_PATH and PATH, in that order, for 
extractors.
* Fail on malformed JSON input


Repository: kfilemetadata


Description
---

This patch introduces support for external metadata extractors in KFileMetaData

The external extractors themselves can be written in any language, provided 
that it can be executed as a standalone executable (compiled or script with a 
hashbang), with command line arguments, and can output data to stdout.

The extractors are executed like so:

* `extractor --mimetypes` - outputs a list of mimetypes supported by the 
extractor, one per line.
* `extractor filename` - outputs a json document with the metadata. The keys 
are such that they can be directly used with PropertyInfo::fromName().

At the KFileMetaData end, an additional internal plugin (ExternalExtractor) is 
provided that forms a conduit between external extractors and the internal API. 
This plugin looks for executables called kfilemetadata_extractor_ in 
/usr/bin to find external extractors, and executes them with the --mimetypes 
arg to find the list of mimetypes each extractor supports. ExternalExtractor 
then claims to support all of these mimetypes, and then delegates to the 
extractor executable when doing the actual extraction.


Diffs (updated)
-

  README.md 19b1a26 
  src/extractors/CMakeLists.txt 5dd223e 
  src/extractors/externalextractor.h PRE-CREATION 
  src/extractors/externalextractor.cpp PRE-CREATION 

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


Testing
---

Tested with the sample executable file extractor (as attched, written in 
python) with the dump manual test in KFileMetaData. Works.


File Attachments


kfilemetadata_extractor_executable
  
https://git.reviewboard.kde.org/media/uploaded/files/2015/10/23/146b657f-31d9-4117-a82f-ef966a6339d4__kfilemetadata_extractor_executable


Thanks,

Boudhayan Gupta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Question about goal of Windows/Mac frameworks

2015-10-24 Thread Kevin Ottens
Hello,

On Wednesday 21 October 2015 22:22:54 Christoph Cullmann wrote:
> > On Wednesday 21 October 2015 21:06:02 Christoph Cullmann wrote:
> > My concern there is more the fact that this kind of "short term" or
> > "temporary" solutions tend to become permanent. As long as it is causing
> > pain you can hope someone will look at it and try to fix it... and
> > indeed, see what you achieved! ;-)
> > But if you put in place something short term lifting the immediate pain,
> > people will learn to live with it and it'll stay. I hope we'll avoid that.
> 
> At the moment my biggest concern is, that we stay in the status quo:

That's why I mention pain. If there's no pain involved then yes, slipping in 
the status quo is involved. If there's some pain, at one point someone (like 
you) gets annoyed and start to shake things.
 
> Frameworks as in our git master branch are more or less "not usable" on non
> linux/xdg conform systems (beside pure functional tier1 or so) and all
> people that do use them on other systems hack happy away using patches for
> Qt/frameworks/applications floating around on github/macports/mingw/ or
> just fork and shrink the frameworks locally.

Right, *that* is a problem. A few years back it wouldn't have happened though. 
In some way we lost the culture of every team having people not shying away 
from touching kdelibs (at the time) in case of needs, now most teams consider 
it as a black box. I wish we'd go back to the old ways regarding people 
involvement in frameworks (and you give me hope there) because otherwise 
middle to longer term we're on the right path for a tragedy of the commons 
(kdelibs now KF being clearly one, especially for our community).

> Given we want to broaden our scope away from the "pure Linux/X11(Wayland)
> Desktop" country, that makes me a bit sad. But I see progress in the right
> direction and interest by people to help out.

Yes, and I'm delighted about that!

Cheers.
-- 
Kévin Ottens, http://ervin.ipsquad.net

KDAB - proud supporter of KDE, http://www.kdab.com


signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 125762: External extractor plugin support for KFileMetaData

2015-10-24 Thread Vishesh Handa

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


I still have many thoughts regarding this particular approach, and third party 
plugins in general. I'm trying to write them into a cohesive blob.


src/extractors/externalextractor.cpp (line 69)


Why move from the C++ for syntax to Q_FOREACH? There doesn't seem to be any 
advantage in this case.


- Vishesh Handa


On Oct. 24, 2015, 12:19 p.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125762/
> ---
> 
> (Updated Oct. 24, 2015, 12:19 p.m.)
> 
> 
> Review request for Baloo, KDE Frameworks, Pinak Ahuja, and Vishesh Handa.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> This patch introduces support for external metadata extractors in 
> KFileMetaData
> 
> The external extractors themselves can be written in any language, provided 
> that it can be executed as a standalone executable (compiled or script with a 
> hashbang), with command line arguments, and can output data to stdout.
> 
> The extractors are executed like so:
> 
> * `extractor --mimetypes` - outputs a list of mimetypes supported by the 
> extractor, one per line.
> * `extractor filename` - outputs a json document with the metadata. The keys 
> are such that they can be directly used with PropertyInfo::fromName().
> 
> At the KFileMetaData end, an additional internal plugin (ExternalExtractor) 
> is provided that forms a conduit between external extractors and the internal 
> API. This plugin looks for executables called 
> kfilemetadata_extractor_ in /usr/bin to find external extractors, 
> and executes them with the --mimetypes arg to find the list of mimetypes each 
> extractor supports. ExternalExtractor then claims to support all of these 
> mimetypes, and then delegates to the extractor executable when doing the 
> actual extraction.
> 
> 
> Diffs
> -
> 
>   README.md 19b1a26 
>   src/extractors/CMakeLists.txt 5dd223e 
>   src/extractors/externalextractor.h PRE-CREATION 
>   src/extractors/externalextractor.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/125762/diff/
> 
> 
> Testing
> ---
> 
> Tested with the sample executable file extractor (as attched, written in 
> python) with the dump manual test in KFileMetaData. Works.
> 
> 
> File Attachments
> 
> 
> kfilemetadata_extractor_executable
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/10/23/146b657f-31d9-4117-a82f-ef966a6339d4__kfilemetadata_extractor_executable
> 
> 
> Thanks,
> 
> Boudhayan Gupta
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Question about goal of Windows/Mac frameworks

2015-10-24 Thread Kevin Ottens
Hello,

On Friday 23 October 2015 16:35:36 Luigi Toscano wrote:
> On Wednesday 21 of October 2015 20:09:33 Kevin Ottens wrote:
> > And then, three areas of efforts which we are missing right now in
> > Frameworks: - systematically try to reduce the tier and type of our
> > frameworks (the maturity direction I was talking about);
> 
> Do you mean that a Framework is mature only if it's tier1?

No, I might have been unclear indeed. The point I'm trying to make is that it 
gives a *direction* toward a framework improvement. A framework is mature when 
it reached it's optimum place which is generally closer to the bottom right 
(in the matrix I posted the last time) than from where it started.

That kind of thinking and investigation per framework takes time and as such 
we tend to not do it. For sure we didn't do it systematically, and I think 
ideally that would be needed.

> Is KIO non mature (and it won't probably ever be)?

Difficult to answer without digging deeper into its current state since I 
didn't look into it for a while now, but I would say it's close to be but not 
yet.
I'm saying this mostly because I assume KIO could become Tier 2, could hardly 
be something else than of type Solution though, and it's not there yet. If my 
assumption is wrong then it's already at the right place, end of story.

> I see some functions being so complex that in order to lower the tier for a
> framework one of the two should happen:
> 1) remove functionalities

Yes, there's remove and remove though. :-)

We always assumed everything is used by everyone or required by everyone, 
might not be true and so could be removed (it's the most difficult to find).
It could be some convenience API that allows our applications to spare two 
lines of code... is it really worth it to carry extra dependencies and 
complexity in the frameworks for that? Perhaps not.
It could be some automated behavior which makes sense only if the application 
links to a combination of frameworks but not if the application links to a 
single framework. In such case probably one of the soft dependencies tricks we 
used in the past could apply. It's a Schrodinger features. :-) 

It's all removal to some extent, but it's far from black and white.

> 2) put code in Qt.

Or in a framework of a lower tier. For instance something which is Tier 3 and 
which could be improved to become Tier 2 might require some code be put in a 
Tier 1 framework.

> Even 2) would be quite hard, and I wonder what would happen if all the
> relevant code would end up in Qt

Well, we'd have put quite some work into it then. :-)

It's doable, we did it in the past, there's probably more we could do, but it 
clearly requires getting out of our comfort zone. Can't have it all I guess. 
:-)

> (yeah, almost "kdelibs merged into Qt"! and maybe someone on some
> forum/known website would start screaming about a bloated Qt without
> thinking about the situation, but I'm digressing).

We don't need to get even close to a kdelibs merged into Qt. Quite a lot of 
things wouldn't apply to something like Qt. But clearly we have some internal 
or lower level mechanisms used at plenty of places, those would make sense in 
Qt.

> My take: it is simply not possible (not before maybe Qt 7) to reduce the
> level of some frameworks, without them being "immature".

For the record, this kind of thinking is not really to my taste because it 
opens the door to build up excuses for our own behavior: "oh well, it's not 
possible, why bother anyway..."

For sure, we have way more opportunities to improve ourselves and our 
frameworks first before even starting to perceive limits really out of our 
control.

Regards.
-- 
Kévin Ottens, http://ervin.ipsquad.net

KDAB - proud supporter of KDE, http://www.kdab.com



signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel