Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix and improve BAR_WINTITLEACTIONS_PATCH #442

Merged
merged 4 commits into from
Mar 2, 2025

Conversation

iokanto
Copy link
Contributor

@iokanto iokanto commented Feb 27, 2025

The first thing that corrects this PR is the loss of hidden clients after rebooting with a RESTARTSIG_PATCH.

Second, this is the addition of the opportunity to show all the hidden clients using a combination of keys, which is sometimes useful.

@bakkeby
Copy link
Owner

bakkeby commented Feb 27, 2025

You are trying to add two independent changes with one pull request here.

It is in general better to raise two separate pull requests because that way one can discuss each change independently and the one change does not depend approval of the other to be merged.

I'm sure you'll see what I mean.

Change 1

In the one commit we introduce a showall function that can be used to bring all hidden clients back into view.

I am not entirely sure if the if (!selmon->sel) case is a realistic scenario, especially since the show function will apply an arrange if a hidden window is revealed. An arrange would also apply a restack in this case.

I'm not against adding such a feature / function. I'd probably not call the show function here, but instead do the mapping and changing of client state in a for loop so that we only apply the arrange once at the end. I'd also be more inclined to call such a function unhideall because showall may be ambiguous or less clear when it comes to what it does.

Change 2

The second change aims to address an issue with hidden windows and restarting dwm.

The approach is to loop through and unhide every hidden client in the quit function. This is similar to how a bare dwm brings all the clients into view on exit.

Besides that the show function is going to trigger an arrange for every call it is also not necessary.

When dwm starts it is going to make a call to scan which makes a call to the X server asking what windows exists.

While looping through the windows that are available it is going to also pick up and manage windows that are hidden.

            if (wa.map_state == IsViewable || getstate(wins[i]) == IconicState)
                manage(wins[i], &wa);

If there is something wrong then that would more likely be with how the window is handled in the manage function.

In the manage function we just don't map the window and change the state if the window is hidden. The expectation being that a hidden window remains hidden on restart.

Digging a bit further I find that the windows are not actually passed from scan to manage in this case because the windows are unampped but no longer in an iconic state, and the reason for that is that as part of the restart all windows are being unmanaged, and the unmanage function, if the client window is not destroyed, will implicitly change the client state to WithdrawnState.

I am not entirely sure why dwm does this. Maybe it is to indicate that the window manager is no longer managing the window. That

One possible solution could be to get rid of the WithdrawnState altogether or not set it if the client is hidden (i.e. in iconic state).

diff --git a/dwm.c b/dwm.c
index 893bd74..c0b332a 100644
--- a/dwm.c
+++ b/dwm.c
@@ -4572,7 +4572,8 @@ unmanage(Client *c, int destroyed)
                XSelectInput(dpy, c->win, NoEventMask);
                XConfigureWindow(dpy, c->win, CWBorderWidth, &wc); /* restore border */
                XUngrabButton(dpy, AnyButton, AnyModifier, c->win);
-               setclientstate(c, WithdrawnState);
+               if (!HIDDEN(c))
+                       setclientstate(c, WithdrawnState);
                XSync(dpy, False);
                XSetErrorHandler(xerror);
                XUngrabServer(dpy);

@iokanto
Copy link
Contributor Author

iokanto commented Mar 1, 2025

That's right, thanks for the advice, I'll be more attentive in the future when publishing PR.

Change 1

Indeed, showall used to do a lot of unnecessary actions, but with your advice, it began to work somewhat more efficiently.

Change 2

I made the changes as you described, and it really works better! Now, when restarting, the hidden windows do not disappear and do not become visible.

@bakkeby
Copy link
Owner

bakkeby commented Mar 1, 2025

I think this looks good.

The s keybinding is quite popular, but MOD+Ctrl+s does not conflict with any other keybinding so it is fine. An alternative default could be MOD+Ctrl+Shift+z considering that MOD+Ctrl+z is used for showhideclient. Might be cognitively easier to remember when related functionality is under the same key.

Just a suggestion. For me this is good to merge, but I'll leave it open for a day more in case someone else have any remarks.

@bakkeby bakkeby merged commit fd6db40 into bakkeby:master Mar 2, 2025
@iokanto iokanto deleted the winactions-improved branch March 3, 2025 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants