zafarkhaja / jsemver

Java implementation of the SemVer Specification
MIT License
429 stars 82 forks source link

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

Closed bripkens closed 6 months ago

bripkens commented 8 years ago

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 commented 8 years ago

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
zafarkhaja commented 8 years ago

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 8 years ago

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

zafarkhaja commented 8 years ago

@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 8 years ago

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 commented 8 years ago

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 commented 6 months ago

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

yuchi commented 6 months ago

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 commented 5 months ago

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