mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-05-13 05:12:55 -07:00
fix(ui): prevent over-eager slash subcommand completion
This commit is contained in:
@@ -20,7 +20,7 @@ import {
|
|||||||
useCommandCompletion,
|
useCommandCompletion,
|
||||||
CompletionMode,
|
CompletionMode,
|
||||||
} from './useCommandCompletion.js';
|
} from './useCommandCompletion.js';
|
||||||
import type { CommandContext } from '../commands/types.js';
|
import type { CommandContext, SlashCommand } from '../commands/types.js';
|
||||||
import type { Config } from '@google/gemini-cli-core';
|
import type { Config } from '@google/gemini-cli-core';
|
||||||
import { useTextBuffer } from '../components/shared/text-buffer.js';
|
import { useTextBuffer } from '../components/shared/text-buffer.js';
|
||||||
import type { Suggestion } from '../components/SuggestionsDisplay.js';
|
import type { Suggestion } from '../components/SuggestionsDisplay.js';
|
||||||
@@ -56,7 +56,11 @@ const setupMocks = ({
|
|||||||
shellSuggestions = [],
|
shellSuggestions = [],
|
||||||
isLoading = false,
|
isLoading = false,
|
||||||
isPerfectMatch = false,
|
isPerfectMatch = false,
|
||||||
slashCompletionRange = { completionStart: 0, completionEnd: 0 },
|
slashCompletionRange = {
|
||||||
|
completionStart: 0,
|
||||||
|
completionEnd: 0,
|
||||||
|
getCommandFromSuggestion: () => undefined,
|
||||||
|
},
|
||||||
shellCompletionRange = { completionStart: 0, completionEnd: 0, query: '' },
|
shellCompletionRange = { completionStart: 0, completionEnd: 0, query: '' },
|
||||||
}: {
|
}: {
|
||||||
atSuggestions?: Suggestion[];
|
atSuggestions?: Suggestion[];
|
||||||
@@ -64,7 +68,13 @@ const setupMocks = ({
|
|||||||
shellSuggestions?: Suggestion[];
|
shellSuggestions?: Suggestion[];
|
||||||
isLoading?: boolean;
|
isLoading?: boolean;
|
||||||
isPerfectMatch?: boolean;
|
isPerfectMatch?: boolean;
|
||||||
slashCompletionRange?: { completionStart: number; completionEnd: number };
|
slashCompletionRange?: {
|
||||||
|
completionStart: number;
|
||||||
|
completionEnd: number;
|
||||||
|
getCommandFromSuggestion: (
|
||||||
|
suggestion: Suggestion,
|
||||||
|
) => SlashCommand | undefined;
|
||||||
|
};
|
||||||
shellCompletionRange?: {
|
shellCompletionRange?: {
|
||||||
completionStart: number;
|
completionStart: number;
|
||||||
completionEnd: number;
|
completionEnd: number;
|
||||||
@@ -427,10 +437,15 @@ describe('useCommandCompletion', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
describe('handleAutocomplete', () => {
|
describe('handleAutocomplete', () => {
|
||||||
it('should complete a partial command', async () => {
|
it('should complete a partial command and NOT add a space if it has an action', async () => {
|
||||||
setupMocks({
|
setupMocks({
|
||||||
slashSuggestions: [{ label: 'memory', value: 'memory' }],
|
slashSuggestions: [{ label: 'memory', value: 'memory' }],
|
||||||
slashCompletionRange: { completionStart: 1, completionEnd: 4 },
|
slashCompletionRange: {
|
||||||
|
completionStart: 1,
|
||||||
|
completionEnd: 4,
|
||||||
|
getCommandFromSuggestion: () =>
|
||||||
|
({ action: vi.fn() }) as unknown as SlashCommand,
|
||||||
|
},
|
||||||
});
|
});
|
||||||
|
|
||||||
const { result } = renderCommandCompletionHook('/mem');
|
const { result } = renderCommandCompletionHook('/mem');
|
||||||
@@ -443,12 +458,40 @@ describe('useCommandCompletion', () => {
|
|||||||
result.current.handleAutocomplete(0);
|
result.current.handleAutocomplete(0);
|
||||||
});
|
});
|
||||||
|
|
||||||
expect(result.current.textBuffer.text).toBe('/memory ');
|
expect(result.current.textBuffer.text).toBe('/memory');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should complete a partial command and ADD a space if it has NO action (e.g. just a parent)', async () => {
|
||||||
|
setupMocks({
|
||||||
|
slashSuggestions: [{ label: 'chat', value: 'chat' }],
|
||||||
|
slashCompletionRange: {
|
||||||
|
completionStart: 1,
|
||||||
|
completionEnd: 5,
|
||||||
|
getCommandFromSuggestion: () => ({}) as unknown as SlashCommand, // No action
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const { result } = renderCommandCompletionHook('/chat');
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(result.current.suggestions.length).toBe(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
act(() => {
|
||||||
|
result.current.handleAutocomplete(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(result.current.textBuffer.text).toBe('/chat ');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should complete a file path', async () => {
|
it('should complete a file path', async () => {
|
||||||
setupMocks({
|
setupMocks({
|
||||||
atSuggestions: [{ label: 'src/file1.txt', value: 'src/file1.txt' }],
|
atSuggestions: [{ label: 'src/file1.txt', value: 'src/file1.txt' }],
|
||||||
|
slashCompletionRange: {
|
||||||
|
completionStart: 0,
|
||||||
|
completionEnd: 0,
|
||||||
|
getCommandFromSuggestion: () => undefined,
|
||||||
|
},
|
||||||
});
|
});
|
||||||
|
|
||||||
const { result } = renderCommandCompletionHook('@src/fi');
|
const { result } = renderCommandCompletionHook('@src/fi');
|
||||||
@@ -470,6 +513,11 @@ describe('useCommandCompletion', () => {
|
|||||||
|
|
||||||
setupMocks({
|
setupMocks({
|
||||||
atSuggestions: [{ label: 'src/file1.txt', value: 'src/file1.txt' }],
|
atSuggestions: [{ label: 'src/file1.txt', value: 'src/file1.txt' }],
|
||||||
|
slashCompletionRange: {
|
||||||
|
completionStart: 0,
|
||||||
|
completionEnd: 0,
|
||||||
|
getCommandFromSuggestion: () => undefined,
|
||||||
|
},
|
||||||
});
|
});
|
||||||
|
|
||||||
const { result } = renderCommandCompletionHook(text, cursorOffset);
|
const { result } = renderCommandCompletionHook(text, cursorOffset);
|
||||||
@@ -490,6 +538,11 @@ describe('useCommandCompletion', () => {
|
|||||||
it('should complete a directory path ending with / without a trailing space', async () => {
|
it('should complete a directory path ending with / without a trailing space', async () => {
|
||||||
setupMocks({
|
setupMocks({
|
||||||
atSuggestions: [{ label: 'src/components/', value: 'src/components/' }],
|
atSuggestions: [{ label: 'src/components/', value: 'src/components/' }],
|
||||||
|
slashCompletionRange: {
|
||||||
|
completionStart: 0,
|
||||||
|
completionEnd: 0,
|
||||||
|
getCommandFromSuggestion: () => undefined,
|
||||||
|
},
|
||||||
});
|
});
|
||||||
|
|
||||||
const { result } = renderCommandCompletionHook('@src/comp');
|
const { result } = renderCommandCompletionHook('@src/comp');
|
||||||
@@ -510,6 +563,11 @@ describe('useCommandCompletion', () => {
|
|||||||
atSuggestions: [
|
atSuggestions: [
|
||||||
{ label: 'src\\components\\', value: 'src\\components\\' },
|
{ label: 'src\\components\\', value: 'src\\components\\' },
|
||||||
],
|
],
|
||||||
|
slashCompletionRange: {
|
||||||
|
completionStart: 0,
|
||||||
|
completionEnd: 0,
|
||||||
|
getCommandFromSuggestion: () => undefined,
|
||||||
|
},
|
||||||
});
|
});
|
||||||
|
|
||||||
const { result } = renderCommandCompletionHook('@src\\comp');
|
const { result } = renderCommandCompletionHook('@src\\comp');
|
||||||
@@ -713,6 +771,11 @@ describe('useCommandCompletion', () => {
|
|||||||
it('should complete file path and add trailing space', async () => {
|
it('should complete file path and add trailing space', async () => {
|
||||||
setupMocks({
|
setupMocks({
|
||||||
atSuggestions: [{ label: 'src/file.txt', value: 'src/file.txt' }],
|
atSuggestions: [{ label: 'src/file.txt', value: 'src/file.txt' }],
|
||||||
|
slashCompletionRange: {
|
||||||
|
completionStart: 0,
|
||||||
|
completionEnd: 0,
|
||||||
|
getCommandFromSuggestion: () => undefined,
|
||||||
|
},
|
||||||
});
|
});
|
||||||
|
|
||||||
const { result } = renderCommandCompletionHook('/cmd @src/fi');
|
const { result } = renderCommandCompletionHook('/cmd @src/fi');
|
||||||
|
|||||||
@@ -361,10 +361,19 @@ export function useCommandCompletion({
|
|||||||
|
|
||||||
const lineCodePoints = toCodePoints(buffer.lines[cursorRow] || '');
|
const lineCodePoints = toCodePoints(buffer.lines[cursorRow] || '');
|
||||||
const charAfterCompletion = lineCodePoints[end];
|
const charAfterCompletion = lineCodePoints[end];
|
||||||
|
|
||||||
|
const command = slashCompletionRange.getCommandFromSuggestion(suggestion);
|
||||||
|
// Don't add a space if the command has an action (can be executed)
|
||||||
|
// and doesn't have a completion function (doesn't REQUIRE more arguments)
|
||||||
|
const isExecutableCommand = !!(command && command.action);
|
||||||
|
const requiresArguments = !!(command && command.completion);
|
||||||
|
const shouldAddSpace = !isExecutableCommand || requiresArguments;
|
||||||
|
|
||||||
if (
|
if (
|
||||||
charAfterCompletion !== ' ' &&
|
charAfterCompletion !== ' ' &&
|
||||||
!suggestionText.endsWith('/') &&
|
!suggestionText.endsWith('/') &&
|
||||||
!suggestionText.endsWith('\\')
|
!suggestionText.endsWith('\\') &&
|
||||||
|
shouldAddSpace
|
||||||
) {
|
) {
|
||||||
suggestionText += ' ';
|
suggestionText += ' ';
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -492,13 +492,45 @@ describe('useSlashCompletion', () => {
|
|||||||
);
|
);
|
||||||
|
|
||||||
await waitFor(() => {
|
await waitFor(() => {
|
||||||
// Should show subcommands of 'chat'
|
// Should show 'chat' as the suggestion, NOT its subcommands
|
||||||
expect(result.current.suggestions).toHaveLength(2);
|
expect(result.current.suggestions).toHaveLength(1);
|
||||||
expect(result.current.suggestions.map((s) => s.label)).toEqual(
|
expect(result.current.suggestions[0].label).toBe('chat');
|
||||||
expect.arrayContaining(['list', 'save']),
|
// completionStart should be at 1 (to replace 'chat')
|
||||||
|
expect(result.current.completionStart).toBe(1);
|
||||||
|
});
|
||||||
|
unmount();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should NOT suggest subcommands when a parent command is fully typed without a trailing space (fix for over-eager completion)', async () => {
|
||||||
|
const slashCommands = [
|
||||||
|
createTestCommand({
|
||||||
|
name: 'stats',
|
||||||
|
description: 'Check session stats',
|
||||||
|
action: vi.fn(), // Has action
|
||||||
|
subCommands: [
|
||||||
|
createTestCommand({
|
||||||
|
name: 'session',
|
||||||
|
description: 'Show session-specific usage statistics',
|
||||||
|
}),
|
||||||
|
],
|
||||||
|
}),
|
||||||
|
];
|
||||||
|
|
||||||
|
const { result, unmount } = renderHook(() =>
|
||||||
|
useTestHarnessForSlashCompletion(
|
||||||
|
true,
|
||||||
|
'/stats',
|
||||||
|
slashCommands,
|
||||||
|
mockCommandContext,
|
||||||
|
),
|
||||||
);
|
);
|
||||||
// completionStart should be at the end of '/chat' to append subcommands
|
|
||||||
expect(result.current.completionStart).toBe(5);
|
await waitFor(() => {
|
||||||
|
// Should show 'stats' as the suggestion, NOT 'session'
|
||||||
|
expect(result.current.suggestions).toHaveLength(1);
|
||||||
|
expect(result.current.suggestions[0].label).toBe('stats');
|
||||||
|
// isPerfectMatch should be true because it has an action
|
||||||
|
expect(result.current.isPerfectMatch).toBe(true);
|
||||||
});
|
});
|
||||||
unmount();
|
unmount();
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -54,7 +54,6 @@ interface CommandParserResult {
|
|||||||
partial: string;
|
partial: string;
|
||||||
currentLevel: readonly SlashCommand[] | undefined;
|
currentLevel: readonly SlashCommand[] | undefined;
|
||||||
leafCommand: SlashCommand | null;
|
leafCommand: SlashCommand | null;
|
||||||
exactMatchAsParent: SlashCommand | undefined;
|
|
||||||
isArgumentCompletion: boolean;
|
isArgumentCompletion: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -70,7 +69,6 @@ function useCommandParser(
|
|||||||
partial: '',
|
partial: '',
|
||||||
currentLevel: slashCommands,
|
currentLevel: slashCommands,
|
||||||
leafCommand: null,
|
leafCommand: null,
|
||||||
exactMatchAsParent: undefined,
|
|
||||||
isArgumentCompletion: false,
|
isArgumentCompletion: false,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
@@ -112,34 +110,6 @@ function useCommandParser(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
let exactMatchAsParent: SlashCommand | undefined;
|
|
||||||
if (!hasTrailingSpace && currentLevel) {
|
|
||||||
exactMatchAsParent = currentLevel.find(
|
|
||||||
(cmd) => matchesCommand(cmd, partial) && cmd.subCommands,
|
|
||||||
);
|
|
||||||
|
|
||||||
if (exactMatchAsParent) {
|
|
||||||
// Only descend if there are NO other matches for the partial at this level.
|
|
||||||
// This ensures that typing "/memory" still shows "/memory-leak" if it exists.
|
|
||||||
const otherMatches = currentLevel.filter(
|
|
||||||
(cmd) =>
|
|
||||||
cmd !== exactMatchAsParent &&
|
|
||||||
(cmd.name.toLowerCase().startsWith(partial.toLowerCase()) ||
|
|
||||||
cmd.altNames?.some((alt) =>
|
|
||||||
alt.toLowerCase().startsWith(partial.toLowerCase()),
|
|
||||||
)),
|
|
||||||
);
|
|
||||||
|
|
||||||
if (otherMatches.length === 0) {
|
|
||||||
leafCommand = exactMatchAsParent;
|
|
||||||
currentLevel = exactMatchAsParent.subCommands as
|
|
||||||
| readonly SlashCommand[]
|
|
||||||
| undefined;
|
|
||||||
partial = '';
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
const depth = commandPathParts.length;
|
const depth = commandPathParts.length;
|
||||||
const isArgumentCompletion = !!(
|
const isArgumentCompletion = !!(
|
||||||
leafCommand?.completion &&
|
leafCommand?.completion &&
|
||||||
@@ -153,7 +123,6 @@ function useCommandParser(
|
|||||||
partial,
|
partial,
|
||||||
currentLevel,
|
currentLevel,
|
||||||
leafCommand,
|
leafCommand,
|
||||||
exactMatchAsParent,
|
|
||||||
isArgumentCompletion,
|
isArgumentCompletion,
|
||||||
};
|
};
|
||||||
}, [query, slashCommands]);
|
}, [query, slashCommands]);
|
||||||
@@ -356,10 +325,10 @@ function useCompletionPositions(
|
|||||||
return { start: -1, end: -1 };
|
return { start: -1, end: -1 };
|
||||||
}
|
}
|
||||||
|
|
||||||
const { hasTrailingSpace, partial, exactMatchAsParent } = parserResult;
|
const { hasTrailingSpace, partial } = parserResult;
|
||||||
|
|
||||||
// Set completion start/end positions
|
// Set completion start/end positions
|
||||||
if (hasTrailingSpace || exactMatchAsParent) {
|
if (hasTrailingSpace) {
|
||||||
return { start: query.length, end: query.length };
|
return { start: query.length, end: query.length };
|
||||||
} else if (partial) {
|
} else if (partial) {
|
||||||
if (parserResult.isArgumentCompletion) {
|
if (parserResult.isArgumentCompletion) {
|
||||||
|
|||||||
Reference in New Issue
Block a user