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

Add moving window to next/previous tag (#751) #757

Merged
merged 1 commit into from
Jun 21, 2022

Conversation

zvolin
Copy link
Contributor

@zvolin zvolin commented May 1, 2022

Description

Add moving window to the next or previous tag.

When executing while working on first/last tag, the command will move window wrapping around tag list.

Fixes #751

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update (wiki / Keybind Commands)

Updated user documentation:

Wiki

Keybind Commands

...

MoveWindowToNextTag

Takes the window that is currently focused and moves it to the next tag. If value is true, the focus will follow the window. Next tag is brought to the current workspace and focused.

Example:

[[keybind]]
command = "MoveWindowToNextTag"
value = "true"
modifier = ["modkey", "Shift"]
key = "l"

MoveWindowToPreviousTag

Takes the window that is currently focused and moves it to the previous tag. If value is true, the focus will follow the window. Previous tag is brought to the current workspace and focused.

Example:

[[keybind]]
command = "MoveWindowToPreviousTag"
value = "true"
modifier = ["modkey", "Shift"]
key = "h"

MoveToLastWorkspace

...


command_handler

I've been struggling a bit with my command not taking an effect when running with leftwm-command so I've updated section about where to look when adding/renaming commands.

See CONTRIBUTING.md User Documentation section for further details. (Seems broken link)


Note: Manual page changes must be performed in a commit, not in this PR section.

Checklist:

  • Ran make test locally with no errors or warnings reported
  • Enhanced review is performed with cargo clippy -- -W clippy::pedantic -A clippy::must_use_candidate -A clippy::cast_precision_loss -A clippy::cast_possible_truncation -A clippy::cast_possible_wrap -A clippy::cast_sign_loss -A clippy::mut_mut
  • Manual page has been updated accordingly (Not needed unless we want this as default)
  • Wiki pages have been updated accordingly (to perform after merge)

@zvolin
Copy link
Contributor Author

zvolin commented May 1, 2022

I'm wondering whether it would be useful to add an argument to this command whether focus should follow

@AethanFoot
Copy link
Member

For the additional argument I would say whether to follow makes sense, then if it goes to a hidden tag it brings that tag to the current workspace and focuses (similar to wmctrl commands). Great work!

@zvolin zvolin force-pushed the add-moving-window-to-tags-relative branch from 10f1950 to a3ec142 Compare May 4, 2022 19:02
@zvolin
Copy link
Contributor Author

zvolin commented May 4, 2022

For the additional argument I would say whether to follow makes sense, then if it goes to a hidden tag it brings that tag to the current workspace and focuses (similar to wmctrl commands). Great work!

So I've added the additional argument whether focus should follow the window. I've enabled it as a default as it feels convenient but this may be subjective.

I use single monitor setup at home, will compile and play with this on 2 monitors tomorrow at work.

I've also discovered an issue when testing this one, when you have selected a layout which doesn't equally size windows (e.g. RightWiderLeftStack) and you change tags back and forth, windows gets resized to being equal until you change layout back and forth again. Will describe that in a day and can dig it further if nobody minds.

@zvolin
Copy link
Contributor Author

zvolin commented May 4, 2022

I've updated Wiki documentation proposal in first comment but just realized I'm also missing external commands section

@zvolin zvolin force-pushed the add-moving-window-to-tags-relative branch from a3ec142 to 93d714a Compare May 5, 2022 15:22
@zvolin
Copy link
Contributor Author

zvolin commented May 5, 2022

So yesterday's implementation had one issue. When focus was Sloppy then there was a bug, if after moving window to the occupied tag the mouse ended up being over the other window, it gained the focus instead of the moved window.

I'm not sure how could I write a test versus such a case but I've just pushed a version with fix for it

@VuiMuich
Copy link
Member

So yesterday's implementation had one issue. When focus was Sloppy then there was a bug, if after moving window to the occupied tag the mouse ended up being over the other window, it gained the focus instead of the moved window.

I'm not sure how could I write a test versus such a case but I've just pushed a version with fix for it

Sorry for the delayed reply.
I am not certain I follow the bug description correctly, how is it possible to move a window from another tag to the focused one? The current focused window always 'enforces' the focused tag. But in sloppy the mouse gets centred on the focused window so it refocuses and I see how that can lead to an unintended behaviour.
Maybe just leave a TODO commend that a test for this situation is desired.

Other then that this looks merge-ready to me. @AethanFoot what do you say?

@zvolin
Copy link
Contributor Author

zvolin commented May 23, 2022

I am not certain I follow the bug description correctly, how is it possible to move a window from another tag to the focused one?

The case was that when moving window to the next/prev tag and then instantly also focusing this tag, current cursor position was deciding factor what window from the new tag got focused. Eg. you had one window on a tag next to the current. You moved the current window to the next tag and brought focus to this tag. If you had the cursor on the left side of the screen, the left window got focused, if on the right side then the right one. Didn't matter where the original window was placed. It resulted in scenarios where when moving window a few tags next or previous, you could start moving other window by accident. Not sure if you'd test this, I can add TODO if you wish

@zvolin
Copy link
Contributor Author

zvolin commented May 23, 2022

Huh, I think that the best explanation of this would be to just show the diff, however it seems like I cannot choose to see diff between last two commits (latter being the former amended). I've never used github much as I work with gitlab on daily basis but does github dislikes amends and rebases? I've always thought this is how git is meant to be used...

Nevertheless, here is probably close to how diff looks like:

-    if follow {
-        manager.state.goto_tag_handler(desired_tag);
-    }
+    if follow {
+        let moved_window = *manager.state.focus_manager.window_history.get(1)?;
+        manager.state.goto_tag_handler(desired_tag);
+        manager.state.handle_window_focus(&moved_window?);
+    }

@VuiMuich
Copy link
Member

After a long wait I finally had some time to put it to a test and every thing seems to be fine.
Thanks!

PS: would you please update the wiki? Thanks.

@VuiMuich VuiMuich merged commit 0730a31 into leftwm:main Jun 21, 2022
@zvolin
Copy link
Contributor Author

zvolin commented Jun 21, 2022

Sure

@zvolin
Copy link
Contributor Author

zvolin commented Jun 23, 2022

done

@VuiMuich
Copy link
Member

Thanks again!

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.

Move to next/previous tag
3 participants