diff --git a/src/shared/git-binary-compatibility.test.ts b/src/shared/git-binary-compatibility.test.ts index e11fa6ed2c4..b76814f8b83 100644 --- a/src/shared/git-binary-compatibility.test.ts +++ b/src/shared/git-binary-compatibility.test.ts @@ -1,5 +1,5 @@ import { execFile } from 'node:child_process' -import { mkdtemp, readFile, rename, rm, writeFile } from 'node:fs/promises' +import { mkdir, mkdtemp, readFile, rename, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { promisify } from 'node:util' @@ -16,6 +16,7 @@ import { isUnsupportedWorktreeListZError } from './git-worktree-command-capabilities' import { gitCredentialPromptGuardEnv } from './git-credential-prompt-env' +import { buildGitGrepArgs } from './text-search' import { parseGitRemoteFetchUrls } from './git-remote-url-index' import { GIT_HISTORY_COMMIT_FORMAT, parseGitHistoryLog } from './git-history-log-parser' import { @@ -566,4 +567,27 @@ describeBinaryCompatibility('real Git binary compatibility', () => { expect(item?.subject).toBe('decorated commit') expect(item?.references?.map((ref) => ref.id)).toContain('refs/tags/compat-decorated') }) + + it('excludes and includes a directory subtree through the generated pathspecs', async () => { + await mkdir(join(repoPath, 'vendored'), { recursive: true }) + await writeFile(join(repoPath, 'vendored', 'inner.txt'), 'pathspecneedle\n') + await writeFile(join(repoPath, 'kept.txt'), 'pathspecneedle\n') + await runGit(['add', '-A']) + await runGit(['commit', '-qm', 'pathspec fixture']) + + const listFiles = async (opts: Parameters[1]): Promise => { + const args = buildGitGrepArgs('pathspecneedle', opts).map((arg) => + arg === '-n' ? '-l' : arg + ) + const { stdout } = await runGit(args) + return stdout.split(/[\0\n]/).filter(Boolean) + } + + const excluded = await listFiles({ excludePattern: 'vendored' }) + expect(excluded).toContain('kept.txt') + expect(excluded.some((file) => file.startsWith('vendored/'))).toBe(false) + + const included = await listFiles({ includePattern: 'vendored' }) + expect(included).toEqual(['vendored/inner.txt']) + }) }) diff --git a/src/shared/text-search-glob-patterns.ts b/src/shared/text-search-glob-patterns.ts index a677f39e8d8..5e1e9487a36 100644 --- a/src/shared/text-search-glob-patterns.ts +++ b/src/shared/text-search-glob-patterns.ts @@ -37,3 +37,13 @@ export function toGitGlobPathspec(glob: string, exclude?: boolean): string { const pattern = needsRecursive ? `**/${glob}` : glob return exclude ? `:(exclude,glob)${pattern}` : `:(glob)${pattern}` } + +export function toGitGlobPathspecs(glob: string, exclude?: boolean): string[] { + const directoryOnly = /\/+$/u.test(glob) + const trimmed = glob.replace(/\/+$/, '') + if (!trimmed) { + return [] + } + const pathspec = toGitGlobPathspec(trimmed, exclude) + return directoryOnly ? [`${pathspec}/**`] : [pathspec, `${pathspec}/**`] +} diff --git a/src/shared/text-search.test.ts b/src/shared/text-search.test.ts index 2ecbc53d8b4..b6528daf6e7 100644 --- a/src/shared/text-search.test.ts +++ b/src/shared/text-search.test.ts @@ -14,7 +14,7 @@ import { MAX_LINE_CONTENT_LENGTH, SEARCH_JSON_STRUCTURE_LIMITS } from './text-search' -import { splitSearchGlobPatterns, toGitGlobPathspec } from './text-search-glob-patterns' +import { splitSearchGlobPatterns, toGitGlobPathspecs } from './text-search-glob-patterns' import { normalizeRelativePath } from './text-search-paths' describe('normalizeRelativePath', () => { @@ -239,6 +239,18 @@ describe('buildGitGrepArgs', () => { expect(args).toContain(':(exclude,glob)dist/**') }) + it('excludes a directory subtree the way rg --glob does', () => { + const args = buildGitGrepArgs('q', { excludePattern: 'node_modules' }) + expect(args).toContain(':(exclude,glob)**/node_modules') + expect(args).toContain(':(exclude,glob)**/node_modules/**') + }) + + it('includes a directory subtree the way rg --glob does', () => { + const args = buildGitGrepArgs('q', { includePattern: 'src' }) + expect(args).toContain(':(glob)**/src') + expect(args).toContain(':(glob)**/src/**') + }) + it('keeps escaped commas inside one generated folder pathspec', () => { const args = buildGitGrepArgs('q', { includePattern: 'foo\\,bar/**, *.ts' }) expect(args).toContain(':(glob)foo\\,bar/**') @@ -246,11 +258,29 @@ describe('buildGitGrepArgs', () => { }) }) -describe('toGitGlobPathspec', () => { +describe('toGitGlobPathspecs', () => { it('wraps bare globs with **/ to match recursively', () => { - expect(toGitGlobPathspec('*.ts')).toBe(':(glob)**/*.ts') - expect(toGitGlobPathspec('src/*.ts')).toBe(':(glob)src/*.ts') - expect(toGitGlobPathspec('*.ts', true)).toBe(':(exclude,glob)**/*.ts') + expect(toGitGlobPathspecs('*.ts')).toContain(':(glob)**/*.ts') + expect(toGitGlobPathspecs('src/*.ts')).toContain(':(glob)src/*.ts') + expect(toGitGlobPathspecs('*.ts', true)).toContain(':(exclude,glob)**/*.ts') + }) + + it('also covers the subtree so a directory name behaves like rg --glob', () => { + expect(toGitGlobPathspecs('node_modules', true)).toEqual([ + ':(exclude,glob)**/node_modules', + ':(exclude,glob)**/node_modules/**' + ]) + expect(toGitGlobPathspecs('src')).toEqual([':(glob)**/src', ':(glob)**/src/**']) + expect(toGitGlobPathspecs('build/out')).toEqual([':(glob)build/out', ':(glob)build/out/**']) + }) + + it('keeps trailing-slash patterns directory-only', () => { + expect(toGitGlobPathspecs('node_modules/', true)).toEqual([':(exclude,glob)**/node_modules/**']) + expect(toGitGlobPathspecs('src/')).toEqual([':(glob)**/src/**']) + }) + + it('drops a pattern that is only separators', () => { + expect(toGitGlobPathspecs('/')).toEqual([]) }) }) @@ -321,6 +351,23 @@ describe('ingestGitGrepLine', () => { } }) + it('does not broaden a separator-only include pattern to the repository', () => { + const rootPath = mkdtempSync(join(tmpdir(), 'orca-search-git-')) + try { + execFileSync('git', ['init'], { cwd: rootPath, stdio: 'ignore' }) + writeFileSync(join(rootPath, 'target.ts'), 'needle\n') + + expect(() => + execFileSync('git', buildGitGrepArgs('needle', { includePattern: '/' }), { + cwd: rootPath, + stdio: 'ignore' + }) + ).toThrow() + } 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', {}) @@ -358,6 +405,23 @@ describe('ingestGitGrepLine', () => { expect(f.matches[0]).toMatchObject({ line: 10, column: 1, matchLength: 12 }) }) + it('does not broaden a separator-only include pattern to the repository', () => { + const rootPath = mkdtempSync(join(tmpdir(), 'orca-search-git-')) + try { + execFileSync('git', ['init'], { cwd: rootPath, stdio: 'ignore' }) + writeFileSync(join(rootPath, 'target.ts'), 'needle\n') + + expect(() => + execFileSync('git', buildGitGrepArgs('needle', { includePattern: '/' }), { + cwd: rootPath, + stdio: 'ignore' + }) + ).toThrow() + } finally { + rmSync(rootPath, { recursive: true, force: true }) + } + }) + it('handles colons in filenames via null delimiter', () => { const acc = createAccumulator() const re = buildSubmatchRegex('x', {}) diff --git a/src/shared/text-search.ts b/src/shared/text-search.ts index 963f0e13faf..8d5972ffabd 100644 --- a/src/shared/text-search.ts +++ b/src/shared/text-search.ts @@ -12,7 +12,7 @@ import { normalizeSearchResult } from './search-match-count' import { escapeRegex } from './string-utils' import type { SearchFileResult, SearchOptions, SearchResult } from './code-search-types' import { pushSearchMatch } from './text-search-match-accumulator' -import { splitSearchGlobPatterns, toGitGlobPathspec } from './text-search-glob-patterns' +import { splitSearchGlobPatterns, toGitGlobPathspecs } from './text-search-glob-patterns' import { joinSearchRoot, normalizeRelativePath, relativeToSearchRoot } from './text-search-paths' export type SearchAccumulator = { @@ -195,20 +195,26 @@ export function buildGitGrepArgs(query: string, opts: SearchOptionsLike): string gitArgs.push('-e', query, '--') let hasPathspecs = false + let hasIncludePathspecs = false if (opts.includePattern) { for (const pat of splitSearchGlobPatterns(opts.includePattern)) { - gitArgs.push(toGitGlobPathspec(pat)) - hasPathspecs = true + const pathspecs = toGitGlobPathspecs(pat) + gitArgs.push(...pathspecs) + hasPathspecs ||= pathspecs.length > 0 + hasIncludePathspecs ||= pathspecs.length > 0 } } if (opts.excludePattern) { for (const pat of splitSearchGlobPatterns(opts.excludePattern)) { - gitArgs.push(toGitGlobPathspec(pat, true)) - hasPathspecs = true + const pathspecs = toGitGlobPathspecs(pat, true) + gitArgs.push(...pathspecs) + hasPathspecs ||= pathspecs.length > 0 } } // Why: git grep needs a pathspec to search the working tree; '.' means everything under cwd. - if (!hasPathspecs) { + if (opts.includePattern && !hasIncludePathspecs) { + gitArgs.push(':(top,literal).git') + } else if (!hasPathspecs) { gitArgs.push('.') } return gitArgs