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

actions: add "loop" variants of "switch-{next,previous,left,right,up,down}" #787

Merged
merged 3 commits into from
Feb 25, 2024

Conversation

davvid
Copy link
Contributor

@davvid davvid commented Feb 19, 2024

The "switch-next", "switch-prev" and related actions currently do
nothing once they reach the last window in the action's direction.
Add "loop" variants of these actions that loop around when the last
window is reached.

The new actions are "switch-previous-loop", "switch-next-loop",
"switch-left-loop", "switch-right-loop", "switch-up-loop" and
"switch-down-loop".

@jtaala
Copy link
Collaborator

jtaala commented Feb 20, 2024

Thanks @davvid!

This would need to be an option though (e.g. in preferences) to enable/disable this behaviour.

It doesn't "feel" right to me, although maybe it's just because I prefer, and am used to the original behaviour. Others might like this though, so would consider accepting this if it's made a user option (with default behaviour as it is currently).

Cheers,

Jay.

@jtaala
Copy link
Collaborator

jtaala commented Feb 20, 2024

Reach out if you need a hand in adding this to preferences!

@jtaala
Copy link
Collaborator

jtaala commented Feb 20, 2024

Just adding this here for context as to why wrap-around was not implemented previously: #278

@davvid
Copy link
Contributor Author

davvid commented Feb 21, 2024

Alrighty, I added a preference for the loop-around so that it's opt-in. Thanks for the context.

I updated everything except for schemas/gschemas.compiled. Do you want me to run make -C schemas to regenerate it and commit that as well? I did regenerate the compiled schemas to get things working (on a separate cycle-windows/gnome-44 branch because I'm currently on gnome 44.8) but I didn't commit it since it's a binary file and it seemed worth holding off on that in case there's review notes that might affect the final schema.

Thanks for being open to adding this feature. cheers!

@Lythenas
Copy link
Collaborator

This works fine as is, so from my side would be fine to merge (after generating the schema).

@smichel17
Copy link
Collaborator

smichel17 commented Feb 21, 2024 via email

@jtaala
Copy link
Collaborator

jtaala commented Feb 21, 2024

This works fine as is, so from my side would be fine to merge (after generating the schema).

I thought I was doing something wrong here as I couldn't get this to work. Then I realised this works for switch-next, switch-previous but NOT for the switch-left, switch-right (which I use - also realised I don't even use switch next/previous).

From the description I would expect this to also work for switch-left, switch-right - thoughts on implementing this for switch-left, switch-right as well (if the option is enabled)?

I guess it comes down to if this behaviour (if enabled) only makes sense for switch-next/switch-previous and not the switch left/right keybinds.

@jtaala
Copy link
Collaborator

jtaala commented Feb 21, 2024

This works fine as is, so from my side would be fine to merge (after generating the schema).

I thought I was doing something wrong here as I couldn't get this to work. Then I realised this works for switch-next, switch-previous but NOT for the switch-left, switch-right (which I use - also realised I don't even use switch next/previous).

From the description I would expect this to also work for switch-left, switch-right - thoughts on implementing this for switch-left, switch-right as well?

This is another option, and would have been much easier to implement (sorry @davvid!). I guess keybinds would allow uses to assign keybinds for both use cases (or just one by replacing the defaults if you always wanted wrap around).

See comment #787 (comment) - keybinds would make more sense here (keybinds for switch-next-loop, switch-previous-loop AND switch-left-loop, switch-right-loop) since this would allow users to choose their preferred use case (maybe they only want loop for left/right and not next/previous..., or vice-versa,... or both). I.e. this would be more customisable than a single option for both (and less confusing that adding another option for left/right).

@davvid
Copy link
Contributor Author

davvid commented Feb 22, 2024

Good stuff, yeah these do feel like distinct actions. I'll take a stab at that when I get a chance later this or next week. Another nice thing is we won't need to change the schemas or add more options to the UI.

@davvid davvid changed the title tiling: cycle through windows when switching beyond the first or last windows actions: add "loop" variants of switch-{next,previous,left,right,up,down} Feb 22, 2024
@davvid
Copy link
Contributor Author

davvid commented Feb 22, 2024

I've split these out to separate actions and regenerated the schemas. Let me know what you think.

…down}"

The "switch-next",  "switch-prev" and related actions currently do
nothing once they reach the last window in the action's direction.
Add "loop" variants of these actions that loop around when the last
window is reached.

The new actions are "switch-previous-loop", "switch-next-loop",
"switch-left-loop", "switch-right-loop", "switch-up-loop" and
"switch-down-loop".
@davvid davvid changed the title actions: add "loop" variants of switch-{next,previous,left,right,up,down} actions: add "loop" variants of "switch-{next,previous,left,right,up,down}" Feb 22, 2024
generally referred to as "wrap around" in PaperWM).
@jtaala jtaala self-requested a review February 23, 2024 20:18
@jtaala
Copy link
Collaborator

jtaala commented Feb 23, 2024

Thanks @davvid - just reordered and simplified keybind descriptions.

Planning on merging this one into develop and doing a release for this soon.

@jtaala jtaala merged commit f8e982c into paperwm:develop Feb 25, 2024
@davvid davvid deleted the cycle-windows/develop branch February 27, 2024 03:31
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.

None yet

4 participants