Fix pressing any key to exit select mode. (#18421)

This commit is contained in:
Jacob Richman
2026-02-11 09:38:01 -08:00
committed by GitHub
parent 65d26e73a2
commit eb9223b6a4
4 changed files with 193 additions and 161 deletions

View File

@@ -84,7 +84,7 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
};
});
import ansiEscapes from 'ansi-escapes';
import { type LoadedSettings, mergeSettings } from '../config/settings.js';
import { mergeSettings, type LoadedSettings } from '../config/settings.js';
import type { InitializationResult } from '../core/initializer.js';
import { useQuotaAndFallback } from './hooks/useQuotaAndFallback.js';
import { UIStateContext, type UIState } from './contexts/UIStateContext.js';
@@ -92,6 +92,7 @@ import {
UIActionsContext,
type UIActions,
} from './contexts/UIActionsContext.js';
import { KeypressProvider } from './contexts/KeypressContext.js';
// Mock useStdout to capture terminal title writes
vi.mock('ink', async (importOriginal) => {
@@ -133,7 +134,6 @@ vi.mock('./hooks/useGeminiStream.js');
vi.mock('./hooks/vim.js');
vi.mock('./hooks/useFocus.js');
vi.mock('./hooks/useBracketedPaste.js');
vi.mock('./hooks/useKeypress.js');
vi.mock('./hooks/useLoadingIndicator.js');
vi.mock('./hooks/useFolderTrust.js');
vi.mock('./hooks/useIdeTrustListener.js');
@@ -197,7 +197,7 @@ import { useTextBuffer } from './components/shared/text-buffer.js';
import { useLogger } from './hooks/useLogger.js';
import { useLoadingIndicator } from './hooks/useLoadingIndicator.js';
import { useInputHistoryStore } from './hooks/useInputHistoryStore.js';
import { useKeypress, type Key } from './hooks/useKeypress.js';
import { useKeypress } from './hooks/useKeypress.js';
import { measureElement } from 'ink';
import { useTerminalSize } from './hooks/useTerminalSize.js';
import {
@@ -232,13 +232,15 @@ describe('AppContainer State Management', () => {
resumedSessionData?: ResumedSessionData;
} = {}) => (
<SettingsContext.Provider value={settings}>
<AppContainer
config={config}
version={version}
initializationResult={initResult}
startupWarnings={startupWarnings}
resumedSessionData={resumedSessionData}
/>
<KeypressProvider config={config}>
<AppContainer
config={config}
version={version}
initializationResult={initResult}
startupWarnings={startupWarnings}
resumedSessionData={resumedSessionData}
/>
</KeypressProvider>
</SettingsContext.Provider>
);
@@ -268,7 +270,6 @@ describe('AppContainer State Management', () => {
const mockedUseTextBuffer = useTextBuffer as Mock;
const mockedUseLogger = useLogger as Mock;
const mockedUseLoadingIndicator = useLoadingIndicator as Mock;
const mockedUseKeypress = useKeypress as Mock;
const mockedUseInputHistoryStore = useInputHistoryStore as Mock;
const mockedUseHookDisplayState = useHookDisplayState as Mock;
const mockedUseTerminalTheme = useTerminalTheme as Mock;
@@ -1770,47 +1771,36 @@ describe('AppContainer State Management', () => {
});
describe('Keyboard Input Handling (CTRL+C / CTRL+D)', () => {
let handleGlobalKeypress: (key: Key) => boolean;
let mockHandleSlashCommand: Mock;
let mockCancelOngoingRequest: Mock;
let rerender: () => void;
let unmount: () => void;
let stdin: ReturnType<typeof render>['stdin'];
// Helper function to reduce boilerplate in tests
const setupKeypressTest = async () => {
const renderResult = renderAppContainer();
stdin = renderResult.stdin;
await act(async () => {
vi.advanceTimersByTime(0);
});
rerender = () => renderResult.rerender(getAppContainer());
rerender = () => {
renderResult.rerender(getAppContainer());
};
unmount = renderResult.unmount;
};
const pressKey = (key: Partial<Key>, times = 1) => {
const pressKey = (sequence: string, times = 1) => {
for (let i = 0; i < times; i++) {
act(() => {
handleGlobalKeypress({
name: 'c',
shift: false,
alt: false,
ctrl: false,
cmd: false,
...key,
} as Key);
stdin.write(sequence);
});
rerender();
}
};
beforeEach(() => {
// Capture the keypress handler from the AppContainer
mockedUseKeypress.mockImplementation(
(callback: (key: Key) => boolean) => {
handleGlobalKeypress = callback;
},
);
// Mock slash command handler
mockHandleSlashCommand = vi.fn();
mockedUseSlashCommandProcessor.mockReturnValue({
@@ -1855,7 +1845,7 @@ describe('AppContainer State Management', () => {
});
await setupKeypressTest();
pressKey({ name: 'c', ctrl: true });
pressKey('\x03'); // Ctrl+C
expect(mockCancelOngoingRequest).toHaveBeenCalledTimes(1);
expect(mockHandleSlashCommand).not.toHaveBeenCalled();
@@ -1865,7 +1855,7 @@ describe('AppContainer State Management', () => {
it('should quit on second press', async () => {
await setupKeypressTest();
pressKey({ name: 'c', ctrl: true }, 2);
pressKey('\x03', 2); // Ctrl+C
expect(mockCancelOngoingRequest).toHaveBeenCalledTimes(2);
expect(mockHandleSlashCommand).toHaveBeenCalledWith(
@@ -1880,7 +1870,7 @@ describe('AppContainer State Management', () => {
it('should reset press count after a timeout', async () => {
await setupKeypressTest();
pressKey({ name: 'c', ctrl: true });
pressKey('\x03'); // Ctrl+C
expect(mockHandleSlashCommand).not.toHaveBeenCalled();
// Advance timer past the reset threshold
@@ -1888,7 +1878,7 @@ describe('AppContainer State Management', () => {
vi.advanceTimersByTime(WARNING_PROMPT_DURATION_MS + 1);
});
pressKey({ name: 'c', ctrl: true });
pressKey('\x03'); // Ctrl+C
expect(mockHandleSlashCommand).not.toHaveBeenCalled();
unmount();
});
@@ -1898,7 +1888,7 @@ describe('AppContainer State Management', () => {
it('should quit on second press if buffer is empty', async () => {
await setupKeypressTest();
pressKey({ name: 'd', ctrl: true }, 2);
pressKey('\x04', 2); // Ctrl+D
expect(mockHandleSlashCommand).toHaveBeenCalledWith(
'/quit',
@@ -1909,7 +1899,7 @@ describe('AppContainer State Management', () => {
unmount();
});
it('should NOT quit if buffer is not empty (bubbles from InputPrompt)', async () => {
it('should NOT quit if buffer is not empty', async () => {
mockedUseTextBuffer.mockReturnValue({
text: 'some text',
setText: vi.fn(),
@@ -1919,30 +1909,12 @@ describe('AppContainer State Management', () => {
});
await setupKeypressTest();
// Capture return value
let result = true;
const originalPressKey = (key: Partial<Key>) => {
act(() => {
result = handleGlobalKeypress({
name: 'd',
shift: false,
alt: false,
ctrl: true,
cmd: false,
...key,
} as Key);
});
rerender();
};
pressKey('\x04'); // Ctrl+D
originalPressKey({ name: 'd', ctrl: true });
// AppContainer's handler should return true if it reaches it
expect(result).toBe(true);
// But it should only be called once, so count is 1, not quitting yet.
// Should only be called once, so count is 1, not quitting yet.
expect(mockHandleSlashCommand).not.toHaveBeenCalled();
originalPressKey({ name: 'd', ctrl: true });
pressKey('\x04'); // Ctrl+D
// Now count is 2, it should quit.
expect(mockHandleSlashCommand).toHaveBeenCalledWith(
'/quit',
@@ -1956,7 +1928,7 @@ describe('AppContainer State Management', () => {
it('should reset press count after a timeout', async () => {
await setupKeypressTest();
pressKey({ name: 'd', ctrl: true });
pressKey('\x04'); // Ctrl+D
expect(mockHandleSlashCommand).not.toHaveBeenCalled();
// Advance timer past the reset threshold
@@ -1964,7 +1936,7 @@ describe('AppContainer State Management', () => {
vi.advanceTimersByTime(WARNING_PROMPT_DURATION_MS + 1);
});
pressKey({ name: 'd', ctrl: true });
pressKey('\x04'); // Ctrl+D
expect(mockHandleSlashCommand).not.toHaveBeenCalled();
unmount();
});
@@ -1982,7 +1954,7 @@ describe('AppContainer State Management', () => {
it('should focus shell input on Tab', async () => {
await setupKeypressTest();
pressKey({ name: 'tab', shift: false });
pressKey('\t');
expect(capturedUIState.embeddedShellFocused).toBe(true);
unmount();
@@ -1992,11 +1964,11 @@ describe('AppContainer State Management', () => {
await setupKeypressTest();
// Focus first
pressKey({ name: 'tab', shift: false });
pressKey('\t');
expect(capturedUIState.embeddedShellFocused).toBe(true);
// Unfocus via Shift+Tab
pressKey({ name: 'tab', shift: true });
pressKey('\x1b[Z');
expect(capturedUIState.embeddedShellFocused).toBe(false);
unmount();
});
@@ -2015,13 +1987,7 @@ describe('AppContainer State Management', () => {
// Focus it
act(() => {
handleGlobalKeypress({
name: 'tab',
shift: false,
alt: false,
ctrl: false,
cmd: false,
} as Key);
renderResult.stdin.write('\t');
});
expect(capturedUIState.embeddedShellFocused).toBe(true);
@@ -2056,7 +2022,7 @@ describe('AppContainer State Management', () => {
expect(capturedUIState.embeddedShellFocused).toBe(false);
// Press Tab
pressKey({ name: 'tab', shift: false });
pressKey('\t');
// Should be focused
expect(capturedUIState.embeddedShellFocused).toBe(true);
@@ -2084,7 +2050,7 @@ describe('AppContainer State Management', () => {
expect(capturedUIState.embeddedShellFocused).toBe(false);
// Press Ctrl+B
pressKey({ name: 'b', ctrl: true });
pressKey('\x02');
// Should have toggled (closed) the shell
expect(mockToggleBackgroundShell).toHaveBeenCalled();
@@ -2113,7 +2079,7 @@ describe('AppContainer State Management', () => {
});
// Press Ctrl+B
pressKey({ name: 'b', ctrl: true });
pressKey('\x02');
// Should have toggled (shown) the shell
expect(mockToggleBackgroundShell).toHaveBeenCalled();
@@ -2126,11 +2092,14 @@ describe('AppContainer State Management', () => {
});
describe('Copy Mode (CTRL+S)', () => {
let handleGlobalKeypress: (key: Key) => boolean;
let rerender: () => void;
let unmount: () => void;
let stdin: ReturnType<typeof render>['stdin'];
const setupCopyModeTest = async (isAlternateMode = false) => {
const setupCopyModeTest = async (
isAlternateMode = false,
childHandler?: Mock,
) => {
// Update settings for this test run
const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true);
const testSettings = {
@@ -2144,23 +2113,39 @@ describe('AppContainer State Management', () => {
},
} as unknown as LoadedSettings;
const renderResult = renderAppContainer({ settings: testSettings });
function TestChild() {
useKeypress(childHandler || (() => {}), {
isActive: !!childHandler,
priority: true,
});
return null;
}
const getTree = (settings: LoadedSettings) => (
<SettingsContext.Provider value={settings}>
<KeypressProvider config={mockConfig}>
<AppContainer
config={mockConfig}
version="1.0.0"
initializationResult={mockInitResult}
/>
<TestChild />
</KeypressProvider>
</SettingsContext.Provider>
);
const renderResult = render(getTree(testSettings));
stdin = renderResult.stdin;
await act(async () => {
vi.advanceTimersByTime(0);
});
rerender = () =>
renderResult.rerender(getAppContainer({ settings: testSettings }));
rerender = () => renderResult.rerender(getTree(testSettings));
unmount = renderResult.unmount;
};
beforeEach(() => {
mocks.mockStdout.write.mockClear();
mockedUseKeypress.mockImplementation(
(callback: (key: Key) => boolean) => {
handleGlobalKeypress = callback;
},
);
vi.useFakeTimers();
});
@@ -2186,15 +2171,7 @@ describe('AppContainer State Management', () => {
mocks.mockStdout.write.mockClear(); // Clear initial enable call
act(() => {
handleGlobalKeypress({
name: 's',
shift: false,
alt: false,
ctrl: true,
cmd: false,
insertable: false,
sequence: '\x13',
});
stdin.write('\x13'); // Ctrl+S
});
rerender();
@@ -2213,30 +2190,14 @@ describe('AppContainer State Management', () => {
// Turn it on (disable mouse)
act(() => {
handleGlobalKeypress({
name: 's',
shift: false,
alt: false,
ctrl: true,
cmd: false,
insertable: false,
sequence: '\x13',
});
stdin.write('\x13'); // Ctrl+S
});
rerender();
expect(disableMouseEvents).toHaveBeenCalled();
// Turn it off (enable mouse)
act(() => {
handleGlobalKeypress({
name: 'any', // Any key should exit copy mode
shift: false,
alt: false,
ctrl: false,
cmd: false,
insertable: true,
sequence: 'a',
});
stdin.write('a'); // Any key should exit copy mode
});
rerender();
@@ -2249,15 +2210,7 @@ describe('AppContainer State Management', () => {
// Enter copy mode
act(() => {
handleGlobalKeypress({
name: 's',
shift: false,
alt: false,
ctrl: true,
cmd: false,
insertable: false,
sequence: '\x13',
});
stdin.write('\x13'); // Ctrl+S
});
rerender();
@@ -2265,15 +2218,7 @@ describe('AppContainer State Management', () => {
// Press any other key
act(() => {
handleGlobalKeypress({
name: 'a',
shift: false,
alt: false,
ctrl: false,
cmd: false,
insertable: true,
sequence: 'a',
});
stdin.write('a');
});
rerender();
@@ -2281,6 +2226,37 @@ describe('AppContainer State Management', () => {
expect(enableMouseEvents).toHaveBeenCalled();
unmount();
});
it('should have higher priority than other priority listeners when enabled', async () => {
// 1. Initial state with a child component's priority listener (already subscribed)
// It should NOT handle Ctrl+S so we can enter copy mode.
const childHandler = vi.fn().mockReturnValue(false);
await setupCopyModeTest(true, childHandler);
// 2. Enter copy mode
act(() => {
stdin.write('\x13'); // Ctrl+S
});
rerender();
// 3. Verify we are in copy mode
expect(disableMouseEvents).toHaveBeenCalled();
// 4. Press any key
childHandler.mockClear();
// Now childHandler should return true for other keys, simulating a greedy listener
childHandler.mockReturnValue(true);
act(() => {
stdin.write('a');
});
rerender();
// 5. Verify that the exit handler took priority and childHandler was NOT called
expect(childHandler).not.toHaveBeenCalled();
expect(enableMouseEvents).toHaveBeenCalled();
unmount();
});
}
});
});

View File

@@ -101,6 +101,7 @@ import { type LoadableSettingScope, SettingScope } from '../config/settings.js';
import { type InitializationResult } from '../core/initializer.js';
import { useFocus } from './hooks/useFocus.js';
import { useKeypress, type Key } from './hooks/useKeypress.js';
import { KeypressPriority } from './contexts/KeypressContext.js';
import { keyMatchers, Command } from './keyMatchers.js';
import { useLoadingIndicator } from './hooks/useLoadingIndicator.js';
import { useShellInactivityStatus } from './hooks/useShellInactivityStatus.js';
@@ -1483,13 +1484,6 @@ Logging in with Google... Restarting Gemini CLI to continue.
const handleGlobalKeypress = useCallback(
(key: Key): boolean => {
if (copyModeEnabled) {
setCopyModeEnabled(false);
enableMouseEvents();
// We don't want to process any other keys if we're in copy mode.
return true;
}
// Debug log keystrokes if enabled
if (settings.merged.general.debugKeystrokeLogging) {
debugLogger.log('[DEBUG] Keystroke:', JSON.stringify(key));
@@ -1656,7 +1650,6 @@ Logging in with Google... Restarting Gemini CLI to continue.
settings.merged.general.debugKeystrokeLogging,
refreshStatic,
setCopyModeEnabled,
copyModeEnabled,
isAlternateBuffer,
backgroundCurrentShell,
toggleBackgroundShell,
@@ -1672,6 +1665,20 @@ Logging in with Google... Restarting Gemini CLI to continue.
useKeypress(handleGlobalKeypress, { isActive: true, priority: true });
useKeypress(
() => {
setCopyModeEnabled(false);
enableMouseEvents();
return true;
},
{
isActive: copyModeEnabled,
// We need to receive keypresses first so they do not bubble to other
// handlers.
priority: KeypressPriority.Critical,
},
);
useEffect(() => {
// Respect hideWindowTitle settings
if (settings.merged.ui.hideWindowTitle) return;

View File

@@ -6,6 +6,7 @@
import { debugLogger, type Config } from '@google/gemini-cli-core';
import { useStdin } from 'ink';
import { MultiMap } from 'mnemonist';
import type React from 'react';
import {
createContext,
@@ -26,6 +27,13 @@ export const ESC_TIMEOUT = 50;
export const PASTE_TIMEOUT = 30_000;
export const FAST_RETURN_TIMEOUT = 30;
export enum KeypressPriority {
Low = -100,
Normal = 0,
High = 100,
Critical = 200,
}
// Parse the key itself
const KEY_INFO_MAP: Record<
string,
@@ -645,7 +653,10 @@ export interface Key {
export type KeypressHandler = (key: Key) => boolean | void;
interface KeypressContextValue {
subscribe: (handler: KeypressHandler, priority?: boolean) => void;
subscribe: (
handler: KeypressHandler,
priority?: KeypressPriority | boolean,
) => void;
unsubscribe: (handler: KeypressHandler) => void;
}
@@ -674,44 +685,75 @@ export function KeypressProvider({
}) {
const { stdin, setRawMode } = useStdin();
const prioritySubscribers = useRef<Set<KeypressHandler>>(new Set()).current;
const normalSubscribers = useRef<Set<KeypressHandler>>(new Set()).current;
const subscribersToPriority = useRef<Map<KeypressHandler, number>>(
new Map(),
).current;
const subscribers = useRef(
new MultiMap<number, KeypressHandler>(Set),
).current;
const sortedPriorities = useRef<number[]>([]);
const subscribe = useCallback(
(handler: KeypressHandler, priority = false) => {
const set = priority ? prioritySubscribers : normalSubscribers;
set.add(handler);
(
handler: KeypressHandler,
priority: KeypressPriority | boolean = KeypressPriority.Normal,
) => {
const p =
typeof priority === 'boolean'
? priority
? KeypressPriority.High
: KeypressPriority.Normal
: priority;
subscribersToPriority.set(handler, p);
const hadPriority = subscribers.has(p);
subscribers.set(p, handler);
if (!hadPriority) {
// Cache sorted priorities only when a new priority level is added
sortedPriorities.current = Array.from(subscribers.keys()).sort(
(a, b) => b - a,
);
}
},
[prioritySubscribers, normalSubscribers],
[subscribers, subscribersToPriority],
);
const unsubscribe = useCallback(
(handler: KeypressHandler) => {
prioritySubscribers.delete(handler);
normalSubscribers.delete(handler);
const p = subscribersToPriority.get(handler);
if (p !== undefined) {
subscribers.remove(p, handler);
subscribersToPriority.delete(handler);
if (!subscribers.has(p)) {
// Cache sorted priorities only when a priority level is completely removed
sortedPriorities.current = Array.from(subscribers.keys()).sort(
(a, b) => b - a,
);
}
}
},
[prioritySubscribers, normalSubscribers],
[subscribers, subscribersToPriority],
);
const broadcast = useCallback(
(key: Key) => {
// Process priority subscribers first, in reverse order (stack behavior: last subscribed is first to handle)
const priorityHandlers = Array.from(prioritySubscribers).reverse();
for (const handler of priorityHandlers) {
if (handler(key) === true) {
return;
}
}
// Use cached sorted priorities to avoid sorting on every keypress
for (const p of sortedPriorities.current) {
const set = subscribers.get(p);
if (!set) continue;
// Then process normal subscribers, also in reverse order
const normalHandlers = Array.from(normalSubscribers).reverse();
for (const handler of normalHandlers) {
if (handler(key) === true) {
return;
// Within a priority level, use stack behavior (last subscribed is first to handle)
const handlers = Array.from(set).reverse();
for (const handler of handlers) {
if (handler(key) === true) {
return;
}
}
}
},
[prioritySubscribers, normalSubscribers],
[subscribers],
);
useEffect(() => {

View File

@@ -5,8 +5,12 @@
*/
import { useEffect } from 'react';
import type { KeypressHandler, Key } from '../contexts/KeypressContext.js';
import { useKeypressContext } from '../contexts/KeypressContext.js';
import {
useKeypressContext,
type KeypressHandler,
type Key,
type KeypressPriority,
} from '../contexts/KeypressContext.js';
export type { Key };
@@ -16,11 +20,14 @@ export type { Key };
* @param onKeypress - The callback function to execute on each keypress.
* @param options - Options to control the hook's behavior.
* @param options.isActive - Whether the hook should be actively listening for input.
* @param options.priority - Whether the hook should have priority over normal subscribers.
* @param options.priority - Priority level (integer or KeypressPriority enum) or boolean for backward compatibility.
*/
export function useKeypress(
onKeypress: KeypressHandler,
{ isActive, priority }: { isActive: boolean; priority?: boolean },
{
isActive,
priority,
}: { isActive: boolean; priority?: KeypressPriority | boolean },
) {
const { subscribe, unsubscribe } = useKeypressContext();