Allow modifier keys in accept shortcuts (⇧Tab, ⌥Tab, …)#341
Conversation
Acceptance shortcuts could only be bare keys: pressing ⇧Tab or ⌥Tab silently fell through to the text-mutation branch because `InputMonitor` required `noModifiers` before classifying a press as `.acceptance` / `.fullAcceptance`. The recorder also stored only the key code, dropping any held modifiers on the floor — so even if a user wanted ⇧Tab, there was no way to express it. Issue #326 asked for this directly, and the non-English-keyboard symptom in that thread shared the same root cause: keys whose natural muscle memory includes a modifier had no representation. Add a normalized 4-bit `ShortcutModifierMask` (⌘⇧⌥⌃) and thread it through storage, the event tap, and the recorder. `SuggestionSettingsModel` gains two parallel `*Modifiers` properties stored under new UserDefaults keys; absent values default to `[]` so existing single-key bindings keep working exactly as before. `InputMonitor.classify` and the active accept tap now mask `event.flags` to those four bits and require strict equality with the bound mask — `Tab` and `⇧Tab` are distinct bindings and may coexist. `KeyRecorderView` captures modifiers via `.flagsChanged` with a live preview, and falls back to `event.characters` then a physical-position description so dead keys (`^` on German QWERTZ, etc.) no longer render as `"Key 10"`. Plain Escape still cancels the recorder; with a modifier held, Escape becomes a bindable shortcut. The acceptance-hint pill above ghost text reads from the saved label, so users who pick ⌥Tab see `"⌥ Tab"` taught back to them. Fixes #326
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let label = KeyCodeLabels.label( | ||
| for: keyCode, | ||
| modifiers: modifiers, | ||
| // Use `characters` ahead of `charactersIgnoringModifiers` so we still get a readable | ||
| // glyph for keys whose first level is a dead key (e.g. `^` on German QWERTZ). When | ||
| // both are empty, `KeyCodeLabels` falls back to a physical-position description. | ||
| fallback: bestCharacterFallback(for: event) | ||
| ) |
There was a problem hiding this comment.
characters ahead of charactersIgnoringModifiers", but the implementation checks charactersIgnoringModifiers first and only falls back to characters. The code logic is correct for the dead-key use case (you want the unmodified character when available), but the comment contradicts it and could mislead the next person maintaining this function.
| let label = KeyCodeLabels.label( | |
| for: keyCode, | |
| modifiers: modifiers, | |
| // Use `characters` ahead of `charactersIgnoringModifiers` so we still get a readable | |
| // glyph for keys whose first level is a dead key (e.g. `^` on German QWERTZ). When | |
| // both are empty, `KeyCodeLabels` falls back to a physical-position description. | |
| fallback: bestCharacterFallback(for: event) | |
| ) | |
| let label = KeyCodeLabels.label( | |
| for: keyCode, | |
| modifiers: modifiers, | |
| // Prefer `charactersIgnoringModifiers` for a stable key label; fall back to | |
| // `characters` for keys whose unmodified level is a dead key (e.g. `^` on German | |
| // QWERTZ). When both are empty, `KeyCodeLabels` falls back to a physical-position | |
| // description. | |
| fallback: bestCharacterFallback(for: event) | |
| ) |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| if event.type == .flagsChanged { | ||
| liveModifiers = ShortcutModifierMask(nsEventFlags: event.modifierFlags) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
flagsChanged events consumed while recording
The handler returns nil for every flagsChanged event, which drops those events from the local responder chain for the entire Cotabby app while recording is active. SwiftUI's own environment modifier tracking (\.eventModifiers) relies on flagsChanged to stay current, so any view in the app that conditionally adjusts its appearance on modifier state won't update until the recorder disappears and the monitor is removed. The fix is straightforward: pass the flagsChanged event through after reading the modifiers from it.
1 participant
Summary
Adds modifier-key support to the word-accept and full-accept shortcuts, so users can bind combinations like ⇧Tab, ⌥Tab, or ⌃Space instead of only bare keys. Also fixes the related non-English-keyboard symptom in #326 where dead keys above Tab rendered as
"Key 10"in the recorder.Validation
Local
testexecution hits the documented Team ID / signing mismatch (CLAUDE.md flags this as an environment issue, not a code failure);build-for-testingis the fallback as described there. Did not run the app end-to-end in a browser since this is a global keyboard-tap change; the build outputs sign and link cleanly.Linked issues
Fixes #326
Risk / rollout notes
cotabbyAcceptanceKeyModifiers,cotabbyFullAcceptanceKeyModifiers) default to0when absent, so existing installs keep their bare-Tab / bare-backtick bindings without prompting. No reset needed.(keyCode, modifiers)tuple, soTaband⇧Tabcan coexist (the old code would have cleared one when the other was assigned to the same key code). Worth a mental note for reviewers.⌘Vto accept will intercept paste while a suggestion is showing. The recorder's docstring already acknowledges this for bare keys; modifiers don't change the shape, only the surface. macOS-reserved combos (⌘Tab, ⌘Space) are still bindable but won't fire in practice — no warning added for this v1.SuggestionAvailabilityEvaluatorTestshad asetAcceptanceKey(keyCode:label:)call that neededmodifiers:added. Worth a follow-up to add round-trip storage tests forShortcutModifierMaskand glyph-composition tests forKeyCodeLabels.label(for:modifiers:fallback:).Greptile Summary
This PR extends Cotabby's word-accept and full-accept shortcuts to support modifier-key combinations (⇧Tab, ⌥Tab, ⌃Space, etc.) and fixes the non-English keyboard label issue (#326) where dead keys above Tab displayed as
\"Key 10\".ShortcutModifierMask(a 4-bitOptionSet) to normalizeCGEventFlags/NSEvent.ModifierFlags, and threads it through settings storage, theInputMonitorevent tap, theKeyRecorderView(with a live modifier preview via.flagsChanged), and both UI flows (Settings + Welcome).(keyCode, modifiers)tuple, soTaband⇧Tabcan coexist as distinct bindings; migration is invisible since the newcotabbyAcceptanceKeyModifiers/cotabbyFullAcceptanceKeyModifiersUserDefaults keys default to0.KeyCodeLabelsgainsphysicalKeyDescriptionsfor ISO/JIS dead-key positions and a newlabel(for:modifiers:fallback:)overload that composes modifier glyphs in macOS convention order (⌃⌥⇧⌘).Confidence Score: 4/5
Safe to merge; the core shortcut-matching and persistence logic is well-structured with no functional defects.
The KeyRecorderView returns nil for flagsChanged events while recording, which silently drops those events from the rest of the app's responder chain. The mismatched comment in bestCharacterFallback also slightly increases maintenance risk for the dead-key handling path. Neither issue blocks correctness today, but they are worth cleaning up before the recorder sees wider use.
Cotabby/UI/KeyRecorderView.swift — the flagsChanged consumption and comment mismatch both live here.
Important Files Changed
Sequence Diagram
sequenceDiagram participant U as User participant KRV as KeyRecorderView participant SSM as SuggestionSettingsModel participant UD as UserDefaults participant IM as InputMonitor (CGEvent tap) U->>KRV: Hold modifier key KRV->>KRV: flagsChanged → liveModifiers updated U->>KRV: Press key (e.g. Tab) KRV->>KRV: build ShortcutModifierMask from NSEvent.modifierFlags KRV->>KRV: bestCharacterFallback → label via KeyCodeLabels KRV->>SSM: setAcceptanceKey(keyCode, modifiers, label) SSM->>SSM: normalise modifiers ([] if disabledKeyCode) SSM->>SSM: "conflict-clear if (keyCode, modifiers) == fullAccept binding" SSM->>UD: persist keyCode + modifiers.rawValue + label note over IM: At keypress time IM->>SSM: acceptanceKeyCodeProvider() IM->>SSM: acceptanceKeyModifiersProvider() IM->>IM: ShortcutModifierMask(eventFlags:) from CGEvent.flags alt exact (keyCode, modifiers) match IM-->>U: return nil (consume event) else no match IM-->>U: passUnretained (event passes through) endReviews (1): Last reviewed commit: "Allow modifier keys in accept shortcuts ..." | Re-trigger Greptile