Moving KDiagram to extragear/graphics/libs (was: Re: KDiagram libs (KChart, KGantt) in KDE Review)

2015-02-21 Thread Friedrich W. H. Kossebau
Hi,

2 weeks have passed, seems there are no stoppers for KDiagram to move on into 
extragear/graphics/libs :) So filing now a ticket to sysadmin to do the move.

Thanks Albert, Aleix, Adriaan for having given KDiagram a check and (helping 
on) solving the issues you found (so KDiagram is Triple A rated? ;) )

Thanks also to whoever silently gave KDiagram some testing but did not report 
anything (hopefully a good sign).

Status:
* KDiagram builds on CI without warnings, all tests pass:
  http://build.kde.org/job/kdiagram_master_qt5/
* bugs.kde.org: added Product "kdiagram", components "KChart" & "KGantt"
* reviewboard: "kdiagram" (first review request handled)
* i18n.kde.org: integrated, first translations done:
  http://i18n.kde.org/stats/gui/trunk-kf5/po/kgantt_qt.po/
  http://i18n.kde.org/stats/gui/trunk-kf5/po/kchart_qt.po/
* KMyMoney frameworks branch ported to KChart (and found first bugs :P)
* Massif-Visualizer frameworks branch has review request for port to KChart
  (maintainer on vacation currently)
* Calligra will start the Qt5/KF5 port the next days, so nothing yet to do

Currently known unsolved issues (which I consider no showstoppers):
* lupdate/Scripty fails on some files, but no deal right now, as no strings
  are in code affected. Will look into once I have more time.
* code is build with different flag if unit tests are build, not perfect
  actually only enables unit tests embedded directly in the source files,
  so not changing actual library code
  (marked as TODO for now, no regression against currently used
  old snapshots of KDChart)

First release:
Other than initially planned I will not do an immediate release of KDiagram 
now, but first collect some more feedback by the projects porting to it, 
especially Calligra. First release will still happen before or at time the 
first release of any project known that ported to it of course (so tell if 
yours does :) ).

Cheers
Friedrich


Re: Review Request 122249: libksysguard: add Kill Window to End Process button and show correct keyboard shortcut

2015-02-21 Thread Thomas Lübking


> On Jan. 26, 2015, 7:05 vorm., Martin Gräßlin wrote:
> > My opinion is that this is a feature which should not be exposed in 
> > libksysguard. It actually ties libksysguard to KWin, while libksysguard was 
> > in the past also used in e.g. kdevelop.
> > 
> > If libksysguard wants to offer the functionality to kill a window, it 
> > should implement it itself.
> 
> Martin Gräßlin wrote:
> In addition: KWin's global shortcut action names are not public API. We 
> do not guarantee that we don't change them, we do not guarantee that they are 
> exposed at all (KWin handling shortcuts internally without kglobalaccel on 
> Wayland?). I do not want to run into situations that we cannot change our 
> code because external usage makes it impossible.
> 
> Thomas Lübking wrote:
> In case there was larger demand for invoking such action (taskbar, 
> dedicated plasmoid, ...) one could move the xkill functionality into 
> KWindowSystem (option for portage) - invoking a kwin shortcut through a 
> kglobalaccel dbus call is a hack. Maybe sufficient for any downstream 
> solution, but easily broken feature.
> 
> Gregor Mi wrote:
> First of all, a clarification of this RR's intentions:
> 1) The original "End Process..." tooltip says "you can always use 
> Ctrl+Alt+Esc..." which is wrong as soon as someone changes the keyboard 
> shortcut exposed by KWin. So this should be fixed.
> 2) Make the Kill Window feature more discoverable. It is a seldom used 
> feature which makes it harder to remember.
> 
> About invoking Kill Window:
> > If libksysguard wants to offer the functionality to kill a window, it 
> should implement it itself. [Martin]
> > ...one could move the xkill functionality into KWindowSystem...  
> [Thomas]
> 
> Without knowing the amount of xkill code I suspect that having a dbus 
> call that loosly couples libksysguard to KWin is probably easier to maintain 
> than 2 times the xkill code.
> Having said that, what about moving the xkill code to a common location 
> as Thomas suggested?
> 
> > We do not guarantee that we don't change them, we do not guarantee that 
> they are exposed at all ... I do not want to run into situations that we 
> cannot change our code because external usage makes it impossible.
> 
> Understood. But would it then be possible at all to get the current 
> shortcut to display it to the user?
> 
> Martin Gräßlin wrote:
> Ok, so this addresses two issues using one solution: exposing KWin's 
> internal shortcut. This is bad as outlined above.
> 
> I agree that 1) needs fixing. This can be done in the way as approached 
> in this review request: check whether kwin is registered on kglobalaccel and 
> get the key command. If it's done that way the fault is with libksysguard in 
> case KWin changes the shortcut name or doesn't use kglobalaccel any more. 
> Another fix is of course to just hide the shortcut.
> 
> 2) is a different issue. Whether it's needed to expose the functionality 
> in addition from libksysguard is probably questionable. A better approach to 
> do this would be through a method in KWindowSystem. This does not need to 
> duplicate the code, it could also just send a client message to the window 
> manager to start the kill window process. Through KWindowSystem we can check 
> whether the feature is supported by the window manager and could exclude if 
> not supported. But and that's a big but: the feature would not be able to 
> work if it's triggered from a (context) menu or drop down list (it needs to 
> grab mouse). Given that I'm hesistant to say that it should be added to 
> kwindowsystem at all.
> 
> Thomas Lübking wrote:
> ad 2)
> I'd have said to rather *move* the code to KWindowSystem and use it from 
> there by any client (incl. kwin)
> This allows porting the solution (in case such is possible on other 
> systems at all) as well as to invoke the feature unconditionally (ie. instead 
> of "is this kwin?  yes? tell kwin to trigger xkill." just trigger the xkill 
> functionality)
> 
> About the popupmenu:
> The issue is global, ie. as long as a popup (or other grabber) is 
> around, the kwin shortcut neither works.
> It's kind of the client codes problem to deal w/ a "false" return (eg. 
> invoke a timer and/or timered retries)
> 
> Gregor Mi wrote:
> ad 1) (shortcut)
> I could live with adapting (or remove) the shortcut retrieval as soon as 
> it will not be possible anymore. As long as it is, I would show it. (I 
> suspect as long as the shortcut is not hard-coded there will be a some way to 
> get it)
> 
> 
> ad 2) (invoke window kill)
> I looked a Kwin's source code. For reference, here are the two methods I 
> found to kill a window:
> ```
> /*!
>   Kill Window feature, similar to xkill
>  */
> void Workspace::slotKillWindow()
> {
> if (m_windowKiller.isNull()) {
> m_windowKiller

Re: Review Request 122249: libksysguard: add Kill Window to End Process button and show correct keyboard shortcut

2015-02-21 Thread Gregor Mi


> On Jan. 26, 2015, 7:05 a.m., Martin Gräßlin wrote:
> > My opinion is that this is a feature which should not be exposed in 
> > libksysguard. It actually ties libksysguard to KWin, while libksysguard was 
> > in the past also used in e.g. kdevelop.
> > 
> > If libksysguard wants to offer the functionality to kill a window, it 
> > should implement it itself.
> 
> Martin Gräßlin wrote:
> In addition: KWin's global shortcut action names are not public API. We 
> do not guarantee that we don't change them, we do not guarantee that they are 
> exposed at all (KWin handling shortcuts internally without kglobalaccel on 
> Wayland?). I do not want to run into situations that we cannot change our 
> code because external usage makes it impossible.
> 
> Thomas Lübking wrote:
> In case there was larger demand for invoking such action (taskbar, 
> dedicated plasmoid, ...) one could move the xkill functionality into 
> KWindowSystem (option for portage) - invoking a kwin shortcut through a 
> kglobalaccel dbus call is a hack. Maybe sufficient for any downstream 
> solution, but easily broken feature.
> 
> Gregor Mi wrote:
> First of all, a clarification of this RR's intentions:
> 1) The original "End Process..." tooltip says "you can always use 
> Ctrl+Alt+Esc..." which is wrong as soon as someone changes the keyboard 
> shortcut exposed by KWin. So this should be fixed.
> 2) Make the Kill Window feature more discoverable. It is a seldom used 
> feature which makes it harder to remember.
> 
> About invoking Kill Window:
> > If libksysguard wants to offer the functionality to kill a window, it 
> should implement it itself. [Martin]
> > ...one could move the xkill functionality into KWindowSystem...  
> [Thomas]
> 
> Without knowing the amount of xkill code I suspect that having a dbus 
> call that loosly couples libksysguard to KWin is probably easier to maintain 
> than 2 times the xkill code.
> Having said that, what about moving the xkill code to a common location 
> as Thomas suggested?
> 
> > We do not guarantee that we don't change them, we do not guarantee that 
> they are exposed at all ... I do not want to run into situations that we 
> cannot change our code because external usage makes it impossible.
> 
> Understood. But would it then be possible at all to get the current 
> shortcut to display it to the user?
> 
> Martin Gräßlin wrote:
> Ok, so this addresses two issues using one solution: exposing KWin's 
> internal shortcut. This is bad as outlined above.
> 
> I agree that 1) needs fixing. This can be done in the way as approached 
> in this review request: check whether kwin is registered on kglobalaccel and 
> get the key command. If it's done that way the fault is with libksysguard in 
> case KWin changes the shortcut name or doesn't use kglobalaccel any more. 
> Another fix is of course to just hide the shortcut.
> 
> 2) is a different issue. Whether it's needed to expose the functionality 
> in addition from libksysguard is probably questionable. A better approach to 
> do this would be through a method in KWindowSystem. This does not need to 
> duplicate the code, it could also just send a client message to the window 
> manager to start the kill window process. Through KWindowSystem we can check 
> whether the feature is supported by the window manager and could exclude if 
> not supported. But and that's a big but: the feature would not be able to 
> work if it's triggered from a (context) menu or drop down list (it needs to 
> grab mouse). Given that I'm hesistant to say that it should be added to 
> kwindowsystem at all.
> 
> Thomas Lübking wrote:
> ad 2)
> I'd have said to rather *move* the code to KWindowSystem and use it from 
> there by any client (incl. kwin)
> This allows porting the solution (in case such is possible on other 
> systems at all) as well as to invoke the feature unconditionally (ie. instead 
> of "is this kwin?  yes? tell kwin to trigger xkill." just trigger the xkill 
> functionality)
> 
> About the popupmenu:
> The issue is global, ie. as long as a popup (or other grabber) is 
> around, the kwin shortcut neither works.
> It's kind of the client codes problem to deal w/ a "false" return (eg. 
> invoke a timer and/or timered retries)
> 
> Gregor Mi wrote:
> ad 1) (shortcut)
> I could live with adapting (or remove) the shortcut retrieval as soon as 
> it will not be possible anymore. As long as it is, I would show it. (I 
> suspect as long as the shortcut is not hard-coded there will be a some way to 
> get it)
> 
> 
> ad 2) (invoke window kill)
> I looked a Kwin's source code. For reference, here are the two methods I 
> found to kill a window:
> ```
> /*!
>   Kill Window feature, similar to xkill
>  */
> void Workspace::slotKillWindow()
> {
> if (m_windowKiller.isNull()) {
> m_windowKiller.

Re: Review Request 122249: libksysguard: add Kill Window to End Process button and show correct keyboard shortcut

2015-02-21 Thread Gregor Mi


> On Feb. 21, 2015, 4:08 p.m., Thomas Lübking wrote:
> > processui/ksysguardprocesslist.cpp, line 367
> > 
> >
> > leaving aside that the patch is not "clean" (still contains 
> > kglobalaccel stuff, ie. is probably just a variant proposal):
> > 
> > I don't have xkill installed.
> > It's not a dependency of anything in KDE - you'd have to add it (for 
> > package maintainers, no idea how to do that, though)
> > 
> > (ftr: the "popup grabs mouse" limitations mostly hold for invoking 
> > xkill as well - just that there it would become harder to check for 
> > successful invocation)

kglobalaccel: I thought this would be ok to get the global shortcut for the 
killing action.

xkill: ah, ok. I thought it comes with X. I'll remove it.

"popup grabs mouse" limitation: I am not that familiar with that. How would 
that affect an xill invocation?


- Gregor


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


On Feb. 20, 2015, 11:35 p.m., Gregor Mi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122249/
> ---
> 
> (Updated Feb. 20, 2015, 11:35 p.m.)
> 
> 
> Review request for KDE Base Apps, Martin Gräßlin and John Tapsell.
> 
> 
> Repository: libksysguard
> 
> 
> Description
> ---
> 
> Current situation:
> The "End Process..." button has a tooltip which says "To target a specific 
> window to kill, press Ctrl+Alt+Esc at any time." The keyboard shortcut is 
> hardcoded.
> 
> RR:
> Add a drop down menu to the "End Process..." button with one action:
> i18n("Kill a specific window... (Global shortcut: %1)", killWindowShortcut)
> 
> 
> Diffs
> -
> 
>   processui/CMakeLists.txt 7f87b85e0201e63d69070a71203bbb34851a79c6 
>   processui/ProcessWidgetUI.ui e50f55cf1813b00d49b1716023df487ffbd536e3 
>   processui/keyboardshortcututil.h PRE-CREATION 
>   processui/keyboardshortcututil.cpp PRE-CREATION 
>   processui/ksysguardprocesslist.cpp 450ca600b8aed7ca611ec638610b6c524c96080c 
>   tests/CMakeLists.txt 967b03fae1e460bfb22e1a07ef05cf7b49412546 
>   tests/keyboardshortcututiltest.h PRE-CREATION 
>   tests/keyboardshortcututiltest.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/122249/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> New End Process button with drop down arrow
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/01/28/16301e88-e21b-4358-9a63-a85dae5722bd__screenshot_default1.png
> Drop down shows Kill Window
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/01/28/58df12c5-7350-4bb0-b602-c5716caa9836__screenshot_default2.png
> 
> 
> Thanks,
> 
> Gregor Mi
> 
>



Re: Warning: KPluginInfo::property("X-KDE-PluginInfo-Name") is deprecated

2015-02-21 Thread Sebastian Kügler
On Saturday, February 21, 2015 17:19:02 Alexander Richardson wrote:
> 2015-02-21 14:43 GMT+00:00 Marco Martin :
> > On Sat, Feb 21, 2015 at 1:34 PM, Alexander Richardson
> > 
> >  wrote:
> >> and then we could also have something like
> >> KServiceTypeTrader::findPlugin(serviceType, name) that expands to
> >> KServiceTypeTrader::self()->query(serviceType, [](const  KPluginInfo
> >> &info) {>>
> >>  return info.pluginName() == name;
> >>
> >> }
> >> 
> >> I have been meaning to add this for quite a while, but I am really
> >> busy at the moment.
> > 
> > So we would maintain such a warning, but start to port users to that
> > new api instead?
> 
> Yes, that would be my suggestion. I won't be able to work on it before
> Thursday though. Ideally there shouldn't be many changes required to
> make it work for KMimeTypeTrader aswell, so then in KF5 we can get rid
> of all the generated parsing code.

That's completely fine. We're not too much in a hurry, it's not a showstopper 
by any means, just some janitorial work to make Plasma a bit easier to debug. 
Currently, about 2/3 of its console output consists of these messages.

Thanks, Alex!
-- 
sebas

Sebastian Kügler|http://vizZzion.org| http://kde.org


Re: Warning: KPluginInfo::property("X-KDE-PluginInfo-Name") is deprecated

2015-02-21 Thread Alexander Richardson
2015-02-21 14:43 GMT+00:00 Marco Martin :
> On Sat, Feb 21, 2015 at 1:34 PM, Alexander Richardson
>  wrote:
>> and then we could also have something like
>> KServiceTypeTrader::findPlugin(serviceType, name) that expands to
>> KServiceTypeTrader::self()->query(serviceType, [](const  KPluginInfo &info) {
>>  return info.pluginName() == name;
>> }
>>
>> I have been meaning to add this for quite a while, but I am really
>> busy at the moment.
>
> So we would maintain such a warning, but start to port users to that
> new api instead?

Yes, that would be my suggestion. I won't be able to work on it before
Thursday though. Ideally there shouldn't be many changes required to
make it work for KMimeTypeTrader aswell, so then in KF5 we can get rid
of all the generated parsing code.

Alex


Re: Warning: KPluginInfo::property("X-KDE-PluginInfo-Name") is deprecated

2015-02-21 Thread Alexander Richardson
2015-02-21 10:02 GMT+00:00 Marco Martin :
> Hi all,
> As you may have noticed, right now starting plasma is a big spam of
> the following error:
> Calling KPluginInfo::property("X-KDE-PluginInfo-Name") is deprecated,
> use KPluginInfo::pluginName() in "/whatever/plugin.so" instead.
>
> i tried to see where it happens, and seems it's in
> ktradeparsetree.cpp , line 30
> QVariant ParseContext::property(const QString &_key) const
>
> I don't think this "properly" fixable, since from the stack trace it
> seems an appropriate use.. i see two ways to fix it:
>
> 1) in ParseContext::property stuf a very long if.. else.. that makes
> it call the proper KPluginInfo::correctAccessor() .. but is ugly and
> slows it down
>
> 2) since this is an appropriate use, consider it not wrong anymore,
> and just get rid of the warning.
>
> Opinions? ideas?
I guess most of these would result from a call to
KServiceTypeTrader::self()->query("KMyApp/Plugin",
"[X-KDE-PluginInfo-Name] = Foo").
My suggestion would be to add an overload to
KServiceTypeTrader::query() that takes a std::function instead of the constraints string.
Ideally we could then deprecate the string based version and use the
std::function version everywhere since that should be safer (and
faster).

KServiceTypeTrader::self()->query("KMyApp/Plugin", [](const
KPluginInfo &info) {
 return info.property("X-KMyApp-InterfaceVersion).toInt() > 15;
}
instead of
KServiceTypeTrader::self()->query("KMyApp/Plugin",
"[X-KMyApp-InterfaceVersion] > 15");

and then we could also have something like
KServiceTypeTrader::findPlugin(serviceType, name) that expands to
KServiceTypeTrader::self()->query(serviceType, [](const  KPluginInfo &info) {
 return info.pluginName() == name;
}

I have been meaning to add this for quite a while, but I am really
busy at the moment.

Alex


Re: Review Request 122249: libksysguard: add Kill Window to End Process button and show correct keyboard shortcut

2015-02-21 Thread Thomas Lübking

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



processui/ksysguardprocesslist.cpp


leaving aside that the patch is not "clean" (still contains kglobalaccel 
stuff, ie. is probably just a variant proposal):

I don't have xkill installed.
It's not a dependency of anything in KDE - you'd have to add it (for 
package maintainers, no idea how to do that, though)

(ftr: the "popup grabs mouse" limitations mostly hold for invoking xkill as 
well - just that there it would become harder to check for successful 
invocation)


- Thomas Lübking


On Feb. 20, 2015, 11:35 nachm., Gregor Mi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122249/
> ---
> 
> (Updated Feb. 20, 2015, 11:35 nachm.)
> 
> 
> Review request for KDE Base Apps, Martin Gräßlin and John Tapsell.
> 
> 
> Repository: libksysguard
> 
> 
> Description
> ---
> 
> Current situation:
> The "End Process..." button has a tooltip which says "To target a specific 
> window to kill, press Ctrl+Alt+Esc at any time." The keyboard shortcut is 
> hardcoded.
> 
> RR:
> Add a drop down menu to the "End Process..." button with one action:
> i18n("Kill a specific window... (Global shortcut: %1)", killWindowShortcut)
> 
> 
> Diffs
> -
> 
>   processui/CMakeLists.txt 7f87b85e0201e63d69070a71203bbb34851a79c6 
>   processui/ProcessWidgetUI.ui e50f55cf1813b00d49b1716023df487ffbd536e3 
>   processui/keyboardshortcututil.h PRE-CREATION 
>   processui/keyboardshortcututil.cpp PRE-CREATION 
>   processui/ksysguardprocesslist.cpp 450ca600b8aed7ca611ec638610b6c524c96080c 
>   tests/CMakeLists.txt 967b03fae1e460bfb22e1a07ef05cf7b49412546 
>   tests/keyboardshortcututiltest.h PRE-CREATION 
>   tests/keyboardshortcututiltest.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/122249/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> New End Process button with drop down arrow
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/01/28/16301e88-e21b-4358-9a63-a85dae5722bd__screenshot_default1.png
> Drop down shows Kill Window
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/01/28/58df12c5-7350-4bb0-b602-c5716caa9836__screenshot_default2.png
> 
> 
> Thanks,
> 
> Gregor Mi
> 
>



Re: Review Request 122249: libksysguard: add Kill Window to End Process button and show correct keyboard shortcut

2015-02-21 Thread Thomas Lübking


> On Jan. 26, 2015, 7:05 vorm., Martin Gräßlin wrote:
> > My opinion is that this is a feature which should not be exposed in 
> > libksysguard. It actually ties libksysguard to KWin, while libksysguard was 
> > in the past also used in e.g. kdevelop.
> > 
> > If libksysguard wants to offer the functionality to kill a window, it 
> > should implement it itself.
> 
> Martin Gräßlin wrote:
> In addition: KWin's global shortcut action names are not public API. We 
> do not guarantee that we don't change them, we do not guarantee that they are 
> exposed at all (KWin handling shortcuts internally without kglobalaccel on 
> Wayland?). I do not want to run into situations that we cannot change our 
> code because external usage makes it impossible.
> 
> Thomas Lübking wrote:
> In case there was larger demand for invoking such action (taskbar, 
> dedicated plasmoid, ...) one could move the xkill functionality into 
> KWindowSystem (option for portage) - invoking a kwin shortcut through a 
> kglobalaccel dbus call is a hack. Maybe sufficient for any downstream 
> solution, but easily broken feature.
> 
> Gregor Mi wrote:
> First of all, a clarification of this RR's intentions:
> 1) The original "End Process..." tooltip says "you can always use 
> Ctrl+Alt+Esc..." which is wrong as soon as someone changes the keyboard 
> shortcut exposed by KWin. So this should be fixed.
> 2) Make the Kill Window feature more discoverable. It is a seldom used 
> feature which makes it harder to remember.
> 
> About invoking Kill Window:
> > If libksysguard wants to offer the functionality to kill a window, it 
> should implement it itself. [Martin]
> > ...one could move the xkill functionality into KWindowSystem...  
> [Thomas]
> 
> Without knowing the amount of xkill code I suspect that having a dbus 
> call that loosly couples libksysguard to KWin is probably easier to maintain 
> than 2 times the xkill code.
> Having said that, what about moving the xkill code to a common location 
> as Thomas suggested?
> 
> > We do not guarantee that we don't change them, we do not guarantee that 
> they are exposed at all ... I do not want to run into situations that we 
> cannot change our code because external usage makes it impossible.
> 
> Understood. But would it then be possible at all to get the current 
> shortcut to display it to the user?
> 
> Martin Gräßlin wrote:
> Ok, so this addresses two issues using one solution: exposing KWin's 
> internal shortcut. This is bad as outlined above.
> 
> I agree that 1) needs fixing. This can be done in the way as approached 
> in this review request: check whether kwin is registered on kglobalaccel and 
> get the key command. If it's done that way the fault is with libksysguard in 
> case KWin changes the shortcut name or doesn't use kglobalaccel any more. 
> Another fix is of course to just hide the shortcut.
> 
> 2) is a different issue. Whether it's needed to expose the functionality 
> in addition from libksysguard is probably questionable. A better approach to 
> do this would be through a method in KWindowSystem. This does not need to 
> duplicate the code, it could also just send a client message to the window 
> manager to start the kill window process. Through KWindowSystem we can check 
> whether the feature is supported by the window manager and could exclude if 
> not supported. But and that's a big but: the feature would not be able to 
> work if it's triggered from a (context) menu or drop down list (it needs to 
> grab mouse). Given that I'm hesistant to say that it should be added to 
> kwindowsystem at all.
> 
> Thomas Lübking wrote:
> ad 2)
> I'd have said to rather *move* the code to KWindowSystem and use it from 
> there by any client (incl. kwin)
> This allows porting the solution (in case such is possible on other 
> systems at all) as well as to invoke the feature unconditionally (ie. instead 
> of "is this kwin?  yes? tell kwin to trigger xkill." just trigger the xkill 
> functionality)
> 
> About the popupmenu:
> The issue is global, ie. as long as a popup (or other grabber) is 
> around, the kwin shortcut neither works.
> It's kind of the client codes problem to deal w/ a "false" return (eg. 
> invoke a timer and/or timered retries)
> 
> Gregor Mi wrote:
> ad 1) (shortcut)
> I could live with adapting (or remove) the shortcut retrieval as soon as 
> it will not be possible anymore. As long as it is, I would show it. (I 
> suspect as long as the shortcut is not hard-coded there will be a some way to 
> get it)
> 
> 
> ad 2) (invoke window kill)
> I looked a Kwin's source code. For reference, here are the two methods I 
> found to kill a window:
> ```
> /*!
>   Kill Window feature, similar to xkill
>  */
> void Workspace::slotKillWindow()
> {
> if (m_windowKiller.isNull()) {
> m_windowKiller

Re: Warning: KPluginInfo::property("X-KDE-PluginInfo-Name") is deprecated

2015-02-21 Thread Marco Martin
On Sat, Feb 21, 2015 at 3:43 PM, Marco Martin  wrote:
> On Sat, Feb 21, 2015 at 1:34 PM, Alexander Richardson
>  wrote:
>> and then we could also have something like
>> KServiceTypeTrader::findPlugin(serviceType, name) that expands to
>> KServiceTypeTrader::self()->query(serviceType, [](const  KPluginInfo &info) {
>>  return info.pluginName() == name;
>> }

anyways, absolutely +1 on this new function, I like the idea a lot :)

--
Marco Martin


Re: Warning: KPluginInfo::property("X-KDE-PluginInfo-Name") is deprecated

2015-02-21 Thread Marco Martin
On Sat, Feb 21, 2015 at 1:34 PM, Alexander Richardson
 wrote:
> and then we could also have something like
> KServiceTypeTrader::findPlugin(serviceType, name) that expands to
> KServiceTypeTrader::self()->query(serviceType, [](const  KPluginInfo &info) {
>  return info.pluginName() == name;
> }
>
> I have been meaning to add this for quite a while, but I am really
> busy at the moment.

So we would maintain such a warning, but start to port users to that
new api instead?

--
Marco Martin


Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-02-21 Thread Sebastian Kügler


> On Feb. 21, 2015, 2:46 a.m., Hrvoje Senjan wrote:
> > can you check what needs to be adjusted in plasma-workspace? it fails to 
> > build with your change:
> > 
> > 
> > 
> > [  451s] 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:471:43:
> >  error: 'proc->KSysGuard::Process::command' does not have class type
> > [  451s]  QString cmdline = proc ? proc->command.simplified() : 
> > QString(); // proc->command has a trailing space???
> > [  451s]^
> > [  451s] 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:501:103:
> >  error: no matching function for call to 'KService::KService( > overloaded function type>, QString&, QString)'
> > [  451s]  services << QExplicitlySharedDataPointer(new 
> > KService(proc->name, cmdline, QString()));
> > [  451s]
> > ^
> > [  451s] 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:501:103:
> >  note: candidates are:
> > [  451s] In file included from /usr/include/KF5/KService/KService:1:0,
> > [  451s]  from 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:32:
> > [  451s] /usr/include/KF5/KService/kservice.h:580:5: note: 
> > KService::KService(QDataStream&, int)
> > [  451s]  KService(QDataStream &str, int offset);
> > [  451s]  ^
> > [  451s] /usr/include/KF5/KService/kservice.h:580:5: note:   candidate 
> > expects 2 arguments, 3 provided
> > [  451s] In file included from /usr/include/KF5/KService/KService:1:0,
> > [  451s]  from 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:32:
> > [  451s] /usr/include/KF5/KService/kservice.h:82:14: note: 
> > KService::KService(const KDesktopFile*, const QString&)
> > [  451s]  explicit KService(const KDesktopFile *config, const QString 
> > &entryPath = QString());
> > [  451s]   ^
> > [  451s] /usr/include/KF5/KService/kservice.h:82:14: note:   candidate 
> > expects 2 arguments, 3 provided
> > [  451s] /usr/include/KF5/KService/kservice.h:75:14: note: 
> > KService::KService(const QString&)
> > [  451s]  explicit KService(const QString &fullpath);
> > [  451s]   ^
> > [  451s] /usr/include/KF5/KService/kservice.h:75:14: note:   candidate 
> > expects 1 argument, 3 provided
> > [  451s] /usr/include/KF5/KService/kservice.h:68:5: note: 
> > KService::KService(const QString&, const QString&, const QString&)
> > [  451s]  KService(const QString &name, const QString &exec, const 
> > QString &icon);
> > [  451s]  ^
> > [  451s] /usr/include/KF5/KService/kservice.h:68:5: note:   no known 
> > conversion for argument 1 from '' to 
> > 'const QString&'
> > [  451s] 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:
> >  In function 'QUrl TaskManager::getServiceLauncherUrl(int, const QString&, 
> > const QStringList&)':
> > [  451s] 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:516:43:
> >  error: 'proc->KSysGuard::Process::command' does not have class type
> > [  451s]  QString cmdline = proc ? proc->command.simplified() : 
> > QString(); // proc->command has a trailing space???
> > [  451s]^
> 
> Gregor Mi wrote:
> Probably `proc->command` must be replace with `proc->command()`. I'll 
> check that.
> 
> Gregor Mi wrote:
> How can I find out if which branch was compiled? I assume it is the 
> master branch.
> 
> Bhushan Shah wrote:
> > How can I find out if which branch was compiled? I assume it is the 
> master branch.
> 
> Yes its master branch

Aleix and me have fixed plasma-workspace's build. Update the master branch.


- Sebastian


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


On Feb. 20, 2015, 9:46 p.m., Gregor Mi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121831/
> ---
> 
> (Updated Feb. 20, 2015, 9:46 p.m.)
> 
> 
> Review request for KDE Base Apps, Dominik Haumann, Eike Hein, and John 
> Tapsell.
> 
> 
> Repository: libksysguard
> 
> 
> Description
> ---
> 
> This is a follow-up to https://git.reviewboard.kde.org/r/121717/:
> 
> In process.h there are several public fields which easily trigger BIC 
> changes. This RR introduces a d-ptr.
> 
> (In a separate commit I would add the .reviewboardrc file)
> 
> 
> Diffs
> -
> 
>   processui/scripting.h 2445c0ab0d81af3283c0f6e9c5f34

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-02-21 Thread Bhushan Shah


> On Feb. 21, 2015, 8:16 a.m., Hrvoje Senjan wrote:
> > can you check what needs to be adjusted in plasma-workspace? it fails to 
> > build with your change:
> > 
> > 
> > 
> > [  451s] 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:471:43:
> >  error: 'proc->KSysGuard::Process::command' does not have class type
> > [  451s]  QString cmdline = proc ? proc->command.simplified() : 
> > QString(); // proc->command has a trailing space???
> > [  451s]^
> > [  451s] 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:501:103:
> >  error: no matching function for call to 'KService::KService( > overloaded function type>, QString&, QString)'
> > [  451s]  services << QExplicitlySharedDataPointer(new 
> > KService(proc->name, cmdline, QString()));
> > [  451s]
> > ^
> > [  451s] 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:501:103:
> >  note: candidates are:
> > [  451s] In file included from /usr/include/KF5/KService/KService:1:0,
> > [  451s]  from 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:32:
> > [  451s] /usr/include/KF5/KService/kservice.h:580:5: note: 
> > KService::KService(QDataStream&, int)
> > [  451s]  KService(QDataStream &str, int offset);
> > [  451s]  ^
> > [  451s] /usr/include/KF5/KService/kservice.h:580:5: note:   candidate 
> > expects 2 arguments, 3 provided
> > [  451s] In file included from /usr/include/KF5/KService/KService:1:0,
> > [  451s]  from 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:32:
> > [  451s] /usr/include/KF5/KService/kservice.h:82:14: note: 
> > KService::KService(const KDesktopFile*, const QString&)
> > [  451s]  explicit KService(const KDesktopFile *config, const QString 
> > &entryPath = QString());
> > [  451s]   ^
> > [  451s] /usr/include/KF5/KService/kservice.h:82:14: note:   candidate 
> > expects 2 arguments, 3 provided
> > [  451s] /usr/include/KF5/KService/kservice.h:75:14: note: 
> > KService::KService(const QString&)
> > [  451s]  explicit KService(const QString &fullpath);
> > [  451s]   ^
> > [  451s] /usr/include/KF5/KService/kservice.h:75:14: note:   candidate 
> > expects 1 argument, 3 provided
> > [  451s] /usr/include/KF5/KService/kservice.h:68:5: note: 
> > KService::KService(const QString&, const QString&, const QString&)
> > [  451s]  KService(const QString &name, const QString &exec, const 
> > QString &icon);
> > [  451s]  ^
> > [  451s] /usr/include/KF5/KService/kservice.h:68:5: note:   no known 
> > conversion for argument 1 from '' to 
> > 'const QString&'
> > [  451s] 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:
> >  In function 'QUrl TaskManager::getServiceLauncherUrl(int, const QString&, 
> > const QStringList&)':
> > [  451s] 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:516:43:
> >  error: 'proc->KSysGuard::Process::command' does not have class type
> > [  451s]  QString cmdline = proc ? proc->command.simplified() : 
> > QString(); // proc->command has a trailing space???
> > [  451s]^
> 
> Gregor Mi wrote:
> Probably `proc->command` must be replace with `proc->command()`. I'll 
> check that.
> 
> Gregor Mi wrote:
> How can I find out if which branch was compiled? I assume it is the 
> master branch.

> How can I find out if which branch was compiled? I assume it is the master 
> branch.

Yes its master branch


- Bhushan


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


On Feb. 21, 2015, 3:16 a.m., Gregor Mi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121831/
> ---
> 
> (Updated Feb. 21, 2015, 3:16 a.m.)
> 
> 
> Review request for KDE Base Apps, Dominik Haumann, Eike Hein, and John 
> Tapsell.
> 
> 
> Repository: libksysguard
> 
> 
> Description
> ---
> 
> This is a follow-up to https://git.reviewboard.kde.org/r/121717/:
> 
> In process.h there are several public fields which easily trigger BIC 
> changes. This RR introduces a d-ptr.
> 
> (In a separate commit I would add the .reviewboardrc file)
> 
> 
> Diffs
> -
> 
>   processui/scripting.h 2445c0ab0d81af3283c0f6e9c5f349a3d70b0de9 
>   processui/scripting.cpp 76291b0ae0a26e486aa81a4ca3976ff4a47cb3c0 
>   processcore/processes_solaris_p.cp

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-02-21 Thread Gregor Mi


> On Feb. 21, 2015, 2:46 a.m., Hrvoje Senjan wrote:
> > can you check what needs to be adjusted in plasma-workspace? it fails to 
> > build with your change:
> > 
> > 
> > 
> > [  451s] 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:471:43:
> >  error: 'proc->KSysGuard::Process::command' does not have class type
> > [  451s]  QString cmdline = proc ? proc->command.simplified() : 
> > QString(); // proc->command has a trailing space???
> > [  451s]^
> > [  451s] 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:501:103:
> >  error: no matching function for call to 'KService::KService( > overloaded function type>, QString&, QString)'
> > [  451s]  services << QExplicitlySharedDataPointer(new 
> > KService(proc->name, cmdline, QString()));
> > [  451s]
> > ^
> > [  451s] 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:501:103:
> >  note: candidates are:
> > [  451s] In file included from /usr/include/KF5/KService/KService:1:0,
> > [  451s]  from 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:32:
> > [  451s] /usr/include/KF5/KService/kservice.h:580:5: note: 
> > KService::KService(QDataStream&, int)
> > [  451s]  KService(QDataStream &str, int offset);
> > [  451s]  ^
> > [  451s] /usr/include/KF5/KService/kservice.h:580:5: note:   candidate 
> > expects 2 arguments, 3 provided
> > [  451s] In file included from /usr/include/KF5/KService/KService:1:0,
> > [  451s]  from 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:32:
> > [  451s] /usr/include/KF5/KService/kservice.h:82:14: note: 
> > KService::KService(const KDesktopFile*, const QString&)
> > [  451s]  explicit KService(const KDesktopFile *config, const QString 
> > &entryPath = QString());
> > [  451s]   ^
> > [  451s] /usr/include/KF5/KService/kservice.h:82:14: note:   candidate 
> > expects 2 arguments, 3 provided
> > [  451s] /usr/include/KF5/KService/kservice.h:75:14: note: 
> > KService::KService(const QString&)
> > [  451s]  explicit KService(const QString &fullpath);
> > [  451s]   ^
> > [  451s] /usr/include/KF5/KService/kservice.h:75:14: note:   candidate 
> > expects 1 argument, 3 provided
> > [  451s] /usr/include/KF5/KService/kservice.h:68:5: note: 
> > KService::KService(const QString&, const QString&, const QString&)
> > [  451s]  KService(const QString &name, const QString &exec, const 
> > QString &icon);
> > [  451s]  ^
> > [  451s] /usr/include/KF5/KService/kservice.h:68:5: note:   no known 
> > conversion for argument 1 from '' to 
> > 'const QString&'
> > [  451s] 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:
> >  In function 'QUrl TaskManager::getServiceLauncherUrl(int, const QString&, 
> > const QStringList&)':
> > [  451s] 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:516:43:
> >  error: 'proc->KSysGuard::Process::command' does not have class type
> > [  451s]  QString cmdline = proc ? proc->command.simplified() : 
> > QString(); // proc->command has a trailing space???
> > [  451s]^
> 
> Gregor Mi wrote:
> Probably `proc->command` must be replace with `proc->command()`. I'll 
> check that.

How can I find out if which branch was compiled? I assume it is the master 
branch.


- Gregor


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


On Feb. 20, 2015, 9:46 p.m., Gregor Mi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121831/
> ---
> 
> (Updated Feb. 20, 2015, 9:46 p.m.)
> 
> 
> Review request for KDE Base Apps, Dominik Haumann, Eike Hein, and John 
> Tapsell.
> 
> 
> Repository: libksysguard
> 
> 
> Description
> ---
> 
> This is a follow-up to https://git.reviewboard.kde.org/r/121717/:
> 
> In process.h there are several public fields which easily trigger BIC 
> changes. This RR introduces a d-ptr.
> 
> (In a separate commit I would add the .reviewboardrc file)
> 
> 
> Diffs
> -
> 
>   processui/scripting.h 2445c0ab0d81af3283c0f6e9c5f349a3d70b0de9 
>   processui/scripting.cpp 76291b0ae0a26e486aa81a4ca3976ff4a47cb3c0 
>   processcore/processes_solaris_p.cpp 
> f054df4b1e762e9cbec1ff8dea78f467b878bee0 
>   processui/ProcessFilter.cpp ec520593fb67c777d56817f2493d40dc5ade0347 
>   processui/Process

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-02-21 Thread Gregor Mi


> On Feb. 21, 2015, 2:46 a.m., Hrvoje Senjan wrote:
> > can you check what needs to be adjusted in plasma-workspace? it fails to 
> > build with your change:
> > 
> > 
> > 
> > [  451s] 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:471:43:
> >  error: 'proc->KSysGuard::Process::command' does not have class type
> > [  451s]  QString cmdline = proc ? proc->command.simplified() : 
> > QString(); // proc->command has a trailing space???
> > [  451s]^
> > [  451s] 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:501:103:
> >  error: no matching function for call to 'KService::KService( > overloaded function type>, QString&, QString)'
> > [  451s]  services << QExplicitlySharedDataPointer(new 
> > KService(proc->name, cmdline, QString()));
> > [  451s]
> > ^
> > [  451s] 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:501:103:
> >  note: candidates are:
> > [  451s] In file included from /usr/include/KF5/KService/KService:1:0,
> > [  451s]  from 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:32:
> > [  451s] /usr/include/KF5/KService/kservice.h:580:5: note: 
> > KService::KService(QDataStream&, int)
> > [  451s]  KService(QDataStream &str, int offset);
> > [  451s]  ^
> > [  451s] /usr/include/KF5/KService/kservice.h:580:5: note:   candidate 
> > expects 2 arguments, 3 provided
> > [  451s] In file included from /usr/include/KF5/KService/KService:1:0,
> > [  451s]  from 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:32:
> > [  451s] /usr/include/KF5/KService/kservice.h:82:14: note: 
> > KService::KService(const KDesktopFile*, const QString&)
> > [  451s]  explicit KService(const KDesktopFile *config, const QString 
> > &entryPath = QString());
> > [  451s]   ^
> > [  451s] /usr/include/KF5/KService/kservice.h:82:14: note:   candidate 
> > expects 2 arguments, 3 provided
> > [  451s] /usr/include/KF5/KService/kservice.h:75:14: note: 
> > KService::KService(const QString&)
> > [  451s]  explicit KService(const QString &fullpath);
> > [  451s]   ^
> > [  451s] /usr/include/KF5/KService/kservice.h:75:14: note:   candidate 
> > expects 1 argument, 3 provided
> > [  451s] /usr/include/KF5/KService/kservice.h:68:5: note: 
> > KService::KService(const QString&, const QString&, const QString&)
> > [  451s]  KService(const QString &name, const QString &exec, const 
> > QString &icon);
> > [  451s]  ^
> > [  451s] /usr/include/KF5/KService/kservice.h:68:5: note:   no known 
> > conversion for argument 1 from '' to 
> > 'const QString&'
> > [  451s] 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:
> >  In function 'QUrl TaskManager::getServiceLauncherUrl(int, const QString&, 
> > const QStringList&)':
> > [  451s] 
> > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:516:43:
> >  error: 'proc->KSysGuard::Process::command' does not have class type
> > [  451s]  QString cmdline = proc ? proc->command.simplified() : 
> > QString(); // proc->command has a trailing space???
> > [  451s]^

Probably `proc->command` must be replace with `proc->command()`. I'll check 
that.


- Gregor


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


On Feb. 20, 2015, 9:46 p.m., Gregor Mi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121831/
> ---
> 
> (Updated Feb. 20, 2015, 9:46 p.m.)
> 
> 
> Review request for KDE Base Apps, Dominik Haumann, Eike Hein, and John 
> Tapsell.
> 
> 
> Repository: libksysguard
> 
> 
> Description
> ---
> 
> This is a follow-up to https://git.reviewboard.kde.org/r/121717/:
> 
> In process.h there are several public fields which easily trigger BIC 
> changes. This RR introduces a d-ptr.
> 
> (In a separate commit I would add the .reviewboardrc file)
> 
> 
> Diffs
> -
> 
>   processui/scripting.h 2445c0ab0d81af3283c0f6e9c5f349a3d70b0de9 
>   processui/scripting.cpp 76291b0ae0a26e486aa81a4ca3976ff4a47cb3c0 
>   processcore/processes_solaris_p.cpp 
> f054df4b1e762e9cbec1ff8dea78f467b878bee0 
>   processui/ProcessFilter.cpp ec520593fb67c777d56817f2493d40dc5ade0347 
>   processui/ProcessModel.cpp 53bc041110c9cdb686fef783895104969b661889 
>   processui/ksysguardprocesslist.cpp 4dc142e864d8353ceafc3a6

Re: Warning: KPluginInfo::property("X-KDE-PluginInfo-Name") is deprecated

2015-02-21 Thread Sebastian Kügler
On Saturday, February 21, 2015 11:02:48 Marco Martin wrote:
> As you may have noticed, right now starting plasma is a big spam of
> the following error:
> Calling KPluginInfo::property("X-KDE-PluginInfo-Name") is deprecated,
> use KPluginInfo::pluginName() in "/whatever/plugin.so" instead.
> 
> i tried to see where it happens, and seems it's in
> ktradeparsetree.cpp , line 30
> QVariant ParseContext::property(const QString &_key) const
> 
> I don't think this "properly" fixable, since from the stack trace it
> seems an appropriate use.. i see two ways to fix it:
> 
> 1) in ParseContext::property stuf a very long if.. else.. that makes
> it call the proper KPluginInfo::correctAccessor() .. but is ugly and
> slows it down
> 
> 2) since this is an appropriate use, consider it not wrong anymore,
> and just get rid of the warning.
> 
> Opinions? ideas?

I've looked at it, too, since the warnings shown are just burying any useful 
information during the startup of Plasmashell. I think the warning is rather 
useless, and I'd just remove it, so 2).

Cheers,
-- 
sebas

Sebastian Kügler|http://vizZzion.org| http://kde.org


Warning: KPluginInfo::property("X-KDE-PluginInfo-Name") is deprecated

2015-02-21 Thread Marco Martin
Hi all,
As you may have noticed, right now starting plasma is a big spam of
the following error:
Calling KPluginInfo::property("X-KDE-PluginInfo-Name") is deprecated,
use KPluginInfo::pluginName() in "/whatever/plugin.so" instead.

i tried to see where it happens, and seems it's in
ktradeparsetree.cpp , line 30
QVariant ParseContext::property(const QString &_key) const

I don't think this "properly" fixable, since from the stack trace it
seems an appropriate use.. i see two ways to fix it:

1) in ParseContext::property stuf a very long if.. else.. that makes
it call the proper KPluginInfo::correctAccessor() .. but is ugly and
slows it down

2) since this is an appropriate use, consider it not wrong anymore,
and just get rid of the warning.

Opinions? ideas?

--
Marco Martin


Re: Review Request 122249: libksysguard: add Kill Window to End Process button and show correct keyboard shortcut

2015-02-21 Thread Gregor Mi

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

(Updated Feb. 20, 2015, 11:35 p.m.)


Review request for KDE Base Apps, Martin Gräßlin and John Tapsell.


Changes
---

- Use QProcess::startDetached("xkill"); instead of using the KWin method. This 
is not as advanced as the KWin method (e.g. the action can only be aborted with 
right-click instead of also Esc) but this way there is no coupling to KWin
- Add a "Don't ask again" message box that warns the user about what will 
happen.


Repository: libksysguard


Description
---

Current situation:
The "End Process..." button has a tooltip which says "To target a specific 
window to kill, press Ctrl+Alt+Esc at any time." The keyboard shortcut is 
hardcoded.

RR:
Add a drop down menu to the "End Process..." button with one action:
i18n("Kill a specific window... (Global shortcut: %1)", killWindowShortcut)


Diffs (updated)
-

  processui/CMakeLists.txt 7f87b85e0201e63d69070a71203bbb34851a79c6 
  processui/ProcessWidgetUI.ui e50f55cf1813b00d49b1716023df487ffbd536e3 
  processui/keyboardshortcututil.h PRE-CREATION 
  processui/keyboardshortcututil.cpp PRE-CREATION 
  processui/ksysguardprocesslist.cpp 450ca600b8aed7ca611ec638610b6c524c96080c 
  tests/CMakeLists.txt 967b03fae1e460bfb22e1a07ef05cf7b49412546 
  tests/keyboardshortcututiltest.h PRE-CREATION 
  tests/keyboardshortcututiltest.cpp PRE-CREATION 

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


Testing
---


File Attachments


New End Process button with drop down arrow
  
https://git.reviewboard.kde.org/media/uploaded/files/2015/01/28/16301e88-e21b-4358-9a63-a85dae5722bd__screenshot_default1.png
Drop down shows Kill Window
  
https://git.reviewboard.kde.org/media/uploaded/files/2015/01/28/58df12c5-7350-4bb0-b602-c5716caa9836__screenshot_default2.png


Thanks,

Gregor Mi



Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-02-21 Thread Hrvoje Senjan

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


can you check what needs to be adjusted in plasma-workspace? it fails to build 
with your change:



[  451s] 
/home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:471:43:
 error: 'proc->KSysGuard::Process::command' does not have class type
[  451s]  QString cmdline = proc ? proc->command.simplified() : QString(); 
// proc->command has a trailing space???
[  451s]^
[  451s] 
/home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:501:103:
 error: no matching function for call to 'KService::KService(, QString&, QString)'
[  451s]  services << QExplicitlySharedDataPointer(new 
KService(proc->name, cmdline, QString()));
[  451s]
^
[  451s] 
/home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:501:103:
 note: candidates are:
[  451s] In file included from /usr/include/KF5/KService/KService:1:0,
[  451s]  from 
/home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:32:
[  451s] /usr/include/KF5/KService/kservice.h:580:5: note: 
KService::KService(QDataStream&, int)
[  451s]  KService(QDataStream &str, int offset);
[  451s]  ^
[  451s] /usr/include/KF5/KService/kservice.h:580:5: note:   candidate expects 
2 arguments, 3 provided
[  451s] In file included from /usr/include/KF5/KService/KService:1:0,
[  451s]  from 
/home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:32:
[  451s] /usr/include/KF5/KService/kservice.h:82:14: note: 
KService::KService(const KDesktopFile*, const QString&)
[  451s]  explicit KService(const KDesktopFile *config, const QString 
&entryPath = QString());
[  451s]   ^
[  451s] /usr/include/KF5/KService/kservice.h:82:14: note:   candidate expects 
2 arguments, 3 provided
[  451s] /usr/include/KF5/KService/kservice.h:75:14: note: 
KService::KService(const QString&)
[  451s]  explicit KService(const QString &fullpath);
[  451s]   ^
[  451s] /usr/include/KF5/KService/kservice.h:75:14: note:   candidate expects 
1 argument, 3 provided
[  451s] /usr/include/KF5/KService/kservice.h:68:5: note: 
KService::KService(const QString&, const QString&, const QString&)
[  451s]  KService(const QString &name, const QString &exec, const QString 
&icon);
[  451s]  ^
[  451s] /usr/include/KF5/KService/kservice.h:68:5: note:   no known conversion 
for argument 1 from '' to 'const QString&'
[  451s] 
/home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:
 In function 'QUrl TaskManager::getServiceLauncherUrl(int, const QString&, 
const QStringList&)':
[  451s] 
/home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:516:43:
 error: 'proc->KSysGuard::Process::command' does not have class type
[  451s]  QString cmdline = proc ? proc->command.simplified() : QString(); 
// proc->command has a trailing space???
[  451s]^

- Hrvoje Senjan


On Feb. 20, 2015, 10:46 p.m., Gregor Mi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121831/
> ---
> 
> (Updated Feb. 20, 2015, 10:46 p.m.)
> 
> 
> Review request for KDE Base Apps, Dominik Haumann, Eike Hein, and John 
> Tapsell.
> 
> 
> Repository: libksysguard
> 
> 
> Description
> ---
> 
> This is a follow-up to https://git.reviewboard.kde.org/r/121717/:
> 
> In process.h there are several public fields which easily trigger BIC 
> changes. This RR introduces a d-ptr.
> 
> (In a separate commit I would add the .reviewboardrc file)
> 
> 
> Diffs
> -
> 
>   processui/scripting.h 2445c0ab0d81af3283c0f6e9c5f349a3d70b0de9 
>   processui/scripting.cpp 76291b0ae0a26e486aa81a4ca3976ff4a47cb3c0 
>   processcore/processes_solaris_p.cpp 
> f054df4b1e762e9cbec1ff8dea78f467b878bee0 
>   processui/ProcessFilter.cpp ec520593fb67c777d56817f2493d40dc5ade0347 
>   processui/ProcessModel.cpp 53bc041110c9cdb686fef783895104969b661889 
>   processui/ksysguardprocesslist.cpp 4dc142e864d8353ceafc3a6735ffa81e48291420 
>   processcore/processes.cpp 580df8db152040f1ad075430fdce08fe7ad4ae2d 
>   processcore/processes_atop_p.cpp 24c76e3e35f62bd8e9e705ad32cc11cbd3662601 
>   processcore/processes_base_p.h 71b8a9cc6ee14bf7934a0a9d3199b257b5ce1be7 
>   processcore/processes_linux_p.cpp 898d4fa491873fe95a8b32a5c1b85642b2e46ad5 
>   processcore/processes.h d09c3265333fe7e2702deaa910c5fbe4bc3ac9e6 
>   tests/process

Re: Review request: QBluez

2015-02-21 Thread David Rosca
> Still failing here

Oops, sorry. It's because the problem is not that you are running
Bluez 4, but because the method call is rejected (auth issue?).

Anyway, I modified the tests again so that it now checks whether
the Bluez 5 is running and is functional (it checks if the call to
GetManagedObjects succeeds). It should catch your error
and correctly skip the tests now.

David

On Fri, Feb 20, 2015 at 9:57 PM, Albert Astals Cid  wrote:
> El Dimecres, 18 de febrer de 2015, a les 11:50:01, David Rosca va escriure:
>> > If it's an expected situation, handle the situation.
>>
>> I have modified the tests (only the ones who would fail) so they will
>> be skipped if Bluez 4 is detected.
>
> Still failing here
>
> 4: Test command: /home/kdeunstable/qbluez/build/autotests/adaptertest
> 4: Test timeout computed to be: 9.99988e+06
> 4: * Start testing of AdapterTest *
> 4: Config: Using QtTest library 5.4.0, Qt 5.4.0 (x86_64-little_endian-lp64 
> shared (dynamic) release build; by GCC 4.9.2)
> 4: QWARN  : AdapterTest::initTestCase() BluezQt: GetManagerJob Error: 
> "Rejected send message, 2 matched rules; type="method_call", sender=":1.145" 
> (uid=1003 pid=10596 
> comm="/home/kdeunstable/qbluez/build/autotests/adapterte") 
> interface="org.freedesktop.DBus.ObjectManager" member="GetManagedObjects" 
> error name="(unset)" requested_reply="0" destination="org.bluez" (uid=0 
> pid=556 comm="/usr/sbin/bluetoothd ")"
> 4: FAIL!  : AdapterTest::initTestCase() '!initJob->error()' returned FALSE. ()
> 4:Loc: [/home/kdeunstable/qbluez/autotests/adaptertest.cpp(76)]
> 4: PASS   : AdapterTest::cleanupTestCase()
> 4: Totals: 1 passed, 1 failed, 0 skipped, 0 blacklisted
> 4: * Finished testing of AdapterTest *
> 4/6 Test #4: bluezqt-adaptertest ..***Failed0.01 sec
> test 5
> Start 5: bluezqt-devicetest
>
> 5: Test command: /home/kdeunstable/qbluez/build/autotests/devicetest
> 5: Test timeout computed to be: 9.99988e+06
> 5: * Start testing of DeviceTest *
> 5: Config: Using QtTest library 5.4.0, Qt 5.4.0 (x86_64-little_endian-lp64 
> shared (dynamic) release build; by GCC 4.9.2)
> 5: QWARN  : DeviceTest::initTestCase() BluezQt: GetManagerJob Error: 
> "Rejected send message, 2 matched rules; type="method_call", sender=":1.146" 
> (uid=1003 pid=10597 
> comm="/home/kdeunstable/qbluez/build/autotests/devicetes") 
> interface="org.freedesktop.DBus.ObjectManager" member="GetManagedObjects" 
> error name="(unset)" requested_reply="0" destination="org.bluez" (uid=0 
> pid=556 comm="/usr/sbin/bluetoothd ")"
> 5: FAIL!  : DeviceTest::initTestCase() '!initJob->error()' returned FALSE. ()
> 5:Loc: [/home/kdeunstable/qbluez/autotests/devicetest.cpp(96)]
> 5: PASS   : DeviceTest::cleanupTestCase()
> 5: Totals: 1 passed, 1 failed, 0 skipped, 0 blacklisted
> 5: * Finished testing of DeviceTest *
> 5/6 Test #5: bluezqt-devicetest ...***Failed0.01 sec
>
>
>>
>> I have also renamed the library to BluezQt. Here is an updated
>> documentation: http://david.rosca.cz/bluezqt-apidocs/html/
>>
>> David
>>
>> On Wed, Feb 18, 2015 at 2:19 AM, Thiago Macieira  wrote:
>> > On Tuesday 17 February 2015 23:00:15 Albert Astals Cid wrote:
>> >> > It doesn't have the DBus
>> >> > ObjectManager, so that call should fail like that.
>> >>
>> >> Well, then the test should try to detect it and QEXPECT_FAIL, havign
>> >> tests
>> >> fail means something is bad, and as you say it's not bad, just expected.
>> >
>> > QEXPECT_FAIL is when you know it's wrong but either can't solve it or
>> > can't do it now.
>> >
>> > If it's an expected situation, handle the situation.
>> > --
>> > Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
>> >
>> >Software Architect - Intel Open Source Technology Center
>> >
>> >   PGP/GPG: 0x6EF45358; fingerprint:
>> >   E067 918B B660 DBD1 105C  966C 33F5 F005 6EF4 5358
>


Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-02-21 Thread Gregor Mi

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

(Updated Feb. 20, 2015, 9:46 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Base Apps, Dominik Haumann, Eike Hein, and John Tapsell.


Repository: libksysguard


Description
---

This is a follow-up to https://git.reviewboard.kde.org/r/121717/:

In process.h there are several public fields which easily trigger BIC changes. 
This RR introduces a d-ptr.

(In a separate commit I would add the .reviewboardrc file)


Diffs
-

  processui/scripting.h 2445c0ab0d81af3283c0f6e9c5f349a3d70b0de9 
  processui/scripting.cpp 76291b0ae0a26e486aa81a4ca3976ff4a47cb3c0 
  processcore/processes_solaris_p.cpp f054df4b1e762e9cbec1ff8dea78f467b878bee0 
  processui/ProcessFilter.cpp ec520593fb67c777d56817f2493d40dc5ade0347 
  processui/ProcessModel.cpp 53bc041110c9cdb686fef783895104969b661889 
  processui/ksysguardprocesslist.cpp 4dc142e864d8353ceafc3a6735ffa81e48291420 
  processcore/processes.cpp 580df8db152040f1ad075430fdce08fe7ad4ae2d 
  processcore/processes_atop_p.cpp 24c76e3e35f62bd8e9e705ad32cc11cbd3662601 
  processcore/processes_base_p.h 71b8a9cc6ee14bf7934a0a9d3199b257b5ce1be7 
  processcore/processes_linux_p.cpp 898d4fa491873fe95a8b32a5c1b85642b2e46ad5 
  processcore/processes.h d09c3265333fe7e2702deaa910c5fbe4bc3ac9e6 
  tests/processtest.cpp f9b36e9a3a3c2048b51f1d935f8c40de2ad8a9b8 
  .reviewboardrc PRE-CREATION 
  CMakeLists.txt cefc86f12be684e195bd148641483e9e1734e636 
  processcore/process.h b6695c0ed301dc5f0fad8ba847da811f19ebfd9a 
  processcore/process.cpp a38b8be71da1a51cb87f636664ebac817b1d20ab 

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


Testing
---

Compiles and runs. Data is still shown; no visible error. Unit tests succeed.


Thanks,

Gregor Mi



Review Request 122653: Set permissions for links in remote:, necessary for correct visualization

2015-02-21 Thread Stefan Brüns

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

Review request for kdelibs.


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


Repository: kde-runtime


Description
---

KFileItem uses UDS_ACCESS to determine permissions. Readability is
subsequently used to create the list of overlay icons.

CCBUG: 339193


Diffs
-

  kioslave/remote/remoteimpl.cpp 5d973c6c1b6c31b7f3107d0d15805ef04bfdd661 

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


Testing
---

dolphin remote:
-> no lock icon on smb:, mtp:, ... links


Thanks,

Stefan Brüns



Review Request 122652: Use correct default value when UDS_ACCESS/UDS_FILE_TYPE is not set

2015-02-21 Thread Stefan Brüns

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

Review request for kdelibs.


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


Repository: kdelibs


Description
---

The default value for UDSEntry::numberValue(...) is 0, whereas KFileItem
uses the special value KFileItem::Unknown == (mode_t) -1.

CCBUG: 339193


Diffs
-

  kio/kio/kfileitem.cpp f431d3608cfe646fb882365921e694af8ff8838f 

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


Testing
---

dolphin remote:
-> no lock icon on smb:, mtp:, ... links


Thanks,

Stefan Brüns



Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-02-21 Thread Gregor Mi


> On Feb. 17, 2015, 10:22 p.m., David Edmundson wrote:
> >

Thx for looking into it. Since it is quite a large change I'll keep my 
intermediate commits and push them now.


- Gregor


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


On Feb. 15, 2015, 4:35 p.m., Gregor Mi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121831/
> ---
> 
> (Updated Feb. 15, 2015, 4:35 p.m.)
> 
> 
> Review request for KDE Base Apps, Dominik Haumann, Eike Hein, and John 
> Tapsell.
> 
> 
> Repository: libksysguard
> 
> 
> Description
> ---
> 
> This is a follow-up to https://git.reviewboard.kde.org/r/121717/:
> 
> In process.h there are several public fields which easily trigger BIC 
> changes. This RR introduces a d-ptr.
> 
> (In a separate commit I would add the .reviewboardrc file)
> 
> 
> Diffs
> -
> 
>   processui/scripting.h 2445c0ab0d81af3283c0f6e9c5f349a3d70b0de9 
>   processui/scripting.cpp 76291b0ae0a26e486aa81a4ca3976ff4a47cb3c0 
>   processcore/processes_solaris_p.cpp 
> f054df4b1e762e9cbec1ff8dea78f467b878bee0 
>   processui/ProcessFilter.cpp ec520593fb67c777d56817f2493d40dc5ade0347 
>   processui/ProcessModel.cpp 53bc041110c9cdb686fef783895104969b661889 
>   processui/ksysguardprocesslist.cpp 4dc142e864d8353ceafc3a6735ffa81e48291420 
>   processcore/processes.cpp 580df8db152040f1ad075430fdce08fe7ad4ae2d 
>   processcore/processes_atop_p.cpp 24c76e3e35f62bd8e9e705ad32cc11cbd3662601 
>   processcore/processes_base_p.h 71b8a9cc6ee14bf7934a0a9d3199b257b5ce1be7 
>   processcore/processes_linux_p.cpp 898d4fa491873fe95a8b32a5c1b85642b2e46ad5 
>   processcore/processes.h d09c3265333fe7e2702deaa910c5fbe4bc3ac9e6 
>   tests/processtest.cpp f9b36e9a3a3c2048b51f1d935f8c40de2ad8a9b8 
>   .reviewboardrc PRE-CREATION 
>   CMakeLists.txt cefc86f12be684e195bd148641483e9e1734e636 
>   processcore/process.h b6695c0ed301dc5f0fad8ba847da811f19ebfd9a 
>   processcore/process.cpp a38b8be71da1a51cb87f636664ebac817b1d20ab 
> 
> Diff: https://git.reviewboard.kde.org/r/121831/diff/
> 
> 
> Testing
> ---
> 
> Compiles and runs. Data is still shown; no visible error. Unit tests succeed.
> 
> 
> Thanks,
> 
> Gregor Mi
> 
>



Re: Review Request 122249: libksysguard: add Kill Window to End Process button and show correct keyboard shortcut

2015-02-21 Thread Gregor Mi


> On Jan. 26, 2015, 7:05 a.m., Martin Gräßlin wrote:
> > My opinion is that this is a feature which should not be exposed in 
> > libksysguard. It actually ties libksysguard to KWin, while libksysguard was 
> > in the past also used in e.g. kdevelop.
> > 
> > If libksysguard wants to offer the functionality to kill a window, it 
> > should implement it itself.
> 
> Martin Gräßlin wrote:
> In addition: KWin's global shortcut action names are not public API. We 
> do not guarantee that we don't change them, we do not guarantee that they are 
> exposed at all (KWin handling shortcuts internally without kglobalaccel on 
> Wayland?). I do not want to run into situations that we cannot change our 
> code because external usage makes it impossible.
> 
> Thomas Lübking wrote:
> In case there was larger demand for invoking such action (taskbar, 
> dedicated plasmoid, ...) one could move the xkill functionality into 
> KWindowSystem (option for portage) - invoking a kwin shortcut through a 
> kglobalaccel dbus call is a hack. Maybe sufficient for any downstream 
> solution, but easily broken feature.
> 
> Gregor Mi wrote:
> First of all, a clarification of this RR's intentions:
> 1) The original "End Process..." tooltip says "you can always use 
> Ctrl+Alt+Esc..." which is wrong as soon as someone changes the keyboard 
> shortcut exposed by KWin. So this should be fixed.
> 2) Make the Kill Window feature more discoverable. It is a seldom used 
> feature which makes it harder to remember.
> 
> About invoking Kill Window:
> > If libksysguard wants to offer the functionality to kill a window, it 
> should implement it itself. [Martin]
> > ...one could move the xkill functionality into KWindowSystem...  
> [Thomas]
> 
> Without knowing the amount of xkill code I suspect that having a dbus 
> call that loosly couples libksysguard to KWin is probably easier to maintain 
> than 2 times the xkill code.
> Having said that, what about moving the xkill code to a common location 
> as Thomas suggested?
> 
> > We do not guarantee that we don't change them, we do not guarantee that 
> they are exposed at all ... I do not want to run into situations that we 
> cannot change our code because external usage makes it impossible.
> 
> Understood. But would it then be possible at all to get the current 
> shortcut to display it to the user?
> 
> Martin Gräßlin wrote:
> Ok, so this addresses two issues using one solution: exposing KWin's 
> internal shortcut. This is bad as outlined above.
> 
> I agree that 1) needs fixing. This can be done in the way as approached 
> in this review request: check whether kwin is registered on kglobalaccel and 
> get the key command. If it's done that way the fault is with libksysguard in 
> case KWin changes the shortcut name or doesn't use kglobalaccel any more. 
> Another fix is of course to just hide the shortcut.
> 
> 2) is a different issue. Whether it's needed to expose the functionality 
> in addition from libksysguard is probably questionable. A better approach to 
> do this would be through a method in KWindowSystem. This does not need to 
> duplicate the code, it could also just send a client message to the window 
> manager to start the kill window process. Through KWindowSystem we can check 
> whether the feature is supported by the window manager and could exclude if 
> not supported. But and that's a big but: the feature would not be able to 
> work if it's triggered from a (context) menu or drop down list (it needs to 
> grab mouse). Given that I'm hesistant to say that it should be added to 
> kwindowsystem at all.
> 
> Thomas Lübking wrote:
> ad 2)
> I'd have said to rather *move* the code to KWindowSystem and use it from 
> there by any client (incl. kwin)
> This allows porting the solution (in case such is possible on other 
> systems at all) as well as to invoke the feature unconditionally (ie. instead 
> of "is this kwin?  yes? tell kwin to trigger xkill." just trigger the xkill 
> functionality)
> 
> About the popupmenu:
> The issue is global, ie. as long as a popup (or other grabber) is 
> around, the kwin shortcut neither works.
> It's kind of the client codes problem to deal w/ a "false" return (eg. 
> invoke a timer and/or timered retries)
> 
> Gregor Mi wrote:
> ad 1) (shortcut)
> I could live with adapting (or remove) the shortcut retrieval as soon as 
> it will not be possible anymore. As long as it is, I would show it. (I 
> suspect as long as the shortcut is not hard-coded there will be a some way to 
> get it)
> 
> 
> ad 2) (invoke window kill)
> I looked a Kwin's source code. For reference, here are the two methods I 
> found to kill a window:
> ```
> /*!
>   Kill Window feature, similar to xkill
>  */
> void Workspace::slotKillWindow()
> {
> if (m_windowKiller.isNull()) {
> m_windowKiller.

Re: plasma-mediacenter in kdereview from extragear

2015-02-21 Thread Bhushan Shah
Hello,

On Fri, Feb 20, 2015 at 5:06 PM, Jonathan Riddell  wrote:
> - mockcpp pointer to obscure repository cmake message
> - qtmultimedia needs a "this is needed at runtime" cmake message

Fixed in 
http://commits.kde.org/plasma-mediacenter/b17c88d0a7cc6499294e872f5217012e21ba3ac9

> - music and video icon are black on black

Discussed with Ken, Probably will need artwork or way to enforce icon theme

> - crashes, at times, probably a baloo issue, will discuss with vishesh
> - doesn't show any music or videos for me, probably a baloo issue

I will add some more debug messages for this, so maybe we can find out
whats going on wrong.

> - needs git plasma-workspace for login to work, cmake should tell me this

current plasma-workspace git version will be shipped with plasma 5.3
so I think this is fine when it will be released, also commit that
introduced this dep already mentions the required plasma-workspace
commit

commit 451eceffaf4a1c06e162dc629bb5fc945e40ae73
Author: Bhushan Shah 
Date:   Tue Feb 17 22:50:08 2015 +0530

   Install session file for pmc

   For that session file to work it will require recent plasma-workspace
   with commit 77112d402e3c22311dd2d70f9d2b264d1465cf03

> - it uses qml bindings from qtmultimedia (from qt5) which uses gstreamer 
> 0.10.  gstreamer 0.10 is old and distros are dropping support for it, in 
> Ubuntu we dropped the libav plugin so it can't play many file formats.  maybe 
> one day phonon will have qml bindings.  it also leads to issues where some 
> other part of Plasma uses phonon which uses gstreamer 1.0 so it loads 
> libraries with clashing symbols.  this is all known and is mostly sad.

I can not do anything for it.. :-(

> - there's no user docs, I don't know if this is still a requirement for 
> inclusion, I don't think they get used much so I don't think it should be

We can add the user documents later on..
http://developer.kde.org/~cfeck/portingstatus.html also mentions about
that.

Thanks for your review

-- 
Bhushan Shah

http://bhush9.github.io
IRC Nick : bshah on Freenode