-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Fix/issue 7921 search files includes files ignored by nested gitignore #8445
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 11 commits
1694bcd
5c2a3e8
7c4635f
d74a9b1
8dcddb4
1ea63af
80ea42d
1a9e264
8031ae1
0b34253
87c0b5d
59193a1
dd62a55
6252c09
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,122 @@ | ||||||
| import path from "path" | ||||||
| import fsSync from "fs" | ||||||
| import ignore, { Ignore } from "ignore" | ||||||
| import * as vscode from "vscode" | ||||||
|
|
||||||
| /** | ||||||
| * Base class for ignore controllers that provides common functionality | ||||||
| * for handling ignore patterns and file validation. | ||||||
| */ | ||||||
| export abstract class BaseIgnoreController { | ||||||
| protected cwd: string | ||||||
| protected ignoreInstance: Ignore | ||||||
| protected disposables: vscode.Disposable[] = [] | ||||||
|
|
||||||
| constructor(cwd: string) { | ||||||
| this.cwd = cwd | ||||||
| this.ignoreInstance = ignore() | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Initialize the controller - must be implemented by subclasses | ||||||
| */ | ||||||
| abstract initialize(): Promise<void> | ||||||
|
|
||||||
| /** | ||||||
| * Check if a file should be accessible (not ignored by patterns) | ||||||
| * Automatically resolves symlinks | ||||||
| * @param filePath - Path to check (relative to cwd) | ||||||
| * @returns true if file is accessible, false if ignored | ||||||
| */ | ||||||
| validateAccess(filePath: string): boolean { | ||||||
| // Allow subclasses to override the "no patterns" check | ||||||
| if (!this.hasPatterns()) { | ||||||
| return true | ||||||
| } | ||||||
|
|
||||||
| try { | ||||||
| const absolutePath = path.resolve(this.cwd, filePath) | ||||||
|
|
||||||
| // Follow symlinks to get the real path | ||||||
| let realPath: string | ||||||
| try { | ||||||
| realPath = fsSync.realpathSync(absolutePath) | ||||||
| } catch { | ||||||
| // If realpath fails (file doesn't exist, broken symlink, etc.), | ||||||
| // use the original path | ||||||
| realPath = absolutePath | ||||||
| } | ||||||
|
|
||||||
| // Convert real path to relative for ignore checking | ||||||
| const relativePath = path.relative(this.cwd, realPath).toPosix() | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P0] String.prototype.toPosix() does not exist and will throw at runtime. Normalize separators explicitly before passing to the ignore matcher.
Suggested change
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we tested this and it's already in prod in the rooIgnoreController line 108 |
||||||
|
|
||||||
| // Check if the real path is ignored | ||||||
| return !this.ignoreInstance.ignores(relativePath) | ||||||
| } catch (error) { | ||||||
| // Allow access to files outside cwd or on errors (backward compatibility) | ||||||
| return true | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Filter an array of paths, removing those that should be ignored | ||||||
| * @param paths - Array of paths to filter (relative to cwd) | ||||||
| * @returns Array of allowed paths | ||||||
| */ | ||||||
| filterPaths(paths: string[]): string[] { | ||||||
| try { | ||||||
| return paths | ||||||
| .map((p) => ({ | ||||||
| path: p, | ||||||
| allowed: this.validateAccess(p), | ||||||
| })) | ||||||
| .filter((x) => x.allowed) | ||||||
| .map((x) => x.path) | ||||||
| } catch (error) { | ||||||
| console.error("Error filtering paths:", error) | ||||||
| return [] // Fail closed for security | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Clean up resources when the controller is no longer needed | ||||||
| */ | ||||||
| dispose(): void { | ||||||
| this.disposables.forEach((d) => d.dispose()) | ||||||
| this.disposables = [] | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Check if the controller has any patterns loaded | ||||||
| * Must be implemented by subclasses | ||||||
| */ | ||||||
| protected abstract hasPatterns(): boolean | ||||||
|
|
||||||
| /** | ||||||
| * Set up file watchers with debouncing to avoid rapid reloads | ||||||
| * @param pattern - VSCode RelativePattern for the files to watch | ||||||
| * @param reloadCallback - Function to call when files change | ||||||
| */ | ||||||
| protected setupFileWatcher(pattern: vscode.RelativePattern, reloadCallback: () => void): void { | ||||||
| const fileWatcher = vscode.workspace.createFileSystemWatcher(pattern) | ||||||
|
|
||||||
| // Debounce rapid changes | ||||||
| let reloadTimeout: NodeJS.Timeout | undefined | ||||||
| const debouncedReload = () => { | ||||||
| if (reloadTimeout) { | ||||||
| clearTimeout(reloadTimeout) | ||||||
| } | ||||||
| reloadTimeout = setTimeout(reloadCallback, 100) | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P3] Debounced reload can fire after dispose, leading to callbacks running on disposed state. Consider storing the timeout on the instance and clearing it in dispose(), or guarding reloadCallback when disposed. |
||||||
| } | ||||||
|
|
||||||
| // Watch for changes, creation, and deletion | ||||||
| this.disposables.push( | ||||||
| fileWatcher.onDidChange(debouncedReload), | ||||||
| fileWatcher.onDidCreate(debouncedReload), | ||||||
| fileWatcher.onDidDelete(debouncedReload), | ||||||
| ) | ||||||
|
|
||||||
| // Add fileWatcher itself to disposables | ||||||
| this.disposables.push(fileWatcher) | ||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,230 @@ | ||||||||||||||||||||||||||||||||||||
| import path from "path" | ||||||||||||||||||||||||||||||||||||
| import { fileExistsAtPath } from "../../utils/fs" | ||||||||||||||||||||||||||||||||||||
| import fs from "fs/promises" | ||||||||||||||||||||||||||||||||||||
| import ignore from "ignore" | ||||||||||||||||||||||||||||||||||||
| import * as vscode from "vscode" | ||||||||||||||||||||||||||||||||||||
| import { BaseIgnoreController } from "./BaseIgnoreController" | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||
| * Controls file access by enforcing nested .gitignore patterns. | ||||||||||||||||||||||||||||||||||||
| * Handles multiple .gitignore files throughout the directory tree, unlike ripgrep which only honors top-level .gitignore. | ||||||||||||||||||||||||||||||||||||
| * Designed to be instantiated once and passed to file manipulation services. | ||||||||||||||||||||||||||||||||||||
| * Uses the 'ignore' library to support standard .gitignore syntax. | ||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||
| export class GitIgnoreController extends BaseIgnoreController { | ||||||||||||||||||||||||||||||||||||
| private gitignoreFiles: string[] = [] | ||||||||||||||||||||||||||||||||||||
| private gitignoreContents: Map<string, string> = new Map() | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| constructor(cwd: string) { | ||||||||||||||||||||||||||||||||||||
| super(cwd) | ||||||||||||||||||||||||||||||||||||
| this.gitignoreFiles = [] | ||||||||||||||||||||||||||||||||||||
| this.gitignoreContents = new Map() | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||
| * Initialize the controller by discovering and loading all .gitignore files | ||||||||||||||||||||||||||||||||||||
| * Must be called after construction and before using the controller | ||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||
| async initialize(): Promise<void> { | ||||||||||||||||||||||||||||||||||||
| await this.discoverAndLoadGitignoreFiles() | ||||||||||||||||||||||||||||||||||||
| this.setupGitIgnoreWatchers() | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||
| * Discover and load .gitignore files (root + common subdirectories) | ||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||
| private async discoverAndLoadGitignoreFiles(): Promise<void> { | ||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||
| // Reset state | ||||||||||||||||||||||||||||||||||||
| this.ignoreInstance = ignore() | ||||||||||||||||||||||||||||||||||||
| this.gitignoreFiles = [] | ||||||||||||||||||||||||||||||||||||
| this.gitignoreContents.clear() | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // Check for common .gitignore file locations (manually defined for simplicity) | ||||||||||||||||||||||||||||||||||||
| const commonGitignorePaths = [ | ||||||||||||||||||||||||||||||||||||
| path.join(this.cwd, ".gitignore"), // Root | ||||||||||||||||||||||||||||||||||||
| path.join(this.cwd, "src", ".gitignore"), // src/ | ||||||||||||||||||||||||||||||||||||
| path.join(this.cwd, "lib", ".gitignore"), // lib/ | ||||||||||||||||||||||||||||||||||||
| path.join(this.cwd, "test", ".gitignore"), // test/ | ||||||||||||||||||||||||||||||||||||
| path.join(this.cwd, "tests", ".gitignore"), // tests/ | ||||||||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // Check each location and load if it exists | ||||||||||||||||||||||||||||||||||||
| for (const gitignorePath of commonGitignorePaths) { | ||||||||||||||||||||||||||||||||||||
| const exists = await fileExistsAtPath(gitignorePath) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| if (exists) { | ||||||||||||||||||||||||||||||||||||
| this.gitignoreFiles.push(gitignorePath) | ||||||||||||||||||||||||||||||||||||
| await this.loadGitignoreFile(gitignorePath) | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // Also discover arbitrary nested .gitignore files across the workspace | ||||||||||||||||||||||||||||||||||||
| await this.findGitignoreFilesRecursively(this.cwd) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // Load any files discovered by recursion that weren't loaded yet | ||||||||||||||||||||||||||||||||||||
| for (const p of this.gitignoreFiles) { | ||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] Potential duplicates in gitignoreFiles if a path is discovered via both the predefined list and recursion. De-duplicate before loading to avoid redundant add() calls and reduce churn.
Suggested change
|
||||||||||||||||||||||||||||||||||||
| if (!this.gitignoreContents.has(p)) { | ||||||||||||||||||||||||||||||||||||
| await this.loadGitignoreFile(p) | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // Always ignore .gitignore files themselves | ||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] Recursive discovery is never used, so nested
Suggested change
|
||||||||||||||||||||||||||||||||||||
| this.ignoreInstance.add(".gitignore") | ||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] Only the root-level .gitignore is ignored; nested .gitignore files are not. Use a glob to ignore at all depths so .gitignore files themselves are consistently excluded throughout the tree.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Only the root .gitignore is excluded from results with this pattern. Nested .gitignore files will still be considered unless we also ignore '**/.gitignore'. This can surface .gitignore files from subdirectories in search results.
Suggested change
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this important? idk if this matters. Sometimes I want roo to edit .gitignore files |
||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||
| console.error("Error discovering .gitignore files:", error) | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||
| * Recursively find all .gitignore files in the directory tree | ||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||
| private async findGitignoreFilesRecursively(dirPath: string): Promise<void> { | ||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||
| // Skip the root directory since we already checked it in discoverAndLoadGitignoreFiles | ||||||||||||||||||||||||||||||||||||
| if (dirPath === this.cwd) { | ||||||||||||||||||||||||||||||||||||
| // Get all subdirectories | ||||||||||||||||||||||||||||||||||||
| const entries = await fs.readdir(dirPath, { withFileTypes: true }) | ||||||||||||||||||||||||||||||||||||
| const subdirs = entries | ||||||||||||||||||||||||||||||||||||
| .filter((entry) => entry.isDirectory() && !entry.name.startsWith(".")) | ||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Hidden directories are skipped during discovery. This prevents picking up
Suggested change
|
||||||||||||||||||||||||||||||||||||
| .map((entry) => path.join(dirPath, entry.name)) | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+87
to
+90
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Recursive discovery traverses large directories like node_modules, dist, build, etc., which can severely impact performance on large workspaces. Consider skipping common heavy directories during traversal.
Suggested change
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // Recursively search subdirectories | ||||||||||||||||||||||||||||||||||||
| for (const subdir of subdirs) { | ||||||||||||||||||||||||||||||||||||
| await this.findGitignoreFilesRecursively(subdir) | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||
| // For subdirectories, check for .gitignore and continue recursively | ||||||||||||||||||||||||||||||||||||
| const gitignorePath = path.join(dirPath, ".gitignore") | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // Check if .gitignore exists in current directory | ||||||||||||||||||||||||||||||||||||
| if (await fileExistsAtPath(gitignorePath)) { | ||||||||||||||||||||||||||||||||||||
| this.gitignoreFiles.push(gitignorePath) | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // Get all subdirectories | ||||||||||||||||||||||||||||||||||||
| const entries = await fs.readdir(dirPath, { withFileTypes: true }) | ||||||||||||||||||||||||||||||||||||
| const subdirs = entries | ||||||||||||||||||||||||||||||||||||
| .filter((entry) => entry.isDirectory() && !entry.name.startsWith(".")) | ||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Same discovery issue in the recursive branch: skipping dot-directories can miss nested
Suggested change
|
||||||||||||||||||||||||||||||||||||
| .map((entry) => path.join(dirPath, entry.name)) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // Recursively search subdirectories | ||||||||||||||||||||||||||||||||||||
| for (const subdir of subdirs) { | ||||||||||||||||||||||||||||||||||||
| await this.findGitignoreFilesRecursively(subdir) | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||
| // Skip directories we can't read | ||||||||||||||||||||||||||||||||||||
| console.debug(`Could not read directory ${dirPath}:`, error) | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||
| * Load content from a specific .gitignore file | ||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||
| private async loadGitignoreFile(gitignoreFile: string): Promise<void> { | ||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||
| const content = await fs.readFile(gitignoreFile, "utf8") | ||||||||||||||||||||||||||||||||||||
| this.gitignoreContents.set(gitignoreFile, content) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // Add patterns to ignore instance with proper context | ||||||||||||||||||||||||||||||||||||
| // For nested .gitignore files, we need to adjust patterns relative to the workspace root | ||||||||||||||||||||||||||||||||||||
| const relativeDir = path.relative(this.cwd, path.dirname(gitignoreFile)) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| if (relativeDir) { | ||||||||||||||||||||||||||||||||||||
| // For nested .gitignore files, we need to create patterns that match files within that directory | ||||||||||||||||||||||||||||||||||||
| const lines = content.split(/\r?\n/).filter((line) => line.trim() && !line.startsWith("#")) | ||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] Escaped literals are misparsed. Lines beginning with an escaped '#' (e.g., '#foo') are treated as comments and dropped; and lines beginning with an escaped '!' (e.g., '!keep.txt') are treated as negations. Gitignore semantics: '#' and '!' at the start denote literal '#' and '!' patterns. Parse comments only when the first non-whitespace char is an unescaped '#', and unescape a leading '#' or '!' before further processing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Comment detection trims content but checks startsWith on the untrimmed line. Lines like ' # comment' will be treated as patterns and then fed to ignore(), which is not intended and can create bogus entries. Use the trimmed string for the comment check (escaped # still passes through as intended).
Suggested change
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @roomote-agent we need test coverage for this at this point. Can you confirm that the tests don't cover this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: Comment detection uses
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] Escaped '#' patterns (\#) are valid gitignore entries (literal '#') and should not be dropped as comments. The current filter removes them.
Suggested change
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is test coverage for this and they all pass. |
||||||||||||||||||||||||||||||||||||
| // Convert Windows paths to POSIX for consistent pattern matching | ||||||||||||||||||||||||||||||||||||
| const normalizedRelativeDir = relativeDir.split(path.sep).join("/") | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const adjustedPatterns = lines.flatMap((pattern) => { | ||||||||||||||||||||||||||||||||||||
| const trimmed = pattern.trim() | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| if (trimmed.startsWith("/")) { | ||||||||||||||||||||||||||||||||||||
| // Absolute patterns (starting with /) are relative to the .gitignore location | ||||||||||||||||||||||||||||||||||||
| return [normalizedRelativeDir + trimmed] | ||||||||||||||||||||||||||||||||||||
| } else if (trimmed.startsWith("!")) { | ||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] Negation handling doesn't account for escaped '!'. A pattern like '!keep.txt' should be treated as a literal starting with '!', not a re-include. Add a guard before the negation branch to detect and strip a leading backslash before '!' so it is treated as a literal pattern. |
||||||||||||||||||||||||||||||||||||
| // Negation patterns | ||||||||||||||||||||||||||||||||||||
| const negatedPattern = trimmed.slice(1) | ||||||||||||||||||||||||||||||||||||
| if (negatedPattern.startsWith("/")) { | ||||||||||||||||||||||||||||||||||||
| return ["!" + normalizedRelativeDir + negatedPattern] | ||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||
| // For relative negation patterns, match in the directory and subdirectories | ||||||||||||||||||||||||||||||||||||
| return [ | ||||||||||||||||||||||||||||||||||||
| "!" + normalizedRelativeDir + "/" + negatedPattern, | ||||||||||||||||||||||||||||||||||||
| "!" + normalizedRelativeDir + "/**/" + negatedPattern, | ||||||||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||
| // Relative patterns - match files in the directory and all subdirectories | ||||||||||||||||||||||||||||||||||||
| // For "*.tmp" in src/.gitignore, we need TWO patterns: | ||||||||||||||||||||||||||||||||||||
| // - src/*.tmp (matches direct children like src/temp.tmp) | ||||||||||||||||||||||||||||||||||||
| // - src/**/*.tmp (matches descendants like src/subdir/temp.tmp) | ||||||||||||||||||||||||||||||||||||
| const patterns = [ | ||||||||||||||||||||||||||||||||||||
| normalizedRelativeDir + "/" + trimmed, | ||||||||||||||||||||||||||||||||||||
| normalizedRelativeDir + "/**/" + trimmed, | ||||||||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||||||||
| return patterns | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| this.ignoreInstance.add(adjustedPatterns) | ||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||
| // Root .gitignore file - add patterns as-is (like RooIgnoreController) | ||||||||||||||||||||||||||||||||||||
| this.ignoreInstance.add(content) | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||
| console.warn(`Could not read .gitignore at ${gitignoreFile}:`, error) | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||
| * Set up file watchers for all .gitignore files in the workspace | ||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||
| private setupGitIgnoreWatchers(): void { | ||||||||||||||||||||||||||||||||||||
| // Create a watcher for .gitignore files throughout the workspace | ||||||||||||||||||||||||||||||||||||
| const gitignorePattern = new vscode.RelativePattern(this.cwd, "**/.gitignore") | ||||||||||||||||||||||||||||||||||||
| this.setupFileWatcher(gitignorePattern, () => this.discoverAndLoadGitignoreFiles()) | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||
| * Check if the controller has any patterns loaded | ||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||
| protected hasPatterns(): boolean { | ||||||||||||||||||||||||||||||||||||
| return this.gitignoreFiles.length > 0 | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||
| * Get all discovered .gitignore file paths | ||||||||||||||||||||||||||||||||||||
| * @returns Array of absolute paths to .gitignore files | ||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||
| getGitignoreFiles(): string[] { | ||||||||||||||||||||||||||||||||||||
| return [...this.gitignoreFiles] | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||
| * Get the content of a specific .gitignore file | ||||||||||||||||||||||||||||||||||||
| * @param gitignoreFile - Absolute path to the .gitignore file | ||||||||||||||||||||||||||||||||||||
| * @returns Content of the file or undefined if not found | ||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||
| getGitignoreContent(gitignoreFile: string): string | undefined { | ||||||||||||||||||||||||||||||||||||
| return this.gitignoreContents.get(gitignoreFile) | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||
| * Check if any .gitignore files exist in the workspace | ||||||||||||||||||||||||||||||||||||
| * @returns true if at least one .gitignore file exists | ||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||
| hasGitignoreFiles(): boolean { | ||||||||||||||||||||||||||||||||||||
| return this.gitignoreFiles.length > 0 | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||
| * Clean up resources when the controller is no longer needed | ||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||
| override dispose(): void { | ||||||||||||||||||||||||||||||||||||
| super.dispose() | ||||||||||||||||||||||||||||||||||||
| this.gitignoreContents.clear() | ||||||||||||||||||||||||||||||||||||
| this.gitignoreFiles = [] | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The expression
path.relative(this.cwd, realPath).toPosix()uses a non-standard method. Consider using a standard alternative (e.g.path.posix.relative(this.cwd, realPath)or replacing backslashes) to ensure cross-platform consistency.