I looked at the patch <http://launchpadlibrarian.net/20858726/sort_human_v3.diff> and have a few comments:
* There's no documentation, either in the .texi file or in the --help string. That's often the hardest part to get right. * The patch slows down "sort -n" a bit, and (worse) complicates the code for "sort -n". There are a lot of gotos. Better, I think, would be to (1) search for a trailing suffix character, and compare based on that, and then (2) fall back on numeric comparison only if the trailing suffix characters match. This will certainly make plain "sort -n" faster and will result in a less-intrusive change to the existing code; arguably it will make the new code faster in many cases, too. * Please don't take the address of an often-used local variable; this hurts optimization, since it often inhibits optimizing that variable into a register. * I suggest ignoring any suffix after the 'k' or 'M' or 'G' or whatever; it's simpler. * I also suggest allowing only the upper-case version (e.g., 'P', not 'p'), as this will allow later support for ISO prefixes like 'p' (for "pico") better. (It's OK to allow both 'k' and 'K' to mean kilo, though.) * exponent_order should be a simple indexed lookup like "table[a]"; there shouldn't be a need for a loop checking for "inverse_table[i] == a". Hope this helps. _______________________________________________ Bug-coreutils mailing list Bug-coreutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-coreutils
