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

the example 1.2.3-alpha.10.beta.0+build.unicorn.rainbow in README is no longer valid #18

Closed
KagamiChan opened this issue Jan 4, 2020 · 6 comments · Fixed by #19
Closed

Comments

@KagamiChan
Copy link

node
Welcome to Node.js v12.6.0.
Type ".help" for more information.
> const semverRegex = require('semver-regex')
undefined
> semverRegex().test('1.2.3-alpha.10.beta.0+build.unicorn.rainbow')
false
> semverRegex().test('1.2.3-alpha.0')
false
> semverRegex().test('1.2.3-alpha.1')
true

while in semver package, .0 is considered correct and in npm doc we can see this usage

@KagamiChan
Copy link
Author

after looking at the history I think #15 is related, then we need to update the example and it will be great to add some notes and an example

@KagamiChan
Copy link
Author

KagamiChan commented Jan 4, 2020

But I don't think the mentioned semver clause 9 forbids this because their grammar as well as the given regex suggest that alpha.0 is still valid

@stroncium

@stroncium
Copy link
Contributor

stroncium commented Jan 22, 2020

@KagamiChan Sorry it took so long to answer.

Quoting https://semver.org/,

  1. A pre-release version MAY be denoted by appending a hyphen and a series of dot separated identifiers immediately following the patch version. Identifiers MUST comprise only ASCII alphanumerics and hyphen [0-9A-Za-z-]. Identifiers MUST NOT be empty. Numeric identifiers MUST NOT include leading zeroes. Pre-release versions have a lower precedence than the associated normal version. A pre-release version indicates that the version is unstable and might not satisfy the intended compatibility requirements as denoted by its associated normal version. Examples: 1.0.0-alpha, 1.0.0-alpha.1, 1.0.0-0.3.7, 1.0.0-x.7.z.92.

From RFC2119:

  1. MUST NOT This phrase, or the phrase "SHALL NOT", mean that the
    definition is an absolute prohibition of the specification.

Also, looking into BNF(available at https://semver.org/ too):

<valid semver> ::= <version core>
                 | <version core> "-" <pre-release>
                 | <version core> "+" <build>
                 | <version core> "-" <pre-release> "+" <build>

<pre-release> ::= <dot-separated pre-release identifiers>

<dot-separated pre-release identifiers> ::= <pre-release identifier>
                                          | <pre-release identifier> "." <dot-separated pre-release identifiers>

<pre-release identifier> ::= <alphanumeric identifier>
                           | <numeric identifier>

<numeric identifier> ::= "0"
                       | <positive digit>
                       | <positive digit> <digits>

<digit> ::= "0"
          | <positive digit>

<positive digit> ::= "1" | "2" | "3" | "4" | "5" | "6" | "7" | "8" | "9"

I don't think there is a chance of me misunderstanding specification.

@KagamiChan
Copy link
Author

KagamiChan commented Jan 24, 2020

Hi @stroncium

Thanks for your reply

I think you're referring "must not include leading zeros", but a single zero does not seem to apply to this case, otherwise we cannot express the 0 itself

and in the BNF, <numeric identifier> has a special case "0", which could be filled into the <dot-separated pre-release identifiers>

In fact, the suggested regexp in semver.org also allows the form '1.0.0-beta.0', you may try this regex in regex101 via the link in the answer

stroncium added a commit to stroncium/semver-regex that referenced this issue Jan 24, 2020
@stroncium
Copy link
Contributor

@KagamiChan Oh hell, sorry. I actually read though this BNF like 5 times when implementing original patch and twice while replying to this issue, and every damn time I managed to overlook this cunning "0" option for <numeric identifier>

@KagamiChan
Copy link
Author

Thanks for you quick fix!

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

Successfully merging a pull request may close this issue.

2 participants