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 --allow-other flag for keybindings #7602

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chayleaf
Copy link

@chayleaf chayleaf commented May 23, 2023

This flag allows other keys to be pressed at the same time as the binding. This acts as follows:

  • All key modifiers not explicitly listed in the binding are ignored. The binding's modifiers are still required to be pressed.
  • All keys not listed in the binding are ignored, the binding is triggered as long as the keys in the binding are a subset of the keys pressed.
  • If other keys are pressed between press and release, release is still triggered.

The particular use case for me is Mumble push-to-talk.

In games, you often talk at the same time as doing something else in-game. Before, I had this rather abysmal config since sway doesn't support ignoring key modifiers for bindings (I have a mouse button that I use for PTT programmed to Scroll Lock, a mostly unused key, to make sure games don't do anything when I press the key):

bindsym --inhibited --no-repeat Scroll_Lock exec mumble rpc starttalking
bindsym --inhibited --no-repeat control+Scroll_Lock exec mumble rpc starttalking
bindsym --inhibited --no-repeat control+lock+Scroll_Lock exec mumble rpc starttalking
bindsym --inhibited --no-repeat control+mod1+Scroll_Lock exec mumble rpc starttalking
bindsym --inhibited --no-repeat control+mod1+mod4+Scroll_Lock exec mumble rpc starttalking
bindsym --inhibited --no-repeat control+mod2+Scroll_Lock exec mumble rpc starttalking
...continues for a total of 33 lines, plus the same for stoptalking:
bindsym --inhibited --no-repeat --release Scroll_Lock exec mumble rpc stoptalking
...

However, this had one major issue - if I pressed any other key between the press and release of Scroll_Lock, the release binding wouldn't execute, and my microphone would be kept active until someone tells me about it and I press the key again, just to release it and execute the release binding. This PR introduces a flag for what I consider ideal push to talk behavior.

The reason this is necessary in the first place is because on X, there is some part of the spec that allows intercepting all keypresses which Mumble uses. There's no such thing on Wayland, closest thing I could find is this merged PR for xdg-desktop-portal, but I don't think there's an easy way to make xdg-desktop-portal-wlr support it without a completely new Wayland protocol supported by wlroots, and Mumble doesn't use the xdg-desktop-portal global shortcuts API yet anyway. See emersion/xdg-desktop-portal-wlr#240

This fills the last major gap I had in experience between X and Wayland. Understandably, i3 doesn't have this feature because it isn't necessary there. Feedback is welcome on making this apply to more situations than just push to talk (or on getting a better name). If the xdg-desktop-portal solution is preferred (and feasible), I'm fine with closing this and using this as a patch until xdg-desktop-portal-wlr becomes more mature.

Closes #6456

Prior art: #6616 #6920

This flag allows other keys to be pressed at the same time as the
binding. This acts as follows:
- All key modifiers not explicitly listed in the binding are ignored.
  The binding's modifiers are still required to be pressed.
- If other keys are pressed between press and release, release is still
  triggered.
@rpigott
Copy link
Member

rpigott commented May 23, 2023

What is the expected behavior if another binding is pressed which has a superset of the listed keys? Both activate? Only one? What if they both have the flag?

I think it makes sense to add some way to allow the user to type while a binding is held, since as you say this functionality exists on X and not in sway currently.

@chayleaf
Copy link
Author

chayleaf commented May 25, 2023

What is the expected behavior if another binding is pressed which has a superset of the listed keys?

I would expect both to activate, assuming the subset has the flag and the superset either does or does not (this reminds me of how annoying it was to have language switching set to Ctrl+Shift/Alt+Shift break resp Ctrl+Shift+Tab/Alt+Shift+Tab when switching from Windows). However, adding that to sway would probably be quite hacky as it seems like it only supports one binding per keypress (or per key sequence?), so in my opinion this is a nice middle ground between required changes and features gained. I can extend it to handle multiple bindings for a single keypress, but I'd like to receive more general feedback first (and it wouldn't be that useful overall).

Also, at the moment the code in this PR is somewhat buggy, in very rare cases the release binding doesn't get called, but it only happened four times in a few days of testing and I couldn't figure out which conditions it happens under (seemingly the release event for the key just didn't trigger at all, I can't find any other explanation from looking at the code).

@TheNyneR
Copy link

Also, at the moment the code in this PR is somewhat buggy, in very rare cases the release binding doesn't get called, but it only happened four times in a few days of testing and I couldn't figure out which conditions it happens under (seemingly the release event for the key just didn't trigger at all, I can't find any other explanation from looking at the code).

I just applied the a patch and found a way to replicate it on my end:
I use the commands
bindsym --allow-other --whole-window --no-repeat button9 exec dbus-send --session --type=method_call --dest=net.sourceforge.mumble.mumble / net.sourceforge.mumble.Mumble.startTalking
and
bindsym --allow-other --whole-window --release button9 exec dbus-send --session --type=method_call --dest=net.sourceforge.mumble.mumble / net.sourceforge.mumble.Mumble.stopTalking

When I press and hold button9 Push-to-talk activates as expected, however if I keep holding it and press any other mouse button at the same time it gets stuck until I press and release button9 again.

@TheNyneR
Copy link

Just tried it again and it seems like the --allow-other flag does not work as intended. Some buttons like Ctrl, Shift, Mod and Alt don't trigger the command when they are pressed together with the bound key. Is this intended or does it just not work on my end @chayleaf?

@chayleaf
Copy link
Author

it definitely worked for me, I'll test this again later

@emersion emersion added enhancement New feature or incremental improvement window-management Configuring and arranging toplevels labels Jan 26, 2024
@chayleaf
Copy link
Author

while I haven't been able to reproduce your issue, here's my config:

bindsym --inhibited --no-repeat --allow-other --release Scroll_Lock exec mumble rpc stoptalking
bindsym --inhibited --no-repeat --allow-other Scroll_Lock exec mumble rpc starttalking

@TheNyneR
Copy link

Just tried the patch again and the allow_other logic seems to work fine.
However the issue in #7602 (comment) still persists.

I use the following bindings:

bindsym --inhibited --no-repeat --allow-other --release button9 exec mumble rpc stoptalking
bindsym --inhibited --no-repeat --allow-other button9 exec mumble rpc starttalking

When I press button9 I start talking like expected. When I release button9 immediately the stoptalking command gets executed as expected. Now when I hold button9 and press e.g. button1 then release button9 the --release event gets swallowed and the stoptalking command does not get executed.

To reproduce I tried the same with swaynag:

bindsym --inhibited --no-repeat --allow-other --release button9 exec swaynag -m "release"

It seems like the issue only occurs if the inputs come from the same device, e.g. pressing button9 and button1 on a mouse or Scroll_Lock and a on a keyboard but not when pressing button9 on the mouse and a on the keyboard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or incremental improvement window-management Configuring and arranging toplevels
Development

Successfully merging this pull request may close these issues.

keysym --release is unreliable
4 participants