Consolidate UI version parse/normalize into shared VersionText (fixes latent two-part-version throw)#1510
Merged
Conversation
…latent two-part-version throw)
Six sites across the Dashboard and Lite server-list / landing / health views hand-rolled
the same snippet:
Version.TryParse(v, out p) ? new Version(p.Major, p.Minor, p.Build).ToString() : v
which carries a latent crash: Version.TryParse("3.1") succeeds with Build == -1, and
new Version(3, 1, -1) throws ArgumentOutOfRangeException. So every one of those sites
would throw on a two-part version string. It is the same class the installer had (fixed
in ScriptProvider.TryParseVersionCore, #1498); this is the UI-layer twin.
New PerformanceMonitor.Ui.VersionText -- Parse (strip +build / -prerelease, clamp Build
>= 0, return null on garbage) and Normalize (three-part display, trimmed original on
failure, empty on blank). Both Dashboard and Lite already reference Ui. The five
Dashboard sites (MainWindow, ManageServersWindow x2, LandingPage, ServerHealthStatus,
ServerListItem) and the one Lite site now call it; the redundant per-call "+"-strips and
copy-pasted local Normalize helpers are gone.
Left SingleInstanceDecision.ParseProductVersion (also in Ui) alone on purpose: it parses
the running processes' always-four-part assembly versions for comparison and deliberately
does NOT clamp; it is on the tested single-instance path, and its contract differs. A
cross-reference comment in VersionText records the distinction.
Pinned by 23 VersionTextTests in Lite.Tests (CI-wired, already references Ui): the
two-part clamp as an explicit "no longer throws" regression, suffix stripping, four-part
collapse, null/blank/garbage, ordering, and the deliberate Revision-drop. Verified the
tests FAIL against the un-clamped form (3 failures), not assumed. All 1158 Lite.Tests and
732 Dashboard.Tests green; behavior-preserving for every realistic input (valid versions
and status strings), the throw being the only changed outcome.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Six sites across the Dashboard and Lite server-list / landing / health views hand-rolled the same version snippet:
That carries a latent crash:
Version.TryParse("3.1")succeeds withBuild == -1, andnew Version(3, 1, -1)throwsArgumentOutOfRangeException. Every one of those sites would throw on a two-part version string — the same class the installer had, fixed inScriptProvider.TryParseVersionCore(#1498). This is the UI-layer twin.The change
PerformanceMonitor.Ui.VersionText:Parse(strip+build/-prerelease, clampBuild >= 0, null on garbage) andNormalize(three-part display; trimmed original on failure; empty on blank). Both apps already referenceUi.MainWindow,ManageServersWindow×2,LandingPage,ServerHealthStatus,ServerListItem) and the one Lite site (ManageServersWindow) now call it. The redundant per-call+-strips and copy-pasted localNormalizehelpers are gone.SingleInstanceDecision.ParseProductVersion(also inUi) alone on purpose — it parses the always-four-part assembly versions for the tested single-instance comparison and deliberately does not clamp; different contract. A cross-reference comment inVersionTextrecords the distinction.Tests
23
VersionTextTestsinLite.Tests(CI-wired, already referencesUi): the two-part clamp as an explicit "no longer throws" regression, suffix stripping, four-part collapse, null/blank/garbage passthrough, ordering, and the deliberate Revision-drop. Verified the tests fail against the un-clamped form (3 failures) rather than assuming it.Behavior-preserving for every realistic input (valid versions and status strings) — the throw is the only changed outcome. All 1158 Lite.Tests and 732 Dashboard.Tests green.
🤖 Generated with Claude Code