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

Only respond to modifier-only keybindings on release #1761

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

spl237
Copy link
Contributor

@spl237 spl237 commented Apr 26, 2024

This is a possible fix for #1756

I have modified the keybinding code so that a flag is set in the keybinding data structure if the keybinding only involves modifier keys.

If this flag is set for a particular keybinding, then that keybinding is detected on key press and stored, and executed on key release if no other (non-modifier) keybinding has executed in the meantime.

This fixes my immediate problem, that of wanting Win+cursor to activate snap behaviour, and Win on its own to launch the application menu, but I have no idea if this is the best way to do this. It seems to work for me, though!

@Consolatis
Copy link
Member

Consolatis commented Apr 26, 2024

I am also not sure the key state tracking is correct here, if we eat the release event but let the press event through to the focused client that will likely result in the client repeating the key on a timer.

In any case I feel very uncomfortable to have this in 0.7.2, the general approach looks good to me though.

@spl237
Copy link
Contributor Author

spl237 commented Apr 26, 2024

Agree this shouldn't be in 0.7.2 - it's a quick hack I put together this morning, and needs a lot of testing for corner cases etc, but if the general approach is ok, I'm happy to look at improvements.

@Consolatis Consolatis added this to the 0.7.3 milestone Apr 26, 2024
@spl237
Copy link
Contributor Author

spl237 commented Apr 29, 2024

I am also not sure the key state tracking is correct here, if we eat the release event but let the press event through to the focused client that will likely result in the client repeating the key on a timer.

I'm not sure I completely understand the key state tracking, but in an attempt to deal with this, I have modified the code so that the stored key state is only updated on press and not on release - does this look like a better approach?

Interestingly, it has also meant that a modifier-only keybinding is only executed if the key is released relatively quickly, not if it is held down for more than a second or so, which I think is actually better.

@Flrian
Copy link
Contributor

Flrian commented Apr 29, 2024

There is also another, older issue for modifier-only keybinds #351
^ Just remembered that one and thought I'd mention it, so it gets closed if this PR fixes the issue.

@Consolatis
Copy link
Member

Consolatis commented Apr 29, 2024

There is also another, older issue for modifier-only keybinds #351 ^ Just remembered that one and thought I'd mention it, so it gets closed if this PR fixes the issue.

For some reason I can't link that issue to this PR (it doesn't show up in the link issue popup). Well, guess we'll have to manually close it then once this is merged.

@ahesford ahesford linked an issue Apr 29, 2024 that may be closed by this pull request
@ahesford
Copy link
Member

Linked. The issue just had to be pinged to convince GitHub that it was still active.

@spl237 spl237 mentioned this pull request May 3, 2024
@johanmalm
Copy link
Collaborator

Sorry it's taken a while to get to this one. Just working down the list.

What's the best way of testing it? I tried binding to W but can't get it to work.

    <keybind key="W">
      <action name="Execute" command="rofi -show drun"/>
    </keybind>

@spl237
Copy link
Contributor Author

spl237 commented Jun 2, 2024

Bind to Super_L:

<keybind key="Super_L"> <action name="Execute"> <command>wfpanelctl smenu menu</command> </action> </keybind>

I think W is the key when used as a modifier, and Super_L is the same key as a plain keystroke.

@johanmalm
Copy link
Collaborator

Bind to Super_L:

<keybind key="Super_L"> <action name="Execute"> <command>wfpanelctl smenu menu</command> </action> </keybind>

I think W is the key when used as a modifier, and Super_L is the same key as a plain keystroke.

Genius. Will have a play 😄

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.

Modifier-only keybind
5 participants