Subject: Re: bug#6176: [PATCH 2/2] sort: --debug: output data independent key warnings

From: Eric Blake <[email-masked]>
Date: Fri, 14 May 2010 09:09:46 -0600

On 05/14/2010 07:10 AM, Pádraig Brady wrote:
> Latest version of the patch attached with new warnings and info.
> Example output...
> 
> $ sort --debug -rb -k2n +2 -1b /dev/null
> sort: using `en_US.utf8' sorting rules
> sort: obsolescent key formats used.  Consider using `-k'
> sort: key 1 is numeric and spans multiple fields
> sort: key 2 has zero width and will be ignored
> sort: leading blanks are significant in key 2.  Consider also specifying `b'
> sort: option `-b' is ignored
> sort: option `-r' only applies to last-resort comparison

*Nice!*

> +/* Nonzero if the obsolescent key option format is used.  */
> +static bool obsolete_used;

s/Nonzero/True/

> +
>  #define NONZERO(x) ((x) != 0)
>  
>  /* The kind of blanks for '-b' to skip in various options. */
> @@ -375,7 +378,8 @@ Other options:\n\
>    -C, --check=quiet, --check=silent  like -c, but do not report first bad line\n\
>        --compress-program=PROG  compress temporaries with PROG;\n\
>                                decompress them with PROG -d\n\
> -      --debug               annotate the part of the line used to sort\n\
> +      --debug               annotate the part of the line used to sort,\n\
> +                              and warn about questionable usage to stderr\n\
>        --files0-from=F       read input from the files specified by\n\
>                              NUL-terminated names in file F;\n\
>                              If F is - then read names from standard input\n\

This makes for a pretty long translation string; time to break it in two?

> +static inline bool
> +key_numeric (struct keyfield const *key)
> +{
> +  return (key->numeric || key->general_numeric || key->human_numeric);

Redundant ().

> +  for (key = keylist; key; key = key->next, keynum++)
> +    {
> +      /* Warn about field specs that will never match.  */
> +      if (key->sword != SIZE_MAX && key->eword < key->sword)
> +        error (0, 0, _("key %zu has zero width and will be ignored"), keynum);

This requires vfprintf-posix to guarantee that %zu will work; I'm not
sure we have that guarantee, and Jim has been reluctant to globally turn
on gnulib printf replacements.

> +
> +      /* Warn about significant leading blanks.  */
> +      if (!gkey_only && tab == TAB_DEFAULT && !key->skipsblanks
> +          && !key_numeric (key) && !key->month)
> +        error (0, 0, _("leading blanks are significant in key %zu.  Consider "
> +                       "also specifying `b'"), keynum);
> +
> +
> +      /* Warn about numeric comparisons spanning fields,

Why two blank lines?

> @@ -3884,6 +3985,18 @@ main (int argc, char **argv)
>    if (debug && outfile)
>      error (SORT_FAILURE, 0, _("options -o and --debug are incompatible"));

Why?

-- 
Eric Blake   [email-masked]    +1-801-349-2682
Libvirt virtualization library http://libvirt.org