-
-
Notifications
You must be signed in to change notification settings - Fork 448
Fix Plugin Uninstallation Issue #3786
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
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 aims to fix the plugin uninstallation issue by removing the redundant external parameter "removePluginFromSettings" and ensuring that only the "removePluginSettings" parameter is exposed.
- Removes the "removePluginFromSettings" parameter from the public API
- Hardcodes "removePluginFromSettings" to true in the internal call to maintain consistent behavior
Comments suppressed due to low confidence (1)
Flow.Launcher.Core/Plugin/PluginManager.cs:557
- [nitpick] Since the parameter 'removePluginFromSettings' is now hardcoded to true, please ensure that this behavior is intentional and document the reasoning. If future changes require flexibility, consider refactoring to allow external configuration.
await UninstallPluginAsync(plugin, removePluginFromSettings: true, removePluginSettings: removePluginSettings, checkModified: true);
🥷 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: To learn more about /:\ gitStream - Visit our Docs |
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 WalkthroughWalkthroughThe method signature of Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant PluginManager
Caller->>PluginManager: UninstallPluginAsync(plugin, removePluginSettings)
PluginManager->>PluginManager: UninstallPluginAsync(plugin, removePluginFromSettings: true, removePluginSettings, checkModified: true)
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🔇 Additional comments (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
Fix Plugin Uninstallation Issue
In
Flow.Launcher/Flow.Launcher/PublicAPIInstance.cs
Lines 575 to 576 in e965b56
we only pass
removePluginSettings
parameter.But in
Flow.Launcher/Flow.Launcher.Core/Plugin/PluginManager.cs
Lines 555 to 558 in e965b56
we have accepted four parameters and
removePluginSettings
parameter is passed toremovePluginFromSettings
parameter which can cause possible issues