Fix search for files starting with periods (e.g., .rooignore, .gitignore) #6510
Conversation
…nore) - Add fallback search mechanism when fzf returns no results for period-prefixed queries - Implement exact substring matching with intelligent sorting for period queries - Prioritize exact filename matches, then prefix matches, then sort by path length - Add comprehensive test suite to verify the fix works correctly Fixes #6508
dosubot
bot
added
size:L
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thank you for this contribution! The implementation successfully addresses the issue where searching for files starting with periods would return no results. The fallback mechanism is well-designed and the test coverage is comprehensive. I have a few suggestions for minor improvements that could enhance the solution further.
| // The fzf library supports exact matching with quotes, but for queries starting | ||
| // with special characters like periods, we need to handle them specially. | ||
| // The issue is that fzf may not handle leading periods well in fuzzy matching. | ||
| let fzfResults = fzf.find(query).map((result) => result.item.original) |
There was a problem hiding this comment.
Great implementation! The fallback logic is clean and only activates when needed. Consider adding a comment here explaining why this fallback is necessary (fzf's limitation with period-prefixed queries) to help future maintainers understand the rationale:
| let fzfResults = fzf.find(query).map((result) => result.item.original) | |
| // Get all matching results from fzf | |
| // The fzf library supports exact matching with quotes, but for queries starting | |
| // with special characters like periods, we need to handle them specially. | |
| // The issue is that fzf may not handle leading periods well in fuzzy matching. | |
| // This fallback ensures users can still find files like .gitignore and .rooignore | |
| let fzfResults = fzf.find(query).map((result) => result.item.original) |
| // try alternative search strategies | ||
| if (fzfResults.length === 0 && query.startsWith(".")) { | ||
| // Try exact substring matching as a fallback for period-prefixed queries | ||
| const exactMatches = allItems.filter((item) => { |
There was a problem hiding this comment.
For large workspaces with thousands of files, this fallback search could potentially be optimized. Since it only triggers for period-prefixed queries that return no fzf results, the performance impact should be minimal. However, if performance becomes a concern in the future, you might consider:
- Using a more efficient search algorithm
- Limiting the search scope
- Implementing a cache for common period-prefixed queries
Is the current performance acceptable for your use cases?
|
|
||
| // If the original query didn't return results and starts with a period, | ||
| // try alternative search strategies | ||
| if (fzfResults.length === 0 && query.startsWith(".")) { |
There was a problem hiding this comment.
Have you considered edge cases like "..." or ".."? These might need special handling to avoid unexpected behavior. You could add a check like:
| if (fzfResults.length === 0 && query.startsWith(".")) { | |
| if (fzfResults.length === 0 && query.startsWith(".") && query !== ".." && query !== "...") { |
| import { Fzf } from "fzf" | ||
|
|
||
| // Test the fallback search logic for period-prefixed queries | ||
| describe("searchWorkspaceFiles period handling", () => { |
There was a problem hiding this comment.
The tests are comprehensive and cover the important scenarios well! However, I noticed that the test duplicates the implementation logic rather than testing the actual searchWorkspaceFiles function. This approach works but has a maintenance risk - if the implementation changes, the tests might not catch regressions.
Would it be possible to refactor these tests to mock the dependencies (like executeRipgrepForFiles and fs.existsSync) and test the actual searchWorkspaceFiles function? This would ensure the tests stay in sync with the implementation and provide better coverage.
hannesrudolph
added
the
Issue/PR - Triage
|
This is related to the website, not the extension |
This PR fixes issue #6508 where searching for files starting with periods would not return results.
Problem
When users searched for files like
.rooignoreor.gitignoreusing the @ symbol file search functionality, no results were returned. This was because the fzf fuzzy search library does not handle queries starting with periods well.Solution
Changes
src/services/search/file-search.tsto add fallback logicsrc/services/search/__tests__/file-search.spec.tswith test coverageTesting
Fixes #6508
Important
Fixes search for period-prefixed files in
searchWorkspaceFiles()by adding a fallback mechanism with exact substring matching and sorting..rooignore,.gitignore) insearchWorkspaceFiles()infile-search.ts.file-search.spec.tswith tests for period-prefixed queries, covering exact matching, partial matching, and prioritization logic.This description was created by
for b287eb0. You can customize this summary. It will automatically update as commits are pushed.