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

incnmaster(const Arg *arg) allows to increase layout indefinitely #311

Open
766F6964 opened this issue Nov 24, 2022 · 3 comments
Open

incnmaster(const Arg *arg) allows to increase layout indefinitely #311

766F6964 opened this issue Nov 24, 2022 · 3 comments

Comments

@766F6964
Copy link

766F6964 commented Nov 24, 2022

By default, the key combination MODKEY + XK_I or MODKEY + XK_D allow modification of the stack.
We can increase and either increase the amount of windows on the stack or decrease it.

However, it seems there is no upper limit when modifying the stack. In other words, when we keep increasing the amount, even when all windows are already on the stack, we also have to press multiple times to see any change when decreasing it. I am unsure if this is a bug or just intuitive behavior. Especially since the same behavior doesn't happen when decreasing the amount. It is is capped at 0.

This video might showcase the problem a bit better:

stack_modification.mp4

There are 6 windows in total. When pressing MODKEY + XK_I we can see that the windows get moved from the stack to the master. After pressing 5x, all 6 windows are on the left. This should be the max. There shouldn't be any further increase possible. However, we can keep pressing MODKEY + XK_I (without seeing any visual change).

However, when we press MODKEY + XK_D to decrease, we have to press as many times as we "overflowed" before windows start moving again. The same behavior does not exist when going in the other direction.

@bakkeby
Copy link
Owner

bakkeby commented Nov 25, 2022

I am unsure if this is a bug or just intuitive behavior.

This is indeed a genuine dwm experience.

The lower cap of 0 is intentional as a negative number does not make sense in the context, and layout functions often check that m->nmaster is not 0. The variable is also intentionally an int as to allow for this cap.

The no upper limit is surely to make the code base simpler, but also the user is free to increment the value as much as they desire.

There is a patch called nmaxmaster that sets an upper limit on how many clients can be placed in the master area.

For general use I would say that one would never really need to increase nmaster beyond + 1 client. That said this behaviour can be quite annoying when testing layouts and other things.

After pressing 5x, all 6 windows are on the left. This should be the max. There shouldn't be any further increase possible.

Your proposed solution to this issue (or behaviour if you wish) is to put a cap on nmaster and that this cap is relative to how many tiled clients are currently visible. I think that this solution could work very well, especially when using the pertag patch.

You would be in the same boat though if you close windows or if you view multiple (or all) tags and increase nmaster to the max and then move back to a single tag.

Another approach could be to allow nmaster to have no upper limit, but when decrementing the value you would use the minimum of nmaster vs number of tiled clients as the base for the new number.

@766F6964
Copy link
Author

766F6964 commented Nov 25, 2022

Your proposed solution to this issue (or behaviour if you wish) is to put a cap on nmaster and that this cap is relative to how many tiled clients are currently visible. I think that this solution could work very well, especially when using the pertag patch.

This seems like the correct way to solve this to me.

You would be in the same boat though if you close windows or if you view multiple (or all) tags and increase nmaster to the max and then move back to a single tag.

I think it would make sense if the nmaster is specific to each tag. When viewing multiple (or all) tags, and increasing nmaster to the max, logicially we would have different max values for each tag. Hence when increasing nmaster while multiple (or all) tags are visible, an increase should happen on multiple (or all) tags, but only up to their individual max.

Another approach could be to allow nmaster to have no upper limit, but when decrementing the value you would use the minimum of nmaster vs number of tiled clients as the base for the new number.

That could work, but it seems more like a hack to me, than a proper solution. It might work fine, but I don't know if this could clash with other patches etc.

@bakkeby
Copy link
Owner

bakkeby commented Nov 26, 2022

To my knowledge the only patches that affect the incnmaster function are pertag and nmaxmaster, so that shouldn't be a problem I'd think.

Example diff for a bare 6.4 dwm.

diff --git a/dwm.c b/dwm.c
index e5efb6a..b776306 100644
--- a/dwm.c
+++ b/dwm.c
@@ -971,7 +971,10 @@ grabkeys(void)
 void
 incnmaster(const Arg *arg)
 {
-       selmon->nmaster = MAX(selmon->nmaster + arg->i, 0);
+       Client *c;
+       int n;
+       for (n = 0, c = nexttiled(selmon->clients); c; c = nexttiled(c->next), n++);
+       selmon->nmaster = MAX(MIN(selmon->nmaster,n) + arg->i, 0);
        arrange(selmon);
 }

The above feels pretty intuitive to me. The user is still allowed to increase nmaster one above the number of tiled clients visible, or to decrement nmaster to 0 when there are no tiled clients. I think those edge cases are not significant, but if one were adamant about it then one could add a return if n is 0.

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

No branches or pull requests

2 participants