Separe Install, update and uninstall custom command-line args#3748
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR splits the handling of custom CLI arguments into separate install, update, and uninstall parameters to resolve issue #3526. Key changes include updates in the UI (XAML) to display separate text boxes and labels, modifications in the C# logic to use the new parameter lists, and corresponding test updates and factory sanitization changes.
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/UniGetUI/Pages/DialogPages/InstallOptions_Package.xaml.cs | Updated to assign and process three separate custom parameter lists. |
| src/UniGetUI/Pages/DialogPages/InstallOptions_Package.xaml | Modified UI elements to support separate CLI argument inputs. |
| src/UniGetUI/Pages/DialogPages/InstallOptions_Manager.xaml.cs | Updated UI and logic to enable/disable and split parameters. |
| src/UniGetUI/Pages/DialogPages/InstallOptions_Manager.xaml | Adjusted XAML layout for three distinct parameter inputs. |
| src/UniGetUI/Controls/MenuForPackage.cs | Added inline comment regarding an invisible character for TabViewItem display. |
| src/UniGetUI.PackageEngine.Serializable/InstallOptions.cs | Replaced one custom parameters list with three separate lists and updated serialization. |
| src/UniGetUI.PackageEngine.Serializable.Tests/TestSerializablePackage.cs | Modified tests to include split parameter arrays. |
| src/UniGetUI.PackageEngine.Serializable.Tests/TestSerializableBundle.cs | Updated test data to use the new split custom parameter arrays. |
| src/UniGetUI.PackageEngine.Serializable.Tests/TestInstallOptions.cs | Adjusted tests to assert against the new parameter fields. |
| src/UniGetUI.PackageEngine.PackageManagerClasses/Packages/Classes/InstallOptionsFactory.cs | Sanitized and logged custom parameters separately. |
| src/UniGetUI.PackageEngine.Managers.WinGet/Helpers/WinGetPkgOperationHelper.cs | Selected parameter list based on operation type using switch expressions. |
| src/UniGetUI.PackageEngine.Managers.Vcpkg/Helpers/VcpkgPkgOperationHelper.cs | Updated to use separate parameter lists via a switch expression. |
| src/UniGetUI.PackageEngine.Managers.PowerShell7/Helpers/PowerShell7PkgOperationHelper.cs | Adopted switch expression for parameter selection. |
| src/UniGetUI.PackageEngine.Managers.PowerShell/Helpers/PowerShellPkgOperationHelper.cs | Replaced old custom parameters usage with the new split parameters. |
| src/UniGetUI.PackageEngine.Managers.Pip/Helpers/PipPkgOperationHelper.cs | Updated to select parameters by operation type using the new fields. |
| src/UniGetUI.PackageEngine.Managers.Npm/Helpers/NpmPkgOperationHelper.cs | Replaced legacy parameter additions with a switch expression selecting proper lists. |
| src/UniGetUI.PackageEngine.Managers.Dotnet/Helpers/DotNetPkgOperationHelper.cs | Modified to add parameters using the new split parameter fields. |
| src/UniGetUI.PackageEngine.Managers.Chocolatey/Helpers/ChocolateyPkgOperationHelper.cs | Adjusted to use separate parameter lists with a switch expression. |
| src/UniGetUI.PackageEngine.Managers.Cargo/Helpers/CargoPkgOperationHelper.cs | Updated to select the proper custom parameter list based on operation type. |
Comments suppressed due to low confidence (1)
src/UniGetUI.PackageEngine.Serializable/InstallOptions.cs:58
- Add a comment to explain the fallback logic that reads legacy 'CustomParameters' from JSON, to improve clarity for future maintainers.
if (this.CustomParameters_Install.Count is 0 &&
| public override string ToString() | ||
| { | ||
| string customparams = CustomParameters.Any() ? string.Join(",", CustomParameters) : "[]"; | ||
| string customparams = CustomParameters_Install.Any() ? string.Join(",", CustomParameters_Install) : "[]"; |
There was a problem hiding this comment.
[nitpick] Consider adding clear delimiters or additional formatting when concatenating install, update, and uninstall parameters in the ToString() method so that the output is easier to read.
| Logger.Warn($"Custom CLI parameters [{string.Join(' ', options.CustomParameters_Install)}] will be discarded"); | ||
| if (options.CustomParameters_Update.Count > 0) | ||
| Logger.Warn($"Custom CLI parameters [{string.Join(' ', options.CustomParameters_Update)}] will be discarded"); | ||
| if (options.CustomParameters_Uninstall.Count > 0) | ||
| Logger.Warn($"Custom CLI parameters [{string.Join(' ', options.CustomParameters_Uninstall)}] will be discarded"); |
There was a problem hiding this comment.
[nitpick] Consider logging warnings for each parameter type (install, update, uninstall) separately when CLI arguments are disabled, to aid in debugging and clarity.
| Logger.Warn($"Custom CLI parameters [{string.Join(' ', options.CustomParameters_Install)}] will be discarded"); | |
| if (options.CustomParameters_Update.Count > 0) | |
| Logger.Warn($"Custom CLI parameters [{string.Join(' ', options.CustomParameters_Update)}] will be discarded"); | |
| if (options.CustomParameters_Uninstall.Count > 0) | |
| Logger.Warn($"Custom CLI parameters [{string.Join(' ', options.CustomParameters_Uninstall)}] will be discarded"); | |
| Logger.Warn($"Install parameters [{string.Join(' ', options.CustomParameters_Install)}] are being discarded because CLI arguments are disabled."); | |
| if (options.CustomParameters_Update.Count > 0) | |
| Logger.Warn($"Update parameters [{string.Join(' ', options.CustomParameters_Update)}] are being discarded because CLI arguments are disabled."); | |
| if (options.CustomParameters_Uninstall.Count > 0) | |
| Logger.Warn($"Uninstall parameters [{string.Join(' ', options.CustomParameters_Uninstall)}] are being discarded because CLI arguments are disabled."); |
Install, update and uninstall custom CLI arguments will be different.
fix #3526