Skip to content

Fix misleading "Installation options" label on Update/Uninstall operation logs#5037

Open
Scott (7ranceaddic7) wants to merge 1 commit into
Devolutions:mainfrom
7ranceaddic7:fix/operation-log-label-mismatch
Open

Fix misleading "Installation options" label on Update/Uninstall operation logs#5037
Scott (7ranceaddic7) wants to merge 1 commit into
Devolutions:mainfrom
7ranceaddic7:fix/operation-log-label-mismatch

Conversation

@7ranceaddic7

@7ranceaddic7 Scott (7ranceaddic7) commented Jul 3, 2026

Copy link
Copy Markdown
  • I have read the contributing guidelines, and I agree with the Code of Conduct.
  • Have you checked that there aren't other open pull requests for the same changes?
  • Have you tested that the committed code can be executed without errors?
  • Have you confirmed that this issue is caused by UniGetUI itself, and not by the package manager or the package involved?

UpdatePackageOperation.Initialize() and UninstallPackageOperation.Initialize() in PackageOperations.cs both logged their Options.ToString() dump under the header "Installation options: ", copy-pasted from InstallPackageOperation. Since InstallOptions already has three independent fields (CustomParameters_Install, CustomParameters_Update, CustomParameters_Uninstall) that are correctly applied per operation type (see NpmPkgOperationHelper._getOperationParameters), the mislabeled header was purely cosmetic, but it's actively misleading during debugging: a user who only set "Custom Install Arguments" sees that value echoed under an "Installation options:" banner during an update run, which reads as confirmation that the flag applies, when in fact it's CustomParameters_Update (a separate, empty field) that actually gets appended to the update command.

This surfaced while diagnosing why --legacy-peer-deps wasn't being applied to npm package updates: the engine behavior was correct, but the log was telling a different story.

Change: rename the header to "Update options: " and "Uninstall options: " in their respective operation classes. No behavior change, no new abstraction; matches the existing per-class string literal style already used for Metadata.Title/Metadata.Status/etc. in the same file.

Verified with dotnet build (0 errors) and dotnet test on UniGetUI.PackageEngine.Tests (252/252 passing, no regressions) against SDK 10.0.301.

N/A. No existing issue tracks this; not creating one per the PR template guidance.

…tion logs

UpdatePackageOperation and UninstallPackageOperation both logged their
options dump under the header "Installation options:", inherited from
InstallPackageOperation via copy-paste. Since InstallOptions.ToString()
already renders CustomParameters_Install/_Update/_Uninstall separately
by field name, this only affected the header text -- but it's actively
misleading: users configuring a package's "Custom Install Arguments"
field see that same value echoed under an "Installation options:"
banner during an update run, which reads as confirmation that the
install-time argument applies to updates too, even though it doesn't
(CustomParameters_Update is the field that actually gets appended for
update operations, per NpmPkgOperationHelper._getOperationParameters).
@7ranceaddic7

Scott (7ranceaddic7) commented Jul 3, 2026

Copy link
Copy Markdown
Author

Real-world verification, beyond the build/test run noted above:

Confirmed the underlying claim of this PR (that CustomParameters_Update was already being correctly applied by the engine, and the only bug was the misleading log label) end to end on a real machine:

  1. Called the actual production code path (manager.OperationHelper.GetParameters(package, options, OperationType.Update), the same method NpmPkgOperationHelper and the command-preview feature use) with CustomParameters_Update = ["--legacy-peer-deps"]. It generated:
    npm install 'markdownlint-cli2@0.23.0' --legacy-peer-deps
    
  2. Ran that exact command against a real project that has the ESLint-9/10-coexistence peer conflict this argument is meant to work around. Result: exit code 0, "up to date, audited 357 packages," 0 vulnerabilities, no ERESOLVE.

This was done via a temporary local test (not committed) calling GetParameters directly, then executing the returned command for real. It's not a hand-written approximation of what the tool "should" produce. No changes to this PR's diff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant