From 1b98c1f806acacb359f7c358a102074c9052a9b8 Mon Sep 17 00:00:00 2001 From: Tommaso Sciortino Date: Mon, 9 Feb 2026 13:19:51 -0800 Subject: [PATCH] refactor: push isValidPath() into parsePastedPaths() (#18664) --- packages/cli/src/ui/AppContainer.tsx | 11 +- packages/cli/src/ui/auth/ApiAuthDialog.tsx | 1 - .../cli/src/ui/components/AskUserDialog.tsx | 2 - .../ui/components/ConfigExtensionDialog.tsx | 2 +- .../cli/src/ui/components/SettingsDialog.tsx | 1 - .../ui/components/shared/performance.test.ts | 2 - .../ui/components/shared/text-buffer.test.ts | 258 +++++++----------- .../src/ui/components/shared/text-buffer.ts | 11 +- .../src/ui/components/triage/TriageIssues.tsx | 1 - .../ui/hooks/useCommandCompletion.test.tsx | 1 - .../hooks/useReverseSearchCompletion.test.tsx | 1 - .../cli/src/ui/utils/clipboardUtils.test.ts | 163 +++++++---- packages/cli/src/ui/utils/clipboardUtils.ts | 23 +- packages/core/src/utils/paths.test.ts | 23 +- packages/core/src/utils/paths.ts | 8 +- 15 files changed, 247 insertions(+), 261 deletions(-) diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index c228bd43ea..12ec88a8ac 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -88,7 +88,6 @@ import { calculatePromptWidths } from './components/InputPrompt.js'; import { useApp, useStdout, useStdin } from 'ink'; import { calculateMainAreaWidth } from './utils/ui-sizing.js'; import ansiEscapes from 'ansi-escapes'; -import * as fs from 'node:fs'; import { basename } from 'node:path'; import { computeTerminalTitle } from '../utils/windowTitle.js'; import { useTextBuffer } from './components/shared/text-buffer.js'; @@ -468,14 +467,6 @@ export const AppContainer = (props: AppContainerProps) => { const staticAreaMaxItemHeight = Math.max(terminalHeight * 4, 100); - const isValidPath = useCallback((filePath: string): boolean => { - try { - return fs.existsSync(filePath) && fs.statSync(filePath).isFile(); - } catch (_e) { - return false; - } - }, []); - const getPreferredEditor = useCallback( () => settings.merged.general.preferredEditor as EditorType, [settings.merged.general.preferredEditor], @@ -486,7 +477,7 @@ export const AppContainer = (props: AppContainerProps) => { viewport: { height: 10, width: inputWidth }, stdin, setRawMode, - isValidPath, + escapePastedPaths: true, shellModeActive, getPreferredEditor, }); diff --git a/packages/cli/src/ui/auth/ApiAuthDialog.tsx b/packages/cli/src/ui/auth/ApiAuthDialog.tsx index a9864e27af..c5ac742955 100644 --- a/packages/cli/src/ui/auth/ApiAuthDialog.tsx +++ b/packages/cli/src/ui/auth/ApiAuthDialog.tsx @@ -49,7 +49,6 @@ export function ApiAuthDialog({ width: viewportWidth, height: 4, }, - isValidPath: () => false, // No path validation needed for API key inputFilter: (text) => text.replace(/[^a-zA-Z0-9_-]/g, '').replace(/[\r\n]/g, ''), singleLine: true, diff --git a/packages/cli/src/ui/components/AskUserDialog.tsx b/packages/cli/src/ui/components/AskUserDialog.tsx index 62a1f3c70b..f60a39311e 100644 --- a/packages/cli/src/ui/components/AskUserDialog.tsx +++ b/packages/cli/src/ui/components/AskUserDialog.tsx @@ -285,7 +285,6 @@ const TextQuestionView: React.FC = ({ initialText: initialAnswer, viewport: { width: Math.max(1, bufferWidth), height: 1 }, singleLine: true, - isValidPath: () => false, }); const { text: textValue } = buffer; @@ -564,7 +563,6 @@ const ChoiceQuestionView: React.FC = ({ initialText: initialCustomText, viewport: { width: Math.max(1, bufferWidth), height: 1 }, singleLine: true, - isValidPath: () => false, }); const customOptionText = customBuffer.text; diff --git a/packages/cli/src/ui/components/ConfigExtensionDialog.tsx b/packages/cli/src/ui/components/ConfigExtensionDialog.tsx index bbecf440f5..b6fb8ce1b6 100644 --- a/packages/cli/src/ui/components/ConfigExtensionDialog.tsx +++ b/packages/cli/src/ui/components/ConfigExtensionDialog.tsx @@ -70,7 +70,7 @@ export const ConfigExtensionDialog: React.FC = ({ initialText: '', viewport: { width: 80, height: 1 }, singleLine: true, - isValidPath: () => true, + escapePastedPaths: true, }); const mounted = useRef(true); diff --git a/packages/cli/src/ui/components/SettingsDialog.tsx b/packages/cli/src/ui/components/SettingsDialog.tsx index 3f606ae22f..a9e2d54aac 100644 --- a/packages/cli/src/ui/components/SettingsDialog.tsx +++ b/packages/cli/src/ui/components/SettingsDialog.tsx @@ -219,7 +219,6 @@ export function SettingsDialog({ width: viewportWidth, height: 1, }, - isValidPath: () => false, singleLine: true, onChange: (text) => setSearchQuery(text), }); diff --git a/packages/cli/src/ui/components/shared/performance.test.ts b/packages/cli/src/ui/components/shared/performance.test.ts index 683995745b..7768d0b9d4 100644 --- a/packages/cli/src/ui/components/shared/performance.test.ts +++ b/packages/cli/src/ui/components/shared/performance.test.ts @@ -19,7 +19,6 @@ describe('text-buffer performance', () => { const { result } = renderHook(() => useTextBuffer({ viewport, - isValidPath: () => false, }), ); @@ -52,7 +51,6 @@ describe('text-buffer performance', () => { useTextBuffer({ initialText, viewport, - isValidPath: () => false, }), ); diff --git a/packages/cli/src/ui/components/shared/text-buffer.test.ts b/packages/cli/src/ui/components/shared/text-buffer.test.ts index 00ecb83c99..50a7fe795b 100644 --- a/packages/cli/src/ui/components/shared/text-buffer.test.ts +++ b/packages/cli/src/ui/components/shared/text-buffer.test.ts @@ -7,10 +7,14 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import stripAnsi from 'strip-ansi'; import { act } from 'react'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as os from 'node:os'; import { renderHook, renderHookWithProviders, } from '../../../test-utils/render.js'; + import type { Viewport, TextBuffer, @@ -738,9 +742,7 @@ describe('useTextBuffer', () => { describe('Initialization', () => { it('should initialize with empty text and cursor at (0,0) by default', () => { - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => false }), - ); + const { result } = renderHook(() => useTextBuffer({ viewport })); const state = getBufferState(result); expect(state.text).toBe(''); expect(state.lines).toEqual(['']); @@ -756,7 +758,6 @@ describe('useTextBuffer', () => { useTextBuffer({ initialText: 'hello', viewport, - isValidPath: () => false, }), ); const state = getBufferState(result); @@ -774,7 +775,6 @@ describe('useTextBuffer', () => { initialText: 'hello\nworld', initialCursorOffset: 7, // Should be at 'o' in 'world' viewport, - isValidPath: () => false, }), ); const state = getBufferState(result); @@ -793,7 +793,6 @@ describe('useTextBuffer', () => { initialText: 'The quick brown fox jumps over the lazy dog.', initialCursorOffset: 2, // After '好' viewport: { width: 15, height: 4 }, - isValidPath: () => false, }), ); const state = getBufferState(result); @@ -810,7 +809,6 @@ describe('useTextBuffer', () => { useTextBuffer({ initialText: 'The quick brown fox jumps over the lazy dog.', viewport: { width: 15, height: 4 }, - isValidPath: () => false, }), ); const state = getBufferState(result); @@ -830,7 +828,6 @@ describe('useTextBuffer', () => { useTextBuffer({ initialText: '123456789012345ABCDEFG', // 4 chars, 12 bytes viewport: { width: 15, height: 2 }, - isValidPath: () => false, }), ); const state = getBufferState(result); @@ -846,7 +843,6 @@ describe('useTextBuffer', () => { initialText: '你好世界', // 4 chars, 12 bytes initialCursorOffset: 2, // After '好' viewport: { width: 5, height: 2 }, - isValidPath: () => false, }), ); const state = getBufferState(result); @@ -861,9 +857,7 @@ describe('useTextBuffer', () => { describe('Basic Editing', () => { it('insert: should insert a character and update cursor', () => { - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => false }), - ); + const { result } = renderHook(() => useTextBuffer({ viewport })); act(() => result.current.insert('a')); let state = getBufferState(result); expect(state.text).toBe('a'); @@ -882,7 +876,6 @@ describe('useTextBuffer', () => { useTextBuffer({ initialText: 'abc', viewport, - isValidPath: () => false, }), ); act(() => result.current.move('right')); @@ -893,9 +886,7 @@ describe('useTextBuffer', () => { }); it('insert: should use placeholder for large text paste', () => { - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => false }), - ); + const { result } = renderHook(() => useTextBuffer({ viewport })); const largeText = '1\n2\n3\n4\n5\n6'; act(() => result.current.insert(largeText, { paste: true })); const state = getBufferState(result); @@ -906,9 +897,7 @@ describe('useTextBuffer', () => { }); it('insert: should NOT use placeholder for large text if NOT a paste', () => { - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => false }), - ); + const { result } = renderHook(() => useTextBuffer({ viewport })); const largeText = '1\n2\n3\n4\n5\n6'; act(() => result.current.insert(largeText, { paste: false })); const state = getBufferState(result); @@ -916,9 +905,7 @@ describe('useTextBuffer', () => { }); it('insert: should clean up pastedContent when placeholder is deleted', () => { - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => false }), - ); + const { result } = renderHook(() => useTextBuffer({ viewport })); const largeText = '1\n2\n3\n4\n5\n6'; act(() => result.current.insert(largeText, { paste: true })); expect(result.current.pastedContent['[Pasted Text: 6 lines]']).toBe( @@ -931,9 +918,7 @@ describe('useTextBuffer', () => { }); it('insert: should clean up pastedContent when placeholder is removed via atomic backspace', () => { - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => false }), - ); + const { result } = renderHook(() => useTextBuffer({ viewport })); const largeText = '1\n2\n3\n4\n5\n6'; act(() => result.current.insert(largeText, { paste: true })); expect(result.current.pastedContent['[Pasted Text: 6 lines]']).toBe( @@ -955,7 +940,6 @@ describe('useTextBuffer', () => { useTextBuffer({ initialText: 'ab', viewport, - isValidPath: () => false, }), ); act(() => result.current.move('end')); // cursor at [0,2] @@ -974,7 +958,6 @@ describe('useTextBuffer', () => { useTextBuffer({ initialText: 'a\nb', viewport, - isValidPath: () => false, }), ); act(() => { @@ -1002,7 +985,6 @@ describe('useTextBuffer', () => { useTextBuffer({ initialText: 'a\nb', viewport, - isValidPath: () => false, }), ); // cursor at [0,0] @@ -1022,36 +1004,49 @@ describe('useTextBuffer', () => { }); describe('Drag and Drop File Paths', () => { + let tempDir: string; + + beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gemini-cli-test-')); + }); + + afterEach(() => { + fs.rmSync(tempDir, { recursive: true, force: true }); + }); + it('should prepend @ to a valid file path on insert', () => { + const filePath = path.join(tempDir, 'file.txt'); + fs.writeFileSync(filePath, ''); + const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => true }), + useTextBuffer({ viewport, escapePastedPaths: true }), ); - const filePath = '/path/to/a/valid/file.txt'; act(() => result.current.insert(filePath, { paste: true })); expect(getBufferState(result).text).toBe(`@${filePath} `); }); it('should not prepend @ to an invalid file path on insert', () => { - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => false }), - ); - const notAPath = 'this is just some long text'; + const { result } = renderHook(() => useTextBuffer({ viewport })); + const notAPath = path.join(tempDir, 'non_existent.txt'); act(() => result.current.insert(notAPath, { paste: true })); expect(getBufferState(result).text).toBe(notAPath); }); it('should handle quoted paths', () => { + const filePath = path.join(tempDir, 'file.txt'); + fs.writeFileSync(filePath, ''); + const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => true }), + useTextBuffer({ viewport, escapePastedPaths: true }), ); - const filePath = "'/path/to/a/valid/file.txt'"; - act(() => result.current.insert(filePath, { paste: true })); - expect(getBufferState(result).text).toBe(`@/path/to/a/valid/file.txt `); + const quotedPath = `'${filePath}'`; + act(() => result.current.insert(quotedPath, { paste: true })); + expect(getBufferState(result).text).toBe(`@${filePath} `); }); it('should not prepend @ to short text that is not a path', () => { const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => true }), + useTextBuffer({ viewport, escapePastedPaths: true }), ); const shortText = 'ab'; act(() => result.current.insert(shortText, { paste: true })); @@ -1059,43 +1054,51 @@ describe('useTextBuffer', () => { }); it('should prepend @ to multiple valid file paths on insert', () => { - // Use Set to model reality: individual paths exist, combined string doesn't - const validPaths = new Set(['/path/to/file1.txt', '/path/to/file2.txt']); + const file1 = path.join(tempDir, 'file1.txt'); + const file2 = path.join(tempDir, 'file2.txt'); + fs.writeFileSync(file1, ''); + fs.writeFileSync(file2, ''); + const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: (p) => validPaths.has(p) }), + useTextBuffer({ viewport, escapePastedPaths: true }), ); - const filePaths = '/path/to/file1.txt /path/to/file2.txt'; + const filePaths = `${file1} ${file2}`; act(() => result.current.insert(filePaths, { paste: true })); - expect(getBufferState(result).text).toBe( - '@/path/to/file1.txt @/path/to/file2.txt ', - ); + expect(getBufferState(result).text).toBe(`@${file1} @${file2} `); }); it('should handle multiple paths with escaped spaces', () => { - // Use Set to model reality: individual paths exist, combined string doesn't - const validPaths = new Set(['/path/to/my file.txt', '/other/path.txt']); + const file1 = path.join(tempDir, 'my file.txt'); + const file2 = path.join(tempDir, 'other.txt'); + fs.writeFileSync(file1, ''); + fs.writeFileSync(file2, ''); + const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: (p) => validPaths.has(p) }), + useTextBuffer({ viewport, escapePastedPaths: true }), ); - const filePaths = '/path/to/my\\ file.txt /other/path.txt'; + // Construct escaped path string: "/path/to/my\ file.txt /path/to/other.txt" + const escapedFile1 = file1.replace(/ /g, '\\ '); + const filePaths = `${escapedFile1} ${file2}`; + act(() => result.current.insert(filePaths, { paste: true })); - expect(getBufferState(result).text).toBe( - '@/path/to/my\\ file.txt @/other/path.txt ', - ); + expect(getBufferState(result).text).toBe(`@${escapedFile1} @${file2} `); }); it('should only prepend @ to valid paths in multi-path paste', () => { + const validFile = path.join(tempDir, 'valid.txt'); + const invalidFile = path.join(tempDir, 'invalid.jpg'); + fs.writeFileSync(validFile, ''); + // Do not create invalidFile + const { result } = renderHook(() => useTextBuffer({ viewport, - isValidPath: (p) => p.endsWith('.txt'), + escapePastedPaths: true, }), ); - const filePaths = '/valid/file.txt /invalid/file.jpg'; + const filePaths = `${validFile} ${invalidFile}`; act(() => result.current.insert(filePaths, { paste: true })); - expect(getBufferState(result).text).toBe( - '@/valid/file.txt /invalid/file.jpg ', - ); + expect(getBufferState(result).text).toBe(`@${validFile} ${invalidFile} `); }); }); @@ -1104,7 +1107,7 @@ describe('useTextBuffer', () => { const { result } = renderHook(() => useTextBuffer({ viewport, - isValidPath: () => true, + escapePastedPaths: true, shellModeActive: true, }), ); @@ -1117,7 +1120,7 @@ describe('useTextBuffer', () => { const { result } = renderHook(() => useTextBuffer({ viewport, - isValidPath: () => true, + escapePastedPaths: true, shellModeActive: true, }), ); @@ -1130,7 +1133,7 @@ describe('useTextBuffer', () => { const { result } = renderHook(() => useTextBuffer({ viewport, - isValidPath: () => false, + shellModeActive: true, }), ); @@ -1143,7 +1146,7 @@ describe('useTextBuffer', () => { const { result } = renderHook(() => useTextBuffer({ viewport, - isValidPath: () => true, + escapePastedPaths: true, shellModeActive: true, }), ); @@ -1165,7 +1168,6 @@ describe('useTextBuffer', () => { useTextBuffer({ initialText: 'long line1next line2', // Corrected: was 'long line1next line2' viewport: { width: 5, height: 4 }, - isValidPath: () => false, }), ); // Initial cursor [0,0] logical, visual [0,0] ("l" of "long ") @@ -1192,7 +1194,6 @@ describe('useTextBuffer', () => { useTextBuffer({ initialText: text, viewport, - isValidPath: () => false, }), ); expect(result.current.allVisualLines).toEqual(['abcde', 'xy', '12345']); @@ -1234,7 +1235,6 @@ describe('useTextBuffer', () => { useTextBuffer({ initialText, viewport: { width: 5, height: 5 }, - isValidPath: () => false, }), ); expect(result.current.allVisualLines).toEqual([ @@ -1263,7 +1263,6 @@ describe('useTextBuffer', () => { useTextBuffer({ initialText: 'This is a very long line of text.', // 33 chars viewport: { width: 10, height: 5 }, - isValidPath: () => false, }), ); const state = getBufferState(result); @@ -1284,7 +1283,6 @@ describe('useTextBuffer', () => { useTextBuffer({ initialText: 'l1\nl2\nl3\nl4\nl5', viewport: { width: 5, height: 3 }, // Can show 3 visual lines - isValidPath: () => false, }), ); // Initial: l1, l2, l3 visible. visualScrollRow = 0. visualCursor = [0,0] @@ -1330,9 +1328,7 @@ describe('useTextBuffer', () => { describe('Undo/Redo', () => { it('should undo and redo an insert operation', () => { - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => false }), - ); + const { result } = renderHook(() => useTextBuffer({ viewport })); act(() => result.current.insert('a')); expect(getBufferState(result).text).toBe('a'); @@ -1350,7 +1346,6 @@ describe('useTextBuffer', () => { useTextBuffer({ initialText: 'test', viewport, - isValidPath: () => false, }), ); act(() => result.current.move('end')); @@ -1369,9 +1364,7 @@ describe('useTextBuffer', () => { describe('Unicode Handling', () => { it('insert: should correctly handle multi-byte unicode characters', () => { - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => false }), - ); + const { result } = renderHook(() => useTextBuffer({ viewport })); act(() => result.current.insert('你好')); const state = getBufferState(result); expect(state.text).toBe('你好'); @@ -1384,7 +1377,6 @@ describe('useTextBuffer', () => { useTextBuffer({ initialText: '你好', viewport, - isValidPath: () => false, }), ); act(() => result.current.move('end')); // cursor at [0,2] @@ -1404,7 +1396,6 @@ describe('useTextBuffer', () => { useTextBuffer({ initialText: '🐶🐱', viewport: { width: 5, height: 1 }, - isValidPath: () => false, }), ); // Initial: visualCursor [0,0] @@ -1432,7 +1423,6 @@ describe('useTextBuffer', () => { const { result } = renderHook(() => useTextBuffer({ viewport: { width: 10, height: 5 }, - isValidPath: () => false, }), ); @@ -1484,7 +1474,6 @@ describe('useTextBuffer', () => { useTextBuffer({ initialText: '你好', // 2 chars, width 4 viewport: { width: 10, height: 1 }, - isValidPath: () => false, }), ); @@ -1510,9 +1499,7 @@ describe('useTextBuffer', () => { describe('handleInput', () => { it('should insert printable characters', () => { - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => false }), - ); + const { result } = renderHook(() => useTextBuffer({ viewport })); act(() => { result.current.handleInput({ name: 'h', @@ -1539,9 +1526,7 @@ describe('useTextBuffer', () => { }); it('should handle "Enter" key as newline', () => { - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => false }), - ); + const { result } = renderHook(() => useTextBuffer({ viewport })); act(() => { result.current.handleInput({ name: 'return', @@ -1557,9 +1542,7 @@ describe('useTextBuffer', () => { }); it('should handle Ctrl+J as newline', () => { - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => false }), - ); + const { result } = renderHook(() => useTextBuffer({ viewport })); act(() => { result.current.handleInput({ name: 'j', @@ -1575,9 +1558,7 @@ describe('useTextBuffer', () => { }); it('should do nothing for a tab key press', () => { - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => false }), - ); + const { result } = renderHook(() => useTextBuffer({ viewport })); act(() => { result.current.handleInput({ name: 'tab', @@ -1593,9 +1574,7 @@ describe('useTextBuffer', () => { }); it('should do nothing for a shift tab key press', () => { - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => false }), - ); + const { result } = renderHook(() => useTextBuffer({ viewport })); act(() => { result.current.handleInput({ name: 'tab', @@ -1615,7 +1594,6 @@ describe('useTextBuffer', () => { useTextBuffer({ initialText: 'hello', viewport, - isValidPath: () => false, }), ); expect(getBufferState(result).text).toBe('hello'); @@ -1636,9 +1614,7 @@ describe('useTextBuffer', () => { }); it('should NOT handle CLEAR_INPUT if buffer is empty', () => { - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => false }), - ); + const { result } = renderHook(() => useTextBuffer({ viewport })); let handled = true; act(() => { handled = result.current.handleInput({ @@ -1659,7 +1635,6 @@ describe('useTextBuffer', () => { useTextBuffer({ initialText: 'a', viewport, - isValidPath: () => false, }), ); act(() => result.current.move('end')); @@ -1682,7 +1657,6 @@ describe('useTextBuffer', () => { useTextBuffer({ initialText: 'abcde', viewport, - isValidPath: () => false, }), ); act(() => result.current.move('end')); // cursor at the end @@ -1726,7 +1700,6 @@ describe('useTextBuffer', () => { useTextBuffer({ initialText: 'abcde', viewport, - isValidPath: () => false, }), ); act(() => result.current.move('end')); // cursor at the end @@ -1744,7 +1717,6 @@ describe('useTextBuffer', () => { useTextBuffer({ initialText: 'abcde', viewport, - isValidPath: () => false, }), ); act(() => result.current.move('end')); // cursor at the end @@ -1762,7 +1734,6 @@ describe('useTextBuffer', () => { useTextBuffer({ initialText: 'ab', viewport, - isValidPath: () => false, }), ); act(() => result.current.move('end')); // cursor [0,2] @@ -1793,9 +1764,7 @@ describe('useTextBuffer', () => { }); it('should strip ANSI escape codes when pasting text', () => { - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => false }), - ); + const { result } = renderHook(() => useTextBuffer({ viewport })); const textWithAnsi = '\x1B[31mHello\x1B[0m \x1B[32mWorld\x1B[0m'; // Simulate pasting by calling handleInput with a string longer than 1 char act(() => { @@ -1813,9 +1782,7 @@ describe('useTextBuffer', () => { }); it('should handle VSCode terminal Shift+Enter as newline', () => { - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => false }), - ); + const { result } = renderHook(() => useTextBuffer({ viewport })); act(() => { result.current.handleInput({ name: 'return', @@ -1839,9 +1806,7 @@ It is a long established fact that a reader will be distracted by the readable c Where does it come from? Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots in a piece of classical Latin literature from 45 BC, making it over 2000 years old. Richard McClintock, a Latin professor at Hampden-Sydney College in Virginia, looked up one of the more obscure Latin words, consectetur, from a Lore `; - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => false }), - ); + const { result } = renderHook(() => useTextBuffer({ viewport })); // Simulate pasting the long text multiple times act(() => { @@ -1887,7 +1852,6 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots useTextBuffer({ initialText: '@pac', viewport, - isValidPath: () => false, }), ); act(() => result.current.replaceRange(0, 1, 0, 4, 'packages')); @@ -1901,7 +1865,6 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots useTextBuffer({ initialText: 'hello\nworld\nagain', viewport, - isValidPath: () => false, }), ); act(() => result.current.replaceRange(0, 2, 1, 3, ' new ')); // replace 'llo\nwor' with ' new ' @@ -1915,7 +1878,6 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots useTextBuffer({ initialText: 'hello world', viewport, - isValidPath: () => false, }), ); act(() => result.current.replaceRange(0, 5, 0, 11, '')); // delete ' world' @@ -1929,7 +1891,6 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots useTextBuffer({ initialText: 'world', viewport, - isValidPath: () => false, }), ); act(() => result.current.replaceRange(0, 0, 0, 0, 'hello ')); @@ -1943,7 +1904,6 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots useTextBuffer({ initialText: 'hello', viewport, - isValidPath: () => false, }), ); act(() => result.current.replaceRange(0, 5, 0, 5, ' world')); @@ -1957,7 +1917,6 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots useTextBuffer({ initialText: 'old text', viewport, - isValidPath: () => false, }), ); act(() => result.current.replaceRange(0, 0, 0, 8, 'new text')); @@ -1971,7 +1930,6 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots useTextBuffer({ initialText: 'hello *** world', viewport, - isValidPath: () => false, }), ); act(() => result.current.replaceRange(0, 6, 0, 9, '你好')); @@ -1985,7 +1943,6 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots useTextBuffer({ initialText: 'test', viewport, - isValidPath: () => false, }), ); act(() => { @@ -2005,7 +1962,6 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots useTextBuffer({ initialText: 'first\nsecond\nthird', viewport, - isValidPath: () => false, }), ); act(() => result.current.replaceRange(0, 2, 2, 3, 'X')); // Replace 'rst\nsecond\nthi' @@ -2019,7 +1975,6 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots useTextBuffer({ initialText: 'one two three', viewport, - isValidPath: () => false, }), ); // Replace "two" with "new\nline" @@ -2063,9 +2018,7 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots desc: 'pasted text with ANSI', }, ])('should strip $desc from input', ({ input, expected }) => { - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => false }), - ); + const { result } = renderHook(() => useTextBuffer({ viewport })); act(() => { result.current.handleInput(createInput(input)); }); @@ -2073,9 +2026,7 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots }); it('should not strip standard characters or newlines', () => { - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => false }), - ); + const { result } = renderHook(() => useTextBuffer({ viewport })); const validText = 'Hello World\nThis is a test.'; act(() => { result.current.handleInput(createInput(validText)); @@ -2084,9 +2035,7 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots }); it('should sanitize large text (>5000 chars) and strip unsafe characters', () => { - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => false }), - ); + const { result } = renderHook(() => useTextBuffer({ viewport })); const unsafeChars = '\x07\x08\x0B\x0C'; const largeTextWithUnsafe = 'safe text'.repeat(600) + unsafeChars + 'more safe text'; @@ -2115,9 +2064,7 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots }); it('should sanitize large ANSI text (>5000 chars) and strip escape codes', () => { - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => false }), - ); + const { result } = renderHook(() => useTextBuffer({ viewport })); const largeTextWithAnsi = '\x1B[31m' + 'red text'.repeat(800) + @@ -2149,9 +2096,7 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots }); it('should not strip popular emojis', () => { - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => false }), - ); + const { result } = renderHook(() => useTextBuffer({ viewport })); const emojis = '🐍🐳🦀🦄'; act(() => { result.current.handleInput({ @@ -2173,7 +2118,7 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots const { result } = renderHook(() => useTextBuffer({ viewport, - isValidPath: () => false, + inputFilter: (text) => text.replace(/[^0-9]/g, ''), }), ); @@ -2186,7 +2131,7 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots const { result } = renderHook(() => useTextBuffer({ viewport, - isValidPath: () => false, + inputFilter: (text) => text.replace(/[^0-9]/g, ''), }), ); @@ -2199,7 +2144,7 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots const { result } = renderHook(() => useTextBuffer({ viewport, - isValidPath: () => false, + inputFilter: (text) => text.toUpperCase(), }), ); @@ -2212,7 +2157,7 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots const { result } = renderHook(() => useTextBuffer({ viewport, - isValidPath: () => false, + inputFilter: (text) => text, // Allow everything including newlines }), ); @@ -2227,7 +2172,7 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots const { result } = renderHook(() => useTextBuffer({ viewport, - isValidPath: () => false, + inputFilter: (text) => text.replace(/\n/g, ''), // Filter out newlines }), ); @@ -2260,11 +2205,8 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots describe('Memoization', () => { it('should keep action references stable across re-renders', () => { - // We pass a stable `isValidPath` so that callbacks that depend on it - // are not recreated on every render. - const isValidPath = () => false; const { result, rerender } = renderHook(() => - useTextBuffer({ viewport, isValidPath }), + useTextBuffer({ viewport }), ); const initialInsert = result.current.insert; @@ -2281,10 +2223,7 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots }); it('should have memoized actions that operate on the latest state', () => { - const isValidPath = () => false; - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath }), - ); + const { result } = renderHook(() => useTextBuffer({ viewport })); // Store a reference to the memoized insert function. const memoizedInsert = result.current.insert; @@ -2310,7 +2249,7 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots const { result } = renderHook(() => useTextBuffer({ viewport, - isValidPath: () => false, + singleLine: true, }), ); @@ -2325,7 +2264,7 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots useTextBuffer({ initialText: 'ab', viewport, - isValidPath: () => false, + singleLine: true, }), ); @@ -2341,7 +2280,7 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots const { result } = renderHook(() => useTextBuffer({ viewport, - isValidPath: () => false, + singleLine: true, }), ); @@ -2363,7 +2302,7 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots const { result } = renderHook(() => useTextBuffer({ viewport, - isValidPath: () => false, + singleLine: true, }), ); @@ -2385,7 +2324,7 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots const { result } = renderHook(() => useTextBuffer({ viewport, - isValidPath: () => false, + singleLine: true, }), ); @@ -2841,7 +2780,6 @@ describe('Unicode helper functions', () => { initialText: '你好世界', initialCursorOffset: 4, // End of string viewport, - isValidPath: () => false, }), ); @@ -2900,7 +2838,6 @@ describe('Unicode helper functions', () => { initialText: 'Hello你好World', initialCursorOffset: 10, // End viewport, - isValidPath: () => false, }), ); @@ -3154,7 +3091,7 @@ describe('Transformation Utilities', () => { useTextBuffer({ initialText: 'original line', viewport, - isValidPath: () => true, + escapePastedPaths: true, }), ); @@ -3177,7 +3114,7 @@ describe('Transformation Utilities', () => { initialText: 'a very long line that will wrap when the viewport is small', viewport: vp, - isValidPath: () => true, + escapePastedPaths: true, }), { initialProps: { vp: viewport } }, ); @@ -3198,7 +3135,7 @@ describe('Transformation Utilities', () => { useTextBuffer({ initialText: text, viewport, - isValidPath: () => true, + escapePastedPaths: true, }), ); @@ -3231,7 +3168,7 @@ describe('Transformation Utilities', () => { useTextBuffer({ initialText, viewport, - isValidPath: () => true, + escapePastedPaths: true, }), ); @@ -3265,7 +3202,6 @@ describe('Transformation Utilities', () => { useTextBuffer({ initialText: placeholder, viewport: scrollViewport, - isValidPath: () => false, }), ); diff --git a/packages/cli/src/ui/components/shared/text-buffer.ts b/packages/cli/src/ui/components/shared/text-buffer.ts index 9366aa0201..83637f4f08 100644 --- a/packages/cli/src/ui/components/shared/text-buffer.ts +++ b/packages/cli/src/ui/components/shared/text-buffer.ts @@ -757,7 +757,7 @@ interface UseTextBufferProps { stdin?: NodeJS.ReadStream | null; // For external editor setRawMode?: (mode: boolean) => void; // For external editor onChange?: (text: string) => void; // Callback for when text changes - isValidPath: (path: string) => boolean; + escapePastedPaths?: boolean; shellModeActive?: boolean; // Whether the text buffer is in shell mode inputFilter?: (text: string) => string; // Optional filter for input text singleLine?: boolean; @@ -2678,7 +2678,7 @@ export function useTextBuffer({ stdin, setRawMode, onChange, - isValidPath, + escapePastedPaths = false, shellModeActive = false, inputFilter, singleLine = false, @@ -2795,7 +2795,8 @@ export function useTextBuffer({ if ( ch.length >= minLengthToInferAsDragDrop && !shellModeActive && - paste + paste && + escapePastedPaths ) { let potentialPath = ch.trim(); const quoteMatch = potentialPath.match(/^'(.*)'$/); @@ -2805,7 +2806,7 @@ export function useTextBuffer({ potentialPath = potentialPath.trim(); - const processed = parsePastedPaths(potentialPath, isValidPath); + const processed = parsePastedPaths(potentialPath); if (processed) { textToInsert = processed; } @@ -2827,7 +2828,7 @@ export function useTextBuffer({ dispatch({ type: 'insert', payload: currentText, isPaste: paste }); } }, - [isValidPath, shellModeActive], + [shellModeActive, escapePastedPaths], ); const newline = useCallback((): void => { diff --git a/packages/cli/src/ui/components/triage/TriageIssues.tsx b/packages/cli/src/ui/components/triage/TriageIssues.tsx index dadc173da5..c1e21e274a 100644 --- a/packages/cli/src/ui/components/triage/TriageIssues.tsx +++ b/packages/cli/src/ui/components/triage/TriageIssues.tsx @@ -99,7 +99,6 @@ export const TriageIssues = ({ const commentBuffer = useTextBuffer({ initialText: '', viewport: { width: 80, height: 5 }, - isValidPath: () => false, }); const currentIssue = state.issues[state.currentIndex]; diff --git a/packages/cli/src/ui/hooks/useCommandCompletion.test.tsx b/packages/cli/src/ui/hooks/useCommandCompletion.test.tsx index 204d9d108f..47f7e63a4e 100644 --- a/packages/cli/src/ui/hooks/useCommandCompletion.test.tsx +++ b/packages/cli/src/ui/hooks/useCommandCompletion.test.tsx @@ -105,7 +105,6 @@ describe('useCommandCompletion', () => { initialText: text, initialCursorOffset: cursorOffset ?? text.length, viewport: { width: 80, height: 20 }, - isValidPath: () => false, onChange: () => {}, }); } diff --git a/packages/cli/src/ui/hooks/useReverseSearchCompletion.test.tsx b/packages/cli/src/ui/hooks/useReverseSearchCompletion.test.tsx index 741e2b04e7..f493be54b7 100644 --- a/packages/cli/src/ui/hooks/useReverseSearchCompletion.test.tsx +++ b/packages/cli/src/ui/hooks/useReverseSearchCompletion.test.tsx @@ -24,7 +24,6 @@ describe('useReverseSearchCompletion', () => { initialText: text, initialCursorOffset: text.length, viewport: { width: 80, height: 20 }, - isValidPath: () => false, onChange: () => {}, }); } diff --git a/packages/cli/src/ui/utils/clipboardUtils.test.ts b/packages/cli/src/ui/utils/clipboardUtils.test.ts index 32cfa24883..5b2df637c3 100644 --- a/packages/cli/src/ui/utils/clipboardUtils.test.ts +++ b/packages/cli/src/ui/utils/clipboardUtils.test.ts @@ -14,8 +14,14 @@ import { type Mock, } from 'vitest'; import * as fs from 'node:fs/promises'; -import { createWriteStream } from 'node:fs'; -import { spawn, execSync } from 'node:child_process'; +import { + createWriteStream, + existsSync, + statSync, + type Stats, + type WriteStream, +} from 'node:fs'; +import { spawn, execSync, type ChildProcess } from 'node:child_process'; import EventEmitter from 'node:events'; import { Stream } from 'node:stream'; import * as path from 'node:path'; @@ -24,6 +30,8 @@ import * as path from 'node:path'; vi.mock('node:fs/promises'); vi.mock('node:fs', () => ({ createWriteStream: vi.fn(), + existsSync: vi.fn(), + statSync: vi.fn(), })); vi.mock('node:child_process', async (importOriginal) => { const actual = await importOriginal(); @@ -67,6 +75,12 @@ describe('clipboardUtils', () => { // Dynamic module instance for stateful functions let clipboardUtils: ClipboardUtilsModule; + const MOCK_FILE_STATS = { + isFile: () => true, + size: 100, + mtimeMs: Date.now(), + } as unknown as Stats; + beforeEach(async () => { vi.resetAllMocks(); originalPlatform = process.platform; @@ -97,9 +111,10 @@ describe('clipboardUtils', () => { it('should return true when wl-paste shows image type (Wayland)', async () => { setPlatform('linux'); process.env['XDG_SESSION_TYPE'] = 'wayland'; - (execSync as Mock).mockReturnValue(Buffer.from('')); // command -v succeeds - (spawnAsync as Mock).mockResolvedValueOnce({ + vi.mocked(execSync).mockReturnValue(Buffer.from('')); // command -v succeeds + vi.mocked(spawnAsync).mockResolvedValueOnce({ stdout: 'image/png\ntext/plain', + stderr: '', }); const result = await clipboardUtils.clipboardHasImage(); @@ -115,9 +130,10 @@ describe('clipboardUtils', () => { it('should return true when xclip shows image type (X11)', async () => { setPlatform('linux'); process.env['XDG_SESSION_TYPE'] = 'x11'; - (execSync as Mock).mockReturnValue(Buffer.from('')); // command -v succeeds - (spawnAsync as Mock).mockResolvedValueOnce({ + vi.mocked(execSync).mockReturnValue(Buffer.from('')); // command -v succeeds + vi.mocked(spawnAsync).mockResolvedValueOnce({ stdout: 'image/png\nTARGETS', + stderr: '', }); const result = await clipboardUtils.clipboardHasImage(); @@ -139,8 +155,8 @@ describe('clipboardUtils', () => { it('should return false if tool fails', async () => { setPlatform('linux'); process.env['XDG_SESSION_TYPE'] = 'wayland'; - (execSync as Mock).mockReturnValue(Buffer.from('')); - (spawnAsync as Mock).mockRejectedValueOnce(new Error('wl-paste failed')); + vi.mocked(execSync).mockReturnValue(Buffer.from('')); + vi.mocked(spawnAsync).mockRejectedValueOnce(new Error('wl-paste failed')); const result = await clipboardUtils.clipboardHasImage(); @@ -150,8 +166,11 @@ describe('clipboardUtils', () => { it('should return false if no image type is found', async () => { setPlatform('linux'); process.env['XDG_SESSION_TYPE'] = 'wayland'; - (execSync as Mock).mockReturnValue(Buffer.from('')); - (spawnAsync as Mock).mockResolvedValueOnce({ stdout: 'text/plain' }); + vi.mocked(execSync).mockReturnValue(Buffer.from('')); + vi.mocked(spawnAsync).mockResolvedValueOnce({ + stdout: 'text/plain', + stderr: '', + }); const result = await clipboardUtils.clipboardHasImage(); @@ -161,7 +180,7 @@ describe('clipboardUtils', () => { it('should return false if tool not found', async () => { setPlatform('linux'); process.env['XDG_SESSION_TYPE'] = 'wayland'; - (execSync as Mock).mockImplementation(() => { + vi.mocked(execSync).mockImplementation(() => { throw new Error('Command not found'); }); @@ -177,8 +196,8 @@ describe('clipboardUtils', () => { beforeEach(() => { setPlatform('linux'); - (fs.mkdir as Mock).mockResolvedValue(undefined); - (fs.unlink as Mock).mockResolvedValue(undefined); + vi.mocked(fs.mkdir).mockResolvedValue(undefined); + vi.mocked(fs.unlink).mockResolvedValue(undefined); }); const createMockChildProcess = ( @@ -209,31 +228,36 @@ describe('clipboardUtils', () => { hasImage = true, ) => { process.env['XDG_SESSION_TYPE'] = type; - (execSync as Mock).mockReturnValue(Buffer.from('')); - (spawnAsync as Mock).mockResolvedValueOnce({ + vi.mocked(execSync).mockReturnValue(Buffer.from('')); + vi.mocked(spawnAsync).mockResolvedValueOnce({ stdout: hasImage ? 'image/png' : 'text/plain', + stderr: '', }); await clipboardUtils.clipboardHasImage(); - (spawnAsync as Mock).mockClear(); - (execSync as Mock).mockClear(); + vi.mocked(spawnAsync).mockClear(); + vi.mocked(execSync).mockClear(); }; it('should save image using wl-paste if detected', async () => { await primeClipboardTool('wayland'); // Mock fs.stat to return size > 0 - (fs.stat as Mock).mockResolvedValue({ size: 100, mtimeMs: Date.now() }); + vi.mocked(fs.stat).mockResolvedValue(MOCK_FILE_STATS); // Mock spawn to return a successful process for wl-paste const mockChild = createMockChildProcess(true, 0); - (spawn as Mock).mockReturnValueOnce(mockChild); + vi.mocked(spawn).mockReturnValueOnce( + mockChild as unknown as ChildProcess, + ); // Mock createWriteStream const mockStream = new EventEmitter() as EventEmitter & { writableFinished: boolean; }; mockStream.writableFinished = false; - (createWriteStream as Mock).mockReturnValue(mockStream); + vi.mocked(createWriteStream).mockReturnValue( + mockStream as unknown as WriteStream, + ); // Use dynamic instance const promise = clipboardUtils.saveClipboardImage(mockTargetDir); @@ -254,16 +278,18 @@ describe('clipboardUtils', () => { await primeClipboardTool('wayland'); // Mock fs.stat to return size > 0 - (fs.stat as Mock).mockResolvedValue({ size: 100, mtimeMs: Date.now() }); + vi.mocked(fs.stat).mockResolvedValue(MOCK_FILE_STATS); // wl-paste fails (non-zero exit code) const child1 = createMockChildProcess(true, 1); - (spawn as Mock).mockReturnValueOnce(child1); + vi.mocked(spawn).mockReturnValueOnce(child1 as unknown as ChildProcess); const mockStream1 = new EventEmitter() as EventEmitter & { writableFinished: boolean; }; - (createWriteStream as Mock).mockReturnValueOnce(mockStream1); + vi.mocked(createWriteStream).mockReturnValueOnce( + mockStream1 as unknown as WriteStream, + ); const promise = clipboardUtils.saveClipboardImage(mockTargetDir); @@ -281,18 +307,22 @@ describe('clipboardUtils', () => { await primeClipboardTool('x11'); // Mock fs.stat to return size > 0 - (fs.stat as Mock).mockResolvedValue({ size: 100, mtimeMs: Date.now() }); + vi.mocked(fs.stat).mockResolvedValue(MOCK_FILE_STATS); // Mock spawn to return a successful process for xclip const mockChild = createMockChildProcess(true, 0); - (spawn as Mock).mockReturnValueOnce(mockChild); + vi.mocked(spawn).mockReturnValueOnce( + mockChild as unknown as ChildProcess, + ); // Mock createWriteStream const mockStream = new EventEmitter() as EventEmitter & { writableFinished: boolean; }; mockStream.writableFinished = false; - (createWriteStream as Mock).mockReturnValue(mockStream); + vi.mocked(createWriteStream).mockReturnValue( + mockStream as unknown as WriteStream, + ); const promise = clipboardUtils.saveClipboardImage(mockTargetDir); @@ -397,64 +427,71 @@ describe('clipboardUtils', () => { describe('parsePastedPaths', () => { it('should return null for empty string', () => { - const result = parsePastedPaths('', () => true); + const result = parsePastedPaths(''); expect(result).toBe(null); }); it('should add @ prefix to single valid path', () => { - const result = parsePastedPaths('/path/to/file.txt', () => true); + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(statSync).mockReturnValue(MOCK_FILE_STATS); + const result = parsePastedPaths('/path/to/file.txt'); expect(result).toBe('@/path/to/file.txt '); }); it('should return null for single invalid path', () => { - const result = parsePastedPaths('/path/to/file.txt', () => false); + vi.mocked(existsSync).mockReturnValue(false); + const result = parsePastedPaths('/path/to/file.txt'); expect(result).toBe(null); }); it('should add @ prefix to all valid paths', () => { - // Use Set to model reality: individual paths exist, combined string doesn't const validPaths = new Set(['/path/to/file1.txt', '/path/to/file2.txt']); - const result = parsePastedPaths( - '/path/to/file1.txt /path/to/file2.txt', - (p) => validPaths.has(p), + vi.mocked(existsSync).mockImplementation((p) => + validPaths.has(p as string), ); + vi.mocked(statSync).mockReturnValue(MOCK_FILE_STATS); + + const result = parsePastedPaths('/path/to/file1.txt /path/to/file2.txt'); expect(result).toBe('@/path/to/file1.txt @/path/to/file2.txt '); }); it('should only add @ prefix to valid paths', () => { - const result = parsePastedPaths( - '/valid/file.txt /invalid/file.jpg', - (p) => p.endsWith('.txt'), + vi.mocked(existsSync).mockImplementation((p) => + (p as string).endsWith('.txt'), ); + vi.mocked(statSync).mockReturnValue(MOCK_FILE_STATS); + + const result = parsePastedPaths('/valid/file.txt /invalid/file.jpg'); expect(result).toBe('@/valid/file.txt /invalid/file.jpg '); }); it('should return null if no paths are valid', () => { - const result = parsePastedPaths( - '/path/to/file1.txt /path/to/file2.txt', - () => false, - ); + vi.mocked(existsSync).mockReturnValue(false); + const result = parsePastedPaths('/path/to/file1.txt /path/to/file2.txt'); expect(result).toBe(null); }); it('should handle paths with escaped spaces', () => { - // Use Set to model reality: individual paths exist, combined string doesn't const validPaths = new Set(['/path/to/my file.txt', '/other/path.txt']); - const result = parsePastedPaths( - '/path/to/my\\ file.txt /other/path.txt', - (p) => validPaths.has(p), + vi.mocked(existsSync).mockImplementation((p) => + validPaths.has(p as string), ); + vi.mocked(statSync).mockReturnValue(MOCK_FILE_STATS); + + const result = parsePastedPaths('/path/to/my\\ file.txt /other/path.txt'); expect(result).toBe('@/path/to/my\\ file.txt @/other/path.txt '); }); it('should unescape paths before validation', () => { - // Use Set to model reality: individual paths exist, combined string doesn't const validPaths = new Set(['/my file.txt', '/other.txt']); const validatedPaths: string[] = []; - parsePastedPaths('/my\\ file.txt /other.txt', (p) => { - validatedPaths.push(p); - return validPaths.has(p); + vi.mocked(existsSync).mockImplementation((p) => { + validatedPaths.push(p as string); + return validPaths.has(p as string); }); + vi.mocked(statSync).mockReturnValue(MOCK_FILE_STATS); + + parsePastedPaths('/my\\ file.txt /other.txt'); // First checks entire string, then individual unescaped segments expect(validatedPaths).toEqual([ '/my\\ file.txt /other.txt', @@ -464,33 +501,45 @@ describe('clipboardUtils', () => { }); it('should handle single path with unescaped spaces from copy-paste', () => { - const result = parsePastedPaths('/path/to/my file.txt', () => true); + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(statSync).mockReturnValue(MOCK_FILE_STATS); + + const result = parsePastedPaths('/path/to/my file.txt'); expect(result).toBe('@/path/to/my\\ file.txt '); }); it('should handle Windows path', () => { - const result = parsePastedPaths('C:\\Users\\file.txt', () => true); + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(statSync).mockReturnValue(MOCK_FILE_STATS); + + const result = parsePastedPaths('C:\\Users\\file.txt'); expect(result).toBe('@C:\\Users\\file.txt '); }); it('should handle Windows path with unescaped spaces', () => { - const result = parsePastedPaths('C:\\My Documents\\file.txt', () => true); + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(statSync).mockReturnValue(MOCK_FILE_STATS); + + const result = parsePastedPaths('C:\\My Documents\\file.txt'); expect(result).toBe('@C:\\My\\ Documents\\file.txt '); }); it('should handle multiple Windows paths', () => { const validPaths = new Set(['C:\\file1.txt', 'D:\\file2.txt']); - const result = parsePastedPaths('C:\\file1.txt D:\\file2.txt', (p) => - validPaths.has(p), + vi.mocked(existsSync).mockImplementation((p) => + validPaths.has(p as string), ); + vi.mocked(statSync).mockReturnValue(MOCK_FILE_STATS); + + const result = parsePastedPaths('C:\\file1.txt D:\\file2.txt'); expect(result).toBe('@C:\\file1.txt @D:\\file2.txt '); }); it('should handle Windows UNC path', () => { - const result = parsePastedPaths( - '\\\\server\\share\\file.txt', - () => true, - ); + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(statSync).mockReturnValue(MOCK_FILE_STATS); + + const result = parsePastedPaths('\\\\server\\share\\file.txt'); expect(result).toBe('@\\\\server\\share\\file.txt '); }); }); diff --git a/packages/cli/src/ui/utils/clipboardUtils.ts b/packages/cli/src/ui/utils/clipboardUtils.ts index a65442c110..a6a7b485cd 100644 --- a/packages/cli/src/ui/utils/clipboardUtils.ts +++ b/packages/cli/src/ui/utils/clipboardUtils.ts @@ -5,7 +5,7 @@ */ import * as fs from 'node:fs/promises'; -import { createWriteStream } from 'node:fs'; +import { createWriteStream, existsSync, statSync } from 'node:fs'; import { execSync, spawn } from 'node:child_process'; import * as path from 'node:path'; import { @@ -462,20 +462,27 @@ export function splitEscapedPaths(text: string): string[] { return paths; } +/** + * Helper to validate if a path exists and is a file. + */ +function isValidFilePath(p: string): boolean { + try { + return existsSync(p) && statSync(p).isFile(); + } catch { + return false; + } +} + /** * Processes pasted text containing file paths, adding @ prefix to valid paths. * Handles both single and multiple space-separated paths. * * @param text The pasted text (potentially space-separated paths) - * @param isValidPath Function to validate if a path exists/is valid * @returns Processed string with @ prefixes on valid paths, or null if no valid paths */ -export function parsePastedPaths( - text: string, - isValidPath: (path: string) => boolean, -): string | null { +export function parsePastedPaths(text: string): string | null { // First, check if the entire text is a single valid path - if (PATH_PREFIX_PATTERN.test(text) && isValidPath(text)) { + if (PATH_PREFIX_PATTERN.test(text) && isValidFilePath(text)) { return `@${escapePath(text)} `; } @@ -492,7 +499,7 @@ export function parsePastedPaths( return segment; } const unescaped = unescapePath(segment); - if (isValidPath(unescaped)) { + if (isValidFilePath(unescaped)) { anyValidPath = true; return `@${segment}`; } diff --git a/packages/core/src/utils/paths.test.ts b/packages/core/src/utils/paths.test.ts index 6759b7978c..64e4e94ddc 100644 --- a/packages/core/src/utils/paths.test.ts +++ b/packages/core/src/utils/paths.test.ts @@ -42,7 +42,11 @@ describe('escapePath', () => { ['double quotes', 'file"name.txt', 'file\\"name.txt'], ['hash symbols', 'file#name.txt', 'file\\#name.txt'], ['exclamation marks', 'file!name.txt', 'file\\!name.txt'], - ['tildes', 'file~name.txt', 'file\\~name.txt'], + [ + 'tildes', + 'file~name.txt', + process.platform === 'win32' ? 'file~name.txt' : 'file\\~name.txt', + ], [ 'less than and greater than signs', 'file.txt', @@ -99,11 +103,16 @@ describe('escapePath', () => { expect(escapePath('')).toBe(''); }); - it('should handle paths with only special characters', () => { - expect(escapePath(' ()[]{};&|*?$`\'"#!~<>')).toBe( - '\\ \\(\\)\\[\\]\\{\\}\\;\\&\\|\\*\\?\\$\\`\\\'\\"\\#\\!\\~\\<\\>', + it('should handle paths with multiple special characters', () => { + expect(escapePath(' ()[]{};&|*?$`\'"#!<>')).toBe( + '\\ \\(\\)\\[\\]\\{\\}\\;\\&\\|\\*\\?\\$\\`\\\'\\"\\#\\!\\<\\>', ); }); + + it('should handle tildes based on platform', () => { + const expected = process.platform === 'win32' ? '~' : '\\~'; + expect(escapePath('~')).toBe(expected); + }); }); describe('unescapePath', () => { @@ -130,12 +139,12 @@ describe('unescapePath', () => { ); }); - it('should handle all special characters', () => { + it('should handle all special characters but tilda', () => { expect( unescapePath( - '\\ \\(\\)\\[\\]\\{\\}\\;\\&\\|\\*\\?\\$\\`\\\'\\"\\#\\!\\~\\<\\>', + '\\ \\(\\)\\[\\]\\{\\}\\;\\&\\|\\*\\?\\$\\`\\\'\\"\\#\\!\\<\\>', ), - ).toBe(' ()[]{};&|*?$`\'"#!~<>'); + ).toBe(' ()[]{};&|*?$`\'"#!<>'); }); it('should be the inverse of escapePath', () => { diff --git a/packages/core/src/utils/paths.ts b/packages/core/src/utils/paths.ts index 94ccd96cf3..c48cb7c2a9 100644 --- a/packages/core/src/utils/paths.ts +++ b/packages/core/src/utils/paths.ts @@ -16,10 +16,12 @@ export const GOOGLE_ACCOUNTS_FILENAME = 'google_accounts.json'; /** * 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. + * Note that windows doesn't escape tilda. */ -export const SHELL_SPECIAL_CHARS = /[ \t()[\]{};|*?$`'"#&<>!~]/; +export const SHELL_SPECIAL_CHARS = + process.platform === 'win32' + ? /[ \t()[\]{};|*?$`'"#&<>!]/ + : /[ \t()[\]{};|*?$`'"#&<>!~]/; /** * Returns the home directory.