refactor: push isValidPath() into parsePastedPaths() (#18664)

This commit is contained in:
Tommaso Sciortino
2026-02-09 13:19:51 -08:00
committed by GitHub
parent 9e41b2cd89
commit 1b98c1f806
15 changed files with 247 additions and 261 deletions
+106 -57
View File
@@ -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<typeof import('node:child_process')>();
@@ -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 ');
});
});
+15 -8
View File
@@ -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}`;
}