Cmd+P: show workspaces only (#844)
* Limit Cmd+P switcher to workspaces * Fix command palette Enter/Escape handling and add regression test * Scope command palette key handling to event window
This commit is contained in:
parent
1f62d770c7
commit
dad52b09d9
5 changed files with 196 additions and 113 deletions
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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)")
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue