Re: Review Request 128362: Add unclutter/cascade window actions

[Eike Hein]

> On July 5, 2016, 2:32 p.m., Martin Gräßlin wrote:
> > I do not understand why this review was discarded. Care to elaborate?
> 
> Eike Hein wrote:
> Based on IRC discussion neither you or I are are available to write the 
> favored implementation so I decided to abandon the proposed code.
> 
> Martin Gräßlin wrote:
> But then the stringly based might be better. I don't see why you 
> discarded the review.

I think your suggestion probably makes sense (considering the reason the 
actions broke seems to be that they used "org.kde.kwin", which I fixed up to 
"org.kde.KWin" ...), but I don't know how to do it so the time/effort required 
outweighed the need I see for the code.

I think Unclutter and Cascade are probably not meaningfully useful anymore in 
an era of things like the Present Windows effect or Alt+Tab with thumbnails or 
the majority of users who prefer maximized windows now:

1. Unclutter: This is probably mostly to un-obscure window contents, but 
Present Windows does this much better, without rearranging windows in an 
essentially unpredictable manner. Plus the implementation isn't very nice - it 
ends up actually moving the panel popup dialog as well.

2. Cascade: I think this is to arrange windows to have the /title bar/ 
unobscured so they can be subsequently handled more easily on an individual 
basis. I'm not sure how many users actually do this anymore, though. Even if 
they do, Present Windows (which is on a hot corner by default) offers a 
convenient way to get a handle of an obscured window.

I was OK with fixing/keeping the actions given minimal required effort, but it 
enters a territory of diminished returns very quickly given the above. And the 
applet might well be better for it: Why should there be buttons just in this 
one applet for Unclutter and Cascade? Why not Present Windows or Show Desktop 
or Minimize All? Or even actions that operate on the selected item? Basically 
the inclusion of them seems somewhat arbitrary, and just to continue a legacy. 
Which already got interrupted when they were broken for many releases without 
leading to any bug reports.


- Eike


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


On July 5, 2016, 2:13 p.m., Eike Hein wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128362/
> ---
> 
> (Updated July 5, 2016, 2:13 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: kdeclarative
> 
> 
> Description
> ---
> 
> This is the remaining useful code from the Tasks engine, which is now 
> scheduled for removal. It will be used by the Window List plasmoid.
> 
> As discussed on IRC.
> 
> 
> Diffs
> -
> 
>   src/qmlcontrols/kwindowsystemplugin/CMakeLists.txt ce0ea74 
>   src/qmlcontrols/kwindowsystemplugin/kwindowsystemproxy.h a9db965 
>   src/qmlcontrols/kwindowsystemplugin/kwindowsystemproxy.cpp 4850011 
> 
> Diff: https://git.reviewboard.kde.org/r/128362/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Eike Hein
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128362: Add unclutter/cascade window actions

[Martin Gräßlin]

> On July 5, 2016, 4:32 p.m., Martin Gräßlin wrote:
> > I do not understand why this review was discarded. Care to elaborate?
> 
> Eike Hein wrote:
> Based on IRC discussion neither you or I are are available to write the 
> favored implementation so I decided to abandon the proposed code.

But then the stringly based might be better. I don't see why you discarded the 
review.


- Martin


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


On July 5, 2016, 4:13 p.m., Eike Hein wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128362/
> ---
> 
> (Updated July 5, 2016, 4:13 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: kdeclarative
> 
> 
> Description
> ---
> 
> This is the remaining useful code from the Tasks engine, which is now 
> scheduled for removal. It will be used by the Window List plasmoid.
> 
> As discussed on IRC.
> 
> 
> Diffs
> -
> 
>   src/qmlcontrols/kwindowsystemplugin/CMakeLists.txt ce0ea74 
>   src/qmlcontrols/kwindowsystemplugin/kwindowsystemproxy.h a9db965 
>   src/qmlcontrols/kwindowsystemplugin/kwindowsystemproxy.cpp 4850011 
> 
> Diff: https://git.reviewboard.kde.org/r/128362/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Eike Hein
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128362: Add unclutter/cascade window actions

[Eike Hein]

> On July 5, 2016, 2:32 p.m., Martin Gräßlin wrote:
> > I do not understand why this review was discarded. Care to elaborate?

Based on IRC discussion neither you or I are are available to write the favored 
implementation so I decided to abandon the proposed code.


- Eike


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


On July 5, 2016, 2:13 p.m., Eike Hein wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128362/
> ---
> 
> (Updated July 5, 2016, 2:13 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: kdeclarative
> 
> 
> Description
> ---
> 
> This is the remaining useful code from the Tasks engine, which is now 
> scheduled for removal. It will be used by the Window List plasmoid.
> 
> As discussed on IRC.
> 
> 
> Diffs
> -
> 
>   src/qmlcontrols/kwindowsystemplugin/CMakeLists.txt ce0ea74 
>   src/qmlcontrols/kwindowsystemplugin/kwindowsystemproxy.h a9db965 
>   src/qmlcontrols/kwindowsystemplugin/kwindowsystemproxy.cpp 4850011 
> 
> Diff: https://git.reviewboard.kde.org/r/128362/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Eike Hein
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128362: Add unclutter/cascade window actions

[Martin Gräßlin]

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



I do not understand why this review was discarded. Care to elaborate?

- Martin Gräßlin


On July 5, 2016, 4:13 p.m., Eike Hein wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128362/
> ---
> 
> (Updated July 5, 2016, 4:13 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: kdeclarative
> 
> 
> Description
> ---
> 
> This is the remaining useful code from the Tasks engine, which is now 
> scheduled for removal. It will be used by the Window List plasmoid.
> 
> As discussed on IRC.
> 
> 
> Diffs
> -
> 
>   src/qmlcontrols/kwindowsystemplugin/CMakeLists.txt ce0ea74 
>   src/qmlcontrols/kwindowsystemplugin/kwindowsystemproxy.h a9db965 
>   src/qmlcontrols/kwindowsystemplugin/kwindowsystemproxy.cpp 4850011 
> 
> Diff: https://git.reviewboard.kde.org/r/128362/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Eike Hein
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128362: Add unclutter/cascade window actions

[Martin Gräßlin]

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



as we have a dbus xml description for KWin, I suggest to copy the xml 
description into the src and use the generation. The advantage is that we don't 
have it bound.

- Martin Gräßlin


On July 5, 2016, 3 p.m., Eike Hein wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128362/
> ---
> 
> (Updated July 5, 2016, 3 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: kdeclarative
> 
> 
> Description
> ---
> 
> This is the remaining useful code from the Tasks engine, which is now 
> scheduled for removal. It will be used by the Window List plasmoid.
> 
> As discussed on IRC.
> 
> 
> Diffs
> -
> 
>   src/qmlcontrols/kwindowsystemplugin/CMakeLists.txt ce0ea74 
>   src/qmlcontrols/kwindowsystemplugin/kwindowsystemproxy.h a9db965 
>   src/qmlcontrols/kwindowsystemplugin/kwindowsystemproxy.cpp 4850011 
> 
> Diff: https://git.reviewboard.kde.org/r/128362/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Eike Hein
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128362: Add unclutter/cascade window actions

[Eike Hein]

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

(Updated July 5, 2016, 1 p.m.)


Review request for Plasma.


Changes
---

Add version information.


Repository: kdeclarative


Description
---

This is the remaining useful code from the Tasks engine, which is now scheduled 
for removal. It will be used by the Window List plasmoid.

As discussed on IRC.


Diffs (updated)
-

  src/qmlcontrols/kwindowsystemplugin/CMakeLists.txt ce0ea74 
  src/qmlcontrols/kwindowsystemplugin/kwindowsystemproxy.h a9db965 
  src/qmlcontrols/kwindowsystemplugin/kwindowsystemproxy.cpp 4850011 

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


Testing
---


Thanks,

Eike Hein

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128362: Add unclutter/cascade window actions

[Martin Gräßlin]

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



I don't like the hard coding of the dbus service name. But I'm also lacking 
ideas how we could not hard code it. On X11 we could use the root-window 
property ORG_KDE_DBUS_SERVICE_NAME, but for Wayland we have no way to figure 
out the name.


src/qmlcontrols/kwindowsystemplugin/kwindowsystemproxy.h (line 156)


@since 5.25



src/qmlcontrols/kwindowsystemplugin/kwindowsystemproxy.h (line 163)


@since 5.25


- Martin Gräßlin


On July 4, 2016, 8:38 p.m., Eike Hein wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128362/
> ---
> 
> (Updated July 4, 2016, 8:38 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: kdeclarative
> 
> 
> Description
> ---
> 
> This is the remaining useful code from the Tasks engine, which is now 
> scheduled for removal. It will be used by the Window List plasmoid.
> 
> As discussed on IRC.
> 
> 
> Diffs
> -
> 
>   src/qmlcontrols/kwindowsystemplugin/CMakeLists.txt ce0ea74 
>   src/qmlcontrols/kwindowsystemplugin/kwindowsystemproxy.h a9db965 
>   src/qmlcontrols/kwindowsystemplugin/kwindowsystemproxy.cpp 4850011 
> 
> Diff: https://git.reviewboard.kde.org/r/128362/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Eike Hein
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128362: Add unclutter/cascade window actions

[Eike Hein]

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

(Updated July 4, 2016, 6:38 p.m.)


Review request for Plasma.


Changes
---

* Implement Kai's code improvement suggestion
* Use the KWin method names, don't invent new names


Repository: kdeclarative


Description
---

This is the remaining useful code from the Tasks engine, which is now scheduled 
for removal. It will be used by the Window List plasmoid.

As discussed on IRC.


Diffs (updated)
-

  src/qmlcontrols/kwindowsystemplugin/CMakeLists.txt ce0ea74 
  src/qmlcontrols/kwindowsystemplugin/kwindowsystemproxy.h a9db965 
  src/qmlcontrols/kwindowsystemplugin/kwindowsystemproxy.cpp 4850011 

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


Testing
---


Thanks,

Eike Hein

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128362: Add unclutter/cascade window actions

[Kai Uwe Broulik]

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



Makes sense to put it there.


src/qmlcontrols/kwindowsystemplugin/kwindowsystemproxy.cpp (line 157)


This looks like it leaks.


QDBusConnection::sessionBus().asyncCall(QDBusMessage::createMethodCall(...));


- Kai Uwe Broulik


On Juli 4, 2016, 6:21 nachm., Eike Hein wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128362/
> ---
> 
> (Updated Juli 4, 2016, 6:21 nachm.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: kdeclarative
> 
> 
> Description
> ---
> 
> This is the remaining useful code from the Tasks engine, which is now 
> scheduled for removal. It will be used by the Window List plasmoid.
> 
> As discussed on IRC.
> 
> 
> Diffs
> -
> 
>   src/qmlcontrols/kwindowsystemplugin/CMakeLists.txt ce0ea74 
>   src/qmlcontrols/kwindowsystemplugin/kwindowsystemproxy.h a9db965 
>   src/qmlcontrols/kwindowsystemplugin/kwindowsystemproxy.cpp 4850011 
> 
> Diff: https://git.reviewboard.kde.org/r/128362/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Eike Hein
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel