mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-05 08:10:14 +00:00
fix(glob): show top-level hidden directories in list_files
- Modified list-files.ts to show hidden directories at top level - Added test coverage for hidden directory visibility behavior - Maintains existing functionality while fixing the .roo directory issue Addresses PR #5176 feedback with simplified implementation
This commit is contained in:
parent
f71bbf0d34
commit
d518f27ecc
3 changed files with 198 additions and 24 deletions
|
|
@ -216,3 +216,124 @@ describe("list-files symlink support", () => {
|
|||
expect(hasCDir).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe("hidden directory exclusion", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
it("should exclude .git subdirectories from recursive directory listing", async () => {
|
||||
// Mock filesystem structure with .git subdirectories
|
||||
const mockReaddir = vi.fn()
|
||||
vi.mocked(fs.promises).readdir = mockReaddir
|
||||
|
||||
// Mock the directory structure:
|
||||
// /test/
|
||||
// .git/
|
||||
// hooks/
|
||||
// objects/
|
||||
// src/
|
||||
// components/
|
||||
mockReaddir
|
||||
.mockResolvedValueOnce([
|
||||
{ name: ".git", isDirectory: () => true, isSymbolicLink: () => false },
|
||||
{ name: "src", isDirectory: () => true, isSymbolicLink: () => false },
|
||||
])
|
||||
.mockResolvedValueOnce([
|
||||
// src subdirectories (should be included)
|
||||
{ name: "components", isDirectory: () => true, isSymbolicLink: () => false },
|
||||
])
|
||||
.mockResolvedValueOnce([]) // components/ is empty
|
||||
|
||||
// Mock ripgrep to return no files
|
||||
const mockSpawn = vi.mocked(childProcess.spawn)
|
||||
const mockProcess = {
|
||||
stdout: {
|
||||
on: vi.fn((event, callback) => {
|
||||
if (event === "data") {
|
||||
// No files returned
|
||||
}
|
||||
}),
|
||||
},
|
||||
stderr: {
|
||||
on: vi.fn(),
|
||||
},
|
||||
on: vi.fn((event, callback) => {
|
||||
if (event === "close") {
|
||||
setTimeout(() => callback(0), 10)
|
||||
}
|
||||
}),
|
||||
kill: vi.fn(),
|
||||
}
|
||||
mockSpawn.mockReturnValue(mockProcess as any)
|
||||
|
||||
// Call listFiles with recursive=true
|
||||
const [result] = await listFiles("/test", true, 100)
|
||||
|
||||
// Verify that .git subdirectories are NOT included
|
||||
const directories = result.filter((item) => item.endsWith("/"))
|
||||
|
||||
// More specific checks - look for exact paths
|
||||
const hasSrcDir = directories.some((dir) => dir.endsWith("/test/src/") || dir.endsWith("src/"))
|
||||
const hasComponentsDir = directories.some(
|
||||
(dir) =>
|
||||
dir.endsWith("/test/src/components/") || dir.endsWith("src/components/") || dir.includes("components/"),
|
||||
)
|
||||
const hasGitDir = directories.some((dir) => dir.includes(".git/"))
|
||||
|
||||
// Should include src/ and src/components/ but NOT .git/ or its subdirectories
|
||||
expect(hasSrcDir).toBe(true)
|
||||
expect(hasComponentsDir).toBe(true)
|
||||
|
||||
// Should NOT include .git (hidden directories are excluded)
|
||||
expect(hasGitDir).toBe(false)
|
||||
})
|
||||
|
||||
it("should allow explicit targeting of hidden directories", async () => {
|
||||
// Mock filesystem structure for explicit .roo-memory targeting
|
||||
const mockReaddir = vi.fn()
|
||||
vi.mocked(fs.promises).readdir = mockReaddir
|
||||
|
||||
// Mock .roo-memory directory contents
|
||||
mockReaddir.mockResolvedValueOnce([
|
||||
{ name: "tasks", isDirectory: () => true, isSymbolicLink: () => false },
|
||||
{ name: "context", isDirectory: () => true, isSymbolicLink: () => false },
|
||||
])
|
||||
|
||||
// Mock ripgrep to return no files
|
||||
const mockSpawn = vi.mocked(childProcess.spawn)
|
||||
const mockProcess = {
|
||||
stdout: {
|
||||
on: vi.fn((event, callback) => {
|
||||
if (event === "data") {
|
||||
// No files returned
|
||||
}
|
||||
}),
|
||||
},
|
||||
stderr: {
|
||||
on: vi.fn(),
|
||||
},
|
||||
on: vi.fn((event, callback) => {
|
||||
if (event === "close") {
|
||||
setTimeout(() => callback(0), 10)
|
||||
}
|
||||
}),
|
||||
kill: vi.fn(),
|
||||
}
|
||||
mockSpawn.mockReturnValue(mockProcess as any)
|
||||
|
||||
// Call listFiles explicitly targeting .roo-memory directory
|
||||
const [result] = await listFiles("/test/.roo-memory", true, 100)
|
||||
|
||||
// When explicitly targeting a hidden directory, its subdirectories should be included
|
||||
const directories = result.filter((item) => item.endsWith("/"))
|
||||
|
||||
const hasTasksDir = directories.some((dir) => dir.includes(".roo-memory/tasks/") || dir.includes("tasks/"))
|
||||
const hasContextDir = directories.some(
|
||||
(dir) => dir.includes(".roo-memory/context/") || dir.includes("context/"),
|
||||
)
|
||||
|
||||
expect(hasTasksDir).toBe(true)
|
||||
expect(hasContextDir).toBe(true)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -20,5 +20,6 @@ export const DIRS_TO_IGNORE = [
|
|||
"deps",
|
||||
"pkg",
|
||||
"Pods",
|
||||
".git",
|
||||
".*",
|
||||
]
|
||||
|
|
|
|||
|
|
@ -245,8 +245,11 @@ function buildNonRecursiveArgs(): string[] {
|
|||
// Apply directory exclusions for non-recursive searches
|
||||
for (const dir of DIRS_TO_IGNORE) {
|
||||
if (dir === ".*") {
|
||||
// For hidden files/dirs in non-recursive mode
|
||||
args.push("-g", "!.*")
|
||||
// For hidden directories in non-recursive mode, we want to show the directories
|
||||
// themselves but not their contents. Since we're using --maxdepth 1, this
|
||||
// naturally happens - we just need to avoid excluding the directories entirely.
|
||||
// We'll let the directory scanning logic handle the visibility.
|
||||
continue
|
||||
} else {
|
||||
// Direct children only
|
||||
args.push("-g", `!${dir}`)
|
||||
|
|
@ -326,7 +329,11 @@ async function listFilteredDirectories(
|
|||
const absolutePath = path.resolve(dirPath)
|
||||
const directories: string[] = []
|
||||
|
||||
async function scanDirectory(currentPath: string, isTargetDir: boolean = false): Promise<void> {
|
||||
async function scanDirectory(
|
||||
currentPath: string,
|
||||
isTargetDir: boolean = false,
|
||||
insideExplicitHiddenTarget: boolean = false,
|
||||
): Promise<void> {
|
||||
try {
|
||||
// List all entries in the current directory
|
||||
const entries = await fs.promises.readdir(currentPath, { withFileTypes: true })
|
||||
|
|
@ -338,19 +345,38 @@ async function listFilteredDirectories(
|
|||
const fullDirPath = path.join(currentPath, dirName)
|
||||
|
||||
// Check if this directory should be included
|
||||
if (shouldIncludeDirectory(dirName, fullDirPath, dirPath, ignoreInstance, isTargetDir)) {
|
||||
// Subdirectories found during scanning are never target directories themselves
|
||||
if (shouldIncludeDirectory(dirName, fullDirPath, dirPath, ignoreInstance, false, insideExplicitHiddenTarget)) {
|
||||
// Add the directory to our results (with trailing slash)
|
||||
const formattedPath = fullDirPath.endsWith("/") ? fullDirPath : `${fullDirPath}/`
|
||||
directories.push(formattedPath)
|
||||
}
|
||||
|
||||
// If recursive mode and not a ignored directory, scan subdirectories
|
||||
// Don't recurse into hidden directories unless they are the explicit target
|
||||
const isHiddenDir = dirName.startsWith(".")
|
||||
const shouldRecurse =
|
||||
recursive && !isDirectoryExplicitlyIgnored(dirName) && !(isHiddenDir && !isTargetDir)
|
||||
if (shouldRecurse) {
|
||||
await scanDirectory(fullDirPath, false) // Subdirectories are not target dirs
|
||||
}
|
||||
// If recursive mode and not a ignored directory, scan subdirectories
|
||||
// Don't recurse into hidden directories unless they are the explicit target
|
||||
// or we're already inside an explicitly targeted hidden directory
|
||||
const isHiddenDir = dirName.startsWith(".")
|
||||
|
||||
// Use the same logic as shouldIncludeDirectory for recursion decisions
|
||||
// When inside an explicitly targeted hidden directory, only block critical directories
|
||||
let shouldRecurseIntoDir = true
|
||||
if (insideExplicitHiddenTarget) {
|
||||
// Only apply the most critical ignore patterns when inside explicit hidden target
|
||||
const criticalIgnorePatterns = ["node_modules", ".git", "__pycache__", "venv", "env"]
|
||||
shouldRecurseIntoDir = !criticalIgnorePatterns.includes(dirName)
|
||||
} else {
|
||||
shouldRecurseIntoDir = !isDirectoryExplicitlyIgnored(dirName)
|
||||
}
|
||||
|
||||
const shouldRecurse =
|
||||
recursive &&
|
||||
shouldRecurseIntoDir &&
|
||||
!(isHiddenDir && DIRS_TO_IGNORE.includes(".*") && !isTargetDir && !insideExplicitHiddenTarget)
|
||||
if (shouldRecurse) {
|
||||
// If we're entering a hidden directory that's the target, or we're already inside one,
|
||||
// mark that we're inside an explicitly targeted hidden directory
|
||||
const newInsideExplicitHiddenTarget = insideExplicitHiddenTarget || (isHiddenDir && isTargetDir)
|
||||
await scanDirectory(fullDirPath, false, newInsideExplicitHiddenTarget)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -360,8 +386,12 @@ async function listFilteredDirectories(
|
|||
}
|
||||
}
|
||||
|
||||
// Start scanning from the root directory - this is the explicitly targeted directory
|
||||
await scanDirectory(absolutePath, true)
|
||||
// Start scanning from the root directory
|
||||
// For environment details generation, we don't want to treat the root as a "target"
|
||||
// if we're doing a general recursive scan, as this would include hidden directories
|
||||
// Only treat as target if we're explicitly scanning a single hidden directory
|
||||
const isExplicitHiddenTarget = path.basename(absolutePath).startsWith(".")
|
||||
await scanDirectory(absolutePath, isExplicitHiddenTarget, isExplicitHiddenTarget)
|
||||
|
||||
return directories
|
||||
}
|
||||
|
|
@ -375,9 +405,10 @@ function shouldIncludeDirectory(
|
|||
basePath: string,
|
||||
ignoreInstance: ReturnType<typeof ignore>,
|
||||
isTargetDir: boolean = false,
|
||||
insideExplicitHiddenTarget: boolean = false,
|
||||
): boolean {
|
||||
// If this is the explicitly targeted directory, always include it
|
||||
// (unless it's explicitly ignored by name, not by the .* pattern)
|
||||
// If this is the explicitly targeted directory, allow it even if it's hidden
|
||||
// This preserves the ability to explicitly target hidden directories like .roo-memory
|
||||
if (isTargetDir) {
|
||||
// Only apply non-hidden-directory ignore rules to target directories
|
||||
const nonHiddenIgnorePatterns = DIRS_TO_IGNORE.filter((pattern) => pattern !== ".*")
|
||||
|
|
@ -389,16 +420,32 @@ function shouldIncludeDirectory(
|
|||
return true
|
||||
}
|
||||
|
||||
// For non-target directories (subdirectories found during traversal), apply all ignore rules
|
||||
|
||||
// Skip hidden directories if configured to ignore them
|
||||
if (dirName.startsWith(".") && DIRS_TO_IGNORE.includes(".*")) {
|
||||
return false
|
||||
// If we're inside an explicitly targeted hidden directory, allow subdirectories
|
||||
// even if they would normally be filtered out by the ".*" pattern or other ignore rules
|
||||
if (insideExplicitHiddenTarget) {
|
||||
// Only apply the most critical ignore patterns when inside explicit hidden target
|
||||
// Allow temp, rules, etc. but still block node_modules, .git, etc.
|
||||
const criticalIgnorePatterns = ["node_modules", ".git", "__pycache__", "venv", "env"]
|
||||
for (const pattern of criticalIgnorePatterns) {
|
||||
if (pattern === dirName || (pattern.includes("/") && pattern.split("/")[0] === dirName)) {
|
||||
return false
|
||||
}
|
||||
}
|
||||
// Check against gitignore patterns using the ignore library
|
||||
const relativePath = path.relative(basePath, fullDirPath)
|
||||
const normalizedPath = relativePath.replace(/\\/g, "/")
|
||||
if (ignoreInstance.ignores(normalizedPath) || ignoreInstance.ignores(normalizedPath + "/")) {
|
||||
return false
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
// Check against explicit ignore patterns
|
||||
if (isDirectoryExplicitlyIgnored(dirName)) {
|
||||
return false
|
||||
// Check against explicit ignore patterns (excluding the ".*" pattern for now)
|
||||
const nonHiddenIgnorePatterns = DIRS_TO_IGNORE.filter((pattern) => pattern !== ".*")
|
||||
for (const pattern of nonHiddenIgnorePatterns) {
|
||||
if (pattern === dirName || (pattern.includes("/") && pattern.split("/")[0] === dirName)) {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
// Check against gitignore patterns using the ignore library
|
||||
|
|
@ -424,6 +471,11 @@ function isDirectoryExplicitlyIgnored(dirName: string): boolean {
|
|||
return true
|
||||
}
|
||||
|
||||
// Skip the ".*" pattern - it's handled specially to allow top-level visibility
|
||||
if (pattern === ".*") {
|
||||
continue
|
||||
}
|
||||
|
||||
// Path patterns that contain /
|
||||
if (pattern.includes("/")) {
|
||||
const pathParts = pattern.split("/")
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue