-
-
Notifications
You must be signed in to change notification settings - Fork 327
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
Add options for setting result on top & keeping result order #3144
Conversation
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
This comment has been minimized.
This comment has been minimized.
📝 WalkthroughWalkthroughThe pull request adds a new property Changes
Possibly Related PRs
Suggested Labels
Suggested Reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
@@ -1473,10 +1473,12 @@ public void UpdateResultView(ICollection<ResultsForUpdate> resultsForUpdates) | |||
{ | |||
result.Score = int.MaxValue; | |||
} | |||
else | |||
else if (result.Score != int.MaxValue) |
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.
Just wanted to note here, at some point I think it's best to provide an actual max value- standardise the scoring system so to not allow randomly assigning any score by plugins. #3022
Also this int max value is not documented anywhere so unless devs look into this Result class they probably won't know that they can do this.
Any how just thought I mention these, but they are out side the scope of this change.
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.
@jjw24 Now I export the max score in the Result
class so developers can see this maximum score.
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. If the flagged items are 🤯 false positivesIf items relate to a ...
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Flow.Launcher.Plugin/Result.cs (2)
266-269
: Documentation could be more descriptiveThe implementation looks good, but the documentation could be improved to better explain the purpose and impact of this property.
Apply this documentation improvement:
/// <summary> - /// Determines if the user selection count should be added to the score. This can be useful when set to false to allow the result sequence order to be the same everytime instead of changing based on selection. + /// Determines if the user selection count should be added to the score. When set to false, the result sequence order will remain consistent across searches, unaffected by the user's selection history. This is particularly useful for plugins that need to maintain a specific ordering of results (e.g., sorting by date/time). /// </summary>
271-274
: Enhance documentation for MaxScore constantThe implementation is good, but the documentation could better explain how this constant interacts with the scoring system.
Apply this documentation improvement:
/// <summary> - /// Maximum score. This can be useful when set one result to the top by default. This is the score for the results set to the topmost by users. + /// Represents the maximum possible score for a result. When a plugin sets a result's score to this value, Flow Launcher will not add the selected count and priority score to it, ensuring the result remains at the top of the query list. This is particularly useful for plugins that need to prioritize specific results above all others. /// </summary>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Flow.Launcher.Plugin/Result.cs
(1 hunks)Flow.Launcher/ViewModel/MainViewModel.cs
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Flow.Launcher/ViewModel/MainViewModel.cs
👍 thanks for adding this long awaited option. |
If one plugin set the score of one result to int.MaxValue,
FL
should not add selected count & priority score.Generally, this plugin want this result to the top of the query list.
If one plugin want to force the sequence of its results, it can set the
AddSelectedCount
to false so thatFL
will not add selected count.Like one plugin with many query items which needs to allow user to set the sequence manually (order by datatime, etc.).