[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Command line parsing of ls with genparse
From: |
Jim Meyering |
Subject: |
Re: [PATCH] Command line parsing of ls with genparse |
Date: |
Tue, 28 Aug 2007 23:03:15 +0200 |
Eric Blake-1 <address@hidden> wrote:
>> - case 'w':
>> - {
>> - unsigned long int tmp_ulong;
>> - if (xstrtoul (optarg, NULL, 0, &tmp_ulong, NULL) != LONGINT_OK
>> - || ! (0 < tmp_ulong && tmp_ulong <= SIZE_MAX))
>> - error (LS_FAILURE, 0, _("invalid line width: %s"),
>> - quotearg (optarg));
>
> [Side issue, not a problem with your patch, but your
> patch highlights the problem - why are we using plain
> error instead of xstrtol_error? It means this particular
> error message loses some quality.]
You're right that technically xstrtol_error is better -- more
specific about why a string is invalid -- and I'll probably make
the change, but I can't help lamenting the small degradation in
quality of the English diagnostic:
Current:
$ ls -w999999999999999999999999999999999999999
ls: invalid line width: 999999999999999999999999999999999999999
[Exit 2]
Probable new diagnostic:
$ ./ls -w999999999999999999999999999999999999999
./ls: -w argument `999999999999999999999999999999999999999' too large
[Exit 2]
As you probably noticed, it's needed at least in many places.
A quick search shows many other cases like that.
$ grep -A5 -E 'xstrto(u?l|i?max) \(' *.c|grep -w error|wc -l
32
Do you feel like working on that?