Skip to content
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

Release notes does not tolerate prerelease versions #1669

Closed
mynkow opened this issue Jul 2, 2017 · 7 comments · Fixed by #3568
Closed

Release notes does not tolerate prerelease versions #1669

mynkow opened this issue Jul 2, 2017 · 7 comments · Fixed by #3568
Assignees
Labels
Milestone

Comments

@mynkow
Copy link

mynkow commented Jul 2, 2017

What You Are Seeing?

Release notes does not tolerate prerelease versions

What is Expected?

Allow prerelease version in release notes

What version of Cake are you using?

0.20

Solution

The problem is in this regex here =>

_versionRegex = new Regex(@"([0-9]+\.)+[0-9]+");

To fix this just use the following regex:
(?<Version>\d+(\s*\.\s*\d+){0,3})(?<Release>-[a-z][0-9a-z-]*)?

Source => https://github.com/NuGet/NuGet2/blob/2.13/src/Core/SemanticVersion.cs

@sgronlund
Copy link
Contributor

Hello! Is this issue still up for grabs? 😃

@augustoproiete
Copy link
Member

Yes, go for it!

@sgronlund
Copy link
Contributor

Hello again! Looking for some feedback.
As the previous asignee might have noticed the parse-method from the Version-class is what's causing problems, and not the regex! The regex functions perfectly fine but the parse-method cannot parse system versions with preleases number which contain characters and not simply digits, i.e. 1.0.0-alpha1 or 1.0.0-beta1 makes the method thrown an error.

Now the question is, should the ReleaseNoteParser-class try to change alpha or beta to some numerical value or does it make sense to introduce a NuGeT package which has a parser for this type of releases? Since this is my first time contributing to this project I'd like to get feedback since an import might be something you'd not want to have 😺

@devlead
Copy link
Member

devlead commented Oct 9, 2021

Don't think we need a new dependency for that, but I wrote the below code for SemVer a while back, maybe it could sufficient?

public struct SemVersion : IComparable, IComparable<SemVersion>, IEquatable<SemVersion>
	{
        public static SemVersion Zero { get; } = new SemVersion(0,0,0, null, null, "0.0.0");

		static readonly Regex SemVerRegex =
			new Regex (
				@"(?<Major>0|(?:[1-9]\d*))(?:\.(?<Minor>0|(?:[1-9]\d*))(?:\.(?<Patch>0|(?:[1-9]\d*)))?(?:\-(?<PreRelease>[0-9A-Z\.-]+))?(?:\+(?<Meta>[0-9A-Z\.-]+))?)?",
				RegexOptions.CultureInvariant | RegexOptions.ExplicitCapture | RegexOptions.IgnoreCase
			);


		public int Major { get; }
		public int Minor { get; }
		public int Patch { get; }
		public string PreRelease { get; }
		public string Meta { get; }
        public bool IsPreRelease { get; }
        public bool HasMeta { get; }
        public string VersionString { get; }

        public SemVersion (int major, int minor, int patch, string preRelease = null, string meta = null) :
	        this (major, minor, patch, preRelease, meta, null)
        {
        }

        SemVersion (int major, int minor, int patch, string preRelease, string meta, string versionString)
		{
			Major = major;
			Minor = minor;
			Patch = patch;
			IsPreRelease = !string.IsNullOrEmpty (preRelease);
			HasMeta = !string.IsNullOrEmpty (meta);
			PreRelease = IsPreRelease ? preRelease : null;
			Meta = HasMeta ? meta : null;

			if (!string.IsNullOrEmpty (versionString)) {
				VersionString = versionString;
			} else {
				var sb = new StringBuilder ();
				sb.AppendFormat (CultureInfo.InvariantCulture, "{0}.{1}.{2}", Major, Minor, Patch);

				if (IsPreRelease) {
					sb.AppendFormat (CultureInfo.InvariantCulture, "-{0}", PreRelease);
				}

				if (HasMeta) {
					sb.AppendFormat (CultureInfo.InvariantCulture, "+{0}", Meta);
				}

				VersionString = sb.ToString ();
			}
		}

		public static bool TryParse (string version, out SemVersion semVersion)
		{
			semVersion = Zero;

			if (string.IsNullOrEmpty(version)) {
				return false;
			}

			var match = SemVerRegex.Match (version);
			if (!match.Success) {
				return false;
			}

			if (!int.TryParse (
				    match.Groups["Major"].Value,
				    NumberStyles.Integer,
				    CultureInfo.InvariantCulture,
				    out var major) ||
			    !int.TryParse (
				    match.Groups["Minor"].Value,
				    NumberStyles.Integer,
				    CultureInfo.InvariantCulture,
				    out var minor) ||
			    !int.TryParse (
				    match.Groups["Patch"].Value,
				    NumberStyles.Integer,
				    CultureInfo.InvariantCulture,
				    out var patch)) {
				return false;
			}

			semVersion = new SemVersion (
				major,
				minor,
				patch,
				match.Groups["PreRelease"]?.Value,
				match.Groups["Meta"]?.Value,
				version);

			return true;
		}

		

		public bool Equals (SemVersion other)
		{
			return Major == other.Major
			       && Minor == other.Minor
			       && Patch == other.Patch
			       && string.Equals(PreRelease, other.PreRelease, StringComparison.OrdinalIgnoreCase)
			       && string.Equals(Meta, other.Meta, StringComparison.OrdinalIgnoreCase);
		}

		public int CompareTo (SemVersion other)
		{
			if (Equals(other))
			{
				return 0;
			}

			if (Major > other.Major) {
				return 1;
			}

			if (Major < other.Major) {
				return -1;
			}

			if (Minor > other.Minor) {
				return 1;
			}

			if (Minor < other.Minor) {
				return -1;
			}

			if (Patch > other.Patch) {
				return 1;
			}

			if (Patch < other.Patch) {
				return -1;
			}

            switch(StringComparer.InvariantCultureIgnoreCase.Compare(PreRelease, other.PreRelease)) {
				case 1:
					return 1;

				case -1:
					return -1;

                default:
	                return StringComparer.InvariantCultureIgnoreCase.Compare (Meta, other.Meta);
			}
		}

		public int CompareTo (object obj)
		{
			return (obj is SemVersion semVersion)
                ? CompareTo (semVersion)
				: -1;
		}

		public override bool Equals (object obj)
		{
			return (obj is SemVersion semVersion)
			       && Equals (semVersion);
		}

		public override int GetHashCode ()
		{
			unchecked {
				var hashCode = Major;
				hashCode = (hashCode * 397) ^ Minor;
				hashCode = (hashCode * 397) ^ Patch;
				hashCode = (hashCode * 397) ^ (PreRelease != null ? StringComparer.OrdinalIgnoreCase.GetHashCode (PreRelease) : 0);
				hashCode = (hashCode * 397) ^ (Meta != null ? StringComparer.OrdinalIgnoreCase.GetHashCode (Meta) : 0);
				return hashCode;
			}
		}

		public override string ToString ()
			=> VersionString;

		// Define the is greater than operator.
		public static bool operator > (SemVersion operand1, SemVersion operand2)
            => operand1.CompareTo (operand2) == 1;

		// Define the is less than operator.
		public static bool operator < (SemVersion operand1, SemVersion operand2)
			=> operand1.CompareTo (operand2) == -1;

		// Define the is greater than or equal to operator.
		public static bool operator >= (SemVersion operand1, SemVersion operand2)
			=> operand1.CompareTo (operand2) >= 0;

		// Define the is less than or equal to operator.
		public static bool operator <= (SemVersion operand1, SemVersion operand2)
			=> operand1.CompareTo (operand2) <= 0;
	}

@sgronlund
Copy link
Contributor

sgronlund commented Oct 9, 2021

@devlead Just by skimming through it seems to fix the problem with having prerelease in the version, would you like me to introduce this class into the code base or since it's yours would you prefer to do it?

Or should this issue be removed and a new one be added for replacing the Version-class with your SemanticVersion-class?

@devlead
Copy link
Member

devlead commented Oct 9, 2021

Start with adding a new SemVersion property and use that to populate Version property.

@cake-build-bot
Copy link

🎉 This issue has been resolved in version v2.0.0 🎉

The release is available on:

Your GitReleaseManager bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants