-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Fix list_files returning .gitignore'd directories and optimize #5395
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
|
Thanks for working on this fix! The issue you're addressing is definitely valid - we should respect I noticed we already have For example, something like: import simpleGit from 'simple-git';
const git = simpleGit(repoRoot);
const ignoredFiles = await git.checkIgnore(filePaths);This would eliminate the need for manual directory traversal, finding the git root, and parsing multiple What do you think about this approach? It might make the code more maintainable while achieving the same goal. |
- 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.
|
@daniel-lxs I updated the code to use Overall, I think these changes + the added unit and integration tests make |
|
Hey @X9VoiD, really appreciate your contribution! It looks like the fix in #5394 ends up covering the same issue and is a bit more aligned with how we want to handle it. Sorry, that's my bad for now checking that PR before asking for changes on yours - thanks again for taking the time to work on this! |
Related GitHub Issue
Closes: #5393
Roo Code Task Context (Optional)
Description
The new implementation:
Test Procedure
list-files.spec.ts have been updated to properly test
Pre-Submission Checklist
Screenshots / Videos
Documentation Updates
Additional Notes
Get in Touch
Important
Fix and optimize
listFilesto respect.gitignoreusinggit ls-filesand add comprehensive tests.listFilesnow usesgit ls-filesfor git repos, respecting.gitignore.ignorepackage.list-files.integration.spec.tsfor nested.gitignorescenarios.list-files.spec.tsfor edge cases and special directories.list-files.tsto improve performance and maintainability.This description was created by
for 4273b04. You can customize this summary. It will automatically update as commits are pushed.