fix: handle invalid gitignore patterns gracefully during codebase indexing #6882
Conversation
…exing - Add try-catch blocks to handle invalid regex patterns in .gitignore - Parse .gitignore line by line when bulk parsing fails - Skip individual invalid patterns while processing valid ones - Add comprehensive tests for various gitignore scenarios - Fixes #6881
dosubot
bot
added
size:L
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
|
|
||
| try { | ||
| // Create a new ignore instance to test each pattern | ||
| const testIgnore = ignore() |
There was a problem hiding this comment.
Creating a new ignore instance for each pattern validation could be inefficient for large .gitignore files. Could we consider reusing a single test instance or implementing a more efficient validation approach?
| } catch (ignoreError) { | ||
| // Log warning about invalid patterns but continue with indexing | ||
| console.warn( | ||
| `Warning: .gitignore contains invalid patterns that could not be parsed. Some files may not be properly ignored during indexing. Error: ${ |
There was a problem hiding this comment.
The warning message could be more actionable. Is it intentional that we don't parse out which specific pattern caused the initial bulk parsing to fail? It might help users fix their .gitignore file more easily if we could identify the problematic pattern in the first error message.
| ) | ||
|
|
||
| // Try to add individual lines to identify and skip problematic patterns | ||
| const lines = content.split("\n") |
There was a problem hiding this comment.
Would it be cleaner to extract this line-by-line parsing logic into a separate helper method? Something like:
This would improve readability and make the method easier to test in isolation.
| location: "_recreateServices", | ||
| }) | ||
| // File reading error - .gitignore might not exist or be inaccessible | ||
| console.info(".gitignore file not found or could not be read, proceeding without gitignore patterns") |
There was a problem hiding this comment.
Minor inconsistency: we're using here but for warnings above. Should we consider using a consistent logging approach or perhaps the VSCode output channel for better user visibility?
| }) | ||
| }) | ||
|
|
||
| describe("gitignore pattern handling", () => { |
There was a problem hiding this comment.
The test coverage is comprehensive! Could we also add an edge case test for .gitignore files with extremely long lines or unusual encoding? This would ensure our solution is robust against all possible user inputs.
hannesrudolph
added
the
Issue/PR - Triage
Fixes #6881
Problem
The codebase indexing feature was failing with an "Invalid regular expression" error when encountering malformed patterns in .gitignore files. Specifically, patterns like
pqh[A-/]where the character range[A-/]is invalid (since/comes beforeAin ASCII) would cause the entire indexing process to fail.Solution
This PR adds robust error handling for invalid gitignore patterns:
Changes
src/services/code-index/manager.tsTesting
Added 4 new test cases covering:
All tests pass successfully.
Important
Adds error handling for invalid .gitignore patterns in
manager.ts, allowing indexing to continue by skipping invalid patterns and logging warnings.manager.ts.manager.spec.tsfor invalid, valid, and mixed gitignore patterns._recreateServices()inmanager.ts.This description was created by
for 45b0b36. You can customize this summary. It will automatically update as commits are pushed.