Re: Review Request 127809: [Platform xcb] Get best icon size when he's not specified

2016-05-12 Thread Anthony Fieroni


> On Май 9, 2016, 3:37 след обяд, Anthony Fieroni wrote:
> > Ping. It is ok, now?

Martin is there any problems with whis patch?


- Anthony


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


On Май 6, 2016, 12:08 след обяд, Anthony Fieroni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127809/
> ---
> 
> (Updated Май 6, 2016, 12:08 след обяд)
> 
> 
> Review request for KDE Frameworks, KDE Usability and Martin Gräßlin.
> 
> 
> Bugs: 362324
> https://bugs.kde.org/show_bug.cgi?id=362324
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> The api function is KWindowSystem::icon (WId win, int width=-1, int 
> height=-1, bool scale=false) so caller must get best size not worst when 
> width/height is not specified.
> 
> 
> Diffs
> -
> 
>   src/platforms/xcb/kwindowsystem.cpp 5b7c65a 
> 
> Diff: https://git.reviewboard.kde.org/r/127809/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> before
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/05/01/6d718ef6-26cf-4866-94d2-4ffbdfc906fe__Screenshot_20160426_232109.png
> after
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/05/01/7dcab4ae-e451-4d43-8799-a0fcab471a3d__Screenshot_20160501_224642.png
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

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


Re: Review Request 127900: Fix redirection of absolut Urls

2016-05-12 Thread Aleix Pol Gonzalez

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




src/atticabasejob.cpp (line 60)


Get 
`m_reply->attribute(QNetworkRequest::RedirectionTargetAttribute).toUrl()` into 
a variable?

also space in `if_(`



src/atticabasejob.cpp (line 63)


remove commented code.



src/atticabasejob.cpp (line 65)


`} else {`


- Aleix Pol Gonzalez


On May 12, 2016, 7:17 p.m., Andreas Cord-Landwehr wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127900/
> ---
> 
> (Updated May 12, 2016, 7:17 p.m.)
> 
> 
> Review request for KDE Frameworks, Cornelius Schumacher and Kevin Funk.
> 
> 
> Bugs: 354748
> http://bugs.kde.org/show_bug.cgi?id=354748
> 
> 
> Repository: attica
> 
> 
> Description
> ---
> 
> Redirection only worked with relative urls. Yet, e.g. the http to https
> redirection on KDE servers uses absolute paths, which resulted in an
> empty url, since the reply attribute was not relative.
> 
> This patch adds a case distinction for redirecting absolut and relative
> paths.
> 
> BUG: 354748
> 
> 
> Diffs
> -
> 
>   src/atticabasejob.cpp f73cb432212be63cff2ae1118c602cc3bae6d615 
> 
> Diff: https://git.reviewboard.kde.org/r/127900/diff/
> 
> 
> Testing
> ---
> 
> Manual testing with GHNS for documentation in KDevelop.
> 
> 
> Thanks,
> 
> Andreas Cord-Landwehr
> 
>

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


Re: Review Request 127900: Fix redirection of absolut Urls

2016-05-12 Thread Jeremy Whiting

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


Ship it!




Ship It!

- Jeremy Whiting


On May 12, 2016, 11:17 a.m., Andreas Cord-Landwehr wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127900/
> ---
> 
> (Updated May 12, 2016, 11:17 a.m.)
> 
> 
> Review request for KDE Frameworks, Cornelius Schumacher and Kevin Funk.
> 
> 
> Bugs: 354748
> http://bugs.kde.org/show_bug.cgi?id=354748
> 
> 
> Repository: attica
> 
> 
> Description
> ---
> 
> Redirection only worked with relative urls. Yet, e.g. the http to https
> redirection on KDE servers uses absolute paths, which resulted in an
> empty url, since the reply attribute was not relative.
> 
> This patch adds a case distinction for redirecting absolut and relative
> paths.
> 
> BUG: 354748
> 
> 
> Diffs
> -
> 
>   src/atticabasejob.cpp f73cb432212be63cff2ae1118c602cc3bae6d615 
> 
> Diff: https://git.reviewboard.kde.org/r/127900/diff/
> 
> 
> Testing
> ---
> 
> Manual testing with GHNS for documentation in KDevelop.
> 
> 
> Thanks,
> 
> Andreas Cord-Landwehr
> 
>

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


Re: Review Request 127817: Don't make KIconThemes depend on Oxygen

2016-05-12 Thread Albert Astals Cid

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


Ship it!




Ok, if noone else wants to review, ship it (wait for 5.22 to be out) and we'll 
see if it makes people unhappy :D

- Albert Astals Cid


On May 2, 2016, 11:09 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127817/
> ---
> 
> (Updated May 2, 2016, 11:09 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> ---
> 
> We were using oxygen as the default theme, this meant that oxygen was 
> processed by every application. This was fixed in the past by changing this 
> setting to `breeze`, but this only brought new kinds of problems (e.g. review 
> 127232).
> 
> See bug 336739, bug 360664.
> 
> By just using hicolor, we don't make the application fetch `oxygen` for 
> little reason.
> 
> 
> Diffs
> -
> 
>   src/kiconloader.cpp 6ce7ecdc0b9c38647994c750e77980fbf9b6fdd4 
>   src/kicontheme.cpp f8c0a37754848a74e42bba5866aa4ce0998bce7c 
> 
> Diff: https://git.reviewboard.kde.org/r/127817/diff/
> 
> 
> Testing
> ---
> 
> On dolphin:
> This has an improvement on callgrind 1556M/1185M instructions (30% 
> improvement).
> KIconLoader allocations correspond to 3.8% instead of 4.8%, going down by: 
> 535K/509K
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

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


Re: Review Request 127900: Fix redirection of absolut Urls

2016-05-12 Thread Andreas Cord-Landwehr

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

(Updated Mai 12, 2016, 5:17 nachm.)


Review request for KDE Frameworks, Cornelius Schumacher and Kevin Funk.


Bugs: 354748
http://bugs.kde.org/show_bug.cgi?id=354748


Repository: attica


Description
---

Redirection only worked with relative urls. Yet, e.g. the http to https
redirection on KDE servers uses absolute paths, which resulted in an
empty url, since the reply attribute was not relative.

This patch adds a case distinction for redirecting absolut and relative
paths.

BUG: 354748


Diffs
-

  src/atticabasejob.cpp f73cb432212be63cff2ae1118c602cc3bae6d615 

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


Testing (updated)
---

Manual testing with GHNS for documentation in KDevelop.


Thanks,

Andreas Cord-Landwehr

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


Review Request 127900: Fix redirection of absolut Urls

2016-05-12 Thread Andreas Cord-Landwehr

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

Review request for KDE Frameworks, Cornelius Schumacher and Kevin Funk.


Bugs: 354748
http://bugs.kde.org/show_bug.cgi?id=354748


Repository: attica


Description
---

Redirection only worked with relative urls. Yet, e.g. the http to https
redirection on KDE servers uses absolute paths, which resulted in an
empty url, since the reply attribute was not relative.

This patch adds a case distinction for redirecting absolut and relative
paths.

BUG: 354748


Diffs
-

  src/atticabasejob.cpp f73cb432212be63cff2ae1118c602cc3bae6d615 

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


Testing
---


Thanks,

Andreas Cord-Landwehr

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


Re: Review Request 127887: Improve usage of Qt APIs

2016-05-12 Thread Jeremy Whiting

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



The knewstuff (non moretools) side looks good to me.

- Jeremy Whiting


On May 11, 2016, 5:01 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127887/
> ---
> 
> (Updated May 11, 2016, 5:01 p.m.)
> 
> 
> Review request for KDE Frameworks and Jeremy Whiting.
> 
> 
> Repository: knewstuff
> 
> 
> Description
> ---
> 
> Mostly clazy fixes, few cleanups
> 
> 
> Diffs
> -
> 
>   src/core/installation.cpp 1887d0f 
>   src/kmoretools/kmoretools.h 32da22c 
>   src/kmoretools/kmoretools.cpp 8bd6751 
>   src/kmoretools/kmoretoolsconfigdialog_p.cpp 466b887 
>   src/kmoretools/kmoretoolsmenufactory.cpp aad3f00 
>   src/kmoretools/kmoretoolspresets.cpp 679e849 
>   src/staticxml/staticxmlprovider.cpp 541bf57 
>   src/ui/itemsviewdelegate.cpp 7001be2 
>   tests/kmoretools/kmoretoolstest_interactive.cpp b1cbdea 
> 
> Diff: https://git.reviewboard.kde.org/r/127887/diff/
> 
> 
> Testing
> ---
> 
> Builds, tests pass.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

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


Re: Review Request 127878: use the "selected" icon mode in file open dialog sidebar

2016-05-12 Thread David Edmundson

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


Ship it!




Ship It!

- David Edmundson


On May 12, 2016, 9:53 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127878/
> ---
> 
> (Updated May 12, 2016, 9:53 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> since now kiconloader can color the icons in the sidebar, by using the 
> "selected" icon mode, when the breeze icon theme is used the icon of the 
> current sidebar item will be of the same color of selected text (white-ish by 
> default), is more in line with breeze style and the monochrome icons become 
> more readable
> 
> 
> Diffs
> -
> 
>   src/filewidgets/kfileplacesview.cpp 03074e0 
>   src/widgets/kfileitemdelegate.cpp 1516b9e 
> 
> Diff: https://git.reviewboard.kde.org/r/127878/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> menu3.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/05/09/41f52d86-6fc5-4799-bd9e-18d17dc5a65a__menu3.png
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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


Re: Review Request 127899: [KCMShell] Honor KAuthorized restrictions

2016-05-12 Thread Kai Uwe Broulik


> On Mai 12, 2016, 3:59 nachm., David Edmundson wrote:
> > kcmshell/main.cpp, line 95
> > 
> >
> > this line is already doing it.
> > 
> > bool KService::noDisplay() const
> > ...
> > if (!KAuthorized::authorizeControlModule(storageId())) {
> > return true;
> > }
> 
> David Edmundson wrote:
> I'm a bit confused. You said "you could still open them" but you've also 
> written you've not tested it.
> 
> If you have tried blocking it them and it's not working, can you show me 
> your config file?

As I said, no testing :D
I just grepped the code of KCMShell for KAuthorized and only found that one 
occurence in --list, my statement was based on that. 

So, we're all set \o/

What's interesting, though, is that KService uses storageId while KCMShell uses 
menuId()


- Kai Uwe


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


On Mai 12, 2016, 1:59 nachm., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127899/
> ---
> 
> (Updated Mai 12, 2016, 1:59 nachm.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kde-cli-tools
> 
> 
> Description
> ---
> 
> This makes KCMShell skip loading modules that are restricted in KDE Control 
> Module Restrictions.
> 
> The --list property already filtered them but you could still open them.
> 
> 
> Diffs
> -
> 
>   kcmshell/main.cpp 3a4f651 
> 
> Diff: https://git.reviewboard.kde.org/r/127899/diff/
> 
> 
> Testing
> ---
> 
> Literally none.
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

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


Re: Review Request 127899: [KCMShell] Honor KAuthorized restrictions

2016-05-12 Thread Kai Uwe Broulik

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

(Updated Mai 12, 2016, 4:03 nachm.)


Status
--

This change has been discarded.


Review request for KDE Frameworks and Plasma.


Repository: kde-cli-tools


Description
---

This makes KCMShell skip loading modules that are restricted in KDE Control 
Module Restrictions.

The --list property already filtered them but you could still open them.


Diffs
-

  kcmshell/main.cpp 3a4f651 

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


Testing
---

Literally none.


Thanks,

Kai Uwe Broulik

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


Re: Review Request 127899: [KCMShell] Honor KAuthorized restrictions

2016-05-12 Thread David Edmundson


> On May 12, 2016, 3:59 p.m., David Edmundson wrote:
> > kcmshell/main.cpp, line 95
> > 
> >
> > this line is already doing it.
> > 
> > bool KService::noDisplay() const
> > ...
> > if (!KAuthorized::authorizeControlModule(storageId())) {
> > return true;
> > }

I'm a bit confused. You said "you could still open them" but you've also 
written you've not tested it.

If you have tried blocking it them and it's not working, can you show me your 
config file?


- David


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


On May 12, 2016, 1:59 p.m., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127899/
> ---
> 
> (Updated May 12, 2016, 1:59 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kde-cli-tools
> 
> 
> Description
> ---
> 
> This makes KCMShell skip loading modules that are restricted in KDE Control 
> Module Restrictions.
> 
> The --list property already filtered them but you could still open them.
> 
> 
> Diffs
> -
> 
>   kcmshell/main.cpp 3a4f651 
> 
> Diff: https://git.reviewboard.kde.org/r/127899/diff/
> 
> 
> Testing
> ---
> 
> Literally none.
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

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


Re: Review Request 127899: [KCMShell] Honor KAuthorized restrictions

2016-05-12 Thread David Edmundson

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



-2


kcmshell/main.cpp (line 95)


this line is already doing it.

bool KService::noDisplay() const
...
if (!KAuthorized::authorizeControlModule(storageId())) {
return true;
}


- David Edmundson


On May 12, 2016, 1:59 p.m., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127899/
> ---
> 
> (Updated May 12, 2016, 1:59 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kde-cli-tools
> 
> 
> Description
> ---
> 
> This makes KCMShell skip loading modules that are restricted in KDE Control 
> Module Restrictions.
> 
> The --list property already filtered them but you could still open them.
> 
> 
> Diffs
> -
> 
>   kcmshell/main.cpp 3a4f651 
> 
> Diff: https://git.reviewboard.kde.org/r/127899/diff/
> 
> 
> Testing
> ---
> 
> Literally none.
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

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


Re: Review Request 127899: [KCMShell] Honor KAuthorized restrictions

2016-05-12 Thread Aleix Pol Gonzalez

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


Ship it!




Ship It!

- Aleix Pol Gonzalez


On May 12, 2016, 3:59 p.m., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127899/
> ---
> 
> (Updated May 12, 2016, 3:59 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kde-cli-tools
> 
> 
> Description
> ---
> 
> This makes KCMShell skip loading modules that are restricted in KDE Control 
> Module Restrictions.
> 
> The --list property already filtered them but you could still open them.
> 
> 
> Diffs
> -
> 
>   kcmshell/main.cpp 3a4f651 
> 
> Diff: https://git.reviewboard.kde.org/r/127899/diff/
> 
> 
> Testing
> ---
> 
> Literally none.
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

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


Review Request 127899: [KCMShell] Honor KAuthorized restrictions

2016-05-12 Thread Kai Uwe Broulik

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

Review request for KDE Frameworks and Plasma.


Repository: kde-cli-tools


Description
---

This makes KCMShell skip loading modules that are restricted in KDE Control 
Module Restrictions.

The --list property already filtered them but you could still open them.


Diffs
-

  kcmshell/main.cpp 3a4f651 

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


Testing
---

Literally none.


Thanks,

Kai Uwe Broulik

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


Re: Review Request 127896: make dbus optional on osx: kauth

2016-05-12 Thread René J . V . Bertin


> On May 12, 2016, 8:15 a.m., Kåre Särs wrote:
> > I think this patch should not include any platform specific defines. 
> > Disabling DBus requirement on Windows might also be interesting for some 
> > projects. I propose to do something similar to what is done in kxmlgui to 
> > disable kglobalaccel. The default is to require kglobalaccel, but if you 
> > knowingly specify "-DFORCE_DISABLE_KGLOBALACCEL=1" kglobalaccel is not 
> > required or searched for.
> 
> René J.V. Bertin wrote:
> True, even if there's probably very little point in disabling DBus on a 
> standard Unix/Linux set-up.
> 
> But indeed, a platform-agnostic option can still be included in the 
> options that will be flipped by a platform-specific option like 
> `APPLE_STANDALONE_BUNDLE` I've proposed in a patch for Marble.
> 
> To come back to what I suggested earlier: even if there were to be a KF5 
> framework that encapsulates DBus or "something more platform native" there 
> would still be a use-case for using DBus through that framework even on OS X 
> (or MS Windows). Projects like HomeBrew, MacPorts and Cygwin don't exist to 
> replace the native desktop, but to complement it; to provide an easy way to 
> install and maintain FOSS and provide a context in which those applications 
> can run as much as possible as they do on their native platform. That 
> evidently includes DBus, but not just so that Gnome apps can talk with other 
> Gnome apps and KDE apps with other KDE apps. I don't have any stats on this, 
> but I'd expect that a good many of the users of such projects use them not so 
> much for software development per se, but as tools for their actual work 
> (often in science and/or engineering, in my impression). That might very well 
> include using Gnome/GTk apps alongside KDE applications, and in that case 
> they'd proba
 bly expect  the same kind of interoperability as those apps would have on the 
other platforms they might use.
> IOW, I don't think a distribution system that aims to provide the best 
> possible context for the applications it carries can get around using DBus.
> 
> But this is probably not the best place for an in-depth discussion around 
> underlying principles; that should probably be done on a ML (and ideally 
> include a DBus dev or two).
> 
> Nick Shaforostoff wrote:
> i don't see a reason behind introducing a special cmake option other than 
> code coverage (test dbus and dbus-free branches with the same qt): why one 
> should want to disable dbus on a system where he/she has Qt with dbus?
> 
> can you explain this to me?
> 
> regarding homebrew, i repeat: by default it installs a precompiled 
> version of Qt without dbus. if user wants dbus then he/she has to have 
> homebrew recompile whole Qt (takes about 1 hour). and what if kde app doesn't 
> need any IPC? that would just pollute his/her system with redundant stuff. 
> how many gtk-based apps require dbus to run on windows and osx?
> 
> Kåre Särs wrote:
> We have Kate as an example. At the moment the main thing we need DBus for 
> on windows is opening documents in only one window when opening new documents 
> through explorer. Without the DBus daemon it does not work. KDevelop has a 
> solution for it without using DBus which means that we could skip DBus usage 
> for this purpose and we would not need to tell people to install the service 
> (DBus) that occasionally has been flagged as mallware.
> 
> You might argue that we should just compile support without installing 
> the service, but how do I know what features don't work without the service? 
> If the frameworks need special options to disable DBus I'm informed about 
> what is disabled and can choose if I need it or not. Without the option the 
> feature is silently disabled. This is the reason I want separate options for 
> each framework that provides it and not a global one in ECM.
> 
> Almost all Qt users on Windows that I know of would not even dream of 
> compiling their own Qt and pre-compiled Qt comes with DBus support.
> 
> René J.V. Bertin wrote:
> > can you explain this to me?
> 
> An option here would allow using a single Qt install to build and bundle 
> software either with or without support for DBus. 
> 
> I wouldn't worry about the "pollution" aspect of redundant stuff here. 
> There are other things that are just as redundant and pollute a lot more 
> (like "tons of" translation files) and that are still installed because disk 
> space is cheap compared to the potential cost of being overly concerned about 
> the overhead. It's also cheap compared to the cost of building Qt from source.
> 
> I'll repeat something too: if HB considers it appropriate to provide a 
> pre-compiled Qt version that doesn't include DBus support, than it's up to HB 
> to assume any consequences that might have on IPC abilities of dependent 
> packages. For instance by building with a 

Re: Review Request 127896: make dbus optional on osx: kauth

2016-05-12 Thread Nick Shaforostoff


> On May 12, 2016, 6:15 a.m., Kåre Särs wrote:
> > I think this patch should not include any platform specific defines. 
> > Disabling DBus requirement on Windows might also be interesting for some 
> > projects. I propose to do something similar to what is done in kxmlgui to 
> > disable kglobalaccel. The default is to require kglobalaccel, but if you 
> > knowingly specify "-DFORCE_DISABLE_KGLOBALACCEL=1" kglobalaccel is not 
> > required or searched for.
> 
> René J.V. Bertin wrote:
> True, even if there's probably very little point in disabling DBus on a 
> standard Unix/Linux set-up.
> 
> But indeed, a platform-agnostic option can still be included in the 
> options that will be flipped by a platform-specific option like 
> `APPLE_STANDALONE_BUNDLE` I've proposed in a patch for Marble.
> 
> To come back to what I suggested earlier: even if there were to be a KF5 
> framework that encapsulates DBus or "something more platform native" there 
> would still be a use-case for using DBus through that framework even on OS X 
> (or MS Windows). Projects like HomeBrew, MacPorts and Cygwin don't exist to 
> replace the native desktop, but to complement it; to provide an easy way to 
> install and maintain FOSS and provide a context in which those applications 
> can run as much as possible as they do on their native platform. That 
> evidently includes DBus, but not just so that Gnome apps can talk with other 
> Gnome apps and KDE apps with other KDE apps. I don't have any stats on this, 
> but I'd expect that a good many of the users of such projects use them not so 
> much for software development per se, but as tools for their actual work 
> (often in science and/or engineering, in my impression). That might very well 
> include using Gnome/GTk apps alongside KDE applications, and in that case 
> they'd proba
 bly expect  the same kind of interoperability as those apps would have on the 
other platforms they might use.
> IOW, I don't think a distribution system that aims to provide the best 
> possible context for the applications it carries can get around using DBus.
> 
> But this is probably not the best place for an in-depth discussion around 
> underlying principles; that should probably be done on a ML (and ideally 
> include a DBus dev or two).
> 
> Nick Shaforostoff wrote:
> i don't see a reason behind introducing a special cmake option other than 
> code coverage (test dbus and dbus-free branches with the same qt): why one 
> should want to disable dbus on a system where he/she has Qt with dbus?
> 
> can you explain this to me?
> 
> regarding homebrew, i repeat: by default it installs a precompiled 
> version of Qt without dbus. if user wants dbus then he/she has to have 
> homebrew recompile whole Qt (takes about 1 hour). and what if kde app doesn't 
> need any IPC? that would just pollute his/her system with redundant stuff. 
> how many gtk-based apps require dbus to run on windows and osx?
> 
> Kåre Särs wrote:
> We have Kate as an example. At the moment the main thing we need DBus for 
> on windows is opening documents in only one window when opening new documents 
> through explorer. Without the DBus daemon it does not work. KDevelop has a 
> solution for it without using DBus which means that we could skip DBus usage 
> for this purpose and we would not need to tell people to install the service 
> (DBus) that occasionally has been flagged as mallware.
> 
> You might argue that we should just compile support without installing 
> the service, but how do I know what features don't work without the service? 
> If the frameworks need special options to disable DBus I'm informed about 
> what is disabled and can choose if I need it or not. Without the option the 
> feature is silently disabled. This is the reason I want separate options for 
> each framework that provides it and not a global one in ECM.
> 
> Almost all Qt users on Windows that I know of would not even dream of 
> compiling their own Qt and pre-compiled Qt comes with DBus support.
> 
> René J.V. Bertin wrote:
> > can you explain this to me?
> 
> An option here would allow using a single Qt install to build and bundle 
> software either with or without support for DBus. 
> 
> I wouldn't worry about the "pollution" aspect of redundant stuff here. 
> There are other things that are just as redundant and pollute a lot more 
> (like "tons of" translation files) and that are still installed because disk 
> space is cheap compared to the potential cost of being overly concerned about 
> the overhead. It's also cheap compared to the cost of building Qt from source.
> 
> I'll repeat something too: if HB considers it appropriate to provide a 
> pre-compiled Qt version that doesn't include DBus support, than it's up to HB 
> to assume any consequences that might have on IPC abilities of dependent 
> packages. For instance by building with a 

Re: Review Request 127850: Let Plasma::Corona load the layout on all cases.

2016-05-12 Thread Marco Martin

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


Fix it, then Ship it!




logic is fine, but should not add public symbols for this completely internal 
function


src/plasma/corona.h (line 363)


move in dpointer



src/plasma/corona.h (line 364)


no private functions in exported classes, ever

this should be just a lambda, is used exactly once in a single connect


- Marco Martin


On May 11, 2016, 11:20 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127850/
> ---
> 
> (Updated May 11, 2016, 11:20 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma and David Rosca.
> 
> 
> Bugs: 60777
> http://bugs.kde.org/show_bug.cgi?id=60777
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Iteration of: https://git.reviewboard.kde.org/r/127848/
> 
> We either load the existing layout or we load a default one.
> 
> With this, it could be removed from ShellCorona.
> 
> 
> Diffs
> -
> 
>   src/plasma/corona.h d8f829a 
>   src/plasma/corona.cpp 1784516 
> 
> Diff: https://git.reviewboard.kde.org/r/127850/diff/
> 
> 
> Testing
> ---
> 
> Tests pass.
> PlasmaShell prints completed once both with and without configuration.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

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


Re: Review Request 127896: make dbus optional on osx: kauth

2016-05-12 Thread René J . V . Bertin


> On May 12, 2016, 8:15 a.m., Kåre Särs wrote:
> > I think this patch should not include any platform specific defines. 
> > Disabling DBus requirement on Windows might also be interesting for some 
> > projects. I propose to do something similar to what is done in kxmlgui to 
> > disable kglobalaccel. The default is to require kglobalaccel, but if you 
> > knowingly specify "-DFORCE_DISABLE_KGLOBALACCEL=1" kglobalaccel is not 
> > required or searched for.
> 
> René J.V. Bertin wrote:
> True, even if there's probably very little point in disabling DBus on a 
> standard Unix/Linux set-up.
> 
> But indeed, a platform-agnostic option can still be included in the 
> options that will be flipped by a platform-specific option like 
> `APPLE_STANDALONE_BUNDLE` I've proposed in a patch for Marble.
> 
> To come back to what I suggested earlier: even if there were to be a KF5 
> framework that encapsulates DBus or "something more platform native" there 
> would still be a use-case for using DBus through that framework even on OS X 
> (or MS Windows). Projects like HomeBrew, MacPorts and Cygwin don't exist to 
> replace the native desktop, but to complement it; to provide an easy way to 
> install and maintain FOSS and provide a context in which those applications 
> can run as much as possible as they do on their native platform. That 
> evidently includes DBus, but not just so that Gnome apps can talk with other 
> Gnome apps and KDE apps with other KDE apps. I don't have any stats on this, 
> but I'd expect that a good many of the users of such projects use them not so 
> much for software development per se, but as tools for their actual work 
> (often in science and/or engineering, in my impression). That might very well 
> include using Gnome/GTk apps alongside KDE applications, and in that case 
> they'd proba
 bly expect  the same kind of interoperability as those apps would have on the 
other platforms they might use.
> IOW, I don't think a distribution system that aims to provide the best 
> possible context for the applications it carries can get around using DBus.
> 
> But this is probably not the best place for an in-depth discussion around 
> underlying principles; that should probably be done on a ML (and ideally 
> include a DBus dev or two).
> 
> Nick Shaforostoff wrote:
> i don't see a reason behind introducing a special cmake option other than 
> code coverage (test dbus and dbus-free branches with the same qt): why one 
> should want to disable dbus on a system where he/she has Qt with dbus?
> 
> can you explain this to me?
> 
> regarding homebrew, i repeat: by default it installs a precompiled 
> version of Qt without dbus. if user wants dbus then he/she has to have 
> homebrew recompile whole Qt (takes about 1 hour). and what if kde app doesn't 
> need any IPC? that would just pollute his/her system with redundant stuff. 
> how many gtk-based apps require dbus to run on windows and osx?
> 
> Kåre Särs wrote:
> We have Kate as an example. At the moment the main thing we need DBus for 
> on windows is opening documents in only one window when opening new documents 
> through explorer. Without the DBus daemon it does not work. KDevelop has a 
> solution for it without using DBus which means that we could skip DBus usage 
> for this purpose and we would not need to tell people to install the service 
> (DBus) that occasionally has been flagged as mallware.
> 
> You might argue that we should just compile support without installing 
> the service, but how do I know what features don't work without the service? 
> If the frameworks need special options to disable DBus I'm informed about 
> what is disabled and can choose if I need it or not. Without the option the 
> feature is silently disabled. This is the reason I want separate options for 
> each framework that provides it and not a global one in ECM.
> 
> Almost all Qt users on Windows that I know of would not even dream of 
> compiling their own Qt and pre-compiled Qt comes with DBus support.
> 
> René J.V. Bertin wrote:
> > can you explain this to me?
> 
> An option here would allow using a single Qt install to build and bundle 
> software either with or without support for DBus. 
> 
> I wouldn't worry about the "pollution" aspect of redundant stuff here. 
> There are other things that are just as redundant and pollute a lot more 
> (like "tons of" translation files) and that are still installed because disk 
> space is cheap compared to the potential cost of being overly concerned about 
> the overhead. It's also cheap compared to the cost of building Qt from source.
> 
> I'll repeat something too: if HB considers it appropriate to provide a 
> pre-compiled Qt version that doesn't include DBus support, than it's up to HB 
> to assume any consequences that might have on IPC abilities of dependent 
> packages. For instance by building with a 

Re: Review Request 127896: make dbus optional on osx: kauth

2016-05-12 Thread René J . V . Bertin


> On May 12, 2016, 8:15 a.m., Kåre Särs wrote:
> > I think this patch should not include any platform specific defines. 
> > Disabling DBus requirement on Windows might also be interesting for some 
> > projects. I propose to do something similar to what is done in kxmlgui to 
> > disable kglobalaccel. The default is to require kglobalaccel, but if you 
> > knowingly specify "-DFORCE_DISABLE_KGLOBALACCEL=1" kglobalaccel is not 
> > required or searched for.
> 
> René J.V. Bertin wrote:
> True, even if there's probably very little point in disabling DBus on a 
> standard Unix/Linux set-up.
> 
> But indeed, a platform-agnostic option can still be included in the 
> options that will be flipped by a platform-specific option like 
> `APPLE_STANDALONE_BUNDLE` I've proposed in a patch for Marble.
> 
> To come back to what I suggested earlier: even if there were to be a KF5 
> framework that encapsulates DBus or "something more platform native" there 
> would still be a use-case for using DBus through that framework even on OS X 
> (or MS Windows). Projects like HomeBrew, MacPorts and Cygwin don't exist to 
> replace the native desktop, but to complement it; to provide an easy way to 
> install and maintain FOSS and provide a context in which those applications 
> can run as much as possible as they do on their native platform. That 
> evidently includes DBus, but not just so that Gnome apps can talk with other 
> Gnome apps and KDE apps with other KDE apps. I don't have any stats on this, 
> but I'd expect that a good many of the users of such projects use them not so 
> much for software development per se, but as tools for their actual work 
> (often in science and/or engineering, in my impression). That might very well 
> include using Gnome/GTk apps alongside KDE applications, and in that case 
> they'd proba
 bly expect  the same kind of interoperability as those apps would have on the 
other platforms they might use.
> IOW, I don't think a distribution system that aims to provide the best 
> possible context for the applications it carries can get around using DBus.
> 
> But this is probably not the best place for an in-depth discussion around 
> underlying principles; that should probably be done on a ML (and ideally 
> include a DBus dev or two).
> 
> Nick Shaforostoff wrote:
> i don't see a reason behind introducing a special cmake option other than 
> code coverage (test dbus and dbus-free branches with the same qt): why one 
> should want to disable dbus on a system where he/she has Qt with dbus?
> 
> can you explain this to me?
> 
> regarding homebrew, i repeat: by default it installs a precompiled 
> version of Qt without dbus. if user wants dbus then he/she has to have 
> homebrew recompile whole Qt (takes about 1 hour). and what if kde app doesn't 
> need any IPC? that would just pollute his/her system with redundant stuff. 
> how many gtk-based apps require dbus to run on windows and osx?
> 
> Kåre Särs wrote:
> We have Kate as an example. At the moment the main thing we need DBus for 
> on windows is opening documents in only one window when opening new documents 
> through explorer. Without the DBus daemon it does not work. KDevelop has a 
> solution for it without using DBus which means that we could skip DBus usage 
> for this purpose and we would not need to tell people to install the service 
> (DBus) that occasionally has been flagged as mallware.
> 
> You might argue that we should just compile support without installing 
> the service, but how do I know what features don't work without the service? 
> If the frameworks need special options to disable DBus I'm informed about 
> what is disabled and can choose if I need it or not. Without the option the 
> feature is silently disabled. This is the reason I want separate options for 
> each framework that provides it and not a global one in ECM.
> 
> Almost all Qt users on Windows that I know of would not even dream of 
> compiling their own Qt and pre-compiled Qt comes with DBus support.

> can you explain this to me?

An option here would allow using a single Qt install to build and bundle 
software either with or without support for DBus. 

I wouldn't worry about the "pollution" aspect of redundant stuff here. There 
are other things that are just as redundant and pollute a lot more (like "tons 
of" translation files) and that are still installed because disk space is cheap 
compared to the potential cost of being overly concerned about the overhead. 
It's also cheap compared to the cost of building Qt from source.

I'll repeat something too: if HB considers it appropriate to provide a 
pre-compiled Qt version that doesn't include DBus support, than it's up to HB 
to assume any consequences that might have on IPC abilities of dependent 
packages. For instance by building with a specific cmake switch to disable 
DBus. 
MacPorts also provides pre-compiled packages, 

Re: Review Request 127896: make dbus optional on osx: kauth

2016-05-12 Thread Kåre Särs


> On May 12, 2016, 6:15 a.m., Kåre Särs wrote:
> > I think this patch should not include any platform specific defines. 
> > Disabling DBus requirement on Windows might also be interesting for some 
> > projects. I propose to do something similar to what is done in kxmlgui to 
> > disable kglobalaccel. The default is to require kglobalaccel, but if you 
> > knowingly specify "-DFORCE_DISABLE_KGLOBALACCEL=1" kglobalaccel is not 
> > required or searched for.
> 
> René J.V. Bertin wrote:
> True, even if there's probably very little point in disabling DBus on a 
> standard Unix/Linux set-up.
> 
> But indeed, a platform-agnostic option can still be included in the 
> options that will be flipped by a platform-specific option like 
> `APPLE_STANDALONE_BUNDLE` I've proposed in a patch for Marble.
> 
> To come back to what I suggested earlier: even if there were to be a KF5 
> framework that encapsulates DBus or "something more platform native" there 
> would still be a use-case for using DBus through that framework even on OS X 
> (or MS Windows). Projects like HomeBrew, MacPorts and Cygwin don't exist to 
> replace the native desktop, but to complement it; to provide an easy way to 
> install and maintain FOSS and provide a context in which those applications 
> can run as much as possible as they do on their native platform. That 
> evidently includes DBus, but not just so that Gnome apps can talk with other 
> Gnome apps and KDE apps with other KDE apps. I don't have any stats on this, 
> but I'd expect that a good many of the users of such projects use them not so 
> much for software development per se, but as tools for their actual work 
> (often in science and/or engineering, in my impression). That might very well 
> include using Gnome/GTk apps alongside KDE applications, and in that case 
> they'd proba
 bly expect  the same kind of interoperability as those apps would have on the 
other platforms they might use.
> IOW, I don't think a distribution system that aims to provide the best 
> possible context for the applications it carries can get around using DBus.
> 
> But this is probably not the best place for an in-depth discussion around 
> underlying principles; that should probably be done on a ML (and ideally 
> include a DBus dev or two).
> 
> Nick Shaforostoff wrote:
> i don't see a reason behind introducing a special cmake option other than 
> code coverage (test dbus and dbus-free branches with the same qt): why one 
> should want to disable dbus on a system where he/she has Qt with dbus?
> 
> can you explain this to me?
> 
> regarding homebrew, i repeat: by default it installs a precompiled 
> version of Qt without dbus. if user wants dbus then he/she has to have 
> homebrew recompile whole Qt (takes about 1 hour). and what if kde app doesn't 
> need any IPC? that would just pollute his/her system with redundant stuff. 
> how many gtk-based apps require dbus to run on windows and osx?

We have Kate as an example. At the moment the main thing we need DBus for on 
windows is opening documents in only one window when opening new documents 
through explorer. Without the DBus daemon it does not work. KDevelop has a 
solution for it without using DBus which means that we could skip DBus usage 
for this purpose and we would not need to tell people to install the service 
(DBus) that occasionally has been flagged as mallware.

You might argue that we should just compile support without installing the 
service, but how do I know what features don't work without the service? If the 
frameworks need special options to disable DBus I'm informed about what is 
disabled and can choose if I need it or not. Without the option the feature is 
silently disabled. This is the reason I want separate options for each 
framework that provides it and not a global one in ECM.

Almost all Qt users on Windows that I know of would not even dream of compiling 
their own Qt and pre-compiled Qt comes with DBus support.


- Kåre


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


On May 12, 2016, 5:16 a.m., Nick Shaforostoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127896/
> ---
> 
> (Updated May 12, 2016, 5:16 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks, David Edmundson, 
> and Martin Gräßlin.
> 
> 
> Repository: kauth
> 
> 
> Description
> ---
> 
> this is the first patch to make kde frameworks build (and then work) without 
> dbus.
> 
> this will allow homebrew users use precompiled vanilla Qt to build kde apps 
> on osx. as dbus is not a common service in osx world, 

Re: Review Request 127896: make dbus optional on osx: kauth

2016-05-12 Thread René J . V . Bertin


> On May 12, 2016, 7:43 a.m., René J.V. Bertin wrote:
> > autotests/BackendsManager.cpp, lines 56-60
> > 
> >
> > Ditto, no risk of a build failure on systems where Qt does provide a 
> > DBus interface?
> 
> Nick Shaforostoff wrote:
> no rosk. please read the 'testing' section of this review request

I did, and it appears you didn't test on OS X with DBus available. That's the 
potential risk I saw.


- René J.V.


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


On May 12, 2016, 7:16 a.m., Nick Shaforostoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127896/
> ---
> 
> (Updated May 12, 2016, 7:16 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks, David Edmundson, 
> and Martin Gräßlin.
> 
> 
> Repository: kauth
> 
> 
> Description
> ---
> 
> this is the first patch to make kde frameworks build (and then work) without 
> dbus.
> 
> this will allow homebrew users use precompiled vanilla Qt to build kde apps 
> on osx. as dbus is not a common service in osx world, kde apps on osx should 
> use native means for interprocess communication instead -- this will make 
> them better citizens in osx ecosystem.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 48dc2d9 
>   autotests/BackendsManager.cpp 59675b3 
>   autotests/CMakeLists.txt b53d760 
>   autotests/HelperTest.cpp 8050a06 
>   src/CMakeLists.txt 1b6930d 
>   src/ConfigureChecks.cmake d46761a 
> 
> Diff: https://git.reviewboard.kde.org/r/127896/diff/
> 
> 
> Testing
> ---
> 
> compiles fine on osx, compiles fine on linux, tests on linux still pass.
> 
> 
> Thanks,
> 
> Nick Shaforostoff
> 
>

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


Re: Review Request 127896: make dbus optional on osx: kauth

2016-05-12 Thread Nick Shaforostoff


> On May 12, 2016, 5:43 a.m., René J.V. Bertin wrote:
> > CMakeLists.txt, lines 14-18
> > 
> >
> > Am I right that on OS X use of DBus is going to depend on whether or 
> > not Qt provides the QtDBus component? If so, shouldn't this be done on MS 
> > Windows too?
> > 
> > If that's *not* the right interpretation:
> > PLEASE introduce proper option variables for this kind of thing, for 
> > instance in ECM. Consider the overal picture; is this only about DBus or is 
> > this ultimately about building standalone app bundles? In other words, 
> > would it be possible to name that option variable appropriately to allow it 
> > to act as a switch for other, related build options?
> > 
> > This is all the more appropriate (and *polite*) if this is a 
> > convenience change for HomeBrew - why would such a change oblige other 
> > packaging/distribution systems to add and maintain unknown amounts of 
> > additional patches to undo it?!

yes, on OS X use of DBus is going to depend on whether or not Qt provides the 
QtDBus component.


> On May 12, 2016, 5:43 a.m., René J.V. Bertin wrote:
> > autotests/BackendsManager.cpp, lines 26-30
> > 
> >
> > Again, double-checking: Is QT_NO_DBUS really going to be defined if the 
> > cmake system didn't do a `find_package(Qt5 ... DBus)`? IOW, is this change 
> > not going to introduce a build failure on systems where Qt does provide a 
> > DBus interface?

i tested on linux - it did build fine


> On May 12, 2016, 5:43 a.m., René J.V. Bertin wrote:
> > autotests/BackendsManager.cpp, lines 56-60
> > 
> >
> > Ditto, no risk of a build failure on systems where Qt does provide a 
> > DBus interface?

no rosk. please read the 'testing' section of this review request


> On May 12, 2016, 5:43 a.m., René J.V. Bertin wrote:
> > src/CMakeLists.txt, line 2
> > 
> >
> > Was this a redundant statement?

yes, it was redundant.


- Nick


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


On May 12, 2016, 5:16 a.m., Nick Shaforostoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127896/
> ---
> 
> (Updated May 12, 2016, 5:16 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks, David Edmundson, 
> and Martin Gräßlin.
> 
> 
> Repository: kauth
> 
> 
> Description
> ---
> 
> this is the first patch to make kde frameworks build (and then work) without 
> dbus.
> 
> this will allow homebrew users use precompiled vanilla Qt to build kde apps 
> on osx. as dbus is not a common service in osx world, kde apps on osx should 
> use native means for interprocess communication instead -- this will make 
> them better citizens in osx ecosystem.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 48dc2d9 
>   autotests/BackendsManager.cpp 59675b3 
>   autotests/CMakeLists.txt b53d760 
>   autotests/HelperTest.cpp 8050a06 
>   src/CMakeLists.txt 1b6930d 
>   src/ConfigureChecks.cmake d46761a 
> 
> Diff: https://git.reviewboard.kde.org/r/127896/diff/
> 
> 
> Testing
> ---
> 
> compiles fine on osx, compiles fine on linux, tests on linux still pass.
> 
> 
> Thanks,
> 
> Nick Shaforostoff
> 
>

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


Re: Review Request 127896: make dbus optional on osx: kauth

2016-05-12 Thread Nick Shaforostoff


> On May 12, 2016, 6:15 a.m., Kåre Särs wrote:
> > I think this patch should not include any platform specific defines. 
> > Disabling DBus requirement on Windows might also be interesting for some 
> > projects. I propose to do something similar to what is done in kxmlgui to 
> > disable kglobalaccel. The default is to require kglobalaccel, but if you 
> > knowingly specify "-DFORCE_DISABLE_KGLOBALACCEL=1" kglobalaccel is not 
> > required or searched for.
> 
> René J.V. Bertin wrote:
> True, even if there's probably very little point in disabling DBus on a 
> standard Unix/Linux set-up.
> 
> But indeed, a platform-agnostic option can still be included in the 
> options that will be flipped by a platform-specific option like 
> `APPLE_STANDALONE_BUNDLE` I've proposed in a patch for Marble.
> 
> To come back to what I suggested earlier: even if there were to be a KF5 
> framework that encapsulates DBus or "something more platform native" there 
> would still be a use-case for using DBus through that framework even on OS X 
> (or MS Windows). Projects like HomeBrew, MacPorts and Cygwin don't exist to 
> replace the native desktop, but to complement it; to provide an easy way to 
> install and maintain FOSS and provide a context in which those applications 
> can run as much as possible as they do on their native platform. That 
> evidently includes DBus, but not just so that Gnome apps can talk with other 
> Gnome apps and KDE apps with other KDE apps. I don't have any stats on this, 
> but I'd expect that a good many of the users of such projects use them not so 
> much for software development per se, but as tools for their actual work 
> (often in science and/or engineering, in my impression). That might very well 
> include using Gnome/GTk apps alongside KDE applications, and in that case 
> they'd proba
 bly expect  the same kind of interoperability as those apps would have on the 
other platforms they might use.
> IOW, I don't think a distribution system that aims to provide the best 
> possible context for the applications it carries can get around using DBus.
> 
> But this is probably not the best place for an in-depth discussion around 
> underlying principles; that should probably be done on a ML (and ideally 
> include a DBus dev or two).

i don't see a reason behind introducing a special cmake option other than code 
coverage (test dbus and dbus-free branches with the same qt): why one should 
want to disable dbus on a system where he/she has Qt with dbus?

can you explain this to me?

regarding homebrew, i repeat: by default it installs a precompiled version of 
Qt without dbus. if user wants dbus then he/she has to have homebrew recompile 
whole Qt (takes about 1 hour). and what if kde app doesn't need any IPC? that 
would just pollute his/her system with redundant stuff. how many gtk-based apps 
require dbus to run on windows and osx?


- Nick


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


On May 12, 2016, 5:16 a.m., Nick Shaforostoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127896/
> ---
> 
> (Updated May 12, 2016, 5:16 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks, David Edmundson, 
> and Martin Gräßlin.
> 
> 
> Repository: kauth
> 
> 
> Description
> ---
> 
> this is the first patch to make kde frameworks build (and then work) without 
> dbus.
> 
> this will allow homebrew users use precompiled vanilla Qt to build kde apps 
> on osx. as dbus is not a common service in osx world, kde apps on osx should 
> use native means for interprocess communication instead -- this will make 
> them better citizens in osx ecosystem.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 48dc2d9 
>   autotests/BackendsManager.cpp 59675b3 
>   autotests/CMakeLists.txt b53d760 
>   autotests/HelperTest.cpp 8050a06 
>   src/CMakeLists.txt 1b6930d 
>   src/ConfigureChecks.cmake d46761a 
> 
> Diff: https://git.reviewboard.kde.org/r/127896/diff/
> 
> 
> Testing
> ---
> 
> compiles fine on osx, compiles fine on linux, tests on linux still pass.
> 
> 
> Thanks,
> 
> Nick Shaforostoff
> 
>

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


Re: Review Request 127878: use the "selected" icon mode in file open dialog sidebar

2016-05-12 Thread Marco Martin

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

(Updated May 12, 2016, 9:53 a.m.)


Review request for KDE Frameworks and Plasma.


Changes
---

similar logic in the behavior as qcommonstyle


Repository: kio


Description
---

since now kiconloader can color the icons in the sidebar, by using the 
"selected" icon mode, when the breeze icon theme is used the icon of the 
current sidebar item will be of the same color of selected text (white-ish by 
default), is more in line with breeze style and the monochrome icons become 
more readable


Diffs (updated)
-

  src/filewidgets/kfileplacesview.cpp 03074e0 
  src/widgets/kfileitemdelegate.cpp 1516b9e 

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


Testing
---


File Attachments


menu3.png
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/05/09/41f52d86-6fc5-4799-bd9e-18d17dc5a65a__menu3.png


Thanks,

Marco Martin

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


Re: Review Request 127878: use the "selected" icon mode in file open dialog sidebar

2016-05-12 Thread Marco Martin


> On May 11, 2016, 11:03 a.m., David Edmundson wrote:
> > src/widgets/kfileitemdelegate.cpp, line 1220
> > 
> >
> > QCommonStyle when rendering CE_ItemViewItem checks for disabled first
> > 
> > QIcon::Mode mode = QIcon::Normal;
> > if (!(vopt->state & QStyle::State_Enabled))
> > mode = QIcon::Disabled;
> > else if (vopt->state & QStyle::State_Selected)
> > mode = QIcon::Selected;
> > 
> > qcommonstyle:2167
> > 
> > it's probably best to emulate that
> > 
> > 
> > also why do you have the && active?
> > 
> > That means it'd need to be both selected and have mouse over at the 
> > same time in order to change icon?

yes, it changes the color only on active palette, changing it on inactive 
palette doesn't look really good.

unfortunately can't really pass the palette state to the iconloader, so when 
recoloring is always using the active palette, not doing the invert (that would 
be with active colors) on inactive palette seems to be the way that looks less 
bad


- Marco


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


On May 10, 2016, 11:57 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127878/
> ---
> 
> (Updated May 10, 2016, 11:57 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> since now kiconloader can color the icons in the sidebar, by using the 
> "selected" icon mode, when the breeze icon theme is used the icon of the 
> current sidebar item will be of the same color of selected text (white-ish by 
> default), is more in line with breeze style and the monochrome icons become 
> more readable
> 
> 
> Diffs
> -
> 
>   src/filewidgets/kfileplacesview.cpp 03074e0 
>   src/widgets/kfileitemdelegate.cpp 1516b9e 
> 
> Diff: https://git.reviewboard.kde.org/r/127878/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> menu3.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/05/09/41f52d86-6fc5-4799-bd9e-18d17dc5a65a__menu3.png
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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


Re: Review Request 127251: [KUnitConversion] Fix downloading currency exchange rates

2016-05-12 Thread Kai Uwe Broulik

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

(Updated May 12, 2016, 8:57 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Vishesh Handa.


Changes
---

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


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


Repository: kunitconversion


Description
---

QNetworkReply does not implement waitForReadyRead

Also, the code never actually created the cache dir it was trying to create a 
file in.


Diffs
-

  src/currency.cpp ad325d8 

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


Testing
---

Works now. It's downloaded once and then taken from cache file in 
~/.local/share/libkunitconversion/currency.xml

Given it's a Tier 2 framework doesn't make sense to add KIO now, also failed to 
reproduce the crashes mentioned in the code.

Tests pass (only if I run them on English locale btw)

Obviously not happy with this being sync but alas that's how the API works.

Not sure if this is a KRunner bug or KUnitConverison but if I enter "5 USD" it 
converts fine, if I enter "5 usd" it returns zero for all the currencies it 
converted to.


Thanks,

Kai Uwe Broulik

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


Review Request 127897: Update documentation of kdoctools_install macro

2016-05-12 Thread Elvis Angelaccio

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

Review request for Documentation, KDE Frameworks and Luigi Toscano.


Repository: kdoctools


Description
---

The macro will not work if the folders structure is `//docs`, so 
its documentation should be updated to the `//docs/` 
structure instead.


Diffs
-

  KF5DocToolsMacros.cmake a0ed3a8281266dcfdc95727184df851b3455cbf1 

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


Testing
---


Thanks,

Elvis Angelaccio

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


Re: Review Request 127896: make dbus optional on osx: kauth

2016-05-12 Thread René J . V . Bertin


> On May 12, 2016, 8:15 a.m., Kåre Särs wrote:
> > I think this patch should not include any platform specific defines. 
> > Disabling DBus requirement on Windows might also be interesting for some 
> > projects. I propose to do something similar to what is done in kxmlgui to 
> > disable kglobalaccel. The default is to require kglobalaccel, but if you 
> > knowingly specify "-DFORCE_DISABLE_KGLOBALACCEL=1" kglobalaccel is not 
> > required or searched for.

True, even if there's probably very little point in disabling DBus on a 
standard Unix/Linux set-up.

But indeed, a platform-agnostic option can still be included in the options 
that will be flipped by a platform-specific option like 
`APPLE_STANDALONE_BUNDLE` I've proposed in a patch for Marble.

To come back to what I suggested earlier: even if there were to be a KF5 
framework that encapsulates DBus or "something more platform native" there 
would still be a use-case for using DBus through that framework even on OS X 
(or MS Windows). Projects like HomeBrew, MacPorts and Cygwin don't exist to 
replace the native desktop, but to complement it; to provide an easy way to 
install and maintain FOSS and provide a context in which those applications can 
run as much as possible as they do on their native platform. That evidently 
includes DBus, but not just so that Gnome apps can talk with other Gnome apps 
and KDE apps with other KDE apps. I don't have any stats on this, but I'd 
expect that a good many of the users of such projects use them not so much for 
software development per se, but as tools for their actual work (often in 
science and/or engineering, in my impression). That might very well include 
using Gnome/GTk apps alongside KDE applications, and in that case they'd 
probably ex
 pect  the same kind of interoperability as those apps would have on the other 
platforms they might use.
IOW, I don't think a distribution system that aims to provide the best possible 
context for the applications it carries can get around using DBus.

But this is probably not the best place for an in-depth discussion around 
underlying principles; that should probably be done on a ML (and ideally 
include a DBus dev or two).


- René J.V.


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


On May 12, 2016, 7:16 a.m., Nick Shaforostoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127896/
> ---
> 
> (Updated May 12, 2016, 7:16 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks, David Edmundson, 
> and Martin Gräßlin.
> 
> 
> Repository: kauth
> 
> 
> Description
> ---
> 
> this is the first patch to make kde frameworks build (and then work) without 
> dbus.
> 
> this will allow homebrew users use precompiled vanilla Qt to build kde apps 
> on osx. as dbus is not a common service in osx world, kde apps on osx should 
> use native means for interprocess communication instead -- this will make 
> them better citizens in osx ecosystem.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 48dc2d9 
>   autotests/BackendsManager.cpp 59675b3 
>   autotests/CMakeLists.txt b53d760 
>   autotests/HelperTest.cpp 8050a06 
>   src/CMakeLists.txt 1b6930d 
>   src/ConfigureChecks.cmake d46761a 
> 
> Diff: https://git.reviewboard.kde.org/r/127896/diff/
> 
> 
> Testing
> ---
> 
> compiles fine on osx, compiles fine on linux, tests on linux still pass.
> 
> 
> Thanks,
> 
> Nick Shaforostoff
> 
>

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


Re: Review Request 127896: make dbus optional on osx: kauth

2016-05-12 Thread Kåre Särs

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



I think this patch should not include any platform specific defines. Disabling 
DBus requirement on Windows might also be interesting for some projects. I 
propose to do something similar to what is done in kxmlgui to disable 
kglobalaccel. The default is to require kglobalaccel, but if you knowingly 
specify "-DFORCE_DISABLE_KGLOBALACCEL=1" kglobalaccel is not required or 
searched for.

- Kåre Särs


On May 12, 2016, 5:16 a.m., Nick Shaforostoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127896/
> ---
> 
> (Updated May 12, 2016, 5:16 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks, David Edmundson, 
> and Martin Gräßlin.
> 
> 
> Repository: kauth
> 
> 
> Description
> ---
> 
> this is the first patch to make kde frameworks build (and then work) without 
> dbus.
> 
> this will allow homebrew users use precompiled vanilla Qt to build kde apps 
> on osx. as dbus is not a common service in osx world, kde apps on osx should 
> use native means for interprocess communication instead -- this will make 
> them better citizens in osx ecosystem.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 48dc2d9 
>   autotests/BackendsManager.cpp 59675b3 
>   autotests/CMakeLists.txt b53d760 
>   autotests/HelperTest.cpp 8050a06 
>   src/CMakeLists.txt 1b6930d 
>   src/ConfigureChecks.cmake d46761a 
> 
> Diff: https://git.reviewboard.kde.org/r/127896/diff/
> 
> 
> Testing
> ---
> 
> compiles fine on osx, compiles fine on linux, tests on linux still pass.
> 
> 
> Thanks,
> 
> Nick Shaforostoff
> 
>

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


Re: Review Request 127896: make dbus optional on osx: kauth

2016-05-12 Thread Martin Gräßlin

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



Approach looks good to me, though I agree with Rene on making this a proper 
cmake option.

- Martin Gräßlin


On May 12, 2016, 7:16 a.m., Nick Shaforostoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127896/
> ---
> 
> (Updated May 12, 2016, 7:16 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks, David Edmundson, 
> and Martin Gräßlin.
> 
> 
> Repository: kauth
> 
> 
> Description
> ---
> 
> this is the first patch to make kde frameworks build (and then work) without 
> dbus.
> 
> this will allow homebrew users use precompiled vanilla Qt to build kde apps 
> on osx. as dbus is not a common service in osx world, kde apps on osx should 
> use native means for interprocess communication instead -- this will make 
> them better citizens in osx ecosystem.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 48dc2d9 
>   autotests/BackendsManager.cpp 59675b3 
>   autotests/CMakeLists.txt b53d760 
>   autotests/HelperTest.cpp 8050a06 
>   src/CMakeLists.txt 1b6930d 
>   src/ConfigureChecks.cmake d46761a 
> 
> Diff: https://git.reviewboard.kde.org/r/127896/diff/
> 
> 
> Testing
> ---
> 
> compiles fine on osx, compiles fine on linux, tests on linux still pass.
> 
> 
> Thanks,
> 
> Nick Shaforostoff
> 
>

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