xiaoxue_admin
/
orca
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix: parse quick open git fallback paths losslessly (#4049)

This commit is contained in:
Neil 2026-05-31 02:43:28 -07:00 committed by GitHub
parent 1c1677eaad
commit b772a9ca42
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 105 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

61
src/main/ipc/filesystem-list-files-git-fallback-real.test.ts Normal file
View File

@ -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'
])
})
})

20
src/main/ipc/filesystem-list-files.test.ts
View File

@ -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)

25
src/main/ipc/filesystem-list-files.ts
View File

@ -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()
}

21
src/relay/fs-handler-git-fallback.ts
View File

@ -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()
}

14
src/relay/fs-handler-list-files-ignored.test.ts
View File

@ -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)

4
src/shared/quick-open-filter.test.ts
View File

@ -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*')
})

6
src/shared/quick-open-filter.ts
View File

@ -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 }
}