Support Querying Results When Query Text is Empty #3399
Conversation
This comment has been minimized.
This comment has been minimized.
Jack251970
added
enhancement
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
Flow.Launcher/ViewModel/MainViewModel.cs (1)
55-59: Consider using a generated GUID instead of hardcoded IDThe hardcoded ID for history metadata might be confusing for future maintainers. While it's only used internally for result grouping rather than permanent storage, consider using
Guid.NewGuid().ToString("N").ToUpper()to generate this ID at runtime.
🧹 Nitpick comments (1)
Flow.Launcher/ViewModel/MainViewModel.cs (1)
1449-1465: Consider adding cancellation check earlier in QueryHistoryTaskThe method checks for cancellation after retrieving and processing history items, but it would be more efficient to check before starting this work.
void QueryHistoryTask() { + if (_updateSource.Token.IsCancellationRequested) return; + // Select last history results and revert its order to make sure last history results are on top var historyItems = _history.Items.TakeLast(Settings.MaxHistoryResultsToShowForHomePage).Reverse(); var results = GetHistoryItems(historyItems); if (_updateSource.Token.IsCancellationRequested) return; App.API.LogDebug(ClassName, $"Update results for history"); if (!_resultsUpdateChannelWriter.TryWrite(new ResultsForUpdate(results, _historyMetadata, query, _updateSource.Token))) { App.API.LogError(ClassName, "Unable to add item to Result Update Queue"); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher/ViewModel/MainViewModel.cs(14 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (13)
Flow.Launcher/ViewModel/MainViewModel.cs (13)
36-36: Enhances query state tracking for home page featureThe addition of a
_lastIsHomeQueryfield enables tracking whether the previous query was a home page query (empty text), which is essential for proper state management when toggling between normal searches and the home page.
1088-1091: Good addition of a simple QueryResults methodThis is a helpful public interface that wraps the async method without requiring callers to deal with Task management.
1149-1162: Good handling of history items in context menuThe logic appropriately handles history items from the home page that don't have a plugin ID. This ensures users can still use context menu actions like pinning items to the top even for history results.
1196-1211: Improved history query implementationThe refactored history query now properly separates concerns by extracting result creation to a dedicated method, making the code more maintainable and reusable for both normal history queries and home page history display.
1213-1241: Well-structured history item conversion methodThis helper method creates properly configured Result objects from history items, with appropriate titles, subtitles, icons, and actions. The extracted method follows the single responsibility principle and enables reuse for both normal history and home page display.
1271-1272: Key home query detection logicThis identifies queries with empty text as "home queries", enabling the core functionality of this PR.
1290-1291: Tracking home query state for result managementStoring the home query state is essential for proper result management, especially when deciding whether to clear results during query transitions.
1292-1320: Well-structured plugin selection for home vs. standard queriesThis code cleanly separates the plugin selection logic for home queries vs. standard queries, checking the appropriate settings and updating UI elements accordingly.
1349-1371: Effective task creation with home query supportThe task creation logic now properly respects both the normal plugin disabled state and the new home-specific disabled state, ensuring users can control which plugins appear on the home page independently from regular search.
1400-1401: Performance optimization for home queriesSkipping the search delay for home queries is a good optimization that ensures the home page results appear immediately when Flow Launcher is opened or the query is cleared.
1413-1415: Clean separation of home vs. standard query executionUsing the conditional operator to select the appropriate query method keeps the code concise while clearly separating the two query paths.
1472-1474: Enhanced query construction for empty inputThis modification ensures that empty input queries are properly constructed and identified as home queries, which is essential for the home page functionality.
1545-1552: Improved result clearing logic with home query supportThe enhanced logic now properly handles transitions between home queries and regular queries, ensuring results are cleared appropriately when switching between these modes.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
Flow.Launcher/ViewModel/MainViewModel.cs (3)
55-59: Hardcoded GUID for _historyMetadata may need explanationThe implementation uses a hardcoded GUID for the history metadata ID. While this works as history is a static concept (not a real plugin), adding a comment explaining why this specific ID is used would help future maintainers understand the purpose.
private readonly PluginMetadata _historyMetadata = new() { - ID = "298303A65D128A845D28A7B83B3968C2", // ID is for ResultsForUpdate constructor + ID = "298303A65D128A845D28A7B83B3968C2", // Fixed ID for history results on home page to enable proper identification in ResultsForUpdate Priority = 0 // Priority is for calculating scores in UpdateResultView };
1350-1356: Consider filtering disabled plugins before creating tasks arrayThe current implementation includes all plugins in the tasks array, but uses Task.CompletedTask for disabled ones. This creates unnecessary elements in the array.
- tasks = plugins.Select(plugin => plugin.Metadata.HomeDisabled switch - { - false => QueryTaskAsync(plugin, _updateSource.Token), - true => Task.CompletedTask - }).ToArray(); + tasks = plugins + .Where(plugin => !plugin.Metadata.HomeDisabled) + .Select(plugin => QueryTaskAsync(plugin, _updateSource.Token)) + .ToArray();
1366-1371: Consider filtering disabled plugins before creating tasks arraySimilar to the home query case, the current implementation includes all plugins in the tasks array, but uses Task.CompletedTask for disabled ones.
- tasks = plugins.Select(plugin => plugin.Metadata.Disabled switch - { - false => QueryTaskAsync(plugin, _updateSource.Token), - true => Task.CompletedTask - }).ToArray(); + tasks = plugins + .Where(plugin => !plugin.Metadata.Disabled) + .Select(plugin => QueryTaskAsync(plugin, _updateSource.Token)) + .ToArray();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher/ViewModel/MainViewModel.cs(14 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (11)
Flow.Launcher/ViewModel/MainViewModel.cs (11)
1088-1091: Good addition of QueryResults methodThis public method provides a way to externally trigger query results refresh, which is useful when home page settings change. This improves the reactivity of the UI.
1149-1162: Appropriate special handling for history items on home pageThe code correctly differentiates between history items (with null PluginID) and regular plugin results when constructing the context menu, showing only the TopMost option for history items.
1213-1241: Good refactoring of history item conversion logicExtracting the history item conversion logic into a separate method eliminates code duplication and allows reuse by both the history view and the home page history query.
1271-1272: Clear detection of home query stateUsing an empty query raw text as the identifier for a home query is simple and effective.
1292-1320: Plugin selection and icon handling logic is well-structuredThe code appropriately selects plugins based on whether it's a home query or a normal query, and handles the plugin icon display logic accordingly. For home queries, it clears the icon and shows the search icon, while for normal queries with exactly one plugin, it shows that plugin's icon.
1399-1401: Efficient handling of search delay for home queriesThe code correctly skips the search delay for home queries, which should make the home page appear more quickly.
1413-1415: Clear separation of query methods for home and regular queriesUsing different query methods (
QueryHomeForPluginAsyncvsQueryForPluginAsync) for home and regular queries provides a clean separation of concerns.
1449-1465: Well-structured history query for home pageThe implementation correctly selects the most recent history items (limited by settings), reverses their order to show the most recent first, and adds them to the results update channel with the history metadata.
1545-1559: Appropriate result clearing logic for home queriesThe updated method correctly clears results if either the last or current query is a home query, ensuring that switching between home and normal queries refreshes the results appropriately.
1472-1474: Proper handling of empty query textThe code correctly handles empty queries by building a query object with an empty string and non-global plugins, ensuring that a valid query object is returned.
1743-1743: Good defensive programming with null checkAdding a null check before accessing
_lastQuery?.ActionKeywordprevents potential null reference exceptions.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Flow.Launcher/ViewModel/MainViewModel.cs (1)
1452-1453: Consider adding a check for empty historyThe code assumes history items exist when querying them. Consider adding a check to handle the case when there are no history items.
- var historyItems = _history.Items.TakeLast(Settings.MaxHistoryResultsToShowForHomePage).Reverse(); + var historyItems = _history.Items?.TakeLast(Settings.MaxHistoryResultsToShowForHomePage).Reverse() ?? Enumerable.Empty<HistoryItem>();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher/ViewModel/MainViewModel.cs(14 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (16)
Flow.Launcher/ViewModel/MainViewModel.cs (16)
55-59: Good use of fixed ID with clear documentationThe implementation of
_historyMetadatawith a hardcoded ID is appropriate since history items need a consistent identifier, and your comments clearly explain the purpose of both the ID and priority fields.
36-36: LGTM: Added flag to track home query stateThe
_lastIsHomeQueryboolean field effectively tracks whether the last query was a home query, which is needed for the new home page functionality.
1088-1091: LGTM: Simple public method added for query refreshingThis wrapper method provides a convenient way to asynchronously refresh results without requiring the caller to handle the returned task.
1149-1162: Good handling of history items in context menuThe modified context menu logic correctly differentiates between regular results and history items from the home page, providing appropriate context menu items for each case.
1196-1241: Good refactoring of history item processingExtracting the
GetHistoryItemsmethod is a clean approach that allows reuse by both theQueryHistoryandQueryHistoryTaskmethods, promoting code reuse and maintainability.
1271-1272: LGTM: Clear identification of home queryThe simple definition of a home query (empty raw query) is straightforward and aligns well with the feature's purpose.
1290-1291: LGTM: Preserving home query stateStoring the current home query state ensures proper handling of results in subsequent queries.
1292-1321: Comprehensive handling of plugin selection based on query typeThe code appropriately differentiates between home query and regular query, selecting the right plugins for each case and managing plugin icons accordingly.
1349-1371: Well-structured tasks generation for different query typesThe code correctly handles both home queries (respecting home disabled state) and regular queries (respecting disabled state), ensuring only appropriate plugins are queried.
1400-1401: Good optimization by skipping delay for home queriesRemoving the search delay for home queries improves user experience by making home page results appear immediately.
1413-1415: Clean plugin query selection based on query typeThe ternary operator elegantly selects between home query and regular query plugin methods.
1449-1465: LGTM: Dedicated task for history results on home pageThe
QueryHistoryTaskmethod efficiently handles adding history results to the home page, prioritizing them by ensuring they're queried first.
1473-1473: LGTM: Handling empty query text appropriatelyThe code correctly builds a query with an empty string for the home page scenario.
1545-1559: Clear logic for clearing results between different query typesThe updated
RemoveOldQueryResultsmethod correctly clears results when switching between home queries and regular queries, ensuring clean transitions.
1743-1744: Fixed potential null reference issueThe null-conditional operator on
_lastQuery?.ActionKeywordprevents a potential null reference exception when hiding the launcher.
1358-1362: Verify history results visibility conditionsThe history results are shown only when both
Settings.ShowHomePageandSettings.ShowHistoryResultsForHomePageare true. Ensure this matches the intended user experience.Could you verify that showing history results only when both settings are enabled (rather than just
Settings.ShowHistoryResultsForHomePage) is the intended behavior?
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Flow.Launcher/ViewModel/MainViewModel.cs (2)
55-59: Consider using a dynamic GUID instead of hardcoded ID.While the comment explains the purpose of the fixed ID, a dynamically generated GUID would be more maintainable and less error-prone.
- private readonly PluginMetadata _historyMetadata = new() - { - ID = "298303A65D128A845D28A7B83B3968C2", // ID is for identifying the update plugin in UpdateActionAsync - Priority = 0 // Priority is for calculating scores in UpdateResultView - }; + private readonly PluginMetadata _historyMetadata = new() + { + ID = Guid.NewGuid().ToString("N").ToUpper(), // Generated ID for identifying history in UpdateActionAsync + Priority = 0 // Priority is for calculating scores in UpdateResultView + };
1271-1321: Add comment explaining search delay skipping for home queries.The code correctly handles different plugin sets for home vs. regular queries and properly manages plugin icon visibility. However, the decision to skip search delay for home queries (line 1400) should be explicitly documented with a comment.
- if (searchDelay && !isHomeQuery) // Do not delay for home query + if (searchDelay && !isHomeQuery) // Do not delay for home query to provide immediate feedback when opening Flow Launcher
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher/ViewModel/MainViewModel.cs(14 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (9)
Flow.Launcher/ViewModel/MainViewModel.cs (9)
36-36: Good addition for tracking home query state.This new field helps track whether the last query was a home query, which is essential for the new home page feature.
1088-1091: LGTM! Simple method to query results.A convenient public method to initiate a query without specifying the search delay parameter.
1149-1162: Good handling of history items context menu.Properly handles context menu for history items on the home page by only showing the TopMost option, while showing regular context menu items for other results.
1196-1214: Good refactoring for code reuse.Extracting
GetHistoryItems()as a static helper method promotes code reuse and cleanly separates the history item conversion logic.
1350-1372: Good conditional task creation based on query type.The code appropriately creates tasks for home and regular queries, properly filtering plugins based on their disabled state.
1449-1466: Well-implemented history task for home page.The
QueryHistoryTask()method correctly retrieves and formats recent history items for display on the home page.
1473-1473: Good support for empty queries.Returns a valid query with empty string for home queries, enabling the home page functionality.
1545-1564: Good handling of result clearing for different query types.The logic correctly preserves results when both the last and current queries are home queries, while clearing results when switching between home and regular queries.
1748-1748: Good defensive programming with null check.Prevents potential null reference exceptions when accessing
_lastQuery.ActionKeyword.
This comment has been minimized.
This comment has been minimized.
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. Forbidden patterns 🙅 (1)In order to address this, you could change the content to not match the forbidden patterns (comments before forbidden patterns may help explain why they're forbidden), add patterns for acceptable instances, or adjust the forbidden patterns themselves. These forbidden patterns matched content: s.b. workaround(s)If the flagged items are 🤯 false positivesIf items relate to a ...
|
Support Querying Results When Query Text is Empty (Home Page)
Firstly, developers need to implement new interface for their plugins. Then users can see a new toggle button in advanced panel in plugin settings page. (Plugin will be shown in home page even if it is disabled)
FL will query results for all enabled plugins which support the new interface.
In general settings page, we can enable home page and add history results into home page.
This feature is currently only available for .Net plugins to implement.
Close #3256, #19.
1. Add
IAsyncHomeQuery&IHomeQueryInterfacesLet plugins developers allow their plugins be queried when query text is empty.
2. Plugin Indicator Support
IHomeQueryInterfacesPlugin Indicatorresults can be shown in home page.Test
Main Window
Setting UI
TODO
Check
Home Pageoption is enabled / disabled.