Skip to content

Commit 5eb29ef

Browse files
committed
feat(glob): bring back ripgrep as fallback
1 parent 4273b04 commit 5eb29ef

File tree

4 files changed

+362
-47
lines changed

4 files changed

+362
-47
lines changed
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
import { vi, describe, it, expect, beforeEach } from "vitest"
2+
import * as path from "path"
3+
4+
// Mock ripgrep to avoid filesystem dependencies
5+
vi.mock("../../ripgrep", () => ({
6+
getBinPath: vi.fn().mockResolvedValue("/mock/path/to/rg"),
7+
}))
8+
9+
// Mock vscode
10+
vi.mock("vscode", () => ({
11+
env: {
12+
appRoot: "/mock/app/root",
13+
},
14+
}))
15+
16+
// Mock filesystem operations
17+
vi.mock("fs", () => ({
18+
promises: {
19+
access: vi.fn().mockRejectedValue(new Error("Not found")),
20+
readFile: vi.fn().mockResolvedValue(""),
21+
readdir: vi.fn().mockResolvedValue([]),
22+
},
23+
existsSync: vi.fn().mockImplementation((path) => {
24+
return Promise.resolve(true)
25+
}),
26+
readFileSync: vi.fn().mockImplementation((path) => {
27+
return Promise.resolve("mock content")
28+
}),
29+
}))
30+
31+
vi.mock("child_process", () => ({
32+
spawn: vi.fn(),
33+
}))
34+
35+
vi.mock("../../path", () => ({
36+
arePathsEqual: vi.fn().mockReturnValue(false),
37+
}))
38+
39+
import { listFiles } from "../list-files"
40+
import * as childProcess from "child_process"
41+
42+
describe("list-files symlink support", () => {
43+
beforeEach(() => {
44+
vi.clearAllMocks()
45+
})
46+
47+
it("should include --follow flag in ripgrep arguments", async () => {
48+
const mockSpawn = vi.mocked(childProcess.spawn)
49+
const mockProcess = {
50+
stdout: {
51+
on: vi.fn((event, callback) => {
52+
if (event === "data") {
53+
// Simulate some output to complete the process
54+
setTimeout(() => callback("test-file.txt\n"), 10)
55+
}
56+
}),
57+
},
58+
stderr: {
59+
on: vi.fn(),
60+
},
61+
on: vi.fn((event, callback) => {
62+
if (event === "close") {
63+
setTimeout(() => callback(0), 20)
64+
}
65+
if (event === "error") {
66+
// No error simulation
67+
}
68+
}),
69+
kill: vi.fn(),
70+
}
71+
72+
mockSpawn.mockReturnValue(mockProcess as any)
73+
74+
// Call listFiles to trigger ripgrep execution
75+
await listFiles("/test/dir", false, 100)
76+
77+
// Verify that spawn was called with --follow flag (the critical fix)
78+
const [rgPath, args] = mockSpawn.mock.calls[0]
79+
expect(rgPath).toBe("/mock/path/to/rg")
80+
expect(args).toContain("--files")
81+
expect(args).toContain("--hidden")
82+
expect(args).toContain("--follow") // This is the critical assertion - the fix should add this flag
83+
84+
// Platform-agnostic path check - verify the last argument is the resolved path
85+
const expectedPath = path.resolve("/test/dir")
86+
expect(args[args.length - 1]).toBe(expectedPath)
87+
})
88+
89+
it("should include --follow flag for recursive listings too", async () => {
90+
const mockSpawn = vi.mocked(childProcess.spawn)
91+
const mockProcess = {
92+
stdout: {
93+
on: vi.fn((event, callback) => {
94+
if (event === "data") {
95+
setTimeout(() => callback("test-file.txt\n"), 10)
96+
}
97+
}),
98+
},
99+
stderr: {
100+
on: vi.fn(),
101+
},
102+
on: vi.fn((event, callback) => {
103+
if (event === "close") {
104+
setTimeout(() => callback(0), 20)
105+
}
106+
if (event === "error") {
107+
// No error simulation
108+
}
109+
}),
110+
kill: vi.fn(),
111+
}
112+
113+
mockSpawn.mockReturnValue(mockProcess as any)
114+
115+
// Call listFiles with recursive=true
116+
await listFiles("/test/dir", true, 100)
117+
118+
// Verify that spawn was called with --follow flag (the critical fix)
119+
const [rgPath, args] = mockSpawn.mock.calls[0]
120+
expect(rgPath).toBe("/mock/path/to/rg")
121+
expect(args).toContain("--files")
122+
expect(args).toContain("--hidden")
123+
expect(args).toContain("--follow") // This should be present in recursive mode too
124+
125+
// Platform-agnostic path check - verify the last argument is the resolved path
126+
const expectedPath = path.resolve("/test/dir")
127+
expect(args[args.length - 1]).toBe(expectedPath)
128+
})
129+
})

src/services/glob/list-files.ts

Lines changed: 74 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { globSync } from "glob"
66
import ignore from "ignore"
77
import { arePathsEqual, getWorkspacePath } from "../../utils/path"
88
import { isPathInIgnoredDirectory } from "./ignore-utils"
9+
import { listFilesWithRipgrep } from "./ripgrep-utils"
910

1011
function normalizePath(p: string): string {
1112
return p.replace(/\\/g, "/")
@@ -106,63 +107,60 @@ async function listAllFilesRecursively(
106107
} catch (error) {
107108
// Fallback to the manual method if git ls-files fails
108109
// (e.g., not a git repo, or an error with the command).
109-
console.warn("`git ls-files` failed, falling back to manual file search:", error)
110-
return listAllFilesRecursivelyWithFs(dir, git, workspacePath, limit)
110+
console.warn("`git ls-files` failed, falling back to ripgrep:", error)
111+
return listAllFilesRecursivelyWithRipGrep(dir, git, workspacePath, limit)
111112
}
112113
}
113114

114115
/**
115-
* A fallback recursive file lister that manually walks the filesystem.
116-
* This is slower but works if Git is not available or fails.
116+
* A fallback recursive file lister that uses ripgrep for performance.
117+
* This is used if Git is not available or fails.
117118
*/
118-
async function listAllFilesRecursivelyWithFs(
119+
async function listAllFilesRecursivelyWithRipGrep(
119120
dir: string,
120121
_git: SimpleGit,
121122
workspacePath: string,
122123
limit: number,
123124
): Promise<string[]> {
124-
const result: string[] = []
125-
const queue: string[] = [dir]
126-
127-
// Create ignore instance using all .gitignore files in the workspace
128125
const ig = createGitignoreFilter(workspacePath)
129126

130-
while (queue.length > 0 && result.length < limit) {
131-
const currentDir = queue.shift()!
132-
133-
// Pre-check if the directory itself is ignored by custom ignore logic
134-
if (isPathInIgnoredDirectory(path.relative(workspacePath, currentDir))) {
135-
continue
136-
}
137-
138-
let entries: fs.Dirent[]
139-
try {
140-
entries = await fs.promises.readdir(currentDir, { withFileTypes: true })
141-
} catch (err) {
142-
continue // Skip unreadable directories
143-
}
127+
let files: string[] = []
128+
let usedRipgrep = false
129+
try {
130+
files = await listFilesWithRipgrep(dir, true, limit)
131+
usedRipgrep = true
132+
} catch (err) {
133+
console.warn("listFilesWithRipgrep failed:", err)
134+
}
144135

145-
for (const entry of entries) {
146-
const fullPath = path.join(currentDir, entry.name)
147-
const relPath = path.relative(workspacePath, fullPath).replace(/\\/g, "/")
148-
if (ig.ignores(relPath)) {
149-
if (entry.isDirectory()) {
150-
// Still need to recurse into subdirectories to find non-ignored children
151-
continue
152-
}
136+
// If ripgrep failed or returned no files, fallback to manual method
137+
if (!files || files.length === 0) {
138+
const queue: string[] = [dir]
139+
while (queue.length > 0) {
140+
const currentDir = queue.shift()!
141+
if (isPathInIgnoredDirectory(path.relative(workspacePath, currentDir))) {
153142
continue
154143
}
155-
if (entry.isDirectory()) {
156-
queue.push(fullPath)
157-
} else {
158-
result.push(fullPath)
159-
if (result.length >= limit) {
160-
break
144+
let entries: fs.Dirent[]
145+
try {
146+
entries = await fs.promises.readdir(currentDir, { withFileTypes: true })
147+
} catch (err) {
148+
continue
149+
}
150+
for (const entry of entries) {
151+
const fullPath = path.join(currentDir, entry.name)
152+
if (entry.isDirectory()) {
153+
queue.push(fullPath)
154+
} else {
155+
files.push(fullPath)
161156
}
162157
}
163158
}
164159
}
165-
return result
160+
161+
// Map to relative paths, filter, then map back to absolute
162+
const filtered = ig.filter(files.map((f) => path.relative(workspacePath, path.resolve(f)).replace(/\\/g, "/")))
163+
return filtered.slice(0, limit).map((rel) => path.join(workspacePath, rel))
166164
}
167165

168166
/**
@@ -218,18 +216,47 @@ function createGitignoreFilter(rootDir: string) {
218216

219217
/**
220218
* List only top-level files and directories, filtering out ignored ones.
219+
* Uses ripgrep for performance and consistency with recursive listing.
221220
*/
222221
async function listNonRecursive(dir: string, _git: SimpleGit, workspacePath: string): Promise<string[]> {
223-
const entries = await fs.promises.readdir(dir, { withFileTypes: true })
224222
const ig = createGitignoreFilter(workspacePath)
225223

226-
const result: string[] = []
227-
for (const entry of entries) {
228-
const fullPath = path.join(dir, entry.name)
229-
const relPath = path.relative(workspacePath, fullPath).replace(/\\/g, "/")
230-
if (!ig.ignores(relPath) && !isPathInIgnoredDirectory(relPath)) {
231-
result.push(fullPath)
232-
}
224+
let files: string[] = []
225+
try {
226+
files = await listFilesWithRipgrep(dir, false, 10000)
227+
} catch (err) {
228+
console.warn("listFilesWithRipgrep (non-recursive) failed:", err)
229+
}
230+
231+
// If ripgrep failed or returned no files, fallback to manual method
232+
if (!files || files.length === 0) {
233+
const entries = await fs.promises.readdir(dir, { withFileTypes: true })
234+
return entries
235+
.map((entry) => path.join(dir, entry.name))
236+
.filter((fullPath) => {
237+
const relPath = path.relative(workspacePath, fullPath).replace(/\\/g, "/")
238+
return !ig.ignores(relPath) && !isPathInIgnoredDirectory(relPath)
239+
})
233240
}
234-
return result
241+
242+
const filtered = ig.filter(files.map((f) => path.relative(workspacePath, path.resolve(f)).replace(/\\/g, "/")))
243+
244+
// Ripgrep --files only lists files, not directories. Add top-level directories manually.
245+
let dirEntries: string[] = []
246+
try {
247+
const entries = await fs.promises.readdir(dir, { withFileTypes: true })
248+
dirEntries = entries
249+
.filter((entry) => entry.isDirectory())
250+
.map((entry) => path.join(dir, entry.name))
251+
.filter((fullPath) => {
252+
const relPath = path.relative(workspacePath, fullPath).replace(/\\/g, "/")
253+
return !ig.ignores(relPath) && !isPathInIgnoredDirectory(relPath)
254+
})
255+
} catch (err) {
256+
// ignore
257+
}
258+
259+
// Enforce limit after combining files and directories
260+
const combined = [...filtered.map((rel) => path.join(workspacePath, rel)), ...dirEntries]
261+
return combined.slice(0, 10000) // 10000 is the same as above, will be sliced by caller
235262
}

0 commit comments

Comments
 (0)