diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index b70ed1d5..1c88b6cc 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -1080,6 +1080,43 @@ func shouldConsumeShortcutWhileCommandPaletteVisible( return true } +func shouldSubmitCommandPaletteWithReturn( + keyCode: UInt16, + flags: NSEvent.ModifierFlags +) -> Bool { + guard keyCode == 36 || keyCode == 76 else { return false } + let normalizedFlags = flags + .intersection(.deviceIndependentFlagsMask) + .subtracting([.numericPad, .function, .capsLock]) + return normalizedFlags == [] || normalizedFlags == [.shift] +} + +func commandPaletteFieldEditorHasMarkedText(in window: NSWindow) -> Bool { + guard let editor = window.firstResponder as? NSTextView, + editor.isFieldEditor else { + return false + } + return editor.hasMarkedText() +} + +func shouldHandleCommandPaletteShortcutEvent( + _ event: NSEvent, + paletteWindow: NSWindow? +) -> Bool { + guard let paletteWindow else { return false } + if let eventWindow = event.window { + return eventWindow === paletteWindow + } + let eventWindowNumber = event.windowNumber + if eventWindowNumber > 0 { + return eventWindowNumber == paletteWindow.windowNumber + } + if let keyWindow = NSApp.keyWindow { + return keyWindow === paletteWindow + } + return false +} + enum BrowserZoomShortcutAction: Equatable { case zoomIn case zoomOut @@ -5787,7 +5824,11 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent let normalizedFlags = flags.subtracting([.numericPad, .function, .capsLock]) let commandPaletteTargetWindow = commandPaletteWindowForShortcutEvent(event) - let commandPaletteVisibleInTargetWindow = commandPaletteTargetWindow.map { + let commandPaletteShortcutWindow = shouldHandleCommandPaletteShortcutEvent( + event, + paletteWindow: commandPaletteTargetWindow + ) ? commandPaletteTargetWindow : nil + let commandPaletteVisibleInTargetWindow = commandPaletteShortcutWindow.map { isCommandPaletteVisible(for: $0) } ?? false @@ -5797,7 +5838,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent keyCode: event.keyCode ), commandPaletteVisibleInTargetWindow, - let paletteWindow = commandPaletteTargetWindow { + let paletteWindow = commandPaletteShortcutWindow { NotificationCenter.default.post( name: .commandPaletteMoveSelection, object: paletteWindow, @@ -5806,6 +5847,29 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent return true } + if commandPaletteVisibleInTargetWindow, + let paletteWindow = commandPaletteShortcutWindow { + let paletteFieldEditorHasMarkedText = commandPaletteFieldEditorHasMarkedText(in: paletteWindow) + if normalizedFlags.isEmpty, event.keyCode == 53 { + if paletteFieldEditorHasMarkedText { + return false + } + NotificationCenter.default.post(name: .commandPaletteDismissRequested, object: paletteWindow) + return true + } + + if shouldSubmitCommandPaletteWithReturn( + keyCode: event.keyCode, + flags: event.modifierFlags + ) { + if paletteFieldEditorHasMarkedText { + return false + } + NotificationCenter.default.post(name: .commandPaletteSubmitRequested, object: paletteWindow) + return true + } + } + // Guard against stale browserAddressBarFocusedPanelId after focus transitions // (e.g., split that doesn't properly blur the address bar). If the first responder // is a terminal surface, the address bar can't be focused. diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index dd19fc85..2b57e7fa 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -2282,6 +2282,30 @@ struct ContentView: View { openCommandPaletteSwitcher() }) + view = AnyView(view.onReceive(NotificationCenter.default.publisher(for: .commandPaletteSubmitRequested)) { notification in + guard isCommandPalettePresented else { return } + let requestedWindow = notification.object as? NSWindow + guard Self.shouldHandleCommandPaletteRequest( + observedWindow: observedWindow, + requestedWindow: requestedWindow, + keyWindow: NSApp.keyWindow, + mainWindow: NSApp.mainWindow + ) else { return } + handleCommandPaletteSubmitRequest() + }) + + view = AnyView(view.onReceive(NotificationCenter.default.publisher(for: .commandPaletteDismissRequested)) { notification in + guard isCommandPalettePresented else { return } + let requestedWindow = notification.object as? NSWindow + guard Self.shouldHandleCommandPaletteRequest( + observedWindow: observedWindow, + requestedWindow: requestedWindow, + keyWindow: NSApp.keyWindow, + mainWindow: NSApp.mainWindow + ) else { return } + dismissCommandPalette() + }) + view = AnyView(view.onReceive(NotificationCenter.default.publisher(for: .commandPaletteRenameTabRequested)) { notification in let requestedWindow = notification.object as? NSWindow guard Self.shouldHandleCommandPaletteRequest( @@ -2963,7 +2987,7 @@ struct ContentView: View { case .commands: return "Type a command" case .switcher: - return "Search workspaces and tabs" + return "Search workspaces" } } @@ -2972,7 +2996,7 @@ struct ContentView: View { case .commands: return "No commands match your search." case .switcher: - return "No workspaces or tabs match your search." + return "No workspaces match your search." } } @@ -3067,9 +3091,6 @@ struct ContentView: View { if command.id.hasPrefix("switcher.workspace.") { return CommandPaletteTrailingLabel(text: "Workspace", style: .kind) } - if command.id.hasPrefix("switcher.surface.") { - return CommandPaletteTrailingLabel(text: "Surface", style: .kind) - } return nil } @@ -3079,7 +3100,7 @@ struct ContentView: View { var entries: [CommandPaletteCommand] = [] let estimatedCount = windowContexts.reduce(0) { partial, context in - partial + max(1, context.tabManager.tabs.count) * 4 + partial + context.tabManager.tabs.count } entries.reserveCapacity(estimatedCount) var nextRank = 0 @@ -3126,62 +3147,12 @@ struct ContentView: View { focusCommandPaletteSwitcherTarget( windowId: windowId, tabManager: windowTabManager, - workspaceId: workspaceId, - panelId: nil + workspaceId: workspaceId ) } ) ) nextRank += 1 - - var orderedPanelIds = workspace.sidebarOrderedPanelIds() - if let focusedPanelId = workspace.focusedPanelId, - let focusedIndex = orderedPanelIds.firstIndex(of: focusedPanelId) { - orderedPanelIds.remove(at: focusedIndex) - orderedPanelIds.insert(focusedPanelId, at: 0) - } - - for panelId in orderedPanelIds { - guard let panel = workspace.panels[panelId] else { continue } - let panelTitle = panelDisplayName(workspace: workspace, panelId: panelId, fallback: panel.displayTitle) - let typeLabel: String = (panel.panelType == .browser) ? "Browser" : "Terminal" - let panelKeywords = CommandPaletteSwitcherSearchIndexer.keywords( - baseKeywords: [ - "tab", - "surface", - "panel", - "switch", - "go", - workspaceName, - panelTitle, - typeLabel.lowercased() - ] + windowKeywords, - metadata: commandPalettePanelSearchMetadata(in: workspace, panelId: panelId) - ) - entries.append( - CommandPaletteCommand( - id: "switcher.surface.\(workspace.id.uuidString.lowercased()).\(panelId.uuidString.lowercased())", - rank: nextRank, - title: panelTitle, - subtitle: commandPaletteSwitcherSubtitle( - base: "\(typeLabel) • \(workspaceName)", - windowLabel: context.windowLabel - ), - shortcutHint: nil, - keywords: panelKeywords, - dismissOnRun: true, - action: { - focusCommandPaletteSwitcherTarget( - windowId: windowId, - tabManager: windowTabManager, - workspaceId: workspaceId, - panelId: panelId - ) - } - ) - ) - nextRank += 1 - } } } @@ -3250,24 +3221,19 @@ struct ContentView: View { private func focusCommandPaletteSwitcherTarget( windowId: UUID, tabManager: TabManager, - workspaceId: UUID, - panelId: UUID? + workspaceId: UUID ) { // Switcher commands dismiss the palette after action dispatch. // Defer focus mutation one turn so browser omnibar autofocus can run // without being blocked by the palette-visibility guard. DispatchQueue.main.async { _ = AppDelegate.shared?.focusMainWindow(windowId: windowId) - if let panelId { - tabManager.focusTab(workspaceId, surfaceId: panelId, suppressFlash: true) - } else { - tabManager.focusTab(workspaceId, suppressFlash: true) - } + tabManager.focusTab(workspaceId, suppressFlash: true) } } private func commandPaletteWorkspaceSearchMetadata(for workspace: Workspace) -> CommandPaletteSwitcherSearchMetadata { - // Keep workspace rows coarse so surface rows win for directory/branch-specific queries. + // Keep workspace rows coarse and stable for predictable workspace switching queries. let directories = [workspace.currentDirectory] let branches = [workspace.gitBranch?.branch].compactMap { $0 } let ports = workspace.listeningPorts @@ -3278,33 +3244,6 @@ struct ContentView: View { ) } - private func commandPalettePanelSearchMetadata(in workspace: Workspace, panelId: UUID) -> CommandPaletteSwitcherSearchMetadata { - var directories: [String] = [] - if let directory = workspace.panelDirectories[panelId] { - directories.append(directory) - } else if workspace.focusedPanelId == panelId { - directories.append(workspace.currentDirectory) - } - - var branches: [String] = [] - if let branch = workspace.panelGitBranches[panelId]?.branch { - branches.append(branch) - } else if workspace.focusedPanelId == panelId, let branch = workspace.gitBranch?.branch { - branches.append(branch) - } - - var ports = workspace.surfaceListeningPorts[panelId] ?? [] - if ports.isEmpty, workspace.panels.count == 1 { - ports = workspace.listeningPorts - } - - return CommandPaletteSwitcherSearchMetadata( - directories: directories, - branches: branches, - ports: ports - ) - } - private func commandPaletteCommands() -> [CommandPaletteCommand] { let context = commandPaletteContextSnapshot() let contributions = commandPaletteCommandContributions() @@ -4543,6 +4482,17 @@ struct ContentView: View { runCommandPaletteCommand(visibleResults[index].command) } + private func handleCommandPaletteSubmitRequest() { + switch commandPaletteMode { + case .commands: + runSelectedCommandPaletteResult() + case .renameInput(let target): + continueRenameFlow(target: target) + case .renameConfirm(let target, let proposedName): + applyRenameFlow(target: target, proposedName: proposedName) + } + } + private func runCommandPaletteCommand(_ command: CommandPaletteCommand) { #if DEBUG dlog("palette.run commandId=\(command.id) dismissOnRun=\(command.dismissOnRun ? 1 : 0)") diff --git a/Sources/TabManager.swift b/Sources/TabManager.swift index aaeac1fe..64fc28dc 100644 --- a/Sources/TabManager.swift +++ b/Sources/TabManager.swift @@ -3635,6 +3635,8 @@ extension Notification.Name { static let commandPaletteToggleRequested = Notification.Name("cmux.commandPaletteToggleRequested") static let commandPaletteRequested = Notification.Name("cmux.commandPaletteRequested") static let commandPaletteSwitcherRequested = Notification.Name("cmux.commandPaletteSwitcherRequested") + static let commandPaletteSubmitRequested = Notification.Name("cmux.commandPaletteSubmitRequested") + static let commandPaletteDismissRequested = Notification.Name("cmux.commandPaletteDismissRequested") static let commandPaletteRenameTabRequested = Notification.Name("cmux.commandPaletteRenameTabRequested") static let commandPaletteRenameWorkspaceRequested = Notification.Name("cmux.commandPaletteRenameWorkspaceRequested") static let commandPaletteMoveSelection = Notification.Name("cmux.commandPaletteMoveSelection") diff --git a/Sources/cmuxApp.swift b/Sources/cmuxApp.swift index 9ae5d3a5..3fff7055 100644 --- a/Sources/cmuxApp.swift +++ b/Sources/cmuxApp.swift @@ -418,7 +418,7 @@ struct cmuxApp: App { // Close tab/workspace CommandGroup(after: .newItem) { - Button("Go to Workspace or Tab…") { + Button("Go to Workspace…") { let targetWindow = NSApp.keyWindow ?? NSApp.mainWindow NotificationCenter.default.post(name: .commandPaletteSwitcherRequested, object: targetWindow) } diff --git a/tests/test_browser_new_tab_surface_focus_omnibar.py b/tests/test_browser_new_tab_surface_focus_omnibar.py index b66bb505..cea196c5 100644 --- a/tests/test_browser_new_tab_surface_focus_omnibar.py +++ b/tests/test_browser_new_tab_surface_focus_omnibar.py @@ -4,7 +4,8 @@ Regression test: 1. Focusing a blank browser surface should focus the omnibar. 2. Focusing a pane that contains a blank browser should focus the omnibar. 3. If command palette is open, focusing that blank browser surface must not steal input. -4. Cmd+P switcher focusing an existing blank browser surface should focus the omnibar. +4. Cmd+P switcher should list only workspaces, then switching to a workspace with a + focused blank browser should focus the omnibar. """ import json @@ -281,24 +282,47 @@ def main() -> int: workspace_ids.remove(workspace_id) time.sleep(0.3) - # Scenario 4: Cmd+P switcher selecting an existing blank browser surface should focus omnibar. - workspace_id = client.new_workspace() - workspace_ids.append(workspace_id) - client.select_workspace(workspace_id) + # Scenario 4: Cmd+P switcher should only list workspaces, and switching to a workspace + # that has a focused blank browser should focus the omnibar. + target_workspace_id = client.new_workspace() + workspace_ids.append(target_workspace_id) + client.select_workspace(target_workspace_id) time.sleep(0.4) window_id = current_window_id(client) if not set_command_palette_visible(client, window_id, False): - raise cmuxError("Failed to reset command palette before scenario 4") + raise cmuxError("Failed to reset command palette before scenario 4 (target setup)") switcher_browser_id = client.new_surface(panel_type="browser") time.sleep(0.3) + client.focus_surface_by_panel(switcher_browser_id) - switcher_surfaces = client.list_surfaces() - switcher_terminal_id = next((surface_id for _, surface_id, _ in switcher_surfaces if surface_id != switcher_browser_id), None) - if not switcher_terminal_id: - raise cmuxError("Missing terminal surface for Cmd+P switcher scenario") + did_focus_target_browser = wait_for( + lambda: bool( + browser_address_bar_focus_state( + client, + surface_id=switcher_browser_id, + request_id="browser-focus-switcher-target-setup" + ).get("focused") + ), + timeout_s=3.0, + interval_s=0.1 + ) + if not did_focus_target_browser: + raise cmuxError("Failed to focus omnibar on target workspace browser before Cmd+P switch") - client.focus_surface_by_panel(switcher_terminal_id) + source_workspace_id = client.new_workspace() + workspace_ids.append(source_workspace_id) + client.select_workspace(source_workspace_id) + time.sleep(0.4) + window_id = current_window_id(client) + if not set_command_palette_visible(client, window_id, False): + raise cmuxError("Failed to reset command palette before scenario 4 (source setup)") + + source_surfaces = client.list_surfaces() + source_terminal_id = next((surface_id for _, surface_id, _ in source_surfaces), None) + if not source_terminal_id: + raise cmuxError("Missing terminal surface for Cmd+P workspace switcher scenario") + client.focus_surface_by_panel(source_terminal_id) time.sleep(0.2) client.simulate_shortcut("cmd+p") @@ -316,11 +340,13 @@ def main() -> int: ): raise cmuxError("Cmd+P did not open command palette switcher") - client.simulate_type("new tab") - time.sleep(0.2) + switcher_results = command_palette_results(client, window_id, limit=100) + switcher_ids = [row.get("command_id") for row in switcher_results if isinstance(row.get("command_id"), str)] + has_surface_rows = any(command_id.startswith("switcher.surface.") for command_id in switcher_ids) + if has_surface_rows: + raise cmuxError("Cmd+P switcher listed unexpected surface rows; expected workspace-only results") - target_command_id = f"switcher.surface.{workspace_id.lower()}.{switcher_browser_id.lower()}" - switcher_results = command_palette_results(client, window_id, limit=50) + target_command_id = f"switcher.workspace.{target_workspace_id.lower()}" target_index = next( ( idx for idx, row in enumerate(switcher_results) @@ -329,7 +355,7 @@ def main() -> int: None ) if target_index is None: - raise cmuxError(f"Cmd+P switcher did not list target surface command {target_command_id}") + raise cmuxError(f"Cmd+P switcher did not list target workspace command {target_command_id}") if not move_command_palette_selection_to_index(client, window_id, target_index): raise cmuxError(f"Failed to move Cmd+P selection to result index {target_index}") @@ -358,9 +384,50 @@ def main() -> int: interval_s=0.1 ) if not did_focus_switcher_target: - raise cmuxError("Cmd+P switcher focus to blank browser did not focus omnibar") + raise cmuxError("Cmd+P workspace switch did not restore blank browser omnibar focus") - print("PASS: blank-browser focus paths (surface, pane, and Cmd+P switcher) drive omnibar, while command palette visibility blocks focus stealing") + # Scenario 5: Cmd+P switcher should dismiss on Escape reliably. + client.select_workspace(source_workspace_id) + time.sleep(0.4) + window_id = current_window_id(client) + if not set_command_palette_visible(client, window_id, False): + raise cmuxError("Failed to reset command palette before scenario 5") + + client.focus_surface_by_panel(source_terminal_id) + time.sleep(0.2) + + client.simulate_shortcut("cmd+p") + if not wait_for( + lambda: bool( + v2_call( + client, + "debug.command_palette.visible", + {"window_id": window_id}, + request_id="palette-visible-switcher-open-escape" + ).get("visible") + ), + timeout_s=2.0, + interval_s=0.1 + ): + raise cmuxError("Cmd+P did not open command palette switcher before Escape scenario") + + client.simulate_shortcut("escape") + did_dismiss_switcher_on_escape = wait_for( + lambda: not bool( + v2_call( + client, + "debug.command_palette.visible", + {"window_id": window_id}, + request_id="palette-visible-switcher-after-escape" + ).get("visible") + ), + timeout_s=3.0, + interval_s=0.1 + ) + if not did_dismiss_switcher_on_escape: + raise cmuxError("Cmd+P Escape did not dismiss command palette switcher") + + print("PASS: blank-browser focus paths (surface, pane, Cmd+P Enter switcher, and Cmd+P Escape dismiss) drive omnibar, while command palette visibility blocks focus stealing") return 0 except cmuxError as exc: