Introduce new search order - System.Search.Rank - for Explorer plugin #567
Conversation
|
I have a though in #356 previously, but haven't implemented it. It is good to hear that there is an option to sort based on match! |
Yeah, I'm experimenting with this new sort for plugin Explorer, by default, seems much better. We can get the System.Search.Rank data out too if needed for debugging, just add it after SELECT. |
|
Will try out after later |
|
I think this one is good to be included in 1.9.0? Also, Rank can be used for scoring. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
Plugins/Flow.Launcher.Plugin.Explorer/Search/WindowsIndex/QueryConstructor.cs:63
- Using LIKE and CONTAINS operators on System.Search.Rank (a numeric field) may lead to unintended behavior. Please verify that filtering with text-based operators is valid for this property or if additional type conversion is required.
var queryConstraint = searchString.IsWhiteSpace() ? "" : $"AND ({OrderIdentifier} LIKE '{searchString}%' OR CONTAINS({OrderIdentifier},'\"{searchString}*\"'))";
Plugins/Flow.Launcher.Plugin.Explorer/Search/WindowsIndex/QueryConstructor.cs:86
- There is no visible test coverage for the new ordering logic using System.Search.Rank. Please consider adding tests to confirm that ordering and filtering behave as expected.
return $"{CreateBaseQuery().GenerateSQLFromUserQuery(replacedSearchString)} AND {RestrictionsForAllFilesAndFoldersSearch} ORDER BY {OrderIdentifier}";
📝 WalkthroughWalkthroughThe changes update the ordering mechanism for search result queries in the WindowsIndex module. A new constant, Changes
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Plugins/Flow.Launcher.Plugin.Explorer/Search/WindowsIndex/QueryConstructor.cs(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
🔇 Additional comments (4)
Plugins/Flow.Launcher.Plugin.Explorer/Search/WindowsIndex/QueryConstructor.cs (4)
69-69: Appropriate use of rank for ordering resultsUsing
OrderIdentifier(System.Search.Rank DESC) in the ORDER BY clause is an appropriate change that will sort results by relevance rather than file name.
86-86: Improved results ordering by relevanceUsing
System.Search.Rankfor ordering search results will provide more relevant matches first, which aligns with the PR objectives of improving search result ordering.
123-128: Well-documented constant with appropriate referenceThe
OrderIdentifierconstant is well-documented with a clear description and link to the official Microsoft documentation, which is excellent for maintainability.
136-136: Consistent use of rank-based orderingApplying the same ordering mechanism to file content searches ensures consistent behavior across different search types, which is good for user experience.
Plugins/Flow.Launcher.Plugin.Explorer/Search/WindowsIndex/QueryConstructor.cs
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Plugins/Flow.Launcher.Plugin.Explorer/Search/WindowsIndex/QueryConstructor.cs (1)
123-128: Well-documented constant with reference to Microsoft docsThe documentation for
OrderIdentifieris clear and includes a helpful link to the official Microsoft documentation.Consider adding a brief explanation of why
System.Search.Rankis preferred over filename ordering for additional clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Flow.Launcher.Test/Plugins/ExplorerTest.cs(4 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Search/WindowsIndex/QueryConstructor.cs(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: gitStream.cm
🔇 Additional comments (8)
Plugins/Flow.Launcher.Plugin.Explorer/Search/WindowsIndex/QueryConstructor.cs (4)
63-63: Fixed logical error by directly using System.FileNameThis correctly fixes the issue identified in previous reviews, where using
OrderIdentifier(System.Search.Rank) with LIKE and CONTAINS operators was problematic since it's a numeric value, not a string column.
69-69: Improved search experience with relevance-based orderingUsing
OrderIdentifierin the ORDER BY clause aligns with the PR objective of ordering search results by relevance rank instead of filename.
86-86: Consistently applied relevance-based orderingUsing
OrderIdentifierfor the FilesAndFolders method ensures a consistent approach to relevance-based ordering across all search types.
136-136: Applied relevance ranking to file content searchesUsing
OrderIdentifierfor file content searches is particularly beneficial, as it will prioritize files where the search terms are more prominent.Flow.Launcher.Test/Plugins/ExplorerTest.cs (4)
42-43: Improved test maintainability with dynamic referencesUsing string interpolation with
{QueryConstructor.OrderIdentifier}improves maintainability by centralizing the ordering mechanism definition. If the ordering approach changes in the future, these tests won't need to be updated separately.
62-62: Updated test expectations for specific item searchThis change correctly aligns the test expectations with the updated behavior in the QueryConstructor.
90-91: Updated test expectations for folders and files searchThese changes ensure the tests properly validate the new relevance-based ordering behavior.
129-129: Updated file content search test expectationsThis change completes the consistent update of all test cases to use the new relevance-based sorting mechanism.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
Plugins/Flow.Launcher.Plugin.Explorer/Search/WindowsIndex/QueryConstructor.cs:40
- Changing CreateQueryHelper to a static method might limit its ability to access instance-specific settings or state if needed in future updates. Please confirm that this change is intentional in the context of the overall design.
internal static CSearchQueryHelper CreateQueryHelper()
This comment has been minimized.
This comment has been minimized.
Jack251970
changed the title
|
🥷 Code experts: Jack251970, jjw24 Jack251970 has most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame: To learn more about /:\ gitStream - Visit our Docs |
1 similar comment
|
🥷 Code experts: Jack251970, jjw24 Jack251970 has most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame: To learn more about /:\ gitStream - Visit our Docs |
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. If the flagged items are 🤯 false positivesIf items relate to a ...
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Plugins/Flow.Launcher.Plugin.Explorer/Search/WindowsIndex/WindowsIndexSearchManager.cs (1)
86-99: These methods support the System.Search.Rank implementationThese new public methods align well with the PR objective of using System.Search.Rank for sorting search results. They provide the interface needed to leverage the updated SQL query handling in QueryConstructor where the ORDER BY clause was changed to use the new OrderIdentifier constant.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Plugins/Flow.Launcher.Plugin.Explorer/Search/WindowsIndex/QueryConstructor.cs(5 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Search/WindowsIndex/WindowsIndexSearchManager.cs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Plugins/Flow.Launcher.Plugin.Explorer/Search/WindowsIndex/QueryConstructor.cs
🧰 Additional context used
🧬 Code Graph Analysis (1)
Plugins/Flow.Launcher.Plugin.Explorer/Search/WindowsIndex/WindowsIndexSearchManager.cs (1)
Plugins/Flow.Launcher.Plugin.Explorer/Search/Constants.cs (1)
Constants(6-41)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
🔇 Additional comments (4)
Plugins/Flow.Launcher.Plugin.Explorer/Search/WindowsIndex/WindowsIndexSearchManager.cs (4)
86-89: Clean implementation of the SearchAsync wrapper methodGood addition of this public async method which provides a clean interface for the WindowsIndexFilesAndFoldersSearchAsync functionality. The method properly accepts a CancellationToken for cancellation support which is important for async operations.
91-94: Well-structured ContentSearchAsync wrapper methodThis addition provides a clean public interface for content search functionality, implementing the IContentIndexProvider interface. The parameter names are clear and descriptive with appropriate token handling.
96-99: Good implementation of the EnumerateAsync wrapper methodThis method properly implements the IPathIndexProvider interface with a clear signature. Good job passing all parameters to the underlying implementation method including the recursive flag and cancellation token.
108-110: Simplified API reference in error handling methodGood refactoring to directly use Main.Context.API instead of storing it in a local variable. This makes the code more direct while maintaining the same functionality in the exception handling.
Also applies to: 116-117
|
@pc223 Thanks for this! |
4 participants
I just found out we can use this field
System.Search.Rankto sort, seems much better than order by file name. Thought?https://docs.microsoft.com/en-us/windows/win32/properties/props-system-search-rank
original results:
current results: