Add focus_wrap action and support NEXT and PREVIOUS directions#151
Open
ArhanChaudhary wants to merge 3 commits into
Open
Add focus_wrap action and support NEXT and PREVIOUS directions#151ArhanChaudhary wants to merge 3 commits into
ArhanChaudhary wants to merge 3 commits into
Conversation
mogenson
reviewed
Mar 28, 2026
| focus_down = Fnutils.partial(Actions.PaperWM.windows.focusWindow, Direction.DOWN), | ||
| focus_prev = Fnutils.partial(Actions.PaperWM.windows.focusWindow, Direction.PREVIOUS), | ||
| focus_next = Fnutils.partial(Actions.PaperWM.windows.focusWindow, Direction.NEXT), | ||
| focus_wrap = Actions.PaperWM.windows.focusWrap, |
Owner
There was a problem hiding this comment.
Does this need to be exposed as an action? All the other actions have the necessary arguments provided via Fnutils.partial. This looks like the only action that requires arguments to work, which makes it a bit harder to bind to a hot key.
mogenson
reviewed
Mar 28, 2026
| ---@param direction Direction use either Direction UP, DOWN, LEFT, RIGHT, PREVIOUS, or NEXT | ||
| ---@param focused_index Index index of focused window within the windowList | ||
| ---@return Window? | ||
| function Windows.focusWrap(direction, focused_index) |
Owner
There was a problem hiding this comment.
It would be great to add some unit tests for this new method to spec/windows_spec.lua
Owner
|
Looks good, sorry I'm a bit late on review. Let me know if you can add unit tests for the new focusWrap method. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I have modified #150 to support infinite window looping with the NEXT and PREVIOUS directions, and refactored it as an action in case people want to call it for whatever reason. I have not added my own test cases, but they pass the existing ones. I might do that later.
For what it is worth, I was testing my implementation on my desktop and ran into a sporadic error that crashed the entire window manager:
So, yeah, I'm not sure if that's related to my changes here or another previously known bug.
(I also fixed the inconsistent spacing in the README in this PR because I didn't think it was worth an entirety separate PR to fix)