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

Exec release binding even if other keys were pressed #6920

Closed
wants to merge 1 commit into from

Conversation

lostmythread
Copy link

@lostmythread lostmythread commented Mar 27, 2022

This commit changes the "release binding" behavior to ignore foreign
keypresses (pressed keys that are not part of a given release binding's
definition) while still triggering the bound action when the original
key from the binding is released. This, unless foreign keypresses match
a different release binding, in which case the latter binding takes
over.

For instance, assuming a release binding is configured for <Mod>, a key
corresponding to one of the possible modifier keys. The previous behavior
for the following sequence was to trigger the action only for the following
sequence:

  • <Mod>(press), <Mod>(release)

but not in the following case:

  • <Mod>(press), <Tab>(press), <Tab>(release), <Mod>(release)

With this commit both sequences trigger the bound action.

This is useful to control external software by triggering possibly
repeated actions, e.g. to navigate windows using <Mod> + <Tab>
and triggering a final action when the key is ultimately released
(e.g. hiding a UI, reactivating LRU accounting, ...)

Closes #6456

This commit changes the "release binding" behavior to ignore foreign
keypresses (pressed keys that are not part of a given release binding's
definition) while still triggering the bound action when the original
key from the binding is released. This, unless foreign keypresses match
a different release binding, in which case the latter binding takes
over.

For instance, assuming a release binding is configured for <Mod>, a key
corresponding to one of the possible modifier keys. The previous behavior
for the following sequence was to trigger the action only for the following
sequence:

- <Mod>(press), <Mod>(release)

but *not* in the following case:

- <Mod>(press), <Tab>(press), <Tab>(release), <Mod>(release)

With this commit both sequences trigger the bound action.

This is useful to control external software by triggering possibly
repeated actions, e.g. to navigate windows using <Mod> + <Tab>
and triggering a final action when the <Mod> key is released (e.g.
hiding a UI, reactivating LRU accounting, ...)

Closes swaywm#6456
@lostmythread lostmythread changed the title exec release binding even if other keys were pressed Exec release binding even if other keys were pressed Apr 1, 2022
@somini
Copy link

somini commented Apr 8, 2022

This sounds great.

It's not exactly #6456, that mentions implementing a new feature of ignoring all modifiers when releasing a key. But I think this does solve that issue.

@emersion
Copy link
Member

Does this break i3 compat?

@lostmythread
Copy link
Author

The Sway and i3 code for bindings are actually pretty different, and I'm not exactly an authority on either of them so I can only speak with limited confidence. My understanding of what i3 does for a "release binding" is based on reading the code in bindings.c::get_binding():

  1. The initial key press matching the binding marks it as ignoring modifiers (B_UPON_KEYRELEASE_IGNORE_MODS) when checking for a match between the binding and the key release event (note: modifiers do need to match on the initial key press).
  2. The key release triggers the binding even if modifiers have been released before the binding's key.
  3. Pressing other unrelated keys between the initial key press from the release binding and its corresponding release will cancel the "modifier ignore behavior" in 2. I.e. the release binding will trigger only if modifiers are still pressed when the key is released.

To be clear, I don't have an i3 on hand, so the above is only based on code analysis and not on the observation of a live instance.

The changes introduced in this Sway PR will trigger the release binding so long as no other release binding is armed by another key press, in which case the most recent release binding takes precedence.
In other words, the behavior is not exactly the same as i3's but is nevertheless much closer than the pre-PR behavior in Sway (at least to my understanding).

@larskotthoff
Copy link

I'd like to see this merged. I'm using sov to show an overview of all workspaces when mod is pressed, which helps me to select the workspace to switch to. Because that involves pressing another key, sov remains visible after switching to the workspace and I have to press mod again to make it disappear.

Merging this PR would solve this issue.

@softmoth
Copy link

softmoth commented Sep 15, 2022

Even if this is not identical to i3 behavior, I think it is a significantly more important feature to have in sway because Wayland puts all of the input handling on the compositor. In X, the window manager is not privileged compared to other clients, and any lowly client can implement alt-tab. That would require root privileges under Wayland.

I hope this patch is accepted.

@humanplayer2
Copy link

This is excellent, thank you @lostmythread ! I just tried the changes in 1.8-dev-9d99bb95 (Oct 25 2022, branch 'master'), and I get exactly the behavior I hope for. @larskotthoff, this indeed is very nice for Sway Overview!

@kennylevinsen
Copy link
Member

In X, the window manager is not privileged compared to other clients, and any lowly client can implement alt-tab.

They can also do that under sway by binding alt-tab. This issue is about unconditionally binding alt-release to make a window preview UI fade away only when the modifier is released.

Such window preview UI already needs layer-shell to hover statically centered over all other content, and layer-shell has provisions for taking keyboard focus so the client can listen to any keyboard event it likes, including release of the alt key.

That would require root privileges under Wayland.

Nothing requires root on Wayland - some things are disallowed under Wayland, but the use-case presented here is covered by layer-shell.


The behavior implemented in this PR deviates a lot from i3 and will break current uses of --release by causing the bindings to fire at unexpected times. As the use-case is already handled by dedicated Wayland protocols, I'm closing this PR.

We can reconsider the decision if the current release semantics are found problematic separately in ways not covered by Wayland protocols.

@softmoth
Copy link

layer-shell has provisions for taking keyboard focus so the client can listen to any keyboard event it likes, including release of the alt key

I haven't seen this done in any alt-tab style client, but it does sound like it should do what's needed.

@lostmythread
Copy link
Author

@kennylevinsen, I'm unfortunately not familiar with "layer-shell". My reason for using this patch was to call swayr using the release key binding to break-out of its window traversal sequence mode (wherein window LRU order ceases to be updated). Swayr doesn't have a UI, but as I understand this would be a sticking point if I were to try and adapt to using layer-shell, no?

Anyway thanks for the time you took to consider this patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

keysym --release is unreliable
7 participants