[Toybox] sort -n -M patch

[isabella parakiss]

On 9/28/16, Rob Landley <rob at landley.net> wrote:
> I'm looking at
> https://github.com/landley/toybox/pull/19/commits/6d6252a8f39e3f813cd79dc96ebae61c6507717c
> which does 2 things:
>
> 1) Hardwire in english month names. Not sure why, since strptime is posix:
>
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/strptime.html
>
> And having libc do it gives us locale-specific translation for free? (I
> don't go out of my way for it, but I don't see why I should turn it down
> if libc already did the work?)
>

https://gist.github.com/izabera/d069eb12fa5d640564378a2f6163dbe8

calling strptime in a tight loop is absolutely great code and you should
feel proud of yourself as an engineer for writing it.

if you want sort -M to have locale-specific support (which... basically
nothing else in toybox has? zero gettext support and all error messages
are "bad xyz"), the obvious solution is to precompute the names of the
months according to the current locale and then use a very similar loop
to what that patch does.

> 2) replace 2 lines of floating point comparison code with 18 lines of
> floating point comparison code.
>
> I'm aware that this offers unlimited precision (in a way I'd have to
> write a lot of corner case tests for; this patch comes with zero new
> testcases), I'm just not sure why we need that? Posix sort -n says to
> support locale-specific thousands separators, so this can't be for
> standards compliance.
>

no, but it's what gnu/freebsd/openbsd/etc sort do and i was assuming that
compatibility with the rest of the world was somehow high priority, and
also the only reason for having -g, which isn't standard and basically a
duplicate of -n as of now.

> I just checked busybox and they have _not_ changed the -n logic I wrote
> for them in 2004, which is doing atof() and comparing the results. So
> nobody's cared enough over there to fiddle with it in the past dozen
> years...
>
> I am leaning towards not accepting either part of this patch.
>
> Rob
> _______________________________________________
> Toybox mailing list
> Toybox at lists.landley.net
> http://lists.landley.net/listinfo.cgi/toybox-landley.net
>