marketing-shibata50/cmux
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(command-palette): address follow-up review comments

Browse source 
PR: https://github.com/manaflow-ai/cmux/pull/1644
This commit is contained in:
Lawrence Chen 2026-03-17 18:59:13 -07:00 • committed by GitHub
parent 1fabe9f33c
commit 3bca43d6b1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 155 additions and 46 deletions
Download patch file Download diff file Expand all files Collapse all files

93
Sources/ContentView.swift
View file

@ -1405,6 +1405,7 @@ struct ContentView: View {
@State private var commandPaletteResolvedSearchScope: CommandPaletteListScope?
@State private var commandPaletteResolvedSearchFingerprint: Int?
@State private var commandPaletteResolvedMatchingQuery = ""
@State private var commandPaletteTerminalOpenTargetAvailability: Set<TerminalDirectoryOpenTarget> = []
@State private var isCommandPaletteSearchPending = false
@State private var commandPalettePendingActivation: CommandPalettePendingActivation?
@State private var commandPaletteResultsRevision: UInt64 = 0
@ -1566,6 +1567,10 @@ struct ContentView: View {
}
}
private struct CommandPaletteCommandsContext {
let snapshot: CommandPaletteContextSnapshot
}
private enum CommandPaletteContextKeys {
static let hasWorkspace = "workspace.hasSelection"
static let workspaceName = "workspace.name"
@ -1588,6 +1593,7 @@ struct ContentView: View {
static let panelHasUnread = "panel.hasUnread"
static let updateHasAvailable = "update.hasAvailable"
static let cliInstalledInPATH = "cli.installedInPATH"
static func terminalOpenTargetAvailable(_ target: TerminalDirectoryOpenTarget) -> String {
"terminal.openTarget.\(target.rawValue).available"
@ -3753,9 +3759,11 @@ struct ContentView: View {
}
private var commandPaletteCurrentSearchFingerprint: Int {
commandPaletteEntriesFingerprint(
for: commandPaletteListScope,
includeSurfaces: commandPaletteSwitcherIncludesSurfaceEntries
let scope = commandPaletteListScope
return commandPaletteEntriesFingerprint(
for: scope,
includeSurfaces: commandPaletteSwitcherIncludesSurfaceEntries,
commandsContext: scope == .commands ? commandPaletteCachedCommandsContext() : nil
)
}
@ -3859,11 +3867,12 @@ struct ContentView: View {
private func commandPaletteEntries(
for scope: CommandPaletteListScope,
includeSurfaces: Bool
includeSurfaces: Bool,
commandsContext: CommandPaletteCommandsContext? = nil
) -> [CommandPaletteCommand] {
switch scope {
case .commands:
return commandPaletteCommands()
return commandPaletteCommands(commandsContext: commandsContext ?? commandPaletteCachedCommandsContext())
case .switcher:
return commandPaletteSwitcherEntries(includeSurfaces: includeSurfaces)
}
@ -3891,15 +3900,27 @@ struct ContentView: View {
searchAllSurfaces: commandPaletteSearchAllSurfaces,
query: effectiveQuery
)
let terminalOpenTargets = resolveCommandPaletteTerminalOpenTargets(for: scope)
if commandPaletteTerminalOpenTargetAvailability != terminalOpenTargets {
commandPaletteTerminalOpenTargetAvailability = terminalOpenTargets
}
let commandsContext = scope == .commands
? commandPaletteCommandsContext(terminalOpenTargets: terminalOpenTargets)
: nil
let fingerprint = commandPaletteEntriesFingerprint(
for: scope,
includeSurfaces: includeSurfaces
includeSurfaces: includeSurfaces,
commandsContext: commandsContext
)
guard force || cachedCommandPaletteScope != scope || cachedCommandPaletteFingerprint != fingerprint else {
return
}
let entries = commandPaletteEntries(for: scope, includeSurfaces: includeSurfaces)
let entries = commandPaletteEntries(
for: scope,
includeSurfaces: includeSurfaces,
commandsContext: commandsContext
)
commandPaletteSearchCommandsByID = Dictionary(uniqueKeysWithValues: entries.map { ($0.id, $0) })
let searchCorpus = entries.map { entry in
CommandPaletteSearchCorpusEntry(
@ -4263,21 +4284,21 @@ struct ContentView: View {
private func commandPaletteEntriesFingerprint(
for scope: CommandPaletteListScope,
includeSurfaces: Bool
includeSurfaces: Bool,
commandsContext: CommandPaletteCommandsContext? = nil
) -> Int {
switch scope {
case .commands:
return commandPaletteCommandsFingerprint()
return commandPaletteCommandsFingerprint(
commandsContext: commandsContext ?? commandPaletteCachedCommandsContext()
)
case .switcher:
return commandPaletteSwitcherEntriesFingerprint(includeSurfaces: includeSurfaces)
}
}
private func commandPaletteCommandsFingerprint() -> Int {
var hasher = Hasher()
hasher.combine(commandPaletteContextSnapshot().fingerprint())
hasher.combine(AppDelegate.shared?.isCmuxCLIInstalledInPATH() ?? false)
return hasher.finalize()
private func commandPaletteCommandsFingerprint(commandsContext: CommandPaletteCommandsContext) -> Int {
commandsContext.snapshot.fingerprint()
}
private func commandPaletteSwitcherEntriesFingerprint(includeSurfaces: Bool) -> Int {
@ -4629,8 +4650,37 @@ struct ContentView: View {
}
}
private func commandPaletteCommands() -> [CommandPaletteCommand] {
let context = commandPaletteContextSnapshot()
private func commandPaletteCachedCommandsContext() -> CommandPaletteCommandsContext {
commandPaletteCommandsContext(
terminalOpenTargets: commandPaletteTerminalOpenTargetAvailability
)
}
private func resolveCommandPaletteTerminalOpenTargets(
for scope: CommandPaletteListScope
) -> Set<TerminalDirectoryOpenTarget> {
guard scope == .commands,
focusedPanelContext?.panel.panelType == .terminal else {
return []
}
return TerminalDirectoryOpenTarget.availableTargets()
}
private func commandPaletteCommandsContext(
terminalOpenTargets: Set<TerminalDirectoryOpenTarget>
) -> CommandPaletteCommandsContext {
let cliInstalledInPATH = AppDelegate.shared?.isCmuxCLIInstalledInPATH() ?? false
var snapshot = commandPaletteContextSnapshot(terminalOpenTargets: terminalOpenTargets)
snapshot.setBool(CommandPaletteContextKeys.cliInstalledInPATH, cliInstalledInPATH)
return CommandPaletteCommandsContext(
snapshot: snapshot
)
}
private func commandPaletteCommands(
commandsContext: CommandPaletteCommandsContext
) -> [CommandPaletteCommand] {
let context = commandsContext.snapshot
let contributions = commandPaletteCommandContributions()
var handlerRegistry = CommandPaletteHandlerRegistry()
registerCommandPaletteHandlers(&handlerRegistry)
@ -4776,7 +4826,9 @@ struct ContentView: View {
}
}
private func commandPaletteContextSnapshot() -> CommandPaletteContextSnapshot {
private func commandPaletteContextSnapshot(
terminalOpenTargets: Set<TerminalDirectoryOpenTarget>? = nil
) -> CommandPaletteContextSnapshot {
var snapshot = CommandPaletteContextSnapshot()
if let workspace = tabManager.selectedWorkspace {
@ -4827,7 +4879,7 @@ struct ContentView: View {
snapshot.setBool(CommandPaletteContextKeys.panelHasUnread, hasUnread)
if panelIsTerminal {
let availableTargets = TerminalDirectoryOpenTarget.availableTargets()
let availableTargets = terminalOpenTargets ?? TerminalDirectoryOpenTarget.availableTargets()
for target in TerminalDirectoryOpenTarget.commandPaletteShortcutTargets {
snapshot.setBool(
CommandPaletteContextKeys.terminalOpenTargetAvailable(target),
@ -4893,7 +4945,7 @@ struct ContentView: View {
title: constant(String(localized: "command.installCLI.title", defaultValue: "Shell Command: Install 'cmux' in PATH")),
subtitle: constant(String(localized: "command.installCLI.subtitle", defaultValue: "CLI")),
keywords: ["install", "cli", "path", "shell", "command", "symlink"],
when: { _ in !(AppDelegate.shared?.isCmuxCLIInstalledInPATH() ?? false) }
when: { !$0.bool(CommandPaletteContextKeys.cliInstalledInPATH) }
)
)
contributions.append(
@ -4902,7 +4954,7 @@ struct ContentView: View {
title: constant(String(localized: "command.uninstallCLI.title", defaultValue: "Shell Command: Uninstall 'cmux' from PATH")),
subtitle: constant(String(localized: "command.uninstallCLI.subtitle", defaultValue: "CLI")),
keywords: ["uninstall", "remove", "cli", "path", "shell", "command", "symlink"],
when: { _ in AppDelegate.shared?.isCmuxCLIInstalledInPATH() ?? false }
when: { $0.bool(CommandPaletteContextKeys.cliInstalledInPATH) }
)
)
contributions.append(
@ -6393,6 +6445,7 @@ struct ContentView: View {
commandPaletteResolvedSearchRequestID = commandPaletteSearchRequestID
commandPaletteResolvedSearchScope = nil
commandPaletteResolvedSearchFingerprint = nil
commandPaletteTerminalOpenTargetAvailability = []
isCommandPaletteSearchPending = false
commandPalettePendingActivation = nil
commandPaletteResultsRevision &+= 1

108
cmuxUITests/SidebarHelpMenuUITests.swift
View file

@ -313,11 +313,24 @@ final class CommandPaletteAllSurfacesUITests: XCTestCase {
super.tearDown()
}
func testCmdShiftPBackspaceReturnsToWorkspaceResults() throws {
let app = XCUIApplication()
private func configureSocketControlledLaunch(
_ app: XCUIApplication,
showSettingsWindow: Bool = false
) {
app.launchArguments += ["-socketControlMode", "allowAll"]
app.launchArguments += ["-AppleLanguages", "(en)", "-AppleLocale", "en_US"]
app.launchEnvironment["CMUX_UI_TEST_MODE"] = "1"
app.launchEnvironment["CMUX_SOCKET_PATH"] = socketPath
app.launchEnvironment["CMUX_SOCKET_ENABLE"] = "1"
app.launchEnvironment["CMUX_SOCKET_MODE"] = "allowAll"
if showSettingsWindow {
app.launchEnvironment["CMUX_UI_TEST_SHOW_SETTINGS"] = "1"
}
}
func testCmdShiftPBackspaceReturnsToWorkspaceResults() throws {
let app = XCUIApplication()
configureSocketControlledLaunch(app)
launchAndActivate(app)
XCTAssertTrue(
@ -399,10 +412,7 @@ final class CommandPaletteAllSurfacesUITests: XCTestCase {
func testCmdPSearchCanIncludeSurfacesFromOtherWorkspacesWhenEnabled() throws {
let app = XCUIApplication()
app.launchArguments += ["-AppleLanguages", "(en)", "-AppleLocale", "en_US"]
app.launchEnvironment["CMUX_UI_TEST_MODE"] = "1"
app.launchEnvironment["CMUX_UI_TEST_SHOW_SETTINGS"] = "1"
app.launchEnvironment["CMUX_SOCKET_PATH"] = socketPath
configureSocketControlledLaunch(app, showSettingsWindow: true)
launchAndActivate(app)
XCTAssertTrue(
@ -478,9 +488,7 @@ final class CommandPaletteAllSurfacesUITests: XCTestCase {
func testSwitcherEmptyStateDoesNotBlinkWhileRefiningNoMatchQuery() throws {
let app = XCUIApplication()
app.launchArguments += ["-AppleLanguages", "(en)", "-AppleLocale", "en_US"]
app.launchEnvironment["CMUX_UI_TEST_MODE"] = "1"
app.launchEnvironment["CMUX_SOCKET_PATH"] = socketPath
configureSocketControlledLaunch(app)
launchAndActivate(app)
XCTAssertTrue(
@ -501,6 +509,25 @@ final class CommandPaletteAllSurfacesUITests: XCTestCase {
XCTAssertTrue(searchField.waitForExistence(timeout: 5.0), "Expected command palette search field")
searchField.click()
let seededWorkspaceTitlePrefix = "\(noMatchWorkspaceQuery)-"
try debugTypeText(noMatchWorkspaceQuery)
let seededSnapshot = try XCTUnwrap(
waitForCommandPaletteSnapshot(windowId: mainWindowId, query: noMatchWorkspaceQuery, timeout: 5.0) { snapshot in
self.commandPaletteResultRows(from: snapshot).contains { row in
((row["title"] as? String) ?? "").hasPrefix(seededWorkspaceTitlePrefix)
}
},
"Expected seeded workspace titles to be indexed before exercising the no-match path"
)
XCTAssertTrue(
commandPaletteResultRows(from: seededSnapshot).contains { row in
((row["title"] as? String) ?? "").hasPrefix(seededWorkspaceTitlePrefix)
},
"Expected the seeded workspace corpus to be searchable before the no-match assertion. snapshot=\(seededSnapshot)"
)
try clearCommandPaletteSearchField(app: app, windowId: mainWindowId)
try debugTypeText(String(repeating: "z", count: 8))
let emptyLabel = app.staticTexts["No workspaces match your search."].firstMatch
@ -516,26 +543,35 @@ final class CommandPaletteAllSurfacesUITests: XCTestCase {
try debugTypeText("z")
let emptyLabelDisappearedWhileRefining = sidebarHelpPollUntil(timeout: 0.5, pollInterval: 0.01) {
!emptyLabel.exists
let refinedQuery = String(repeating: "z", count: 9)
var refinedSnapshot: [String: Any]?
var emptyLabelDisappearedWhileRefining = false
let refinedQueryResolvedWhileKeepingEmptyStateVisible = sidebarHelpPollUntil(
timeout: 5.0,
pollInterval: 0.01
) {
guard emptyLabel.exists else {
emptyLabelDisappearedWhileRefining = true
return false
}
guard let snapshot = commandPaletteSnapshot(windowId: mainWindowId) else { return false }
guard (snapshot["query"] as? String) == refinedQuery else { return false }
guard self.commandPaletteResultRows(from: snapshot).isEmpty else { return false }
refinedSnapshot = snapshot
return true
}
XCTAssertFalse(
emptyLabelDisappearedWhileRefining,
"Expected refining an already-empty switcher query to keep the empty-state label visible"
)
let refinedSnapshot = try XCTUnwrap(
waitForCommandPaletteSnapshot(
windowId: mainWindowId,
query: String(repeating: "z", count: 9),
timeout: 5.0
) { snapshot in
self.commandPaletteResultRows(from: snapshot).isEmpty
}
)
XCTAssertTrue(
commandPaletteResultRows(from: refinedSnapshot).isEmpty,
"Expected the refined no-match query to stay empty. snapshot=\(refinedSnapshot)"
refinedQueryResolvedWhileKeepingEmptyStateVisible,
"Expected the refined no-match query to resolve while keeping the empty-state label visible"
)
let resolvedRefinedSnapshot = try XCTUnwrap(refinedSnapshot)
XCTAssertTrue(
commandPaletteResultRows(from: resolvedRefinedSnapshot).isEmpty,
"Expected the refined no-match query to stay empty. snapshot=\(resolvedRefinedSnapshot)"
)
}
@ -570,7 +606,7 @@ final class CommandPaletteAllSurfacesUITests: XCTestCase {
let searchField = app.textFields["CommandPaletteSearchField"]
for _ in 0..<2 {
app.typeKey(XCUIKeyboardKey.escape.rawValue, modifierFlags: [])
if sidebarHelpPollUntil(timeout: 1.0) { !searchField.exists } {
if sidebarHelpPollUntil(timeout: 1.0, condition: { !searchField.exists }) {
return
}
}
@ -667,6 +703,22 @@ final class CommandPaletteAllSurfacesUITests: XCTestCase {
XCTAssertEqual(response["ok"] as? Bool, true, "Expected debug.type to succeed. response=\(response)")
}
private func clearCommandPaletteSearchField(app: XCUIApplication, windowId: String) throws {
let searchField = app.textFields["CommandPaletteSearchField"]
searchField.click()
app.typeKey("a", modifierFlags: [.command])
app.typeKey(XCUIKeyboardKey.delete.rawValue, modifierFlags: [])
let clearedSnapshot = try XCTUnwrap(
waitForCommandPaletteSnapshot(windowId: windowId, query: "", timeout: 5.0),
"Expected the command palette query to clear"
)
XCTAssertEqual(
clearedSnapshot["query"] as? String,
"",
"Expected the command palette query to clear"
)
}
private func seedWorkspaceSwitcherCorpus(workspaceCount: Int) throws {
guard workspaceCount > 1 else { return }
@ -675,7 +727,7 @@ final class CommandPaletteAllSurfacesUITests: XCTestCase {
okUUID(from: socketCommand("new_workspace")),
"Expected new_workspace to return a workspace ID"
)
let title = "\(noMatchWorkspaceQuery)-\(index)-" + String(repeating: "workspace-", count: 8)
let title = seededWorkspaceTitle(index: index)
let response = try XCTUnwrap(
socketJSON(
method: "workspace.rename",
@ -696,6 +748,10 @@ final class CommandPaletteAllSurfacesUITests: XCTestCase {
XCTAssertEqual(socketCommand("select_workspace 0"), "OK")
}
private func seededWorkspaceTitle(index: Int) -> String {
"\(noMatchWorkspaceQuery)-\(index)-" + String(repeating: "workspace-", count: 8)
}
private func socketCommand(_ command: String) -> String? {
ControlSocketClient(path: socketPath, responseTimeout: 2.0).sendLine(command)
}
@ -720,7 +776,7 @@ final class CommandPaletteAllSurfacesUITests: XCTestCase {
guard (snapshot["query"] as? String) == query else { return false }
return predicate?(snapshot) ?? true
}
return matched ? latest : latest
return matched ? latest : nil
}
private func commandPaletteSnapshot(windowId: String) -> [String: Any]? {