feat(eventtap): let a mode bind the physical mouse buttons - #1418
feat(eventtap): let a mode bind the physical mouse buttons#1418mitchellngoodman wants to merge 1 commit into
Conversation
The mode event tap watched keyboard events only, so a click from a real mouse — or from a keyboard key that sends one — never reached the active mode: the click landed, the overlay stayed up, and dismissing it took a second keystroke. MouseLeft, MouseRight and MouseMiddle are now named keys a mode binds like any other. A mode observes a button without consuming it, so the binding runs in addition to the click reaching the application underneath, and clicks Neru posts itself are marked so a binding cannot fire on its own action. Modifiers are not encoded into the name: a modified click reports the button alone. A mouse button in the global [hotkeys] table is refused rather than accepted, because a global hotkey resolves to a virtual key code that no mouse button has and the binding would never fire. Only the macOS tap reports these today; they stay in the shared vocabulary as Insert and F21-F24 do, so one config file still validates everywhere. Closes y3owk1n#1417
1f7da04 to
09b8d38
Compare
|
i will comment on this later when i am done and happy with my first round of refactoring the codebase, so that you don't have to rebase that often. |
| // | ||
| // Events Neru posted itself already returned above via the 0x1337 marker | ||
| // check, so `action left_click` cannot trigger these. | ||
| if (type == kCGEventLeftMouseDown || type == kCGEventRightMouseDown || type == kCGEventOtherMouseDown) { |
There was a problem hiding this comment.
I think we have to also consider what happens when held_repeat.enabled is enabled. Looking at it, I think it will repeat the run forever until the mode is idle, probably not something expected. Maybe a guard in key_dispatch.go is needed around the repeat?
| // fire — the silent failure a mode binding exists to avoid. Refusing | ||
| // keeps the behavior these keys already had before the vocabulary | ||
| // carried them, so no configuration that loads today stops loading. | ||
| if keyvocab.IsMouseButton(key) { |
There was a problem hiding this comment.
Looks like this test the whole key, i think the validation might get skipped if a user binded it as Cmd+MouseLeft or some sort. Worth a double check.
| if (type == kCGEventLeftMouseDown || type == kCGEventRightMouseDown || type == kCGEventOtherMouseDown) { | ||
| NSString *buttonName = nil; | ||
| if (type == kCGEventLeftMouseDown) { | ||
| buttonName = @"MouseLeft"; |
There was a problem hiding this comment.
Referring to the literals of mouse left, right and middle, i think we should add them into the keyvocab nativeKeyEventEmitters for consistency, as that's part of the guardrail to prevent weird cases in the future.
| /// Marks events Neru posts itself, so the mode event tap can tell its own | ||
| /// synthetic clicks apart from ones the user physically produced. Must match | ||
| /// the marker in keyfeed_darwin.m and the check in eventTapCallback. | ||
| static const int neruSyntheticMouseEventMarker = 0x1337; |
There was a problem hiding this comment.
i think moving this to accessibility_constants.h probably make more sense.
| // IsMouseButton reports whether name is one of the mouse buttons, | ||
| // case-insensitively. | ||
| func IsMouseButton(name string) bool { | ||
| lowered := strings.ToLower(name) |
There was a problem hiding this comment.
we already have lowercase helper down below, we probably can just use them for consistency.
| // listing them in a diagnostic reads the declaration rather than restating it. | ||
| func MouseButtons() []string { | ||
| buttons := slices.Clone(mouseButtons) | ||
| slices.Sort(buttons) |
There was a problem hiding this comment.
i think sorting this will fight against the comment where it should be reads the declaration no? We will need to pick between either keep sort and rewrite the comments, or another way round.
| reaching the application underneath, so the click still does whatever it | ||
| normally would. | ||
| - **Modifiers are not encoded.** `Cmd`-click and `Shift`-click both report | ||
| `MouseLeft`; there is no `Cmd+MouseLeft`. |
There was a problem hiding this comment.
there is no Cmd+MouseLeft, this is true, but we got no validation to reject this also, i think a rephrase is needed.
|
|
||
| See [CLI.md](CLI.md#neru-action-feed) for a full key reference with key codes and platform behavior. | ||
|
|
||
| **Mouse buttons** are reported only while a mode is active, so they belong in a |
There was a problem hiding this comment.
Since we have some info about the refusal, i think it's worth to tell the user that the eventtap (mouse clicks in this case) will be ignored when entered the search input mode, probably not alot of users using that, but worth a message i think.
2 participants
Description
This PR adds the physical mouse buttons to the key vocabulary, so a mode can bind them the way it binds a key. Until now the mode event tap watched keyboard events only, so a click from a real mouse — or from a keyboard key that sends one — was invisible to the active mode: the click landed, the overlay stayed up, and dismissing it took a second keystroke.
A mode now observes a button press without consuming it, so the binding runs in addition to the click reaching the application underneath. Clicks Neru performs itself are marked and ignored, so a binding cannot recurse into its own action. Modifiers are deliberately not encoded into the name — a modified click reports the button alone — because a mode that wants to react to a click wants to react however the click was modified.
Only the macOS tap reports these today; the buttons stay in the shared vocabulary the way
Insertand F21–F24 do, so one config file still validates on every platform.Related Issues
Closes #1417
Target Platform
Type of Change
feat— New featurefix— Bug fixrefactor— Code restructuring (no behavior change)perf— Performance improvementdocs— Documentation onlytest— Adding or updating testschore— Build, CI, dependencies, toolingCross-Platform Checklist
//go:build darwin) — the new code sits in existing_darwin.msources, which carry no Go build tag of their ownCodeNotSupported) — N/A, nothing new is added to a port; the Linux and Windows taps simply never emit these names, the same shape asInsertand F21–F24General Checklist
just fmt)just cipasses — the exact checks CI runs (format check, lint, vet,tests including
-race, vulnerability scan, build)Config surface
Three new named keys, valid in any mode's hotkey table:
MouseLeftMouseRightMouseMiddleThey are reported only while a mode is active, so they belong in a mode table. Writing one in the global
[hotkeys]table is refused with an error naming the mode tables instead — a global hotkey resolves to a virtual key code and a mouse button has none, so accepting it would produce a binding that parses and never fires.No existing option changes, no default changes, and every configuration that loads today still loads: these spellings were already refused as an invalid key format before the vocabulary carried them, so the global-table refusal preserves the behavior they already had.
Additional Context
Why bindable keys rather than a setting. A boolean such as
exit_on_external_clickwould answer only the one question, could not express right- or middle-click, and would add a config option. Making the buttons bindable adds none, composes with macros, and works in every mode.Telling a physical click from Neru's own. The tap already ignores events Neru posts, by a marker the keyboard feed sets. The mouse path was not setting it; it is now set in the single helper every synthetic mouse event is posted through, which is what keeps
action left_clickfrom firing aMouseLeftbinding.Middle button. Only button number 2 is named
MouseMiddle; buttons 3 and up stay unbindable rather than being folded onto the middle button.Why macOS only. Not a platform limitation, just a different amount of work, so this PR stays scoped to the platform it can verify. The macOS tap is session-wide and was already receiving these events and discarding them by mask, so widening the mask was the change. The evdev capture is device-scoped to keyboards — a device qualifies by reporting Q/W/E/R/Space/Enter, which a mouse does not — so buttons would need a new device class to enumerate and hotplug-track, and a non-grabbing read path, since the existing capture is an exclusive
EVIOCGRABand a button must not be consumed. X11 would need raw XInput2 button events on top of that, and Windows a second low-level hook beside the keyboard one. Happy to follow up on any of these if they are wanted.Testing.
just cipasses locally, and so do the containerized cross-platform runs:just test-linux(CGO on),just test-linux-nocgo,just lint-crossandjust test-windows-compile.The Linux run is what caught the one real cross-platform consequence.
TestX11KeysymFor_EveryNamedKeyResolvesasserts every named key resolves to an X11 keysym, and a mouse button has none — the three new keys failed it. That test is taggedlinux && cgo, so a macOS-onlyjust cidoes not compile it. Mouse buttons are now skipped there with the reason recorded: the invariant is about a key a grab could reject, and a button never reaches a grab. Verified in both directions — the three fail without the skip and pass with it.Beyond the suites, the three buttons were exercised by hand on macOS 15.7.4 against recursive grid: each dismissed the overlay while the click still reached the application underneath.