From b759ca280520e7ee9c0becc11166ad987e554a74 Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Mon, 1 Jun 2026 23:16:47 -0700 Subject: [PATCH 1/6] Add a configurable keyboard shortcut to open the diff viewer cmux already opens the diff viewer via the `cmux diff` CLI and the "Open Diff Viewer" command-palette entry (which spawns that CLI). This adds a first-class, user-rebindable keyboard shortcut that opens the diff viewer for the focused workspace through the same shared path, with no duplicated diff-open logic. - Register an `openDiffViewer` action in KeyboardShortcutSettings (label "Open Diff Viewer"); it is automatically visible/editable in Settings and configurable via ~/.config/cmux/cmux.json. - Default Cmd+Ctrl+D: Cmd+Shift+D collides with Split Down and the whole Cmd-based "D" family is taken by split actions, so use Cmd+Ctrl+D, which is free and matches cmux's Cmd+Ctrl+ app-shortcut family (Close Window, Toggle Full Screen). - Extract the command palette's diff launcher into a shared AppDelegate.openDiffViewerForFocusedWorkspace(for:); the palette and the shortcut both call it (spawning `cmux diff`). Lift cwd resolution to Workspace.resolvedWorkingDirectory(). - Update docs (web/data/cmux-shortcuts.ts feeds both the keyboard shortcut and configuration docs pages) and add en/ja localization. - Add a dispatch test asserting Cmd+Ctrl+D routes to the shared path and that the default is conflict-free. Closes #5169 Co-Authored-By: Claude Opus 4.8 --- Resources/Localizable.xcstrings | 17 +++ Sources/AppDelegate.swift | 124 +++++++++++++++++ Sources/ContentView.swift | 126 +----------------- Sources/KeyboardShortcutSettings.swift | 10 ++ Sources/Workspace.swift | 19 +++ .../AppDelegateShortcutRoutingTests.swift | 58 ++++++++ web/data/cmux-shortcuts.ts | 5 + 7 files changed, 236 insertions(+), 123 deletions(-) diff --git a/Resources/Localizable.xcstrings b/Resources/Localizable.xcstrings index 0a8e2373ebd2..ddf0acfa1944 100644 --- a/Resources/Localizable.xcstrings +++ b/Resources/Localizable.xcstrings @@ -53,6 +53,23 @@ } } }, + "shortcut.openDiffViewer.label": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "Open Diff Viewer" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "差分ビューアを開く" + } + } + } + }, "command.markdownZoomIn.title": { "extractionState": "manual", "localizations": { diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index f0cb6cfaede0..7c2ba8ab63a8 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -972,6 +972,12 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent let isFirstResponder: Bool } var debugCloseMainWindowConfirmationHandler: ((NSWindow) -> Bool)? + /// Test seam: when set, ``openDiffViewerForFocusedWorkspace(for:)`` invokes this + /// instead of spawning the bundled `cmux diff` CLI, so shortcut-dispatch tests can + /// assert routing without launching a subprocess. + var debugOpenDiffViewerHandler: (() -> Void)? + /// Live `cmux diff` viewer subprocesses, keyed by pid, retained until they exit. + private var diffViewerProcesses: [Int32: Process] = [:] var debugCreateMainWindowSourceIsNativeFullScreenOverride: Bool? // Keep debug-only windows alive when tests intentionally inject key mismatches. private var debugDetachedContextWindows: [NSWindow] = [] @@ -5865,6 +5871,114 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent }?.tabManager } + /// Opens the diff viewer for the focused workspace of `tabManager` by spawning the + /// bundled `cmux diff` CLI. This is the single shared diff-open path: both the + /// command-palette entry and the Open Diff Viewer keyboard shortcut funnel through + /// here so neither duplicates diff-open logic. Returns `false` (caller beeps) when + /// there is no focused workspace or the bundled CLI is missing. + @discardableResult + func openDiffViewerForFocusedWorkspace(for tabManager: TabManager?) -> Bool { + if let debugOpenDiffViewerHandler { + debugOpenDiffViewerHandler() + return true + } + guard let workspace = tabManager?.selectedWorkspace, + let cliURL = Bundle.main.resourceURL?.appendingPathComponent("bin/cmux"), + FileManager.default.isExecutableFile(atPath: cliURL.path) else { + return false + } + let socketPath = TerminalController.shared.activeSocketPath( + preferredPath: SocketControlSettings.socketPath() + ) + let cwd = workspace.resolvedWorkingDirectory() + ?? FileManager.default.homeDirectoryForCurrentUser.path + return launchDiffViewerProcess( + cliURL: cliURL, + socketPath: socketPath, + cwd: cwd, + workspaceId: workspace.id, + surfaceId: workspace.focusedPanelId + ) + } + + @discardableResult + private func launchDiffViewerProcess( + cliURL: URL, + socketPath: String, + cwd: String, + workspaceId: UUID, + surfaceId: UUID? + ) -> Bool { + let process = Process() + process.executableURL = cliURL + var arguments = [ + "--socket", socketPath, + "diff", + "--unstaged", + "--cwd", cwd, + "--workspace", workspaceId.uuidString, + "--focus", "true", + ] + if let surfaceId { + arguments.append(contentsOf: ["--surface", surfaceId.uuidString]) + } + process.arguments = arguments + process.currentDirectoryURL = URL(fileURLWithPath: cwd, isDirectory: true) + var environment = ProcessInfo.processInfo.environment + environment["CMUX_SOCKET_PATH"] = socketPath + environment["CMUX_BUNDLED_CLI_PATH"] = cliURL.path + environment["CMUX_WORKSPACE_ID"] = workspaceId.uuidString + if let surfaceId { + environment["CMUX_SURFACE_ID"] = surfaceId.uuidString + } + environment.removeValue(forKey: "CMUX_SOCKET") + process.environment = environment + process.standardInput = FileHandle.nullDevice + + let stdoutPipe = Pipe() + let stderrPipe = Pipe() + process.standardOutput = stdoutPipe + process.standardError = stderrPipe + let outputCollector = ProcessOutputCollector(stdout: stdoutPipe, stderr: stderrPipe) + outputCollector.start() + process.terminationHandler = { terminatedProcess in + let output = outputCollector.finish() + let processIdentifier = terminatedProcess.processIdentifier + let terminationStatus = terminatedProcess.terminationStatus + Task { @MainActor in + AppDelegate.shared?.diffViewerProcesses.removeValue(forKey: processIdentifier) + guard terminationStatus != 0 else { return } + let detail = output.trimmingCharacters(in: .whitespacesAndNewlines) + let limitedDetail = detail.isEmpty + ? "status=\(terminationStatus)" + : String(detail.prefix(240)) +#if DEBUG + cmuxDebugLog("openDiffViewer exited status=\(terminationStatus) detail=\(limitedDetail)") +#endif + NSSound.beep() + } + } + + do { + try process.run() + let processIdentifier = process.processIdentifier + diffViewerProcesses[processIdentifier] = process + if !process.isRunning { + diffViewerProcesses.removeValue(forKey: processIdentifier) + } +#if DEBUG + cmuxDebugLog("openDiffViewer pid=\(process.processIdentifier) cwd=\(cwd)") +#endif + return true + } catch { + outputCollector.cancel() +#if DEBUG + cmuxDebugLog("openDiffViewer failed cwd=\(cwd) error=\(error.localizedDescription)") +#endif + return false + } + } + func allMainWindowTabManagersForDebug() -> [TabManager] { Array(mainWindowContexts.values).compactMap { context in resolvedWindow(for: context) == nil ? nil : context.tabManager @@ -12720,6 +12834,16 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent return true } + if matchConfiguredShortcut(event: event, action: .openDiffViewer) { + // Shares the command palette's diff-open path; targets the event window's + // focused workspace and beeps if it can't be opened (matching the palette). + let manager = activeTabManagerForCommands(preferredWindow: mainWindowForShortcutEvent(event)) + if !openDiffViewerForFocusedWorkspace(for: manager) { + NSSound.beep() + } + return true + } + if matchConfiguredShortcut(event: event, action: .toggleRightSidebar) { // Escape AppKit's performKeyEquivalent animation context. Without // deferring the toggle, NSAnimationContext implicitly animates the diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index c11c32815701..362397e7b0a2 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -8166,7 +8166,7 @@ struct ContentView: View { } } registry.register(commandId: "palette.openDiffViewer") { - if !openDiffViewerFromCommandPalette() { + if AppDelegate.shared?.openDiffViewerForFocusedWorkspace(for: tabManager) != true { NSSound.beep() } } @@ -8400,129 +8400,9 @@ struct ContentView: View { ) } - @discardableResult - private func openDiffViewerFromCommandPalette() -> Bool { - guard let workspace = tabManager.selectedWorkspace, - let cliURL = Bundle.main.resourceURL?.appendingPathComponent("bin/cmux"), - FileManager.default.isExecutableFile(atPath: cliURL.path) else { - return false - } - - let preferredSocketPath = SocketControlSettings.socketPath() - let activeSocketPath = TerminalController.shared.activeSocketPath(preferredPath: preferredSocketPath) - return CommandPaletteDiffViewerLauncher.shared.start( - cliURL: cliURL, - socketPath: activeSocketPath, - cwd: configuredActionBaseCwd(), - workspaceId: workspace.id, - surfaceId: workspace.focusedPanelId - ) - } - - @MainActor - private final class CommandPaletteDiffViewerLauncher { - static let shared = CommandPaletteDiffViewerLauncher() - - private var processes: [Int32: Process] = [:] - - private init() {} - - @discardableResult - func start( - cliURL: URL, - socketPath: String, - cwd: String, - workspaceId: UUID, - surfaceId: UUID? - ) -> Bool { - let process = Process() - process.executableURL = cliURL - var arguments = [ - "--socket", socketPath, - "diff", - "--unstaged", - "--cwd", cwd, - "--workspace", workspaceId.uuidString, - "--focus", "true", - ] - if let surfaceId { - arguments.append(contentsOf: ["--surface", surfaceId.uuidString]) - } - process.arguments = arguments - process.currentDirectoryURL = URL(fileURLWithPath: cwd, isDirectory: true) - var environment = ProcessInfo.processInfo.environment - environment["CMUX_SOCKET_PATH"] = socketPath - environment["CMUX_BUNDLED_CLI_PATH"] = cliURL.path - environment["CMUX_WORKSPACE_ID"] = workspaceId.uuidString - if let surfaceId { - environment["CMUX_SURFACE_ID"] = surfaceId.uuidString - } - environment.removeValue(forKey: "CMUX_SOCKET") - process.environment = environment - process.standardInput = FileHandle.nullDevice - - let stdoutPipe = Pipe() - let stderrPipe = Pipe() - process.standardOutput = stdoutPipe - process.standardError = stderrPipe - let outputCollector = ProcessOutputCollector(stdout: stdoutPipe, stderr: stderrPipe) - outputCollector.start() - process.terminationHandler = { terminatedProcess in - let output = outputCollector.finish() - let processIdentifier = terminatedProcess.processIdentifier - let terminationStatus = terminatedProcess.terminationStatus - Task { @MainActor in - Self.shared.processes.removeValue(forKey: processIdentifier) - guard terminationStatus != 0 else { return } - let detail = output.trimmingCharacters(in: .whitespacesAndNewlines) - let limitedDetail = detail.isEmpty - ? "status=\(terminationStatus)" - : String(detail.prefix(240)) -#if DEBUG - cmuxDebugLog("commandPalette.openDiffViewer exited status=\(terminationStatus) detail=\(limitedDetail)") -#endif - NSSound.beep() - } - } - - do { - try process.run() - let processIdentifier = process.processIdentifier - processes[processIdentifier] = process - if !process.isRunning { - processes.removeValue(forKey: processIdentifier) - } -#if DEBUG - cmuxDebugLog("commandPalette.openDiffViewer pid=\(process.processIdentifier) cwd=\(cwd)") -#endif - return true - } catch { - outputCollector.cancel() -#if DEBUG - cmuxDebugLog("commandPalette.openDiffViewer failed cwd=\(cwd) error=\(error.localizedDescription)") -#endif - return false - } - } - } - private func configuredActionBaseCwd() -> String { - guard let workspace = tabManager.selectedWorkspace else { - return FileManager.default.homeDirectoryForCurrentUser.path - } - let focusedPanelId = workspace.focusedPanelId - let candidates = [ - focusedPanelId.flatMap { workspace.panelDirectories[$0] }, - focusedPanelId.flatMap { workspace.terminalPanel(for: $0)?.requestedWorkingDirectory }, - workspace.currentDirectory - ] - for candidate in candidates { - let trimmed = candidate?.trimmingCharacters(in: .whitespacesAndNewlines) ?? "" - if !trimmed.isEmpty { - return trimmed - } - } - return FileManager.default.homeDirectoryForCurrentUser.path + tabManager.selectedWorkspace?.resolvedWorkingDirectory() + ?? FileManager.default.homeDirectoryForCurrentUser.path } var focusedPanelContext: (workspace: Workspace, panelId: UUID, panel: any Panel)? { diff --git a/Sources/KeyboardShortcutSettings.swift b/Sources/KeyboardShortcutSettings.swift index 8c65ce62b176..64f8ff4273e9 100644 --- a/Sources/KeyboardShortcutSettings.swift +++ b/Sources/KeyboardShortcutSettings.swift @@ -150,6 +150,7 @@ enum KeyboardShortcutSettings { case toggleBrowserDeveloperTools case showBrowserJavaScriptConsole case toggleReactGrab + case openDiffViewer case diffViewerScrollDown case diffViewerScrollUp case diffViewerScrollToBottom @@ -243,6 +244,7 @@ enum KeyboardShortcutSettings { case .toggleBrowserDeveloperTools: return String(localized: "shortcut.toggleBrowserDevTools.label", defaultValue: "Toggle Browser Developer Tools") case .showBrowserJavaScriptConsole: return String(localized: "shortcut.showBrowserJSConsole.label", defaultValue: "Show Browser JavaScript Console") case .toggleReactGrab: return String(localized: "shortcut.toggleReactGrab.label", defaultValue: "Toggle React Grab") + case .openDiffViewer: return String(localized: "shortcut.openDiffViewer.label", defaultValue: "Open Diff Viewer") case .diffViewerScrollDown: return String(localized: "shortcut.diffViewerScrollDown.label", defaultValue: "Diff Viewer: Scroll Down") case .diffViewerScrollUp: return String(localized: "shortcut.diffViewerScrollUp.label", defaultValue: "Diff Viewer: Scroll Up") case .diffViewerScrollToBottom: return String(localized: "shortcut.diffViewerScrollToBottom.label", defaultValue: "Diff Viewer: Scroll to Bottom") @@ -452,6 +454,14 @@ enum KeyboardShortcutSettings { return StoredShortcut(key: "c", command: true, shift: false, option: true, control: false) case .toggleReactGrab: return StoredShortcut(key: "g", command: true, shift: true, option: false, control: false) + case .openDiffViewer: + // Cmd+Ctrl+D. The natural "D for Diff" chord (Cmd+Shift+D) collides + // with Split Down, and the entire Cmd-based "D" family is already + // taken by split actions (Cmd+D, Cmd+Shift+D, Cmd+Opt+D, + // Cmd+Shift+Opt+D). Cmd+Ctrl+D is free and matches cmux's + // Cmd+Ctrl+ app-shortcut family (Close Window = Cmd+Ctrl+W, + // Toggle Full Screen = Cmd+Ctrl+F). + return StoredShortcut(key: "d", command: true, shift: false, option: false, control: true) case .diffViewerScrollDown: return StoredShortcut(key: "j", command: false, shift: false, option: false, control: false) case .diffViewerScrollUp: diff --git a/Sources/Workspace.swift b/Sources/Workspace.swift index c18e960658d7..9aa0b528c093 100644 --- a/Sources/Workspace.swift +++ b/Sources/Workspace.swift @@ -11588,6 +11588,25 @@ final class Workspace: Identifiable, ObservableObject { panels[panelId] as? FilePreviewPanel } + /// The working directory app-level actions (diff viewer, configured commands) + /// should target for this workspace: the focused panel's tracked directory, then + /// its terminal's requested directory, then the workspace's current directory. + /// Returns `nil` when none is known so callers can apply their own fallback. + func resolvedWorkingDirectory() -> String? { + let candidates = [ + focusedPanelId.flatMap { panelDirectories[$0] }, + focusedPanelId.flatMap { terminalPanel(for: $0)?.requestedWorkingDirectory }, + currentDirectory, + ] + for candidate in candidates { + let trimmed = candidate?.trimmingCharacters(in: .whitespacesAndNewlines) ?? "" + if !trimmed.isEmpty { + return trimmed + } + } + return nil + } + private func surfaceKind(for panel: any Panel) -> String { switch panel.panelType { case .terminal: diff --git a/cmuxTests/AppDelegateShortcutRoutingTests.swift b/cmuxTests/AppDelegateShortcutRoutingTests.swift index 5668bd979956..c74be2f99530 100644 --- a/cmuxTests/AppDelegateShortcutRoutingTests.swift +++ b/cmuxTests/AppDelegateShortcutRoutingTests.swift @@ -1971,6 +1971,64 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { ) } + func testOpenDiffViewerShortcutDefaultsToCmdCtrlDAndRoutesToSharedDiffPath() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + // Default is Cmd+Ctrl+D (the natural Cmd+Shift+D collides with Split Down), and + // it must be conflict-free so the recorder accepts it as-is. + let cmdCtrlD = StoredShortcut(key: "d", command: true, shift: false, option: false, control: true) + XCTAssertEqual(KeyboardShortcutSettings.shortcut(for: .openDiffViewer), cmdCtrlD) + XCTAssertEqual( + KeyboardShortcutSettings.Action.openDiffViewer.normalizedRecordedShortcutResult(cmdCtrlD), + .accepted(cmdCtrlD), + "Default Open Diff Viewer shortcut must not conflict with any other action" + ) + XCTAssertTrue( + KeyboardShortcutSettings.settingsVisibleActions.contains(.openDiffViewer), + "Open Diff Viewer must be visible/editable in Settings → Keyboard Shortcuts" + ) + + let windowId = appDelegate.createMainWindow() + defer { closeWindow(withId: windowId) } + guard let targetWindow = window(withId: windowId) else { + XCTFail("Expected test window") + return + } + + // Intercept the shared diff-open path so the dispatch test never spawns a + // subprocess; we only assert the shortcut routes here. + var openDiffViewerCount = 0 + appDelegate.debugOpenDiffViewerHandler = { openDiffViewerCount += 1 } + defer { appDelegate.debugOpenDiffViewerHandler = nil } + + guard let event = makeKeyDownEvent( + key: "d", + modifiers: [.command, .control], + keyCode: 2, // kVK_ANSI_D + windowNumber: targetWindow.windowNumber + ) else { + XCTFail("Failed to construct Cmd+Ctrl+D event") + return + } + +#if DEBUG + XCTAssertTrue( + appDelegate.debugHandleCustomShortcut(event: event), + "Cmd+Ctrl+D should be consumed by the Open Diff Viewer shortcut" + ) +#else + XCTFail("debugHandleCustomShortcut is only available in DEBUG") +#endif + XCTAssertEqual( + openDiffViewerCount, + 1, + "Cmd+Ctrl+D must route to the shared diff-open path (same path as the command palette)" + ) + } + func testCmdCtrlWPromptsBeforeClosingWindow() { guard let appDelegate = AppDelegate.shared else { XCTFail("Expected AppDelegate.shared") diff --git a/web/data/cmux-shortcuts.ts b/web/data/cmux-shortcuts.ts index bfb8b60b3906..1762dc605d02 100644 --- a/web/data/cmux-shortcuts.ts +++ b/web/data/cmux-shortcuts.ts @@ -220,6 +220,11 @@ export const shortcutCategories: ShortcutCategory[] = [ id: "diff-viewer", titleKey: "diffViewer", shortcuts: [ + { + id: "openDiffViewer", + combos: [["⌃", "⌘", "D"]], + description: { en: "Open diff viewer", ja: "差分ビューアを開く" }, + }, { id: "diffViewerScrollDown", combos: [["J"]], From fdcdd195e5c85088a89c19d33c3fc828f29c6ee5 Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Tue, 2 Jun 2026 01:17:36 -0700 Subject: [PATCH 2/6] Address review feedback: localize label, dedupe cwd helper, trim debug logs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Populate all 20 catalog locales for shortcut.openDiffViewer.label, reusing command.openDiffViewer.title's existing translations (identical English source). [greptile P1] - resolvedWorkingDirectory() now delegates to the existing configTrackingDirectory(for:) helper instead of duplicating the candidate-resolution logic. [cubic P3] - Diff-viewer DEBUG logs report only status, pid, and an output byte count — no raw child output or workspace cwd. [coderabbit] Co-Authored-By: Claude Opus 4.8 --- Resources/Localizable.xcstrings | 108 ++++++++++++++++++++++++++++++++ Sources/AppDelegate.swift | 12 ++-- Sources/Workspace.swift | 13 +--- 3 files changed, 114 insertions(+), 19 deletions(-) diff --git a/Resources/Localizable.xcstrings b/Resources/Localizable.xcstrings index 8ec089011640..7f3a2a1bcfd4 100644 --- a/Resources/Localizable.xcstrings +++ b/Resources/Localizable.xcstrings @@ -56,17 +56,125 @@ "shortcut.openDiffViewer.label": { "extractionState": "manual", "localizations": { + "ar": { + "stringUnit": { + "state": "translated", + "value": "فتح عارض الاختلافات" + } + }, + "bs": { + "stringUnit": { + "state": "translated", + "value": "Otvori preglednik razlika" + } + }, + "da": { + "stringUnit": { + "state": "translated", + "value": "Åbn diffvisning" + } + }, + "de": { + "stringUnit": { + "state": "translated", + "value": "Diff-Viewer öffnen" + } + }, "en": { "stringUnit": { "state": "translated", "value": "Open Diff Viewer" } }, + "es": { + "stringUnit": { + "state": "translated", + "value": "Abrir visor de diferencias" + } + }, + "fr": { + "stringUnit": { + "state": "translated", + "value": "Ouvrir la visionneuse de diff" + } + }, + "it": { + "stringUnit": { + "state": "translated", + "value": "Apri visualizzatore diff" + } + }, "ja": { "stringUnit": { "state": "translated", "value": "差分ビューアを開く" } + }, + "km": { + "stringUnit": { + "state": "translated", + "value": "Open Diff Viewer" + } + }, + "ko": { + "stringUnit": { + "state": "translated", + "value": "Diff 뷰어 열기" + } + }, + "nb": { + "stringUnit": { + "state": "translated", + "value": "Åpne diffvisning" + } + }, + "pl": { + "stringUnit": { + "state": "translated", + "value": "Otwórz podgląd różnic" + } + }, + "pt-BR": { + "stringUnit": { + "state": "translated", + "value": "Abrir visualizador de diff" + } + }, + "ru": { + "stringUnit": { + "state": "translated", + "value": "Открыть просмотр diff" + } + }, + "th": { + "stringUnit": { + "state": "translated", + "value": "เปิดตัวดู diff" + } + }, + "tr": { + "stringUnit": { + "state": "translated", + "value": "Diff Görüntüleyiciyi Aç" + } + }, + "uk": { + "stringUnit": { + "state": "translated", + "value": "Відкрити переглядач diff" + } + }, + "zh-Hans": { + "stringUnit": { + "state": "translated", + "value": "打开差异查看器" + } + }, + "zh-Hant": { + "stringUnit": { + "state": "translated", + "value": "開啟差異檢視器" + } } } }, diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index 352f04f99f86..09c5cdeeeb5a 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -6001,12 +6001,10 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent Task { @MainActor in AppDelegate.shared?.diffViewerProcesses.removeValue(forKey: processIdentifier) guard terminationStatus != 0 else { return } - let detail = output.trimmingCharacters(in: .whitespacesAndNewlines) - let limitedDetail = detail.isEmpty - ? "status=\(terminationStatus)" - : String(detail.prefix(240)) #if DEBUG - cmuxDebugLog("openDiffViewer exited status=\(terminationStatus) detail=\(limitedDetail)") + // Log only non-sensitive metadata: the child's stdout/stderr can echo + // repo paths and file contents, so report a byte count, not the text. + cmuxDebugLog("openDiffViewer exited status=\(terminationStatus) outputBytes=\(output.utf8.count)") #endif NSSound.beep() } @@ -6020,13 +6018,13 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent diffViewerProcesses.removeValue(forKey: processIdentifier) } #if DEBUG - cmuxDebugLog("openDiffViewer pid=\(process.processIdentifier) cwd=\(cwd)") + cmuxDebugLog("openDiffViewer pid=\(process.processIdentifier)") #endif return true } catch { outputCollector.cancel() #if DEBUG - cmuxDebugLog("openDiffViewer failed cwd=\(cwd) error=\(error.localizedDescription)") + cmuxDebugLog("openDiffViewer failed error=\(error.localizedDescription)") #endif return false } diff --git a/Sources/Workspace.swift b/Sources/Workspace.swift index 9aa0b528c093..4090f8ddb9b7 100644 --- a/Sources/Workspace.swift +++ b/Sources/Workspace.swift @@ -11593,18 +11593,7 @@ final class Workspace: Identifiable, ObservableObject { /// its terminal's requested directory, then the workspace's current directory. /// Returns `nil` when none is known so callers can apply their own fallback. func resolvedWorkingDirectory() -> String? { - let candidates = [ - focusedPanelId.flatMap { panelDirectories[$0] }, - focusedPanelId.flatMap { terminalPanel(for: $0)?.requestedWorkingDirectory }, - currentDirectory, - ] - for candidate in candidates { - let trimmed = candidate?.trimmingCharacters(in: .whitespacesAndNewlines) ?? "" - if !trimmed.isEmpty { - return trimmed - } - } - return nil + configTrackingDirectory(for: focusedPanelId) } private func surfaceKind(for panel: any Panel) -> String { From 453ab277c655293d817ecce9a3368fcd04160a0a Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Tue, 2 Jun 2026 01:57:22 -0700 Subject: [PATCH 3/6] Fix Release build: hoist diffViewerProcesses out of #if DEBUG, gate debug seam MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `diffViewerProcesses` and the `debugOpenDiffViewerHandler` test seam were declared inside AppDelegate's `#if DEBUG` block, but `openDiffViewerForFocusedWorkspace`/`launchDiffViewerProcess` referenced them unconditionally — so the Release build failed with "cannot find 'debugOpenDiffViewerHandler' in scope" and "no member 'diffViewerProcesses'" (release-build CI job; the Debug `tests` build compiled fine, which is why it slipped through initially). - `diffViewerProcesses` is production state (CLI process retention) → declared outside `#if DEBUG`. - `debugOpenDiffViewerHandler` stays a DEBUG-only seam; its use is now `#if DEBUG`-gated, matching the existing `debugCloseMainWindowConfirmationHandler` pattern. Verified with a local `xcodebuild -configuration Release` (arm64): the cmux app target compiles cleanly. Co-Authored-By: Claude Opus 4.8 --- Sources/AppDelegate.swift | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index 09c5cdeeeb5a..e63beb8fe1c9 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -942,6 +942,10 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent method_exchangeImplementations(originalMethod, swizzledMethod) }() + /// Live `cmux diff` viewer subprocesses, keyed by pid, retained until they exit. + /// Declared outside `#if DEBUG` because process retention is production behavior. + private var diffViewerProcesses: [Int32: Process] = [:] + #if DEBUG private var didSetupJumpUnreadUITest = false private var jumpUnreadFocusExpectation: (tabId: UUID, surfaceId: UUID)? @@ -976,8 +980,6 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent /// instead of spawning the bundled `cmux diff` CLI, so shortcut-dispatch tests can /// assert routing without launching a subprocess. var debugOpenDiffViewerHandler: (() -> Void)? - /// Live `cmux diff` viewer subprocesses, keyed by pid, retained until they exit. - private var diffViewerProcesses: [Int32: Process] = [:] var debugCreateMainWindowSourceIsNativeFullScreenOverride: Bool? // Keep debug-only windows alive when tests intentionally inject key mismatches. private var debugDetachedContextWindows: [NSWindow] = [] @@ -5931,10 +5933,12 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent /// there is no focused workspace or the bundled CLI is missing. @discardableResult func openDiffViewerForFocusedWorkspace(for tabManager: TabManager?) -> Bool { +#if DEBUG if let debugOpenDiffViewerHandler { debugOpenDiffViewerHandler() return true } +#endif guard let workspace = tabManager?.selectedWorkspace, let cliURL = Bundle.main.resourceURL?.appendingPathComponent("bin/cmux"), FileManager.default.isExecutableFile(atPath: cliURL.path) else { From c05d39c7d70c2298bbabcf7c8bab0fd2a98daba9 Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Tue, 2 Jun 2026 02:11:37 -0700 Subject: [PATCH 4/6] Log error type instead of localizedDescription on diff-viewer spawn failure Process spawn failures surface file paths via error.localizedDescription (e.g. "The file doesn't exist at /path/to/bin/cmux"). Consistent with the rest of this DEBUG log's path/output redaction, log type(of: error) instead. Co-Authored-By: Claude Opus 4.8 --- Sources/AppDelegate.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index e63beb8fe1c9..de7206ee0242 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -6028,7 +6028,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent } catch { outputCollector.cancel() #if DEBUG - cmuxDebugLog("openDiffViewer failed error=\(error.localizedDescription)") + cmuxDebugLog("openDiffViewer failed errorType=\(type(of: error))") #endif return false } From c79752de37fa45b78ceb6cc116abd1c7938b5215 Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Tue, 2 Jun 2026 02:45:51 -0700 Subject: [PATCH 5/6] Inline the explicit working-directory fallback chain in resolvedWorkingDirectory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit (blocking) flagged that the delegating one-liner didn't visibly honor the documented three-tier order. Spell the tiers out in the method body — panelDirectories[focusedPanelId] -> focused terminal's requestedWorkingDirectory -> currentDirectory -> nil — matching configTrackingDirectory(for:)'s behavior. Co-Authored-By: Claude Opus 4.8 --- Sources/Workspace.swift | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/Sources/Workspace.swift b/Sources/Workspace.swift index 4090f8ddb9b7..b6410835dd2d 100644 --- a/Sources/Workspace.swift +++ b/Sources/Workspace.swift @@ -11592,8 +11592,23 @@ final class Workspace: Identifiable, ObservableObject { /// should target for this workspace: the focused panel's tracked directory, then /// its terminal's requested directory, then the workspace's current directory. /// Returns `nil` when none is known so callers can apply their own fallback. + /// + /// This is the focused-panel case of ``configTrackingDirectory(for:)`` (the same + /// three-tier order); the tiers are spelled out here so the public entry point is + /// self-contained. func resolvedWorkingDirectory() -> String? { - configTrackingDirectory(for: focusedPanelId) + let candidates = [ + focusedPanelId.flatMap { panelDirectories[$0] }, + focusedPanelId.flatMap { terminalPanel(for: $0)?.requestedWorkingDirectory }, + currentDirectory, + ] + for candidate in candidates { + let trimmed = candidate?.trimmingCharacters(in: .whitespacesAndNewlines) ?? "" + if !trimmed.isEmpty { + return trimmed + } + } + return nil } private func surfaceKind(for panel: any Panel) -> String { From f5c3051b83267902acd835372a6ad7f66fa106c6 Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Tue, 2 Jun 2026 03:51:57 -0700 Subject: [PATCH 6/6] Fix Open Diff Viewer: show in Settings UI + use a default macOS doesn't eat Two real bugs the first pass missed (verified by driving the running app): 1. Settings UI: the keyboard-shortcuts pane is rendered from the CmuxSettings.ShortcutAction package enum, NOT KeyboardShortcutSettings.Action. `openDiffViewer` was never added there, so "Open Diff Viewer" never appeared in Settings and couldn't be rebound in the UI. Add the case (+ group, displayName, defaultStroke) and a search synonym so "open diff" surfaces the Keyboard Shortcuts section. 2. Default shortcut: macOS reserves Cmd+Ctrl+D for "Look Up & data detectors" and swallows the key before it reaches the app's monitor (confirmed: a Cmd+Ctrl+D keystroke never reaches handleCustomShortcut, while Cmd+, does). The rest of the Cmd+D family is taken by split actions, so move the default to Cmd+Ctrl+Shift+D, which reaches cmux and opens the viewer (verified end-to-end). Updates the dispatch test and the keyboard-shortcut/configuration docs accordingly. Co-Authored-By: Claude Opus 4.8 --- .../Values/ShortcutAction+Defaults.swift | 1 + .../CmuxSettings/Values/ShortcutAction.swift | 4 +++- .../CuratedSettingEntry+Default.swift | 2 +- Sources/KeyboardShortcutSettings.swift | 14 ++++++------- .../AppDelegateShortcutRoutingTests.swift | 21 ++++++++++--------- web/data/cmux-shortcuts.ts | 2 +- 6 files changed, 24 insertions(+), 20 deletions(-) diff --git a/Packages/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction+Defaults.swift b/Packages/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction+Defaults.swift index b44e659b0c01..b719609b7946 100644 --- a/Packages/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction+Defaults.swift +++ b/Packages/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction+Defaults.swift @@ -69,6 +69,7 @@ extension ShortcutAction { case .focusTextBoxInput: return ShortcutStroke(key: "a", command: true, shift: true) case .attachTextBoxFile: return ShortcutStroke(key: "a", command: true, shift: true, option: true) case .toggleRightSidebar: return ShortcutStroke(key: "b", command: true, option: true) + case .openDiffViewer: return ShortcutStroke(key: "d", command: true, shift: true, control: true) case .saveFilePreview: return ShortcutStroke(key: "s", command: true) case .openBrowser: return ShortcutStroke(key: "l", command: true, shift: true) case .focusBrowserAddressBar: return ShortcutStroke(key: "l", command: true) diff --git a/Packages/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction.swift b/Packages/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction.swift index 93af8a0bdeb5..88d538c1f7b2 100644 --- a/Packages/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction.swift +++ b/Packages/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction.swift @@ -78,6 +78,7 @@ public enum ShortcutAction: String, CaseIterable, Sendable, Hashable, SettingCod case toggleRightSidebar = "toggleFileExplorer" // MARK: Browser & Find + case openDiffViewer case saveFilePreview case openBrowser case focusBrowserAddressBar @@ -145,7 +146,7 @@ extension ShortcutAction { .toggleSplitZoom, .equalizeSplits, .splitBrowserRight, .splitBrowserDown, .toggleRightSidebar: return .panes - case .saveFilePreview, .openBrowser, .focusBrowserAddressBar, .browserBack, + case .openDiffViewer, .saveFilePreview, .openBrowser, .focusBrowserAddressBar, .browserBack, .browserForward, .browserReload, .browserZoomIn, .browserZoomOut, .browserZoomReset, .markdownZoomIn, .markdownZoomOut, .markdownZoomReset, .find, .findInDirectory, .findNext, .findPrevious, @@ -216,6 +217,7 @@ extension ShortcutAction { case .splitBrowserRight: return "Split Browser Right" case .splitBrowserDown: return "Split Browser Down" case .toggleRightSidebar: return "Toggle Right Sidebar" + case .openDiffViewer: return "Open Diff Viewer" case .saveFilePreview: return "Save File Preview" case .openBrowser: return "Open Browser" case .focusBrowserAddressBar: return "Focus Address Bar" diff --git a/Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/CuratedSettingEntry+Default.swift b/Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/CuratedSettingEntry+Default.swift index 3b024cc26743..32389621ff4b 100644 --- a/Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/CuratedSettingEntry+Default.swift +++ b/Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/CuratedSettingEntry+Default.swift @@ -127,7 +127,7 @@ extension Array where Element == CuratedSettingEntry { .init(section: .globalHotkey, id: "shortcut", title: "Show/Hide All Windows", synonyms: "global hotkey shortcut recorder key command option control"), // Keyboard shortcuts - .init(section: .keyboardShortcuts, id: "shortcuts", title: "Keyboard Shortcuts", synonyms: "shortcuts.bindings hotkeys keybindings key bindings commands keyboard accelerators chords cmux json"), + .init(section: .keyboardShortcuts, id: "shortcuts", title: "Keyboard Shortcuts", synonyms: "shortcuts.bindings hotkeys keybindings key bindings commands keyboard accelerators chords cmux json open diff viewer changes review git unstaged"), .init(section: .keyboardShortcuts, id: "shortcut-chords", title: "Shortcut Chords", synonyms: "tmux prefix ctrl-b control-b multi key sequence chord cmux json"), .init(section: .keyboardShortcuts, id: "reset-defaults", title: "Reset Default Shortcuts", synonyms: "reset restore default defaults built in builtin shortcuts hotkeys keybindings commands"), diff --git a/Sources/KeyboardShortcutSettings.swift b/Sources/KeyboardShortcutSettings.swift index 64f8ff4273e9..7ceda4290d8b 100644 --- a/Sources/KeyboardShortcutSettings.swift +++ b/Sources/KeyboardShortcutSettings.swift @@ -455,13 +455,13 @@ enum KeyboardShortcutSettings { case .toggleReactGrab: return StoredShortcut(key: "g", command: true, shift: true, option: false, control: false) case .openDiffViewer: - // Cmd+Ctrl+D. The natural "D for Diff" chord (Cmd+Shift+D) collides - // with Split Down, and the entire Cmd-based "D" family is already - // taken by split actions (Cmd+D, Cmd+Shift+D, Cmd+Opt+D, - // Cmd+Shift+Opt+D). Cmd+Ctrl+D is free and matches cmux's - // Cmd+Ctrl+ app-shortcut family (Close Window = Cmd+Ctrl+W, - // Toggle Full Screen = Cmd+Ctrl+F). - return StoredShortcut(key: "d", command: true, shift: false, option: false, control: true) + // Cmd+Ctrl+Shift+D. The plain Cmd+Ctrl+D chord is reserved by macOS for + // "Look Up & data detectors" — the OS swallows it before it reaches the + // app's key monitor — and the rest of the Cmd-based "D" family is taken + // by split actions (Cmd+D, Cmd+Shift+D, Cmd+Opt+D, Cmd+Shift+Opt+D). + // Adding Shift yields a chord that reaches cmux while keeping the "D for + // Diff" mnemonic. Rebindable in Settings → Keyboard Shortcuts. + return StoredShortcut(key: "d", command: true, shift: true, option: false, control: true) case .diffViewerScrollDown: return StoredShortcut(key: "j", command: false, shift: false, option: false, control: false) case .diffViewerScrollUp: diff --git a/cmuxTests/AppDelegateShortcutRoutingTests.swift b/cmuxTests/AppDelegateShortcutRoutingTests.swift index c74be2f99530..e82cc3fc259a 100644 --- a/cmuxTests/AppDelegateShortcutRoutingTests.swift +++ b/cmuxTests/AppDelegateShortcutRoutingTests.swift @@ -1977,13 +1977,14 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { return } - // Default is Cmd+Ctrl+D (the natural Cmd+Shift+D collides with Split Down), and - // it must be conflict-free so the recorder accepts it as-is. - let cmdCtrlD = StoredShortcut(key: "d", command: true, shift: false, option: false, control: true) - XCTAssertEqual(KeyboardShortcutSettings.shortcut(for: .openDiffViewer), cmdCtrlD) + // Default is Cmd+Ctrl+Shift+D. Plain Cmd+Ctrl+D is reserved by macOS ("Look Up") + // and never reaches the app, and the rest of the Cmd+D family is taken by split + // actions; the default must be conflict-free so the recorder accepts it as-is. + let cmdCtrlShiftD = StoredShortcut(key: "d", command: true, shift: true, option: false, control: true) + XCTAssertEqual(KeyboardShortcutSettings.shortcut(for: .openDiffViewer), cmdCtrlShiftD) XCTAssertEqual( - KeyboardShortcutSettings.Action.openDiffViewer.normalizedRecordedShortcutResult(cmdCtrlD), - .accepted(cmdCtrlD), + KeyboardShortcutSettings.Action.openDiffViewer.normalizedRecordedShortcutResult(cmdCtrlShiftD), + .accepted(cmdCtrlShiftD), "Default Open Diff Viewer shortcut must not conflict with any other action" ) XCTAssertTrue( @@ -2006,18 +2007,18 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { guard let event = makeKeyDownEvent( key: "d", - modifiers: [.command, .control], + modifiers: [.command, .control, .shift], keyCode: 2, // kVK_ANSI_D windowNumber: targetWindow.windowNumber ) else { - XCTFail("Failed to construct Cmd+Ctrl+D event") + XCTFail("Failed to construct Cmd+Ctrl+Shift+D event") return } #if DEBUG XCTAssertTrue( appDelegate.debugHandleCustomShortcut(event: event), - "Cmd+Ctrl+D should be consumed by the Open Diff Viewer shortcut" + "Cmd+Ctrl+Shift+D should be consumed by the Open Diff Viewer shortcut" ) #else XCTFail("debugHandleCustomShortcut is only available in DEBUG") @@ -2025,7 +2026,7 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { XCTAssertEqual( openDiffViewerCount, 1, - "Cmd+Ctrl+D must route to the shared diff-open path (same path as the command palette)" + "Cmd+Ctrl+Shift+D must route to the shared diff-open path (same path as the command palette)" ) } diff --git a/web/data/cmux-shortcuts.ts b/web/data/cmux-shortcuts.ts index 1762dc605d02..85d5cabda390 100644 --- a/web/data/cmux-shortcuts.ts +++ b/web/data/cmux-shortcuts.ts @@ -222,7 +222,7 @@ export const shortcutCategories: ShortcutCategory[] = [ shortcuts: [ { id: "openDiffViewer", - combos: [["⌃", "⌘", "D"]], + combos: [["⌃", "⌘", "⇧", "D"]], description: { en: "Open diff viewer", ja: "差分ビューアを開く" }, }, {