From f41cecc7129626c69e3546131bd07f4b2c178cbc Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 26 Sep 2026 21:58:10 -0700 Subject: [PATCH] fix(search): keep commas inside grouped file filters (#23319) Keep grouped commas intact when building search filters for ripgrep and Git. Based on contributor proposal #22915. Co-authored-by: KAPUIST --- src/shared/text-search-glob-patterns.test.ts | 205 +++++++++++++++++++ src/shared/text-search-glob-patterns.ts | 37 +++- src/shared/text-search.ts | 8 +- 3 files changed, 242 insertions(+), 8 deletions(-) create mode 100644 src/shared/text-search-glob-patterns.test.ts diff --git a/src/shared/text-search-glob-patterns.test.ts b/src/shared/text-search-glob-patterns.test.ts new file mode 100644 index 00000000000..0bd96723a9d --- /dev/null +++ b/src/shared/text-search-glob-patterns.test.ts @@ -0,0 +1,205 @@ +import { afterAll, beforeAll, describe, expect, it } from 'vitest' +import { mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { runProcess } from './child-process/run-process' +import { splitSearchGlobPatterns } from './text-search-glob-patterns' +import { + buildGitGrepArgs, + buildRgArgs, + createAccumulator, + finalize, + ingestRgJsonLine +} from './text-search' + +describe('compound search globs', () => { + it.each([ + ['', []], + [' , *.ts,, *.md, ', ['*.ts', '*.md']], + ['}.ts, *.md', ['}.ts', '*.md']], + ['*.{ts,tsx}, *.md', ['*.{ts,tsx}', '*.md']], + ['{src,{lib,test}}/**, *.md', ['{src,{lib,test}}/**', '*.md']], + ['*[a,b].ts, *.md', ['*[a,b].ts', '*.md']], + ['[{},].ts, *.md', ['[{},].ts', '*.md']], + ['[]a,b].ts, *.md', ['[]a,b].ts', '*.md']], + ['[!]a,b].ts, *.md', ['[!]a,b].ts', '*.md']], + ['[^]a,b].ts, *.md', ['[^]a,b].ts', '*.md']], + ['[[:alpha:],].ts, *.md', ['[[:alpha:],].ts', '*.md']], + ['[a\\],b].ts, *.md', ['[a\\],b].ts', '*.md']], + ['foo\\,bar/**, *.ts', ['foo\\,bar/**', '*.ts']], + ['\\{a,b\\}, *.ts', ['\\{a', 'b\\}', '*.ts']], + ['\\[a,b\\], *.ts', ['\\[a', 'b\\]', '*.ts']], + ['src\\', ['src\\']], + ['*.{ts,tsx', ['*.{ts,tsx']], + ['[a,b', ['[a,b']] + ])('preserves grouped and escaped commas in %s', (input, expected) => { + expect(splitSearchGlobPatterns(input, 'git')).toEqual(expected) + }) + + it('scans deeply nested groups without recursion or expanding alternatives', () => { + const pattern = `${'{'.repeat(20_000)}a,b${'}'.repeat(20_000)}` + expect(splitSearchGlobPatterns(`${pattern}, *.md`)).toEqual([pattern, '*.md']) + }) +}) + +describe('file search with real search engines', () => { + let root: string + + beforeAll(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-search-globs-')) + for (const file of ['a.ts', 'b.tsx', 'c.md', ',.ts']) { + await writeFile(join(root, file), 'needle\n') + } + const initialized = await runProcess({ program: 'git', args: ['init'], cwd: root }) + expect(initialized.code).toBe(0) + }) + + afterAll(async () => { + if (root) { + await rm(root, { recursive: true, force: true }) + } + }) + + it.each([ + [{ includePattern: '*.{ts,tsx}' }, [',.ts', 'a.ts', 'b.tsx']], + [{ includePattern: '[a,b].ts, *.md' }, [',.ts', 'a.ts', 'c.md']], + [{ excludePattern: '*.{ts,tsx}' }, ['c.md']], + [{ includePattern: '[]a,b].ts, *.md' }, [',.ts', 'a.ts', 'c.md']], + [{ excludePattern: '[a,b].ts' }, ['b.tsx', 'c.md']], + [{ includePattern: '*.ts, *.tsx', excludePattern: '[a,b].ts' }, ['b.tsx']] + ])('matches files with ripgrep using %j', async (options, expected) => { + const { rgPath } = await import('@vscode/ripgrep-universal') + const result = await runProcess({ + program: rgPath, + args: buildRgArgs('needle', root, options), + cwd: root + }) + expect(result.stderr).toBe('') + expect(result.code).toBe(0) + const accumulator = createAccumulator() + for (const line of result.stdout.split('\n')) { + ingestRgJsonLine(line, root, accumulator, 100) + } + expect( + finalize(accumulator) + .files.map((file) => file.relativePath) + .sort() + ).toEqual(expected) + }) + + it.each(['*.{ts,tsx', '[a,b'])('leaves invalid %s for ripgrep to reject', async (pattern) => { + const { rgPath } = await import('@vscode/ripgrep-universal') + for (const options of [{ includePattern: pattern }, { excludePattern: pattern }]) { + const result = await runProcess({ + program: rgPath, + args: buildRgArgs('needle', root, options), + cwd: root + }) + expect(result.code).toBe(2) + expect(result.stderr).toContain('error parsing glob') + expect(result.stderr).toContain(pattern) + } + }) + + it('keeps Git brace syntax literal instead of expanding it', async () => { + const result = await runProcess({ + program: 'git', + args: buildGitGrepArgs('needle', { includePattern: '*.{ts,tsx}' }), + cwd: root + }) + expect(result.code).toBe(1) + expect(result.stdout).toBe('') + expect(result.stderr).toBe('') + }) + + it('preserves character classes in Git exclusions', async () => { + const result = await runProcess({ + program: 'git', + args: buildGitGrepArgs('needle', { excludePattern: '[[:alpha:],].ts' }), + cwd: root + }) + expect(result.code).toBe(0) + expect(result.stdout).toContain('b.tsx\0') + expect(result.stdout).toContain('c.md\0') + expect(result.stdout).not.toContain('a.ts\0') + expect(result.stdout).not.toContain(',.ts\0') + }) + + it.each(['[a,b].ts', '[[:alpha:],].ts'])( + 'preserves %s in the Git fallback too', + async (includePattern) => { + const result = await runProcess({ + program: 'git', + args: buildGitGrepArgs('needle', { includePattern }), + cwd: root + }) + expect(result.code).toBe(0) + expect(result.stdout).toContain('a.ts\0') + expect(result.stdout).toContain(',.ts\0') + expect(result.stdout).not.toContain('b.tsx\0') + expect(result.stdout).not.toContain('c.md\0') + } + ) +}) + +describe('engine-specific character classes', () => { + let root: string + const files = [',.ts', '[.ts', '].ts', 'a.ts', 'a,b].ts', 'b.tsx', 'c.md'] + + beforeAll(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-search-class-syntax-')) + for (const file of files) { + await writeFile(join(root, file), 'needle\n') + } + expect((await runProcess({ program: 'git', args: ['init'], cwd: root })).code).toBe(0) + }) + + afterAll(async () => { + if (root) { + await rm(root, { recursive: true, force: true }) + } + }) + + it.each([ + ['rg', '[a\\].ts, *.md', ['a.ts', 'c.md']], + ['rg', '[[:].ts, *.md', ['[.ts', 'c.md']], + ['git', '[[:].ts, *.md', ['[.ts', 'c.md']], + ['git', '[a\\],b].ts, *.md', [',.ts', '].ts', 'a.ts', 'c.md']], + ['git', '[[:alpha:],].ts, *.md', [',.ts', 'a.ts', 'c.md']], + ['rg', '[[:alpha:], *.md', ['c.md']] + ] as const)( + 'respects %s syntax in %s for includes and excludes', + async (engine, pattern, matches) => { + const { rgPath } = await import('@vscode/ripgrep-universal') + for (const exclude of [false, true]) { + const options = exclude ? { excludePattern: pattern } : { includePattern: pattern } + const result = await runProcess({ + program: engine === 'rg' ? rgPath : 'git', + args: + engine === 'rg' + ? buildRgArgs('needle', root, options) + : buildGitGrepArgs('needle', options), + cwd: root + }) + expect(result.code).toBe(0) + expect(result.stderr).toBe('') + let actual: string[] + if (engine === 'rg') { + const accumulator = createAccumulator() + for (const line of result.stdout.split('\n')) { + ingestRgJsonLine(line, root, accumulator, 100) + } + actual = finalize(accumulator).files.map((file) => file.relativePath) + } else { + actual = result.stdout + .trim() + .split('\n') + .map((line) => line.split('\0')[0]) + } + const selected = new Set(matches) + const expected = exclude ? files.filter((file) => !selected.has(file)) : [...matches] + expect(actual.sort()).toEqual(expected.sort()) + } + } + ) +}) diff --git a/src/shared/text-search-glob-patterns.ts b/src/shared/text-search-glob-patterns.ts index 5e1e9487a36..b8d511f072d 100644 --- a/src/shared/text-search-glob-patterns.ts +++ b/src/shared/text-search-glob-patterns.ts @@ -1,18 +1,47 @@ -export function splitSearchGlobPatterns(patterns: string): string[] { +/** Keeps grouped and escaped commas inside globs so search engines receive complete patterns. */ +export function splitSearchGlobPatterns(patterns: string, engine: 'rg' | 'git' = 'rg'): string[] { const out: string[] = [] let current = '' let escaping = false - for (const ch of patterns) { + let braceDepth = 0 + let characterClassContentStart = -1 + let posixClassStart = -1 + for (let index = 0; index < patterns.length; index += 1) { + const ch = patterns[index] if (escaping) { current += `\\${ch}` escaping = false continue } - if (ch === '\\') { + // Ripgrep treats backslashes literally inside classes; Git uses them as escapes. + if (ch === '\\' && (characterClassContentStart === -1 || engine === 'git')) { escaping = true continue } - if (ch === ',') { + if (characterClassContentStart !== -1) { + // A leading ']' (also after negation) is a member, not the end of the class. + if (posixClassStart !== -1) { + if (ch === ']') { + // An incomplete POSIX opener can just be literal '[' and ':' members. + if (patterns[index - 1] !== ':' || index <= posixClassStart + 3) { + characterClassContentStart = -1 + } + posixClassStart = -1 + } + } else if (engine === 'git' && ch === '[' && patterns[index + 1] === ':') { + posixClassStart = index + } else if (ch === ']' && index > characterClassContentStart) { + characterClassContentStart = -1 + } + } else if (ch === '[') { + const next = patterns[index + 1] + characterClassContentStart = index + (next === '!' || next === '^' ? 2 : 1) + } else if (ch === '{') { + braceDepth += 1 + } else if (ch === '}') { + braceDepth = Math.max(0, braceDepth - 1) + } + if (ch === ',' && braceDepth === 0 && characterClassContentStart === -1) { const trimmed = current.trim() if (trimmed) { out.push(trimmed) diff --git a/src/shared/text-search.ts b/src/shared/text-search.ts index 8d5972ffabd..7bb54243c4d 100644 --- a/src/shared/text-search.ts +++ b/src/shared/text-search.ts @@ -75,12 +75,12 @@ export function buildRgArgs(query: string, target: string, opts: SearchOptionsLi args.push('--fixed-strings') } if (opts.includePattern) { - for (const pat of splitSearchGlobPatterns(opts.includePattern)) { + for (const pat of splitSearchGlobPatterns(opts.includePattern, 'rg')) { args.push('--glob', pat) } } if (opts.excludePattern) { - for (const pat of splitSearchGlobPatterns(opts.excludePattern)) { + for (const pat of splitSearchGlobPatterns(opts.excludePattern, 'rg')) { args.push('--glob', `!${pat}`) } } @@ -197,7 +197,7 @@ export function buildGitGrepArgs(query: string, opts: SearchOptionsLike): string let hasPathspecs = false let hasIncludePathspecs = false if (opts.includePattern) { - for (const pat of splitSearchGlobPatterns(opts.includePattern)) { + for (const pat of splitSearchGlobPatterns(opts.includePattern, 'git')) { const pathspecs = toGitGlobPathspecs(pat) gitArgs.push(...pathspecs) hasPathspecs ||= pathspecs.length > 0 @@ -205,7 +205,7 @@ export function buildGitGrepArgs(query: string, opts: SearchOptionsLike): string } } if (opts.excludePattern) { - for (const pat of splitSearchGlobPatterns(opts.excludePattern)) { + for (const pat of splitSearchGlobPatterns(opts.excludePattern, 'git')) { const pathspecs = toGitGlobPathspecs(pat, true) gitArgs.push(...pathspecs) hasPathspecs ||= pathspecs.length > 0