diff --git a/.gitignore b/.gitignore index 6ff1d950be..9734c67057 100644 --- a/.gitignore +++ b/.gitignore @@ -56,6 +56,9 @@ bundle junit.xml packages/*/coverage/ +# PR body draft +pr_body.md + # Generated files packages/cli/src/generated/ packages/core/src/generated/ diff --git a/packages/cli/src/ui/commands/directoryCommand.test.tsx b/packages/cli/src/ui/commands/directoryCommand.test.tsx index 5ad0bb1b13..23421ad2b1 100644 --- a/packages/cli/src/ui/commands/directoryCommand.test.tsx +++ b/packages/cli/src/ui/commands/directoryCommand.test.tsx @@ -5,13 +5,18 @@ */ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { directoryCommand, expandHomeDir } from './directoryCommand.js'; +import { + directoryCommand, + expandHomeDir, + getDirPathCompletions, +} from './directoryCommand.js'; import type { Config, WorkspaceContext } from '@qwen-code/qwen-code-core'; import type { CommandContext } from './types.js'; import { MessageType } from '../types.js'; import { SettingScope } from '../../config/settings.js'; import * as os from 'node:os'; import * as path from 'node:path'; +import * as fs from 'node:fs'; describe('directoryCommand', () => { let mockContext: CommandContext; @@ -323,3 +328,116 @@ describe('directoryCommand', () => { ); }); }); + +describe('getDirPathCompletions', () => { + let tempTestDir = ''; + + beforeEach(() => { + // Clean up any previous test runs + if (tempTestDir) { + try { + fs.rmSync(tempTestDir, { recursive: true, force: true }); + } catch (err) { + // ignore cleanup errors + void err; + } + } + + tempTestDir = fs.mkdtempSync(path.join(os.tmpdir(), 'qwen-dir-test-')); + // Create a nested directory structure: root/sub1, root/sub2, root/sub1/deep + fs.mkdirSync(tempTestDir, { recursive: true }); + fs.mkdirSync(path.join(tempTestDir, 'sub1'), { recursive: true }); + fs.mkdirSync(path.join(tempTestDir, 'sub2'), { recursive: true }); + fs.mkdirSync(path.join(tempTestDir, 'sub1', 'deep'), { recursive: true }); + // Add some non-directory files (should be filtered out) + fs.writeFileSync(path.join(tempTestDir, 'file.txt'), ''); + fs.writeFileSync( + path.join(tempTestDir, 'sub1', 'nested.txt'), + '', + ); + }); + + afterAll(() => { + // Cleanup after all tests + if (tempTestDir) { + try { + fs.rmSync(tempTestDir, { recursive: true, force: true }); + } catch (err) { + // ignore cleanup errors + void err; + } + } + }); + + describe('directory completions should include isDirectory flag', () => { + it('should return suggestions with isDirectory: true and trailing /', () => { + // Use "/" suffix so getDirPathCompletions searches INSIDE the directory + const results = getDirPathCompletions(`${tempTestDir}/`); + + expect(results.length).toBeGreaterThan(0); + + // Each suggestion should be a CommandCompletionItem with isDirectory: true + results.forEach((suggestion) => { + expect(suggestion.value).toBeDefined(); + expect(suggestion.isDirectory).toBe(true); + + // Directory values should end with path separator for continued navigation + expect(suggestion.value.endsWith(path.sep)).toBe(true); + + // Should match one of our created directories + const dirNameWithoutSlash = suggestion.value.slice(0, -1); + const basename = path.basename(dirNameWithoutSlash); + expect(['sub1', 'sub2'].includes(basename)).toBe(true); + }); + }); + + it('should filter by prefix while preserving isDirectory flag', () => { + const results = getDirPathCompletions(`${tempTestDir}/su`); + + expect(results.length).toBeGreaterThan(0); + + // Only directories starting with "su" should be returned + results.forEach((suggestion) => { + expect(suggestion.isDirectory).toBe(true); + const sepRe = path.sep === '\\' ? '\\\\' : path.sep; + expect(suggestion.value).toMatch(new RegExp(`${sepRe}su.+$`)); + // Only top-level directories matching the prefix are returned + const basename = path.basename(suggestion.value.slice(0, -1)); + expect(basename).toMatch(/^su/); + const dirname = path.dirname(suggestion.value); + expect(dirname).toContain(tempTestDir); + }); + }); + + it('should support comma-separated paths with isDirectory flag on last segment', () => { + const multiPath = `${tempTestDir}, ${tempTestDir}/`; + const results = getDirPathCompletions(multiPath); + + expect(results.length).toBeGreaterThan(0); + + // Results should start with the prefix from first part + results.forEach((suggestion) => { + expect(suggestion.isDirectory).toBe(true); + expect(suggestion.value.startsWith(`${tempTestDir}`)).toBe(true); + expect(suggestion.value.endsWith(path.sep)).toBe(true); + }); + }); + + it('should handle deeply nested directories with isDirectory flag', () => { + // Navigate into sub1 + const deepResults = getDirPathCompletions(`${tempTestDir}/sub1/`); + + expect(deepResults.length).toBeGreaterThan(0); + + // Only directories inside sub1 should be returned + deepResults.forEach((suggestion) => { + expect(suggestion.isDirectory).toBe(true); + expect(suggestion.value).toContain('sub1'); + expect(suggestion.value.endsWith(path.sep)).toBe(true); + // The nested 'deep' directory should be in the results + const basename = path.basename(suggestion.value.slice(0, -1)); + expect(basename).toBe('deep'); + }); + }); + }); +}); diff --git a/packages/cli/src/ui/commands/directoryCommand.tsx b/packages/cli/src/ui/commands/directoryCommand.tsx index d12a183ff2..1919e8c113 100644 --- a/packages/cli/src/ui/commands/directoryCommand.tsx +++ b/packages/cli/src/ui/commands/directoryCommand.tsx @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import type { SlashCommand, CommandContext } from './types.js'; +import type { SlashCommand, CommandContext, CommandCompletionItem } from './types.js'; import { CommandKind } from './types.js'; import { MessageType } from '../types.js'; import * as fs from 'node:fs'; @@ -58,7 +58,7 @@ function findExistingWorkspaceDirectory( * Returns directory path completions for the given partial argument. * Supports comma-separated paths by completing only the last segment. */ -export function getDirPathCompletions(partialArg: string): string[] { +export function getDirPathCompletions(partialArg: string): CommandCompletionItem[] { const lastComma = partialArg.lastIndexOf(','); const prefix = lastComma >= 0 ? partialArg.substring(0, lastComma + 1) : ''; const partial = @@ -85,7 +85,10 @@ export function getDirPathCompletions(partialArg: string): string[] { e.name.startsWith(namePrefix) && !e.name.startsWith('.'), ) - .map((e) => prefix + path.join(searchDir, e.name)) + .map((e) => ({ + value: prefix + path.join(searchDir, e.name) + path.sep, + isDirectory: true, + })) .slice(0, 8); } catch { return []; diff --git a/packages/cli/src/ui/commands/types.ts b/packages/cli/src/ui/commands/types.ts index b127326275..24aee4a1df 100644 --- a/packages/cli/src/ui/commands/types.ts +++ b/packages/cli/src/ui/commands/types.ts @@ -292,6 +292,8 @@ export interface CommandCompletionItem { value: string; label?: string; description?: string; + /** Whether the completion represents a directory path. When true, handleAutocomplete should NOT append a trailing space so the user can continue tab-completing deeper into the directory tree. */ + isDirectory?: boolean; } // The standardized contract for any command in the system. diff --git a/packages/cli/src/ui/components/SuggestionsDisplay.tsx b/packages/cli/src/ui/components/SuggestionsDisplay.tsx index d2514ba13c..8eaee4df89 100644 --- a/packages/cli/src/ui/components/SuggestionsDisplay.tsx +++ b/packages/cli/src/ui/components/SuggestionsDisplay.tsx @@ -28,6 +28,8 @@ export interface Suggestion { matchedAlias?: string; supportedModes?: ExecutionMode[]; modelInvocable?: boolean; + /** Whether the suggestion represents a directory path. When true, handleAutocomplete should NOT append a trailing space so the user can continue tab-completing deeper into the directory tree. */ + isDirectory?: boolean; } interface SuggestionsDisplayProps { suggestions: Suggestion[]; diff --git a/packages/cli/src/ui/hooks/useAtCompletion.test.ts b/packages/cli/src/ui/hooks/useAtCompletion.test.ts index e2162924bb..fc88425c2d 100644 --- a/packages/cli/src/ui/hooks/useAtCompletion.test.ts +++ b/packages/cli/src/ui/hooks/useAtCompletion.test.ts @@ -143,6 +143,15 @@ describe('useAtCompletion', () => { 'dir/', 'file.txt', ]); + // Verify isDirectory flag + const dirSuggestion = result.current.suggestions.find( + (s) => s.value === 'dir/', + ); + const fileSuggestion = result.current.suggestions.find( + (s) => s.value === 'file.txt', + ); + expect(dirSuggestion?.isDirectory).toBe(true); + expect(fileSuggestion?.isDirectory).toBe(false); }); }); diff --git a/packages/cli/src/ui/hooks/useAtCompletion.ts b/packages/cli/src/ui/hooks/useAtCompletion.ts index 8f3c870ba6..d1793139b7 100644 --- a/packages/cli/src/ui/hooks/useAtCompletion.ts +++ b/packages/cli/src/ui/hooks/useAtCompletion.ts @@ -211,9 +211,13 @@ export function useAtCompletion(props: UseAtCompletionProps): void { return; } + // isDirectory relies on crawler.ts in @qwen-code/qwen-code-core + // always normalizing paths with posix '/' via fdir.withPathSeparator('/'). + // If the crawler ever switches to path.sep, this check must be updated. const suggestions = results.map((p) => ({ label: p, value: escapePath(p), + isDirectory: p.endsWith('/'), })); dispatch({ type: 'SEARCH_SUCCESS', payload: suggestions }); } catch (error) { diff --git a/packages/cli/src/ui/hooks/useCommandCompletion.test.ts b/packages/cli/src/ui/hooks/useCommandCompletion.test.ts index a918348ea5..15fc438b3d 100644 --- a/packages/cli/src/ui/hooks/useCommandCompletion.test.ts +++ b/packages/cli/src/ui/hooks/useCommandCompletion.test.ts @@ -552,6 +552,37 @@ describe('useCommandCompletion', () => { expect(result.current.textBuffer.text).toBe('@src/file1.txt '); }); + it('should not append trailing space for directory completions', async () => { + setupMocks({ + atSuggestions: [ + { label: 'src/components/', value: 'src/components/', isDirectory: true }, + ], + }); + + const { result } = renderHook(() => { + const textBuffer = useTextBufferForTest('@src/com'); + const completion = useCommandCompletion( + textBuffer, + testRootDir, + [], + mockCommandContext, + false, + mockConfig, + ); + return { ...completion, textBuffer }; + }); + + await waitFor(() => { + expect(result.current.suggestions.length).toBe(1); + }); + + act(() => { + result.current.handleAutocomplete(0); + }); + + expect(result.current.textBuffer.text).toBe('@src/components/'); + }); + it('should complete a file path when cursor is not at the end of the line', async () => { const text = '@src/fi is a good file'; const cursorOffset = 7; // after "i" @@ -585,6 +616,46 @@ describe('useCommandCompletion', () => { '@src/file1.txt is a good file', ); }); + + it('should preserve existing space after directory completions at mid-line cursor', async () => { + const text = '@src/com is a dir'; + const cursorOffset = 8; // after "m" + + setupMocks({ + atSuggestions: [ + { + label: 'src/components/', + value: 'src/components/', + isDirectory: true, + }, + ], + }); + + const { result } = renderHook(() => { + const textBuffer = useTextBufferForTest(text, cursorOffset); + const completion = useCommandCompletion( + textBuffer, + testRootDir, + [], + mockCommandContext, + false, + mockConfig, + ); + return { ...completion, textBuffer }; + }); + + await waitFor(() => { + expect(result.current.suggestions.length).toBe(1); + }); + + act(() => { + result.current.handleAutocomplete(0); + }); + + expect(result.current.textBuffer.text).toBe( + '@src/components/ is a dir', + ); + }); }); describe('argument hint ghost text', () => { diff --git a/packages/cli/src/ui/hooks/useCommandCompletion.tsx b/packages/cli/src/ui/hooks/useCommandCompletion.tsx index eb19978541..d4a45dfa87 100644 --- a/packages/cli/src/ui/hooks/useCommandCompletion.tsx +++ b/packages/cli/src/ui/hooks/useCommandCompletion.tsx @@ -228,7 +228,8 @@ export function useCommandCompletion( const lineCodePoints = toCodePoints(buffer.lines[cursorRow] || ''); const charAfterCompletion = lineCodePoints[end]; - if (charAfterCompletion !== ' ') { + const isDirectory = suggestions[indexToUse].isDirectory; + if (charAfterCompletion !== ' ' && !(isDirectory && !charAfterCompletion)) { suggestionText += ' '; } diff --git a/packages/cli/src/ui/hooks/useSlashCompletion.test.ts b/packages/cli/src/ui/hooks/useSlashCompletion.test.ts index bb8e666582..733efbc5ae 100644 --- a/packages/cli/src/ui/hooks/useSlashCompletion.test.ts +++ b/packages/cli/src/ui/hooks/useSlashCompletion.test.ts @@ -1122,4 +1122,39 @@ describe('useSlashCompletion', () => { expect(mockSetIsLoadingSuggestions).not.toHaveBeenCalled(); expect(mockSetIsPerfectMatch).not.toHaveBeenCalled(); }); + + describe('isDirectory propagation', () => { + it('should propagate isDirectory from CommandCompletionItem to Suggestion', async () => { + const mockCompletionFn = vi.fn().mockResolvedValue([ + { value: '/tmp/workspace/', isDirectory: true }, + { value: '/tmp/file.txt' }, + ]); + + const slashCommands = [ + createTestCommand({ + name: 'dir', + description: 'test', + completion: mockCompletionFn, + }), + ]; + + const { result } = renderHook(() => + useTestHarnessForSlashCompletion( + true, + '/dir ', + slashCommands, + mockCommandContext, + ), + ); + + await waitFor(() => { + expect(result.current.suggestions.length).toBe(2); + }); + + // First suggestion (directory) should have isDirectory: true + expect(result.current.suggestions[0].isDirectory).toBe(true); + // Second suggestion (file) should NOT have isDirectory flag + expect(result.current.suggestions[1].isDirectory).toBeFalsy(); + }); + }); }); diff --git a/packages/cli/src/ui/hooks/useSlashCompletion.ts b/packages/cli/src/ui/hooks/useSlashCompletion.ts index 034291ae56..9f6b0ead4f 100644 --- a/packages/cli/src/ui/hooks/useSlashCompletion.ts +++ b/packages/cli/src/ui/hooks/useSlashCompletion.ts @@ -521,6 +521,7 @@ function toSuggestion(item: string | CommandCompletionItem): Suggestion | null { label: item.label ?? item.value, value: item.value, description: item.description, + ...(item.isDirectory !== undefined && { isDirectory: item.isDirectory }), }; }