bug(ui) make it clear when users need to enter selection mode and fix clear issue. (#13083)

This commit is contained in:
Jacob Richman
2025-11-14 12:02:15 -08:00
committed by GitHub
parent d683e1c0db
commit ba15eeb55f
14 changed files with 320 additions and 57 deletions

View File

@@ -109,7 +109,7 @@ import { disableMouseEvents, enableMouseEvents } from './utils/mouse.js';
import { useAlternateBuffer } from './hooks/useAlternateBuffer.js';
import { useSettings } from './contexts/SettingsContext.js';
const CTRL_EXIT_PROMPT_DURATION_MS = 1000;
const WARNING_PROMPT_DURATION_MS = 1000;
const QUEUE_ERROR_DISPLAY_DURATION_MS = 3000;
function isToolExecuting(pendingHistoryItems: HistoryItemWithoutId[]) {
@@ -892,6 +892,7 @@ Logging in with Google... Please restart Gemini CLI to continue.
>();
const [showEscapePrompt, setShowEscapePrompt] = useState(false);
const [showIdeRestartPrompt, setShowIdeRestartPrompt] = useState(false);
const [selectionWarning, setSelectionWarning] = useState(false);
const { isFolderTrustDialogOpen, handleFolderTrustSelect, isRestarting } =
useFolderTrust(settings, setIsTrustedFolder, historyManager.addItem);
@@ -901,6 +902,26 @@ Logging in with Google... Please restart Gemini CLI to continue.
} = useIdeTrustListener();
const isInitialMount = useRef(true);
useEffect(() => {
let timeoutId: NodeJS.Timeout;
const handleSelectionWarning = () => {
setSelectionWarning(true);
if (timeoutId) {
clearTimeout(timeoutId);
}
timeoutId = setTimeout(() => {
setSelectionWarning(false);
}, WARNING_PROMPT_DURATION_MS);
};
appEvents.on(AppEvent.SelectionWarning, handleSelectionWarning);
return () => {
appEvents.off(AppEvent.SelectionWarning, handleSelectionWarning);
if (timeoutId) {
clearTimeout(timeoutId);
}
};
}, []);
useEffect(() => {
if (ideNeedsRestart) {
// IDE trust changed, force a restart.
@@ -976,7 +997,7 @@ Logging in with Google... Please restart Gemini CLI to continue.
ctrlCTimerRef.current = setTimeout(() => {
setCtrlCPressCount(0);
ctrlCTimerRef.current = null;
}, CTRL_EXIT_PROMPT_DURATION_MS);
}, WARNING_PROMPT_DURATION_MS);
}
}, [ctrlCPressCount, config, setCtrlCPressCount, handleSlashCommand]);
@@ -994,7 +1015,7 @@ Logging in with Google... Please restart Gemini CLI to continue.
ctrlDTimerRef.current = setTimeout(() => {
setCtrlDPressCount(0);
ctrlDTimerRef.current = null;
}, CTRL_EXIT_PROMPT_DURATION_MS);
}, WARNING_PROMPT_DURATION_MS);
}
}, [ctrlDPressCount, config, setCtrlDPressCount, handleSlashCommand]);
@@ -1345,6 +1366,7 @@ Logging in with Google... Please restart Gemini CLI to continue.
embeddedShellFocused,
showDebugProfiler,
copyModeEnabled,
selectionWarning,
}),
[
isThemeDialogOpen,
@@ -1430,6 +1452,7 @@ Logging in with Google... Please restart Gemini CLI to continue.
apiKeyDefaultValue,
authState,
copyModeEnabled,
selectionWarning,
],
);

View File

@@ -99,6 +99,10 @@ export const Composer = () => {
<Text color={theme.status.warning}>
Press Ctrl+C again to exit.
</Text>
) : uiState.selectionWarning ? (
<Text color={theme.status.warning}>
Press Ctrl-S to enter selection mode to copy text.
</Text>
) : uiState.ctrlDPressedOnce ? (
<Text color={theme.status.warning}>
Press Ctrl+D again to exit.

View File

@@ -60,4 +60,15 @@ describe('Help Component', () => {
expect(output).not.toContain('hidden-child');
unmount();
});
it('should render keyboard shortcuts', () => {
const { lastFrame, unmount } = render(<Help commands={mockCommands} />);
const output = lastFrame();
expect(output).toContain('Keyboard Shortcuts:');
expect(output).toContain('Ctrl+C');
expect(output).toContain('Ctrl+S');
expect(output).toContain('Page Up/Down');
unmount();
});
});

View File

@@ -136,6 +136,12 @@ export const Help: React.FC<Help> = ({ commands }) => (
</Text>{' '}
- Clear the screen
</Text>
<Text color={theme.text.primary}>
<Text bold color={theme.text.accent}>
Ctrl+S
</Text>{' '}
- Enter selection mode to copy text
</Text>
<Text color={theme.text.primary}>
<Text bold color={theme.text.accent}>
{process.platform === 'darwin' ? 'Ctrl+X / Meta+Enter' : 'Ctrl+X'}
@@ -160,6 +166,12 @@ export const Help: React.FC<Help> = ({ commands }) => (
</Text>{' '}
- Cancel operation / Clear input (double press)
</Text>
<Text color={theme.text.primary}>
<Text bold color={theme.text.accent}>
Page Up/Down
</Text>{' '}
- Scroll page up/down
</Text>
<Text color={theme.text.primary}>
<Text bold color={theme.text.accent}>
Shift+Tab

View File

@@ -379,8 +379,10 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
const relY = mouseY - y;
const visualRow = buffer.visualScrollRow + relY;
buffer.moveToVisualPosition(visualRow, relX);
return true;
}
}
return false;
},
[buffer],
);

View File

@@ -11,6 +11,7 @@ import { vi, type Mock } from 'vitest';
import type React from 'react';
import { useStdin } from 'ink';
import { EventEmitter } from 'node:events';
import { appEvents, AppEvent } from '../../utils/events.js';
// Mock the 'ink' module to control stdin
vi.mock('ink', async (importOriginal) => {
@@ -21,6 +22,18 @@ vi.mock('ink', async (importOriginal) => {
};
});
// Mock appEvents
vi.mock('../../utils/events.js', () => ({
appEvents: {
emit: vi.fn(),
on: vi.fn(),
off: vi.fn(),
},
AppEvent: {
SelectionWarning: 'selection-warning',
},
}));
class MockStdin extends EventEmitter {
isTTY = true;
setRawMode = vi.fn();
@@ -47,6 +60,7 @@ describe('MouseContext', () => {
wrapper = ({ children }: { children: React.ReactNode }) => (
<MouseProvider mouseEventsEnabled={true}>{children}</MouseProvider>
);
vi.mocked(appEvents.emit).mockClear();
});
afterEach(() => {
@@ -91,6 +105,34 @@ describe('MouseContext', () => {
expect(handler).not.toHaveBeenCalled();
});
it('should emit SelectionWarning when move event is unhandled and has coordinates', () => {
renderHook(() => useMouseContext(), { wrapper });
act(() => {
// Move event (32) at 10, 20
stdin.write('\x1b[<32;10;20M');
});
expect(appEvents.emit).toHaveBeenCalledWith(AppEvent.SelectionWarning);
});
it('should not emit SelectionWarning when move event is handled', () => {
const handler = vi.fn().mockReturnValue(true);
const { result } = renderHook(() => useMouseContext(), { wrapper });
act(() => {
result.current.subscribe(handler);
});
act(() => {
// Move event (32) at 10, 20
stdin.write('\x1b[<32;10;20M');
});
expect(handler).toHaveBeenCalled();
expect(appEvents.emit).not.toHaveBeenCalled();
});
describe('SGR Mouse Events', () => {
it.each([
{

View File

@@ -15,6 +15,7 @@ import {
} from 'react';
import { ESC } from '../utils/input.js';
import { debugLogger } from '@google/gemini-cli-core';
import { appEvents, AppEvent } from '../../utils/events.js';
import {
isIncompleteMouseSequence,
parseMouseEvent,
@@ -89,8 +90,23 @@ export function MouseProvider({
let mouseBuffer = '';
const broadcast = (event: MouseEvent) => {
let handled = false;
for (const handler of subscribers) {
handler(event);
if (handler(event) === true) {
handled = true;
}
}
if (
!handled &&
event.name === 'move' &&
event.col >= 0 &&
event.row >= 0
) {
// Terminal apps only receive mouse move events when the mouse is down
// so this always indicates a mouse drag that the user was expecting
// would trigger text selection but does not as we are handling mouse
// events not the terminal.
appEvents.emit(AppEvent.SelectionWarning);
}
};

View File

@@ -16,12 +16,12 @@ import { Box, type DOMElement } from 'ink';
import type { MouseEvent } from '../hooks/useMouse.js';
// Mock useMouse hook
const mockUseMouseCallbacks = new Set<(event: MouseEvent) => void>();
const mockUseMouseCallbacks = new Set<(event: MouseEvent) => void | boolean>();
vi.mock('../hooks/useMouse.js', async () => {
// We need to import React dynamically because this factory runs before top-level imports
const React = await import('react');
return {
useMouse: (callback: (event: MouseEvent) => void) => {
useMouse: (callback: (event: MouseEvent) => void | boolean) => {
React.useEffect(() => {
mockUseMouseCallbacks.add(callback);
return () => {
@@ -81,6 +81,81 @@ describe('ScrollProvider', () => {
vi.useRealTimers();
});
describe('Event Handling Status', () => {
it('returns true when scroll event is handled', () => {
const scrollBy = vi.fn();
const getScrollState = vi.fn(() => ({
scrollTop: 0,
scrollHeight: 100,
innerHeight: 10,
}));
render(
<ScrollProvider>
<TestScrollable
id="test-scrollable"
scrollBy={scrollBy}
getScrollState={getScrollState}
/>
</ScrollProvider>,
);
let handled = false;
for (const callback of mockUseMouseCallbacks) {
if (
callback({
name: 'scroll-down',
col: 5,
row: 5,
shift: false,
ctrl: false,
meta: false,
}) === true
) {
handled = true;
}
}
expect(handled).toBe(true);
});
it('returns false when scroll event is ignored (cannot scroll further)', () => {
const scrollBy = vi.fn();
// Already at bottom
const getScrollState = vi.fn(() => ({
scrollTop: 90,
scrollHeight: 100,
innerHeight: 10,
}));
render(
<ScrollProvider>
<TestScrollable
id="test-scrollable"
scrollBy={scrollBy}
getScrollState={getScrollState}
/>
</ScrollProvider>,
);
let handled = false;
for (const callback of mockUseMouseCallbacks) {
if (
callback({
name: 'scroll-down',
col: 5,
row: 5,
shift: false,
ctrl: false,
meta: false,
}) === true
) {
handled = true;
}
}
expect(handled).toBe(false);
});
});
it('calls scrollTo when clicking scrollbar track if available', async () => {
const scrollBy = vi.fn();
const scrollTo = vi.fn();

View File

@@ -146,15 +146,16 @@ export const ScrollProvider: React.FC<{ children: React.ReactNode }> = ({
if (direction === 'up' && canScrollUp) {
pendingScrollsRef.current.set(candidate.id, pendingDelta + delta);
scheduleFlush();
return;
return true;
}
if (direction === 'down' && canScrollDown) {
pendingScrollsRef.current.set(candidate.id, pendingDelta + delta);
scheduleFlush();
return;
return true;
}
}
return false;
};
const handleLeftPress = (mouseEvent: MouseEvent) => {
@@ -238,7 +239,7 @@ export const ScrollProvider: React.FC<{ children: React.ReactNode }> = ({
id: entry.id,
offset,
};
return;
return true;
}
}
@@ -250,21 +251,27 @@ export const ScrollProvider: React.FC<{ children: React.ReactNode }> = ({
if (candidates.length > 0) {
// The first candidate is the innermost one.
candidates[0].flashScrollbar();
// We don't consider just flashing the scrollbar as handling the event
// in a way that should prevent other handlers (like drag warning)
// from checking it, although for left-press it doesn't matter much.
// But returning false is safer.
return false;
}
return false;
};
const handleMove = (mouseEvent: MouseEvent) => {
const state = dragStateRef.current;
if (!state.active || !state.id) return;
if (!state.active || !state.id) return false;
const entry = scrollablesRef.current.get(state.id);
if (!entry || !entry.ref.current) {
state.active = false;
return;
return false;
}
const boundingBox = getBoundingBox(entry.ref.current);
if (!boundingBox) return;
if (!boundingBox) return false;
const { y } = boundingBox;
const { scrollTop, scrollHeight, innerHeight } = entry.getScrollState();
@@ -276,7 +283,7 @@ export const ScrollProvider: React.FC<{ children: React.ReactNode }> = ({
const maxScrollTop = scrollHeight - innerHeight;
const maxThumbY = innerHeight - thumbHeight;
if (maxThumbY <= 0) return;
if (maxThumbY <= 0) return false;
const relativeMouseY = mouseEvent.row - y;
// Calculate the target thumb position based on the mouse position and the offset.
@@ -295,6 +302,7 @@ export const ScrollProvider: React.FC<{ children: React.ReactNode }> = ({
} else {
entry.scrollBy(targetScrollTop - scrollTop);
}
return true;
};
const handleLeftRelease = () => {
@@ -304,22 +312,25 @@ export const ScrollProvider: React.FC<{ children: React.ReactNode }> = ({
id: null,
offset: 0,
};
return true;
}
return false;
};
useMouse(
(event: MouseEvent) => {
if (event.name === 'scroll-up') {
handleScroll('up', event);
return handleScroll('up', event);
} else if (event.name === 'scroll-down') {
handleScroll('down', event);
return handleScroll('down', event);
} else if (event.name === 'left-press') {
handleLeftPress(event);
return handleLeftPress(event);
} else if (event.name === 'move') {
handleMove(event);
return handleMove(event);
} else if (event.name === 'left-release') {
handleLeftRelease();
return handleLeftRelease();
}
return false;
},
{ isActive: true },
);

View File

@@ -124,6 +124,7 @@ export interface UIState {
showDebugProfiler: boolean;
showFullTodos: boolean;
copyModeEnabled: boolean;
selectionWarning: boolean;
}
export const UIStateContext = createContext<UIState | null>(null);

View File

@@ -28,8 +28,27 @@ import {
} from '@google/gemini-cli-core';
import { appEvents } from '../../utils/events.js';
const { logSlashCommand } = vi.hoisted(() => ({
const {
logSlashCommand,
mockBuiltinLoadCommands,
mockFileLoadCommands,
mockMcpLoadCommands,
mockIdeClientGetInstance,
mockUseAlternateBuffer,
} = vi.hoisted(() => ({
logSlashCommand: vi.fn(),
mockBuiltinLoadCommands: vi.fn().mockResolvedValue([]),
mockFileLoadCommands: vi.fn().mockResolvedValue([]),
mockMcpLoadCommands: vi.fn().mockResolvedValue([]),
mockIdeClientGetInstance: vi.fn().mockResolvedValue({
addStatusChangeListener: vi.fn(),
removeStatusChangeListener: vi.fn(),
}),
mockUseAlternateBuffer: vi.fn().mockReturnValue(false),
}));
vi.mock('./useAlternateBuffer.js', () => ({
useAlternateBuffer: mockUseAlternateBuffer,
}));
vi.mock('@google/gemini-cli-core', async (importOriginal) => {
@@ -41,10 +60,7 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
logSlashCommand,
getIdeInstaller: vi.fn().mockReturnValue(null),
IdeClient: {
getInstance: vi.fn().mockResolvedValue({
addStatusChangeListener: vi.fn(),
removeStatusChangeListener: vi.fn(),
}),
getInstance: mockIdeClientGetInstance,
},
};
});
@@ -65,23 +81,20 @@ vi.mock('node:process', () => {
};
});
const mockBuiltinLoadCommands = vi.fn();
vi.mock('../../services/BuiltinCommandLoader.js', () => ({
BuiltinCommandLoader: vi.fn().mockImplementation(() => ({
BuiltinCommandLoader: vi.fn(() => ({
loadCommands: mockBuiltinLoadCommands,
})),
}));
const mockFileLoadCommands = vi.fn();
vi.mock('../../services/FileCommandLoader.js', () => ({
FileCommandLoader: vi.fn().mockImplementation(() => ({
FileCommandLoader: vi.fn(() => ({
loadCommands: mockFileLoadCommands,
})),
}));
const mockMcpLoadCommands = vi.fn();
vi.mock('../../services/McpPromptLoader.js', () => ({
McpPromptLoader: vi.fn().mockImplementation(() => ({
McpPromptLoader: vi.fn(() => ({
loadCommands: mockMcpLoadCommands,
})),
}));
@@ -130,6 +143,12 @@ describe('useSlashCommandProcessor', () => {
mockBuiltinLoadCommands.mockResolvedValue([]);
mockFileLoadCommands.mockResolvedValue([]);
mockMcpLoadCommands.mockResolvedValue([]);
mockUseAlternateBuffer.mockReturnValue(false);
mockIdeClientGetInstance.mockResolvedValue({
addStatusChangeListener: vi.fn(),
removeStatusChangeListener: vi.fn(),
});
vi.spyOn(console, 'clear').mockImplementation(() => {});
});
afterEach(async () => {
@@ -137,6 +156,7 @@ describe('useSlashCommandProcessor', () => {
await unmountHook();
unmountHook = undefined;
}
vi.restoreAllMocks();
});
const setupProcessorHook = async (
@@ -205,6 +225,44 @@ describe('useSlashCommandProcessor', () => {
};
};
describe('Console Clear Safety', () => {
it('should not call console.clear if alternate buffer is active', async () => {
mockUseAlternateBuffer.mockReturnValue(true);
const clearCommand = createTestCommand({
name: 'clear',
action: async (context) => {
context.ui.clear();
},
});
const result = await setupProcessorHook([clearCommand]);
await act(async () => {
await result.current.handleSlashCommand('/clear');
});
expect(mockClearItems).toHaveBeenCalled();
expect(console.clear).not.toHaveBeenCalled();
});
it('should call console.clear if alternate buffer is not active', async () => {
mockUseAlternateBuffer.mockReturnValue(false);
const clearCommand = createTestCommand({
name: 'clear',
action: async (context) => {
context.ui.clear();
},
});
const result = await setupProcessorHook([clearCommand]);
await act(async () => {
await result.current.handleSlashCommand('/clear');
});
expect(mockClearItems).toHaveBeenCalled();
expect(console.clear).toHaveBeenCalled();
});
});
describe('Initialization and Command Loading', () => {
it('should initialize CommandService with all required loaders', async () => {
await setupProcessorHook();
@@ -947,36 +1005,37 @@ describe('useSlashCommandProcessor', () => {
describe('Slash Command Logging', () => {
const mockCommandAction = vi.fn().mockResolvedValue({ type: 'handled' });
const loggingTestCommands: SlashCommand[] = [
createTestCommand({
name: 'logtest',
action: vi
.fn()
.mockResolvedValue({ type: 'message', content: 'hello world' }),
}),
createTestCommand({
name: 'logwithsub',
subCommands: [
createTestCommand({
name: 'sub',
action: mockCommandAction,
}),
],
}),
createTestCommand({
name: 'fail',
action: vi.fn().mockRejectedValue(new Error('oh no!')),
}),
createTestCommand({
name: 'logalias',
altNames: ['la'],
action: mockCommandAction,
}),
];
let loggingTestCommands: SlashCommand[];
beforeEach(() => {
mockCommandAction.mockClear();
vi.mocked(logSlashCommand).mockClear();
loggingTestCommands = [
createTestCommand({
name: 'logtest',
action: vi
.fn()
.mockResolvedValue({ type: 'message', content: 'hello world' }),
}),
createTestCommand({
name: 'logwithsub',
subCommands: [
createTestCommand({
name: 'sub',
action: mockCommandAction,
}),
],
}),
createTestCommand({
name: 'fail',
action: vi.fn().mockRejectedValue(new Error('oh no!')),
}),
createTestCommand({
name: 'logalias',
altNames: ['la'],
action: mockCommandAction,
}),
];
});
it.each([

View File

@@ -44,6 +44,7 @@ import {
type ExtensionUpdateStatus,
} from '../state/extensions.js';
import { appEvents } from '../../utils/events.js';
import { useAlternateBuffer } from './useAlternateBuffer.js';
interface SlashCommandProcessorActions {
openAuthDialog: () => void;
@@ -81,6 +82,7 @@ export const useSlashCommandProcessor = (
const [commands, setCommands] = useState<readonly SlashCommand[] | undefined>(
undefined,
);
const alternateBuffer = useAlternateBuffer();
const [reloadTrigger, setReloadTrigger] = useState(0);
const reloadCommands = useCallback(() => {
@@ -196,7 +198,9 @@ export const useSlashCommandProcessor = (
addItem,
clear: () => {
clearItems();
console.clear();
if (!alternateBuffer) {
console.clear();
}
refreshStatic();
},
loadHistory,
@@ -218,6 +222,7 @@ export const useSlashCommandProcessor = (
},
}),
[
alternateBuffer,
config,
settings,
gitService,

View File

@@ -35,7 +35,7 @@ export interface MouseEvent {
ctrl: boolean;
}
export type MouseHandler = (event: MouseEvent) => void;
export type MouseHandler = (event: MouseEvent) => void | boolean;
export function getMouseEventName(
buttonCode: number,

View File

@@ -13,6 +13,7 @@ export enum AppEvent {
OauthDisplayMessage = 'oauth-display-message',
Flicker = 'flicker',
McpClientUpdate = 'mcp-client-update',
SelectionWarning = 'selection-warning',
}
export interface AppEvents extends ExtensionEvents {
@@ -21,6 +22,7 @@ export interface AppEvents extends ExtensionEvents {
[AppEvent.OauthDisplayMessage]: string[];
[AppEvent.Flicker]: never[];
[AppEvent.McpClientUpdate]: Array<Map<string, McpClient> | never>;
[AppEvent.SelectionWarning]: never[];
}
export const appEvents = new EventEmitter<AppEvents>();