Skip to content

Commit 777ae1f

Browse files
X9VoiDX9VoiD
committed
fix(services/glob): handle nested .gitignores
- Rewrote listFiles to use git ls-files for git repos (faster and respects all .gitignores) - Added fallback to manual file walk with ignore package for non-git repos - Optimized recursion to avoid traversing ignored directories - Added comprehensive integration tests - Improved test coverage with new unit tests for edge cases - Fixed path normalization to ensure consistent forward slashes The new implementation: 1. Handles nested .gitignore files correctly 2. Provides significant performance improvements in large repos 3. Maintains compatibility with non-git projects 4. Ensures consistent behavior across environments A caveat of this is that directories by themselves are not reported anymore if they don't contain any files.
1 parent 7ae7440 commit 777ae1f

File tree

3 files changed

+604
-433
lines changed

3 files changed

+604
-433
lines changed

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

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
import { describe, it, expect, beforeAll, afterAll } from "vitest"
2+
import os from "os"
3+
import * as path from "path"
4+
import * as fs from "fs"
5+
import simpleGit from "simple-git"
6+
import { listFiles } from "../list-files"
7+
import { getWorkspacePath } from "../../../utils/path"
8+
9+
vi.mock("../../../utils/path", () => ({
10+
getWorkspacePath: vi.fn(),
11+
arePathsEqual: (a: string, b: string) => path.resolve(a) === path.resolve(b),
12+
}))
13+
14+
describe("listFiles integration tests", () => {
15+
let testRepoPath: string
16+
17+
beforeAll(async () => {
18+
// Create a temporary directory for the Git repo
19+
testRepoPath = path.join(os.tmpdir(), `test-repo-${Date.now()}`)
20+
await fs.promises.mkdir(testRepoPath, { recursive: true })
21+
22+
// Initialize a Git repository
23+
const git = simpleGit(testRepoPath)
24+
await git.init()
25+
26+
// Create root .gitignore
27+
await fs.promises.writeFile(path.join(testRepoPath, ".gitignore"), "root_ignored.txt\nignored_in_root")
28+
29+
// Create files and directories
30+
await fs.promises.writeFile(path.join(testRepoPath, "root_kept.txt"), "content")
31+
await fs.promises.writeFile(path.join(testRepoPath, "root_ignored.txt"), "content")
32+
33+
// Subdirectory 1 with its own .gitignore
34+
const subdir1Path = path.join(testRepoPath, "subdir1")
35+
await fs.promises.mkdir(subdir1Path)
36+
await fs.promises.writeFile(path.join(subdir1Path, ".gitignore"), "subdir1_ignored.txt")
37+
await fs.promises.writeFile(path.join(subdir1Path, "subdir1_kept.txt"), "content")
38+
await fs.promises.writeFile(path.join(subdir1Path, "subdir1_ignored.txt"), "content")
39+
40+
// Subdirectory 2 (should be affected by root .gitignore)
41+
const subdir2Path = path.join(testRepoPath, "subdir2")
42+
await fs.promises.mkdir(subdir2Path)
43+
await fs.promises.writeFile(path.join(subdir2Path, "subdir2_kept.txt"), "content")
44+
await fs.promises.writeFile(path.join(testRepoPath, "ignored_in_root"), "content")
45+
})
46+
47+
afterAll(async () => {
48+
// Cleanup the temporary directory
49+
await fs.promises.rm(testRepoPath, { recursive: true, force: true })
50+
})
51+
52+
it("should correctly handle multiple .gitignore files", async () => {
53+
vi.mocked(getWorkspacePath).mockReturnValue(testRepoPath)
54+
55+
const [files] = await listFiles(testRepoPath, true, 1000)
56+
57+
const expectedFiles = [
58+
path.join(testRepoPath, "root_kept.txt"),
59+
path.join(testRepoPath, "subdir1", "subdir1_kept.txt"),
60+
path.join(testRepoPath, "subdir2", "subdir2_kept.txt"),
61+
].map((p) => p.replace(/\\/g, "/"))
62+
63+
const normalizedFiles = files.map((p) => p.replace(/\\/g, "/"))
64+
const sortedFiles = normalizedFiles.sort()
65+
const sortedExpected = expectedFiles.sort()
66+
67+
expect(sortedFiles).toEqual(sortedExpected)
68+
69+
// Verify that ignored files are not present
70+
expect(normalizedFiles.some((f) => f.endsWith("root_ignored.txt"))).toBe(false)
71+
expect(normalizedFiles.some((f) => f.endsWith("subdir1_ignored.txt"))).toBe(false)
72+
expect(normalizedFiles.some((f) => f.endsWith("ignored_in_root"))).toBe(false)
73+
})
74+
75+
it("should list only top-level non-ignored files in root (git repo)", async () => {
76+
vi.mocked(getWorkspacePath).mockReturnValue(testRepoPath)
77+
const [files] = await listFiles(testRepoPath, false, 1000)
78+
const expectedFiles = [
79+
path.join(testRepoPath, "root_kept.txt"),
80+
path.join(testRepoPath, "subdir1"),
81+
path.join(testRepoPath, "subdir2"),
82+
].map((p) => p.replace(/\\/g, "/"))
83+
const normalizedFiles = files.map((p) => p.replace(/\\/g, "/"))
84+
const sortedFiles = normalizedFiles.sort()
85+
const sortedExpected = expectedFiles.sort()
86+
expect(sortedFiles).toEqual(sortedExpected)
87+
expect(normalizedFiles.some((f) => f.endsWith("root_ignored.txt"))).toBe(false)
88+
expect(normalizedFiles.some((f) => f.endsWith("ignored_in_root"))).toBe(false)
89+
})
90+
91+
it("should list only top-level non-ignored files in subdir1 (git repo, nested)", async () => {
92+
vi.mocked(getWorkspacePath).mockReturnValue(testRepoPath)
93+
const subdir1Path = path.join(testRepoPath, "subdir1")
94+
const [files] = await listFiles(subdir1Path, false, 1000)
95+
const expectedFiles = [path.join(subdir1Path, "subdir1_kept.txt")].map((p) => p.replace(/\\/g, "/"))
96+
const normalizedFiles = files.map((p) => p.replace(/\\/g, "/"))
97+
const sortedFiles = normalizedFiles.sort()
98+
const sortedExpected = expectedFiles.sort()
99+
expect(sortedFiles).toEqual(sortedExpected)
100+
expect(normalizedFiles.some((f) => f.endsWith("subdir1_ignored.txt"))).toBe(false)
101+
})
102+
})
103+
104+
describe("listFiles in non-Git repository", () => {
105+
let testRepoPath: string
106+
107+
beforeAll(async () => {
108+
// Create a temporary directory that is NOT a Git repo
109+
testRepoPath = path.join(os.tmpdir(), `test-non-git-repo-${Date.now()}`)
110+
await fs.promises.mkdir(testRepoPath, { recursive: true })
111+
112+
// Create files and directories
113+
await fs.promises.writeFile(path.join(testRepoPath, "root_kept.txt"), "content")
114+
115+
// Subdirectory 1
116+
const subdir1Path = path.join(testRepoPath, "subdir1")
117+
await fs.promises.mkdir(subdir1Path)
118+
await fs.promises.writeFile(path.join(subdir1Path, "subdir1_kept.txt"), "content")
119+
120+
// Subdirectory 2
121+
const subdir2Path = path.join(testRepoPath, "subdir2")
122+
await fs.promises.mkdir(subdir2Path)
123+
await fs.promises.writeFile(path.join(subdir2Path, "subdir2_kept.txt"), "content")
124+
125+
vi.mocked(getWorkspacePath).mockReturnValue(testRepoPath)
126+
})
127+
128+
afterAll(async () => {
129+
// Cleanup the temporary directory
130+
await fs.promises.rm(testRepoPath, { recursive: true, force: true })
131+
})
132+
133+
it("should fall back to manual file search when not a git repository", async () => {
134+
const [files] = await listFiles(testRepoPath, true, 1000)
135+
136+
const expectedFiles = [
137+
path.join(testRepoPath, "root_kept.txt"),
138+
path.join(testRepoPath, "subdir1", "subdir1_kept.txt"),
139+
path.join(testRepoPath, "subdir2", "subdir2_kept.txt"),
140+
].map((p) => p.replace(/\\/g, "/"))
141+
142+
const normalizedFiles = files.map((p) => p.replace(/\\/g, "/"))
143+
const sortedFiles = normalizedFiles.sort()
144+
const sortedExpected = expectedFiles.sort()
145+
146+
expect(sortedFiles).toEqual(sortedExpected)
147+
148+
// Verify that ignored files are not present
149+
expect(normalizedFiles.some((f) => f.endsWith("root_ignored.txt"))).toBe(false)
150+
expect(normalizedFiles.some((f) => f.endsWith("subdir1_ignored.txt"))).toBe(false)
151+
expect(normalizedFiles.some((f) => f.endsWith("ignored_in_root"))).toBe(false)
152+
})
153+
154+
it("should list only top-level non-ignored files in root (no git)", async () => {
155+
vi.mocked(getWorkspacePath).mockReturnValue(testRepoPath)
156+
const [files] = await listFiles(testRepoPath, false, 1000)
157+
const expectedFiles = [
158+
path.join(testRepoPath, "root_kept.txt"),
159+
path.join(testRepoPath, "subdir1"),
160+
path.join(testRepoPath, "subdir2"),
161+
].map((p) => p.replace(/\\/g, "/"))
162+
const normalizedFiles = files.map((p) => p.replace(/\\/g, "/"))
163+
const sortedFiles = normalizedFiles.sort()
164+
const sortedExpected = expectedFiles.sort()
165+
expect(sortedFiles).toEqual(sortedExpected)
166+
expect(normalizedFiles.some((f) => f.endsWith("root_ignored.txt"))).toBe(false)
167+
expect(normalizedFiles.some((f) => f.endsWith("ignored_in_root"))).toBe(false)
168+
})
169+
170+
it("should list only top-level non-ignored files in subdir1 (no git, nested)", async () => {
171+
vi.mocked(getWorkspacePath).mockReturnValue(testRepoPath)
172+
const subdir1Path = path.join(testRepoPath, "subdir1")
173+
const [files] = await listFiles(subdir1Path, false, 1000)
174+
const expectedFiles = [path.join(subdir1Path, "subdir1_kept.txt")].map((p) => p.replace(/\\/g, "/"))
175+
const normalizedFiles = files.map((p) => p.replace(/\\/g, "/"))
176+
const sortedFiles = normalizedFiles.sort()
177+
const sortedExpected = expectedFiles.sort()
178+
expect(sortedFiles).toEqual(sortedExpected)
179+
expect(normalizedFiles.some((f) => f.endsWith("subdir1_ignored.txt"))).toBe(false)
180+
})
181+
})

0 commit comments

Comments
 (0)