-
Notifications
You must be signed in to change notification settings - Fork 7.6k
CmdPal: Make Indexer great again - part 1 - hotfix #44729
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
base: main
Are you sure you want to change the base?
Conversation
- Add search cancellation to the file search page and the indexer fallback. - Stop reusing the search engine; create a new instance per search to avoid synchronization issues. - Remove search priming to simplify the code and improve performance. - Fix the subject line not updating in some branches of the indexer fallback flow. - Show the number of matched files in the fallback result. - Fix the English keyword "kind" being hardcoded in the search page filter. - Optimize the indexer fallback by reducing the number of items processed but not used. - Adds some extra diagnostics to SearchQuery and makes logging more precise.
src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Indexer/SearchQuery.cs
Fixed
Show fixed
Hide fixed
src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Indexer/SearchQuery.cs
Fixed
Show fixed
Hide fixed
This comment has been minimized.
This comment has been minimized.
src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/FallbackOpenFileItem.cs
Outdated
Show resolved
Hide resolved
src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/FallbackOpenFileItem.cs
Outdated
Show resolved
Hide resolved
- Stop reading total count from the rowset and remove exact count from the UI - Add fine-grained locking to the indexer fallback - Fix icon blinking
This comment has been minimized.
This comment has been minimized.
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.
Pull request overview
This PR implements a hotfix for several indexer-related issues in the CmdPal file search functionality. The changes focus on improving search cancellation, thread-safety, and performance by creating new SearchEngine instances per search rather than reusing them, removing search priming logic, and optimizing the fallback behavior.
Changes:
- Adds search cancellation support using CancellationTokenSource to prevent stale results from overwriting newer searches
- Replaces SearchEngine reuse with per-search instantiation to avoid multi-threading issues
- Removes search priming logic from QueryStringBuilder and SearchQuery to simplify code and improve performance
- Fixes fallback subject line to update correctly when multiple matches are found
- Optimizes DataPackage creation by deferring storage item loading until needed
- Fixes hardcoded English "kind" mnemonic keyword by using canonical "System.Kind" property name
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| SearchEngine.cs | Adds nullable support, returns int from Query (though always 0), adds noIcons parameter to FetchItems, improves disposal pattern |
| Resources.resx | Adds new localized string for multiple results subtitle |
| Resources.Designer.cs | Auto-generated code updated to version 18.0.0.0 |
| IndexerPage.cs | Implements cancellation with CancellationTokenSource, adds Lock-based synchronization, recreates SearchEngine per search, implements deferred loading, fixes System.Kind filter keywords |
| QueryStringBuilder.cs | Removes priming query generation and ReuseWhere functionality, simplifies to static class |
| SearchQuery.cs | Adds QueryState tracking, removes ReuseWhere mechanism, improves diagnostics and error handling, adds ALLNOISE constant |
| DataPackageHelper.cs | Defers storage item loading using SetDataProvider for lazy evaluation |
| FallbackOpenFileItem.cs | Adds cancellation support, implements async icon loading, adds result locking, creates new SearchEngine per query |
| expect.txt | Adds ALLNOISE to spell-check dictionary |
Files not reviewed (1)
- src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Properties/Resources.Designer.cs: Language not supported
| OnPropertyChanged(nameof(EmptyContent)); | ||
| } | ||
| }, | ||
| ct); |
Copilot
AI
Jan 20, 2026
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.
Unobserved exceptions from Task.Run can cause application crashes. The fire-and-forget pattern with Task.Run should handle exceptions explicitly. Consider wrapping the task body in a try-catch block or using await with proper exception handling.
| ct); | |
| ct).ContinueWith( | |
| t => | |
| { | |
| _ = t.Exception; | |
| }, | |
| TaskContinuationOptions.OnlyOnFaulted); |
src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Pages/IndexerPage.cs
Show resolved
Hide resolved
src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Helpers/DataPackageHelper.cs
Show resolved
Hide resolved
src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/FallbackOpenFileItem.cs
Show resolved
Hide resolved
src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/FallbackOpenFileItem.cs
Show resolved
Hide resolved
src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/FallbackOpenFileItem.cs
Show resolved
Hide resolved
| { | ||
| var pQueryHelper = (SearchQuery)state; | ||
| pQueryHelper.ExecuteSyncInternal(); | ||
| return 0; |
Copilot
AI
Jan 20, 2026
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 Execute method returns 0 unconditionally, making the return value meaningless. If the method signature was changed to return int to provide total results count, it should return the actual count. Otherwise, consider changing the return type to void.
| return 0; | |
| return SearchResults.Count; |
Summary of the Pull Request
This PR introduces a rough hotfix for several indexer-related issues.
SeachEngineandSearchQueryis slightly trickier). This patch also removes some dead code for future refactor.SeachEngineandSearchQueryare not multi-threading friendly.SearchQuerycancels and re-primes on every search, priming provides little benefit and can hide extra work (for example, cancellation triggering re-priming).Shows the number of matched files in the fallback result.kindbeing hardcoded in the search page filter. Windows Search uses localized mnemonic keyword names, so this PR replaces it with canonical keywordSystem.Kindthat is universaly recognized.SearchQueryand makes logging more precise.PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed