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

SemVer range represents range-set #3967

Closed
timreichen opened this issue Dec 15, 2023 · 4 comments
Closed

SemVer range represents range-set #3967

timreichen opened this issue Dec 15, 2023 · 4 comments

Comments

@timreichen
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Ref: #3957 (comment)

The semver mod was taken over from node-semver. range actually represents a range-set.
I opened an issue on node-semver to find out the reason why.

We should differentiate between range and range-set to avoid confusion.

Describe the solution you'd like

rename range files and functions to range_set aka. RangeSet and deprecate all old functions.
In a next step, reintroduce old range functions with the implementation of a real range.

Describe alternatives you've considered

@iuioiua
Copy link
Contributor

iuioiua commented Dec 17, 2023

Another suggested name would be Range. If we proceed with the suggested change, we should also promote the ranges property of SemVerRange as the value itself. It being nested looks unnecessary. I.e. functions should use ranges instead of the current range.ranges.

Another requirement we should question is whether the ranges property needs to be a nested array. In other words, is the outer array being used for OR comparisons and inner arrays being used for AND comparisons actually necessary? I expect that it is necessary, but I think it should be questioned. If it were found to not be necessary, we could just have a ranges type that is a flat array with a depth of 1 that could make things much simpler.

I hope my thoughts make sense. Happy to try clarify if they don't.

@timreichen
Copy link
Contributor Author

timreichen commented Dec 18, 2023

Another suggested name would be Range. If we proceed with the suggested change, we should also promote the ranges property of SemVerRange as the value itself. It being nested looks unnecessary. I.e. functions should use ranges instead of the current range.ranges.

What do you mean exactly with "promoting the ranges property"?

Another requirement we should question is whether the ranges property needs to be a nested array. In other words, is the outer array being used for OR comparisons and inner arrays being used for AND comparisons actually necessary? I expect that it is necessary, but I think it should be questioned. If it were found to not be necessary, we could just have a ranges type that is a flat array with a depth of 1 that could make things much simpler.

imo a Range should be defined as the following:

type Range = [Comparator, Comparator]

which actually represents a range between two versions, like a segment on a line:
Example: >=1.2.3 [AND] <2.0.0.

A RangeSet represents multiple segments on the line and therefore needs to be an array of ranges:

type RangeSet = Range[] // [Comparator, Comparator][]

Example: >=1.2.3 [AND] <2.0.0 [OR] >=3.0.0 [AND] <4.0.0

It therefore necessarily needs to be a nested array and cannot be flattened.
But we can definitely reduce the nested array complexity because after we reimplement Range functions, we can deprecate most of the RangeSet functions again.

@timreichen timreichen mentioned this issue Jan 1, 2024
13 tasks
@kt3k
Copy link
Member

kt3k commented Jan 2, 2024

The semver mod was taken over from node-semver. range actually represents a range-set.

That BNF file only looks like a non official reference for implementers of node-semver. That doesn't give reason for introducing RangeSet concept in std/semver. Having both SemVerRange and RangeSet sounds just confusing and too strict to me.

@timreichen
Copy link
Contributor Author

Closing this since Comparator is deprecated now.

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

No branches or pull requests

3 participants