(feat) add semver.org example

[dselman]

Signed-off-by: Dan Selman danscode@selman.org

Adds an example grammar that describes a SemVer.org v2 semantic version.

[hildjj]

This looks really good now.

Minor nits. If you like, I can merge it as-is.

[hildjj]

I'll fix this up after merge. Apparently I didn't leave a template in the changelog after 2.0.1, because that was done i n such a rush.

[hildjj]

Refactor parseInt into numericIdentifier.

[Mingun]

Moving parseInt to numericIdentifier may be not so good, because that rule is also used in contexts where you do not want a number, but only a match result. I'm fine to leave parsing here.

[hildjj]

then an intermediate rule that does the parsing, I think.

[hildjj]

Regardless, the $ should go in numericIdentifier.

[hildjj]

newline at end, please

[reverofevil]

Could you please create an issue to set up husky and some kind of linter, so that nobody has to enforce code style by hand?

[hildjj]

We don't have a linter for .peggy files yet, as far as I know. There should be one.

We lint all of the JS on each GHA run.

[hildjj]

Maybe this?

numericIdentifier
  = num:$(positiveDigit digit*) { return parseInt(num) }
  / '0' ![0-9] { return 0 }

[reverofevil]

TBH I don't understand how this works right now. positiveDigit digit* should return ['1', ['2', '3']], and parseInt(['1', ['2', '3']], 10) = 1.

[reverofevil]

I'd suggest a bit different code, though:

numericIdentifier = num:$('0' / [1-9][0-9]*) { return parseInt(num); }

IMO you don't have to write a lot of code to make it more readable. "Positive digit" doesn't add any more information to [1-9].

[hildjj]

Trying to keep it closer to https://semver.org/spec/v2.0.0.html I think.

[Mingun]

TBH I don't understand how this works right now.

The magic in the $ operator that returns the text from the input instead of results of subexpressions.

"Positive digit" doesn't add any more information to [1-9].

Agreed, especially since this rule is used only once

[reverofevil]

Oh, I missed $ up above.

[reverofevil]

Anyway, I didn't find a place in the spec that would limit numbers by any limit, so I suspect parseInt doesn't formally comply to the spec :/

[reverofevil]

Thank you for this. I automatically kept using return tail.unshift(head), tail;.

[hildjj]

It's in the docs now, look for "Parsing Lists".

[reverofevil]

According to spec, it should be identifierChar* nonDigit identifierChar*.

If you substitute everything into alphanumericIdentifier, it turns out the only reason why it exists is to avoid parse conflict in preReleaseIdentifier. If there is no nonDigit, it might match same things as numericIdentifier.

In PEG parsers there is no parse conflicts, it can be greatly simplified.

[reverofevil]

I think this is all you need to comply to the spec:

semver = major:numericId "." minor:numericId "." patch:numericId prerelease:("-" @prerelease)? build:("+" @build)? {
	return {major, minor, patch, prerelease, build};
}
prerelease = head:prereleaseId tail:("." @prereleaseId)* { return [head, ...tail]; }
build = head:alnumId tail:("." @alnumId)* { return [head, ...tail]; }
prereleaseId = alnumId / $("0" / [1-9][0-9]*)
alnumId = $[0-9a-z-]i+

[reverofevil]

Actually buildId is just alnumId. It will never hit the second branch. The only reason why grammar in spec distinguishes them, is to stress that numeric identifiers are sorted in numeric order. As this is a parser, and not comparator, it shouldn't really care.

[MarcelBolten]

I think this is all you need to comply to the spec:

semver = major:numericId "." minor:numericId "." patch:numericId prerelease:("-" @prerelease)? build:("+" @build)? {
	return {major, minor, patch, prerelease, build};
}
prerelease = head:prereleaseId tail:("." @prereleaseId)* { return [head, ...tail]; }
build = head:alnumId tail:("." @alnumId)* { return [head, ...tail]; }
prereleaseId = alnumId / $("0" / [1-9][0-9]*)
alnumId = $[0-9a-z-]i+

rule numericId is missing, maybe like so:

semver = major:numericId "." minor:numericId "." patch:numericId prerelease:("-" @prerelease)? build:("+" @build)? {
	return {major, minor, patch, prerelease, build};
}
prerelease = head:prereleaseId tail:("." @prereleaseId)* { return [head, ...tail]; }
build = head:alnumId tail:("." @alnumId)* { return [head, ...tail]; }
prereleaseId = alnumId / numericId 
alnumId = $[0-9a-z-]i+
numericId = $("0" / [1-9][0-9]*)

However, while this will successfully parse all valid examples it will also successfully parse invalid examples (1.2.3-0123, 1.2.3-0123.0123) but "numeric identifiers MUST NOT include leading zeroes" in the pre-release part.

[reverofevil]

Don't think that hiding most important information into a deeper object with non-descript name is a good idea. At least {...versionCore, pre, build}, please.

[hildjj]

@dselman sorry this is getting so many nitpicks from all of us here. I think it's just because this seems so useful to have as an example!

[reverofevil]

@hildjj Say for yourself. I personally just like nitpicking (and very sorry for that).

[dselman]

@dselman sorry this is getting so many nitpicks from all of us here. I think it's just because this seems so useful to have as an example!

No problem, at all. I'm enjoying the discussion and learning! I suggest this is merged (functionally correct) and then another PR can be created to refactor it to be more concise and/or elegant. That takes me off the critical path. How does that sound?

[hildjj]

another PR can be created to refactor it to be more concise and/or elegant

We can do that. I think the most efficient way would be for me to create a new branch here on the main repo, edit this PR to target that branch, we iterate there until we're done, then another PR to pull that branch into main. I'm starting that process now, with the branch name semver-grammar.

[hildjj]

I'm going to go through all of the comments here, expect a PR with my changes, which I'll link here.