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

[BUG] BNF does not match the implementation #665

Open
1 task done
ShuiRuTian opened this issue Dec 6, 2023 · 3 comments
Open
1 task done

[BUG] BNF does not match the implementation #665

ShuiRuTian opened this issue Dec 6, 2023 · 3 comments
Labels
Bug thing that needs fixing Needs Triage needs an initial review

Comments

@ShuiRuTian
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

semver.valid('1.2.3-00') // 'null'
semver.valid('1.2.3+00') // '1.2.3'

However, according to the BNF

qualifier  ::= ( '-' pre )? ( '+' build )?
pre        ::= parts
build      ::= parts
parts      ::= part ( '.' part ) *
part       ::= nr | [-0-9A-Za-z]+

pre and build are just the same.

Expected Behavior

The behavior of program is correct, but BNF is not.

We need to update the BNF form.

Steps To Reproduce

No response

Environment

  • npm: 9.9.1
  • Node: v18.17.0
  • OS: windows
@ShuiRuTian ShuiRuTian added Bug thing that needs fixing Needs Triage needs an initial review labels Dec 6, 2023
@ShuiRuTian ShuiRuTian changed the title [BUG] <title> BNF does not match the implmentation [BUG] BNF does not match the implementation Dec 6, 2023
@mbtools
Copy link
Contributor

mbtools commented Feb 1, 2024

Agree. Numeric parts of a pre-release must not have leading zero. Also the part definition isn't BNF but a regex.

How about this?

alphanum   ::= ( ['0'-'9'] | ['A'-'Z'] | ['a'-'z'] | '-' ) +
pre        ::= ( nr | alphanum ) ( '.' ( nr | alphanum ) ) *
build      ::= alphanum ( '.' alphanum ) *

PS: This isn't perfect since alphanum could be a just numbers starting with a zero but the definition makes the difference between pre-release and build much clearer.

@ShuiRuTian
Copy link
Author

Sorry for the late response. I was on Chinese Spring Festival.

It looks good to me!

And just out of curious, why we did not keep it same with(or close to) Semver?

@mbtools
Copy link
Contributor

mbtools commented Feb 25, 2024

Because it was invented 10 years ago. I'm working on a update to a formal EBNF that will settle several issues (PR after my vacation)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Needs Triage needs an initial review
Projects
None yet
Development

No branches or pull requests

2 participants