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.

Sign up for GitHub

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

Jump to bottom

semver module: missing MINOR, PATCH should not be ignored #17

Closed
zzamboni opened this issue Jun 29, 2021 · 5 comments
Closed

semver module: missing MINOR, PATCH should not be ignored #17

zzamboni opened this issue Jun 29, 2021 · 5 comments

Comments

@zzamboni
Copy link
Owner

(Reported by @krader1961)

According to the documentation:

A normal version number MUST take the form X.Y.Z where X, Y, and Z are non-negative integers

Currently semver considers missing Y or Z as zeros, so that:

> semver:eq 1.0 1.0.0
▶ $true

This should produce an error - maybe there should be a &non-strict option to allow parsing non-strictly-compliant numbers.
@zzamboni zzamboni mentioned this issue Jun 29, 2021
Closed
@krader1961
Copy link
Contributor

There is also the question of how blatantly invalid semvers are handled; e.g., "x.y.z". IMHO all such cases should throw an exception by default and the user should have to explicitly use a &nonstrict option if they want to be lenient. The exception being a "v" (or "V") prefix since far too many otherwise valid semantic version strings include that invalid prefix.

@zzamboni
Copy link
Owner Author

I think I'm going to do some work on the module adding the official regular expression to check for validity. A &nonstrict option would allow a [Vv] at the beginning, but probably no more.

@krader1961
Copy link
Contributor

I think I'm going to do some work on the module adding the official regular expression....

I think that is a good idea. Note that a leading "v" was valid in SemVer 1.0.0 but is invalid in the 2.0.0 spec. No idea why that change was made but even if it is a good idea for the SemVer spec I think a "v" prefix should be silently ignored if the string is otherwise valid. That is, always preface the official regexp with [vV]?. Obviously that's just my opinion and opinions are like assholes: everyone has one. 😄 It is perfectly reasonable to make the default behavior strictly conforming and require a &nonstrict option if the user wants to be lenient on this point. I've created PR elves/elvish#1348 that incidentally removes the leading "v" from the Elvish semver string.

zzamboni added a commit that referenced this issue Jul 1, 2021
@zzamboni
Improve adherence to SemVer spec
98da838 
Rewrite of the parsing code to use the official semver regex and to
improve the algorithm, both according to the spec.

This fixes #16 and #17.

Changes:

- New function semver:validate which throws an exception for invalid
  version strings.
- New function semver:parse which returns a map containing the parts of
  a valid version string. Uses semver:validate automatically.
- The main semver:cmp function now ignores the build metadata, if
  present.
- All validation, parsing and comparison functions now can take an
  &allow-v option which allows a "v" or "V" at the beginning without
  raising an error (this is not part of the spec but it's common use).
  The default behavior can be set by assigning to
  `$semver:allow-v-default`.
@zzamboni zzamboni mentioned this issue Jul 1, 2021
Merged
@zzamboni
Copy link
Owner Author

zzamboni commented Jul 1, 2021

@krader1961 please check out #18.

@zzamboni
Copy link
Owner Author

I think #18 addresses this issue, so I'm closing it. @krader1961 feel free to reopen if there's something still missing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@zzamboni @krader1961