Fix close-tab shortcut remapping by austinywang · Pull Request #2512 · manaflow-ai/cmux

austinywang · 2026-04-01T21:46:09Z

Summary

Registers closeTab as a remappable action in KeyboardShortcutSettings so Cmd+W respects user-configured shortcuts
Updates AppDelegate interceptor to use the remapped shortcut instead of a hardcoded Cmd+W check
Wires up closeTabMenuShortcut in the File menu via splitCommandButton so the menu item reflects the remap
Replaces hardcoded command palette hints for Close Tab / Close Workspace with dynamic shortcut lookup

Test plan

Default behavior: Cmd+W still closes the focused tab
Remap close-tab to a different shortcut in settings, verify the new shortcut closes tabs and the menu item updates
Verify the command palette shows the remapped shortcut hint for Close Tab / Close Workspace
Verify Cmd+W in browser popup windows still works as a fallback

🤖 Generated with Claude Code

Summary by cubic

Make Close Tab respect user-remapped shortcuts across the app. Updates the interceptor, File menu, and command palette to use the dynamic shortcut. Fixes #1710.

Bug Fixes
- Registered Close Tab as a remappable action in KeyboardShortcutSettings.
- Interceptor now matches the remapped Close Tab shortcut (with Cmd+W fallback for browser popups).
- File menu Close Tab uses splitCommandButton with the decoded shortcut (no hardcoded ⌘W).
- Command palette maps palette.closeTab and palette.closeWorkspace to actions and removes hardcoded hints, so execution and hints use dynamic lookup.

^{Written for commit 27be97e. Summary will update on new commits.}

Summary by CodeRabbit

Bug Fixes
- Improved keyboard shortcut resolution to properly reflect configured settings for closing tabs and workspaces, ensuring shortcut hints display correctly throughout the application.

vercel · 2026-04-01T21:46:15Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
cmux	Ready	Preview, Comment	Apr 6, 2026 7:09am

coderabbitai · 2026-04-01T21:46:28Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 76875cc3-29eb-41fd-994f-bc144080f5ec

📥 Commits

Reviewing files that changed from the base of the PR and between 377f590 and 27be97e.

📒 Files selected for processing (2)

Sources/AppDelegate.swift
Sources/ContentView.swift

✅ Files skipped from review due to trivial changes (1)

Sources/AppDelegate.swift

🚧 Files skipped from review as they are similar to previous changes (1)

Sources/ContentView.swift

📝 Walkthrough

Walkthrough

The pull request updates shortcut handling across two files. AppDelegate's close-tab comment is reflowed, and ContentView refactors command palette shortcut mapping to resolve dynamic shortcuts from KeyboardShortcutSettings instead of returning hardcoded hint strings.

Changes

Cohort / File(s)	Summary
Comment Update `Sources/AppDelegate.swift`	Updated comment preceding close-tab shortcut handling to refer to "close-tab shortcut" with reflow across lines; no logic changes.
Shortcut Mapping Refactoring `Sources/ContentView.swift`	Restructured command palette shortcut handling to dynamically map `palette.closeTab` and `palette.closeWorkspace` via `commandPaletteShortcutAction(for:)` instead of hardcoded hints; removed corresponding cases from static hint resolution.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

PR #1473: Modifies close-shortcut behavior in AppDelegate's handleCustomShortcut, directly related to shortcut interception logic alongside these comment and mapping updates.

Poem

🐰 Shortcuts dance in dynamic ways,
No more hardcoded holiday,
Cmd+W finds its resting place,
Settings shape the terminal's grace! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Fix close-tab shortcut remapping' is concise, clear, and directly summarizes the main change—enabling close-tab as a remappable action and updating the interceptor to respect user-configured shortcuts rather than hardcoded Cmd+W.
Description check	✅ Passed	The PR description covers the main objectives, references the fixed issue (`#1710`), provides a clear test plan with multiple verification steps, and includes auto-generated summaries that explain the bug fixes and implementation approach.
Linked Issues check	✅ Passed	The code changes address all core requirements: close-tab is registered as remappable in KeyboardShortcutSettings [`#1710`], the AppDelegate interceptor now respects the remapped shortcut with Cmd+W fallback [`#1710`], the File menu reflects the dynamic shortcut via splitCommandButton [`#1710`], and command palette hints are now dynamically resolved instead of hardcoded [`#1710`].
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing issue `#1710`: AppDelegate comment clarification, command palette shortcut action mapping, and removal of hardcoded hints for close-tab and close-workspace are all within scope of the remapping fix.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch issue-1710-cmdw-remap-intercepted

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

greptile-apps · 2026-04-01T21:50:04Z

Greptile Summary

This PR makes the close-tab shortcut (Cmd+W) user-remappable end-to-end by adding closeTab to KeyboardShortcutSettings.Action and threading the dynamic shortcut through all three dispatch sites: the AppDelegate low-level key interceptor, the SwiftUI File menu item, and the command palette shortcut hint display.

Key changes:

KeyboardShortcutSettings.swift: Adds .closeTab to the Action enum with default Cmd+W, a defaultsKey of "shortcut.closeTab", and a label that reuses menu.file.closeTab (see inline comment about key-prefix consistency).
AppDelegate.swift: Replaces the hardcoded StoredShortcut(key: "w", …) comparison with KeyboardShortcutSettings.shortcut(for: .closeTab), matching how .closeWorkspace is already handled. Three #if DEBUG dlog strings still reference cmdW by name and will mislead when the shortcut is remapped.
cmuxApp.swift: Adds @AppStorage + closeTabMenuShortcut computed property and updates the menu button to splitCommandButton, so the menu item updates reactively without an app restart.
ContentView.swift: Migrates palette.closeTab and palette.closeWorkspace from static symbol strings ("⌘W" / "⌘⇧W") to the dynamic commandPaletteShortcutAction lookup, so command palette hints always reflect the user's current mapping.

Confidence Score: 5/5

Safe to merge — all dispatch sites are correctly updated and the implementation is consistent with existing remappable shortcuts.
All remaining findings are P2 style/cosmetic: stale cmdW in debug-only log strings and an inconsistent localization key prefix. Neither affects correctness or runtime behavior.
No files require special attention.

Important Files Changed

Filename	Overview
Sources/KeyboardShortcutSettings.swift	Registers `closeTab` as a remappable action with correct default (Cmd+W), defaultsKey, and label. Minor: label reuses `menu.file.closeTab` instead of a dedicated `shortcut.closeTab.label` key, inconsistent with all other actions.
Sources/AppDelegate.swift	Replaces hardcoded `StoredShortcut(key: "w", …)` with `KeyboardShortcutSettings.shortcut(for: .closeTab)`, consistent with how `.closeWorkspace` is handled. Three `#if DEBUG` log strings still say `cmdW` and will mislead when the shortcut is remapped.
Sources/ContentView.swift	Moves `palette.closeTab` and `palette.closeWorkspace` from static `⌘W`/`⌘⇧W` hint strings to the dynamic `commandPaletteShortcutAction` lookup, so command palette hints now track user-configured shortcuts correctly.
Sources/cmuxApp.swift	Adds `@AppStorage` binding for `closeTabShortcutData`, a `closeTabMenuShortcut` computed property, and wires the File menu "Close Tab" item through `splitCommandButton` so the menu item updates reactively when the shortcut is remapped.

Sequence Diagram

sequenceDiagram
    participant User
    participant AppDelegate
    participant KSS as KeyboardShortcutSettings
    participant Menu as File Menu
    participant Palette as Command Palette

    Note over User,Palette: Before remap (default Cmd+W)
    User->>AppDelegate: key event
    AppDelegate->>KSS: shortcut(for: .closeTab)
    KSS-->>AppDelegate: StoredShortcut(w, ⌘)
    AppDelegate->>AppDelegate: matchShortcut → closePanelOrWindow()

    Note over User,Palette: After user remaps to Cmd+Q
    User->>KSS: setShortcut(.closeTab, Cmd+Q)
    KSS->>KSS: UserDefaults.set("shortcut.closeTab")
    KSS-->>Menu: @AppStorage triggers closeTabMenuShortcut update
    Menu-->>Menu: splitCommandButton rebinds to Cmd+Q
    KSS-->>Palette: dynamic lookup via commandPaletteShortcutAction
    Palette-->>Palette: hint shows "⌘Q"

    User->>AppDelegate: Cmd+Q key event
    AppDelegate->>KSS: shortcut(for: .closeTab)
    KSS-->>AppDelegate: StoredShortcut(q, ⌘)
    AppDelegate->>AppDelegate: matchShortcut → closePanelOrWindow()

Comments Outside Diff (1)

Sources/AppDelegate.swift, line 9937-9959 (link)

Stale cmdW debug log strings

The three dlog calls inside this block still hardcode "shortcut.cmdW" in their log strings (lines 9937, 9949, 9959). Now that the close-tab shortcut is dynamic, these messages will be misleading when debugging a remapped shortcut — e.g. if the user remaps close-tab to Cmd+Q, the log will still report shortcut.cmdW route=....

Consider renaming to a key-agnostic label like shortcut.closeTab:

_{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!}

_{Reviews (1): Last reviewed commit: "Fix close-tab shortcut remapping" | Re-trigger Greptile}

greptile-apps · 2026-04-01T21:50:11Z

            case .toggleSidebar: return String(localized: "shortcut.toggleSidebar.label", defaultValue: "Toggle Sidebar")
            case .newTab: return String(localized: "shortcut.newWorkspace.label", defaultValue: "New Workspace")
            case .newWindow: return String(localized: "shortcut.newWindow.label", defaultValue: "New Window")
+            case .closeTab: return String(localized: "menu.file.closeTab", defaultValue: "Close Tab")


Inconsistent localization key prefix for closeTab label

Every other Action.label in this switch uses a shortcut.* prefixed key (e.g. shortcut.closeWindow.label, shortcut.newWindow.label), but closeTab reuses the menu-scoped key menu.file.closeTab. The string renders correctly ("Close Tab") since the key already exists, but it creates a naming inconsistency in the settings UI — the translator context for a menu item differs from that for a shortcut settings row.

Consider introducing a dedicated key to stay consistent with the rest of the labels:

Suggested change

case .closeTab: return String(localized: "menu.file.closeTab", defaultValue: "Close Tab")

case .closeTab: return String(localized: "shortcut.closeTab.label", defaultValue: "Close Tab")

(Then add "shortcut.closeTab.label" to Resources/Localizable.xcstrings with the same translations as menu.file.closeTab.)

cubic-dev-ai

No issues found across 4 files

…-1710-cmdw-remap-intercepted # Conflicts: # Sources/AppDelegate.swift # Sources/KeyboardShortcutSettings.swift # Sources/cmuxApp.swift

…-1710-cmdw-remap-intercepted

…60406

Fix close-tab shortcut remapping

377f590

greptile-apps Bot reviewed Apr 1, 2026

View reviewed changes

cubic-dev-ai Bot reviewed Apr 1, 2026

View reviewed changes

austinywang added 3 commits April 4, 2026 17:54

Merge branch 'main' of https://github.com/manaflow-ai/cmux into issue…

ac0c706

…-1710-cmdw-remap-intercepted # Conflicts: # Sources/AppDelegate.swift # Sources/KeyboardShortcutSettings.swift # Sources/cmuxApp.swift

Merge branch 'main' of https://github.com/manaflow-ai/cmux into issue…

8694b3e

…-1710-cmdw-remap-intercepted

Merge remote-tracking branch 'origin/main' into pr2512-merge-main-202…

27be97e

…60406

vercel Bot deployed to Preview April 6, 2026 07:09 View deployment

austinywang mentioned this pull request May 20, 2026

Fix remapped Cmd+W close shortcuts #4406

Merged

1 task

teamleaderleo added this to cmux backlog Sep 23, 2026

github-project-automation Bot moved this to Todo in cmux backlog Sep 23, 2026

lawrencecchen closed this Sep 23, 2026

lawrencecchen added the stale-revisit Closed after 30+ days without activity; preserved for possible revisit or reopening. label Sep 23, 2026

github-project-automation Bot moved this from Todo to Done in cmux backlog Sep 23, 2026

Navigation Menu

Repository navigation

Fix close-tab shortcut remapping - #2512

Fix close-tab shortcut remapping#2512
austinywang wants to merge 4 commits into
mainfrom
issue-1710-cmdw-remap-intercepted

austinywang commented Apr 1, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

vercel Bot commented Apr 1, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Apr 1, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Uh oh!

greptile-apps Bot commented Apr 1, 2026 •

edited

Loading

Comments Outside Diff (1)

Uh oh!

greptile-apps Bot Apr 1, 2026

Uh oh!

cubic-dev-ai Bot left a comment

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	case .closeTab: return String(localized: "menu.file.closeTab", defaultValue: "Close Tab")
	case .closeTab: return String(localized: "shortcut.closeTab.label", defaultValue: "Close Tab")

Conversation

austinywang commented Apr 1, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Summary by cubic

Summary by CodeRabbit

Uh oh!

vercel Bot commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Uh oh!

greptile-apps Bot commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Comments Outside Diff (1)

Uh oh!

greptile-apps Bot Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

austinywang commented Apr 1, 2026 •

edited by coderabbitai Bot

Loading

vercel Bot commented Apr 1, 2026 •

edited

Loading

coderabbitai Bot commented Apr 1, 2026 •

edited

Loading

greptile-apps Bot commented Apr 1, 2026 •

edited

Loading