-
-
Notifications
You must be signed in to change notification settings - Fork 445
Support non action keyword for plugins #3951
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
Conversation
🥷 Code experts: no user but you matched threshold 10 Jack251970 has most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame: ✨ Comment |
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdjusts GetPluginMetadata in PluginConfig to derive ActionKeyword from Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant PC as PluginConfig
participant GM as GetPluginMetadata()
participant AK as ActionKeywords (IList)
participant MD as Metadata
PC->>GM: Build metadata
GM->>AK: Evaluate metadata.ActionKeywords
alt ActionKeywords != null and has elements
GM->>MD: ActionKeyword = ActionKeywords.FirstOrDefault()
else ActionKeywords != null but empty
GM->>MD: ActionKeyword = string.Empty
else ActionKeywords == null
GM->>MD: (unchanged initialization logic applies)
end
GM-->>PC: Return metadata
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (3)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Pull Request Overview
This PR adds support for plugins that do not need any action keywords by modifying how the ActionKeyword property is assigned when ActionKeywords list is empty.
- Modified the ActionKeyword assignment logic to handle empty ActionKeywords collections
- Ensures plugins without action keywords are not added to Flow's action keyword dictionary
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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: 1
♻️ Duplicate comments (1)
Flow.Launcher.Core/Plugin/PluginConfig.cs (1)
122-122
: Redundant null-conditionals
ActionKeywords
can’t be null after Line 120, so?.Count
and?[0]
are unnecessary and slightly obscure intent.
🧹 Nitpick comments (3)
Flow.Launcher.Core/Plugin/PluginConfig.cs (3)
120-123
: Normalize keywords and avoid nullable first-element; also simplify null handlingAfter Line 120,
ActionKeywords
is guaranteed non-null. Use directCount
/indexer and normalize away null/whitespace entries soActionKeyword
never ends upnull
. This also cleanly represents “no action keyword” asstring.Empty
.Apply:
- // for plugins which doesn't has ActionKeywords key - metadata.ActionKeywords ??= new List<string> { metadata.ActionKeyword }; - // for plugin still use old ActionKeyword - metadata.ActionKeyword = metadata.ActionKeywords?.Count == 0 ? string.Empty : metadata.ActionKeywords?[0]; + // For plugins that don't define ActionKeywords, seed from legacy ActionKeyword, then normalize + metadata.ActionKeywords = (metadata.ActionKeywords ?? new List<string> { metadata.ActionKeyword }) + .Where(k => !string.IsNullOrWhiteSpace(k)) + .ToList(); + // Back-compat: expose first keyword, or empty string if none + metadata.ActionKeyword = metadata.ActionKeywords.Count == 0 ? string.Empty : metadata.ActionKeywords[0];
138-139
: Fix typos and missing bracket in log messageCurrent message has “conifg” and a missing closing ‘>’.
Apply:
- API.LogError(ClassName, $"Execute file path didn't exist <{metadata.ExecuteFilePath}> for conifg <{configPath}"); + API.LogError(ClassName, $"Execute file path doesn't exist <{metadata.ExecuteFilePath}> for config <{configPath}>");
30-31
: Polish TODO comment (spelling/grammar)Minor readability cleanup.
Apply:
- // todo use linq when diable plugin is implmented since parallel.foreach + list is not thread saft + // TODO: Use LINQ when disable-plugin is implemented, since Parallel.ForEach + List<T> is not thread-safe
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher.Core/Plugin/PluginConfig.cs
(1 hunks)
⏰ 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 workflow automation
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: build
Co-authored-by: Copilot <[email protected]>
Some plugins do not need any action keywords. And we should support that so that Flow will not add this plugin into action keyword dictionary.
Currently,
"ActionKeyword": ""
is supported, but"ActionKeywords": []
is not. This PR enables support for that.Related:
Flow-Launcher/Flow.Launcher.Plugin.QuickLook#10
Flow-Launcher/Flow.Launcher.Plugin.QuickLook#13