search

elves / elvish

Powerful scripting language & versatile interactive shell
https://elv.sh/
BSD 2-Clause "Simplified" License
5.53k stars 297 forks source link

Move the `cmp` function into the `vals` package #1701

Closed krader1961 closed 1 year ago

krader1961 commented 1 year ago

Move the value comparison logic into pkg/eval/vals so it can be used by packges other than eval. For example, the vals package repr implementation as well as other Elvish modules which may want to compare Elvish values.

This also augments the comparison logic to handle types that do not have an obvious, explicitly handled, ordering. For example, Elvish exceptions, functions, and styled text. If the types are the same then the values are considered equal. This means you can now do things like compare $nil $nil or put ?(fail y) ?(fail x) | order without raising an exception.

Related #1495

krader1961 commented 1 year ago

I force pushed an update because I realized there were some minor tweaks to my this change that I made in a subsequent change that really belongs in this change. These tweaks to the code are all cosmetic but avoid introducing the same tweaks in subsequent changes I have in my pipeline.

xiaq commented 1 year ago

I did the move in https://github.com/elves/elvish/commit/1e249ed7fbcab6c5c0e3d18e523af0eac620c69e so I'll close this now.

As a general point of advice, it's easier for me to merge PRs when the PR doesn't mix simple changes (like moving a function to a different package, which is what I requested) and bigger changes that impact the existing semantics (like changing how compare behaves).