Eric Blake wrote:
> On 12/09/2011 12:22 PM, Jim Meyering wrote:
>> jida...@jidanni.org wrote:
>>> $ ls -t1 --time-style=%c -og
>>> ls: invalid argument `%c' for `time style'
>>> Valid arguments are:
>>> - `full-iso'
>>> - `long-iso'
>>> - `iso'
>>> - `locale' <-------------you forgot to also mention "+FORMAT"
>>> Try `ls --help' for more information.
>>> $ ls -t1 --time-style=+%c -og
>>
>> Thanks for the report.
>> However, that doesn't lend itself well to a clean fix, since +FORMAT
>> is not a literal string option argument like the others, and currently
>> that diagnostic is generated automatically based on the list of literal
>> strings.
>
> I agree that listing `+FORMAT' doesn't fit the pattern. But maybe we
> can make it obvious that there is a non-literal possibility, as well as
> also mentioning the posix- prefix already covered by 'ls --help':
>
> Valid arguments are:
> - `[posix-]full-iso'
> - `[posix-]long-iso'
> - `[posix-]iso'
> - `[posix-]locale'
> - `+' followed by formatting directives
> Try `ls --help' for more information.
Sure, that is possible, and adding the [posix-] prefixes would be a nice bonus.
The trouble is that the current code is not only nice and compact, but will
safely and automatically reflect any addition to the list of time styles:
switch (XARGMATCH ("time style", style,
time_style_args,
time_style_types))
That handles everything, including printing the offending diagnostic
and exiting. If you pick it apart in preparation for open-coding, you
then have to find a way to ensure that the new diagnostic stays in sync
with list of possible option arguments.
All feasible, but is it worth it, just for an improved diagnostic that
won't be seen unless you use ls's --time-style=V option with an invalid V ?
I'm leaning away, but if someone comes up with a clean and
maintainable way to do it, no problem.
> Also, I noticed that 'ls --help' uses FORMAT in the text for
> --time-style, but doesn't define FORMAT anywhere else; we probably ought
> to have a one-line sentence at the bottom, after the paragraph on SIZE,
> stating:
>
> See `date --help' for valid directives that may appear in FORMAT.
Good idea. Patch most welcome.
