fix(keyevent): tear down old event tap on rebuild, cancel lone-modifier gesture on chords, post keyUp for media-key remaps#122
Merged
Conversation
…er gesture on chords, post keyUp for media-key remaps - setupCGEventTap() now fully tears down the previous CGEvent tap (disable, remove run-loop source, invalidate mach port) before creating a new one, and deinit does the same. Previously the CFMachPort and its run-loop source were never released across a rebuild, so every display reconfiguration left the old tap alive and registered on the main run loop, doubling keystroke processing and leaking mach ports. Fixes #107. - modifierKeyDown/modifierKeyUp now track the set of currently-held modifier keyCodes (`downModifierKeyCodes`) instead of relying solely on the single tracked-keyCode slot. modifierKeyDown arms the lone-press gesture only when the incoming key is the sole held modifier; any other case (a chord, or a re-press while a sibling modifier is still held) cancels the gesture. This closes a residual hole in a plain "cancel on second keyDown" fix: holding Command_L, tapping Command_R once (cancels, correct), then tapping Command_R again while Command_L is still held used to re-arm the lone-press slot with 54 — since the slot alone can't distinguish "nothing else is held" from "something else is still held but its slot got cancelled" — and falsely fire on release. - mediaKeyUp now mirrors mediaKeyDown: resolves the mapping for the media keyCode and, on remap, posts a synthesized key-up CGEvent for the mapped output key instead of passing the raw event through unchanged, so remapped media keys no longer get stuck down in the frontmost app. Adds unit test coverage for the modifier chord/lone-press gesture tracking (previously uncovered, now including the held-modifier-set regression) and for the mediaKeyUp remap/pass-through paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Merged
kazukinakai
added a commit
that referenced
this pull request
Jul 6, 2026
…trip residual flags from the synthesized tap Regression from the 2.4.11 chord-cancel (#122): holding Shift (or any other modifier) and tapping ⌘ no longer switched the input source, breaking "commit kana conversion with left ⌘". Three defects in the tap gesture path: - modifierKeyDown cancelled the lone-press gesture whenever a second modifier was physically held, so every Shift/Option/Control+⌘ tap was a no-op. Replaced the single tracked-keyCode slot + downModifierKeyCodes chord-cancel with per-modifier pending taps (`pendingModifierTaps`): a modifier release fires its mapping iff nothing but modifier activity happened since its own press, regardless of press/release order. This also removes the inconsistent re-arm hole #122 was closing — taps now fire consistently instead of never. - The synthesized Eisu/Kana event inherited residual held-modifier flags (convertedEvent keeps event.flags), so even when a tap fired with Shift held the IME saw Shift+英数 and ignored it. modifierKeyUp now posts the bare output shortcut (output keyCode + output flags only) through a postModifierTap seam (injectable for tests). - keyUp of a regular key unconditionally cancelled the pending gesture, so pressing ⌘ to commit while the last typed character key was still physically depressed (routine in fast typing) misfired. Only keyDown, mouse events, and media keys cancel now. Reworks the modifier-gesture unit tests around the new semantics and the capture seam: taps with Shift held (both orders), independent sibling taps, keyDown-cancel, keyUp-no-cancel, disable-mapping, and residual-flag stripping. Release 2.5.0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three hot-path fixes in
KeyEvent.swift:setupCGEventTap()never invalidated the previousCFMachPortnor removed its run-loop source before overwritingeventTap, so every rebuild (display reconfiguration via fix(keyevent): recover event tap after external monitor connect/disconnect (#107) #111's handler, or a failed heartbeat re-enable) left the old tap alive on the main run loop: each keystroke got processed once per accumulated tap (duplicate IME toggles, reinjected events) plus a mach-port leak. NewtearDownEventTap()(disable → remove run-loop source →CFMachPortInvalidate) runs at the top ofsetupCGEventTap()and indeinit, ordered before the oldtapObserverrelease since the old callback holds it as refcon.flagsChangedoverwrote, so holding Left ⌘ and tapping Right ⌘ fired the kana switch mid-chord and lost the original gesture. Now adownModifierKeyCodesset tracks physically-held modifiers and the gesture only arms when exactly one modifier is down — covering the re-press-while-still-held case too.mediaKeyUppassed the raw NX_SYSDEFINED event through without ever posting a keyUp for the remapped output key, leaving it stuck down in the frontmost app. It now mirrorsmediaKeyDownand posts the matching synthesized keyUp.Tests
swift test: 39 executed, 0 failures (1 pre-existing unrelated skip). Adds first-ever coverage formodifierKeyDown/modifierKeyUp(lone press, chord cancel, re-press regression, keyDown-cancels-gesture) and formediaKeyUpremap/pass-through. The tap-rebuild fix itself is not unit-tested — CGEvent tap creation cannot run headless.Known trade-off: tearing the old tap down before
CGEvent.tapCreatemeans a rebuild whose create fails leaves no tap during the retry window (previously the old half-dead tap lingered). The retry loop + heartbeat recover as before.🤖 Generated with Claude Code