-
-
Notifications
You must be signed in to change notification settings - Fork 379
Add internal model for plugin management #3572
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
base: dev
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
🥷 Code experts: onesounds onesounds has 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:
Activity based on git-commit:
Knowledge based on git-blame:
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. |
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 introduces an internal model for plugin management to remove the dependency on the external Plugin Manager plugin, thereby preventing Flow Launcher from entering a bad state when that plugin is deleted.
- Refactored the deletion command in PluginViewModel to use an internal uninstall method.
- Updated PluginStoreItemViewModel with new async methods for installing, uninstalling, and updating plugins, and refactored plugin properties.
- Added a new user setting with corresponding UI and language translation keys for automatically restarting Flow Launcher after plugin changes.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
Flow.Launcher/ViewModel/PluginViewModel.cs | Removed PluginManagerActionKeyword and updated delete plugin command to use the internal model. |
Flow.Launcher/ViewModel/PluginStoreItemViewModel.cs | Refactored plugin data handling and introduced async plugin management methods (install, uninstall, update). |
Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml | Added a card to toggle the new auto restart setting. |
Flow.Launcher/Languages/en.xaml | Added new translation keys for plugin management messages and auto restart functionality. |
Flow.Launcher/Infrastructure/UserSettings/Settings.cs | Introduced a new AutoRestartAfterChanging setting. |
Comments suppressed due to low confidence (1)
Flow.Launcher/ViewModel/PluginStoreItemViewModel.cs:26
- [nitpick] Consider renaming '_newPlugin' to 'currentPlugin' and '_oldPluginPair' to 'installedPluginPair' for clearer understanding.
_newPlugin = plugin;
Co-authored-by: Copilot <[email protected]>
📝 Walkthrough""" WalkthroughThis update introduces an "Auto Restart After Changing" option to user settings and the general settings UI, along with corresponding localization resources. It significantly refactors the plugin store item view model to support robust, asynchronous plugin install, uninstall, and update workflows, addressing potential null reference exceptions and improving error handling. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsPane
participant Settings
User->>SettingsPane: Toggles "Auto Restart After Changing"
SettingsPane->>Settings: Updates AutoRestartAfterChanging property
sequenceDiagram
participant User
participant PluginStoreUI
participant PluginStoreItemViewModel
participant PluginManager
User->>PluginStoreUI: Clicks Install/Uninstall/Update Plugin
PluginStoreUI->>PluginStoreItemViewModel: ShowCommandQueryAsync(action)
PluginStoreItemViewModel->>User: Show confirmation dialog
User-->>PluginStoreItemViewModel: Confirms
PluginStoreItemViewModel->>PluginManager: Call async Install/Uninstall/Update method
PluginManager->>User: Show progress and handle cancellation
PluginManager->>PluginStoreItemViewModel: Return success/failure
PluginStoreItemViewModel->>User: Show result message or prompt restart
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 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 (
|
This comment has been minimized.
This comment has been minimized.
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: 4
♻️ Duplicate comments (1)
Flow.Launcher/ViewModel/PluginStoreItemViewModel.cs (1)
160-197
: Mirror the ‘restart-on-error’ fix in Uninstall flowSame unconditional restart occurs here. Insert an early
return
insidecatch
, or capture asuccess
flag shared with the final block.
🧹 Nitpick comments (3)
Flow.Launcher/ViewModel/PluginStoreItemViewModel.cs (3)
119-137
: Partial download artefact not cleaned when user cancelsWhen the user presses “Cancel” in the progress box the temp zip (possibly half-written) is left on disk, and the next install run deletes only if
deleteFile
is true and the filename collides. Consider explicit cleanup on cancellation:if (cts.IsCancellationRequested) { - return; + if (!newPlugin.IsFromLocalInstallPath && File.Exists(filePath)) + File.Delete(filePath); + return; }
199-254
: Update flow: stale zip file not deleted after successful update
UpdatePluginAsync
downloads to%TEMP%
but never deletes the zip afterwards (unlike install). This can quickly clutter%TEMP%
with large archives.@@ else { await App.API.UpdatePluginAsync(oldPlugin, newPlugin, filePath); + if (!newPlugin.IsFromLocalInstallPath && File.Exists(filePath)) + File.Delete(filePath); }
256-289
: Download helper: typo and double-download logic
- Comment line 269 spells “expcetion”.
- When the progress box fails, we silently fall back to a background download without progress or user feedback. Confirm this is desired UX; otherwise surface the error.
Consider surfacing the original exception to the caller instead of a silent retry.
🧰 Tools
🪛 GitHub Check: Check Spelling
[warning] 264-264:
prg
is not a recognized word. (unrecognized-spelling)🪛 GitHub Actions: Check Spelling
[warning] 264-264: Spell check warning:
prg
is not a recognized word. (unrecognized-spelling)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Flow.Launcher.Infrastructure/UserSettings/Settings.cs
(1 hunks)Flow.Launcher/Languages/en.xaml
(2 hunks)Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml
(1 hunks)Flow.Launcher/ViewModel/PluginStoreItemViewModel.cs
(3 hunks)Flow.Launcher/ViewModel/PluginViewModel.cs
(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: Check Spelling
Flow.Launcher/ViewModel/PluginViewModel.cs
[warning] 17-17: Spell check warning: Ioc
is not a recognized word. (unrecognized-spelling)
Flow.Launcher/ViewModel/PluginStoreItemViewModel.cs
[warning] 19-19: Spell check warning: Ioc
is not a recognized word. (unrecognized-spelling)
[warning] 264-264: Spell check warning: prg
is not a recognized word. (unrecognized-spelling)
🪛 GitHub Check: Check Spelling
Flow.Launcher/ViewModel/PluginStoreItemViewModel.cs
[warning] 19-19:
Ioc
is not a recognized word. (unrecognized-spelling)
[warning] 264-264:
prg
is not a recognized word. (unrecognized-spelling)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: gitStream workflow automation
- GitHub Check: gitStream workflow automation
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (7)
Flow.Launcher.Infrastructure/UserSettings/Settings.cs (1)
179-179
: Good addition of the auto-restart settingThe new
AutoRestartAfterChanging
property with a default value offalse
is a good addition that will help control the application's restart behavior after plugin changes.Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml (1)
205-214
: Well-structured UI control for auto-restart settingThe toggle switch for the auto-restart setting is properly implemented with:
- Appropriate binding to the Settings.AutoRestartAfterChanging property
- Localized title and tooltip
- Consistent styling with other toggle switches
- Good positioning in the settings hierarchy
This follows the established pattern for settings controls in the application.
Flow.Launcher/ViewModel/PluginViewModel.cs (2)
1-1
: Appropriate addition of Task namespaceThe addition of the System.Threading.Tasks namespace is necessary for the async implementation.
173-175
: Good conversion to async uninstall methodConverting the plugin uninstall operation to an asynchronous method is a good improvement that:
- Properly uses async/await pattern
- Leverages the centralized uninstall method from PluginStoreItemViewModel
- Supports the PR objective of creating an internal model for plugin management
This change will make the plugin uninstallation process more robust.
Flow.Launcher/Languages/en.xaml (2)
134-135
: Well-defined localization strings for auto-restart settingThe localization strings for the auto-restart feature are clear and descriptive, explaining both the purpose of the setting and its effects.
189-204
: Comprehensive localization for plugin managementThis set of localization strings provides thorough coverage for the plugin management workflow:
- Error messages for installation/uninstallation/updating failures
- Success messages with restart instructions
- Confirmation prompts with plugin details
- Settings preservation options
- Download progress indicators
These strings will ensure a good user experience with appropriate feedback throughout the plugin management process.
Flow.Launcher/ViewModel/PluginStoreItemViewModel.cs (1)
98-106
: Generated filename may contain invalid path characters
newPlugin.Name
can legally contain:
or other characters invalid on Windows, makingPath.Combine
throw.
Sanitise the name (usePath.GetInvalidFileNameChars()
) before composing the filename.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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
♻️ Duplicate comments (1)
Flow.Launcher/ViewModel/PluginStoreItemViewModel.cs (1)
32-33
: Version parsing remains a potential exception source.This addresses a previous review concern about
new Version(string)
throwing exceptions on malformed version strings. The issue persists in the current implementation.Consider using safe version parsing to prevent UI crashes:
- public bool LabelUpdate => LabelInstalled && new Version(_newPlugin.Version) > new Version(_oldPluginPair.Metadata.Version); + public bool LabelUpdate => LabelInstalled && TryParseVersionComparison(_newPlugin.Version, _oldPluginPair.Metadata.Version); + + private static bool TryParseVersionComparison(string newVersion, string oldVersion) + { + return Version.TryParse(newVersion, out var newVer) && + Version.TryParse(oldVersion, out var oldVer) && + newVer > oldVer; + }
🧹 Nitpick comments (1)
Flow.Launcher.Core/Plugin/PluginManager.cs (1)
871-904
: Fix spelling issues in parameter name and comment.The download helper method is well-designed with progress reporting and retry logic, but has spelling issues flagged by static analysis.
Apply this diff to fix the spelling issues:
- internal static async Task DownloadFileAsync(string prgBoxTitle, string downloadUrl, string filePath, CancellationTokenSource cts, bool deleteFile = true, bool showProgress = true) + internal static async Task DownloadFileAsync(string progressBoxTitle, string downloadUrl, string filePath, CancellationTokenSource cts, bool deleteFile = true, bool showProgress = true) { if (deleteFile && File.Exists(filePath)) File.Delete(filePath); if (showProgress) { var exceptionHappened = false; - await API.ShowProgressBoxAsync(prgBoxTitle, + await API.ShowProgressBoxAsync(progressBoxTitle, async (reportProgress) => { if (reportProgress == null) { - // when reportProgress is null, it means there is expcetion with the progress box + // when reportProgress is null, it means there is exception with the progress box // so we record it with exceptionHappened and return so that progress box will close instantly exceptionHappened = true; return;🧰 Tools
🪛 GitHub Check: Check Spelling
[warning] 879-879:
prg
is not a recognized word. (unrecognized-spelling)🪛 GitHub Actions: Check Spelling
[warning] 879-883:
prg
is not a recognized word. (unrecognized-spelling)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Flow.Launcher.Core/Plugin/PluginManager.cs
(4 hunks)Flow.Launcher/ViewModel/PluginStoreItemViewModel.cs
(4 hunks)Flow.Launcher/ViewModel/PluginViewModel.cs
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Flow.Launcher/ViewModel/PluginViewModel.cs
🧰 Additional context used
🧬 Code Graph Analysis (1)
Flow.Launcher.Core/Plugin/PluginManager.cs (5)
Flow.Launcher.Infrastructure/UserSettings/Settings.cs (1)
Settings
(16-478)Plugins/Flow.Launcher.Plugin.PluginsManager/Utilities.cs (1)
UserPlugin
(62-80)Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (8)
GetTranslation
(136-136)MessageBoxResult
(366-366)InstallPlugin
(511-511)LogException
(276-276)ShowMsgError
(85-85)RestartApp
(34-34)ShowMsg
(114-114)ShowMsg
(123-123)Flow.Launcher.Core/Resource/Internationalization.cs (1)
GetTranslation
(247-259)Flow.Launcher.Plugin/PluginMetadata.cs (1)
PluginMetadata
(10-163)
🪛 GitHub Check: Check Spelling
Flow.Launcher.Core/Plugin/PluginManager.cs
[warning] 879-879:
prg
is not a recognized word. (unrecognized-spelling)
🪛 GitHub Actions: Check Spelling
Flow.Launcher.Core/Plugin/PluginManager.cs
[warning] 39-53: Ioc
is not a recognized word. (unrecognized-spelling)
[warning] 117-133: Reloadable
is not a recognized word. (unrecognized-spelling)
[warning] 179-183: metadatas
is not a recognized word. (unrecognized-spelling)
[warning] 181-185: metadatas
is not a recognized word. (unrecognized-spelling)
[warning] 182-186: metadatas
is not a recognized word. (unrecognized-spelling)
[warning] 184-188: metadatas
is not a recognized word. (unrecognized-spelling)
[warning] 879-883: prg
is not a recognized word. (unrecognized-spelling)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (6)
Flow.Launcher.Core/Plugin/PluginManager.cs (4)
28-28
: LGTM: FlowSettings integration looks good.The static field retrieval of Settings via IoC is appropriate here since PluginManager is a static class and the settings are used for runtime behavior control.
553-624
: Excellent async plugin installation with comprehensive user experience.The method provides:
- User confirmation before proceeding
- Progress reporting during download with cancellation support
- Proper error handling with early return on failure (prevents unwanted restart)
- Conditional restart based on user settings
- Clean file management for temporary downloads
626-664
: Well-implemented uninstall flow with proper user choices.The method correctly:
- Confirms the uninstall action with the user
- Provides choice for keeping/removing plugin settings
- Handles errors gracefully without restarting on failure
- Follows consistent UX pattern with other plugin operations
666-722
: Robust update implementation following established patterns.The update flow properly handles the two-plugin scenario (old and new) and maintains consistency with install/uninstall operations regarding user confirmation, error handling, and restart behavior.
Flow.Launcher/ViewModel/PluginStoreItemViewModel.cs (2)
12-18
: Excellent refactoring improves code clarity and separation of concerns.The split into
_newPlugin
and_oldPluginPair
fields makes the code more readable and eliminates repeated lookups. The constructor properly initializes both fields for consistent state.
63-79
: Async conversion successfully integrates with new plugin management workflow.The method correctly dispatches to the appropriate async PluginManager methods based on the action. The developer has chosen to keep the default case minimal as noted in past reviews.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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 (5)
Flow.Launcher.Core/Plugin/PluginManager.cs (4)
564-635
: Well-structured install method with comprehensive user interaction.The method properly handles user confirmation, downloads, installation, and restart flow. The error handling ensures graceful failure without unwanted restarts.
Some minor suggestions for improvement:
- var downloadFilename = string.IsNullOrEmpty(newPlugin.Version) - ? $"{newPlugin.Name}-{Guid.NewGuid()}.zip" - : $"{newPlugin.Name}-{newPlugin.Version}.zip"; + var downloadFilename = string.IsNullOrWhiteSpace(newPlugin.Version) + ? $"{newPlugin.Name}-{Guid.NewGuid()}.zip" + : $"{newPlugin.Name}-{newPlugin.Version}.zip";Consider using
IsNullOrWhiteSpace
for more robust empty string detection.
642-646
: Simplify plugin.json extraction logic.The current implementation has redundant code for getting the plugin.json entry.
Apply this diff to streamline the logic:
- var pluginJsonPath = archive.Entries.FirstOrDefault(x => x.Name == "plugin.json") ?? - throw new FileNotFoundException("The zip file does not contain a plugin.json file."); - var pluginJsonEntry = archive.GetEntry(pluginJsonPath.ToString()) ?? - throw new FileNotFoundException("The zip file does not contain a plugin.json file."); + var pluginJsonEntry = archive.Entries.FirstOrDefault(x => x.Name == "plugin.json") ?? + throw new FileNotFoundException("The zip file does not contain a plugin.json file.");
918-951
: Fix typos and improve error handling.The download helper has good retry logic but contains typos and could benefit from clearer documentation.
Apply these fixes:
- internal static async Task DownloadFileAsync(string prgBoxTitle, string downloadUrl, string filePath, CancellationTokenSource cts, bool deleteFile = true, bool showProgress = true) + internal static async Task DownloadFileAsync(string progressBoxTitle, string downloadUrl, string filePath, CancellationTokenSource cts, bool deleteFile = true, bool showProgress = true)- await API.ShowProgressBoxAsync(prgBoxTitle, + await API.ShowProgressBoxAsync(progressBoxTitle,- // when reportProgress is null, it means there is expcetion with the progress box + // when reportProgress is null, it means there is exception with the progress box🧰 Tools
🪛 GitHub Check: Check Spelling
[warning] 926-926:
prg
is not a recognized word. (unrecognized-spelling)
953-969
: Enhance URL validation robustness.The InstallSourceKnown method should handle edge cases more gracefully.
Consider this improvement:
private static bool InstallSourceKnown(string url) { + if (string.IsNullOrWhiteSpace(url)) + return false; + var pieces = url.Split('/'); if (pieces.Length < 4) return false; var author = pieces[3]; var acceptedSource = "https://github.com"; var constructedUrlPart = string.Format("{0}/{1}/", acceptedSource, author); return url.StartsWith(acceptedSource) && API.GetAllPlugins().Any(x => !string.IsNullOrEmpty(x.Metadata.Website) && x.Metadata.Website.StartsWith(constructedUrlPart) ); }Flow.Launcher/SettingPages/ViewModels/SettingsPanePluginStoreViewModel.cs (1)
113-130
: Improve cross-platform compatibility and error handling.The file dialog helper has some platform-specific assumptions that could be improved.
Consider these improvements:
private static string GetFileFromDialog(string title, string filter = "") { + var downloadsPath = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); + try + { + downloadsPath = Path.Combine(downloadsPath, "Downloads"); + } + catch + { + // Fallback to user profile if Downloads folder access fails + } + var dlg = new OpenFileDialog { - InitialDirectory = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile) + "\\Downloads", + InitialDirectory = downloadsPath, Multiselect = false, CheckFileExists = true, CheckPathExists = true, Title = title, Filter = filter }; return dlg.ShowDialog() switch { DialogResult.OK => dlg.FileName, _ => string.Empty }; }This approach:
- Uses Path.Combine for better cross-platform support
- Adds error handling for Downloads folder access
- Maintains the same functionality while being more robust
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Flow.Launcher.Core/Plugin/PluginManager.cs
(4 hunks)Flow.Launcher/Languages/en.xaml
(2 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPanePluginStoreViewModel.cs
(2 hunks)Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml
(1 hunks)Flow.Launcher/SettingPages/Views/SettingsPanePluginStore.xaml
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml
- Flow.Launcher/Languages/en.xaml
🧰 Additional context used
🧬 Code Graph Analysis (1)
Flow.Launcher.Core/Plugin/PluginManager.cs (7)
Flow.Launcher/PublicAPIInstance.cs (12)
Task
(121-121)Task
(215-234)Task
(247-248)Task
(250-251)Task
(253-254)GetTranslation
(240-240)MessageBoxResult
(470-473)InstallPlugin
(540-541)LogException
(276-277)ShowMsgError
(123-124)ShowMsg
(126-127)ShowMsg
(129-132)Flow.Launcher.Infrastructure/UserSettings/Settings.cs (1)
Settings
(16-494)Plugins/Flow.Launcher.Plugin.PluginsManager/Utilities.cs (1)
UserPlugin
(62-80)Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (8)
GetTranslation
(141-141)MessageBoxResult
(371-371)InstallPlugin
(516-516)LogException
(281-281)ShowMsgError
(85-85)RestartApp
(34-34)ShowMsg
(119-119)ShowMsg
(128-128)Flow.Launcher.Core/Resource/Internationalization.cs (1)
GetTranslation
(247-259)Flow.Launcher.Plugin/PluginMetadata.cs (2)
ToString
(159-162)PluginMetadata
(10-163)Plugins/Flow.Launcher.Plugin.PluginsManager/PluginsManager.cs (1)
InstallSourceKnown
(581-597)
🪛 GitHub Check: Check Spelling
Flow.Launcher.Core/Plugin/PluginManager.cs
[warning] 926-926:
prg
is not a recognized word. (unrecognized-spelling)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (8)
Flow.Launcher.Core/Plugin/PluginManager.cs (5)
5-5
: LGTM! Appropriate new imports added.The new using statements for
System.IO.Compression
andSystem.Windows
are correctly added to support zip file handling and MessageBox operations in the new plugin management methods.Also applies to: 10-10
29-29
: LGTM! Clean integration with user settings.The FlowSettings field provides proper access to user configuration for controlling auto-restart behavior and warning preferences.
660-668
: LGTM! Proper unknown source validation.The security check for unknown plugin sources with user warning is well-implemented. The conditional check based on settings provides good user control.
673-711
: Excellent uninstall workflow with user choice preservation.The method properly handles user confirmation for both uninstall and settings retention. The error handling prevents restart on failure.
713-769
: Well-implemented update method with proper flow control.The update workflow correctly handles confirmation, download, and restart logic. Good consistency with other plugin management methods.
Flow.Launcher/SettingPages/Views/SettingsPanePluginStore.xaml (1)
95-101
: LGTM! Well-integrated install button.The new button is properly positioned in the toolbar, has appropriate command binding to
InstallPluginCommand
, and includes a helpful tooltip. The styling is consistent with other buttons in the interface.Flow.Launcher/SettingPages/ViewModels/SettingsPanePluginStoreViewModel.cs (2)
1-2
: LGTM! Appropriate using statements added.The new imports are correctly added to support the file dialog functionality and plugin manager integration.
Also applies to: 5-5, 7-7
102-111
: LGTM! Clean command implementation.The InstallPluginAsync method is well-structured with proper async handling and delegates to the PluginManager for the actual work.
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 replaces the external Plugin Manager plugin with an internal model, adds install/uninstall/update flows (including local zip installs), and introduces new UI controls and settings for plugin management.
- Added async install/uninstall/update methods with optional auto-restart and warnings in
PluginManager
. - Updated ViewModels to call the new internal API and added a local-zip install command.
- Extended settings UI with toggles for auto-restart after plugin changes and warnings for unknown sources.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
Flow.Launcher/ViewModel/PluginViewModel.cs | Switched delete action to use PluginManager.UninstallPluginAndCheckRestartAsync and removed the old keyword helper. |
Flow.Launcher/ViewModel/PluginStoreItemViewModel.cs | Replaced flyout keyword logic with async install/uninstall/update switch. |
Flow.Launcher/SettingPages/Views/SettingsPanePluginStore.xaml | Added a button to install a plugin from a local zip. |
Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml | Added toggle switches for auto-restart and unknown source warnings. |
Flow.Launcher/SettingPages/ViewModels/SettingsPanePluginStoreViewModel.cs | Introduced OpenFileDialog logic and InstallPluginCommand . |
Flow.Launcher/Languages/en.xaml | Added translation strings for install/uninstall/update prompts and new settings. |
Flow.Launcher.Infrastructure/UserSettings/Settings.cs | Added AutoRestartAfterChanging and ShowUnknownSourceWarning settings. |
Flow.Launcher.Core/Plugin/PluginManager.cs | Implemented internal plugin install/uninstall/update flows, file downloads, and helper methods. |
Comments suppressed due to low confidence (2)
Flow.Launcher/ViewModel/PluginViewModel.cs:173
- [nitpick] The method name
OpenDeletePluginWindowAsync
does not reflect that it actually uninstalls the plugin and may restart the app. Consider renaming toUninstallPluginAsync
or similar for clarity.
private async Task OpenDeletePluginWindowAsync()
Flow.Launcher/Languages/en.xaml:206
- [nitpick] Translation key
AutoRestartAfterChange
is inconsistent with code’sautoRestartAfterChanging
property and other keys. Consider unifying the naming convention for clarity.
<system:String x:Key="AutoRestartAfterChange">Automatically restart after installing/uninstalling/updating plugins in plugin store</system:String>
await PluginManager.InstallPluginAndCheckRestartAsync(file); | ||
} | ||
|
||
private static string GetFileFromDialog(string title, string filter = "") |
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.
[nitpick] This viewmodel directly uses System.Windows.Forms.OpenFileDialog
, introducing a WinForms dependency into WPF. Consider using Microsoft.Win32.OpenFileDialog
or abstracting the file picker behind an interface.
Copilot uses AI. Check for mistakes.
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.
no need
Add internal model for plugin management
Let us do not rely
Plugin Manager
plugin for plugin management in Flow Launcher. Flow Launcher will get into a very bad state if this plugin is deleted (plugin store page will lose ability to install/uninstall/update plugins).Resolve #3555, #3236.
Additionally, now we support to select local zip file to install in Plugin Store page.
Test