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 <thsxornjs12@gmail.com>

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<string>(matches)
+        const expected = exclude ? files.filter((file) => !selected.has(file)) : [...matches]
+        expect(actual.sort()).toEqual(expected.sort())
+      }
+    }
+  )
+})

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)

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