Skip to content

Add per-app shortcut overrides#764

Open
t-h-tech wants to merge 3 commits into
FuJacob:mainfrom
t-h-tech:feat/per-app-shortcuts
Open

Add per-app shortcut overrides#764
t-h-tech wants to merge 3 commits into
FuJacob:mainfrom
t-h-tech:feat/per-app-shortcuts

Conversation

@t-h-tech

@t-h-tech t-h-tech commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Per-app shortcut overrides: let users set a different accept / full-accept shortcut per application — or disable Cotabby's accept key for specific apps. Bindings resolve at keystroke time against the frontmost app and fall back to the global binding when no override matches.

Validation

xcodebuild test -project Cotabby.xcodeproj -scheme Cotabby \
  -destination 'platform=macOS' CODE_SIGNING_ALLOWED=NO
# ** TEST SUCCEEDED **   1652 tests, 0 failures
#   incl. ShortcutResolverTests, PerAppShortcutOverrideStoreTests, and the new
#   per-app cases in ShortcutConflictTests

swiftlint lint --config .swiftlint.yml --quiet
# exit 0

xcodegen generate && git diff --exit-code -- Cotabby.xcodeproj
# clean — committed project matches project.yml (5 new files wired in)

Linked issues

None.

Risk / rollout notes

  • Additive / opt-in. No behavior change unless a user adds an override. Storage is a new cotabbyPerAppShortcutOverrides UserDefaults key (JSON array), modeled exactly on the existing disabledAppRules pattern; the key is wired into load, the unconditional write-back, resetToDefaults, and allPreferenceDefaultsKeys.
  • InputMonitor is unchanged — resolution rides the existing accept-key provider closures, which are rewired in CotabbyAppEnvironment to resolve through ShortcutResolver against the live frontmost bundle id. Those four assignments were moved to just after focusModel is constructed (they capture it weakly).
  • Cotabby.xcodeproj/project.pbxproj was regenerated with xcodegen generate to wire in the 5 new files — no signing/team or other changes.
  • Includes the 2-line init build fix from Fix build: initialize fadeIn properties in SuggestionSettingsModel.init #763 (fadeInSuggestions / fadeInDurationSeconds) so this branch compiles against the currently-broken main. Once Fix build: initialize fadeIn properties in SuggestionSettingsModel.init #763 lands I'll rebase and those two lines drop out of this diff.

Greptile Summary

This PR adds per-app accept/full-accept shortcut overrides: users can assign a different key per application or disable acceptance entirely in a specific app. Bindings are resolved at keystroke time through a new ShortcutResolver against the frontmost bundle id, falling back to the global binding when no override exists.

  • ShortcutResolver (new pure function) and PerAppShortcutOverride (new model) introduce the data model and resolution logic; both are well-tested and the nil-means-inherit design cleanly separates "no override" from "disabled".
  • SuggestionSettingsStore gains load/save/sanitize for overrides following the existing disabledAppRules discipline; bindingsNormalized is correctly called before the isEmpty guard, closing the phantom-row window.
  • InputMonitor providers are consolidated from four closures into two paired-binding closures, eliminating the prior per-keystroke split-read race and halving the number of resolver scans per event.

Confidence Score: 4/5

Safe to merge for the common case; one conflict-detection gap lets a per-app override silently shadow the effective global binding for the other action, causing the wrong acceptance event to fire.

The resolver, model, store, and InputMonitor wiring are solid. The gap is in conflictingPerAppShortcutName: it only inspects explicitly stored per-app override fields for the other action, never falling back to the global binding when no per-app override is present. A user who records a per-app accept key equal to the global full-accept key (or vice versa) gets no warning, and at event time the wrong action fires because full-accept always takes priority.

Cotabby/Models/SuggestionSettingsModel.swift — the conflictingPerAppShortcutName function and its corresponding test in CotabbyTests/ShortcutConflictTests.swift need a case covering global-fallback collision.

Important Files Changed

Filename Overview
Cotabby/Support/ShortcutResolver.swift New pure resolver that applies per-app → global precedence; clean and well-tested.
Cotabby/Models/PerAppShortcutOverride.swift New model with correct nil-means-inherit semantics and three-field atomicity via bindingsNormalized.
Cotabby/Models/SuggestionSettingsModel.swift Adds per-app CRUD and conflict detection, but conflictingPerAppShortcutName checks only stored per-app overrides for the other action, missing global-fallback effective bindings.
Cotabby/Support/SuggestionSettingsStore.swift Adds load/save/sanitize for per-app overrides, mirroring disabledAppRules discipline; bindingsNormalized called before isEmpty check resolves prior review concern.
Cotabby/UI/Settings/Panes/AppsPaneView.swift New per-app override section with correct inherit/override state toggling; uses NSOpenPanel for app selection.
Cotabby/App/Core/CotabbyAppEnvironment.swift Rewires accept providers to unified acceptanceBindingProvider/fullAcceptanceBindingProvider closures that resolve through ShortcutResolver; captures focusModel weakly.
Cotabby/Services/Input/InputMonitor.swift Collapses four separate provider closures into two binding-pair providers; keeps event-classification logic identical.
Cotabby/UI/Settings/Components/KeybindRow.swift Extracted and promoted to shared component; adds .help() on Clear button; no logic changes vs the previous private struct.
CotabbyTests/ShortcutResolverTests.swift Thorough coverage of resolver precedence, nil bundle id, disabled-sentinel, and cross-app isolation.
CotabbyTests/PerAppShortcutOverrideStoreTests.swift Good coverage of load/save round-trip, partial-binding collapse, deduplication, and sanitization.
CotabbyTests/ShortcutConflictTests.swift Tests cross-app non-conflict and same-app collision, but does not cover the global-fallback collision scenario flagged in this review.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant User as User Keystroke
    participant IM as InputMonitor
    participant Env as CotabbyAppEnvironment
    participant SR as ShortcutResolver
    participant FM as FocusTrackingModel
    participant SSM as SuggestionSettingsModel

    User->>IM: key event (keyCode, flags)
    IM->>Env: acceptanceBindingProvider()
    Env->>FM: snapshot.bundleIdentifier
    FM-->>Env: com.example.app
    Env->>SSM: perAppShortcutOverrides
    SSM-->>Env: [PerAppShortcutOverride]
    Env->>SR: "acceptBinding(bundleId, overrides, globalKey*)"
    SR-->>Env: ResolvedBinding(keyCode, modifiers, label)
    Env-->>IM: (keyCode, modifiers)
    IM->>Env: fullAcceptanceBindingProvider()
    Env->>SR: "fullAcceptBinding(bundleId, overrides, globalKey*)"
    SR-->>Env: ResolvedBinding(keyCode, modifiers, label)
    Env-->>IM: (keyCode, modifiers)
    IM->>IM: compare event vs resolved bindings
    IM-->>User: .acceptance / .fullAcceptance / nil
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant User as User Keystroke
    participant IM as InputMonitor
    participant Env as CotabbyAppEnvironment
    participant SR as ShortcutResolver
    participant FM as FocusTrackingModel
    participant SSM as SuggestionSettingsModel

    User->>IM: key event (keyCode, flags)
    IM->>Env: acceptanceBindingProvider()
    Env->>FM: snapshot.bundleIdentifier
    FM-->>Env: com.example.app
    Env->>SSM: perAppShortcutOverrides
    SSM-->>Env: [PerAppShortcutOverride]
    Env->>SR: "acceptBinding(bundleId, overrides, globalKey*)"
    SR-->>Env: ResolvedBinding(keyCode, modifiers, label)
    Env-->>IM: (keyCode, modifiers)
    IM->>Env: fullAcceptanceBindingProvider()
    Env->>SR: "fullAcceptBinding(bundleId, overrides, globalKey*)"
    SR-->>Env: ResolvedBinding(keyCode, modifiers, label)
    Env-->>IM: (keyCode, modifiers)
    IM->>IM: compare event vs resolved bindings
    IM-->>User: .acceptance / .fullAcceptance / nil
Loading

Comments Outside Diff (1)

  1. Cotabby/Models/SuggestionSettingsModel.swift, line 783-801 (link)

    P1 Ghost-text hint shows wrong key when a per-app override is active

    acceptanceHintLabel and emojiPickerAcceptKeyLabel both read the global acceptanceKeyCode / acceptanceKeyLabel, so a user who sets a different accept key for a specific app (or who uses the disabled-sentinel to suppress the accept key in that app) will still see the global key label in the ghost-text overlay. The worst case is the "disabled" override: the hint keeps showing e.g. "Tab" while Tab no longer does anything in that app, making Cotabby look broken.

    Both properties need to resolve through ShortcutResolver against the current frontmost bundle id, the same way the InputMonitor providers do.

    Fix in Codex Fix in Claude Code

Reviews (3): Last reviewed commit: "Merge origin/main into feat/per-app-shor..." | Re-trigger Greptile

Let users override Cotabby's accept / full-accept shortcuts on a per-application
basis (or disable them for an app). Bindings are resolved at keystroke time
against the frontmost app, falling back to the global binding when no override
matches.

- New PerAppShortcutOverride model + ShortcutResolver (per-app -> global).
- Storage re-homed into the SuggestionSettingsData / SuggestionSettingsStore /
  facade pattern, mirroring disabledAppRules: a new "cotabbyPerAppShortcutOverrides"
  UserDefaults key, store-routed accessors, sanitize/sort, and conflict detection.
- Settings UI: a "Per-App Shortcuts" section in the Apps pane (app picker +
  KeybindRow); the shared KeybindRow is promoted out of ShortcutsPaneView into a
  reusable component.
- Resolver wired through CotabbyAppEnvironment's accept-key providers (moved below
  focusModel construction so they read the live frontmost snapshot); InputMonitor
  unchanged.

Adds ShortcutResolverTests, PerAppShortcutOverrideStoreTests, and per-app
ShortcutConflictTests.
Comment thread Cotabby/UI/Settings/Panes/AppsPaneView.swift
Comment thread Cotabby/App/Core/CotabbyAppEnvironment.swift Outdated
Comment thread Cotabby/Support/SuggestionSettingsStore.swift Outdated
- Sanitizer now collapses a partially-specified per-app binding (a key code
  without its modifiers/label) to "inherit global" on load, so a phantom row
  can't survive load yet silently never fire in ShortcutResolver. Adds
  PerAppShortcutOverride.bindingsNormalized + a store test.
- Replace the four separate accept/full-accept key+modifier provider closures
  with two combined providers returning (keyCode, modifiers), so each binding
  resolves once as a unit per keystroke — no redundant scans, and the key code
  and modifiers can't come from different resolutions. Updates InputMonitor,
  the CotabbyAppEnvironment wiring, and InputMonitorTests.
@t-h-tech

Copy link
Copy Markdown
Contributor Author

Thanks for the review! Pushed 3c545c8 addressing two of these, and a note on the third:

#3 — sanitizer atomicity (fixed). Added PerAppShortcutOverride.bindingsNormalized, applied in sanitizedPerAppShortcutOverrides on load: any partially-specified binding (a key code without its modifiers/label) now collapses back to "inherit global" before the empty check, so a phantom row can't survive a load. Added test_sanitize_collapsesPartialBindingOnLoad. The setters always write all three fields together, so in practice this only hardens against a corrupted/hand-edited default — but you're right that the stored shape should match exactly what ShortcutResolver honors.

#2 — resolver called 4× / consistency window (fixed). Replaced the four separate acceptance*Provider / fullAcceptance*Provider closures with two combined providers — acceptanceBindingProvider / fullAcceptanceBindingProvider returning (keyCode, modifiers). acceptanceKind now resolves each binding once, as a unit (2 scans instead of 4, and the key code and modifiers can no longer come from different resolutions). Updated InputMonitor, the CotabbyAppEnvironment wiring, and the InputMonitorTests call sites.

#1 — seeding the accept key on "Add App…" (intentional, but happy to revisit). A per-app override row exists only to hold a concrete binding; an all-nil row is indistinguishable from "no override" and the sanitizer drops it on the next load. So "Add App…" seeds the current global accept key to give a concrete, editable starting point — without it, the freshly-added row would just vanish on reload. The downside you flagged is real: the row then stops inheriting future global accept-key changes, with no "inherits global" indicator. The clean fix would be to persist named-but-unbound rows and render an explicit "inherits global (click to set)" state — a slightly larger UX change I left out to keep this PR focused. Glad to add it here or as a follow-up if you'd prefer that behavior.

@FuJacob

FuJacob commented Jun 29, 2026

Copy link
Copy Markdown
Owner

Thanks for the work @t-h-tech!! Will review this soon.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants