Repository navigation
Custom keyboard shortcuts for Command Palette commands - #6906
Closed
austinywang wants to merge 45 commits into
Closed
austinywang wants to merge 45 commits into
austinywang wants to merge 45 commits into
Conversation
Adds a "Custom Commands" section under Settings > Keyboard Shortcuts that lets users bind a single keystroke to any Command Palette command that ships without one (e.g. "Open Folder in VS Code"). Bindings persist under `shortcuts.commands` in cmux.json and fire on the focused window. - shortcuts.commands map: parsed by KeyboardShortcutSettingsFileStore (single-stroke only; chords rejected) and exposed via KeyboardShortcutSettings.commandShortcuts(). - Dispatch: handleCustomShortcut matches command shortcuts last so a built-in action always wins a shared keystroke, then fires the command by id on the focused window through a new .commandPaletteRunCommandRequested notification. A command gated out by its when/enablement clause is a silent no-op; the event is consumed on any binding match. - Settings UI: an "Add Shortcut" button opens a fuzzy command picker that reuses the Command Palette's own ranking engine (via a SettingsHostActions bridge) and a single-stroke recorder. Recording a keystroke already used by a built-in action or another command is blocked with a banner. - Built-in palette contributions are extracted to a shared static source (ContentView.builtInCommandPaletteCommandContributions) so the picker catalog and the live palette never drift. - Schema (shortcuts.commands), configuration + keyboard-shortcuts docs, en/ja localization, and parse/dispatch/conflict tests. Fixes #6431 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds persisted ChangesCustom command shortcuts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 2 warnings)
✅ Passed checks (21 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Greptile SummaryThis PR implements custom keyboard shortcuts for Command Palette commands, letting users bind a single keystroke (with a primary modifier) to any palette command without a built-in shortcut — addressing issue #6431. Bindings are stored under
Confidence Score: 5/5Safe to merge — all dispatch, persistence, conflict-detection, and Settings-UI paths are well-tested and follow existing codebase conventions. The change is a well-scoped additive feature with no modifications to existing hotpaths. The O(1) modifier-mask dispatch, memoized catalog/search-engine, lossless per-entry JSON mutation, and notification-based run-command path are all correctly implemented. Conflict detection reads fresh effective bindings on each record attempt, preventing stale-snapshot shadowing of same-session rebinds. The static builtInCommandPaletteCommandContributions() refactor is clean, and all callers correctly switched to Self.. Tests cover parse validation, end-to-end dispatch notification, and index capping. No regressions to existing shortcut routing, palette, or persistence paths were found. No files require special attention. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant AppDelegate
participant CmuxSettingsFileStore
participant NotificationCenter
participant ContentView
Note over User,ContentView: Dispatch Flow
User->>AppDelegate: Press custom shortcut (e.g. ⌘⌃O)
AppDelegate->>CmuxSettingsFileStore: commandShortcutsMatchingModifierMask(mask)
CmuxSettingsFileStore-->>AppDelegate: [(commandId, shortcut)] from indexed bucket
AppDelegate->>AppDelegate: matchConfiguredShortcut(event, shortcut)
AppDelegate->>NotificationCenter: post(.commandPaletteRunCommandRequested)
NotificationCenter->>ContentView: onReceive
ContentView->>ContentView: runCommandPaletteCommandById
alt Command available
ContentView->>ContentView: runCommandPaletteCommand
else Gated out
ContentView->>ContentView: silent no-op
end
Note over User,ContentView: Settings Binding Flow
User->>CustomCommandShortcutsCard: Click Add Shortcut
CustomCommandShortcutsCard->>HostSettingsActions: searchCommandShortcutCatalog [memoized]
HostSettingsActions-->>CustomCommandShortcutsCard: [CommandShortcutCatalogEntry]
User->>CustomCommandShortcutsCard: Select command + record keystroke
CustomCommandShortcutsCard->>CommandShortcutConflictChecker: conflictLabel(stroke, excludingCommandId)
CommandShortcutConflictChecker-->>CustomCommandShortcutsCard: nil
CustomCommandShortcutsCard->>JSONConfigStore: setMapEntry(shortcut, forKey: commandId)
JSONConfigStore-->>CmuxSettingsFileStore: file change detected → reload
CmuxSettingsFileStore->>CmuxSettingsFileStore: makeCommandShortcutDispatchIndex
%%{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
participant AppDelegate
participant CmuxSettingsFileStore
participant NotificationCenter
participant ContentView
Note over User,ContentView: Dispatch Flow
User->>AppDelegate: Press custom shortcut (e.g. ⌘⌃O)
AppDelegate->>CmuxSettingsFileStore: commandShortcutsMatchingModifierMask(mask)
CmuxSettingsFileStore-->>AppDelegate: [(commandId, shortcut)] from indexed bucket
AppDelegate->>AppDelegate: matchConfiguredShortcut(event, shortcut)
AppDelegate->>NotificationCenter: post(.commandPaletteRunCommandRequested)
NotificationCenter->>ContentView: onReceive
ContentView->>ContentView: runCommandPaletteCommandById
alt Command available
ContentView->>ContentView: runCommandPaletteCommand
else Gated out
ContentView->>ContentView: silent no-op
end
Note over User,ContentView: Settings Binding Flow
User->>CustomCommandShortcutsCard: Click Add Shortcut
CustomCommandShortcutsCard->>HostSettingsActions: searchCommandShortcutCatalog [memoized]
HostSettingsActions-->>CustomCommandShortcutsCard: [CommandShortcutCatalogEntry]
User->>CustomCommandShortcutsCard: Select command + record keystroke
CustomCommandShortcutsCard->>CommandShortcutConflictChecker: conflictLabel(stroke, excludingCommandId)
CommandShortcutConflictChecker-->>CustomCommandShortcutsCard: nil
CustomCommandShortcutsCard->>JSONConfigStore: setMapEntry(shortcut, forKey: commandId)
JSONConfigStore-->>CmuxSettingsFileStore: file change detected → reload
CmuxSettingsFileStore->>CmuxSettingsFileStore: makeCommandShortcutDispatchIndex
Reviews (39): Last reviewed commit: "Address command shortcut review feedback" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+212
to
+220
| } | ||
| if let conflictMessage { | ||
| let format = String( | ||
| localized: "shortcut.recorder.error.conflictsWithAction", | ||
| defaultValue: "This shortcut conflicts with %@ (%@)." | ||
| ) | ||
| return String.localizedStringWithFormat(format, conflictMessage, "") | ||
| } | ||
| return nil |
Contributor
There was a problem hiding this comment.
()
shortcut.recorder.error.conflictsWithAction has two %@ placeholders — the original call site in KeyboardShortcutSettingsControls.swift fills them with the conflicting action's display name and its keystroke string (e.g. "Command Palette" + "⌘P"). Here the second argument is hardcoded to "", so every conflict banner reads "This shortcut conflicts with Command Palette ()." — the trailing () is always visible and always wrong. The same issue appears in CustomCommandShortcutsCard.swift at its validationMessage closure.
Contributor
Author
There was a problem hiding this comment.
Fixed: the banner now uses a single-placeholder shortcut.recorder.error.conflictsWithBinding (en + ja) in both CommandShortcutPickerSheet and CustomCommandShortcutsCard, so the empty () is gone.
— Claude Code
Contributor
Author
There was a problem hiding this comment.
Already resolved: both CommandShortcutPickerSheet and CustomCommandShortcutsCard now use the single-placeholder shortcut.recorder.error.conflictsWithBinding ("This shortcut conflicts with %@."), so the empty () is gone.
— Claude Code
Comment on lines
+348
to
+370
| } | ||
|
|
||
| /// Bridges the app's built-in Command Palette command metadata into the | ||
| /// Foundation-only ``CommandShortcutCatalogEntry`` values the Settings | ||
| /// **Custom Commands** section consumes, and ranks them with the Command | ||
| /// Palette's own search engine. | ||
| /// | ||
| /// The catalog is derived from | ||
| /// ``ContentView/builtInCommandPaletteCommandContributions()`` — the same | ||
| /// single source of truth the live palette uses — evaluated against a neutral | ||
| /// context so every bindable command appears with a generic title regardless of | ||
| /// the current window's focus. Config-derived `actions` are intentionally | ||
| /// excluded: those already support a `shortcut` field directly in cmux.json, so | ||
| /// surfacing them here would offer two ways to bind the same thing. | ||
| @MainActor | ||
| enum CommandShortcutCatalog { | ||
| /// All built-in palette commands a custom shortcut can target, in a stable | ||
| /// display order (deduplicated by command id, keeping the first occurrence). | ||
| static func entries() -> [CommandShortcutCatalogEntry] { | ||
| let neutralContext = CommandPaletteContextSnapshot() | ||
| var seen = Set<String>() | ||
| var entries: [CommandShortcutCatalogEntry] = [] | ||
| for contribution in ContentView.builtInCommandPaletteCommandContributions() { |
Contributor
There was a problem hiding this comment.
CommandShortcutCatalog is a caseless enum whose entire API is two static funcs — exactly the pattern the cmux no-ambient-global-state rule flags. The two functions form a cohesive bridge between ContentView's command contributions and the settings package; the right shape is a struct or final class that is constructed with a ContentView-derived contributions closure and injected into HostSettingsActions (or simply inlined as private helpers on HostSettingsActions itself, since the type already owns both search methods).
Rule Used: Flag new ambient global state in production Swift:... (source)
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!
Contributor
Author
There was a problem hiding this comment.
Addressed: the CommandShortcutCatalog static namespace was folded into HostSettingsActions instance methods, so the caseless-enum-as-namespace is gone.
— Claude Code
Comment on lines
+317
to
+355
| .cmuxFont(.caption) | ||
| .foregroundStyle(.red) | ||
| .fixedSize(horizontal: false, vertical: true) | ||
| Button(String(localized: "shortcut.recorder.undo", defaultValue: "Undo")) { | ||
| onUndo() | ||
| } | ||
| .buttonStyle(.link) | ||
| .cmuxFont(.caption) | ||
| } | ||
| .padding(.horizontal, 8) | ||
| .padding(.vertical, 6) | ||
| .frame(maxWidth: .infinity, alignment: .leading) | ||
| .background { | ||
| RoundedRectangle(cornerRadius: 6).fill(Color.red.opacity(0.12)) | ||
| } | ||
| .overlay { | ||
| RoundedRectangle(cornerRadius: 6).stroke(Color.red.opacity(0.35), lineWidth: 1) | ||
| } | ||
| .accessibilityIdentifier("ShortcutRecorderValidationMessage") | ||
| } | ||
| } | ||
|
|
||
| /// The label of the first existing binding a proposed command shortcut collides | ||
| /// with — a built-in action's display name or another command's title — or | ||
| /// `nil` when the keystroke is free. Built-in actions are checked with | ||
| /// numbered-digit family semantics so binding `⌃5` is blocked by the `⌃1…9` | ||
| /// workspace selector. The proposed command stroke is never a numbered family. | ||
| @MainActor | ||
| func commandShortcutConflictLabel( | ||
| stroke: StoredShortcut, | ||
| excludingCommandId: String?, | ||
| actionBindings: [String: StoredShortcut], | ||
| commandShortcuts: [String: StoredShortcut], | ||
| title: (String) -> String | ||
| ) -> String? { | ||
| for action in ShortcutAction.allCases { | ||
| let effective = actionBindings[action.rawValue] ?? action.defaultShortcut | ||
| guard let effective, !effective.isUnbound else { continue } | ||
| if numberedAwareStrokesConflict( |
Contributor
There was a problem hiding this comment.
internal function used as module API
commandShortcutConflictLabel is a file-scope function with default (internal) visibility — it is called directly from CustomCommandShortcutsCard, from CommandShortcutPickerSheet, and from the test suite via @testable import CmuxSettingsUI. The cmux no-ambient-global-state rule flags top-level functions used as API when a method on a constructable type is the right shape. The logic belongs on a value type (e.g. a CommandShortcutConflictChecker struct, or as a static func on CustomCommandShortcutsCard) so it is not ambient internal surface of the module.
Rule Used: Flag new ambient global state in production Swift:... (source)
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!
Contributor
Author
There was a problem hiding this comment.
Fixed: commandShortcutConflictLabel is now a CommandShortcutConflictChecker value type (constructed with the bindings + title resolver); the card, sheet, and tests call .conflictLabel(stroke:excludingCommandId:) on an instance instead of a top-level function.
— Claude Code
Comment on lines
+385
to
+408
|
|
||
| /// Ranks ``entries()`` for `query` using ``CommandPaletteSearchEngine`` — the | ||
| /// exact ranking engine the Command Palette uses — returning at most `limit` | ||
| /// entries best-first. An empty query yields the default order capped to | ||
| /// `limit`. | ||
| static func search(query: String, limit: Int) -> [CommandShortcutCatalogEntry] { | ||
| let entries = entries() | ||
| let trimmed = query.trimmingCharacters(in: .whitespacesAndNewlines) | ||
| if trimmed.isEmpty { | ||
| return limit >= 0 ? Array(entries.prefix(limit)) : entries | ||
| } | ||
| let corpus = entries.enumerated().map { index, entry in | ||
| CommandPaletteSearchCorpusEntry( | ||
| payload: entry, | ||
| rank: index, | ||
| title: entry.title, | ||
| searchableTexts: [entry.title, entry.subtitle] + entry.keywords | ||
| ) | ||
| } | ||
| let engine = CommandPaletteSearchEngine(entries: corpus) | ||
| let results = engine.search( | ||
| query: trimmed, | ||
| resultLimit: limit >= 0 ? limit : nil, | ||
| historyBoost: { _, _ in 0 } |
Contributor
There was a problem hiding this comment.
entries() and CommandPaletteSearchEngine re-allocated on every keystroke
search(query:limit:) calls entries() unconditionally, and entries() calls ContentView.builtInCommandPaletteCommandContributions() each time — rebuilding the full contribution list, evaluating every closure against a neutral context, and allocating a new [CommandShortcutCatalogEntry]. Then search additionally constructs a fresh CommandPaletteSearchEngine from scratch per query. Both happen on every character the user types in the picker's search field. The catalog is static (same commands every time), so entries() could be computed once and memoised; the search engine corpus could similarly be built once and retained for the picker session rather than per-call.
Contributor
Author
There was a problem hiding this comment.
Fixed: HostSettingsActions now memoizes both the catalog entries and the prepared CommandPaletteSearchEngine (the catalog is static for the process lifetime — locale changes restart the app), so per-keystroke search no longer rebuilds the corpus.
— Claude Code
Contributor
Author
There was a problem hiding this comment.
Already resolved: HostSettingsActions memoizes both the catalog entries (cachedCommandShortcutCatalog) and the prepared CommandPaletteSearchEngine (cachedCommandShortcutSearchEngine), so the per-keystroke search no longer rebuilds the corpus.
— Claude Code
…board-shortcut-for-command-palette # Conflicts: # Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Environment/SettingsHostActions.swift
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codex review found two issues:
P2 — The Settings Custom Commands card read and wrote shortcuts.commands
through the package's typed `[String: StoredShortcut]` decode, which only
understands the object form. A user following the docs and writing the
string form (`"palette.newWindow": "cmd+ctrl+n"`) would have the whole map
fail to decode (Dictionary decode is all-or-nothing), and the card's full
`set(_:)` write would then clobber those valid bindings.
- Add `JSONConfigStore.setMapEntry(_:forKey:in:)`: a per-entry,
raw-preserving merge that mutates only the touched key and leaves every
sibling's on-disk form intact. The card now uses it for add/remove.
- Add `SettingsHostActions.commandShortcuts()` so the card reads the bound
map through the app's lenient parser (string/object/null) instead of the
package's object-only decode. Display, conflict detection, and the
write-base all use it; edits update local state optimistically.
- Tests: JSONConfigStore setMapEntry preserves a string-form sibling while
adding an object-form entry, and prunes the map when emptied.
P3 — The new shortcuts.commands configuration-docs section was hard-coded
English under the localized [locale] route. Move it into the
docs.configuration message catalog (en + ja) via t.rich, matching the
sibling shortcuts.when section.
Also fold the CommandShortcutCatalog static namespace into HostSettingsActions
instance methods (avoids a caseless-namespace policy finding).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…banner fix Codex + Greptile found: P2 (Codex) — HostSettingsActions.commandShortcuts() returned the app's flat `StoredShortcut`, not the package `CmuxSettings.StoredShortcut` the protocol requires, so it was not the protocol witness and the Settings card silently got the empty default — bound shortcuts vanished and conflict detection was bypassed. Qualify the return type and bridge app→package via a new `packageStoredShortcut(from:)` (the two Codable shapes differ, so no JSON round-trip). P2 (Codex) — Built-in conflict detection read overrides from the package's object-only `shortcuts.bindings` decode (all-or-nothing); one string-form override blanked the map and let a command shadow a rebound built-in. Add `SettingsHostActions.effectiveActionShortcuts()` (host, lenient, every action's effective binding incl. unbound markers) and use it as the conflict base. P1 (Greptile) — Conflict banner always rendered an empty "()": the two-`%@` `conflictsWithAction` format was filled with one real arg + "". Use a new single-placeholder `shortcut.recorder.error.conflictsWithBinding` (en + ja). P2 (Greptile) — `commandShortcutConflictLabel` was an ambient top-level function; refactor to a `CommandShortcutConflictChecker` value type. P2 (Greptile) — Catalog + search engine were rebuilt per keystroke; memoize both on the host (the catalog is static for the process lifetime). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
P1 (Codex) — JSONConfigStore.setMapEntry took `Any?`, so a non-Sendable existential crossed the actor boundary (rejected by Swift 6 strict concurrency, breaking the build). Take the typed `Value?` (Sendable & SettingCodable, like set(_:for:)) and call encodeForJSON() inside the actor; sibling entries are only ever handled as raw JSON within the method and never cross the boundary. P2 (Codex) — firstMatchingCommandShortcut filtered + sorted the command-shortcut dictionary on every key event (terminal typing hot path). Cache a prefiltered, id-ordered snapshot in KeyboardShortcutSettingsFileStore, recomputed once per reload, and iterate that cached array on the hot path (single lock-guarded COW fetch, no per-keystroke filter/sort). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… side effect
P2 (Codex) — Settings persists shortcuts.commands in the package object form
(`{ "first": { ... } }`), but the new schema only allowed string/null, so a
shortcut created from Settings could fail cmux.json schema/editor validation.
Extend the schema with a precise single-stroke object binding
(`singleStrokeObjectBinding` + `shortcutStrokeObject`) matching exactly what the
recorder writes; the string form stays the documented hand-edit shape and chords
remain rejected.
P2 (Codex) — requestCommandPaletteRunCommand cleared focused browser focus mode
before ContentView checked command availability, so pressing a context-gated
(unavailable) command shortcut still mutated focus/keyboard routing. Drop the
up-front focus-mode reset; a gated-out command is now a true no-op and any focus
change is left to the command's own action when it actually runs.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…lict checks P2 (Codex) — the Custom Commands card snapshotted effectiveActionShortcuts() on appear, so a built-in shortcut rebound in the same Keyboard Shortcuts session (via the per-action recorder rows on the same page) wasn't reflected: a command could be assigned a keystroke just given to a built-in, or stay blocked on a just-freed key, until the view was reopened. Read the built-in bindings fresh in conflictChecker() (only on a record attempt, so cheap) and drop the stale actionBindings snapshot. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
P2 (Codex) — some bindable commands (rename workspace, edit description, rename tab) drive an in-palette input flow by switching commandPaletteMode to an input mode that only renders while the palette is presented. The direct shortcut runner executed their handler without showing the palette, so the keystroke was consumed but no rename/description UI appeared and the command couldn't complete. These commands are exactly the `dismissOnRun == false` contributions (they keep the palette open to host the flow). runCommandPaletteCommandById now presents the palette first for those before running the handler, so a custom shortcut bound to them behaves like selecting them from the palette; self-contained commands still run directly with no palette UI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
P2 (Codex) — parseCommandShortcutValue let a bare Space through (parseConfig's allowBareFirstStroke=false still permits Space, and the object branch repeated a `key == "space"` exception). A hand-edited `"palette.x": "space"` would then make the dispatcher consume ordinary Space key events away from terminal/browser text input. The Settings recorder already requires a modifier and the docs say so — enforce it uniformly in the parser: reject any first stroke without ⌘ ⌥ ⌃ or ⇧ (while still accepting cmd+space). Test covers bare-space rejection + modifier+space. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… error) P1 (Codex) — the cmuxTests parse assertions used `newWindow.first` / `modifierSpace.first`, but KeyboardShortcutSettings.commandShortcuts() returns the app target's flat StoredShortcut whose primary stroke is `firstStroke` (`.first` is the CmuxSettings package type's property). That wouldn't compile, blocking the whole cmuxTests target. Switch the assertions to `.firstStroke`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…heck P2 (Codex) — the custom-command conflict checker compared only against built-in KeyboardShortcutSettings actions and other command shortcuts, not user-defined cmux.json `actions` that carry their own `shortcut`. Those are dispatched by the key router *before* the new command-shortcut path, so a command bound to a keystroke a configured action owns would be accepted in Settings yet never fire at runtime. Add SettingsHostActions.configuredActionShortcuts() (host reads cmuxConfigStore.shortcutActions() and bridges to the package StoredShortcut) and fold it into CommandShortcutConflictChecker. Comparisons use the first stroke, so a command bound to a configured chord's prefix is also blocked. Tests cover the configured-action and chord-prefix cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…a title map P2 (Codex) — keying configuredActionShortcuts by the action title dropped same-titled actions with different shortcuts, so the conflict checker could miss one and allow a dead command binding. Return a `(label, shortcut)` list (titles are free-form display labels, not unique keys) so every configured action's shortcut is checked; the title is used only as the displayed conflict label. Test covers duplicate titles. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/app/`[locale]/docs/configuration/page.tsx:
- Around line 504-518: The docs page uses t.rich("shortcutsCommandsIntro", ...)
in the configuration section, but that translation key is missing from every
locale file under web/messages, so the content cannot render. Add
shortcutsCommandsIntro to all supported locale JSON files, including the English
base messages, and ensure the key matches the usage in page.tsx with the code
and settings rich-text placeholders.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1c3a6ca0-ffd6-4401-b99c-ca3e7862f98a
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (22)
Packages/macOS/CmuxSettings/Sources/CmuxSettings/Keys/KeyboardShortcutsCatalogSection.swiftPackages/macOS/CmuxSettings/Sources/CmuxSettings/Stores/JSONConfigStore.swiftPackages/macOS/CmuxSettings/Tests/CmuxSettingsTests/JSONConfigStoreTests.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Environment/SettingsHostActions.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/CommandShortcutPickerSheet.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/CustomCommandShortcutsCard.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/KeyboardShortcutsSection.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Values/CommandShortcutCatalogEntry.swiftPackages/macOS/CmuxSettingsUI/Tests/CmuxSettingsUITests/CommandShortcutConflictTests.swiftResources/Localizable.xcstringsSources/AppDelegate.swiftSources/ContentView.swiftSources/HostSettingsActions.swiftSources/KeyboardShortcutSettingsFileStore.swiftSources/KeyboardShortcutSettingsLookup.swiftSources/TabManager.swiftcmuxTests/CommandPaletteShortcutCustomizationTests.swiftweb/app/[locale]/docs/configuration/page.tsxweb/app/[locale]/docs/keyboard-shortcuts/page.tsxweb/data/cmux.schema.jsonweb/messages/en.jsonweb/messages/ja.json
Aziz policy findings from the autoreview gate: - Document the new public CommandShortcutCatalogEntry members and the SettingsHostActions font-size default impls (Swift-DocC triple-slash). - File-organization: split ShortcutValidationBanner and CommandShortcutConflictChecker out of CustomCommandShortcutsCard.swift into their own files (one major type per file). - Move the default searchCommandShortcutCatalog substring filter into a file-private helper so the public extension method body stays declaration-free. The remaining test-framework finding (XCTest in CommandPaletteShortcutCustomizationTests) is consciously kept: the two new tests were added to that existing XCTest class and reuse its window/event harness; moving only them to Swift Testing would split one behavior suite. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…cuts P3 (Codex) — the shortcuts.commands schema reused shortcutFirstStroke, which allows a bare Space, but parseCommandShortcutValue rejects any modifier-less first stroke (including space). So `"palette.foo": "space"` passed schema/editor validation yet was silently ignored at reload. Add a command-specific commandShortcutFirstStroke (shortcutStroke + a required modifier prefix, no bare Space) for the string form, and require at least one modifier on the object form's stroke, matching the runtime parser. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/data/cmux.schema.json (1)
1483-1489: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a
descriptionKeyfor the newshortcuts.commandsschema copy.This description is user-facing, but unlike neighboring schema fields it has no
descriptionKey, so the docs/localized schema path has no way to provide non-English coverage for it.Suggested schema change
"commands": { "type": "object", "default": {}, + "descriptionKey": "schemaDescriptions.shortcuts.commands", "description": "Custom single-stroke keyboard shortcuts for Command Palette commands, keyed by command id (for example \"palette.openFolderInVSCodeInline\"). The shortcut fires the command on the focused window; a command unavailable in the current context does nothing. Single-stroke only — chords are not supported here. Use null, an empty string, none, clear, unbound, or disabled to clear a binding. Manage these in Settings > Keyboard Shortcuts > Custom Commands.", "additionalProperties": { "$ref": "`#/`$defs/singleShortcutBindingNullable" } }Then add the matching entries in
web/messages/en.jsonandweb/messages/ja.json. As per path instructions,web/data/**/*.jsonmust "verify user-facing schema titles, descriptions, and data copy have matching localized message coverage for every locale listed in web/i18n/routing.ts". Based on learnings,descriptionKeyhere is web-only documentation metadata and should be wired throughweb/messages/<locale>.json, notResources/Localizable.xcstrings.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/data/cmux.schema.json` around lines 1483 - 1489, The new shortcuts.commands schema copy is user-facing but lacks a descriptionKey, so it cannot be localized like neighboring schema fields. Update the relevant schema entry in cmux.schema.json to add a descriptionKey for commands, then add matching message keys and translations in web/messages/en.json and web/messages/ja.json. Keep the wiring in the web schema/message path, using the existing schema copy and localization patterns around related shortcut fields as a guide.Sources: Path instructions, Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@web/data/cmux.schema.json`:
- Around line 1483-1489: The new shortcuts.commands schema copy is user-facing
but lacks a descriptionKey, so it cannot be localized like neighboring schema
fields. Update the relevant schema entry in cmux.schema.json to add a
descriptionKey for commands, then add matching message keys and translations in
web/messages/en.json and web/messages/ja.json. Keep the wiring in the web
schema/message path, using the existing schema copy and localization patterns
around related shortcut fields as a guide.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 99c81387-6e26-4ae9-9fba-953aaea9449b
📒 Files selected for processing (1)
web/data/cmux.schema.json
…vive no window P2 (Codex) — the Custom Commands Unbind button deleted the shortcuts.commands entry. Since the settings resolver merges shortcuts.commands from fallback config files (fillMissingSettings), a fallback-inherited shortcut would reappear on the next reload, leaving the binding active. Write the documented "none" unbind marker instead (parsed as unbound, suppresses the fallback, and schema-valid — unlike the unbound object form, which the new modifier requirement now rejects). P3 (Codex) — configuredActionShortcuts() returned [] when no main window context existed (Settings open after the last window closed), so the picker could accept a command shortcut that a user actions[].shortcut already owns. Fall back to a standalone CmuxConfigStore().loadAll() (global actions) so the conflict check still runs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
P2 (Codex) — runCommandPaletteCommandById resolved commands through the cached palette context, whose terminal open-target availability only refreshes on a palette search refresh; a context-gated command shortcut (e.g. VS Code serve-web stop/restart) could no-op until the palette was opened. Build a fresh commands context (resolveCommandPaletteTerminalOpenTargets) before filtering by when/enablement. P2 (Codex) — configuredActionShortcuts() used a single window's config store, but stores merge local+global config and runtime dispatch checks the focused window's store; with multiple project windows Settings could miss a configured action's shortcut from another window. Aggregate shortcutActions() across every live config store (dedup by id+keystroke), keeping the transient global store only when no window is live. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ound 3 regression) P1 (Codex) — extracting builtInCommandPaletteCommandContributions() to `static` (round 3) left it calling instance-only helpers, so the app target would not compile. Codex flagged commandPaletteExtensionSidebarCommandID / commandPaletteWorkspaceColorCommandID; an exhaustive scan also found appendIdentifierCopyCommandContributions and appendMoveTabToNewWorkspaceCommandContribution called from the same static body. All four are pure metadata/id builders, so make them `static` and qualify the call sites with `Self.`. No bare instance-property accesses remain in the static contributions builder. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…cuts P2 (Codex) — command shortcuts fire in the app-level key monitor before the terminal/browser sees the key, so a shift-only printable binding like ⇧Y would run the command every time the user types an uppercase Y. The modifier check accepted shift alone. Require a primary modifier (⌘/⌥/⌃) — shift is no longer sufficient — across all surfaces: - parser (parseCommandShortcutValue), - recorder validation (card + picker sheet reject shift-only with a command-specific banner), - schema (string form needs a primary-modifier prefix; object form needs command/option/control true), - help text + configuration docs. Test covers shift-only rejection alongside bare Space. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The docs.configuration namespace is fully translated across all 20 locales, so the new shortcutsCommandsIntro must be too (it was en + ja only). Add real translations for the remaining 18 locales, preserving the exact <code>/<settings> tag structure and code literals (validated programmatically). (CodeRabbit's "missing from all locales" check looked for a top-level key; the key is correctly nested under docs.configuration — the configuration page's useTranslations namespace — and rendered via the en fallback meanwhile.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add docs.keyboardShortcuts.customCommandsTitle/Intro/Callout to the 18 locales that previously only had en+ja, matching the all-20 coverage the configuration-page shortcutsCommandsIntro key already has. Tag structure in customCommandsIntro (<settings>/<settingsFile>/<configurationLink>) validated identical to en across every locale. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/CommandShortcutPickerSheet.swift (1)
194-197: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemove ⇧ from the recorder help text.
Line 196 still tells users Shift is valid, but Lines 226-230 now reject shift-only shortcuts. This makes the UI contradict the enforced primary-modifier rule.
Suggested fix
Text(String( localized: "settings.customCommands.record.help", - defaultValue: "Press a single keystroke that includes ⌘, ⌥, ⌃, or ⇧." + defaultValue: "Press a single keystroke that includes ⌘, ⌥, or ⌃." ))Also applies to: 223-230
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/CommandShortcutPickerSheet.swift` around lines 194 - 197, Update the recorder help text in CommandShortcutPickerSheet so it no longer tells users that ⇧ is an allowed modifier; the message shown by the Text localization should only mention the primary modifiers that are actually accepted. Then align the shortcut validation logic in the recording/acceptance path around the relevant shortcut handling methods so the UI text and enforced rules match, especially where shift-only or shift-inclusive shortcuts are rejected.Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/CustomCommandShortcutsCard.swift (1)
199-207: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winCache catalog lookups before sorting bound rows.
sortedBoundCommandIdsis used frombody, and each comparison callstitle(for:), which linearly scanscatalogEntries. With many custom bindings/catalog entries this becomes repeated full scans during SwiftUI rendering. Keep an id→entry/title dictionary when loading the catalog. As per path instructions, avoid “full rescans in nested loops on scalable collections.”Suggested fix
`@State` private var catalogEntries: [CommandShortcutCatalogEntry] = [] + `@State` private var catalogEntriesById: [String: CommandShortcutCatalogEntry] = [:] @@ private func title(for commandId: String) -> String { - catalogEntries.first { $0.commandId == commandId }?.title ?? commandId + catalogEntriesById[commandId]?.title ?? commandId } @@ private func reloadCatalogIfNeeded() { if catalogEntries.isEmpty { - catalogEntries = hostActions.commandShortcutCatalog() + let entries = hostActions.commandShortcutCatalog() + catalogEntries = entries + catalogEntriesById = entries.reduce(into: [:]) { result, entry in + result[entry.commandId] = entry + } } }Also applies to: 223-226
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/CustomCommandShortcutsCard.swift` around lines 199 - 207, The sortedBoundCommandIds sorting path is repeatedly calling title(for:) and linearly scanning catalogEntries during SwiftUI rendering. Add a cached id-to-title or id-to-entry lookup alongside the catalog load in CustomCommandShortcutsCard, then make sortedBoundCommandIds use that cache instead of calling title(for:) inside the comparator; keep title(for:) as a simple fallback if needed and update any other referenced uses in the same area.Source: Path instructions
web/messages/tr.json (1)
967-1032: 🛠️ Refactor suggestion | 🟠 MajorAdd missing
keyboardShortcuts.customCommands*translations to all supported localesThe PR introduces three new user-facing keys under
docs.keyboardShortcuts(customCommandsTitle,customCommandsIntro,customCommandsCallout) to support the new Custom Commands UI. These keys are currently only defined inen.jsonandja.json.Per the project's full internationalization policy, all 18 other supported locales must have matching entries before the PR can merge. Affected files require additions for:
ar,zh-CN,zh-TW,ko,de,es,fr,it,da,pl,ru,bs,no,pt-BR,th,tr,km,ukWithout these, users of these locales will silently see raw English text in the Settings → Keyboard Shortcuts page.
Required action:
- Add the three missing keys to every
web/messages/<locale>.jsonfile listed above.- Translate the content appropriately; do not leave English fallbacks or placeholder text.
- Ensure the translations match the semantic meaning of the English source (single-stroke shortcut assignment, blocking of existing bindings, context-aware execution).
Example structure to add:
"customCommandsTitle": "...", "customCommandsIntro": "...", "customCommandsCallout": "..."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/messages/tr.json` around lines 967 - 1032, The keyboard shortcuts localization is missing the new custom commands strings in this locale. Add the `keyboardShortcuts.customCommandsTitle`, `keyboardShortcuts.customCommandsIntro`, and `keyboardShortcuts.customCommandsCallout` entries to this translation file, matching the existing `keyboardShortcuts` structure in `tr.json`, and make sure the wording is fully translated and consistent with the new Custom Commands UI.Source: Path instructions
web/data/cmux.schema.json (1)
1567-1578: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAllow the recorder's object-form clear sentinel here.
parseCommandShortcutValue(_:)inSources/KeyboardShortcutSettingsFileStore.swifttreats{ "first": { "key": "" } }as an explicit unbind, butshortcutStrokeObjectnow requirescommand,option, orcontrolto betrue. If Settings writes its existing empty-first sentinel for a cleared custom command shortcut, the saved config becomes schema-invalid even though the app accepts it. Please either admit that sentinel insingleShortcutBindingNullableor serialize clears asnullconsistently.Also applies to: 1580-1639
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/data/cmux.schema.json` around lines 1567 - 1578, The schema for single-stroke command bindings currently rejects the recorder’s clear sentinel, so update the binding definition to accept the object-form empty-first unbind used by parseCommandShortcutValue(_:) in Sources/KeyboardShortcutSettingsFileStore.swift, or change the recorder/store to serialize clears as null everywhere. Adjust the relevant shortcut binding schema symbols such as singleShortcutBindingNullable and shortcutStrokeObject so a cleared custom command shortcut remains valid when Settings writes { "first": { "key": "" } }.Sources/KeyboardShortcutSettingsFileStore.swift (1)
181-185: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject duplicate command-shortcut collisions instead of ordering them.
Manual
cmux.jsonedits can bind the same keystroke to two command ids, and this code keeps both entries. Downstream,AppDelegate.firstMatchingCommandShortcut(event:)walks the ordered snapshot and fires whichever command id sorts first, so shortcut routing becomes heuristic instead of having one authoritative target. Ambiguous bindings should be dropped or marked invalid so the shortcut fails closed. As per path instructions,.github/review-bot-rules/reliability-single-source-of-truth.mdsays correctness-critical routing should fail closed rather than guess.Also applies to: 1014-1020
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/KeyboardShortcutSettingsFileStore.swift` around lines 181 - 185, The shortcut snapshot building in KeyboardShortcutSettingsFileStore is currently preserving duplicate keystroke collisions by ordering them, which makes routing in AppDelegate.firstMatchingCommandShortcut(event:) heuristic instead of authoritative. Update the logic around commandShortcutsByCommandId and commandShortcutsOrderedSnapshot to detect when multiple command ids resolve to the same keystroke, then drop or invalidate the ambiguous bindings so the shortcut fails closed rather than keeping both entries. Ensure the resolved snapshot only contains unambiguous command-shortcut mappings and that collisions do not reach downstream matching.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/CommandShortcutPickerSheet.swift`:
- Around line 194-197: Update the recorder help text in
CommandShortcutPickerSheet so it no longer tells users that ⇧ is an allowed
modifier; the message shown by the Text localization should only mention the
primary modifiers that are actually accepted. Then align the shortcut validation
logic in the recording/acceptance path around the relevant shortcut handling
methods so the UI text and enforced rules match, especially where shift-only or
shift-inclusive shortcuts are rejected.
In
`@Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/CustomCommandShortcutsCard.swift`:
- Around line 199-207: The sortedBoundCommandIds sorting path is repeatedly
calling title(for:) and linearly scanning catalogEntries during SwiftUI
rendering. Add a cached id-to-title or id-to-entry lookup alongside the catalog
load in CustomCommandShortcutsCard, then make sortedBoundCommandIds use that
cache instead of calling title(for:) inside the comparator; keep title(for:) as
a simple fallback if needed and update any other referenced uses in the same
area.
In `@Sources/KeyboardShortcutSettingsFileStore.swift`:
- Around line 181-185: The shortcut snapshot building in
KeyboardShortcutSettingsFileStore is currently preserving duplicate keystroke
collisions by ordering them, which makes routing in
AppDelegate.firstMatchingCommandShortcut(event:) heuristic instead of
authoritative. Update the logic around commandShortcutsByCommandId and
commandShortcutsOrderedSnapshot to detect when multiple command ids resolve to
the same keystroke, then drop or invalidate the ambiguous bindings so the
shortcut fails closed rather than keeping both entries. Ensure the resolved
snapshot only contains unambiguous command-shortcut mappings and that collisions
do not reach downstream matching.
In `@web/data/cmux.schema.json`:
- Around line 1567-1578: The schema for single-stroke command bindings currently
rejects the recorder’s clear sentinel, so update the binding definition to
accept the object-form empty-first unbind used by parseCommandShortcutValue(_:)
in Sources/KeyboardShortcutSettingsFileStore.swift, or change the recorder/store
to serialize clears as null everywhere. Adjust the relevant shortcut binding
schema symbols such as singleShortcutBindingNullable and shortcutStrokeObject so
a cleared custom command shortcut remains valid when Settings writes { "first":
{ "key": "" } }.
In `@web/messages/tr.json`:
- Around line 967-1032: The keyboard shortcuts localization is missing the new
custom commands strings in this locale. Add the
`keyboardShortcuts.customCommandsTitle`,
`keyboardShortcuts.customCommandsIntro`, and
`keyboardShortcuts.customCommandsCallout` entries to this translation file,
matching the existing `keyboardShortcuts` structure in `tr.json`, and make sure
the wording is fully translated and consistent with the new Custom Commands UI.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 51c72759-98e8-40af-95e9-48bfb07ef6ec
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (26)
Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/CommandShortcutPickerSheet.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/CustomCommandShortcutsCard.swiftResources/Localizable.xcstringsSources/KeyboardShortcutSettingsFileStore.swiftcmuxTests/CommandPaletteShortcutCustomizationTests.swiftweb/data/cmux.schema.jsonweb/messages/ar.jsonweb/messages/bs.jsonweb/messages/da.jsonweb/messages/de.jsonweb/messages/en.jsonweb/messages/es.jsonweb/messages/fr.jsonweb/messages/it.jsonweb/messages/ja.jsonweb/messages/km.jsonweb/messages/ko.jsonweb/messages/no.jsonweb/messages/pl.jsonweb/messages/pt-BR.jsonweb/messages/ru.jsonweb/messages/th.jsonweb/messages/tr.jsonweb/messages/uk.jsonweb/messages/zh-CN.jsonweb/messages/zh-TW.json
…board-shortcut-for-command-palette # Conflicts: # .github/swift-file-length-budget.tsv
…board-shortcut-for-command-palette # Conflicts: # .github/swift-file-length-budget.tsv
…board-shortcut-for-command-palette
…board-shortcut-for-command-palette # Conflicts: # .github/swift-file-length-budget.tsv
…board-shortcut-for-command-palette # Conflicts: # .github/swift-file-length-budget.tsv
…board-shortcut-for-command-palette # Conflicts: # .github/swift-file-length-budget.tsv # Resources/Localizable.xcstrings
…board-shortcut-for-command-palette # Conflicts: # .github/swift-file-length-budget.tsv
…board-shortcut-for-command-palette # Conflicts: # .github/swift-file-length-budget.tsv
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 45 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/KeyboardShortcutSettingsFileStore.swift">
<violation number="1" location="Sources/KeyboardShortcutSettingsFileStore.swift:248">
P2: Some configured command shortcuts can silently stop working when `shortcuts.commands` exceeds the dispatch cap, because dispatch indexes only the first `limit` entries while settings still expose all bindings. Consider removing the silent truncation (or applying the same truncation consistently to exposed state with explicit user feedback) so configured shortcuts and runtime behavior stay aligned.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
| let ordered = commandShortcuts | ||
| .filter { !$0.value.isUnbound } | ||
| .sorted { $0.key < $1.key } | ||
| .prefix(max(0, limit)) |
There was a problem hiding this comment.
P2: Some configured command shortcuts can silently stop working when shortcuts.commands exceeds the dispatch cap, because dispatch indexes only the first limit entries while settings still expose all bindings. Consider removing the silent truncation (or applying the same truncation consistently to exposed state with explicit user feedback) so configured shortcuts and runtime behavior stay aligned.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/KeyboardShortcutSettingsFileStore.swift, line 248:
<comment>Some configured command shortcuts can silently stop working when `shortcuts.commands` exceeds the dispatch cap, because dispatch indexes only the first `limit` entries while settings still expose all bindings. Consider removing the silent truncation (or applying the same truncation consistently to exposed state with explicit user feedback) so configured shortcuts and runtime behavior stay aligned.</comment>
<file context>
@@ -190,6 +212,45 @@ final class CmuxSettingsFileStore {
+ let ordered = commandShortcuts
+ .filter { !$0.value.isUnbound }
+ .sorted { $0.key < $1.key }
+ .prefix(max(0, limit))
+ .map { (commandId: $0.key, shortcut: $0.value) }
+ let byModifierMask = Dictionary(grouping: ordered) { $0.shortcut.modifierFlags.rawValue }
</file context>
…board-shortcut-for-command-palette # Conflicts: # .github/swift-file-length-budget.tsv
…board-shortcut-for-command-palette # Conflicts: # .github/swift-file-length-budget.tsv
…board-shortcut-for-command-palette # Conflicts: # .github/swift-file-length-budget.tsv
…board-shortcut-for-command-palette # Conflicts: # .github/swift-file-length-budget.tsv
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
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
3 participants
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.
Fixes #6431
What
Adds a Custom Commands section under Settings → Keyboard Shortcuts so users can assign their own single keystroke to any Command Palette command that ships without a built-in shortcut (the issue's example: Open Current Directory in VS Code) — without hand-editing
cmux.jsonor waiting for a dedicated built-in.How it maps to the request
CommandPaletteSearchEngine, bridged to the settings package viaSettingsHostActions).ShortcutRecorderView, modifier required, chords disabled).~/.config/cmux/cmux.jsonunder ashortcuts.commandsmap (commandId→ shortcut)..commandPaletteRunCommandRequestednotification →ContentView.runCommandPaletteCommandById.commandShortcutConflictLabel, numbered-digit aware).when/enablementclause is a silent no-op; the event is consumed on any binding match. Built-in actions win a shared keystroke (command shortcuts are matched last inhandleCustomShortcut).Shared-behavior / DRY
The built-in palette contributions are extracted to a single static source (
ContentView.builtInCommandPaletteCommandContributions) consumed by both the live palette and the Settings picker catalog, so the bindable command list and the palette never drift.Tests
CommandShortcutConflictTests(package): built-in/other-command/self-exclusion conflict cases.CommandPaletteShortcutCustomizationTests(app):shortcuts.commandsparsing (single-stroke kept, chord & null rejected) and end-to-end dispatch (debugHandleCustomShortcut→ consume + post run-command request).Docs / schema / localization
cmux.schema.json:shortcuts.commands(single-stroke binding map).Resources/Localizable.xcstrings(en + ja) for all new strings;web/messages/{en,ja}.jsonfor the new docs section.🤖 Generated with Claude Code
Summary by cubic
Adds “Custom Commands” to Settings → Keyboard Shortcuts so you can bind a single keystroke (primary modifier required) to any Command Palette command without a built‑in shortcut. Shortcuts live in
shortcuts.commands, are conflict‑checked, and run on the focused window with fresh context; input‑hosting commands auto‑present the palette. Addresses #6431.New Features
shortcuts.commands; per‑entryJSONConfigStore.setMapEntry()preserves sibling string/object forms; unbind writesnone; exposed viaKeyboardShortcutSettings.commandShortcuts().commandPaletteRunCommandRequested, rebuilds context, and runs by id on the focused window; palette shown for input‑hosting commands; hot‑path dispatch uses a capped modifier‑mask index.Bug Fixes / Validation
noneclears a binding.actionsacross all windows (deduped); honor numbered‑digit families; block chord prefixes and other command collisions; works with no open windows.Written for commit c0e8d95. Summary will update on new commits.
Summary by CodeRabbit
shortcuts.commands.