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

[QUESTION/FEATURE] Usage of commas inside constraints. #468

Open
internalsystemerror opened this issue Aug 11, 2022 · 6 comments
Open

[QUESTION/FEATURE] Usage of commas inside constraints. #468

internalsystemerror opened this issue Aug 11, 2022 · 6 comments
Labels
spec related to the semver spec

Comments

@internalsystemerror
Copy link

OK, first allow me to lay out the scenario:

We are using a JS app to parse PHP composer.json files and wish to use this library to compare a version number with that constraint.

Currently: What is considered a valid constraint for composer >=5.6,<=8.1.99 will not parse with this library.

The Hopeful Ideal: That we can use this library to parse this constraint with the , being considered the same as a .

For background, there is an issue that pertains to this, however I will try to lay out the argument here.

I tried to lookup whether the semver specification says anything regarding specifying version ranges but came up empty handed. As I understand it PHP's composer may not be the only library which parses a version range constraint in this way, some examples:

https://docs.microsoft.com/en-us/nuget/concepts/package-versioning#version-ranges
https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#multiple-requirements
https://golang.github.io/dep/docs/Gopkg.toml.html#version (deprecated I believe with the new approach being specific versions not ranges)

I'm sure there are more, so my request is if either, a comma could be considered a valid alternative to a space within this package, or if the .coerce() or .clean() methods could be modified for use in these instances? Or maybe there already exists a way and this is all for naught. Thanks for considering this either way.

@internalsystemerror internalsystemerror changed the title [QUESTION] Usage of commas inside constraints. [QUESTION/FEATURE] Usage of commas inside constraints. Aug 11, 2022
@ljharb
Copy link

ljharb commented Aug 11, 2022

It doesn’t yet, but when it does it’ll be like npm does it - spaces, not commas.

@boesing
Copy link

boesing commented Aug 12, 2022

The question would be: why not supporting commas if it is at least supported by other semver parsers in 3 different languages?

PHPs composer/semver implementation does support it as well:
https://getcomposer.org/doc/articles/versions.md#version-range

@ljharb
Copy link

ljharb commented Aug 12, 2022

Because adding that complexity isn't particularly valuable - there's one way to do it; we don't need two.

@internalsystemerror
Copy link
Author

@ljharb I think you're talking about changing the official npm specification. I'm asking if this package can support semver ranges as currently implemented outside of npm so we can use it parse values from other semver compliant package managers.

Since that would no doubt have the side effect of what would be allowed within package.json, the fallback request is whether this package could clean/coerce these non-npm-valid ranges into npm-valid ones. The 3 package manager formats listed in this issue were just the first 3 I checked (add in PHP and that's 4).

Assuming that semver were to add ranges to their specification, I would also find it unlikely that they would reject the use of a comma when it already has widespread use.

@ljharb
Copy link

ljharb commented Aug 12, 2022

This package is exclusively for npm. You could comment on the ranges PR on the semver spec about it tho, I'm sure

@internalsystemerror
Copy link
Author

internalsystemerror commented Aug 12, 2022

This package is exclusively for npm.

It's a published publicly available package so you'll forgive my assumption then.

You could comment on the ranges PR on the semver spec about it tho, I'm sure.

I wasn't aware there was one, but thanks for the heads up!

(Edit: linking the PR for reference semver/semver#584)

@lukekarrys lukekarrys added the spec related to the semver spec label Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec related to the semver spec
Projects
None yet
Development

No branches or pull requests

4 participants