Skip to content

Commit 60fbd5d

Browse files
roomoteroomote
committed
fix: prevent listFilesRecursive from excluding projects under /tmp directory
- Changed ripgrep exclusion patterns from !**/${dir}/** to !**/${dir}/ to only exclude directories with the specified name, not paths containing the name - This fixes the issue where projects under /tmp were having all their files excluded because "tmp" appeared in the parent path - Added comprehensive test cases to verify the fix works correctly - Ensures that nested tmp directories are still properly excluded while allowing projects under /tmp to work Fixes #6545
1 parent 836371c commit 60fbd5d

File tree

2 files changed

+238
-5
lines changed

2 files changed

+238
-5
lines changed

‎src/services/glob/__tests__/list-files-tmp-directory.spec.ts‎

Lines changed: 232 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,232 @@
1+
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"
2+
import * as path from "path"
3+
import { listFiles } from "../list-files"
4+
import * as childProcess from "child_process"
5+
import * as fs from "fs"
6+
7+
// Mock child_process.spawn
8+
vi.mock("child_process", () => ({
9+
spawn: vi.fn(),
10+
}))
11+
12+
// Mock fs.promises.readdir
13+
vi.mock("fs", async () => {
14+
const actual = await vi.importActual<typeof import("fs")>("fs")
15+
return {
16+
...actual,
17+
promises: {
18+
...actual.promises,
19+
readdir: vi.fn(),
20+
access: vi.fn(),
21+
readFile: vi.fn(),
22+
},
23+
}
24+
})
25+
26+
// Import getBinPath type for mocking
27+
import { getBinPath } from "../../../services/ripgrep"
28+
29+
// Mock getBinPath
30+
vi.mock("../../../services/ripgrep", () => ({
31+
getBinPath: vi.fn(),
32+
}))
33+
34+
// Mock vscode
35+
vi.mock("vscode", () => ({
36+
env: {
37+
appRoot: "/mock/app/root",
38+
},
39+
}))
40+
41+
describe("list-files with projects under /tmp directory", () => {
42+
beforeEach(() => {
43+
vi.clearAllMocks()
44+
// Set up getBinPath mock
45+
vi.mocked(getBinPath).mockResolvedValue("/path/to/rg")
46+
})
47+
48+
afterEach(() => {
49+
vi.restoreAllMocks()
50+
})
51+
52+
it("should list files in a project under /tmp directory", async () => {
53+
const mockSpawn = vi.mocked(childProcess.spawn)
54+
const mockProcess = {
55+
stdout: { on: vi.fn() },
56+
stderr: { on: vi.fn() },
57+
on: vi.fn(),
58+
kill: vi.fn(),
59+
}
60+
61+
mockSpawn.mockReturnValue(mockProcess as any)
62+
63+
// Simulate ripgrep output for files under /tmp/project
64+
mockProcess.stdout.on.mockImplementation((event, callback) => {
65+
if (event === "data") {
66+
// Simulate files that should be found in /tmp/project
67+
const files = ["a/b/c/a.js", "src/index.ts", "package.json", "README.md"].join("\n") + "\n"
68+
setTimeout(() => callback(files), 10)
69+
}
70+
})
71+
72+
mockProcess.on.mockImplementation((event, callback) => {
73+
if (event === "close") {
74+
setTimeout(() => callback(0), 20)
75+
}
76+
})
77+
78+
// Mock directory listing for /tmp/project
79+
const mockReaddir = vi.mocked(fs.promises.readdir)
80+
mockReaddir.mockImplementation(async (dirPath) => {
81+
const pathStr = dirPath.toString()
82+
if (pathStr === path.resolve("/tmp/project")) {
83+
return [
84+
{ name: "a", isDirectory: () => true, isSymbolicLink: () => false },
85+
{ name: "src", isDirectory: () => true, isSymbolicLink: () => false },
86+
{ name: "package.json", isDirectory: () => false, isSymbolicLink: () => false },
87+
{ name: "README.md", isDirectory: () => false, isSymbolicLink: () => false },
88+
] as any
89+
} else if (pathStr === path.resolve("/tmp/project/a")) {
90+
return [{ name: "b", isDirectory: () => true, isSymbolicLink: () => false }] as any
91+
} else if (pathStr === path.resolve("/tmp/project/a/b")) {
92+
return [{ name: "c", isDirectory: () => true, isSymbolicLink: () => false }] as any
93+
} else if (pathStr === path.resolve("/tmp/project/a/b/c")) {
94+
return [{ name: "a.js", isDirectory: () => false, isSymbolicLink: () => false }] as any
95+
}
96+
return []
97+
})
98+
99+
// Mock gitignore access (no .gitignore files)
100+
vi.mocked(fs.promises.access).mockRejectedValue(new Error("Not found"))
101+
102+
// Call listFiles targeting /tmp/project
103+
const [files, didHitLimit] = await listFiles("/tmp/project", true, 100)
104+
105+
// Verify ripgrep was called with correct arguments
106+
expect(mockSpawn).toHaveBeenCalledWith(
107+
"/path/to/rg",
108+
expect.arrayContaining([
109+
"--files",
110+
"--hidden",
111+
"--follow",
112+
"-g",
113+
"!**/node_modules/",
114+
"-g",
115+
"!**/__pycache__/",
116+
"-g",
117+
"!**/env/",
118+
"-g",
119+
"!**/venv/",
120+
"-g",
121+
"!**/target/dependency/",
122+
"-g",
123+
"!**/build/dependencies/",
124+
"-g",
125+
"!**/dist/",
126+
"-g",
127+
"!**/out/",
128+
"-g",
129+
"!**/bundle/",
130+
"-g",
131+
"!**/vendor/",
132+
"-g",
133+
"!**/tmp/", // This should exclude tmp directories, but not the parent /tmp
134+
"-g",
135+
"!**/temp/",
136+
"-g",
137+
"!**/deps/",
138+
"-g",
139+
"!**/pkg/",
140+
"-g",
141+
"!**/Pods/",
142+
"-g",
143+
"!**/.git/",
144+
"-g",
145+
"!**/.*/**", // Hidden directories pattern
146+
"/tmp/project",
147+
]),
148+
)
149+
150+
// Verify files were found
151+
expect(files).toContain(path.resolve("/tmp/project/a/b/c/a.js"))
152+
expect(files).toContain(path.resolve("/tmp/project/src/index.ts"))
153+
expect(files).toContain(path.resolve("/tmp/project/package.json"))
154+
expect(files).toContain(path.resolve("/tmp/project/README.md"))
155+
156+
// Verify directories were included
157+
expect(files).toContain(path.resolve("/tmp/project/a") + "/")
158+
expect(files).toContain(path.resolve("/tmp/project/a/b") + "/")
159+
expect(files).toContain(path.resolve("/tmp/project/a/b/c") + "/")
160+
expect(files).toContain(path.resolve("/tmp/project/src") + "/")
161+
162+
expect(didHitLimit).toBe(false)
163+
})
164+
165+
it("should exclude nested tmp directories within a project under /tmp", async () => {
166+
const mockSpawn = vi.mocked(childProcess.spawn)
167+
const mockProcess = {
168+
stdout: { on: vi.fn() },
169+
stderr: { on: vi.fn() },
170+
on: vi.fn(),
171+
kill: vi.fn(),
172+
}
173+
174+
mockSpawn.mockReturnValue(mockProcess as any)
175+
176+
// Simulate ripgrep output - should not include files from nested tmp directory
177+
mockProcess.stdout.on.mockImplementation((event, callback) => {
178+
if (event === "data") {
179+
const files =
180+
[
181+
"src/index.ts",
182+
"package.json",
183+
// Note: src/tmp/cache.js should NOT be included
184+
].join("\n") + "\n"
185+
setTimeout(() => callback(files), 10)
186+
}
187+
})
188+
189+
mockProcess.on.mockImplementation((event, callback) => {
190+
if (event === "close") {
191+
setTimeout(() => callback(0), 20)
192+
}
193+
})
194+
195+
// Mock directory listing
196+
const mockReaddir = vi.mocked(fs.promises.readdir)
197+
mockReaddir.mockImplementation(async (dirPath) => {
198+
const pathStr = dirPath.toString()
199+
if (pathStr === path.resolve("/tmp/myproject")) {
200+
return [
201+
{ name: "src", isDirectory: () => true, isSymbolicLink: () => false },
202+
{ name: "package.json", isDirectory: () => false, isSymbolicLink: () => false },
203+
] as any
204+
} else if (pathStr === path.resolve("/tmp/myproject/src")) {
205+
return [
206+
{ name: "index.ts", isDirectory: () => false, isSymbolicLink: () => false },
207+
{ name: "tmp", isDirectory: () => true, isSymbolicLink: () => false },
208+
] as any
209+
}
210+
return []
211+
})
212+
213+
// Mock gitignore access (no .gitignore files)
214+
vi.mocked(fs.promises.access).mockRejectedValue(new Error("Not found"))
215+
216+
// Call listFiles
217+
const [files, didHitLimit] = await listFiles("/tmp/myproject", true, 100)
218+
219+
// Verify files from root project are included
220+
expect(files).toContain(path.resolve("/tmp/myproject/src/index.ts"))
221+
expect(files).toContain(path.resolve("/tmp/myproject/package.json"))
222+
223+
// Verify nested tmp directory is NOT included
224+
expect(files).not.toContain(path.resolve("/tmp/myproject/src/tmp") + "/")
225+
226+
// Verify the exclusion pattern was applied correctly
227+
const spawnCall = mockSpawn.mock.calls[0]
228+
const args = spawnCall[1] as string[]
229+
expect(args).toContain("-g")
230+
expect(args).toContain("!**/tmp/")
231+
})
232+
})

‎src/services/glob/list-files.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -281,8 +281,10 @@ function buildRecursiveArgs(dirPath: string): string[] {
281281
continue
282282
}
283283

284-
// For all other cases, exclude the directory pattern globally
285-
args.push("-g", `!**/${dir}/**`)
284+
// For all other cases, exclude directories with this name at any level
285+
// Use !**/${dir}/ to match directories only (with trailing slash)
286+
// This prevents excluding files when the parent path contains the directory name
287+
args.push("-g", `!**/${dir}/`)
286288
}
287289

288290
return args
@@ -310,9 +312,8 @@ function buildNonRecursiveArgs(): string[] {
310312
// We'll let the directory scanning logic handle the visibility.
311313
continue
312314
} else {
313-
// Direct children only
314-
args.push("-g", `!${dir}`)
315-
args.push("-g", `!${dir}/**`)
315+
// Direct children only - exclude directories with this name
316+
args.push("-g", `!${dir}/`)
316317
}
317318
}
318319

0 commit comments

Comments
 (0)