Merge pull request #2394 from manaflow-ai/task-ctrl-k-command-palette

Fix Ctrl+K in the command palette
This commit is contained in:
Lawrence Chen 2026-03-30 21:00:21 -07:00 committed by GitHub
commit 56deaf66e7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 102 additions and 24 deletions

View file

@ -1620,10 +1620,10 @@ func commandPaletteSelectionDeltaForKeyboardNavigation(
if normalizedFlags == [.control] { if normalizedFlags == [.control] {
// Control modifiers can surface as either printable chars or ASCII control chars. // Control modifiers can surface as either printable chars or ASCII control chars.
// Keep Emacs-style next/previous navigation, but leave other control bindings
// (for example Ctrl+K text editing in the palette search field) to AppKit.
if keyCode == 45 || normalizedChars == "n" || normalizedChars == "\u{0e}" { return 1 } // Ctrl+N if keyCode == 45 || normalizedChars == "n" || normalizedChars == "\u{0e}" { return 1 } // Ctrl+N
if keyCode == 35 || normalizedChars == "p" || normalizedChars == "\u{10}" { return -1 } // Ctrl+P if keyCode == 35 || normalizedChars == "p" || normalizedChars == "\u{10}" { return -1 } // Ctrl+P
if keyCode == 38 || normalizedChars == "j" || normalizedChars == "\u{0a}" { return 1 } // Ctrl+J
if keyCode == 40 || normalizedChars == "k" || normalizedChars == "\u{0b}" { return -1 } // Ctrl+K
} }
return nil return nil

View file

@ -4538,7 +4538,6 @@ struct ContentView: View {
} }
return currentMatchingQuery == resolvedMatchingQuery return currentMatchingQuery == resolvedMatchingQuery
|| currentMatchingQuery.hasPrefix(resolvedMatchingQuery)
} }
private func scheduleCommandPaletteResultsRefresh( private func scheduleCommandPaletteResultsRefresh(

View file

@ -2351,8 +2351,8 @@ final class AppDelegateShortcutRoutingTests: XCTestCase {
XCTFail("debugMarkCommandPaletteOpenPending is only available in DEBUG") XCTFail("debugMarkCommandPaletteOpenPending is only available in DEBUG")
#endif #endif
// Simulate a visibility sync lag/race where AppDelegate does not yet know the palette is open. // Model the normal open-palette state so the test reads like the user-facing scenario.
appDelegate.setCommandPaletteVisible(false, for: window) appDelegate.setCommandPaletteVisible(true, for: window)
guard let escapeEvent = makeKeyDownEvent( guard let escapeEvent = makeKeyDownEvent(
key: "\u{1b}", key: "\u{1b}",
@ -2445,6 +2445,72 @@ final class AppDelegateShortcutRoutingTests: XCTestCase {
XCTAssertEqual(observedDelta, 1) XCTAssertEqual(observedDelta, 1)
} }
func testControlKDoesNotRoutePaletteMoveSelectionWhenSearchFieldIsFocused() {
guard let appDelegate = AppDelegate.shared else {
XCTFail("Expected AppDelegate.shared")
return
}
let windowId = appDelegate.createMainWindow()
defer {
closeWindow(withId: windowId)
}
guard let window = window(withId: windowId),
let contentView = window.contentView else {
XCTFail("Expected test window")
return
}
let overlayContainer = NSView(frame: contentView.bounds)
overlayContainer.identifier = commandPaletteOverlayContainerIdentifier
overlayContainer.alphaValue = 1
overlayContainer.isHidden = false
contentView.addSubview(overlayContainer)
let fieldEditor = CommandPaletteMarkedTextFieldEditor(frame: NSRect(x: 0, y: 0, width: 200, height: 24))
fieldEditor.isFieldEditor = true
overlayContainer.addSubview(fieldEditor)
XCTAssertTrue(window.makeFirstResponder(fieldEditor))
appDelegate.setCommandPaletteVisible(false, for: window)
defer {
overlayContainer.removeFromSuperview()
fieldEditor.removeFromSuperview()
}
let moveExpectation = expectation(
description: "Ctrl+K should not be rerouted as command palette move-selection"
)
moveExpectation.isInverted = true
let moveToken = NotificationCenter.default.addObserver(
forName: .commandPaletteMoveSelection,
object: nil,
queue: nil
) { _ in
moveExpectation.fulfill()
}
defer { NotificationCenter.default.removeObserver(moveToken) }
guard let controlKEvent = makeKeyDownEvent(
key: "\u{0b}",
modifiers: [.control],
keyCode: 40,
windowNumber: window.windowNumber
) else {
XCTFail("Failed to construct Ctrl+K event")
return
}
#if DEBUG
XCTAssertFalse(appDelegate.debugHandleCustomShortcut(event: controlKEvent))
#else
XCTFail("debugHandleCustomShortcut is only available in DEBUG")
#endif
wait(for: [moveExpectation], timeout: 0.2)
}
func testEscapeDismissesCommandPaletteWhenVisibilityStateStaysStalePastInitialPendingWindow() { func testEscapeDismissesCommandPaletteWhenVisibilityStateStaysStalePastInitialPendingWindow() {
guard let appDelegate = AppDelegate.shared else { guard let appDelegate = AppDelegate.shared else {
XCTFail("Expected AppDelegate.shared") XCTFail("Expected AppDelegate.shared")

View file

@ -485,8 +485,8 @@ final class CommandPaletteSearchEngineTests: XCTestCase {
) )
} }
func testPendingEmptyStateIsPreservedWhenRefiningAResolvedNoMatchQuery() { func testPendingEmptyStateIsNotPreservedWhileSearchIsStillPending() {
XCTAssertTrue( XCTAssertFalse(
ContentView.commandPaletteShouldPreserveEmptyStateWhileSearchPending( ContentView.commandPaletteShouldPreserveEmptyStateWhileSearchPending(
isSearchPending: true, isSearchPending: true,
visibleResultsScopeMatches: true, visibleResultsScopeMatches: true,
@ -499,6 +499,20 @@ final class CommandPaletteSearchEngineTests: XCTestCase {
) )
} }
func testPendingEmptyStateIsPreservedForSameResolvedNoMatchQuery() {
XCTAssertTrue(
ContentView.commandPaletteShouldPreserveEmptyStateWhileSearchPending(
isSearchPending: true,
visibleResultsScopeMatches: true,
resolvedSearchScopeMatches: true,
resolvedSearchFingerprintMatches: true,
resolvedResultsAreEmpty: true,
currentMatchingQuery: "zzzzzzzz",
resolvedMatchingQuery: "zzzzzzzz"
)
)
}
func testPendingEmptyStateIsNotPreservedWhenQueryDoesNotRefineResolvedNoMatch() { func testPendingEmptyStateIsNotPreservedWhenQueryDoesNotRefineResolvedNoMatch() {
XCTAssertFalse( XCTAssertFalse(
ContentView.commandPaletteShouldPreserveEmptyStateWhileSearchPending( ContentView.commandPaletteShouldPreserveEmptyStateWhileSearchPending(

View file

@ -213,7 +213,7 @@ final class CommandPaletteKeyboardNavigationTests: XCTestCase {
) )
} }
func testControlLetterNavigationSupportsPrintableAndControlChars() { func testControlLetterNavigationSupportsPrintableAndControlCharsForNPOnly() {
XCTAssertEqual( XCTAssertEqual(
commandPaletteSelectionDeltaForKeyboardNavigation( commandPaletteSelectionDeltaForKeyboardNavigation(
flags: [.control], flags: [.control],
@ -246,37 +246,36 @@ final class CommandPaletteKeyboardNavigationTests: XCTestCase {
), ),
-1 -1
) )
XCTAssertEqual( }
func testDoesNotTreatControlJKAsPaletteNavigation() {
XCTAssertNil(
commandPaletteSelectionDeltaForKeyboardNavigation( commandPaletteSelectionDeltaForKeyboardNavigation(
flags: [.control], flags: [.control],
chars: "j", chars: "j",
keyCode: 38 keyCode: 38
),
1
) )
XCTAssertEqual( )
XCTAssertNil(
commandPaletteSelectionDeltaForKeyboardNavigation( commandPaletteSelectionDeltaForKeyboardNavigation(
flags: [.control], flags: [.control],
chars: "\u{0a}", chars: "\u{0a}",
keyCode: 38 keyCode: 38
),
1
) )
XCTAssertEqual( )
XCTAssertNil(
commandPaletteSelectionDeltaForKeyboardNavigation( commandPaletteSelectionDeltaForKeyboardNavigation(
flags: [.control], flags: [.control],
chars: "k", chars: "k",
keyCode: 40 keyCode: 40
),
-1
) )
XCTAssertEqual( )
XCTAssertNil(
commandPaletteSelectionDeltaForKeyboardNavigation( commandPaletteSelectionDeltaForKeyboardNavigation(
flags: [.control], flags: [.control],
chars: "\u{0b}", chars: "\u{0b}",
keyCode: 40 keyCode: 40
), )
-1
) )
} }

View file

@ -3,8 +3,8 @@
Regression test: command palette list navigation keys. Regression test: command palette list navigation keys.
Validates: Validates:
- Down: ArrowDown, Ctrl+N, Ctrl+J - Down: ArrowDown, Ctrl+N
- Up: ArrowUp, Ctrl+P, Ctrl+K - Up: ArrowUp, Ctrl+P
""" """
import os import os
@ -125,10 +125,10 @@ def main() -> int:
message="no focused surface available for command palette context", message="no focused surface available for command palette context",
) )
for combo in ("down", "ctrl+n", "ctrl+j"): for combo in ("down", "ctrl+n"):
_assert_move(client, window_id, combo, start_index=0, expected_index=1) _assert_move(client, window_id, combo, start_index=0, expected_index=1)
for combo in ("up", "ctrl+p", "ctrl+k"): for combo in ("up", "ctrl+p"):
_assert_move(client, window_id, combo, start_index=1, expected_index=0) _assert_move(client, window_id, combo, start_index=1, expected_index=0)
_assert_can_navigate_past_ten_results(client, window_id) _assert_can_navigate_past_ten_results(client, window_id)