chore: Move natural-sort functionality into packages/internal #10320
Conversation
| /** | ||
| * @param {string} a | ||
| * @param {string} b | ||
| * @returns {-1 | 0 | 1} | ||
| */ | ||
| const compareNats = (a, b) => { |
There was a problem hiding this comment.
My understanding is that this logic works even is the params are number, right?
There was a problem hiding this comment.
Mostly, but not when either is NaN or both are infinite. And this function is not exported, so we can count on the input arguments always being decimal digit strings.
Deploying agoric-sdk with
|
| Latest commit: |
8c475e8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8e6ccafb.agoric-sdk.pages.dev |
| Branch Preview URL: | https://gibson-2024-10-natural-sort.agoric-sdk.pages.dev |
Removes unnecessary use of localeCompare Ref endojs/endo#2113
gibson042
force-pushed
the
gibson-2024-10-natural-sort
branch
from
3240449 to
9e6b7ad
Compare
| return 1; | ||
| // Perform an element-by-element natural sort. | ||
| kernelTable.sort((a, b) => { | ||
| const len = Math.min(a.length, b.length); |
There was a problem hiding this comment.
This feels wrong.. actually the original feels wrong too. The kernelTable data is mostly created by calling vatKeeper.dumpState(), and that returns a 3-tuple of [kernelSlot, vatID, vatSlot]. It would make the most sense to use naturalCompare separately on each of the three components (krefs, vatIDs, and vrefs all have the same letters-then-digits shape).
I don't know why the original was treating the rows as six-tuples. My best guess is it thought a[0], a[1], a[2] was individual characters of a kref, so the ko prefix would be compared by strings, and the digit suffix would be compared by number. But that doesn't make a lot of sense either.
In any case, I think we want something more like:
kernelTable.sort((a,b) =>
naturalCompare(a[0], b[0]) ||
naturalCompare(a[1], b[1]) ||
naturalCompare(a[2], b[2]) || 0);unless you can make more sense of the old code than me..
There was a problem hiding this comment.
Yeah, that makes sense. I just realized I typo'd the general form anyway (a[0] and b[0] rather than a[i] and b[i]).
Ref endojs/endo#2113
Description
Removes unnecessary use of
localeCompareand consolidates natural-order sorting.Security Considerations
n/a
Scaling Considerations
n/a
Documentation Considerations
n/a
Testing Considerations
None in particular, although I probably should add something to packages/internal/test.
Upgrade Considerations
This code runs outside of all vats (and AFAIK is not exercised on-chain at all) and is safe to include in any release.