Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
202 changes: 201 additions & 1 deletion src/services/glob/__tests__/list-files.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { describe, it, expect, vi } from "vitest"
import { vi, describe, it, expect, beforeEach } from "vitest"
import * as path from "path"
import { listFiles } from "../list-files"
import * as childProcess from "child_process"

vi.mock("../list-files", async () => {
const actual = await vi.importActual("../list-files")
Expand All @@ -16,3 +18,201 @@ describe("listFiles", () => {
expect(result).toEqual([[], false])
})
})

// Mock ripgrep to avoid filesystem dependencies
vi.mock("../../ripgrep", () => ({
getBinPath: vi.fn().mockResolvedValue("/mock/path/to/rg"),
}))

// Mock vscode
vi.mock("vscode", () => ({
env: {
appRoot: "/mock/app/root",
},
}))

// Mock filesystem operations
vi.mock("fs", () => ({
promises: {
access: vi.fn().mockRejectedValue(new Error("Not found")),
readFile: vi.fn().mockResolvedValue(""),
readdir: vi.fn().mockResolvedValue([]),
},
}))

// Import fs to set up mocks
import * as fs from "fs"

vi.mock("child_process", () => ({
spawn: vi.fn(),
}))

vi.mock("../../path", () => ({
arePathsEqual: vi.fn().mockReturnValue(false),
}))

describe("list-files symlink support", () => {
beforeEach(() => {
vi.clearAllMocks()
})

it("should include --follow flag in ripgrep arguments", async () => {
const mockSpawn = vi.mocked(childProcess.spawn)
const mockProcess = {
stdout: {
on: vi.fn((event, callback) => {
if (event === "data") {
// Simulate some output to complete the process
setTimeout(() => callback("test-file.txt\n"), 10)
}
}),
},
stderr: {
on: vi.fn(),
},
on: vi.fn((event, callback) => {
if (event === "close") {
setTimeout(() => callback(0), 20)
}
if (event === "error") {
// No error simulation
}
}),
kill: vi.fn(),
}

mockSpawn.mockReturnValue(mockProcess as any)

// Call listFiles to trigger ripgrep execution
await listFiles("/test/dir", false, 100)

// Verify that spawn was called with --follow flag (the critical fix)
const [rgPath, args] = mockSpawn.mock.calls[0]
expect(rgPath).toBe("/mock/path/to/rg")
expect(args).toContain("--files")
expect(args).toContain("--hidden")
expect(args).toContain("--follow") // This is the critical assertion - the fix should add this flag

// Platform-agnostic path check - verify the last argument is the resolved path
const expectedPath = path.resolve("/test/dir")
expect(args[args.length - 1]).toBe(expectedPath)
})

it("should include --follow flag for recursive listings too", async () => {
const mockSpawn = vi.mocked(childProcess.spawn)
const mockProcess = {
stdout: {
on: vi.fn((event, callback) => {
if (event === "data") {
setTimeout(() => callback("test-file.txt\n"), 10)
}
}),
},
stderr: {
on: vi.fn(),
},
on: vi.fn((event, callback) => {
if (event === "close") {
setTimeout(() => callback(0), 20)
}
if (event === "error") {
// No error simulation
}
}),
kill: vi.fn(),
}

mockSpawn.mockReturnValue(mockProcess as any)

// Call listFiles with recursive=true
await listFiles("/test/dir", true, 100)

// Verify that spawn was called with --follow flag (the critical fix)
const [rgPath, args] = mockSpawn.mock.calls[0]
expect(rgPath).toBe("/mock/path/to/rg")
expect(args).toContain("--files")
expect(args).toContain("--hidden")
expect(args).toContain("--follow") // This should be present in recursive mode too

// Platform-agnostic path check - verify the last argument is the resolved path
const expectedPath = path.resolve("/test/dir")
expect(args[args.length - 1]).toBe(expectedPath)
})

it("should ensure first-level directories are included when limit is reached", async () => {
// Mock fs.promises.readdir to simulate a directory structure
const mockReaddir = vi.mocked(fs.promises.readdir)

// Root directory with first-level directories
mockReaddir.mockResolvedValueOnce([
{ name: "a_dir", isDirectory: () => true, isSymbolicLink: () => false, isFile: () => false } as any,
{ name: "b_dir", isDirectory: () => true, isSymbolicLink: () => false, isFile: () => false } as any,
{ name: "c_dir", isDirectory: () => true, isSymbolicLink: () => false, isFile: () => false } as any,
{ name: "file1.txt", isDirectory: () => false, isSymbolicLink: () => false, isFile: () => true } as any,
{ name: "file2.txt", isDirectory: () => false, isSymbolicLink: () => false, isFile: () => true } as any,
])

// Mock ripgrep to return many files (simulating hitting the limit)
const mockSpawn = vi.mocked(childProcess.spawn)
const mockProcess = {
stdout: {
on: vi.fn((event, callback) => {
if (event === "data") {
// Return many file paths to trigger the limit
const paths =
[
"/test/dir/a_dir/",
"/test/dir/a_dir/subdir1/",
"/test/dir/a_dir/subdir1/file1.txt",
"/test/dir/a_dir/subdir1/file2.txt",
"/test/dir/a_dir/subdir2/",
"/test/dir/a_dir/subdir2/file3.txt",
"/test/dir/a_dir/file4.txt",
"/test/dir/a_dir/file5.txt",
"/test/dir/file1.txt",
"/test/dir/file2.txt",
// Note: b_dir and c_dir are missing from ripgrep output
].join("\n") + "\n"
setTimeout(() => callback(paths), 10)
}
}),
},
stderr: {
on: vi.fn(),
},
on: vi.fn((event, callback) => {
if (event === "close") {
setTimeout(() => callback(0), 20)
}
}),
kill: vi.fn(),
}
mockSpawn.mockReturnValue(mockProcess as any)

// Mock fs.promises.access to simulate .gitignore doesn't exist
vi.mocked(fs.promises.access).mockRejectedValue(new Error("File not found"))

// Call listFiles with recursive=true and a small limit
const [results, limitReached] = await listFiles("/test/dir", true, 10)

// Verify that we got results and hit the limit
expect(results.length).toBe(10)
expect(limitReached).toBe(true)

// Count directories in results
const directories = results.filter((r) => r.endsWith("/"))

// We should have at least the 3 first-level directories
// even if ripgrep didn't return all of them
expect(directories.length).toBeGreaterThanOrEqual(3)

// Verify all first-level directories are included
const hasADir = results.some((r) => r.endsWith("a_dir/"))
const hasBDir = results.some((r) => r.endsWith("b_dir/"))
const hasCDir = results.some((r) => r.endsWith("c_dir/"))

expect(hasADir).toBe(true)
expect(hasBDir).toBe(true)
expect(hasCDir).toBe(true)
})
})
103 changes: 96 additions & 7 deletions src/services/glob/list-files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,105 @@ export async function listFiles(dirPath: string, recursive: boolean, limit: numb
// Get ripgrep path
const rgPath = await getRipgrepPath()

// Get files using ripgrep
const files = await listFilesWithRipgrep(rgPath, dirPath, recursive, limit)
if (!recursive) {
// For non-recursive, use the existing approach
const files = await listFilesWithRipgrep(rgPath, dirPath, false, limit)
const ignoreInstance = await createIgnoreInstance(dirPath)
const directories = await listFilteredDirectories(dirPath, false, ignoreInstance)
return formatAndCombineResults(files, directories, limit)
}

// Get directories with proper filtering using ignore library
// For recursive mode, use the original approach but ensure first-level directories are included
const files = await listFilesWithRipgrep(rgPath, dirPath, true, limit)
const ignoreInstance = await createIgnoreInstance(dirPath)
const directories = await listFilteredDirectories(dirPath, recursive, ignoreInstance)
const directories = await listFilteredDirectories(dirPath, true, ignoreInstance)

// Combine and check if we hit the limit
const [results, limitReached] = formatAndCombineResults(files, directories, limit)

// Combine and format the results
return formatAndCombineResults(files, directories, limit)
// If we hit the limit, ensure all first-level directories are included
if (limitReached) {
const firstLevelDirs = await getFirstLevelDirectories(dirPath, ignoreInstance)
return ensureFirstLevelDirectoriesIncluded(results, firstLevelDirs, limit)
}

return [results, limitReached]
}

/**
* Get only the first-level directories in a path
*/
async function getFirstLevelDirectories(dirPath: string, ignoreInstance: ReturnType<typeof ignore>): Promise<string[]> {
const absolutePath = path.resolve(dirPath)
const directories: string[] = []

try {
const entries = await fs.promises.readdir(absolutePath, { withFileTypes: true })

for (const entry of entries) {
if (entry.isDirectory() && !entry.isSymbolicLink()) {
const fullDirPath = path.join(absolutePath, entry.name)
if (shouldIncludeDirectory(entry.name, fullDirPath, dirPath, ignoreInstance)) {
const formattedPath = fullDirPath.endsWith("/") ? fullDirPath : `${fullDirPath}/`
directories.push(formattedPath)
}
}
}
} catch (err) {
console.warn(`Could not read directory ${absolutePath}: ${err}`)
}

return directories
}

/**
* Ensure all first-level directories are included in the results
*/
function ensureFirstLevelDirectoriesIncluded(
results: string[],
firstLevelDirs: string[],
limit: number,
): [string[], boolean] {
// Create a set of existing paths for quick lookup
const existingPaths = new Set(results)

// Find missing first-level directories
const missingDirs = firstLevelDirs.filter((dir) => !existingPaths.has(dir))

if (missingDirs.length === 0) {
// All first-level directories are already included
return [results, true]
}

// We need to make room for the missing directories
// Remove items from the end (which are likely deeper in the tree)
const itemsToRemove = Math.min(missingDirs.length, results.length)
const adjustedResults = results.slice(0, results.length - itemsToRemove)

// Add the missing directories at the beginning (after any existing first-level dirs)
// First, separate existing results into first-level and others
const resultPaths = adjustedResults.map((r) => path.resolve(r))
const basePath = path.resolve(firstLevelDirs[0]).split(path.sep).slice(0, -1).join(path.sep)

const firstLevelResults: string[] = []
const otherResults: string[] = []

for (let i = 0; i < adjustedResults.length; i++) {
const resolvedPath = resultPaths[i]
const relativePath = path.relative(basePath, resolvedPath)
const depth = relativePath.split(path.sep).length

if (depth === 1) {
firstLevelResults.push(adjustedResults[i])
} else {
otherResults.push(adjustedResults[i])
}
}

// Combine: existing first-level dirs + missing first-level dirs + other results
const finalResults = [...firstLevelResults, ...missingDirs, ...otherResults].slice(0, limit)

return [finalResults, true]
}

/**
Expand Down Expand Up @@ -312,7 +402,6 @@ function isDirectoryExplicitlyIgnored(dirName: string): boolean {
return false
}


/**
* Combine file and directory results and format them properly
*/
Expand Down