From 32809a7be795b974506b893a179091a83b285b7b Mon Sep 17 00:00:00 2001 From: Sandy Tao Date: Thu, 31 Jul 2025 16:07:12 -0700 Subject: [PATCH] feat(cli): Improve @ autocompletion for mid-sentence edits (#5321) --- .../cli/src/ui/hooks/useCompletion.test.ts | 164 ++++++++++-------- packages/cli/src/ui/hooks/useCompletion.ts | 78 +++++---- 2 files changed, 143 insertions(+), 99 deletions(-) diff --git a/packages/cli/src/ui/hooks/useCompletion.test.ts b/packages/cli/src/ui/hooks/useCompletion.test.ts index f876eea1..3a401194 100644 --- a/packages/cli/src/ui/hooks/useCompletion.test.ts +++ b/packages/cli/src/ui/hooks/useCompletion.test.ts @@ -14,7 +14,7 @@ import * as path from 'path'; import * as os from 'os'; import { CommandContext, SlashCommand } from '../commands/types.js'; import { Config, FileDiscoveryService } from '@google/gemini-cli-core'; -import { useTextBuffer, TextBuffer } from '../components/shared/text-buffer.js'; +import { useTextBuffer } from '../components/shared/text-buffer.js'; describe('useCompletion', () => { let testRootDir: string; @@ -38,10 +38,10 @@ describe('useCompletion', () => { } // Helper to create real TextBuffer objects within renderHook - function useTextBufferForTest(text: string) { + function useTextBufferForTest(text: string, cursorOffset?: number) { return useTextBuffer({ initialText: text, - initialCursorOffset: text.length, + initialCursorOffset: cursorOffset ?? text.length, viewport: { width: 80, height: 20 }, isValidPath: () => false, onChange: () => {}, @@ -1113,22 +1113,19 @@ describe('useCompletion', () => { ], }, ] as unknown as SlashCommand[]; - // Create a mock buffer that we can spy on directly - const mockBuffer = { - text: '/mem', - setText: vi.fn(), - } as unknown as TextBuffer; - const { result } = renderHook(() => - useCompletion( - mockBuffer, + const { result } = renderHook(() => { + const textBuffer = useTextBufferForTest('/mem'); + const completion = useCompletion( + textBuffer, testDirs, testRootDir, slashCommands, mockCommandContext, mockConfig, - ), - ); + ); + return { ...completion, textBuffer }; + }); expect(result.current.suggestions.map((s) => s.value)).toEqual([ 'memory', @@ -1138,14 +1135,10 @@ describe('useCompletion', () => { result.current.handleAutocomplete(0); }); - expect(mockBuffer.setText).toHaveBeenCalledWith('/memory '); + expect(result.current.textBuffer.text).toBe('/memory '); }); it('should append a sub-command when the parent is complete', () => { - const mockBuffer = { - text: '/memory', - setText: vi.fn(), - } as unknown as TextBuffer; const slashCommands = [ { name: 'memory', @@ -1163,16 +1156,18 @@ describe('useCompletion', () => { }, ] as unknown as SlashCommand[]; - const { result } = renderHook(() => - useCompletion( - mockBuffer, + const { result } = renderHook(() => { + const textBuffer = useTextBufferForTest('/memory'); + const completion = useCompletion( + textBuffer, testDirs, testRootDir, slashCommands, mockCommandContext, mockConfig, - ), - ); + ); + return { ...completion, textBuffer }; + }); // Suggestions are populated by useEffect expect(result.current.suggestions.map((s) => s.value)).toEqual([ @@ -1184,14 +1179,10 @@ describe('useCompletion', () => { result.current.handleAutocomplete(1); // index 1 is 'add' }); - expect(mockBuffer.setText).toHaveBeenCalledWith('/memory add '); + expect(result.current.textBuffer.text).toBe('/memory add '); }); it('should complete a command with an alternative name', () => { - const mockBuffer = { - text: '/?', - setText: vi.fn(), - } as unknown as TextBuffer; const slashCommands = [ { name: 'memory', @@ -1209,16 +1200,18 @@ describe('useCompletion', () => { }, ] as unknown as SlashCommand[]; - const { result } = renderHook(() => - useCompletion( - mockBuffer, + const { result } = renderHook(() => { + const textBuffer = useTextBufferForTest('/?'); + const completion = useCompletion( + textBuffer, testDirs, testRootDir, slashCommands, mockCommandContext, mockConfig, - ), - ); + ); + return { ...completion, textBuffer }; + }); result.current.suggestions.push({ label: 'help', @@ -1230,44 +1223,22 @@ describe('useCompletion', () => { result.current.handleAutocomplete(0); }); - expect(mockBuffer.setText).toHaveBeenCalledWith('/help '); + expect(result.current.textBuffer.text).toBe('/help '); }); - it('should complete a file path', async () => { - const mockBuffer = { - text: '@src/fi', - lines: ['@src/fi'], - cursor: [0, 7], - setText: vi.fn(), - replaceRangeByOffset: vi.fn(), - } as unknown as TextBuffer; - const slashCommands = [ - { - name: 'memory', - description: 'Manage memory', - subCommands: [ - { - name: 'show', - description: 'Show memory', - }, - { - name: 'add', - description: 'Add to memory', - }, - ], - }, - ] as unknown as SlashCommand[]; - - const { result } = renderHook(() => - useCompletion( - mockBuffer, + it('should complete a file path', () => { + const { result } = renderHook(() => { + const textBuffer = useTextBufferForTest('@src/fi'); + const completion = useCompletion( + textBuffer, testDirs, testRootDir, - slashCommands, + [], mockCommandContext, mockConfig, - ), - ); + ); + return { ...completion, textBuffer }; + }); result.current.suggestions.push({ label: 'file1.txt', @@ -1278,11 +1249,64 @@ describe('useCompletion', () => { result.current.handleAutocomplete(0); }); - expect(mockBuffer.replaceRangeByOffset).toHaveBeenCalledWith( - 5, // after '@src/' - mockBuffer.text.length, - 'file1.txt', - ); + expect(result.current.textBuffer.text).toBe('@src/file1.txt'); + }); + + it('should complete a file path when cursor is not at the end of the line', () => { + const text = '@src/fi le.txt'; + const cursorOffset = 7; // after "i" + + const { result } = renderHook(() => { + const textBuffer = useTextBufferForTest(text, cursorOffset); + const completion = useCompletion( + textBuffer, + testDirs, + testRootDir, + [], + mockCommandContext, + mockConfig, + ); + return { ...completion, textBuffer }; + }); + + result.current.suggestions.push({ + label: 'file1.txt', + value: 'file1.txt', + }); + + act(() => { + result.current.handleAutocomplete(0); + }); + + expect(result.current.textBuffer.text).toBe('@src/file1.txt le.txt'); + }); + + it('should complete the correct file path with multiple @-commands', () => { + const text = '@file1.txt @src/fi'; + + const { result } = renderHook(() => { + const textBuffer = useTextBufferForTest(text); + const completion = useCompletion( + textBuffer, + testDirs, + testRootDir, + [], + mockCommandContext, + mockConfig, + ); + return { ...completion, textBuffer }; + }); + + result.current.suggestions.push({ + label: 'file2.txt', + value: 'file2.txt', + }); + + act(() => { + result.current.handleAutocomplete(0); + }); + + expect(result.current.textBuffer.text).toBe('@file1.txt @src/file2.txt'); }); }); }); diff --git a/packages/cli/src/ui/hooks/useCompletion.ts b/packages/cli/src/ui/hooks/useCompletion.ts index 4b106c1b..77b0ded4 100644 --- a/packages/cli/src/ui/hooks/useCompletion.ts +++ b/packages/cli/src/ui/hooks/useCompletion.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { useState, useEffect, useCallback, useMemo } from 'react'; +import { useState, useEffect, useCallback, useMemo, useRef } from 'react'; import * as fs from 'fs/promises'; import * as path from 'path'; import { glob } from 'glob'; @@ -22,7 +22,10 @@ import { Suggestion, } from '../components/SuggestionsDisplay.js'; import { CommandContext, SlashCommand } from '../commands/types.js'; -import { TextBuffer } from '../components/shared/text-buffer.js'; +import { + logicalPosToOffset, + TextBuffer, +} from '../components/shared/text-buffer.js'; import { isSlashCommand } from '../utils/commandUtils.js'; import { toCodePoints } from '../utils/textUtils.js'; @@ -57,6 +60,11 @@ export function useCompletion( const [isLoadingSuggestions, setIsLoadingSuggestions] = useState(false); const [isPerfectMatch, setIsPerfectMatch] = useState(false); + const completionStart = useRef(-1); + const completionEnd = useRef(-1); + + const cursorRow = buffer.cursor[0]; + const cursorCol = buffer.cursor[1]; const resetCompletionState = useCallback(() => { setSuggestions([]); @@ -127,17 +135,15 @@ export function useCompletion( }, [suggestions.length]); // Check if cursor is after @ or / without unescaped spaces - const isActive = useMemo(() => { + const commandIndex = useMemo(() => { if (isSlashCommand(buffer.text.trim())) { - return true; + return 0; } // For other completions like '@', we search backwards from the cursor. - const [row, col] = buffer.cursor; - const currentLine = buffer.lines[row] || ''; - const codePoints = toCodePoints(currentLine); - for (let i = col - 1; i >= 0; i--) { + const codePoints = toCodePoints(buffer.lines[cursorRow] || ''); + for (let i = cursorCol - 1; i >= 0; i--) { const char = codePoints[i]; if (char === ' ') { @@ -147,19 +153,19 @@ export function useCompletion( backslashCount++; } if (backslashCount % 2 === 0) { - return false; // Inactive on unescaped space. + return -1; // Inactive on unescaped space. } } else if (char === '@') { // Active if we find an '@' before any unescaped space. - return true; + return i; } } - return false; - }, [buffer.text, buffer.cursor, buffer.lines]); + return -1; + }, [buffer.text, cursorRow, cursorCol, buffer.lines]); useEffect(() => { - if (!isActive) { + if (commandIndex === -1) { resetCompletionState(); return; } @@ -311,14 +317,29 @@ export function useCompletion( } // Handle At Command Completion - const atIndex = buffer.text.lastIndexOf('@'); - if (atIndex === -1) { - resetCompletionState(); - return; + const currentLine = buffer.lines[cursorRow] || ''; + const codePoints = toCodePoints(currentLine); + + completionEnd.current = codePoints.length; + for (let i = cursorCol; i < codePoints.length; i++) { + if (codePoints[i] === ' ') { + let backslashCount = 0; + for (let j = i - 1; j >= 0 && codePoints[j] === '\\'; j--) { + backslashCount++; + } + + if (backslashCount % 2 === 0) { + completionEnd.current = i; + break; + } + } } - const partialPath = buffer.text.substring(atIndex + 1); + const pathStart = commandIndex + 1; + const partialPath = currentLine.substring(pathStart, completionEnd.current); const lastSlashIndex = partialPath.lastIndexOf('/'); + completionStart.current = + lastSlashIndex === -1 ? pathStart : pathStart + lastSlashIndex + 1; const baseDirRelative = lastSlashIndex === -1 ? '.' @@ -601,9 +622,12 @@ export function useCompletion( }; }, [ buffer.text, + cursorRow, + cursorCol, + buffer.lines, dirs, cwd, - isActive, + commandIndex, resetCompletionState, slashCommands, commandContext, @@ -669,23 +693,19 @@ export function useCompletion( buffer.setText(newValue); } else { - const atIndex = query.lastIndexOf('@'); - if (atIndex === -1) return; - const pathPart = query.substring(atIndex + 1); - const lastSlashIndexInPath = pathPart.lastIndexOf('/'); - let autoCompleteStartIndex = atIndex + 1; - if (lastSlashIndexInPath !== -1) { - autoCompleteStartIndex += lastSlashIndexInPath + 1; + if (completionStart.current === -1 || completionEnd.current === -1) { + return; } + buffer.replaceRangeByOffset( - autoCompleteStartIndex, - buffer.text.length, + logicalPosToOffset(buffer.lines, cursorRow, completionStart.current), + logicalPosToOffset(buffer.lines, cursorRow, completionEnd.current), suggestion, ); } resetCompletionState(); }, - [resetCompletionState, buffer, suggestions, slashCommands], + [cursorRow, resetCompletionState, buffer, suggestions, slashCommands], ); return {