From b9fe4fc263340c7a614e3e36462284b865e641c7 Mon Sep 17 00:00:00 2001 From: Sandy Tao Date: Mon, 4 Aug 2025 10:49:15 -0700 Subject: [PATCH] feat(cli): Handle Punctuation in @ Command Parsing (#5482) --- .../src/ui/hooks/atCommandProcessor.test.ts | 394 +++++++++++++++++- .../cli/src/ui/hooks/atCommandProcessor.ts | 15 +- .../src/ui/hooks/useSlashCompletion.test.ts | 258 ++++++++++++ .../cli/src/ui/hooks/useSlashCompletion.tsx | 13 +- packages/core/src/utils/paths.test.ts | 214 ++++++++++ packages/core/src/utils/paths.ts | 38 +- 6 files changed, 917 insertions(+), 15 deletions(-) create mode 100644 packages/core/src/utils/paths.test.ts diff --git a/packages/cli/src/ui/hooks/atCommandProcessor.test.ts b/packages/cli/src/ui/hooks/atCommandProcessor.test.ts index 2b4c81a3..583c0b2e 100644 --- a/packages/cli/src/ui/hooks/atCommandProcessor.test.ts +++ b/packages/cli/src/ui/hooks/atCommandProcessor.test.ts @@ -689,5 +689,397 @@ describe('handleAtCommand', () => { `Ignored 1 files:\nGemini-ignored: ${geminiIgnoredFile}`, ); }); - // }); + + describe('punctuation termination in @ commands', () => { + const punctuationTestCases = [ + { + name: 'comma', + fileName: 'test.txt', + fileContent: 'File content here', + queryTemplate: (filePath: string) => + `Look at @${filePath}, then explain it.`, + messageId: 400, + }, + { + name: 'period', + fileName: 'readme.md', + fileContent: 'File content here', + queryTemplate: (filePath: string) => + `Check @${filePath}. What does it say?`, + messageId: 401, + }, + { + name: 'semicolon', + fileName: 'example.js', + fileContent: 'Code example', + queryTemplate: (filePath: string) => + `Review @${filePath}; check for bugs.`, + messageId: 402, + }, + { + name: 'exclamation mark', + fileName: 'important.txt', + fileContent: 'Important content', + queryTemplate: (filePath: string) => + `Look at @${filePath}! This is critical.`, + messageId: 403, + }, + { + name: 'question mark', + fileName: 'config.json', + fileContent: 'Config settings', + queryTemplate: (filePath: string) => + `What is in @${filePath}? Please explain.`, + messageId: 404, + }, + { + name: 'opening parenthesis', + fileName: 'func.ts', + fileContent: 'Function definition', + queryTemplate: (filePath: string) => + `Analyze @${filePath}(the main function).`, + messageId: 405, + }, + { + name: 'closing parenthesis', + fileName: 'data.json', + fileContent: 'Test data', + queryTemplate: (filePath: string) => + `Use data from @${filePath}) for testing.`, + messageId: 406, + }, + { + name: 'opening square bracket', + fileName: 'array.js', + fileContent: 'Array data', + queryTemplate: (filePath: string) => + `Check @${filePath}[0] for the first element.`, + messageId: 407, + }, + { + name: 'closing square bracket', + fileName: 'list.md', + fileContent: 'List content', + queryTemplate: (filePath: string) => + `Review item @${filePath}] from the list.`, + messageId: 408, + }, + { + name: 'opening curly brace', + fileName: 'object.ts', + fileContent: 'Object definition', + queryTemplate: (filePath: string) => + `Parse @${filePath}{prop1: value1}.`, + messageId: 409, + }, + { + name: 'closing curly brace', + fileName: 'config.yaml', + fileContent: 'Configuration', + queryTemplate: (filePath: string) => + `Use settings from @${filePath}} for deployment.`, + messageId: 410, + }, + ]; + + it.each(punctuationTestCases)( + 'should terminate @path at $name', + async ({ fileName, fileContent, queryTemplate, messageId }) => { + const filePath = await createTestFile( + path.join(testRootDir, fileName), + fileContent, + ); + const query = queryTemplate(filePath); + + const result = await handleAtCommand({ + query, + config: mockConfig, + addItem: mockAddItem, + onDebugMessage: mockOnDebugMessage, + messageId, + signal: abortController.signal, + }); + + expect(result).toEqual({ + processedQuery: [ + { text: query }, + { text: '\n--- Content from referenced files ---' }, + { text: `\nContent from @${filePath}:\n` }, + { text: fileContent }, + { text: '\n--- End of content ---' }, + ], + shouldProceed: true, + }); + }, + ); + + it('should handle multiple @paths terminated by different punctuation', async () => { + const content1 = 'First file'; + const file1Path = await createTestFile( + path.join(testRootDir, 'first.txt'), + content1, + ); + const content2 = 'Second file'; + const file2Path = await createTestFile( + path.join(testRootDir, 'second.txt'), + content2, + ); + const query = `Compare @${file1Path}, @${file2Path}; what's different?`; + + const result = await handleAtCommand({ + query, + config: mockConfig, + addItem: mockAddItem, + onDebugMessage: mockOnDebugMessage, + messageId: 411, + signal: abortController.signal, + }); + + expect(result).toEqual({ + processedQuery: [ + { text: `Compare @${file1Path}, @${file2Path}; what's different?` }, + { text: '\n--- Content from referenced files ---' }, + { text: `\nContent from @${file1Path}:\n` }, + { text: content1 }, + { text: `\nContent from @${file2Path}:\n` }, + { text: content2 }, + { text: '\n--- End of content ---' }, + ], + shouldProceed: true, + }); + }); + + it('should still handle escaped spaces in paths before punctuation', async () => { + const fileContent = 'Spaced file content'; + const filePath = await createTestFile( + path.join(testRootDir, 'spaced file.txt'), + fileContent, + ); + const escapedPath = path.join(testRootDir, 'spaced\\ file.txt'); + const query = `Check @${escapedPath}, it has spaces.`; + + const result = await handleAtCommand({ + query, + config: mockConfig, + addItem: mockAddItem, + onDebugMessage: mockOnDebugMessage, + messageId: 412, + signal: abortController.signal, + }); + + expect(result).toEqual({ + processedQuery: [ + { text: `Check @${filePath}, it has spaces.` }, + { text: '\n--- Content from referenced files ---' }, + { text: `\nContent from @${filePath}:\n` }, + { text: fileContent }, + { text: '\n--- End of content ---' }, + ], + shouldProceed: true, + }); + }); + + it('should not break file paths with periods in extensions', async () => { + const fileContent = 'TypeScript content'; + const filePath = await createTestFile( + path.join(testRootDir, 'example.d.ts'), + fileContent, + ); + const query = `Analyze @${filePath} for type definitions.`; + + const result = await handleAtCommand({ + query, + config: mockConfig, + addItem: mockAddItem, + onDebugMessage: mockOnDebugMessage, + messageId: 413, + signal: abortController.signal, + }); + + expect(result).toEqual({ + processedQuery: [ + { text: `Analyze @${filePath} for type definitions.` }, + { text: '\n--- Content from referenced files ---' }, + { text: `\nContent from @${filePath}:\n` }, + { text: fileContent }, + { text: '\n--- End of content ---' }, + ], + shouldProceed: true, + }); + }); + + it('should handle file paths ending with period followed by space', async () => { + const fileContent = 'Config content'; + const filePath = await createTestFile( + path.join(testRootDir, 'config.json'), + fileContent, + ); + const query = `Check @${filePath}. This file contains settings.`; + + const result = await handleAtCommand({ + query, + config: mockConfig, + addItem: mockAddItem, + onDebugMessage: mockOnDebugMessage, + messageId: 414, + signal: abortController.signal, + }); + + expect(result).toEqual({ + processedQuery: [ + { text: `Check @${filePath}. This file contains settings.` }, + { text: '\n--- Content from referenced files ---' }, + { text: `\nContent from @${filePath}:\n` }, + { text: fileContent }, + { text: '\n--- End of content ---' }, + ], + shouldProceed: true, + }); + }); + + it('should handle comma termination with complex file paths', async () => { + const fileContent = 'Package info'; + const filePath = await createTestFile( + path.join(testRootDir, 'package.json'), + fileContent, + ); + const query = `Review @${filePath}, then check dependencies.`; + + const result = await handleAtCommand({ + query, + config: mockConfig, + addItem: mockAddItem, + onDebugMessage: mockOnDebugMessage, + messageId: 415, + signal: abortController.signal, + }); + + expect(result).toEqual({ + processedQuery: [ + { text: `Review @${filePath}, then check dependencies.` }, + { text: '\n--- Content from referenced files ---' }, + { text: `\nContent from @${filePath}:\n` }, + { text: fileContent }, + { text: '\n--- End of content ---' }, + ], + shouldProceed: true, + }); + }); + + it('should not terminate at period within file name', async () => { + const fileContent = 'Version info'; + const filePath = await createTestFile( + path.join(testRootDir, 'version.1.2.3.txt'), + fileContent, + ); + const query = `Check @${filePath} contains version information.`; + + const result = await handleAtCommand({ + query, + config: mockConfig, + addItem: mockAddItem, + onDebugMessage: mockOnDebugMessage, + messageId: 416, + signal: abortController.signal, + }); + + expect(result).toEqual({ + processedQuery: [ + { text: `Check @${filePath} contains version information.` }, + { text: '\n--- Content from referenced files ---' }, + { text: `\nContent from @${filePath}:\n` }, + { text: fileContent }, + { text: '\n--- End of content ---' }, + ], + shouldProceed: true, + }); + }); + + it('should handle end of string termination for period and comma', async () => { + const fileContent = 'End file content'; + const filePath = await createTestFile( + path.join(testRootDir, 'end.txt'), + fileContent, + ); + const query = `Show me @${filePath}.`; + + const result = await handleAtCommand({ + query, + config: mockConfig, + addItem: mockAddItem, + onDebugMessage: mockOnDebugMessage, + messageId: 417, + signal: abortController.signal, + }); + + expect(result).toEqual({ + processedQuery: [ + { text: `Show me @${filePath}.` }, + { text: '\n--- Content from referenced files ---' }, + { text: `\nContent from @${filePath}:\n` }, + { text: fileContent }, + { text: '\n--- End of content ---' }, + ], + shouldProceed: true, + }); + }); + + it('should handle files with special characters in names', async () => { + const fileContent = 'File with special chars content'; + const filePath = await createTestFile( + path.join(testRootDir, 'file$with&special#chars.txt'), + fileContent, + ); + const query = `Check @${filePath} for content.`; + + const result = await handleAtCommand({ + query, + config: mockConfig, + addItem: mockAddItem, + onDebugMessage: mockOnDebugMessage, + messageId: 418, + signal: abortController.signal, + }); + + expect(result).toEqual({ + processedQuery: [ + { text: `Check @${filePath} for content.` }, + { text: '\n--- Content from referenced files ---' }, + { text: `\nContent from @${filePath}:\n` }, + { text: fileContent }, + { text: '\n--- End of content ---' }, + ], + shouldProceed: true, + }); + }); + + it('should handle basic file names without special characters', async () => { + const fileContent = 'Basic file content'; + const filePath = await createTestFile( + path.join(testRootDir, 'basicfile.txt'), + fileContent, + ); + const query = `Check @${filePath} please.`; + + const result = await handleAtCommand({ + query, + config: mockConfig, + addItem: mockAddItem, + onDebugMessage: mockOnDebugMessage, + messageId: 421, + signal: abortController.signal, + }); + + expect(result).toEqual({ + processedQuery: [ + { text: `Check @${filePath} please.` }, + { text: '\n--- Content from referenced files ---' }, + { text: `\nContent from @${filePath}:\n` }, + { text: fileContent }, + { text: '\n--- End of content ---' }, + ], + shouldProceed: true, + }); + }); + }); }); diff --git a/packages/cli/src/ui/hooks/atCommandProcessor.ts b/packages/cli/src/ui/hooks/atCommandProcessor.ts index 7b9005fa..165b7b30 100644 --- a/packages/cli/src/ui/hooks/atCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/atCommandProcessor.ts @@ -87,9 +87,17 @@ function parseAllAtCommands(query: string): AtCommandPart[] { inEscape = false; } else if (char === '\\') { inEscape = true; - } else if (/\s/.test(char)) { - // Path ends at first whitespace not escaped + } else if (/[,\s;!?()[\]{}]/.test(char)) { + // Path ends at first whitespace or punctuation not escaped break; + } else if (char === '.') { + // For . we need to be more careful - only terminate if followed by whitespace or end of string + // This allows file extensions like .txt, .js but terminates at sentence endings like "file.txt. Next sentence" + const nextChar = + pathEndIndex + 1 < query.length ? query[pathEndIndex + 1] : ''; + if (nextChar === '' || /\s/.test(nextChar)) { + break; + } } pathEndIndex++; } @@ -320,8 +328,7 @@ export async function handleAtCommand({ if ( i > 0 && initialQueryText.length > 0 && - !initialQueryText.endsWith(' ') && - resolvedSpec + !initialQueryText.endsWith(' ') ) { // Add space if previous part was text and didn't end with space, or if previous was @path const prevPart = commandParts[i - 1]; diff --git a/packages/cli/src/ui/hooks/useSlashCompletion.test.ts b/packages/cli/src/ui/hooks/useSlashCompletion.test.ts index 13f8c240..206c4dc9 100644 --- a/packages/cli/src/ui/hooks/useSlashCompletion.test.ts +++ b/packages/cli/src/ui/hooks/useSlashCompletion.test.ts @@ -1350,4 +1350,262 @@ describe('useSlashCompletion', () => { expect(result.current.textBuffer.text).toBe('@file1.txt @src/file2.txt'); }); }); + + describe('File Path Escaping', () => { + it('should escape special characters in file names', async () => { + await createTestFile('', 'my file.txt'); + await createTestFile('', 'file(1).txt'); + await createTestFile('', 'backup[old].txt'); + + const { result } = renderHook(() => + useSlashCompletion( + useTextBufferForTest('@my'), + testDirs, + testRootDir, + [], + mockCommandContext, + false, + mockConfig, + ), + ); + + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 150)); + }); + + const suggestion = result.current.suggestions.find( + (s) => s.label === 'my file.txt', + ); + expect(suggestion).toBeDefined(); + expect(suggestion!.value).toBe('my\\ file.txt'); + }); + + it('should escape parentheses in file names', async () => { + await createTestFile('', 'document(final).docx'); + await createTestFile('', 'script(v2).sh'); + + const { result } = renderHook(() => + useSlashCompletion( + useTextBufferForTest('@doc'), + testDirs, + testRootDir, + [], + mockCommandContext, + false, + mockConfig, + ), + ); + + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 150)); + }); + + const suggestion = result.current.suggestions.find( + (s) => s.label === 'document(final).docx', + ); + expect(suggestion).toBeDefined(); + expect(suggestion!.value).toBe('document\\(final\\).docx'); + }); + + it('should escape square brackets in file names', async () => { + await createTestFile('', 'backup[2024-01-01].zip'); + await createTestFile('', 'config[dev].json'); + + const { result } = renderHook(() => + useSlashCompletion( + useTextBufferForTest('@backup'), + testDirs, + testRootDir, + [], + mockCommandContext, + false, + mockConfig, + ), + ); + + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 150)); + }); + + const suggestion = result.current.suggestions.find( + (s) => s.label === 'backup[2024-01-01].zip', + ); + expect(suggestion).toBeDefined(); + expect(suggestion!.value).toBe('backup\\[2024-01-01\\].zip'); + }); + + it('should escape multiple special characters in file names', async () => { + await createTestFile('', 'my file (backup) [v1.2].txt'); + await createTestFile('', 'data & config {prod}.json'); + + const { result } = renderHook(() => + useSlashCompletion( + useTextBufferForTest('@my'), + testDirs, + testRootDir, + [], + mockCommandContext, + false, + mockConfig, + ), + ); + + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 150)); + }); + + const suggestion = result.current.suggestions.find( + (s) => s.label === 'my file (backup) [v1.2].txt', + ); + expect(suggestion).toBeDefined(); + expect(suggestion!.value).toBe( + 'my\\ file\\ \\(backup\\)\\ \\[v1.2\\].txt', + ); + }); + + it('should preserve path separators while escaping special characters', async () => { + await createTestFile( + '', + 'projects', + 'my project (2024)', + 'file with spaces.txt', + ); + + const { result } = renderHook(() => + useSlashCompletion( + useTextBufferForTest('@projects/my'), + testDirs, + testRootDir, + [], + mockCommandContext, + false, + mockConfig, + ), + ); + + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 150)); + }); + + const suggestion = result.current.suggestions.find((s) => + s.label.includes('my project'), + ); + expect(suggestion).toBeDefined(); + // Should escape spaces and parentheses but preserve forward slashes + expect(suggestion!.value).toMatch(/my\\ project\\ \\\(2024\\\)/); + expect(suggestion!.value).toContain('/'); // Should contain forward slash for path separator + }); + + it('should normalize Windows path separators to forward slashes while preserving escaping', async () => { + // Create test with complex nested structure + await createTestFile( + '', + 'deep', + 'nested', + 'special folder', + 'file with (parentheses).txt', + ); + + const { result } = renderHook(() => + useSlashCompletion( + useTextBufferForTest('@deep/nested/special'), + testDirs, + testRootDir, + [], + mockCommandContext, + false, + mockConfig, + ), + ); + + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 150)); + }); + + const suggestion = result.current.suggestions.find((s) => + s.label.includes('special folder'), + ); + expect(suggestion).toBeDefined(); + // Should use forward slashes for path separators and escape spaces + expect(suggestion!.value).toContain('special\\ folder/'); + expect(suggestion!.value).not.toContain('\\\\'); // Should not contain double backslashes for path separators + }); + + it('should handle directory names with special characters', async () => { + await createEmptyDir('my documents (personal)'); + await createEmptyDir('config [production]'); + await createEmptyDir('data & logs'); + + const { result } = renderHook(() => + useSlashCompletion( + useTextBufferForTest('@'), + testDirs, + testRootDir, + [], + mockCommandContext, + false, + mockConfig, + ), + ); + + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 150)); + }); + + const suggestions = result.current.suggestions; + + const docSuggestion = suggestions.find( + (s) => s.label === 'my documents (personal)/', + ); + expect(docSuggestion).toBeDefined(); + expect(docSuggestion!.value).toBe('my\\ documents\\ \\(personal\\)/'); + + const configSuggestion = suggestions.find( + (s) => s.label === 'config [production]/', + ); + expect(configSuggestion).toBeDefined(); + expect(configSuggestion!.value).toBe('config\\ \\[production\\]/'); + + const dataSuggestion = suggestions.find( + (s) => s.label === 'data & logs/', + ); + expect(dataSuggestion).toBeDefined(); + expect(dataSuggestion!.value).toBe('data\\ \\&\\ logs/'); + }); + + it('should handle files with various shell metacharacters', async () => { + await createTestFile('', 'file$var.txt'); + await createTestFile('', 'important!.md'); + + const { result } = renderHook(() => + useSlashCompletion( + useTextBufferForTest('@'), + testDirs, + testRootDir, + [], + mockCommandContext, + false, + mockConfig, + ), + ); + + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 150)); + }); + + const suggestions = result.current.suggestions; + + const dollarSuggestion = suggestions.find( + (s) => s.label === 'file$var.txt', + ); + expect(dollarSuggestion).toBeDefined(); + expect(dollarSuggestion!.value).toBe('file\\$var.txt'); + + const importantSuggestion = suggestions.find( + (s) => s.label === 'important!.md', + ); + expect(importantSuggestion).toBeDefined(); + expect(importantSuggestion!.value).toBe('important\\!.md'); + }); + }); }); diff --git a/packages/cli/src/ui/hooks/useSlashCompletion.tsx b/packages/cli/src/ui/hooks/useSlashCompletion.tsx index f68d52d8..c6821358 100644 --- a/packages/cli/src/ui/hooks/useSlashCompletion.tsx +++ b/packages/cli/src/ui/hooks/useSlashCompletion.tsx @@ -16,6 +16,7 @@ import { Config, FileDiscoveryService, DEFAULT_FILE_FILTERING_OPTIONS, + SHELL_SPECIAL_CHARS, } from '@google/gemini-cli-core'; import { Suggestion } from '../components/SuggestionsDisplay.js'; import { CommandContext, SlashCommand } from '../commands/types.js'; @@ -513,11 +514,17 @@ export function useSlashCompletion( ]; } - // Like glob, we always return forwardslashes, even in windows. + // Like glob, we always return forward slashes for path separators, even on Windows. + // But preserve backslash escaping for special characters. + const specialCharsLookahead = `(?![${SHELL_SPECIAL_CHARS.source.slice(1, -1)}])`; + const pathSeparatorRegex = new RegExp( + `\\\\${specialCharsLookahead}`, + 'g', + ); fetchedSuggestions = fetchedSuggestions.map((suggestion) => ({ ...suggestion, - label: suggestion.label.replace(/\\/g, '/'), - value: suggestion.value.replace(/\\/g, '/'), + label: suggestion.label.replace(pathSeparatorRegex, '/'), + value: suggestion.value.replace(pathSeparatorRegex, '/'), })); // Sort by depth, then directories first, then alphabetically diff --git a/packages/core/src/utils/paths.test.ts b/packages/core/src/utils/paths.test.ts new file mode 100644 index 00000000..d688c072 --- /dev/null +++ b/packages/core/src/utils/paths.test.ts @@ -0,0 +1,214 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import { escapePath, unescapePath } from './paths.js'; + +describe('escapePath', () => { + it('should escape spaces', () => { + expect(escapePath('my file.txt')).toBe('my\\ file.txt'); + }); + + it('should escape tabs', () => { + expect(escapePath('file\twith\ttabs.txt')).toBe('file\\\twith\\\ttabs.txt'); + }); + + it('should escape parentheses', () => { + expect(escapePath('file(1).txt')).toBe('file\\(1\\).txt'); + }); + + it('should escape square brackets', () => { + expect(escapePath('file[backup].txt')).toBe('file\\[backup\\].txt'); + }); + + it('should escape curly braces', () => { + expect(escapePath('file{temp}.txt')).toBe('file\\{temp\\}.txt'); + }); + + it('should escape semicolons', () => { + expect(escapePath('file;name.txt')).toBe('file\\;name.txt'); + }); + + it('should escape ampersands', () => { + expect(escapePath('file&name.txt')).toBe('file\\&name.txt'); + }); + + it('should escape pipes', () => { + expect(escapePath('file|name.txt')).toBe('file\\|name.txt'); + }); + + it('should escape asterisks', () => { + expect(escapePath('file*.txt')).toBe('file\\*.txt'); + }); + + it('should escape question marks', () => { + expect(escapePath('file?.txt')).toBe('file\\?.txt'); + }); + + it('should escape dollar signs', () => { + expect(escapePath('file$name.txt')).toBe('file\\$name.txt'); + }); + + it('should escape backticks', () => { + expect(escapePath('file`name.txt')).toBe('file\\`name.txt'); + }); + + it('should escape single quotes', () => { + expect(escapePath("file'name.txt")).toBe("file\\'name.txt"); + }); + + it('should escape double quotes', () => { + expect(escapePath('file"name.txt')).toBe('file\\"name.txt'); + }); + + it('should escape hash symbols', () => { + expect(escapePath('file#name.txt')).toBe('file\\#name.txt'); + }); + + it('should escape exclamation marks', () => { + expect(escapePath('file!name.txt')).toBe('file\\!name.txt'); + }); + + it('should escape tildes', () => { + expect(escapePath('file~name.txt')).toBe('file\\~name.txt'); + }); + + it('should escape less than and greater than signs', () => { + expect(escapePath('file.txt')).toBe('file\\.txt'); + }); + + it('should handle multiple special characters', () => { + expect(escapePath('my file (backup) [v1.2].txt')).toBe( + 'my\\ file\\ \\(backup\\)\\ \\[v1.2\\].txt', + ); + }); + + it('should not double-escape already escaped characters', () => { + expect(escapePath('my\\ file.txt')).toBe('my\\ file.txt'); + expect(escapePath('file\\(name\\).txt')).toBe('file\\(name\\).txt'); + }); + + it('should handle escaped backslashes correctly', () => { + // Double backslash (escaped backslash) followed by space should escape the space + expect(escapePath('path\\\\ file.txt')).toBe('path\\\\\\ file.txt'); + // Triple backslash (escaped backslash + escaping backslash) followed by space should not double-escape + expect(escapePath('path\\\\\\ file.txt')).toBe('path\\\\\\ file.txt'); + // Quadruple backslash (two escaped backslashes) followed by space should escape the space + expect(escapePath('path\\\\\\\\ file.txt')).toBe('path\\\\\\\\\\ file.txt'); + }); + + it('should handle complex escaped backslash scenarios', () => { + // Escaped backslash before special character that needs escaping + expect(escapePath('file\\\\(test).txt')).toBe('file\\\\\\(test\\).txt'); + // Multiple escaped backslashes + expect(escapePath('path\\\\\\\\with space.txt')).toBe( + 'path\\\\\\\\with\\ space.txt', + ); + }); + + it('should handle paths without special characters', () => { + expect(escapePath('normalfile.txt')).toBe('normalfile.txt'); + expect(escapePath('path/to/normalfile.txt')).toBe('path/to/normalfile.txt'); + }); + + it('should handle complex real-world examples', () => { + expect(escapePath('My Documents/Project (2024)/file [backup].txt')).toBe( + 'My\\ Documents/Project\\ \\(2024\\)/file\\ \\[backup\\].txt', + ); + expect(escapePath('file with $special &chars!.txt')).toBe( + 'file\\ with\\ \\$special\\ \\&chars\\!.txt', + ); + }); + + it('should handle empty strings', () => { + expect(escapePath('')).toBe(''); + }); + + it('should handle paths with only special characters', () => { + expect(escapePath(' ()[]{};&|*?$`\'"#!~<>')).toBe( + '\\ \\(\\)\\[\\]\\{\\}\\;\\&\\|\\*\\?\\$\\`\\\'\\"\\#\\!\\~\\<\\>', + ); + }); +}); + +describe('unescapePath', () => { + it('should unescape spaces', () => { + expect(unescapePath('my\\ file.txt')).toBe('my file.txt'); + }); + + it('should unescape tabs', () => { + expect(unescapePath('file\\\twith\\\ttabs.txt')).toBe( + 'file\twith\ttabs.txt', + ); + }); + + it('should unescape parentheses', () => { + expect(unescapePath('file\\(1\\).txt')).toBe('file(1).txt'); + }); + + it('should unescape square brackets', () => { + expect(unescapePath('file\\[backup\\].txt')).toBe('file[backup].txt'); + }); + + it('should unescape curly braces', () => { + expect(unescapePath('file\\{temp\\}.txt')).toBe('file{temp}.txt'); + }); + + it('should unescape multiple special characters', () => { + expect(unescapePath('my\\ file\\ \\(backup\\)\\ \\[v1.2\\].txt')).toBe( + 'my file (backup) [v1.2].txt', + ); + }); + + it('should handle paths without escaped characters', () => { + expect(unescapePath('normalfile.txt')).toBe('normalfile.txt'); + expect(unescapePath('path/to/normalfile.txt')).toBe( + 'path/to/normalfile.txt', + ); + }); + + it('should handle all special characters', () => { + expect( + unescapePath( + '\\ \\(\\)\\[\\]\\{\\}\\;\\&\\|\\*\\?\\$\\`\\\'\\"\\#\\!\\~\\<\\>', + ), + ).toBe(' ()[]{};&|*?$`\'"#!~<>'); + }); + + it('should be the inverse of escapePath', () => { + const testCases = [ + 'my file.txt', + 'file(1).txt', + 'file[backup].txt', + 'My Documents/Project (2024)/file [backup].txt', + 'file with $special &chars!.txt', + ' ()[]{};&|*?$`\'"#!~<>', + 'file\twith\ttabs.txt', + ]; + + testCases.forEach((testCase) => { + expect(unescapePath(escapePath(testCase))).toBe(testCase); + }); + }); + + it('should handle empty strings', () => { + expect(unescapePath('')).toBe(''); + }); + + it('should not affect backslashes not followed by special characters', () => { + expect(unescapePath('file\\name.txt')).toBe('file\\name.txt'); + expect(unescapePath('path\\to\\file.txt')).toBe('path\\to\\file.txt'); + }); + + it('should handle escaped backslashes in unescaping', () => { + // Should correctly unescape when there are escaped backslashes + expect(unescapePath('path\\\\\\ file.txt')).toBe('path\\\\ file.txt'); + expect(unescapePath('path\\\\\\\\\\ file.txt')).toBe( + 'path\\\\\\\\ file.txt', + ); + expect(unescapePath('file\\\\\\(test\\).txt')).toBe('file\\\\(test).txt'); + }); +}); diff --git a/packages/core/src/utils/paths.ts b/packages/core/src/utils/paths.ts index 5370a7cb..7bab888e 100644 --- a/packages/core/src/utils/paths.ts +++ b/packages/core/src/utils/paths.ts @@ -13,6 +13,13 @@ export const GOOGLE_ACCOUNTS_FILENAME = 'google_accounts.json'; const TMP_DIR_NAME = 'tmp'; const COMMANDS_DIR_NAME = 'commands'; +/** + * Special characters that need to be escaped in file paths for shell compatibility. + * Includes: spaces, parentheses, brackets, braces, semicolons, ampersands, pipes, + * asterisks, question marks, dollar signs, backticks, quotes, hash, and other shell metacharacters. + */ +export const SHELL_SPECIAL_CHARS = /[ \t()[\]{};|*?$`'"#&<>!~]/; + /** * Replaces the home directory with a tilde. * @param path - The path to tildeify. @@ -119,26 +126,43 @@ export function makeRelative( } /** - * Escapes spaces in a file path. + * Escapes special characters in a file path like macOS terminal does. + * Escapes: spaces, parentheses, brackets, braces, semicolons, ampersands, pipes, + * asterisks, question marks, dollar signs, backticks, quotes, hash, and other shell metacharacters. */ export function escapePath(filePath: string): string { let result = ''; for (let i = 0; i < filePath.length; i++) { - // Only escape spaces that are not already escaped. - if (filePath[i] === ' ' && (i === 0 || filePath[i - 1] !== '\\')) { - result += '\\ '; + const char = filePath[i]; + + // Count consecutive backslashes before this character + let backslashCount = 0; + for (let j = i - 1; j >= 0 && filePath[j] === '\\'; j--) { + backslashCount++; + } + + // Character is already escaped if there's an odd number of backslashes before it + const isAlreadyEscaped = backslashCount % 2 === 1; + + // Only escape if not already escaped + if (!isAlreadyEscaped && SHELL_SPECIAL_CHARS.test(char)) { + result += '\\' + char; } else { - result += filePath[i]; + result += char; } } return result; } /** - * Unescapes spaces in a file path. + * Unescapes special characters in a file path. + * Removes backslash escaping from shell metacharacters. */ export function unescapePath(filePath: string): string { - return filePath.replace(/\\ /g, ' '); + return filePath.replace( + new RegExp(`\\\\([${SHELL_SPECIAL_CHARS.source.slice(1, -1)}])`, 'g'), + '$1', + ); } /**