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 option to not move cursor when focussing a window via ipc #1094

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

prog-r-amer
Copy link

Hi, I added a flag for not moving the cursor when focussing a window via ipc. Seems like I was the only one interested in that, but it was a personal pet peeve of mine.
#935

Thanks for the amazing compositor.

@@ -765,6 +765,13 @@ impl State {
self.niri.queue_redraw_all();
}

/// Focus a specific window without moving the cursor.
pub fn focus_window_without_moving_cursor(&mut self, window: &Window) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe instead a bool argument to focus_window()?

Copy link
Author

@prog-r-amer prog-r-amer Feb 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, I just thought maybe there's more and more arguments coming and it's better to just duplicate since I didn't know what to do with the maybe warp cursor. I guess not moving cursor should override warp to focus but I wasnt sure.

Comment on lines 217 to 219
/// Dont move the cursor when focussing the window.
#[cfg_attr(feature = "clap", arg(long = "dont-move-cursor", action = clap::ArgAction::SetFalse))]
move_cursor: bool,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking to make it something like no_mouse_warp: bool (especially since it's "warp mouse to focus" in the config).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I just thought maybe the negation in the argument is confusing, since we then have a no_mouse_warp that is by default false.

@@ -1322,7 +1322,7 @@ pub enum Action {
#[knuffel(skip)]
FullscreenWindowById(u64),
#[knuffel(skip)]
FocusWindow(u64),
FocusWindow(u64, bool),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change it to named fields please { id: u64, ... }

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

src/input/mod.rs Outdated
@@ -674,11 +674,15 @@ impl State {
self.niri.queue_redraw_all();
}
}
Action::FocusWindow(id) => {
Action::FocusWindow(id, move_cursor) => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you also want to add it to FocusWorkspace, FocusWorkspaceDown, FocusWorkspaceUp? Same argument and works the same way. Others were asking for these

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, but I had a problem. I didn't manage to do the focus workspace the same as the others cause it had the #[knuffel(argument)] WorkspaceReference in it.

@prog-r-amer prog-r-amer marked this pull request as draft February 15, 2025 17:18
@prog-r-amer prog-r-amer force-pushed the focus-window-cursor-option branch from 956b484 to d6e7203 Compare February 15, 2025 17:26
@prog-r-amer
Copy link
Author

git didn't do what I wanted...

@prog-r-amer prog-r-amer force-pushed the focus-window-cursor-option branch from c1a4697 to 45832ce Compare February 15, 2025 17:58
FocusWorkspaceDown,
FocusWorkspaceUp,
FocusWorkspace(#[knuffel(argument)] WorkspaceReference),
#[knuffel(skip)]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They shouldn't be skipped, otherwise you cannot put them in the config.

@@ -214,6 +214,9 @@ pub enum Action {
/// Id of the window to focus.
#[cfg_attr(feature = "clap", arg(long))]
id: u64,
/// Dont move the cursor when focussing the window.
#[cfg_attr(feature = "clap", arg(long = "no_mouse_warp", action = clap::ArgAction::SetTrue))]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why spell out the name in long = "no_mouse_warp"? Also, is SetTrue needed even for bools?

@@ -214,6 +214,9 @@ pub enum Action {
/// Id of the window to focus.
#[cfg_attr(feature = "clap", arg(long))]
id: u64,
/// Dont move the cursor when focussing the window.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Dont move the cursor when focussing the window.
/// Disable moving the cursor to the newly focused window or output.

Same change in other places

@prog-r-amer
Copy link
Author

@YaLTeR sorry I am super stuck.
image

The reason I commented out the needed knuffel thing is because I wanted to change it to a named struct so there are no weird bools flying around. It seems I have to implement something for the knuffle derive to work but I don't have any clue how to do it.

Sorry I am new to the macro stuff. Maybe I am on a completely wrong track.

@YaLTeR
Copy link
Owner

YaLTeR commented Feb 16, 2025

It seems I have to implement something for the knuffle derive to work

No, you should be able to just skip the field itself (rather than the whole enum variant).

@prog-r-amer prog-r-amer force-pushed the focus-window-cursor-option branch from 45832ce to e1523e8 Compare February 17, 2025 20:16
@prog-r-amer prog-r-amer force-pushed the focus-window-cursor-option branch from e1523e8 to 6315557 Compare February 17, 2025 20:25
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.

2 participants