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

Odd behaviour with shiftviewclients and scratchpads patch #12

Closed
paniash opened this issue Oct 27, 2020 · 22 comments
Closed

Odd behaviour with shiftviewclients and scratchpads patch #12

paniash opened this issue Oct 27, 2020 · 22 comments

Comments

@paniash
Copy link

paniash commented Oct 27, 2020

On my current build, I'm using the scratchpads patch. When patched along with shiftviewclients and cycling through non-empty tags, the hidden scratchpads appear on the screen. I don't suppose this is the intended behaviour.

@bakkeby
Copy link
Owner

bakkeby commented Oct 27, 2020

That's because the two patches are not compatible with each other out of the box.

The shiftviewclient patch comes with integration hints though:
https://github.com/bakkeby/patches/blob/master/dwm/dwm-shiftviewclients-6.2.diff#L49-L52

You can just delete the code you don't need (lines 49, 52, 53, 54).

@paniash
Copy link
Author

paniash commented Oct 27, 2020

Thanks a lot! Didn't realize the integration hints were for this incompatibility :P

@paniash paniash closed this as completed Oct 27, 2020
@paniash paniash reopened this Oct 28, 2020
@paniash
Copy link
Author

paniash commented Oct 28, 2020

For some reason, the scratchpads still do appear when cycling through tags. Not sure why :( . This is my current shiftviewclients function:

void
shiftviewclients(const Arg *arg)
{
	Arg shifted;
	Client *c;
	unsigned int tagmask = 0;

	for (c = selmon->clients; c; c = c->next)
		if (!(c->tags & SPTAGMASK))
			tagmask = tagmask | c->tags;

	shifted.ui = selmon->tagset[selmon->seltags];
	if (arg->i > 0) // left circular shift
		do {
			shifted.ui = (shifted.ui << arg->i)
			   | (shifted.ui >> (LENGTH(tags) - arg->i));
		} while (tagmask && !(shifted.ui & tagmask));
	else // right circular shift
		do {
			shifted.ui = (shifted.ui >> (- arg->i)
			   | shifted.ui << (LENGTH(tags) + arg->i));
		} while (tagmask && !(shifted.ui & tagmask));

	view(&shifted);
}

@bakkeby
Copy link
Owner

bakkeby commented Oct 28, 2020

I see, there was a second integration hint that is missing from the patch:
https://github.com/bakkeby/dwm-flexipatch/blob/master/patch/shiftviewclients.c#L19-L23

I'll update it.

@paniash
Copy link
Author

paniash commented Oct 28, 2020

As per your latest commit, I updated shiftviewclients in my dwm.c as follows:

void
shiftviewclients(const Arg *arg)
{
	Arg shifted;
	Client *c;
	unsigned int tagmask = 0;

	for (c = selmon->clients; c; c = c->next)
		if (!(c->tags & SPTAGMASK))
			tagmask = tagmask | c->tags;

	shifted.ui = selmon->tagset[selmon->seltags] & ~SPTAGMASK;
	if (arg->i > 0) // left circular shift
		do {
			shifted.ui = (shifted.ui << arg->i)
			   | (shifted.ui >> (LENGTH(tags) - arg->i));
		} while (tagmask && !(shifted.ui & tagmask));
	else // right circular shift
		do {
			shifted.ui = (shifted.ui >> (- arg->i)
			   | shifted.ui << (LENGTH(tags) + arg->i));
		} while (tagmask && !(shifted.ui & tagmask));

	view(&shifted);
}

However, I'm still facing the same issue. :(

@bakkeby
Copy link
Owner

bakkeby commented Oct 28, 2020

OK, assuming you didn't forget to compile and/or restart dwm, could you talk me through how you replicate this issue?

I see the change is not there in your build, but that's likely just because you didn't commit or push it yet.
https://github.com/paniash/dwm/blob/master/dwm.c#L2105

Other than that your build seems fine.

@paniash
Copy link
Author

paniash commented Oct 29, 2020

OK, assuming you didn't forget to compile and/or restart dwm, could you talk me through how you replicate this issue?

Naturally with no scratchpads open i.e. not running in the background, the patch works fine. However, let me go through the following workflow:

  • Open a scratchpad when in tag 1, and then hide it.
  • Go to tag 3 and launch a browser.
  • Switch to tag 1 using shiftviewclients.
  • The hidden scratchpad is no more hidden (however, it is not focused).

I see the change is not there in your build, but that's likely just because you didn't commit or push it yet.

I have pushed the updated function as mentioned in my earlier post.

@paniash paniash closed this as completed Oct 29, 2020
@paniash paniash reopened this Oct 29, 2020
@bakkeby
Copy link
Owner

bakkeby commented Oct 29, 2020

Hi @paniash,

I tried this in your build, but I can't replicate the behaviour you are describing.


Thanks,

-Stein

@paniash
Copy link
Author

paniash commented Oct 29, 2020

@bakkeby Have you tried going back and forth between the tags using shiftviewclients for some time? If not the first time, it does appear on the subsequent times on the tag where the scratchpad was originally launched.

@paniash
Copy link
Author

paniash commented Oct 29, 2020

Also, I realized something right now. I have 3 scratchpads generally in use:

  • A plain st instance
  • st running ncmpcpp
  • st running bc calculator

The issue seems to persist only if I have the plain st instance running as scratchpad. The ncmpcpp and bc ones don't have this issue.

At the end of the day, all the scratchpads are st instances. I don't see why this issue has to be there only for one such scratchpad :(

@bakkeby
Copy link
Owner

bakkeby commented Oct 29, 2020

That's interesting, I'll have another look.

@bakkeby
Copy link
Owner

bakkeby commented Oct 29, 2020

Nah, I still can't replicate this. I know it is stupid, but are you absolutely sure that you have recompiled, installed, and restarted dwm?

@paniash
Copy link
Author

paniash commented Oct 29, 2020

Yes, just to make sure once again, I compiled & restarted it and have the same issue. I guess I'll have to live with it. Thanks for all your help @bakkeby!

@bakkeby
Copy link
Owner

bakkeby commented Oct 29, 2020

Hmm, it is bizarre though that I'm not able to replicate it. Would be interesting if someone else would.

Unrelated, but the scratchpads patch has a few holes. E.g.

    { MODKEY,                       XK_0,      view,           {.ui = ~0 } },
    { MODKEY|ShiftMask,             XK_0,      tag,            {.ui = ~0 } },

The ~ is a binary operator which means inverse, so ~0 becomes a binary 111111111.. etc. When you have the scratchpads patch you'll want this to be .ui = ~SPTAGMASK (unless you actually want all the scratchpads to show as well when you do MOD+0).

There are other places in the code where ~0 is used as well to select all tags (and subsequently all scratchpads as well).

@paniash
Copy link
Author

paniash commented Oct 29, 2020

The ~ is a binary operator which means inverse, so ~0 becomes a binary 111111111.. etc. When you have the scratchpads patch you'll want this to be .ui = ~SPTAGMASK (unless you actually want all the scratchpads to show as well when you do MOD+0).

I tried changing it to ~SPTAGMASK as per your suggestion but to no avail. :(

Let me see if this issue still persists with other scratchpad patches or is it just my system for some reason.

@paniash
Copy link
Author

paniash commented Oct 29, 2020

Just to be sure that we're on the same page, here's a screenshot of my issue:

  • Start on tag 1 (notice that tags 5 and 7 are non empty)
    pic-full-201029-1753-03

  • Spawn plain st scratchpad and hide it
    pic-full-201029-1753-31

  • Spawn st running ncmpcpp scratchpad and hide it
    pic-full-201029-1753-52

  • Move to tag 5 using shiftviewclients
    pic-full-201029-1754-30

  • Similarly, move to tag 7
    pic-full-201029-1754-48

  • Cycle back to tag 1 (where the plain st scratchpad appears unfocused)
    pic-full-201029-1755-05

@bakkeby
Copy link
Owner

bakkeby commented Oct 29, 2020

That is clearer, tag 1 is occupied by other clients as well.

@bakkeby
Copy link
Owner

bakkeby commented Oct 29, 2020

Hi @paniash,

that bit shifting is a bit funky, can you try this?

void
shiftviewclients(const Arg *arg)
{
	Arg shifted;
	Client *c;
	unsigned int tagmask = 0;

	for (c = selmon->clients; c; c = c->next)
		if (!(c->tags & SPTAGMASK))
			tagmask = tagmask | c->tags;

	shifted.ui = selmon->tagset[selmon->seltags] & ~SPTAGMASK;
	if (arg->i > 0) // left circular shift
		do {
			shifted.ui = (shifted.ui << arg->i)
			   | (shifted.ui >> (LENGTH(tags) - arg->i));
			shifted.ui &= ~SPTAGMASK; // <--- add this
		} while (tagmask && !(shifted.ui & tagmask));
	else // right circular shift
		do {
			shifted.ui = (shifted.ui >> (- arg->i)
			   | shifted.ui << (LENGTH(tags) + arg->i));
			shifted.ui &= ~SPTAGMASK; // <--- add this
		} while (tagmask && !(shifted.ui & tagmask));

	view(&shifted);
}

Seems to do the trick with my preliminary testing.


Thanks,

-Stein

@paniash
Copy link
Author

paniash commented Oct 29, 2020

Hey @bakkeby!

This seems to have done the trick! Thank you very much! btw were you able to replicate the issue after I posted the screenshots?

Thanks,
-Ashish

@bakkeby
Copy link
Owner

bakkeby commented Oct 29, 2020

Hi @paniash,

yes I was. I was trying to replicate only opening the scratchpad on tag 1, then moving to tag 3, opening an application and then using shiftviewclients to move back to 1. That it only affected the first scratchpad and not the others is a bit circumstantial, likely the others would also be affected depending on what tags are selected when you shift.


Thanks,

-Stein

@paniash
Copy link
Author

paniash commented Oct 29, 2020

That's great! Thank you once again. :)

@paniash paniash closed this as completed Oct 29, 2020
@bakkeby
Copy link
Owner

bakkeby commented Oct 29, 2020

Thank you for reporting. Getting to the bottom of issues like this helps everyone who takes / uses these patches.

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