Fix: make Select Workspace 1…9 / Surface 1…9 shortcuts actually rebindable (#5189)

Merged
AU
4mo ago
+13407-8341

Summary

Fixes #5189 — Select Workspace 1…9 (default ⌘1–9) could not be rebound: after remapping it in Settings → Keyboard Shortcuts, the old ⌘1–9 kept selecting workspaces, so the binding behaved as if hardcoded.

Root cause

cmux owns the numbered shortcuts through KeyboardShortcutSettings, and every cmux-owned path already honors a rebind:

  • the recorder / conflict detection normalizes and persists the new modifier,
  • the keyDown router (numberedConfiguredShortcutDigit) reads the live configured value,
  • the View menu rebuilds its key-equivalents on change and shouldSuppressStaleCmuxMenuShortcut suppresses any stale ⌘1–9 menu item.

The one path that was not driven by the configured value is Ghostty. Ghostty's built-in super+1…8 = goto_tab / super+9 = last_tab keybinds (ghostty/src/config/Config.zig) were never unbound. cmux already unbinds super+d / super+w for its split/close shortcuts in loadCmuxOwnedGhosttyKeybindOverrides — the numbered keys were simply missed.

So after a user remapped ⌘1–9 away, cmux stopped consuming ⌘1–9 and forwarded the event to the focused terminal (AppDelegate.sendEvent → ghosttyView.keyDown(with:)), where Ghostty still performed goto_tab. This is exactly why only the numbered shortcuts were reported broken — they were the only cmux-owned shortcuts colliding with a still-live Ghostty default.

Fix

Unbind Ghostty's super+1…9 (both the Unicode super+1 and physical super+digit_1 forms it registers) in loadCmuxOwnedGhosttyKeybindOverrides, mirroring the existing super+d / super+w pattern. KeyboardShortcutSettings becomes the single source of truth for the numbered shortcut. The default ⌘1–9 still works because cmux's default config binds it through the remappable layer (router + View menu), not Ghostty. Rebind to ⌥1–9 and ⌘1–9 goes inert; ⌥1–9 works.

Surface ⌃1–9 needs no Ghostty change — Ghostty binds super, not ctrl, so that shortcut was already cmux-only.

Tests (two-commit red/green)

  1. Commit 1 (failing): testGhosttyConfigDoesNotRetainNumberedGotoTabFallback asserts GhosttyApp.shared.config does not bind super+1…9, using the same forward ghostty_config_key_is_binding check as the existing super+w close-surface-fallback test. Fails without the fix.
  2. Commit 2 (fix): adds the unbind (test goes green) plus testRebindingSelectWorkspaceByNumberHonorsNewModifierAndDropsDefault, a router-level test confirming a rebound ⌃⌥1 selects the workspace while the old ⌘1 no longer routes through cmux.

🤖 Generated with Claude Code

View with Codesmith Autofix with Codesmith
Need help on this PR? Tag @codesmith with what you need. Autofix is disabled.

[!NOTE]
Medium Risk
Touches global key routing, menu key equivalents, Ghostty config, and system-wide hotkey registration; mistakes could swallow shortcuts or miss conflicts, but behavior is heavily tested and conflict detection is conservative.

Overview
Fixes #5189 so Select Workspace / Surface 1…9 can be rebound without Ghostty still handling ⌘1–9: cmux now unbinds Ghostty’s super+1…9 / super+digit_N goto_tab fallbacks alongside the existing split/close unbinds, leaving KeyboardShortcutSettings as the only dispatcher.

The PR also generalizes keyboard shortcut gating with a VS Code–style shortcuts.when layer in CmuxSettings: parse/evaluate predicates over ShortcutContext (focus atoms plus keys like commandPaletteVisible, sidebarMode, paneCount), persist overrides via shortcuts.when, and use ShortcutWhenClause.canCoexist so the app router, menu equivalents, chord arming, and Settings conflict UI treat context-disjoint bindings as non-colliding.

Settings / recording polish: shared ShortcutDisplayFormatter and 1…9 range display; numbered actions require digits 1–9 and normalize to "1"; RecorderHostButton.isActivelyRecording plus notifications so app routing, stale menu suppression, Carbon global hotkeys, and Text Box shortcuts stand down while the package recorder is armed. New catalog actions/defaults (workspace grouping, Ctrl-F to terminal, diff-viewer navigation/chords, etc.) and expanded default-stroke tables round out the change.

Reviewed by Cursor Bugbot for commit 5da4c5a0ab33c509207cd7bc5e6970295e84f70e. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by cubic

Fixes #5189 by making Select Workspace 1…9 and Surface 1…9 truly rebindable and context-aware, with priority-aware conflict handling so sidebar mode keys and digit families coexist. Settings now shows each binding’s scope with clear captions.

  • Bug Fixes

    • Unbound Ghostty super+1…9 (Unicode and digit_N) so old ⌘1…9 goes inert after a rebind.
    • Enforced shortcuts.when across routing, chords, right‑sidebar mode, menu key equivalents, and Text Box; empty = “always”; boolean literals supported; menus drop only when a non‑empty override exists.
    • Treated same‑stroke pairs resolved by router priority (e.g. sidebar mode vs Select Surface 1…9) as coexisting; fully shadowed bindings still report conflicts.
    • Recorder stand‑down: app/router, all menus (incl. stale‑menu suppression), system‑wide hotkeys, and Text Box shortcuts yield while the CmuxSettingsUI recorder is active and on teardown.
    • Numbered actions now require digits 1–9, normalize to “1”, and detect family conflicts; the 1–9 banner is localized and clears only when that action’s binding changes.

  • Refactors

    • Generalized shortcuts.when in CmuxSettings to typed context keys (commandPaletteVisible, terminalFindVisible, sidebarMode, paneCount, workspaceCount) and operators (==, !=, =~, <, <=, >, >=, in [a, b]); added priority‑aware bindingsCollide/canCoexist used across the app and CmuxSettingsUI.
    • Settings polish: stream shortcuts.when overrides, skip context‑disjoint conflicts, render scope captions (“When: …” or built‑in focus scope), and share a formatter for chords and 1…9 ranges; new actions/defaults (workspace grouping, toggle group collapse, sendCtrlFToTerminal, diff viewer open/scroll incl. gg); docs/schema and all locales updated; focused tests added.

Summary by CodeRabbit

  • New Features

    • Shortcut UI shows numbered-digit actions with a clear "1…9" range hint, unified formatter for chord/keystroke display, and "None" for unbound shortcuts; recorder placeholders and conflict banners use this display.

  • Bug Fixes

    • Numbered workspace shortcuts (1–9) no longer conflict with built-in fallbacks; workspace selection routing remains correct after rebinding.

  • Tests

    • Added regression and formatter tests covering numbered-digit display and shortcut routing after rebinding.

Update — VS Code-style when-clause expansion

This branch now also generalizes the shortcuts.when engine introduced for #5189 from four boolean focus atoms toward VS Code's when-clause vocabulary, while staying fully backwards compatible (existing clauses parse and evaluate identically).

Operators — added ==, !=, =~ (regex), <, <=, >, >=, and in [a, b], layered into the grammar with VS Code precedence (a && b == c → a && (b == c)).

Context keys — beyond the focus atoms (sidebarFocus/browserFocus/markdownFocus/terminalFocus), a typed registry (ShortcutContextKnownKey) now exposes:

  • commandPaletteVisible, terminalFindVisible (bool)
  • sidebarMode (string: files/find/sessions/feed/dock)
  • paneCount, workspaceCount (int)

e.g. { "selectSurfaceByNumber": "sidebarMode == 'find' && paneCount > 1" }.

Design

  • New Sendable value types in CmuxSettings (ShortcutContext, ShortcutContextValue, ShortcutContextOperand, ShortcutRegex, ShortcutContextKnownKey); the app populates a context snapshot at key dispatch, so the package never imports app types.
  • ShortcutWhenClause stays additive (.key/.compare added; .atom and the evaluate(ShortcutFocusState) overload unchanged). One intentional change: an unknown bare key now parses to .key (always-false), matching VS Code.
  • canCoexist (conflict detection) generalized to a sound free-variable enumeration — byte-identical for focus-only clauses, conservative for typed comparisons (never hides a real conflict).
  • Also treats an empty/whitespace shortcuts.when clause as non-restricting so it no longer suppresses an action's menu equivalent.

Tests / docs — new CmuxSettingsTests/ShortcutWhenClauseTests.swift (Swift Testing, 20 cases) + an app integration test through the settings file path; cmux.schema.json and the en/ja message catalogs updated. inputFocus and terminalHasSelection are deferred follow-ups (registry-trimmed, not yet wired).

Update — main merge, VS Code literal parity, full locale sync

  • Merged origin/main — conflicts were all 20 web/messages/*.json (this branch's schemaDescriptions.shortcuts.when key vs main's new diffViewer key, both appended at the same spot); resolved keeping both.
  • Boolean literals (VS Code parity): when clauses now accept bare true / false, and boolean-literal equality folds onto the key itself — key == true ≡ key, key == false ≡ !key (inverted for !=), mirroring VS Code's ContextKeyExpr.has/not normalization. Previously bare true parsed to an unknown key and evaluated false (the opposite of VS Code), and key == true silently never matched. Folding keeps canCoexist conflict detection exact. New cases in ShortcutWhenClauseTests.
  • Locale sync: the expanded when-clause schema description (typed keys + comparison operators) was only translated in en/ja — the other 18 catalogs still described the four focus atoms. All 20 locales now carry the full description plus the boolean-literal sentence (also in the inline en copy in cmux.schema.json). The hand-written shortcuts.when section in the configuration docs page predated the typed-key expansion and was hardcoded English; it now matches the shipped engine and is localized via new docs.configuration.shortcutsWhen* message keys (key/operator tokens stay as <code> in JSX outside the translated strings). The shortcut display strings the package recorder renders (shortcut.key.*, shortcut.unbound.displayValue) were filled from en/ja(+ar) to all 20 xcstrings locales.