Skip to content

fix(whkd): bind rwin by default when binding win#82

Open
akozlev wants to merge 2 commits intoLGUG2Z:masterfrom
akozlev:fix/binding-rwin-modifier-by-default
Open

fix(whkd): bind rwin by default when binding win#82
akozlev wants to merge 2 commits intoLGUG2Z:masterfrom
akozlev:fix/binding-rwin-modifier-by-default

Conversation

@akozlev
Copy link
Copy Markdown

@akozlev akozlev commented Jun 5, 2025

Thank you for you work on whkd and komorebi.

I was experiencing an issue with binding the Windows key. I was able to use the left windows key to trigger a hotkey but not the right one. Let me know if this is the intended behavior. Otherwise, please read my reasoning below about why I created this PR.

The user might expect that a hotkey like win + k would trigger regardless of which windows key is pressed.

This pull request ensures that VKey::RWIN is registered alongside with VKey::LWIN when specifying a win as a modifier. [1]

// https://github.com/iholston/win-hotkeys/blob/b967bca6559a1b34ac09425389be46117554be2a/src/keys.rs#L547  
"SHIFT" => VKey::Shift,
"CTRL" => VKey::Control,
"CONTROL" => VKey::Control,
"ALT" => VKey::Menu,
...     
"WIN" => VKey::LWin,

This is required because win-hotkeys doesn't handle lwin and rwin the same way that it handles shift, alt and ctrl.

Things to consider:

  • If pause hotkey is assigned to a hotkey with win modifier it will not register rwin by default.
  • Hotkeys are not checked for conflicts. For example, if you register win + k and rwin + k it might result in a conflicting binding because of the default.

Let me know if there is anything you would like me to modify.

[1] https://github.com/iholston/win-hotkeys/blob/b967bca6559a1b34ac09425389be46117554be2a/src/keys.rs#L547

@akozlev
Copy link
Copy Markdown
Author

akozlev commented Oct 11, 2025

Hey @LGUG2Z,
Do you have time to share your thoughts on this PR?

Thanks.

@LGUG2Z
Copy link
Copy Markdown
Owner

LGUG2Z commented Oct 11, 2025

Hotkeys are not checked for conflicts. For example, if you register win + k and rwin + k it might result in a conflicting binding because of the default.

I think the only change I'd like to see before adding this is that we ensure that binds to rwin will take take priority over the default double-bind with win

This commit ensures that `VKey::RWIN` is registered alongside with
`VKey::LWIN` when specifying a `win` as a modifier.

This is required  because `win-hotkeys` doesn't handle lwin and rwin the
same way that it handles shift, alt and ctrl.
@akozlev akozlev force-pushed the fix/binding-rwin-modifier-by-default branch from 17957a9 to ee97062 Compare October 12, 2025 14:15
@akozlev akozlev marked this pull request as draft October 12, 2025 14:24
@akozlev akozlev force-pushed the fix/binding-rwin-modifier-by-default branch from ee97062 to c26a89f Compare October 12, 2025 22:10
@akozlev
Copy link
Copy Markdown
Author

akozlev commented Oct 13, 2025

Thank you for the feedback, @LGUG2Z.

I've updated it to check the initial bindings list for ones specified with rwin and only create a new rwin binding if is not yet specified. I've used a HashSet for comparison to ensure that equality is not dependent of the order of the specified keys.
I added a couple of simple tests to verify the behaviour.

@akozlev akozlev marked this pull request as ready for review October 13, 2025 17:48
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