Add "Last opened" history mode & Fix non-result history item save issue #4042
Conversation
…action between query result or last opened
|
🥷 Code experts: Jack251970, taooceros Jack251970, taooceros have 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:
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: ✨ Comment |
|
I decided to revert my recent changes to follow the approach that was originally planned. I tested your changes, @Jack251970 , and everything looks good! OBS: I get your point now. When switching between styles, the same items should be shown; only the display style “HistoryStyle” and the action change. I was thinking there would be a separation between Query items and Last Opened items. But after understanding your view better, I see it makes more sense for them to be the same items, just with a different display. Makes a lot more sense! Thanks for the review! |
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: 3
♻️ Duplicate comments (1)
Flow.Launcher/Storage/QueryHistory.cs (1)
42-45: Fix the off-by-one in history trimming.With the current
> _maxHistorycheck the list happily grows to_maxHistory + 1entries before trimming, so the cap is never respected exactly.- if (LastOpenedHistoryItems.Count > _maxHistory) + if (LastOpenedHistoryItems.Count >= _maxHistory) { LastOpenedHistoryItems.RemoveAt(0); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Flow.Launcher.Infrastructure/UserSettings/Settings.cs(3 hunks)Flow.Launcher/Helper/ResultHelper.cs(1 hunks)Flow.Launcher/Storage/LastOpenedHistoryItem.cs(1 hunks)Flow.Launcher/Storage/QueryHistory.cs(1 hunks)Flow.Launcher/ViewModel/MainViewModel.cs(16 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
Flow.Launcher/Storage/QueryHistory.cs (2)
Flow.Launcher/ViewModel/MainViewModel.cs (4)
List(1318-1379)Query(1198-1226)Result(1755-1796)Result(1798-1826)Flow.Launcher/Storage/LastOpenedHistoryItem.cs (2)
LastOpenedHistoryItem(6-31)Equals(15-30)
Flow.Launcher/Helper/ResultHelper.cs (2)
Flow.Launcher/Storage/LastOpenedHistoryItem.cs (1)
LastOpenedHistoryItem(6-31)Flow.Launcher.Core/Plugin/QueryBuilder.cs (1)
QueryBuilder(7-61)
Flow.Launcher/ViewModel/MainViewModel.cs (4)
Flow.Launcher/PublicAPIInstance.cs (4)
List(251-251)List(530-530)BackToQueryResults(520-520)ChangeQuery(72-75)Flow.Launcher/Storage/QueryHistory.cs (3)
PopulateHistoryFromLegacyHistory(21-34)History(9-66)Add(36-65)Flow.Launcher/Storage/LastOpenedHistoryItem.cs (1)
LastOpenedHistoryItem(6-31)Flow.Launcher/Helper/ResultHelper.cs (1)
ResultHelper(12-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: build
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
This comment has been minimized.
This comment has been minimized.
Added logic to record user-selected results in `_userSelectedRecord` before executing both synchronous and asynchronous actions. This enables tracking of user interactions for result ranking or analytics. Comments were added to clarify the purpose of the new logic.
Moved `_userSelectedRecord.Add(result)` outside the `if (queryResultsSelected)` block to ensure all user-selected results are recorded, regardless of their source (query results, context menu, or history). Added a comment to clarify that only query results are added to history.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Flow.Launcher/ViewModel/MainViewModel.cs (1)
1317-1384: Consider preserving the original subtitle for Last Opened history.For
HistoryStyle.Query(lines 1324-1339), showing only the timestamp as subtitle makes sense. However, for Last Opened history (lines 1343-1382), line 1350 replaces the subtitle with just the timestamp, discardingh.SubTitlewhich often contains important metadata like file paths or descriptions. This breaks fuzzy search on subtitle and makes it harder to distinguish items with identical titles.Apply this diff to preserve the subtitle while still showing the timestamp:
var result = new Result { Title = string.IsNullOrEmpty(h.Title) ? // Old migrated history items have no title Localize.executeQuery(h.Query) : h.Title, - SubTitle = Localize.lastExecuteTime(h.ExecutedDateTime), + SubTitle = string.IsNullOrEmpty(h.SubTitle) + ? Localize.lastExecuteTime(h.ExecutedDateTime) + : $"{h.SubTitle} • {Localize.lastExecuteTime(h.ExecutedDateTime)}", IcoPath = Constant.HistoryIcon,This preserves searchability while still surfacing when the item was last opened.
Note: The rest of the Last Opened implementation is solid:
- Line 1355: Correctly re-queries the plugin to get the fresh result and its action
- Lines 1358-1370: Properly handles both sync and async actions
- Lines 1373-1376: Good fallback when the result is no longer available
- Line 1347-1349: Appropriate fallback for migrated legacy items
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher/ViewModel/MainViewModel.cs(15 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
Flow.Launcher/ViewModel/MainViewModel.cs (5)
Flow.Launcher/PublicAPIInstance.cs (4)
List(251-251)List(530-530)BackToQueryResults(520-520)ChangeQuery(72-75)Flow.Launcher.Infrastructure/Helper.cs (1)
Helper(7-31)Flow.Launcher/Storage/QueryHistory.cs (3)
PopulateHistoryFromLegacyHistory(21-34)History(9-66)Add(36-65)Flow.Launcher/Storage/LastOpenedHistoryItem.cs (1)
LastOpenedHistoryItem(6-31)Flow.Launcher/Helper/ResultHelper.cs (1)
ResultHelper(12-44)
🪛 GitHub Actions: Check Spelling
Flow.Launcher/ViewModel/MainViewModel.cs
[warning] 27-27: Spell check warning: 'NKORE' is not a recognized word. (unrecognized-spelling)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (6)
Flow.Launcher/ViewModel/MainViewModel.cs (6)
155-155: LGTM!The migration call is correctly placed after loading history and will migrate legacy
HistoryItementries to the newLastOpenedHistoryItemformat.
357-357: LGTM!The history API has been correctly updated to use
LastOpenedHistoryItemsthroughout. Index bounds are properly validated before access using the index-from-end operator.Also applies to: 385-393, 399-407, 614-618
494-495: LGTM!Caching the
queryResultsSelectedflag before executing the action is correct, as the action execution might navigate away from query results. The updatedHistory.Add(result)API correctly passes the full result object to store title, subtitle, plugin ID, and record key.Also applies to: 535-540
1273-1284: LGTM!The context menu query logic has been reformatted for better readability without changing functionality.
1613-1613: LGTM!Correctly retrieves the most recent history items using
LastOpenedHistoryItemsand reverses the order to display them with the most recent first.
12-12: No action required:using System.Windows.Input;is required by KeyGestureConverter, KeyGesture and Keyboard.Focus.
The tooltip text for the key `historyStyleTooltip` was updated to improve capitalization and consistency. "History" and "Home Page" were capitalized to align with the formatting of other keys in the file.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 10 out of 12 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Flow.Launcher/ViewModel/MainViewModel.cs (1)
1353-1353: Preserve original subtitle for searchability and context.Setting
SubTitle = Localize.lastExecuteTime(h.ExecutedDateTime)discards the storedh.SubTitle(e.g., file path, plugin info), making it impossible to search by that metadata or distinguish between identically titled entries. Consider combining both pieces of information.This is similar to a previously flagged concern. If this design is intentional, please clarify the reasoning.
Apply this diff to preserve the original subtitle:
- SubTitle = Localize.lastExecuteTime(h.ExecutedDateTime), + SubTitle = string.IsNullOrEmpty(h.SubTitle) + ? Localize.lastExecuteTime(h.ExecutedDateTime) + : $"{h.SubTitle} • {Localize.lastExecuteTime(h.ExecutedDateTime)}",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Directory.Build.props(1 hunks)Flow.Launcher/Flow.Launcher.csproj(1 hunks)Flow.Launcher/Languages/en.xaml(1 hunks)Flow.Launcher/MainWindow.xaml.cs(3 hunks)Flow.Launcher/ViewModel/MainViewModel.cs(15 hunks)
✅ Files skipped from review due to trivial changes (2)
- Flow.Launcher/Flow.Launcher.csproj
- Directory.Build.props
🚧 Files skipped from review as they are similar to previous changes (2)
- Flow.Launcher/MainWindow.xaml.cs
- Flow.Launcher/Languages/en.xaml
🧰 Additional context used
🧬 Code graph analysis (1)
Flow.Launcher/ViewModel/MainViewModel.cs (4)
Flow.Launcher/Storage/QueryHistory.cs (3)
PopulateHistoryFromLegacyHistory(21-34)History(9-66)Add(36-65)Flow.Launcher.Plugin/Result.cs (2)
Result(13-374)Result(293-327)Flow.Launcher/Storage/LastOpenedHistoryItem.cs (1)
LastOpenedHistoryItem(6-31)Flow.Launcher/Helper/ResultHelper.cs (1)
ResultHelper(12-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: gitStream.cm
- GitHub Check: build
Updated the `catch` block in the `ResultHelper` class to explicitly catch `System.Exception` and log the error using `App.API.LogException`. The log includes the class name, a failure message for querying results, and the exception details. This improves error visibility and debugging.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 10 out of 12 changed files in this pull request and generated 4 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
3 participants
CHANGES