search

composer / semver

Semantic versioning utilities with the addition of version constraints parsing and checking.
MIT License
3.15k stars 76 forks source link

MultiConstraint::__toString is not outputting a parsable constraint #54

Closed stof closed 5 years ago

stof commented 7 years ago

The string representation includes additional [ ] around the constraint, which means that parsing the string representation as a constraint fails

alcohol commented 7 years ago

It has been doing that since like forever though, and I am also not sure if the output is supposed to be reversible (as in, valid for parsing) or merely just an easier way to visually render it.

stof commented 7 years ago

Well, there is no way to get a reversible representation. which means that there is no way to get an existing Constraint object and build an argument for the VersionSelector of composer currently (which expects a string representation of the constraint). See https://github.com/composer/satis/pull/392 for such use case.

alcohol commented 7 years ago

Understood, I am just saying the purpose of __toString() is not properly documented. I will happily accept a change, but would prefer to also see consensus on what it should return at the same time, and making sure all other constraints behave in the same way.

stof commented 7 years ago

Constraint outputs a reversible string. MultiConstraint does not in __toString. It may do it in getPrettyString when it was created with a pretty string (but not all of them are, only the top-level constraint of the parsing IIRC). EmptyConstraint should return * rather than [] to be reversible (as this is what it represents)

Seldaek commented 7 years ago

Might be safe to add a new method on the Constraint classes to allow retrieving a stringified but parseable version of the constraint. Not sure what the consequences of modifying __toString would be tbh. It's pretty hard to check for usage of that..

alcohol commented 5 years ago

Should we close this, or do we actually want to come to a resolution?

Seldaek commented 5 years ago

I don't really see a strong need for this so maybe rather close.