GNU bug report logs - #6176
[PATCH 1/2] sort: add a --debug option to highlight key extents

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 6176 in the body.
You can then email your comments to 6176 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Report bugs to <bug-coreutils <at> gnu.org>
Subject: [PATCH 1/2] sort: add a --debug option to highlight key extents
Date: Tue, 11 May 2010 23:56:49 +0100

[Message part 1 (text/plain, inline)]

The attached patch allows one to:

$ printf "one 2 3e3e" | .sort --debug -k3,3g -k2,2
one 2 3e3e
      ___
   __
__________


cheers,
Pádraig

[0001-sort-add-a-debug-option-to-highlight-key-extents.patch (text/x-patch, attachment)]

Message #8 received at 6176 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: 6176 <at> debbugs.gnu.org
Subject: [PATCH 2/2] sort: --debug: output data independent key warnings
Date: Wed, 12 May 2010 00:39:36 +0100

[Message part 1 (text/plain, inline)]

The attached patch gives warnings about questionable
option combinations. For example:

$ sort --debug -rb -k1,1n /dev/null
! options `-b' are ignored
! option `-r' only applies to last-resort comparison

cheers,
Pádraig.

[0002-sort-debug-output-data-independent-key-warnings.patch (text/x-patch, attachment)]

Message #11 received at 6176 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: 6176 <at> debbugs.gnu.org
Subject: Re: bug#6176: [PATCH 2/2] sort: --debug: output data independent
	key	warnings
Date: Wed, 12 May 2010 10:21:48 +0100

On 12/05/10 00:39, Pádraig Brady wrote:
> The attached patch gives warnings about questionable
> option combinations. For example:
> 
> $ sort --debug -rb -k1,1n /dev/null
> ! options `-b' are ignored
> ! option `-r' only applies to last-resort comparison

Oops, The previous patch warned about -r even when no keys specified,
and didn't so the sort -ru special case right either.
This should fix it up.

diff --git a/src/sort.c b/src/sort.c
index 66a00ef..211415d 100644
--- a/src/sort.c
+++ b/src/sort.c
@@ -2245,18 +2245,20 @@ key_warnings (struct keyfield const *gkey)
       ugkey.reverse &= !key->reverse;
     }

-  /* Warn about ignored global options flagged above.  */
-  if (!default_key_compare (&ugkey) || (stable && ugkey.reverse))
+  /* Warn about ignored global options flagged above.
+     Note if gkey is the only one in the list, all flags are cleared.  */
+  if (!default_key_compare (&ugkey)
+       || (ugkey.reverse && (stable || unique) && keylist))
     {
       bool ugkey_reverse = ugkey.reverse;
-      if (!stable)
+      if (!(stable || unique))
         ugkey.reverse = false;
       char *opts = key_to_opts (&ugkey);
       fprintf (stderr, _("! options `-%s' are ignored\n"), opts);
       free (opts);
       ugkey.reverse = ugkey_reverse;
     }
-  if (!stable && ugkey.reverse)
+  if (ugkey.reverse && !(stable || unique) && keylist)
     fprintf (stderr,
              _("! option `-r' only applies to last-resort comparison\n"));
 }
diff --git a/tests/misc/sort-debug-warn b/tests/misc/sort-debug-warn
index 5295b4b..c2ff01d 100755
--- a/tests/misc/sort-debug-warn
+++ b/tests/misc/sort-debug-warn
@@ -47,6 +47,9 @@ sort -r -k1,1r --debug /dev/null 2>>out # no warning for redundant options
 sort -i -k1,1i --debug /dev/null 2>>out # no warning
 sort -d -k1,1b --debug /dev/null 2>>out
 sort -i -k1,1d --debug /dev/null 2>>out
+sort -r --debug /dev/null 2>>out #no warning
+sort -rM --debug /dev/null 2>>out #no warning
+sort -rM -k1,1 --debug /dev/null 2>>out #no warning

 compare out exp || fail=1

Message #16 received at 6176-done <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Pádraig Brady <P <at> draigBrady.com>
Cc: 6176-done <at> debbugs.gnu.org
Subject: Re: bug#6176: [PATCH 1/2] sort: add a --debug option to highlight key
	extents
Date: Wed, 12 May 2010 13:08:35 +0200

Pádraig Brady wrote:
> The attached patch allows one to:
>
> $ printf "one 2 3e3e" | .sort --debug -k3,3g -k2,2
> one 2 3e3e
>       ___
>    __
> __________

Nice.  Thanks for writing all of that.
These changes are sure to be useful.

I have merely glanced through them (look fine so far),
but would not mind at all if you were to push the series as-is.

Jim

PS: I've gone ahead and marked this as "done".

Message #17 received at 6176-done <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Jim Meyering <jim <at> meyering.net>
Cc: 6176-done <at> debbugs.gnu.org
Subject: Re: bug#6176: [PATCH 1/2] sort: add a --debug option to highlight
	key extents
Date: Wed, 12 May 2010 12:39:01 +0100

On 12/05/10 12:08, Jim Meyering wrote:
> Pádraig Brady wrote:
>> The attached patch allows one to:
>>
>> $ printf "one 2 3e3e" | .sort --debug -k3,3g -k2,2
>> one 2 3e3e
>>       ___
>>    __
>> __________
> 
> Nice.  Thanks for writing all of that.
> These changes are sure to be useful.
> 
> I have merely glanced through them (look fine so far),
> but would not mind at all if you were to push the series as-is.
> 
> Jim
> 
> PS: I've gone ahead and marked this as "done".

Thanks.

The changes shouldn't be risky as everything
is protected with if (debug).

I was also wondering about informational class
messages that could be output by --debug.
Perhaps stats on the number of comparisons done
could be useful? Also mentioning the oft confusing
collating order with something like:

if (debug)
  {
    if (hard_LC_COLLATE)
      fprintf (stderr, _("# Using %s locale rules\n"),
               quote(setlocale (LC_COLLATE, NULL));
    else
      fprintf (stderr, _("# Using simple byte comparison\n");
  }

Anyway, something for another day.

cheers,
Pádraig.

Message #20 received at 6176 <at> debbugs.gnu.org (full text, mbox):

From: Eric Blake <eblake <at> redhat.com>
To: Pádraig Brady <P <at> draigBrady.com>
Cc: 6176 <at> debbugs.gnu.org
Subject: Re: bug#6176: [PATCH 2/2] sort: --debug: output data independent
	key	warnings
Date: Wed, 12 May 2010 07:53:23 -0600

[Message part 1 (text/plain, inline)]

On 05/11/2010 05:39 PM, Pádraig Brady wrote:
> The attached patch gives warnings about questionable
> option combinations. For example:
> 
> $ sort --debug -rb -k1,1n /dev/null
> ! options `-b' are ignored
> ! option `-r' only applies to last-resort comparison

That looks awkward, both when compared to the GCS convention of listing
the program name rather than !, and in respect to plurality:

sort: option `-b' is ignored
sort: option `-r' only applies to last-resort comparison

-- 
Eric Blake   eblake <at> redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

[signature.asc (application/pgp-signature, attachment)]

Message #23 received at 6176 <at> debbugs.gnu.org (full text, mbox):

From: Eric Blake <eblake <at> redhat.com>
To: Eric Blake <eblake <at> redhat.com>
Cc: 6176 <at> debbugs.gnu.org, Pádraig Brady <P <at> draigBrady.com>
Subject: Re: bug#6176: [PATCH 2/2] sort: --debug: output data independent
	key	warnings
Date: Wed, 12 May 2010 07:55:47 -0600

[Message part 1 (text/plain, inline)]

On 05/12/2010 07:53 AM, Eric Blake wrote:
> On 05/11/2010 05:39 PM, Pádraig Brady wrote:
>> The attached patch gives warnings about questionable
>> option combinations. For example:
>>
>> $ sort --debug -rb -k1,1n /dev/null
>> ! options `-b' are ignored
>> ! option `-r' only applies to last-resort comparison
> 
> That looks awkward, both when compared to the GCS convention of listing
> the program name rather than !, and in respect to plurality:
> 
> sort: option `-b' is ignored
> sort: option `-r' only applies to last-resort comparison

Or, to put it more concretely,

> +      fprintf (stderr, _("! options `-%s' are ignored\n"), opts);
> +      free (opts);
> +      ugkey.reverse = ugkey_reverse;
> +    }
> +  if (!stable && ugkey.reverse)
> +    fprintf (stderr,
> +             _("! option `-r' only applies to last-resort comparison\n"));

Why are we using fprintf(stderr) instead of error()?

-- 
Eric Blake   eblake <at> redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

[signature.asc (application/pgp-signature, attachment)]

Message #26 received at 6176 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Eric Blake <eblake <at> redhat.com>
Cc: 6176 <at> debbugs.gnu.org
Subject: Re: bug#6176: [PATCH 2/2] sort: --debug: output data independent
	key	warnings
Date: Wed, 12 May 2010 15:07:05 +0100

On 12/05/10 14:55, Eric Blake wrote:
> On 05/12/2010 07:53 AM, Eric Blake wrote:
>> On 05/11/2010 05:39 PM, Pádraig Brady wrote:
>>> The attached patch gives warnings about questionable
>>> option combinations. For example:
>>>
>>> $ sort --debug -rb -k1,1n /dev/null
>>> ! options `-b' are ignored
>>> ! option `-r' only applies to last-resort comparison
>>
>> That looks awkward, both when compared to the GCS convention of listing
>> the program name rather than !, and in respect to plurality:
>>
>> sort: option `-b' is ignored
>> sort: option `-r' only applies to last-resort comparison
> 
> Or, to put it more concretely,
> 
>> +      fprintf (stderr, _("! options `-%s' are ignored\n"), opts);
>> +      free (opts);
>> +      ugkey.reverse = ugkey_reverse;
>> +    }
>> +  if (!stable && ugkey.reverse)
>> +    fprintf (stderr,
>> +             _("! option `-r' only applies to last-resort comparison\n"));
> 
> Why are we using fprintf(stderr) instead of error()?
> 

I was thinking it was redundant to print the command
name when explicitly asking for warnings,but yes I think
you're right, I'll just error(). I'll fix up the plurality also.

BTW I'm not intending to push this second patch for at least a day.

cheers,
Pádraig.

Message #29 received at 6176 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: 6176 <at> debbugs.gnu.org
Subject: Re: bug#6176: [PATCH 1/2] sort: add a --debug option to highlight
	key extents
Date: Wed, 12 May 2010 15:13:21 +0100

On 12/05/10 12:08, Jim Meyering wrote:
> Pádraig Brady wrote:
>> The attached patch allows one to:
>>
>> $ printf "one 2 3e3e" | .sort --debug -k3,3g -k2,2
>> one 2 3e3e
>>       ___
>>    __
>> __________
> 
> Nice.  Thanks for writing all of that.
> These changes are sure to be useful.
> 
> I have merely glanced through them (look fine so far),
> but would not mind at all if you were to push the series as-is.

I pushed the first, and I then pushed the following
to fix a test failure pointed out by our continuous integration box
http://hydra.nixos.org/jobset/gnu/coreutils-master

I'll probably push patch 2 tomorrow.

cheers,
Pádraig.

diff --git a/tests/misc/sort-debug-keys b/tests/misc/sort-debug-keys
index 0437678..0f05025 100755
--- a/tests/misc/sort-debug-keys
+++ b/tests/misc/sort-debug-keys
@@ -300,8 +300,8 @@ _____
 ___________________
 EOF

-(
 if test "$LOCALE_FR_UTF8"; then
+  (
   echo "   1²---++3   1,234  Mi" |
     LC_ALL=C sort --debug -k2g -k1b,1
   echo "   1²---++3   1,234  Mi" |
@@ -309,9 +309,8 @@ if test "$LOCALE_FR_UTF8"; then
   echo "+1234 1234Gi 1,234M" |
     LC_ALL=$LOCALE_FR_UTF8 sort --debug -k1,1n -k1,1g \
     -k1,1h -k2,2n -k2,2g -k2,2h -k3,3n -k3,3g -k3,3h
+  ) > out
+  compare out exp || fail=1
 fi
-) > out
-
-compare out exp || fail=1

 Exit $fail

Message #32 received at 6176 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: 6176 <at> debbugs.gnu.org
Subject: Re: bug#6176: [PATCH 2/2] sort: --debug: output data independent
	key	warnings
Date: Fri, 14 May 2010 14:10:19 +0100

[Message part 1 (text/plain, inline)]

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

$ LANG=en_USA sort --debug -srb -k1,1n /dev/null
sort: using simple byte comparison
sort: options `-br' are ignored

I'll commit tonight sometime I think.

cheers,
Pádraig.

[0001-sort-debug-output-data-independent-key-warnings.patch (text/x-patch, attachment)]

Message #35 received at 6176 <at> debbugs.gnu.org (full text, mbox):

From: Eric Blake <eblake <at> redhat.com>
To: Pádraig Brady <P <at> draigBrady.com>
Cc: 6176 <at> debbugs.gnu.org
Subject: Re: bug#6176: [PATCH 2/2] sort: --debug: output data independent
	key	warnings
Date: Fri, 14 May 2010 09:09:46 -0600

[Message part 1 (text/plain, inline)]

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   eblake <at> redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

[signature.asc (application/pgp-signature, attachment)]

Message #38 received at 6176 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: 6176 <at> debbugs.gnu.org
Subject: Re: bug#6176: [PATCH 2/2] sort: --debug: output data independent
	key	warnings
Date: Fri, 14 May 2010 16:52:10 +0100

On 14/05/10 16:09, Eric Blake wrote:
> On 05/14/2010 07:10 AM, Pádraig Brady wrote:
>>  
>>  /* 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?

OK will do.

>> +  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.

Oops, right. I've been doing too much C99 lately.
I quickly grepped but didn't notice the existing %zu
was inside #if DEBUG. I'll use something more standard.

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

That was from the previous commit, and the log message there said:

  (main): Process the --debug option and make it mutually exlusive
  with the -o option as I don't see it useful there, even potentially
  harmful if someone left a --debug in by mistake when updating a file.
  Also restricting debug output to stdout, simplifies the logic
  for dealing with temporary files.

I'll add a comment to the code also.

thanks for the review.
Pádraig.

Message #41 received at 6176 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> CS.UCLA.EDU>
To: Pádraig Brady <P <at> draigBrady.com>
Cc: 6176 <at> debbugs.gnu.org
Subject: Re: bug#6176: [PATCH 2/2] sort: --debug: output data independent
	key	warnings
Date: Fri, 14 May 2010 14:23:12 -0700

On 05/14/10 06:10, Pádraig Brady wrote:

> -    if ((1 < (key->random + key->numeric + key->general_numeric + key->month
> -              + key->version + !!key->ignore + key->human_numeric))
> +    if ((1 < (key->random + key_numeric (key) + key->month + key->version
> +              + !!key->ignore))

This change doesn't look right, since it won't catch the error of
specifying both numeric and general_numeric options.  Am I missing
something?

> sort: obsolescent key formats used.  Consider using `-k'

Something like the following diagnostic would be far more helpful for
users who are not 'sort' experts:

  sort: obsolescent key `+2 -4' used; consider `-k 3,4' instead

Can you please arrange for that?

> +static char*

Missing space before "*".

> +  /* The following is too big, but guaranteed to be "big enough". */
> +  char *opts = xstrdup (short_options);

This unnecessarily copies short_options.  Better would be:

  char *opts = xmalloc (sizeof short_options);

But, come to think of it, the interface for key_to_opts is awkward.
Callers must currently do this:

   char *opts = key_to_opts (key);
   F (opts);
   free (opts);

where F is some function.  It'd be nicer for callers to do something
like this instead:

  char opts[sizeof short_options];
  key_to_opts (key, opts);
  F (opts);

This is a bit faster and is easier to understand (at least, for me).

Message #44 received at 6176 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Paul Eggert <eggert <at> CS.UCLA.EDU>
Cc: 6176 <at> debbugs.gnu.org
Subject: Re: bug#6176: [PATCH 2/2] sort: --debug: output data independent
	key	warnings
Date: Fri, 14 May 2010 22:47:41 +0100

On 14/05/10 22:23, Paul Eggert wrote:
> On 05/14/10 06:10, Pádraig Brady wrote:
> 
>> -    if ((1 < (key->random + key->numeric + key->general_numeric +
>> key->month
>> -              + key->version + !!key->ignore + key->human_numeric))
>> +    if ((1 < (key->random + key_numeric (key) + key->month +
>> key->version
>> +              + !!key->ignore))
> 
> This change doesn't look right, since it won't catch the error of
> specifying both numeric and general_numeric options.  Am I missing
> something?

Well spotted
test/misc/sort::h7 had caught my silliness also

> 
>> sort: obsolescent key formats used.  Consider using `-k'
> 
> Something like the following diagnostic would be far more helpful for
> users who are not 'sort' experts:
> 
>   sort: obsolescent key `+2 -4' used; consider `-k 3,4' instead
> 
> Can you please arrange for that?

Yes that would be more useful. I'll have a look.

>> +  /* The following is too big, but guaranteed to be "big enough". */
>> +  char *opts = xstrdup (short_options);
> 
> This unnecessarily copies short_options.  Better would be:

>   char opts[sizeof short_options];
>   key_to_opts (key, opts);
>   F (opts);
> 
> This is a bit faster and is easier to understand (at least, for me).

Yes that's better.

thanks a lot!
Pádraig.

Message #47 received at 6176 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: 6176 <at> debbugs.gnu.org
Subject: Re: bug#6176: [PATCH 2/2] sort: --debug: output data independent
	key	warnings
Date: Sat, 15 May 2010 01:26:21 +0100

On 14/05/10 22:47, Pádraig Brady wrote:
> On 14/05/10 22:23, Paul Eggert wrote:

>> Something like the following diagnostic would be far more helpful for
>> users who are not 'sort' experts:
>>
>>   sort: obsolescent key `+2 -4' used; consider `-k 3,4' instead
>>
>> Can you please arrange for that?

I did that using this chunk.
Note that it doesn't reproduce char offsets or flags
on the old or new formats, but they're the same on
both formats, so it's not worth the complexity I think.

if (key->obsolete_used)
  {
    /* obsolescent syntax +A.x -B.y is equivalent to:
         -k A+1.x+1,B.y   (when y = 0)
         -k A+1.x+1,B+1.y (when y > 0)  */
    char obuf[INT_BUFSIZE_BOUND (size_t) * 2 + 4]; /* +# -#  */
    char nbuf[INT_BUFSIZE_BOUND (size_t) * 2 + 5]; /* -k #,#  */

    char *po = obuf;
    char *pn = nbuf;

    size_t sword = key->sword;
    size_t eword = key->eword;
    if (sword == SIZE_MAX)
      sword++;

    po += sprintf (po, "+%" PRIuMAX, (uintmax_t) sword);
    pn += sprintf (pn, "-k %" PRIuMAX, (uintmax_t) sword + 1);
    if (key->eword != SIZE_MAX)
      {
        po += sprintf (po, " -%" PRIuMAX, (uintmax_t) eword + 1);
        pn += sprintf (pn, ",%" PRIuMAX,
                       (uintmax_t) eword + 1 + (key->echar == SIZE_MAX));
      }
    error (0, 0, _("obsolescent key `%s' used; consider `%s' instead"),
           obuf, nbuf);
  }

That latest patch is at http://url.ie/660o

cheers,
Pádraig.

Message #50 received at 6176 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: 6176 <at> debbugs.gnu.org
Subject: [PATCH] sort: adjust the leading blanks --debug warning
Date: Sat, 22 May 2010 14:21:20 +0100

I was reviewing some tricky sorting I've done previously to see did
the new --debug option warn me about the issues I encountered.
One thing I missed was that if one specifies character offsets,
then we should warn, even if the type implicitly skips spaces when sorting.
For example, one has to specify 'b' to both start and end positions
in the second key in the example below (which sorts apache access.log by date).

printf "127.0.0.1 - - [01/Jan/2008:02:08:26 +0000] ...\n" |
sort --debug -b -k4.9,4.12 -k4.5b,4.7Mb -k4.2,4.3 -k4.14,4

One caveat with always warning about a missing 'b' when character
offsets are specified, is that this form is now warned about: -k1.x,1.y
That is commonly used to specify offsets into a fixed spaced file.
I.E. it can be seen as a line offset, rather than a field offset.
So I've excluded that form from any warnings.

Also previously we would warn about a missing 'b' when
it could have been legitimately unused to support sorting
right aligned indexes, aligned with spaces.

I'm wary about being "too clever" with these warnings,
but I think it worth suppressing this warning in the
above 2 fairly common cases, as the user can still see the
blanks being included when the key annotations are displayed.

cheers,
Pádraig.

diff --git a/src/sort.c b/src/sort.c
index 8a9309a..bebbd11 100644
--- a/src/sort.c
+++ b/src/sort.c
@@ -2261,8 +2261,13 @@ key_warnings (struct keyfield const *gkey, bool gkey_only)
         error (0, 0, _("key %lu has zero width and will be ignored"), keynum);

       /* Warn about significant leading blanks.  */
-      if (!gkey_only && tab == TAB_DEFAULT && !key->skipsblanks
-          && !key_numeric (key) && !key->month)
+      bool implicit_skip = key_numeric (key) || key->month;
+      bool support_space_aligned = !hard_LC_COLLATE && default_key_compare (key) && !key->schar;
+      bool line_offset = key->eword == 0 && key->echar != 0; /* -k1.x,1.y  */
+      if (!gkey_only && tab == TAB_DEFAULT && !line_offset
+          && ((!key->skipsblanks && !(implicit_skip || support_space_aligned))
+              || (!key->skipsblanks && key->schar)
+              || (!key->skipeblanks && key->echar)))
         error (0, 0, _("leading blanks are significant in key %lu; "
                        "consider also specifying `b'"), keynum);

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.