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
We need a "compare" command independent of "order" #1347
Comments
In case it's useful, note that I wrote also a module for semantic version
comparisons:
https://github.com/zzamboni/elvish-modules/blob/master/semver.org
…On Mon, 28 Jun 2021 at 06:20, Kurtis Rader ***@***.***> wrote:
I have been working on an Elvish way to compare values in order to
implement semantic version <https://semver.org> comparisons. The default
comparison function
<https://github.com/elves/elvish/blob/9981b2a777961f3669200e9b890c16cbb6935d3d/pkg/eval/builtin_fn_container.go#L899>
used by the order command appears to do what I want. It is a shame that I
have to reinvent that wheel using Elvish commands.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#1347>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAIA3CZRRLK5QKDQEA7HMDTU72CRANCNFSM47NEXALA>
.
|
@zzamboni, Also, your version doesn't correctly handle invalid semver strings. In particular, it is overly permissive by treating strings like "1.0.0" and "1.0" as equal. While at the same time not handling a very common, but incorrect, inclusion of "v" or "V" as a semver prefix. A mistake made by the current elvish implementation: elvish/pkg/buildinfo/buildinfo.go Line 18 in fc887b2
I still think your module is awesome but it points out why a solution for the semver issue I opened is so important. Specifically, the semver project should include a file of test cases so that every implementation can be validated against that implementation agnostic set of test cases. |
@krader1961 I think my implementation correctly handles prerelease strings, e.g. using the example from the SemVer spec:
See also a whole bunch of other tests at https://github.com/zzamboni/elvish-modules/blob/master/semver.org. You are correct that a missing trailing numbers (MINOR or PATCH) are considered as zeros. In my defense, this is even documented 😉: https://github.com/zzamboni/elvish-modules/blob/master/semver.org#main-comparison-function. The spec says that all three components MUST be there, so I guess this is a bug. I've opened zzamboni/elvish-modules#17 Build metadata: this is also a bug, it should be ignored. I've created zzamboni/elvish-modules#16 Invalid things like the Thanks for the feedback. PR's welcome! 😉 |
@zzamboni, A "v" (or "V") prefix is technically invalid in a semver string. However, I would argue it should be silently ignored since it is all too common for an otherwise valid semver string to include that prefix. I didn't actually test that your implementation does so. I simply inferred it didn't from the documentation. If (or when) your implementation silently ignores a "v" prefix that is notable enough to warrant a mention in the documentation and why it is different from how every other invalid semver string is treated. |
@krader1961 regarding your original issue, please note that as part of my The
About SemVer comparison, please check out zzamboni/elvish-modules#18, in which I address all the bugs mentioned above. |
Expose the default comparison function used by the `builtin:order` command as a command in its own right. This command is useful when writing, in Elvish, something like `builtin:order`. Such as a semantic version comparison command. Resolves elves#1347
I have been working on an Elvish way to compare values in order to implement semantic version comparisons. The default comparison function used by the
order
command appears to do what I want. It is a shame that I have to reinvent that wheel using Elvish commands.The text was updated successfully, but these errors were encountered: