-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat: add .rooindex support to override gitignore for code indexing #7878
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
Conversation
- Created RooIndexController to handle .rooindex file patterns - Updated list-files.ts to support includeGitignored parameter - Modified scanner.ts to use RooIndexController for override logic - Added comprehensive tests for RooIndexController - Allows developers to specify files that should be indexed even if gitignored This addresses the need for indexing generated code, nested repositories, and other files excluded from version control but valuable for AI context.
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.
Reviewing my own code is like debugging in a mirror - everything looks backwards but the bugs are still mine.
| // Get all files recursively (handles .gitignore automatically) | ||
| const [allPaths, _] = await listFiles(directoryPath, true, MAX_LIST_FILES_LIMIT_CODE_INDEX) | ||
| // Initialize RooIndexController to handle .rooindex overrides | ||
| const rooIndexController = new RooIndexController(directoryPath) |
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.
Is this intentional? The RooIndexController is created here but never disposed. This could lead to memory leaks as file watchers accumulate over time. Consider storing the controller instance and disposing it when the scanner is done or when the DirectoryScanner itself is disposed.
| // Initialize RooIndexController if we need to include gitignored files | ||
| let rooIndexController: RooIndexController | undefined | ||
| if (includeGitignored) { | ||
| rooIndexController = new RooIndexController(dirPath) |
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.
Could we improve resource management here? Multiple RooIndexController instances are created without disposal. Since these controllers set up file watchers, this could cause resource leaks. Consider either:
- Accepting an optional RooIndexController parameter that can be reused
- Ensuring proper disposal after use
- Implementing a singleton pattern for the controller
| } | ||
| } catch (error) { | ||
| // Should never happen: reading file failed even though it exists | ||
| console.error("Unexpected error loading .rooindex:", error) |
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 error handling here silently swallows errors and only logs them. Is this the intended behavior? If .rooindex has syntax errors or the file becomes corrupted, the indexer will silently ignore all patterns. Consider:
- Propagating the error to let callers decide how to handle it
- Emitting an event or notification about the error
- At minimum, documenting this behavior in the method's JSDoc
| private async loadRooIndex(): Promise<void> { | ||
| try { | ||
| // Reset include instance to prevent duplicate patterns | ||
| this.includeInstance = ignore() |
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.
Performance consideration: The ignore instance is recreated on every loadRooIndex call, which happens on file changes. For large .rooindex files, this could be inefficient. Consider checking if the content actually changed before recreating the instance.
| * Controls code indexer file inclusion by providing override patterns for gitignored files. | ||
| * Uses the 'ignore' library to support standard .gitignore syntax in .rooindex files. | ||
| * | ||
| * The .rooindex file allows developers to specify patterns for files that should be |
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.
Documentation suggestion: The .rooindex file format and pattern examples would be valuable to document. Consider adding a comment block here or creating a separate documentation file explaining:
- Supported pattern syntax
- Common use cases and examples
- How patterns interact with .gitignore
- Performance considerations for large pattern sets
|
The issue needs some scoping |
Summary
This PR addresses Issue #7877 by introducing a
.rooindexfile that allows developers to specify patterns for files that should be indexed by the code indexer even if they are gitignored.Problem Solved
The code indexer currently respects .gitignore patterns with no override mechanism, preventing it from accessing code that developers intentionally exclude from version control but still need for development context, such as:
Solution
Introduced a new
.rooindexconfiguration file that:Implementation Details
Testing
Example Usage
Create a
.rooindexfile in your project root:Review Confidence
The implementation review showed:
Fixes #7877
Important
Adds
.rooindexsupport to override.gitignorefor code indexing, with a newRooIndexControllerand updates to file scanning and listing processes..rooindexfile to override.gitignorefor code indexing..rooindexuses.gitignoresyntax to specify files for indexing.RooIndexControllerinRooIndexController.tsto manage.rooindexpatterns.scanner.tsto integrateRooIndexControllerfor file scanning.list-files.tsto supportincludeGitignoredparameter for.rooindex.RooIndexControllerinRooIndexController.spec.ts.This description was created by
for b1bde61. You can customize this summary. It will automatically update as commits are pushed.