Ensure plugin manifest is non-nullable#4355
Conversation
Removed remarks from GetPluginManifest docs. GetPluginManifest now returns an empty list if null. ExternalPlugins property always enumerates and sorts plugins. Added BOM to PublicAPIInstance.cs.
|
🥷 Code experts: no user but you matched threshold 10 Jack251970, jjw24 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: ✨ Comment |
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
There was a problem hiding this comment.
Pull request overview
Updates Flow Launcher’s public API so plugin manifest access is non-nullable, preventing null-sequence LINQ crashes when the manifest fetch fails (as in #4251).
Changes:
- Make
PublicAPIInstance.GetPluginManifest()return an empty list when the manifest isn’t available (instead ofnull). - Remove the null-conditional usage in
SettingsPanePluginStoreViewModel.ExternalPluginsnow that the API guarantees a non-null list. - Remove outdated API docs indicating
GetPluginManifest()may returnnull.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Flow.Launcher/SettingPages/ViewModels/SettingsPanePluginStoreViewModel.cs | Stops treating the manifest as nullable when building the plugin store list. |
| Flow.Launcher/PublicAPIInstance.cs | Ensures GetPluginManifest() never returns null by coalescing to an empty list. |
| Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs | Removes outdated remark about null return (needs updated contract wording). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughGetPluginManifest() documentation was clarified to promise a non-null IReadOnlyList. Implementation now returns an empty list when underlying manifest entries are null. A consumer callsite was updated to rely on the non-null guarantee. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can use TruffleHog to scan for secrets in your code with verification capabilities.Add a TruffleHog config file (e.g. trufflehog-config.yml, trufflehog.yml) to your project to customize detectors and scanning behavior. The tool runs only when a config file is present. |
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs">
<violation number="1" location="Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs:539">
P3: The new return docs incorrectly say a failed manifest refresh can make this API return an empty list; in practice it keeps returning the last successful manifest. That can mislead plugin authors into using `Count == 0` as a failure signal.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Resolve #4251
Summary by cubic
Ensures
GetPluginManifestalways returns a non-null list. Simplifies callers, clarifies docs/comments, and removes null checks in the plugin store view.IPublicAPI.GetPluginManifestis non-nullable and documented to return a list that may be empty.SettingsPanePluginStoreViewModel.ExternalPluginsnow enumerates and sorts without null checks. Improved XML docs and inline comments for clarity.[]) inPublicAPIInstance.GetPluginManifestwhenPluginsManifest.UserPluginsis null.GetPluginManifestcould be null; null-conditional operator inExternalPlugins.Written for commit 015c32e. Summary will update on new commits.