Fix file search match parsing (#4542)
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Jinwoo Hong
GitHub
parent
195f3e94f9
commit
205494b7ba
|
|
@ -43,9 +43,12 @@ describe('filesystem-search-git', () => {
|
|||
setTimeout(() => {
|
||||
;(proc.stdout as unknown as EventEmitter).emit(
|
||||
'data',
|
||||
'src/index.ts\x005: console.log("hello world")\n'
|
||||
'src/index.ts\x005\x00 console.log("hello world")\n'
|
||||
)
|
||||
;(proc.stdout as unknown as EventEmitter).emit(
|
||||
'data',
|
||||
'src/main.ts\x0012\x00 return "hello"\n'
|
||||
)
|
||||
;(proc.stdout as unknown as EventEmitter).emit('data', 'src/main.ts\x0012: return "hello"\n')
|
||||
proc.emit('close')
|
||||
}, 10)
|
||||
|
||||
|
|
@ -74,7 +77,7 @@ describe('filesystem-search-git', () => {
|
|||
const promise = searchWithGitGrep('/mock/root', { query: 'ab', rootPath: '/mock/root' }, 100)
|
||||
|
||||
setTimeout(() => {
|
||||
;(proc.stdout as unknown as EventEmitter).emit('data', 'file.txt\x001:ab cd ab ef ab\n')
|
||||
;(proc.stdout as unknown as EventEmitter).emit('data', 'file.txt\x001\x00ab cd ab ef ab\n')
|
||||
proc.emit('close')
|
||||
}, 10)
|
||||
|
||||
|
|
@ -98,7 +101,7 @@ describe('filesystem-search-git', () => {
|
|||
setTimeout(() => {
|
||||
;(proc.stdout as unknown as EventEmitter).emit(
|
||||
'data',
|
||||
'a.ts\x001:x\n' + 'b.ts\x001:x\n' + 'c.ts\x001:x\n'
|
||||
'a.ts\x001\x00x\n' + 'b.ts\x001\x00x\n' + 'c.ts\x001\x00x\n'
|
||||
)
|
||||
proc.emit('close')
|
||||
}, 10)
|
||||
|
|
@ -177,7 +180,7 @@ describe('filesystem-search-git', () => {
|
|||
|
||||
const promise = searchWithGitGrep('/mock/root', { query: 'ok', rootPath: '/mock/root' }, 100)
|
||||
|
||||
;(proc.stdout as unknown as EventEmitter).emit('data', 'valid.ts\x001:ok\npartial')
|
||||
;(proc.stdout as unknown as EventEmitter).emit('data', 'valid.ts\x001\x00ok\npartial')
|
||||
|
||||
await vi.runOnlyPendingTimersAsync()
|
||||
|
||||
|
|
@ -203,7 +206,7 @@ describe('filesystem-search-git', () => {
|
|||
setTimeout(() => {
|
||||
// A line without \0 should be skipped (e.g. git header output)
|
||||
;(proc.stdout as unknown as EventEmitter).emit('data', 'no-null-here:1:ok\n')
|
||||
;(proc.stdout as unknown as EventEmitter).emit('data', 'valid.ts\x003:ok\n')
|
||||
;(proc.stdout as unknown as EventEmitter).emit('data', 'valid.ts\x003\x00ok\n')
|
||||
proc.emit('close')
|
||||
}, 10)
|
||||
|
||||
|
|
|
|||
|
|
@ -39,7 +39,7 @@ describe('relay git grep fallback', () => {
|
|||
|
||||
const promise = searchWithGitGrep('/remote/root', 'ok', { maxResults: 100 })
|
||||
|
||||
;(proc.stdout as unknown as EventEmitter).emit('data', 'valid.ts\x001:ok\npartial')
|
||||
;(proc.stdout as unknown as EventEmitter).emit('data', 'valid.ts\x001\x00ok\npartial')
|
||||
|
||||
await vi.runOnlyPendingTimersAsync()
|
||||
|
||||
|
|
|
|||
|
|
@ -1,4 +1,8 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
import { execFileSync } from 'child_process'
|
||||
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'fs'
|
||||
import { tmpdir } from 'os'
|
||||
import { join } from 'path'
|
||||
import {
|
||||
buildGitGrepArgs,
|
||||
buildRgArgs,
|
||||
|
|
@ -112,6 +116,22 @@ describe('ingestRgJsonLine', () => {
|
|||
expect(acc.totalMatches).toBe(0)
|
||||
})
|
||||
|
||||
it('creates a navigable fallback match when rg omits submatch ranges', () => {
|
||||
const acc = createAccumulator()
|
||||
const verdict = ingestRgJsonLine(makeMatch('/root/a.ts', 4, [], 'foobar'), '/root', acc, 100)
|
||||
expect(verdict).toBe('continue')
|
||||
expect(acc.totalMatches).toBe(1)
|
||||
const file = Array.from(acc.fileMap.values())[0]
|
||||
expect(file.matches).toEqual([{ line: 4, column: 1, matchLength: 1, lineContent: 'foobar' }])
|
||||
})
|
||||
|
||||
it('keeps empty-line rg matches navigable when rg omits submatch ranges', () => {
|
||||
const acc = createAccumulator()
|
||||
ingestRgJsonLine(makeMatch('/root/a.ts', 5, [], ''), '/root', acc, 100)
|
||||
const file = Array.from(acc.fileMap.values())[0]
|
||||
expect(file.matches).toEqual([{ line: 5, column: 1, matchLength: 0, lineContent: '' }])
|
||||
})
|
||||
|
||||
it('stops at maxResults and sets truncated synchronously', () => {
|
||||
const acc = createAccumulator()
|
||||
const verdict = ingestRgJsonLine(
|
||||
|
|
@ -241,10 +261,49 @@ describe('buildSubmatchRegex', () => {
|
|||
})
|
||||
|
||||
describe('ingestGitGrepLine', () => {
|
||||
it('parses null-byte delimited line, finds all submatch positions', () => {
|
||||
it('parses actual git grep null-delimited output from the current git binary', () => {
|
||||
const rootPath = mkdtempSync(join(tmpdir(), 'orca-search-git-'))
|
||||
try {
|
||||
execFileSync('git', ['init'], { cwd: rootPath, stdio: 'ignore' })
|
||||
mkdirSync(join(rootPath, 'src'))
|
||||
writeFileSync(
|
||||
join(rootPath, 'src', 'a.ts'),
|
||||
[
|
||||
"reportError(err, { action: 'save' })",
|
||||
'reportError(err); reportError(next)',
|
||||
'unrelated'
|
||||
].join('\n')
|
||||
)
|
||||
|
||||
const stdout = execFileSync(
|
||||
'git',
|
||||
buildGitGrepArgs('reportError(', { caseSensitive: false, useRegex: false }),
|
||||
{ cwd: rootPath, encoding: 'utf8' }
|
||||
)
|
||||
const acc = createAccumulator()
|
||||
const re = buildSubmatchRegex('reportError(', {})
|
||||
for (const line of stdout.split('\n')) {
|
||||
ingestGitGrepLine(line, rootPath, re, acc, 100)
|
||||
}
|
||||
|
||||
const result = finalize(acc)
|
||||
expect(result.totalMatches).toBe(3)
|
||||
expect(result.files).toHaveLength(1)
|
||||
expect(result.files[0].relativePath).toBe('src/a.ts')
|
||||
expect(result.files[0].matches.map((match) => [match.line, match.column])).toEqual([
|
||||
[1, 1],
|
||||
[2, 1],
|
||||
[2, 19]
|
||||
])
|
||||
} finally {
|
||||
rmSync(rootPath, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
it('parses git grep null-delimited line, finds all submatch positions', () => {
|
||||
const acc = createAccumulator()
|
||||
const re = buildSubmatchRegex('foo', {})
|
||||
const verdict = ingestGitGrepLine('src/a.ts\x005:foo and foo again\n', '/root', re, acc, 100)
|
||||
const verdict = ingestGitGrepLine('src/a.ts\x005\x00foo and foo again\n', '/root', re, acc, 100)
|
||||
expect(verdict).toBe('continue')
|
||||
const f = Array.from(acc.fileMap.values())[0]
|
||||
expect(f.matches).toHaveLength(2)
|
||||
|
|
@ -252,10 +311,33 @@ describe('ingestGitGrepLine', () => {
|
|||
expect(f.matches[1]).toMatchObject({ line: 5, column: 9 })
|
||||
})
|
||||
|
||||
it('keeps compatibility with colon-delimited git grep lines', () => {
|
||||
const acc = createAccumulator()
|
||||
const re = buildSubmatchRegex('foo', {})
|
||||
ingestGitGrepLine('src/a.ts\x005:foo', '/root', re, acc, 100)
|
||||
const f = Array.from(acc.fileMap.values())[0]
|
||||
expect(f.matches[0]).toMatchObject({ line: 5, column: 1 })
|
||||
})
|
||||
|
||||
it('does not treat colons in matched content as the line-number delimiter', () => {
|
||||
const acc = createAccumulator()
|
||||
const re = buildSubmatchRegex('reportError(', {})
|
||||
ingestGitGrepLine(
|
||||
"src/a.ts\x0010\x00reportError(err, { action: 'save' })\n",
|
||||
'/root',
|
||||
re,
|
||||
acc,
|
||||
100
|
||||
)
|
||||
const f = Array.from(acc.fileMap.values())[0]
|
||||
expect(f.matches).toHaveLength(1)
|
||||
expect(f.matches[0]).toMatchObject({ line: 10, column: 1, matchLength: 12 })
|
||||
})
|
||||
|
||||
it('handles colons in filenames via null delimiter', () => {
|
||||
const acc = createAccumulator()
|
||||
const re = buildSubmatchRegex('x', {})
|
||||
ingestGitGrepLine('weird:name.ts\x001:x', '/root', re, acc, 100)
|
||||
ingestGitGrepLine('weird:name.ts\x001\x00x', '/root', re, acc, 100)
|
||||
const f = Array.from(acc.fileMap.values())[0]
|
||||
expect(f.relativePath).toBe('weird:name.ts')
|
||||
})
|
||||
|
|
@ -273,7 +355,7 @@ describe('ingestGitGrepLine', () => {
|
|||
const acc = createAccumulator()
|
||||
// A pattern that matches zero-length at every position.
|
||||
const re = new RegExp('', 'g')
|
||||
ingestGitGrepLine('a.ts\x001:abc', '/r', re, acc, 5)
|
||||
ingestGitGrepLine('a.ts\x001\x00abc', '/r', re, acc, 5)
|
||||
expect(acc.totalMatches).toBeGreaterThan(0)
|
||||
expect(acc.totalMatches).toBeLessThanOrEqual(5)
|
||||
})
|
||||
|
|
@ -281,7 +363,7 @@ describe('ingestGitGrepLine', () => {
|
|||
it('stops at maxResults boundary and sets truncated synchronously', () => {
|
||||
const acc = createAccumulator()
|
||||
const re = buildSubmatchRegex('a', {})
|
||||
const verdict = ingestGitGrepLine('f\x001:aaaa', '/r', re, acc, 2)
|
||||
const verdict = ingestGitGrepLine('f\x001\x00aaaa', '/r', re, acc, 2)
|
||||
expect(verdict).toBe('stop')
|
||||
expect(acc.truncated).toBe(true)
|
||||
expect(acc.totalMatches).toBe(2)
|
||||
|
|
@ -289,7 +371,7 @@ describe('ingestGitGrepLine', () => {
|
|||
|
||||
it('falls back to whole-line highlight when submatchRegex is null', () => {
|
||||
const acc = createAccumulator()
|
||||
const verdict = ingestGitGrepLine('a.ts\x003:hello world', '/r', null, acc, 100)
|
||||
const verdict = ingestGitGrepLine('a.ts\x003\x00hello world', '/r', null, acc, 100)
|
||||
expect(verdict).toBe('continue')
|
||||
const f = Array.from(acc.fileMap.values())[0]
|
||||
expect(f.matches).toHaveLength(1)
|
||||
|
|
@ -305,13 +387,35 @@ describe('ingestGitGrepLine', () => {
|
|||
describe('finalize', () => {
|
||||
it('returns the expected SearchResult shape', () => {
|
||||
const acc = createAccumulator()
|
||||
acc.fileMap.set('/r/a.ts', { filePath: '/r/a.ts', relativePath: 'a.ts', matches: [] })
|
||||
acc.totalMatches = 3
|
||||
acc.fileMap.set('/r/a.ts', {
|
||||
filePath: '/r/a.ts',
|
||||
relativePath: 'a.ts',
|
||||
matches: [{ line: 1, column: 1, matchLength: 3, lineContent: 'foo' }]
|
||||
})
|
||||
acc.totalMatches = 1
|
||||
acc.truncated = true
|
||||
expect(finalize(acc)).toEqual({
|
||||
files: [{ filePath: '/r/a.ts', relativePath: 'a.ts', matches: [] }],
|
||||
totalMatches: 3,
|
||||
files: [
|
||||
{
|
||||
filePath: '/r/a.ts',
|
||||
relativePath: 'a.ts',
|
||||
matches: [{ line: 1, column: 1, matchLength: 3, lineContent: 'foo' }]
|
||||
}
|
||||
],
|
||||
totalMatches: 1,
|
||||
truncated: true
|
||||
})
|
||||
})
|
||||
|
||||
it('filters impossible empty file rows before returning results', () => {
|
||||
const acc = createAccumulator()
|
||||
acc.fileMap.set('/r/a.ts', { filePath: '/r/a.ts', relativePath: 'a.ts', matches: [] })
|
||||
acc.fileMap.set('/r/b.ts', {
|
||||
filePath: '/r/b.ts',
|
||||
relativePath: 'b.ts',
|
||||
matches: [{ line: 1, column: 1, matchLength: 3, lineContent: 'foo' }]
|
||||
})
|
||||
acc.totalMatches = 1
|
||||
expect(finalize(acc).files.map((file) => file.relativePath)).toEqual(['b.ts'])
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -237,15 +237,19 @@ export function ingestRgJsonLine(
|
|||
const relPath = normalizeRelativePath(relative(rootPath, absPath))
|
||||
const lineContent = (data.lines?.text ?? '').replace(/\n$/, '')
|
||||
const lineNumber = data.line_number ?? 0
|
||||
const submatches = data.submatches ?? []
|
||||
|
||||
let fileResult = acc.fileMap.get(absPath)
|
||||
if (!fileResult) {
|
||||
fileResult = { filePath: absPath, relativePath: relPath, matches: [] }
|
||||
acc.fileMap.set(absPath, fileResult)
|
||||
let submatches = data.submatches ?? []
|
||||
if (submatches.length === 0) {
|
||||
// Why: some rg regex matches report the line but no submatch ranges.
|
||||
// Surface a navigable line-level result instead of a file row with count 0.
|
||||
submatches = [{ start: 0, end: lineContent.length > 0 ? 1 : 0 }]
|
||||
}
|
||||
|
||||
for (const sub of submatches) {
|
||||
let fileResult = acc.fileMap.get(absPath)
|
||||
if (!fileResult) {
|
||||
fileResult = { filePath: absPath, relativePath: relPath, matches: [] }
|
||||
acc.fileMap.set(absPath, fileResult)
|
||||
}
|
||||
const clamped = clampLineContext(lineContent, sub.start, sub.end - sub.start)
|
||||
fileResult.matches.push({
|
||||
line: lineNumber,
|
||||
|
|
@ -386,28 +390,42 @@ export function ingestGitGrepLine(
|
|||
return 'continue'
|
||||
}
|
||||
|
||||
// Why: with --null -n the output format is filename\0linenum:content.
|
||||
// Why: with --null -n, modern git emits filename\0linenum\0content.
|
||||
// Keep the older colon parser too so relay hosts with different git output
|
||||
// remain searchable.
|
||||
const nullIdx = line.indexOf('\0')
|
||||
if (nullIdx === -1) {
|
||||
return 'continue'
|
||||
}
|
||||
const relPath = normalizeRelativePath(line.substring(0, nullIdx))
|
||||
const rest = line.substring(nullIdx + 1)
|
||||
const colonIdx = rest.indexOf(':')
|
||||
if (colonIdx === -1) {
|
||||
const secondNullIdx = rest.indexOf('\0')
|
||||
let lineNumberText: string
|
||||
let lineContent: string
|
||||
if (secondNullIdx >= 0) {
|
||||
lineNumberText = rest.substring(0, secondNullIdx)
|
||||
lineContent = rest.substring(secondNullIdx + 1).replace(/\n$/, '')
|
||||
} else {
|
||||
const colonIdx = rest.indexOf(':')
|
||||
if (colonIdx === -1) {
|
||||
return 'continue'
|
||||
}
|
||||
lineNumberText = rest.substring(0, colonIdx)
|
||||
lineContent = rest.substring(colonIdx + 1).replace(/\n$/, '')
|
||||
}
|
||||
if (!/^\d+$/.test(lineNumberText)) {
|
||||
return 'continue'
|
||||
}
|
||||
const lineNum = parseInt(rest.substring(0, colonIdx), 10)
|
||||
if (isNaN(lineNum)) {
|
||||
return 'continue'
|
||||
}
|
||||
const lineContent = rest.substring(colonIdx + 1).replace(/\n$/, '')
|
||||
const lineNum = Number(lineNumberText)
|
||||
|
||||
const absPath = join(rootPath, relPath)
|
||||
let fileResult = acc.fileMap.get(absPath)
|
||||
if (!fileResult) {
|
||||
fileResult = { filePath: absPath, relativePath: relPath, matches: [] }
|
||||
acc.fileMap.set(absPath, fileResult)
|
||||
const getFileResult = (): SearchFileResult => {
|
||||
let fileResult = acc.fileMap.get(absPath)
|
||||
if (!fileResult) {
|
||||
fileResult = { filePath: absPath, relativePath: relPath, matches: [] }
|
||||
acc.fileMap.set(absPath, fileResult)
|
||||
}
|
||||
return fileResult
|
||||
}
|
||||
|
||||
// Why: git grep already confirmed the line matched — if we can't build a
|
||||
|
|
@ -416,7 +434,7 @@ export function ingestGitGrepLine(
|
|||
// whole-line highlight so the result still shows up in the UI.
|
||||
if (submatchRegex === null) {
|
||||
const clamped = clampLineContext(lineContent, 0, lineContent.length)
|
||||
fileResult.matches.push({
|
||||
getFileResult().matches.push({
|
||||
line: lineNum,
|
||||
column: clamped.column,
|
||||
matchLength: clamped.matchLength,
|
||||
|
|
@ -438,7 +456,7 @@ export function ingestGitGrepLine(
|
|||
let m: RegExpExecArray | null
|
||||
while ((m = submatchRegex.exec(lineContent)) !== null) {
|
||||
const clamped = clampLineContext(lineContent, m.index, m[0].length)
|
||||
fileResult.matches.push({
|
||||
getFileResult().matches.push({
|
||||
line: lineNum,
|
||||
column: clamped.column,
|
||||
matchLength: clamped.matchLength,
|
||||
|
|
@ -465,7 +483,7 @@ export function ingestGitGrepLine(
|
|||
|
||||
export function finalize(acc: SearchAccumulator): SearchResult {
|
||||
return {
|
||||
files: Array.from(acc.fileMap.values()),
|
||||
files: Array.from(acc.fileMap.values()).filter((file) => file.matches.length > 0),
|
||||
totalMatches: acc.totalMatches,
|
||||
truncated: acc.truncated
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue