diff --git a/src/main/ipc/filesystem-list-files-git-fallback-real.test.ts b/src/main/ipc/filesystem-list-files-git-fallback-real.test.ts new file mode 100644 index 00000000000..327614faf0a --- /dev/null +++ b/src/main/ipc/filesystem-list-files-git-fallback-real.test.ts @@ -0,0 +1,61 @@ +import { execFile as execFileCallback } from 'node:child_process' +import { mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { promisify } from 'node:util' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { Store } from '../persistence' + +const { checkRgAvailableMock } = vi.hoisted(() => ({ + checkRgAvailableMock: vi.fn() +})) + +vi.mock('./rg-availability', () => ({ + checkRgAvailable: checkRgAvailableMock +})) + +import { listQuickOpenFiles } from './filesystem-list-files' + +const execFile = promisify(execFileCallback) + +function makeStore(repoPath: string): Store { + return { + getRepos: () => [ + { + id: 'repo-1', + path: repoPath, + displayName: 'repo', + badgeColor: '#000000', + addedAt: 0, + kind: 'git' + } + ], + getSettings: () => ({}) + } as unknown as Store +} + +describe('filesystem-list-files real git fallback', () => { + let tempDir: string | null = null + + afterEach(async () => { + if (tempDir) { + await rm(tempDir, { recursive: true, force: true }) + tempDir = null + } + vi.clearAllMocks() + }) + + it('returns real paths for filenames Git would C-quote in newline output', async () => { + checkRgAvailableMock.mockResolvedValue(false) + tempDir = await mkdtemp(join(tmpdir(), 'orca-quick-open-git-fallback-')) + const repoPath = join(tempDir, 'repo') + await execFile('git', ['init', '-q', repoPath]) + const tabbedPath = join(repoPath, 'tab\tfile.txt') + await writeFile(tabbedPath, 'content') + await execFile('git', ['add', '.'], { cwd: repoPath }) + + await expect(listQuickOpenFiles(repoPath, makeStore(repoPath))).resolves.toEqual([ + 'tab\tfile.txt' + ]) + }) +}) diff --git a/src/main/ipc/filesystem-list-files.test.ts b/src/main/ipc/filesystem-list-files.test.ts index b57a07faf08..a09739b143d 100644 --- a/src/main/ipc/filesystem-list-files.test.ts +++ b/src/main/ipc/filesystem-list-files.test.ts @@ -256,13 +256,13 @@ describe('filesystem-list-files', () => { const promise = listQuickOpenFiles('/mock/root', storeMock) setTimeout(() => { - ;(gitP1.stdout as unknown as EventEmitter).emit('data', 'src/index.ts\n') - ;(gitP1.stdout as unknown as EventEmitter).emit('data', 'package.json\n') - ;(gitP1.stdout as unknown as EventEmitter).emit('data', 'node_modules/dep/index.js\n') + ;(gitP1.stdout as unknown as EventEmitter).emit('data', 'src/index.ts\0') + ;(gitP1.stdout as unknown as EventEmitter).emit('data', 'package.json\0') + ;(gitP1.stdout as unknown as EventEmitter).emit('data', 'node_modules/dep/index.js\0') gitP1.emit('close', 0, null) - ;(gitP2.stdout as unknown as EventEmitter).emit('data', '.env.local\n') - ;(gitP2.stdout as unknown as EventEmitter).emit('data', 'dist/generated.js\n') + ;(gitP2.stdout as unknown as EventEmitter).emit('data', '.env.local\0') + ;(gitP2.stdout as unknown as EventEmitter).emit('data', 'dist/generated.js\0') gitP2.emit('close', 0, null) }, 10) @@ -304,10 +304,10 @@ describe('filesystem-list-files', () => { const promise = listQuickOpenFiles('/mock/root', storeMock) setTimeout(() => { - ;(gitP1.stdout as unknown as EventEmitter).emit('data', '.next/cache/1.js\n') - ;(gitP1.stdout as unknown as EventEmitter).emit('data', '.vscode/settings.json\n') - ;(gitP1.stdout as unknown as EventEmitter).emit('data', '.github/workflows/ci.yml\n') - ;(gitP1.stdout as unknown as EventEmitter).emit('data', 'valid.ts\n') + ;(gitP1.stdout as unknown as EventEmitter).emit('data', '.next/cache/1.js\0') + ;(gitP1.stdout as unknown as EventEmitter).emit('data', '.vscode/settings.json\0') + ;(gitP1.stdout as unknown as EventEmitter).emit('data', '.github/workflows/ci.yml\0') + ;(gitP1.stdout as unknown as EventEmitter).emit('data', 'valid.ts\0') gitP1.emit('close', 0, null) gitP2.emit('close', 0, null) @@ -342,7 +342,7 @@ describe('filesystem-list-files', () => { await Promise.resolve() await Promise.resolve() - ;(gitP1.stdout as unknown as EventEmitter).emit('data', 'src/index.ts\npartial') + ;(gitP1.stdout as unknown as EventEmitter).emit('data', 'src/index.ts\0partial') await vi.advanceTimersByTimeAsync(10000) diff --git a/src/main/ipc/filesystem-list-files.ts b/src/main/ipc/filesystem-list-files.ts index c08aa4817f6..90ff30895dc 100644 --- a/src/main/ipc/filesystem-list-files.ts +++ b/src/main/ipc/filesystem-list-files.ts @@ -198,21 +198,18 @@ function listFilesWithGit( let buf = '' let done = false - const processLine = (line: string): void => { - if (line.charCodeAt(line.length - 1) === 13 /* \r */) { - line = line.substring(0, line.length - 1) - } - if (!line) { + const processPath = (path: string): void => { + if (!path) { return } // Why: git exclude pathspecs prune most hits, but post-filter is // still required because pathspec semantics differ subtly from the // rg globs and exist as a correctness backstop. - if (shouldExcludeQuickOpenRelPath(line, excludePathPrefixes)) { + if (shouldExcludeQuickOpenRelPath(path, excludePathPrefixes)) { return } - if (shouldIncludeQuickOpenPath(line)) { - files.add(line) + if (shouldIncludeQuickOpenPath(path)) { + files.add(path) } } @@ -226,11 +223,11 @@ function listFilesWithGit( const handleStdoutData = (chunk: string): void => { buf += chunk let start = 0 - let newlineIdx = buf.indexOf('\n', start) - while (newlineIdx !== -1) { - processLine(buf.substring(start, newlineIdx)) - start = newlineIdx + 1 - newlineIdx = buf.indexOf('\n', start) + let nulIdx = buf.indexOf('\0', start) + while (nulIdx !== -1) { + processPath(buf.substring(start, nulIdx)) + start = nulIdx + 1 + nulIdx = buf.indexOf('\0', start) } buf = start < buf.length ? buf.substring(start) : '' } @@ -243,7 +240,7 @@ function listFilesWithGit( } const handleClose = (): void => { if (buf) { - processLine(buf) + processPath(buf) } finish() } diff --git a/src/relay/fs-handler-git-fallback.ts b/src/relay/fs-handler-git-fallback.ts index 246199a8dfd..d4c46f35e9c 100644 --- a/src/relay/fs-handler-git-fallback.ts +++ b/src/relay/fs-handler-git-fallback.ts @@ -49,18 +49,15 @@ export function listFilesWithGit( let buf = '' let done = false - const processLine = (line: string): void => { - if (line.charCodeAt(line.length - 1) === 13) { - line = line.substring(0, line.length - 1) - } - if (!line) { + const processPath = (path: string): void => { + if (!path) { return } - if (shouldExcludeQuickOpenRelPath(line, excludePathPrefixes)) { + if (shouldExcludeQuickOpenRelPath(path, excludePathPrefixes)) { return } - if (shouldIncludeQuickOpenPath(line)) { - files.add(line) + if (shouldIncludeQuickOpenPath(path)) { + files.add(path) } } @@ -106,11 +103,11 @@ export function listFilesWithGit( function handleStdoutData(chunk: string): void { buf += chunk let start = 0 - let idx = buf.indexOf('\n', start) + let idx = buf.indexOf('\0', start) while (idx !== -1) { - processLine(buf.substring(start, idx)) + processPath(buf.substring(start, idx)) start = idx + 1 - idx = buf.indexOf('\n', start) + idx = buf.indexOf('\0', start) } buf = start < buf.length ? buf.substring(start) : '' } @@ -132,7 +129,7 @@ export function listFilesWithGit( return } if (buf) { - processLine(buf) + processPath(buf) } resolvePass() } diff --git a/src/relay/fs-handler-list-files-ignored.test.ts b/src/relay/fs-handler-list-files-ignored.test.ts index 0a859f8c928..6329c78fcdc 100644 --- a/src/relay/fs-handler-list-files-ignored.test.ts +++ b/src/relay/fs-handler-list-files-ignored.test.ts @@ -84,19 +84,21 @@ describe('relay quick open ignored file listing', () => { const promise = listFilesWithGit('/remote/root', ['packages/other']) setTimeout(() => { - ;(primaryProc.stdout as unknown as EventEmitter).emit('data', 'src/index.ts\n') + ;(primaryProc.stdout as unknown as EventEmitter).emit('data', 'src/index.ts\0') + ;(primaryProc.stdout as unknown as EventEmitter).emit('data', 'tab\tfile.txt\0') primaryProc.emit('close', 0, null) - ;(ignoredProc.stdout as unknown as EventEmitter).emit('data', 'dist/generated.js\n') - ;(ignoredProc.stdout as unknown as EventEmitter).emit('data', 'packages/other/src/x.ts\n') + ;(ignoredProc.stdout as unknown as EventEmitter).emit('data', 'dist/generated.js\0') + ;(ignoredProc.stdout as unknown as EventEmitter).emit('data', 'packages/other/src/x.ts\0') ignoredProc.emit('close', 0, null) }, 10) - await expect(promise).resolves.toEqual(['src/index.ts', 'dist/generated.js']) + await expect(promise).resolves.toEqual(['src/index.ts', 'tab\tfile.txt', 'dist/generated.js']) const ignoredArgs = spawnMock.mock.calls[1][1] as string[] expect(ignoredArgs).toEqual([ 'ls-files', + '-z', '--others', '--ignored', '--exclude-standard', @@ -120,10 +122,10 @@ describe('relay quick open ignored file listing', () => { const promise = listFilesWithGit('/remote/root') setTimeout(() => { - ;(primaryProc.stdout as unknown as EventEmitter).emit('data', 'src/index.ts\n') + ;(primaryProc.stdout as unknown as EventEmitter).emit('data', 'src/index.ts\0') primaryProc.emit('close', 0, null) - ;(ignoredProc.stdout as unknown as EventEmitter).emit('data', 'dist/generated.js\n') + ;(ignoredProc.stdout as unknown as EventEmitter).emit('data', 'dist/generated.js\0') ignoredProc.emit('close', null, 'SIGTERM') }, 10) diff --git a/src/shared/quick-open-filter.test.ts b/src/shared/quick-open-filter.test.ts index f330b0384e8..9f12e814aaa 100644 --- a/src/shared/quick-open-filter.test.ts +++ b/src/shared/quick-open-filter.test.ts @@ -233,12 +233,12 @@ describe('normalizeQuickOpenRgLine', () => { describe('buildGitLsFilesArgsForQuickOpen', () => { it('primary pass is --cached --others --exclude-standard', () => { const { primary } = buildGitLsFilesArgsForQuickOpen() - expect(primary).toEqual(['--cached', '--others', '--exclude-standard']) + expect(primary).toEqual(['-z', '--cached', '--others', '--exclude-standard']) }) it('ignored pass surfaces ignored files without .env* pathspec whitelist', () => { const { ignoredPass } = buildGitLsFilesArgsForQuickOpen() - expect(ignoredPass).toEqual(['--others', '--ignored', '--exclude-standard']) + expect(ignoredPass).toEqual(['-z', '--others', '--ignored', '--exclude-standard']) expect(ignoredPass).not.toContain('.env*') expect(ignoredPass).not.toContain(':(glob)**/.env*') }) diff --git a/src/shared/quick-open-filter.ts b/src/shared/quick-open-filter.ts index 68f25958f3e..90cdedfaddb 100644 --- a/src/shared/quick-open-filter.ts +++ b/src/shared/quick-open-filter.ts @@ -356,7 +356,9 @@ export function buildGitLsFilesArgsForQuickOpen( } const trailingPathspecs = excludeSpecs.length > 0 ? ['--', '.', ...excludeSpecs] : [] - const primary = ['--cached', '--others', '--exclude-standard', ...trailingPathspecs] - const ignoredPass = ['--others', '--ignored', '--exclude-standard', ...trailingPathspecs] + // Why: newline output C-quotes tabs/newlines, which makes Quick Open return + // fake paths when rg is unavailable. NUL output preserves real Git paths. + const primary = ['-z', '--cached', '--others', '--exclude-standard', ...trailingPathspecs] + const ignoredPass = ['-z', '--others', '--ignored', '--exclude-standard', ...trailingPathspecs] return { primary, ignoredPass } }