Support for numeric only release labels with floating ranges #1601
Conversation
* Allow 1.0.0-* to float from 1.0.0-0 instead of 1.0.0-- which is a non-numeric label. * Treat floating labels that end with . in the same way as labels ending in - when converting to non-snapshot versions for display purposes. Fixes NuGet/Home#4513
emgarten
requested review from
rohit21agrawal,
PatoBeltran,
nkolev92,
zhili1208,
alpaix,
mishra14 and
jainaashish
| get { return _minVersion != null; } | ||
| } | ||
|
|
||
| public bool HasMinVersion => _minVersion != null; |
| @@ -202,17 +197,16 @@ public static bool TryParse(string versionString, out FloatRange range) | |||
| { | |||
| releasePrefix = actualVersion.Substring(versionString.LastIndexOf('-') + 1); | |||
|
|
|||
| if (actualVersion.EndsWith("-")) | |||
| // For numeric labels 0 is the lowest. For alpha-numeric - is the lowest. | |||
| if (releasePrefix.Length == 0 || actualVersion.EndsWith(".")) | |||
There was a problem hiding this comment.
The previous code had a comment around actualVersion.EndsWith(".") - // TODO: Solve this better.
Have we addressed that? If not then we should leave the comment there.
There was a problem hiding this comment.
I would say it is solved, it was probably due to speculation that a bug like that one fixed here could exist. SemVer 2.0.0 was very new at that time.
| { | ||
| // Remove the - for 1.0.0-* (1.0.0--) | ||
| fixedReleaseLabel = lastLabel.Substring(0, lastLabel.Length - 1); | ||
| if (lastLabel.EndsWith("--", StringComparison.Ordinal)) |
There was a problem hiding this comment.
Can be simplified as -
// Remove the - for 1.0.0-* (1.0.0--)
var charsToRemove = 1;
if (lastLabel.EndsWith("--", StringComparison.Ordinal))
{
// For labels such as rc1-* an additional - is added by nuget
charsToRemove = 2;
}
fixedReleaseLabel = lastLabel.Substring(0, lastLabel.Length - charsToRemove);
There was a problem hiding this comment.
that is more compact, do you think the flow/intent is as obvious there?
There was a problem hiding this comment.
I personally like that style. it implies that we have a default behavior which is over written in some cases. Your call! :)
There was a problem hiding this comment.
Normally I'd agree, but here the default behavior is to do nothing. Two special cases exists where extra truncation is needed. Appreciate the extra feedback on it though.
For alpha-numeric labels in SemVer
-is the lowest possible label in lexicographical order. In SemVer 2.0.0 numeric labels are lower than alpha-numeric labels per the rules, which puts0before-despite the lexicographical order.When converting
1.0.0-*to the min version of range a release label is needed to avoid ending up with1.0.0which is stable. To handle this NuGet adds the lowest possible release label as a place holder, previously it was incorrectly using-, now it will use0only when the label is empty.This extra label is primarily an internal thing, when displayed to users it is removed using ToNonSnapshotVersion, the result of which is only for display, it isn't used for comparing versions.
I've added more tests for this, and there are quite a few existing tests around this already verifying that the behavior is the same.
Fixes NuGet/Home#4513