From e00e8f4728c1cf3934f1ad5078eac71b45e3de18 Mon Sep 17 00:00:00 2001 From: Jacob Richman Date: Fri, 27 Feb 2026 11:03:37 -0800 Subject: [PATCH] fix(cli): Shell autocomplete polish (#20411) --- .../cli/src/ui/components/InputPrompt.tsx | 4 +- .../ui/hooks/useCommandCompletion.test.tsx | 38 +++-- .../cli/src/ui/hooks/useCommandCompletion.tsx | 76 ++++------ .../src/ui/hooks/useShellCompletion.test.ts | 11 +- .../cli/src/ui/hooks/useShellCompletion.ts | 141 ++++++++++++++---- 5 files changed, 183 insertions(+), 87 deletions(-) diff --git a/packages/cli/src/ui/components/InputPrompt.tsx b/packages/cli/src/ui/components/InputPrompt.tsx index ac17284189..38b62ad927 100644 --- a/packages/cli/src/ui/components/InputPrompt.tsx +++ b/packages/cli/src/ui/components/InputPrompt.tsx @@ -895,7 +895,9 @@ export const InputPrompt: React.FC = ({ completion.isPerfectMatch && keyMatchers[Command.SUBMIT](key) && recentUnsafePasteTime === null && - (!completion.showSuggestions || completion.activeSuggestionIndex <= 0) + (!completion.showSuggestions || + (completion.activeSuggestionIndex <= 0 && + !hasUserNavigatedSuggestions.current)) ) { handleSubmit(buffer.text); return true; diff --git a/packages/cli/src/ui/hooks/useCommandCompletion.test.tsx b/packages/cli/src/ui/hooks/useCommandCompletion.test.tsx index c7bb2afb50..8f91013070 100644 --- a/packages/cli/src/ui/hooks/useCommandCompletion.test.tsx +++ b/packages/cli/src/ui/hooks/useCommandCompletion.test.tsx @@ -28,6 +28,7 @@ import type { UseAtCompletionProps } from './useAtCompletion.js'; import { useAtCompletion } from './useAtCompletion.js'; import type { UseSlashCompletionProps } from './useSlashCompletion.js'; import { useSlashCompletion } from './useSlashCompletion.js'; +import { useShellCompletion } from './useShellCompletion.js'; vi.mock('./useAtCompletion', () => ({ useAtCompletion: vi.fn(), @@ -40,29 +41,35 @@ vi.mock('./useSlashCompletion', () => ({ })), })); -vi.mock('./useShellCompletion', async () => { - const actual = await vi.importActual< - typeof import('./useShellCompletion.js') - >('./useShellCompletion'); - return { - ...actual, - useShellCompletion: vi.fn(), - }; -}); +vi.mock('./useShellCompletion', () => ({ + useShellCompletion: vi.fn(() => ({ + completionStart: 0, + completionEnd: 0, + query: '', + })), +})); // Helper to set up mocks in a consistent way for both child hooks const setupMocks = ({ atSuggestions = [], slashSuggestions = [], + shellSuggestions = [], isLoading = false, isPerfectMatch = false, slashCompletionRange = { completionStart: 0, completionEnd: 0 }, + shellCompletionRange = { completionStart: 0, completionEnd: 0, query: '' }, }: { atSuggestions?: Suggestion[]; slashSuggestions?: Suggestion[]; + shellSuggestions?: Suggestion[]; isLoading?: boolean; isPerfectMatch?: boolean; slashCompletionRange?: { completionStart: number; completionEnd: number }; + shellCompletionRange?: { + completionStart: number; + completionEnd: number; + query: string; + }; }) => { // Mock for @-completions (useAtCompletion as Mock).mockImplementation( @@ -99,6 +106,19 @@ const setupMocks = ({ return slashCompletionRange; }, ); + + // Mock for shell completions + (useShellCompletion as Mock).mockImplementation( + ({ enabled, setSuggestions, setIsLoadingSuggestions }) => { + useEffect(() => { + if (enabled) { + setIsLoadingSuggestions(isLoading); + setSuggestions(shellSuggestions); + } + }, [enabled, setSuggestions, setIsLoadingSuggestions]); + return shellCompletionRange; + }, + ); }; describe('useCommandCompletion', () => { diff --git a/packages/cli/src/ui/hooks/useCommandCompletion.tsx b/packages/cli/src/ui/hooks/useCommandCompletion.tsx index 097a1e63b3..f9b772bc93 100644 --- a/packages/cli/src/ui/hooks/useCommandCompletion.tsx +++ b/packages/cli/src/ui/hooks/useCommandCompletion.tsx @@ -13,7 +13,7 @@ import { isSlashCommand } from '../utils/commandUtils.js'; import { toCodePoints } from '../utils/textUtils.js'; import { useAtCompletion } from './useAtCompletion.js'; import { useSlashCompletion } from './useSlashCompletion.js'; -import { useShellCompletion, getTokenAtCursor } from './useShellCompletion.js'; +import { useShellCompletion } from './useShellCompletion.js'; import type { PromptCompletion } from './usePromptCompletion.js'; import { usePromptCompletion, @@ -103,40 +103,22 @@ export function useCommandCompletion({ const { completionMode, - query, + query: memoQuery, completionStart, completionEnd, - shellTokenIsCommand, - shellTokens, - shellCursorIndex, - shellCommandToken, } = useMemo(() => { const currentLine = buffer.lines[cursorRow] || ''; const codePoints = toCodePoints(currentLine); if (shellModeActive) { - const tokenInfo = getTokenAtCursor(currentLine, cursorCol); - if (tokenInfo) { - return { - completionMode: CompletionMode.SHELL, - query: tokenInfo.token, - completionStart: tokenInfo.start, - completionEnd: tokenInfo.end, - shellTokenIsCommand: tokenInfo.isFirstToken, - shellTokens: tokenInfo.tokens, - shellCursorIndex: tokenInfo.cursorIndex, - shellCommandToken: tokenInfo.commandToken, - }; - } return { - completionMode: CompletionMode.SHELL, + completionMode: + currentLine.trim().length === 0 + ? CompletionMode.IDLE + : CompletionMode.SHELL, query: '', - completionStart: cursorCol, - completionEnd: cursorCol, - shellTokenIsCommand: currentLine.trim().length === 0, - shellTokens: [''], - shellCursorIndex: 0, - shellCommandToken: '', + completionStart: -1, + completionEnd: -1, }; } @@ -176,10 +158,6 @@ export function useCommandCompletion({ query: partialPath, completionStart: pathStart, completionEnd: end, - shellTokenIsCommand: false, - shellTokens: [], - shellCursorIndex: -1, - shellCommandToken: '', }; } } @@ -191,10 +169,6 @@ export function useCommandCompletion({ query: currentLine, completionStart: 0, completionEnd: currentLine.length, - shellTokenIsCommand: false, - shellTokens: [], - shellCursorIndex: -1, - shellCommandToken: '', }; } @@ -212,10 +186,6 @@ export function useCommandCompletion({ query: trimmedText, completionStart: 0, completionEnd: trimmedText.length, - shellTokenIsCommand: false, - shellTokens: [], - shellCursorIndex: -1, - shellCommandToken: '', }; } @@ -224,16 +194,12 @@ export function useCommandCompletion({ query: null, completionStart: -1, completionEnd: -1, - shellTokenIsCommand: false, - shellTokens: [], - shellCursorIndex: -1, - shellCommandToken: '', }; }, [cursorRow, cursorCol, buffer.lines, buffer.text, shellModeActive]); useAtCompletion({ enabled: active && completionMode === CompletionMode.AT, - pattern: query || '', + pattern: memoQuery || '', config, cwd, setSuggestions, @@ -243,7 +209,7 @@ export function useCommandCompletion({ const slashCompletionRange = useSlashCompletion({ enabled: active && completionMode === CompletionMode.SLASH && !shellModeActive, - query, + query: memoQuery, slashCommands, commandContext, setSuggestions, @@ -251,18 +217,20 @@ export function useCommandCompletion({ setIsPerfectMatch, }); - useShellCompletion({ + const shellCompletionRange = useShellCompletion({ enabled: active && completionMode === CompletionMode.SHELL, - query: query || '', - isCommandPosition: shellTokenIsCommand, - tokens: shellTokens, - cursorIndex: shellCursorIndex, - commandToken: shellCommandToken, + line: buffer.lines[cursorRow] || '', + cursorCol, cwd, setSuggestions, setIsLoadingSuggestions, }); + const query = + completionMode === CompletionMode.SHELL + ? shellCompletionRange.query + : memoQuery; + const promptCompletion = usePromptCompletion({ buffer, }); @@ -321,6 +289,9 @@ export function useCommandCompletion({ if (completionMode === CompletionMode.SLASH) { start = slashCompletionRange.completionStart; end = slashCompletionRange.completionEnd; + } else if (completionMode === CompletionMode.SHELL) { + start = shellCompletionRange.completionStart; + end = shellCompletionRange.completionEnd; } if (start === -1 || end === -1) { @@ -350,6 +321,7 @@ export function useCommandCompletion({ completionStart, completionEnd, slashCompletionRange, + shellCompletionRange, ], ); @@ -370,6 +342,9 @@ export function useCommandCompletion({ if (completionMode === CompletionMode.SLASH) { start = slashCompletionRange.completionStart; end = slashCompletionRange.completionEnd; + } else if (completionMode === CompletionMode.SHELL) { + start = shellCompletionRange.completionStart; + end = shellCompletionRange.completionEnd; } // Add space padding for Tab completion (auto-execute gets padding from getCompletedText) @@ -408,6 +383,7 @@ export function useCommandCompletion({ completionStart, completionEnd, slashCompletionRange, + shellCompletionRange, getCompletedText, ], ); diff --git a/packages/cli/src/ui/hooks/useShellCompletion.test.ts b/packages/cli/src/ui/hooks/useShellCompletion.test.ts index 477db3b184..dfe33cf7c4 100644 --- a/packages/cli/src/ui/hooks/useShellCompletion.test.ts +++ b/packages/cli/src/ui/hooks/useShellCompletion.test.ts @@ -384,6 +384,10 @@ describe('useShellCompletion utilities', () => { // Very basic sanity check: common commands should be found if (process.platform !== 'win32') { expect(results).toContain('ls'); + } else { + expect(results).toContain('dir'); + expect(results).toContain('cls'); + expect(results).toContain('copy'); } }); @@ -398,7 +402,12 @@ describe('useShellCompletion utilities', () => { it('should handle empty PATH', async () => { vi.stubEnv('PATH', ''); const results = await scanPathExecutables(); - expect(results).toEqual([]); + if (process.platform === 'win32') { + expect(results.length).toBeGreaterThan(0); + expect(results).toContain('dir'); + } else { + expect(results).toEqual([]); + } vi.unstubAllEnvs(); }); }); diff --git a/packages/cli/src/ui/hooks/useShellCompletion.ts b/packages/cli/src/ui/hooks/useShellCompletion.ts index 8569ab5cfb..cc73128344 100644 --- a/packages/cli/src/ui/hooks/useShellCompletion.ts +++ b/packages/cli/src/ui/hooks/useShellCompletion.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { useEffect, useRef, useCallback } from 'react'; +import { useEffect, useRef, useCallback, useMemo } from 'react'; import * as fs from 'node:fs/promises'; import * as path from 'node:path'; import * as os from 'node:os'; @@ -196,6 +196,60 @@ export async function scanPathExecutables( const seen = new Set(); const executables: string[] = []; + // Add Windows shell built-ins + if (isWindows) { + const builtins = [ + 'assoc', + 'break', + 'call', + 'cd', + 'chcp', + 'chdir', + 'cls', + 'color', + 'copy', + 'date', + 'del', + 'dir', + 'echo', + 'endlocal', + 'erase', + 'exit', + 'for', + 'ftype', + 'goto', + 'if', + 'md', + 'mkdir', + 'mklink', + 'move', + 'path', + 'pause', + 'popd', + 'prompt', + 'pushd', + 'rd', + 'rem', + 'ren', + 'rename', + 'rmdir', + 'set', + 'setlocal', + 'shift', + 'start', + 'time', + 'title', + 'type', + 'ver', + 'verify', + 'vol', + ]; + for (const builtin of builtins) { + seen.add(builtin); + executables.push(builtin); + } + } + const dirResults = await Promise.all( dirs.map(async (dir) => { if (signal?.aborted) return []; @@ -365,16 +419,10 @@ export async function resolvePathCompletions( export interface UseShellCompletionProps { /** Whether shell completion is active. */ enabled: boolean; - /** The partial query string (the token under the cursor). */ - query: string; - /** Whether the token is in command position (first word). */ - isCommandPosition: boolean; - /** The full list of parsed tokens */ - tokens: string[]; - /** The cursor index in the full list of parsed tokens */ - cursorIndex: number; - /** The root command token */ - commandToken: string; + /** The current line text. */ + line: string; + /** The current cursor column. */ + cursorCol: number; /** The current working directory for path resolution. */ cwd: string; /** Callback to set suggestions on the parent state. */ @@ -383,33 +431,53 @@ export interface UseShellCompletionProps { setIsLoadingSuggestions: (isLoading: boolean) => void; } +export interface UseShellCompletionReturn { + completionStart: number; + completionEnd: number; + query: string; +} + +const EMPTY_TOKENS: string[] = []; + export function useShellCompletion({ enabled, - query, - isCommandPosition, - tokens, - cursorIndex, - commandToken, + line, + cursorCol, cwd, setSuggestions, setIsLoadingSuggestions, -}: UseShellCompletionProps): void { - const pathCacheRef = useRef(null); +}: UseShellCompletionProps): UseShellCompletionReturn { + const pathCachePromiseRef = useRef | null>(null); const pathEnvRef = useRef(process.env['PATH'] ?? ''); const abortRef = useRef(null); const debounceRef = useRef(null); + const tokenInfo = useMemo( + () => (enabled ? getTokenAtCursor(line, cursorCol) : null), + [enabled, line, cursorCol], + ); + + const { + token: query = '', + start: completionStart = -1, + end: completionEnd = -1, + isFirstToken: isCommandPosition = false, + tokens = EMPTY_TOKENS, + cursorIndex = -1, + commandToken = '', + } = tokenInfo || {}; + // Invalidate PATH cache when $PATH changes useEffect(() => { const currentPath = process.env['PATH'] ?? ''; if (currentPath !== pathEnvRef.current) { - pathCacheRef.current = null; + pathCachePromiseRef.current = null; pathEnvRef.current = currentPath; } }); const performCompletion = useCallback(async () => { - if (!enabled) { + if (!enabled || !tokenInfo) { setSuggestions([]); return; } @@ -434,15 +502,25 @@ export function useShellCompletion({ if (isCommandPosition) { setIsLoadingSuggestions(true); - if (!pathCacheRef.current) { - pathCacheRef.current = await scanPathExecutables(signal); + if (!pathCachePromiseRef.current) { + // We don't pass the signal here because we want the cache to finish + // even if this specific completion request is aborted. + pathCachePromiseRef.current = scanPathExecutables(); } + const executables = await pathCachePromiseRef.current; if (signal.aborted) return; const queryLower = query.toLowerCase(); - results = pathCacheRef.current + results = executables .filter((cmd) => cmd.toLowerCase().startsWith(queryLower)) + .sort((a, b) => { + // Prioritize shorter commands as they are likely common built-ins + if (a.length !== b.length) { + return a.length - b.length; + } + return a.localeCompare(b); + }) .slice(0, MAX_SHELL_SUGGESTIONS) .map((cmd) => ({ label: cmd, @@ -501,6 +579,7 @@ export function useShellCompletion({ } }, [ enabled, + tokenInfo, query, isCommandPosition, tokens, @@ -511,13 +590,17 @@ export function useShellCompletion({ setIsLoadingSuggestions, ]); - // Debounced effect to trigger completion useEffect(() => { if (!enabled) { + abortRef.current?.abort(); setSuggestions([]); setIsLoadingSuggestions(false); - return; } + }, [enabled, setSuggestions, setIsLoadingSuggestions]); + + // Debounced effect to trigger completion + useEffect(() => { + if (!enabled) return; if (debounceRef.current) { clearTimeout(debounceRef.current); @@ -533,7 +616,7 @@ export function useShellCompletion({ clearTimeout(debounceRef.current); } }; - }, [enabled, performCompletion, setSuggestions, setIsLoadingSuggestions]); + }, [enabled, performCompletion]); // Cleanup on unmount useEffect( @@ -545,4 +628,10 @@ export function useShellCompletion({ }, [], ); + + return { + completionStart, + completionEnd, + query, + }; }