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

AND and OR operators differ from common semver range implementations #23

Closed
bripkens opened this issue Nov 1, 2015 · 9 comments
Closed

Comments

@bripkens
Copy link

bripkens commented Nov 1, 2015

I noticed that the OR (|) notation differs from other implementation like node-semver and the Haskell implementation.

Example:

jsemver: <1 | >3
node semver: <1 || >3

Is this on purpose or accidental?

For me this is quite unfortunate as I cannot parse security advisories due to notational differences.

@bripkens
Copy link
Author

bripkens commented Nov 1, 2015

Just noticed the same thing for the and operator which both other libraries define as whitespace.

Example:

jsemver: <3.11 | (>= 4 & <4.5)
node semver: <3.11 | >= 4 <4.5

@bripkens bripkens changed the title OR differs from other common semver range implementations AND and OR operators differ from common semver range implementations Nov 1, 2015
@zafarkhaja
Copy link
Owner

Unfortunately it was made so intentionally. Here is the final grammar and here is my motivation behind it.

However, I'm planning to cover the whole node-semver grammar in future releases as an extension to the library along with some other common grammars such as Maven's version ranges.

@yuchi
Copy link

yuchi commented Apr 27, 2016

There are a lot of differences currently. Would you accept a PR to add full node semver compatibility?

@zafarkhaja
Copy link
Owner

@yuchi it wasn't supposed to be fully compatible with node-semver. I have previously accepted a change introducing some compatibility but that's because those changes didn't break any existing grammar rules. Adding full compatibility, like removing support for parentheses and using whitespaces for logical AND operator, would result in a completely different grammar which wasn't my goal in the first place. Combining the two grammars, on the other hand, would make the grammar and the parsing more complicated than it needs to be. However, as I mentioned in the comment prior to yours, I wouldn't mind adding the node-semver ranges support as an extension. I know, it's been a while since I "promised" the new version of the library with support for extensions but the good news is that I'm already working on the new parser which would allow for different grammars.

If you still want to contribute with less radical changes we can discuss those in advance. I would also be very happy to accept your help with the node-semver grammar parser when the time comes. Thank you.

@yuchi
Copy link

yuchi commented Apr 27, 2016

What I’d like jsemver to support is a superset of node-semver expressions.

If I can make jsemver pass node-semver unit tests, without adding too much o it, would you review the changes?

@zafarkhaja
Copy link
Owner

It's not about amount of code you'll add, it's about complexity you'll bring making the code less readable and maintainable. If you can do that without complicating the parser then sure. why not, I'll gladly review the changes. But I still believe the two grammars should be kept separate each in its own parser.

@zafarkhaja
Copy link
Owner

Wasn't very hard to implement after all. Will be shipped with 0.10.0.

@zafarkhaja zafarkhaja self-assigned this Jan 4, 2024
@yuchi
Copy link

yuchi commented Jan 8, 2024

Please also have a look at https://github.com/yuchi/java-npm-semver which passes the full npm’s semver test suite (as of 2017!)

@zafarkhaja
Copy link
Owner

Hey Yuchi, thanks for the link! This might come useful once I start implementing the node-semver ranges syntax.

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

No branches or pull requests

3 participants