Skip to content

Commit 87c0b5d

Browse files
committed
Revert "address duplicate filtering and Combine predicates to reduce traversal" add additional test coverage for escaped literals
This reverts commit 0b34253.
1 parent 0b34253 commit 87c0b5d

File tree

3 files changed

+189
-8
lines changed

3 files changed

+189
-8
lines changed

src/core/ignore/GitIgnoreController.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,6 @@ export class GitIgnoreController extends BaseIgnoreController {
6363
await this.findGitignoreFilesRecursively(this.cwd)
6464

6565
// Load any files discovered by recursion that weren't loaded yet
66-
// De-duplicate discovered paths to avoid redundant loads
67-
this.gitignoreFiles = Array.from(new Set(this.gitignoreFiles))
6866
for (const p of this.gitignoreFiles) {
6967
if (!this.gitignoreContents.has(p)) {
7068
await this.loadGitignoreFile(p)

src/core/ignore/__tests__/GitIgnoreController.spec.ts

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,4 +282,190 @@ describe("GitIgnoreController", () => {
282282
expect(controller.getGitignoreContent(gitignoreFile)).toBe(content)
283283
})
284284
})
285+
286+
describe("escaped literals", () => {
287+
it("should treat \\# as a literal # pattern, not a comment", async () => {
288+
// Setup .gitignore with escaped # pattern
289+
mockFileExists.mockImplementation((filePath: string) => {
290+
return Promise.resolve(filePath === path.join(TEST_CWD, ".gitignore"))
291+
})
292+
// \#foo should match a file literally named "#foo"
293+
// Also include a real comment to verify it's ignored
294+
mockReadFile.mockResolvedValue("\\#foo\n# This is a comment\n*.log\n")
295+
296+
await controller.initialize()
297+
298+
// File named "#foo" should be blocked (pattern matches literal #)
299+
expect(controller.validateAccess("#foo")).toBe(false)
300+
301+
// File named "# This is a comment" should NOT be blocked (it was a comment line)
302+
expect(controller.validateAccess("# This is a comment")).toBe(true)
303+
304+
// Other files should follow normal patterns
305+
expect(controller.validateAccess("test.log")).toBe(false)
306+
expect(controller.validateAccess("src/index.ts")).toBe(true)
307+
})
308+
it("should treat \\# as a literal # pattern in nested .gitignore (exposes bug in line 136)", async () => {
309+
// This test exposes the bug in line 136 of GitIgnoreController.ts
310+
// where !line.startsWith("#") incorrectly filters out \# patterns
311+
mockFileExists.mockImplementation((filePath: string) => {
312+
return Promise.resolve(
313+
filePath === path.join(TEST_CWD, ".gitignore") ||
314+
filePath === path.join(TEST_CWD, "src", ".gitignore"),
315+
)
316+
})
317+
318+
mockReadFile.mockImplementation((filePath: any) => {
319+
const normalizedPath = filePath.toString().replace(/\\/g, "/")
320+
if (normalizedPath.endsWith("src/.gitignore")) {
321+
// Escaped # should match literal # file
322+
return Promise.resolve("\\#special\n# Real comment\n")
323+
}
324+
return Promise.resolve("")
325+
})
326+
327+
await controller.initialize()
328+
329+
// File named "#special" in src/ should be blocked
330+
// BUG: This currently passes (file is allowed) because line 136 filters out \#special
331+
expect(controller.validateAccess("src/#special")).toBe(false)
332+
333+
// Real comment should not create a pattern
334+
expect(controller.validateAccess("src/# Real comment")).toBe(true)
335+
})
336+
337+
it("should treat \\! as a literal ! pattern, not a negation", async () => {
338+
// Setup .gitignore with escaped ! pattern
339+
mockFileExists.mockImplementation((filePath: string) => {
340+
return Promise.resolve(filePath === path.join(TEST_CWD, ".gitignore"))
341+
})
342+
// First ignore all .txt files, then \!keep.txt should match literal "!keep.txt"
343+
mockReadFile.mockResolvedValue("*.txt\n\\!keep.txt\n")
344+
345+
await controller.initialize()
346+
347+
// All .txt files should be blocked
348+
expect(controller.validateAccess("file.txt")).toBe(false)
349+
expect(controller.validateAccess("keep.txt")).toBe(false)
350+
351+
// File literally named "!keep.txt" should also be blocked (not negated)
352+
expect(controller.validateAccess("!keep.txt")).toBe(false)
353+
354+
// Non-.txt files should be allowed
355+
expect(controller.validateAccess("src/index.ts")).toBe(true)
356+
})
357+
358+
it("should handle multiple escaped patterns in the same file", async () => {
359+
mockFileExists.mockImplementation((filePath: string) => {
360+
return Promise.resolve(filePath === path.join(TEST_CWD, ".gitignore"))
361+
})
362+
// Mix of escaped and normal patterns
363+
mockReadFile.mockResolvedValue("\\#comment-like\n\\!negation-like\n*.log\n")
364+
365+
await controller.initialize()
366+
367+
// Escaped patterns should match literal files
368+
expect(controller.validateAccess("#comment-like")).toBe(false)
369+
expect(controller.validateAccess("!negation-like")).toBe(false)
370+
371+
// Normal patterns should work
372+
expect(controller.validateAccess("debug.log")).toBe(false)
373+
expect(controller.validateAccess("src/index.ts")).toBe(true)
374+
})
375+
376+
it("should handle escaped patterns in nested .gitignore files", async () => {
377+
mockFileExists.mockImplementation((filePath: string) => {
378+
return Promise.resolve(
379+
filePath === path.join(TEST_CWD, ".gitignore") ||
380+
filePath === path.join(TEST_CWD, "src", ".gitignore"),
381+
)
382+
})
383+
384+
mockReadFile.mockImplementation((filePath: any) => {
385+
const normalizedPath = filePath.toString().replace(/\\/g, "/")
386+
if (normalizedPath.endsWith("src/.gitignore")) {
387+
// Nested .gitignore with escaped patterns
388+
// Include a real comment to verify it's properly ignored
389+
return Promise.resolve("\\#special\n# This is a comment\n\\!important\n")
390+
}
391+
return Promise.resolve("*.log\n")
392+
})
393+
394+
await controller.initialize()
395+
396+
// Escaped patterns in nested .gitignore should match literal files in that directory
397+
expect(controller.validateAccess("src/#special")).toBe(false)
398+
expect(controller.validateAccess("src/!important")).toBe(false)
399+
400+
// Real comment should not create a pattern
401+
expect(controller.validateAccess("src/# This is a comment")).toBe(true)
402+
403+
// Should not affect files outside src/
404+
expect(controller.validateAccess("#special")).toBe(true)
405+
expect(controller.validateAccess("!important")).toBe(true)
406+
407+
// Root patterns should still work
408+
expect(controller.validateAccess("debug.log")).toBe(false)
409+
})
410+
411+
it("should not treat escaped \\! as negation in nested .gitignore", async () => {
412+
mockFileExists.mockImplementation((filePath: string) => {
413+
return Promise.resolve(
414+
filePath === path.join(TEST_CWD, ".gitignore") ||
415+
filePath === path.join(TEST_CWD, "src", ".gitignore"),
416+
)
417+
})
418+
419+
mockReadFile.mockImplementation((filePath: any) => {
420+
const normalizedPath = filePath.toString().replace(/\\/g, "/")
421+
if (normalizedPath.endsWith("src/.gitignore")) {
422+
// First ignore all .txt, then try to use escaped ! (should NOT negate)
423+
return Promise.resolve("*.txt\n\\!keep.txt\n")
424+
}
425+
return Promise.resolve("")
426+
})
427+
428+
await controller.initialize()
429+
430+
// All .txt files in src/ should be blocked
431+
expect(controller.validateAccess("src/file.txt")).toBe(false)
432+
expect(controller.validateAccess("src/keep.txt")).toBe(false)
433+
434+
// File literally named "!keep.txt" should also be blocked (not negated)
435+
expect(controller.validateAccess("src/!keep.txt")).toBe(false)
436+
437+
// Non-.txt files should be allowed
438+
expect(controller.validateAccess("src/index.ts")).toBe(true)
439+
})
440+
441+
it("should correctly distinguish between comments and escaped # patterns", async () => {
442+
mockFileExists.mockImplementation((filePath: string) => {
443+
return Promise.resolve(
444+
filePath === path.join(TEST_CWD, ".gitignore") ||
445+
filePath === path.join(TEST_CWD, "src", ".gitignore"),
446+
)
447+
})
448+
449+
mockReadFile.mockImplementation((filePath: any) => {
450+
const normalizedPath = filePath.toString().replace(/\\/g, "/")
451+
if (normalizedPath.endsWith("src/.gitignore")) {
452+
// Mix of real comments and escaped # patterns
453+
return Promise.resolve("# This is a comment\n" + "\\#not-a-comment\n" + "*.tmp\n")
454+
}
455+
return Promise.resolve("# Root comment\n*.log\n")
456+
})
457+
458+
await controller.initialize()
459+
460+
// Escaped # pattern should match literal file
461+
expect(controller.validateAccess("src/#not-a-comment")).toBe(false)
462+
463+
// Comments should not create patterns
464+
expect(controller.validateAccess("src/# This is a comment")).toBe(true)
465+
466+
// Normal patterns should work
467+
expect(controller.validateAccess("src/file.tmp")).toBe(false)
468+
expect(controller.validateAccess("test.log")).toBe(false)
469+
})
470+
})
285471
})

src/services/ripgrep/index.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -215,12 +215,9 @@ export async function regexSearchFiles(
215215
// console.log(results)
216216

217217
// Filter results using both controllers if provided
218-
const filteredResults = results.filter((result) => {
219-
const file = result.file
220-
return (
221-
(gitIgnoreController?.validateAccess(file) ?? true) && (rooIgnoreController?.validateAccess(file) ?? true)
222-
)
223-
})
218+
const filteredResults = results
219+
.filter((result) => gitIgnoreController?.validateAccess(result.file) ?? true)
220+
.filter((result) => rooIgnoreController?.validateAccess(result.file) ?? true)
224221

225222
return formatResults(filteredResults, cwd)
226223
}

0 commit comments

Comments
 (0)