zafarkhaja/jsemver

AND and OR operators differ from common semver range implementations

Closed this issue · 9 comments

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.

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

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 commented

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

@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 commented

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?

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.

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

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!)

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