-
-
Notifications
You must be signed in to change notification settings - Fork 329
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
New API for Customizing Equal Rules for Results & Fix Result Clone Issue #3178
Conversation
🥷 Code experts: no user matched threshold 10 See details
Knowledge based on git-blame:
Knowledge based on git-blame:
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 ...
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 WalkthroughWalkthroughThe pull request introduces significant enhancements to the Changes
Sequence DiagramsequenceDiagram
participant Plugin
participant Result
participant TopMostRecord
participant UserSelectedRecord
Plugin->>Result: Create Result with new properties
Result-->>Plugin: Configured Result object
TopMostRecord->>Result: Check Equality
Result-->>TopMostRecord: Compare using custom keys
UserSelectedRecord->>Result: Generate Hash Code
Result-->>UserSelectedRecord: Compute hash using custom methods
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 (
|
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: 2
🧹 Nitpick comments (1)
Flow.Launcher/Storage/UserSelectedRecord.cs (1)
47-48
: Improve readability of hash code generation.While the implementation is correct, the long ternary expressions make it harder to read and maintain.
Consider extracting the key retrieval logic:
-int hashcode = GenerateStaticHashCode(result.GetTitleKey != null ? result.GetTitleKey(result.Title) : result.Title); -return GenerateStaticHashCode(result.GetSubTitleKey != null ? result.GetSubTitleKey(result.SubTitle) : result.SubTitle, hashcode); +private static string GetEffectiveTitle(Result result) +{ + return result.GetTitleKey != null ? result.GetTitleKey(result.Title) : result.Title; +} + +private static string GetEffectiveSubTitle(Result result) +{ + return result.GetSubTitleKey != null ? result.GetSubTitleKey(result.SubTitle) : result.SubTitle; +} + +int hashcode = GenerateStaticHashCode(GetEffectiveTitle(result)); +return GenerateStaticHashCode(GetEffectiveSubTitle(result), hashcode);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Flow.Launcher.Plugin/Result.cs
(2 hunks)Flow.Launcher/Storage/TopMostRecord.cs
(1 hunks)Flow.Launcher/Storage/UserSelectedRecord.cs
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
🔇 Additional comments (2)
Flow.Launcher.Plugin/Result.cs (1)
284-302
: LGTM! Well-documented API for customizing equal rules.The new API with
GetTitleKey
andGetSubTitleKey
is well-designed and properly documented. The implementation aligns perfectly with the PR objectives, particularly for plugins that need to handle dynamic titles/subtitles.Flow.Launcher/Storage/TopMostRecord.cs (1)
55-62
: LGTM! Robust implementation of the new equal rules.The implementation correctly handles both the new key-based comparison and falls back to direct string comparison when keys are not provided. This ensures backward compatibility while supporting the new customization API.
I actually think we don't need an action to handle this. If plugin want to specify the equality of two result, they just need one Key, so we can add an identification field that is nullable? That field can be long probably (so plugin can calculate hashcode or something directly)? One problem is the |
Yeah. I see. What about introducing From my view, That way one plugin can specify any unique key, and FL will use it, ignoring title and subtitle. Plugin can get full control and can modify title or subtitle without breaking |
A nullable identification field / |
I think this is cool. Sorry got trapped by many things lately. Let's do that. |
New API for Customizing Equal Rules for Results
GetTitleKey
andGetSubTitleKey
are the actions to get the key of the title or the subtitle.These keys will be used when FL checks whether the result is the topmost record.
Or FL calculates the hashcode of the result for user selected records.
This is useful if one plugin needs to change the title or the subtitle of one record dynamically.
E.g. The records of plugin
Edge Workspace
have subtitles which can change based on the tab number of the edge workspace, but FL should regard these records as the same. (Full issue in cspotcode/Flow.Launcher.Plugin.EdgeWorkspaces#3)Fix Result Clone Issue
I find that some properties are not cloned when calling
Clone
function in theResult
, so I fix this clone issue.