Bug 356161 - [RFC] Should the minimize all script favor minimizing?
Summary: [RFC] Should the minimize all script favor minimizing?
Status: RESOLVED FIXED
Alias: None
Product: kwin
Classification: Plasma
Component: general (show other bugs)
Version: git master
Platform: Other Linux
: NOR wishlist (vote)
Target Milestone: ---
Assignee: KWin default assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-12-01 15:19 UTC by Thomas Lübking
Modified: 2019-09-18 15:24 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.17.0


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Lübking 2015-12-01 15:19:35 UTC
Presently, the script will rather unminimize what it has minimized as long as such minimized window is around, ignoring changes on screen.

Given the approach seems to be used to "unclutter" the desktop, it sounds reasonable that users wil
- spam windows, work with them
- minimize all to unclutter the desktop
- spam windows, work with them
- minimize all to unclutter the desktop
at which point "unclutter" won't happen, but the windows minimized in the first cycle will be restored.

The question is whether
1. we should not ignore the interim changes and put the emphasis on *minimize* all, ie. minimize as long as there's something to minimize.

A follow-up question is whether
2. the restoring call (if that actually happens...) should restore all windows that are minimized because of the script or just those in the last cycle.

The pros would be that this is claimed to be the status ante w/ the secret ShowDesktopIsMinimizeAll key and may support the "unclutter" usecase.

The con would be "yet another behavioral change" and so far only one user complained.
Since we'll have to wait for 5.6 anyway (behavioral), we could introduce an option.
The alternative to the option is to pick one habit for the kwin stock and ship the rest via GHNS.

Thoughts?

Reproducible: Always
Comment 1 Martin Flöser 2015-12-01 15:25:11 UTC
what about never restoring? Just having an easy way to minimize all windows?
Comment 2 Martin Flöser 2015-12-02 09:17:54 UTC
@Thomas P: what's your opinion.
Comment 3 Thomas Lübking 2015-12-02 12:28:59 UTC
Though I'm the wrong Thomas, that would pretty much equal "prefer minimizing - minus loss of reasonable action when there's nothing to minimize, because we don't know what's reasonable here ;-)", would it not?
Comment 4 Martin Flöser 2015-12-02 12:37:29 UTC
yeah. My main problem with this is that I don't understand what users actually want. Which might even be impossible to know.
Comment 5 Thomas Pfeiffer 2015-12-02 14:47:48 UTC
Thank you for getting me involved.

Guessing what an individual user wants can only be helpful if we know for a fact what the majority wants, and infer from this data that with a high probability, a given user X wants that. 
Since we have zero data on what the majority of users want with this script, we should not guess.

Therefore, the script should do what it says on the tin: Minimize all, and nothing else. Nobody has the right to complain if a script doesn't do something which it never claims it does.

So, as long as the script is called "Minimize all windows", it should do nothing but that. If we think it might be useful o have the same script restore all windows if it's invoked immediately again, it should be called "Minimize/restore all".

A script which minimizes windows that are not minimized, but restores minimized windows at the same time (which te script does now, if I understood Thomas' description correctly) does not sound very useful to me in the first place. What would be the usecase for that?
Comment 6 Thomas Lübking 2015-12-02 15:00:11 UTC
(In reply to Thomas Pfeiffer from comment #5)
> What would be the usecase for that?
Bug #67406 :-P

Basically, a bunch of users didn't like how ShowDesktop behaved (when some window was brought to screen, all windows restored, because "show desktop" is meant modal - it's not a way to unclutter the desktop) and Lubos added a secret key to make it behave like that (to spare him endless discussions ;-)

When we altered the "show desktop" implementation to raise the desktop rather than to unmap ("minimize") all windows, the script was added as compensation, since the key could obviously no longer be used (while unmapping windows, this was merely a check on "what do I do if a window appears", but now it's an entirely different process)

The name "minimize all" stems from there; the legacy behavior seems to have been:
If there is something to minimize, do that, otherwise restore what was minimized on last invocation.
The script presently strictly toggles (ie. as long as there's something been minimized by the script, it will unminimize all those windows rather than minimizing new ones) - mostly because I implemented it "free handed".

> A script which minimizes windows that are not minimized, but restores minimized windows at the same time
This does not happen. It either minimizes XOR restores everything. The quarrel is on "when do we do what" - we do not flip minimization (for that would *really* just be virtual desktops ;-)
Comment 7 Thomas Lübking 2015-12-02 15:02:08 UTC
PS: I'm fine with throwing random modifications at the users and shipping all alternatives via GHNS.
If there had not been this <*censored*> <*censored*> <*really?? you kiss your mom with that mouth? - censored!*> config key, the script would not be part of the KWin installation anyway.
Comment 8 Thomas Pfeiffer 2015-12-02 15:27:17 UTC
Aaaah, okay, now I understand what it does atm, I had missed the "what it has minimized" part.

I agree with you, then: It should only un-minimize anything if it is activated again immediately without any further minimizing or unminimizing happening in-between. The main function is minimizing (as the name suggests), the un-minimize function serves only as an "undo" function.
Comment 9 Thomas Lübking 2015-12-02 16:17:33 UTC
(In reply to Thomas Pfeiffer from comment #8)
> serves only as an "undo" function.
"Undo" would support to only restore the windows minimized in the last positive invocation (and ignore all windows formerly minimized by the script)?
Comment 10 Thomas Pfeiffer 2015-12-02 18:04:10 UTC
(In reply to Thomas Lübking from comment #9)
> "Undo" would support to only restore the windows minimized in the last
> positive invocation (and ignore all windows formerly minimized by the
> script)?

That's what I'd expect, yes. But honestly, it would just be a bonus. the only thing that the script needs to accomplish is minimizing all windows. If one can reverse the action by executing it again, that's nice, but it cannot be expected.
Comment 11 Thomas Lübking 2015-12-08 20:09:39 UTC
Git commit 8885a6798a348cc709a3423f37593cc3f6ce6eee by Thomas Lübking.
Committed on 08/12/2015 at 20:08.
Pushed by luebking into branch 'master'.

emphasize minimization in m. all script

As long as there's something to minimize, do that.
Otherwise unminimize only the windows we minimized
on last invocation.
FIXED-IN: 5.6
REVIEW: 126225

M  +7    -4    scripts/minimizeall/contents/code/main.js

http://commits.kde.org/kwin/8885a6798a348cc709a3423f37593cc3f6ce6eee
Comment 12 Anton 2019-09-18 07:25:40 UTC
(In reply to Thomas Lübking from comment #11)
> emphasize minimization in m. all script
> 
> As long as there's something to minimize, do that.
> Otherwise unminimize only the windows we minimized
> on last invocation.

Sorry for my english.

Current code does not fulfill these duties. Try next case: run minimizeall on some windows, then click on one of windows, then run minimizeall again. All windows would be unminimized!

I have tried to make small changes to original code of main.js, but i can't because i don't understand it.

Thats why i wrote my own version.

Sorry for my english again.

var minimizeAllWindows = function() {
    var allClients = workspace.clientList();
    var clients = [];
    var minimize = false;
    for (var i = 0; i < allClients.length; ++i) {
        if (isRelevant(allClients[i])) {
            clients.push(allClients[i]);
            if (!allClients[i].minimized) {
                minimize = true;
            }
        }
    }
    if (minimize == true) {
        for (var i = 0; i < clients.length; ++i) {
            delete clients[i].minimizedForMinimizeAll;
            if (clients[i].minimized == true) {
                continue;
            }
            else {
                clients[i].minimized = true;
                clients[i].minimizedForMinimizeAll = true;
            }
       }
    }
    else {
        for (var i = 0; i < clients.length; ++i) {
            if (clients[i].minimizedForMinimizeAll == true) {
                clients[i].minimized = false;
            }
        }

    }
    clients = [];
}
Comment 13 Vlad Zahorodnii 2019-09-18 08:03:25 UTC
@Anton Would you like to submit a patch?

https://community.kde.org/Get_Involved/development#Submit_a_patch
Comment 14 Anton 2019-09-18 08:38:33 UTC
(In reply to Vlad Zahorodnii from comment #13)
> @Anton Would you like to submit a patch?
After your suggestion i made it.
https://phabricator.kde.org/D24044
Comment 15 Vlad Zahorodnii 2019-09-18 15:12:11 UTC
Git commit fec0ab818d2460d2b2dd48e698eb2d48a2f1f0ab by Vlad Zahorodnii, on behalf of Anton Smerkov.
Committed on 18/09/2019 at 15:11.
Pushed by vladz into branch 'master'.

Emphasize minimization in MinimizeAll script

Summary:
MinimizeAll should work as follows:

> As long as there's something to minimize, do that.
> Otherwise unminimize only the windows we minimized
> on last invocation.

Quote above is from Comment 11 by Thomas Lübking of BUG: 356161.

But current code does not fulfill these duties. Try next case: run minimizeall on some windows, then click on one of windows, then run minimizeall again. All windows would be unminimized!

I have tried to make small changes to original code of main.js, but i can't because i don't understand it.

Thats why i wrote my own version. Then i changed my version to version of zzag, because his code is more simple. I have tested both versions.

Reviewers: colomar, #kwin, zzag

Reviewed By: #kwin, zzag

Subscribers: romangg, zzag, kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D24044

M  +25   -15   scripts/minimizeall/contents/code/main.js

https://commits.kde.org/kwin/fec0ab818d2460d2b2dd48e698eb2d48a2f1f0ab