GNU bug report logs - #10253
mention +FORMAT in ls time style reminder help blurb

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 10253 in the body.
You can then email your comments to 10253 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: jidanni <at> jidanni.org
To: bug-coreutils <at> gnu.org
Subject: mention +FORMAT in ls time style reminder help blurb
Date: Fri, 09 Dec 2011 09:51:37 +0800

$ 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

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

From: Jim Meyering <jim <at> meyering.net>
To: jidanni <at> jidanni.org
Cc: 10253 <at> debbugs.gnu.org
Subject: Re: bug#10253: mention +FORMAT in ls time style reminder help blurb
Date: Fri, 09 Dec 2011 20:22:24 +0100

jidanni <at> 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'm inclined to leave it as-is.

Any other opinions?

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

From: Eric Blake <eblake <at> redhat.com>
To: Jim Meyering <jim <at> meyering.net>
Cc: 10253 <at> debbugs.gnu.org, jidanni <at> jidanni.org
Subject: Re: bug#10253: mention +FORMAT in ls time style reminder help blurb
Date: Fri, 09 Dec 2011 14:30:14 -0700

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

On 12/09/2011 12:22 PM, Jim Meyering wrote:
> jidanni <at> 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.

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.

-- 
Eric Blake   eblake <at> redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

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

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

From: Jim Meyering <jim <at> meyering.net>
To: Eric Blake <eblake <at> redhat.com>
Cc: 10253 <at> debbugs.gnu.org, jidanni <at> jidanni.org
Subject: Re: bug#10253: mention +FORMAT in ls time style reminder help blurb
Date: Sat, 10 Dec 2011 18:29:29 +0100

Eric Blake wrote:
> On 12/09/2011 12:22 PM, Jim Meyering wrote:
>> jidanni <at> 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.

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

From: jidanni <at> jidanni.org
To: jim <at> meyering.net
Cc: 10253 <at> debbugs.gnu.org, eblake <at> redhat.com
Subject: Re: bug#10253: mention +FORMAT in ls time style reminder help blurb
Date: Sun, 11 Dec 2011 08:02:11 +0800

Well wherever you say 4/5ths of something
that means the other 1/5th does not exist,
so it would be better to say nothing.

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

From: Jim Meyering <jim <at> meyering.net>
To: Eric Blake <eblake <at> redhat.com>
Cc: 10253 <at> debbugs.gnu.org, jidanni <at> jidanni.org
Subject: Re: bug#10253: mention +FORMAT in ls time style reminder help blurb
Date: Sun, 11 Dec 2011 12:07:08 +0100

Jim Meyering wrote:
> Eric Blake wrote:
>> On 12/09/2011 12:22 PM, Jim Meyering wrote:
>>> jidanni <at> 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.

I went ahead and started writing, to see just how bad it would be...
This looks worse than it is due to the indentation change.
Below is the much smaller white-space-ignoring diff.
It seems worthwhile after all.

With this patch, ls would print this:

    $ src/ls -l --time-style=%c
    src/ls: invalid argument `%c' for `time style'
    Valid arguments are:  - `[posix-]full-iso'
      - `[posix-]long-iso'
      - `[posix-]iso'
      - `[posix-]locale'
      - `+' followed by a date format string
    Try `src/ls --help' for more information.


From 4114b1c0e1116e05d50ea3a2377edfb8bb090e78 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering <at> redhat.com>
Date: Sun, 11 Dec 2011 11:59:31 +0100
Subject: [PATCH] ls: give a more useful diagnostic for a bogus --time-style
 arg

* src/ls.c (decode_switches): Replace our use of XARGMATCH
with open-coded version so that we can give a better diagnostic.
Reported by Dan Jacobson in http://bugs.gnu.org/10253
---
 src/ls.c |   72 +++++++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 48 insertions(+), 24 deletions(-)

diff --git a/src/ls.c b/src/ls.c
index 0d64bab..ac01f3d 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -2039,33 +2039,57 @@ decode_switches (int argc, char **argv)
           long_time_format[1] = p1;
         }
       else
-        switch (XARGMATCH ("time style", style,
-                           time_style_args,
-                           time_style_types))
-          {
-          case full_iso_time_style:
-            long_time_format[0] = long_time_format[1] =
-              "%Y-%m-%d %H:%M:%S.%N %z";
-            break;
+        {
+          ptrdiff_t res = argmatch (style, time_style_args,
+                                    (char const *) time_style_types,
+                                    sizeof (*time_style_types));
+          if (res < 0)
+            {
+              /* This whole block used to be a simple use of XARGMATCH.
+                 but that didn't print the "posix-"-prefixed variants or
+                 the "+"-prefixed format string option upon failure.  */
+              argmatch_invalid ("time style", style, res);
+
+              /* The following is a manual expansion of argmatch_valid,
+                 but with the added "+ ..." description and the [posix-]
+                 prefixes prepended.  Note that this simplification works
+                 only because all four existing time_style_types values
+                 are distinct.  */
+              fprintf (stderr, _("Valid arguments are:"));
+              char const *const *p = time_style_args;
+              while (*p)
+                fprintf (stderr, "  - `[posix-]%s'\n", *p++);
+              fprintf (stderr,
+                       _("  - `+' followed by a date format string\n"));
+              usage (LS_FAILURE);
+            }
+          switch (res)
+            {
+            case full_iso_time_style:
+              long_time_format[0] = long_time_format[1] =
+                "%Y-%m-%d %H:%M:%S.%N %z";
+              break;

-          case long_iso_time_style:
-            long_time_format[0] = long_time_format[1] = "%Y-%m-%d %H:%M";
-            break;
+            case long_iso_time_style:
+              long_time_format[0] = long_time_format[1] = "%Y-%m-%d %H:%M";
+              break;

-          case iso_time_style:
-            long_time_format[0] = "%Y-%m-%d ";
-            long_time_format[1] = "%m-%d %H:%M";
-            break;
+            case iso_time_style:
+              long_time_format[0] = "%Y-%m-%d ";
+              long_time_format[1] = "%m-%d %H:%M";
+              break;
+
+            case locale_time_style:
+              if (hard_locale (LC_TIME))
+                {
+                  int i;
+                  for (i = 0; i < 2; i++)
+                    long_time_format[i] =
+                      dcgettext (NULL, long_time_format[i], LC_TIME);
+                }
+            }
+        }

-          case locale_time_style:
-            if (hard_locale (LC_TIME))
-              {
-                int i;
-                for (i = 0; i < 2; i++)
-                  long_time_format[i] =
-                    dcgettext (NULL, long_time_format[i], LC_TIME);
-              }
-          }
       /* Note we leave %5b etc. alone so user widths/flags are honored.  */
       if (strstr (long_time_format[0], "%b")
           || strstr (long_time_format[1], "%b"))
--
1.7.8.163.g9859a



Ignoring white space changes:

diff --git a/src/ls.c b/src/ls.c
index 0d64bab..ac01f3d 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -2039,9 +2039,31 @@ decode_switches (int argc, char **argv)
           long_time_format[1] = p1;
         }
       else
-        switch (XARGMATCH ("time style", style,
-                           time_style_args,
-                           time_style_types))
+        {
+          ptrdiff_t res = argmatch (style, time_style_args,
+                                    (char const *) time_style_types,
+                                    sizeof (*time_style_types));
+          if (res < 0)
+            {
+              /* This whole block used to be a simple use of XARGMATCH.
+                 but that didn't print the "posix-"-prefixed variants or
+                 the "+"-prefixed format string option upon failure.  */
+              argmatch_invalid ("time style", style, res);
+
+              /* The following is a manual expansion of argmatch_valid,
+                 but with the added "+ ..." description and the [posix-]
+                 prefixes prepended.  Note that this simplification works
+                 only because all four existing time_style_types values
+                 are distinct.  */
+              fprintf (stderr, _("Valid arguments are:"));
+              char const *const *p = time_style_args;
+              while (*p)
+                fprintf (stderr, "  - `[posix-]%s'\n", *p++);
+              fprintf (stderr,
+                       _("  - `+' followed by a date format string\n"));
+              usage (LS_FAILURE);
+            }
+          switch (res)
           {
           case full_iso_time_style:
             long_time_format[0] = long_time_format[1] =
@@ -2066,6 +2088,8 @@ decode_switches (int argc, char **argv)
                     dcgettext (NULL, long_time_format[i], LC_TIME);
               }
           }
+        }
+
       /* Note we leave %5b etc. alone so user widths/flags are honored.  */
       if (strstr (long_time_format[0], "%b")
           || strstr (long_time_format[1], "%b"))

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

From: jidanni <at> jidanni.org
To: jim <at> meyering.net
Cc: 10253 <at> debbugs.gnu.org, eblake <at> redhat.com
Subject: Re: bug#10253: mention +FORMAT in ls time style reminder help blurb
Date: Sun, 11 Dec 2011 20:36:51 +0800

Thanks!

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

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Jim Meyering <jim <at> meyering.net>
Cc: 10253 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>, jidanni <at> jidanni.org
Subject: Re: bug#10253: mention +FORMAT in ls time style reminder help blurb
Date: Sun, 11 Dec 2011 14:48:20 -0800

I like the change, thanks.  A couple of nits:

On 12/11/11 03:07, Jim Meyering wrote:
> +              fprintf (stderr,
> +                       _("  - `+' followed by a date format string\n"));

I suggest supplying an example and quoting "date" so that it's clearer
that it's talking about the `date' command.  Something like this, perhaps?

   _("  - +FORMAT (e.g., +%H:%M) for a `date'-style format\n")

> +                fprintf (stderr, "  - `[posix-]%s'\n", *p++);

I suggest removing the ` and ' since they are locale-dependent
and aren't needed here (plus, that works better with the above
suggestion....).

> +              fprintf (stderr, _("Valid arguments are:"));

Isn't the usual style to use fputs when there's no directive
in the format?  There's one other example of this.

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

From: Jim Meyering <jim <at> meyering.net>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 10253 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>, jidanni <at> jidanni.org
Subject: Re: bug#10253: mention +FORMAT in ls time style reminder help blurb
Date: Mon, 12 Dec 2011 11:11:53 +0100

Paul Eggert wrote:
> I like the change, thanks.  A couple of nits:
>
> On 12/11/11 03:07, Jim Meyering wrote:
>> +              fprintf (stderr,
>> +                       _("  - `+' followed by a date format string\n"));
>
> I suggest supplying an example and quoting "date" so that it's clearer
> that it's talking about the `date' command.  Something like this, perhaps?
>
>    _("  - +FORMAT (e.g., +%H:%M) for a `date'-style format\n")

Thanks.  That is better.

>> +                fprintf (stderr, "  - `[posix-]%s'\n", *p++);
>
> I suggest removing the ` and ' since they are locale-dependent
> and aren't needed here (plus, that works better with the above
> suggestion....).

Good point.  Besides, I'd say that using quotes around syntax including
the likes of `[posix-]...' is misleading in that it might encourage
someone to use the []'s.

>> +              fprintf (stderr, _("Valid arguments are:"));
>
> Isn't the usual style to use fputs when there's no directive
> in the format?  There's one other example of this.

Yes, that is my preference, too.
Thanks for pointing it out.

I copied both that format-less fprintf and the `' mark-up from argmatch.c.
The fix there was easy: just use quote (...), since argmatch.c already
includes quote.h, so I've just fixed that in gnulib.

Here's a new version of the patch:
[slightly risky for translators and fuzzy string matchers:
 now there are two very similar strings:

  "Valid arguments are:\n" (here in ls.c)
  "Valid arguments are:"   (in gnulib's argmatch.c)

 It'd be easy rework argmatch.c to include the \n.
 ]

Now it prints this:

    $ src/ls -l --time-style=x
    src/ls: invalid argument `x' for `time style'
    Valid arguments are:
      - [posix-]full-iso
      - [posix-]long-iso
      - [posix-]iso
      - [posix-]locale
      - +FORMAT (e.g., +%H:%M) for a `date'-style format
    Try `src/ls --help' for more information.



From 79a14f0481df2bd45a20f92ce4c156f42fdae660 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering <at> redhat.com>
Date: Sun, 11 Dec 2011 11:59:31 +0100
Subject: [PATCH] ls: give a more useful diagnostic for a bogus --time-style
 arg

* src/ls.c (decode_switches): Replace our use of XARGMATCH
with open-coded version so that we can give a better diagnostic.
Reported by Dan Jacobson in http://bugs.gnu.org/10253
with suggestions from Eric Blake and Paul Eggert.
---
 src/ls.c |   72 +++++++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 48 insertions(+), 24 deletions(-)

diff --git a/src/ls.c b/src/ls.c
index 0d64bab..672237a 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -2039,33 +2039,57 @@ decode_switches (int argc, char **argv)
           long_time_format[1] = p1;
         }
       else
-        switch (XARGMATCH ("time style", style,
-                           time_style_args,
-                           time_style_types))
-          {
-          case full_iso_time_style:
-            long_time_format[0] = long_time_format[1] =
-              "%Y-%m-%d %H:%M:%S.%N %z";
-            break;
+        {
+          ptrdiff_t res = argmatch (style, time_style_args,
+                                    (char const *) time_style_types,
+                                    sizeof (*time_style_types));
+          if (res < 0)
+            {
+              /* This whole block used to be a simple use of XARGMATCH.
+                 but that didn't print the "posix-"-prefixed variants or
+                 the "+"-prefixed format string option upon failure.  */
+              argmatch_invalid ("time style", style, res);
+
+              /* The following is a manual expansion of argmatch_valid,
+                 but with the added "+ ..." description and the [posix-]
+                 prefixes prepended.  Note that this simplification works
+                 only because all four existing time_style_types values
+                 are distinct.  */
+              fputs (_("Valid arguments are:\n"), stderr);
+              char const *const *p = time_style_args;
+              while (*p)
+                fprintf (stderr, "  - [posix-]%s\n", *p++);
+              fputs (_("  - +FORMAT (e.g., +%H:%M) for a `date'-style"
+                       " format\n"), stderr);
+              usage (LS_FAILURE);
+            }
+          switch (res)
+            {
+            case full_iso_time_style:
+              long_time_format[0] = long_time_format[1] =
+                "%Y-%m-%d %H:%M:%S.%N %z";
+              break;

-          case long_iso_time_style:
-            long_time_format[0] = long_time_format[1] = "%Y-%m-%d %H:%M";
-            break;
+            case long_iso_time_style:
+              long_time_format[0] = long_time_format[1] = "%Y-%m-%d %H:%M";
+              break;

-          case iso_time_style:
-            long_time_format[0] = "%Y-%m-%d ";
-            long_time_format[1] = "%m-%d %H:%M";
-            break;
+            case iso_time_style:
+              long_time_format[0] = "%Y-%m-%d ";
+              long_time_format[1] = "%m-%d %H:%M";
+              break;
+
+            case locale_time_style:
+              if (hard_locale (LC_TIME))
+                {
+                  int i;
+                  for (i = 0; i < 2; i++)
+                    long_time_format[i] =
+                      dcgettext (NULL, long_time_format[i], LC_TIME);
+                }
+            }
+        }

-          case locale_time_style:
-            if (hard_locale (LC_TIME))
-              {
-                int i;
-                for (i = 0; i < 2; i++)
-                  long_time_format[i] =
-                    dcgettext (NULL, long_time_format[i], LC_TIME);
-              }
-          }
       /* Note we leave %5b etc. alone so user widths/flags are honored.  */
       if (strstr (long_time_format[0], "%b")
           || strstr (long_time_format[1], "%b"))
--
1.7.8.163.g9859a

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

From: Jim Meyering <jim <at> meyering.net>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 10253-done <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>,
	jidanni <at> jidanni.org
Subject: Re: bug#10253: mention +FORMAT in ls time style reminder help blurb
Date: Mon, 12 Dec 2011 12:20:51 +0100

Jim Meyering wrote:

> Paul Eggert wrote:
>> I like the change, thanks.  A couple of nits:
>>
>> On 12/11/11 03:07, Jim Meyering wrote:
>>> +              fprintf (stderr,
>>> +                       _("  - `+' followed by a date format string\n"));
>>
>> I suggest supplying an example and quoting "date" so that it's clearer
>> that it's talking about the `date' command.  Something like this, perhaps?
>>
>>    _("  - +FORMAT (e.g., +%H:%M) for a `date'-style format\n")
>
> Thanks.  That is better.
>
>>> +                fprintf (stderr, "  - `[posix-]%s'\n", *p++);
>>
>> I suggest removing the ` and ' since they are locale-dependent
>> and aren't needed here (plus, that works better with the above
>> suggestion....).
>
> Good point.  Besides, I'd say that using quotes around syntax including
> the likes of `[posix-]...' is misleading in that it might encourage
> someone to use the []'s.
>
>>> +              fprintf (stderr, _("Valid arguments are:"));
>>
>> Isn't the usual style to use fputs when there's no directive
>> in the format?  There's one other example of this.
>
> Yes, that is my preference, too.
> Thanks for pointing it out.
>
> I copied both that format-less fprintf and the `' mark-up from argmatch.c.
> The fix there was easy: just use quote (...), since argmatch.c already
> includes quote.h, so I've just fixed that in gnulib.
>
> Here's a new version of the patch:
> [slightly risky for translators and fuzzy string matchers:
>  now there are two very similar strings:
>
>   "Valid arguments are:\n" (here in ls.c)
>   "Valid arguments are:"   (in gnulib's argmatch.c)
>
>  It'd be easy rework argmatch.c to include the \n.
>  ]
>
> Now it prints this:
>
>     $ src/ls -l --time-style=x
>     src/ls: invalid argument `x' for `time style'
>     Valid arguments are:
>       - [posix-]full-iso
>       - [posix-]long-iso
>       - [posix-]iso
>       - [posix-]locale
>       - +FORMAT (e.g., +%H:%M) for a `date'-style format
>     Try `src/ls --help' for more information.

I wrote a test, amended the preceding to include it
and pushed this result:

From a3fee8b6afdbb70317d2124d5a3bb0d2887ab31b Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering <at> redhat.com>
Date: Sun, 11 Dec 2011 11:59:31 +0100
Subject: [PATCH] ls: give a more useful diagnostic for a bogus --time-style
 arg

* src/ls.c (decode_switches): Replace our use of XARGMATCH
with open-coded version so that we can give a better diagnostic.
* tests/ls/time-style-diag: New file.
* tests/Makefile.am (TESTS): Add it.
Reported by Dan Jacobson in http://bugs.gnu.org/10253
with suggestions from Eric Blake and Paul Eggert.
---
 gnulib                   |    2 +-
 src/ls.c                 |   72 ++++++++++++++++++++++++++++++---------------
 tests/Makefile.am        |    1 +
 tests/ls/time-style-diag |   39 +++++++++++++++++++++++++
 4 files changed, 89 insertions(+), 25 deletions(-)
 create mode 100755 tests/ls/time-style-diag

diff --git a/gnulib b/gnulib
index a5f6df2..f5c2e2a 160000
--- a/gnulib
+++ b/gnulib
@@ -1 +1 @@
-Subproject commit a5f6df2b1f3f0fdc73635de3ad285d21703dab18
+Subproject commit f5c2e2ac7d4ca2f6ba15e56a245f348899360a00
diff --git a/src/ls.c b/src/ls.c
index 0d64bab..672237a 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -2039,33 +2039,57 @@ decode_switches (int argc, char **argv)
           long_time_format[1] = p1;
         }
       else
-        switch (XARGMATCH ("time style", style,
-                           time_style_args,
-                           time_style_types))
-          {
-          case full_iso_time_style:
-            long_time_format[0] = long_time_format[1] =
-              "%Y-%m-%d %H:%M:%S.%N %z";
-            break;
+        {
+          ptrdiff_t res = argmatch (style, time_style_args,
+                                    (char const *) time_style_types,
+                                    sizeof (*time_style_types));
+          if (res < 0)
+            {
+              /* This whole block used to be a simple use of XARGMATCH.
+                 but that didn't print the "posix-"-prefixed variants or
+                 the "+"-prefixed format string option upon failure.  */
+              argmatch_invalid ("time style", style, res);
+
+              /* The following is a manual expansion of argmatch_valid,
+                 but with the added "+ ..." description and the [posix-]
+                 prefixes prepended.  Note that this simplification works
+                 only because all four existing time_style_types values
+                 are distinct.  */
+              fputs (_("Valid arguments are:\n"), stderr);
+              char const *const *p = time_style_args;
+              while (*p)
+                fprintf (stderr, "  - [posix-]%s\n", *p++);
+              fputs (_("  - +FORMAT (e.g., +%H:%M) for a `date'-style"
+                       " format\n"), stderr);
+              usage (LS_FAILURE);
+            }
+          switch (res)
+            {
+            case full_iso_time_style:
+              long_time_format[0] = long_time_format[1] =
+                "%Y-%m-%d %H:%M:%S.%N %z";
+              break;

-          case long_iso_time_style:
-            long_time_format[0] = long_time_format[1] = "%Y-%m-%d %H:%M";
-            break;
+            case long_iso_time_style:
+              long_time_format[0] = long_time_format[1] = "%Y-%m-%d %H:%M";
+              break;

-          case iso_time_style:
-            long_time_format[0] = "%Y-%m-%d ";
-            long_time_format[1] = "%m-%d %H:%M";
-            break;
+            case iso_time_style:
+              long_time_format[0] = "%Y-%m-%d ";
+              long_time_format[1] = "%m-%d %H:%M";
+              break;
+
+            case locale_time_style:
+              if (hard_locale (LC_TIME))
+                {
+                  int i;
+                  for (i = 0; i < 2; i++)
+                    long_time_format[i] =
+                      dcgettext (NULL, long_time_format[i], LC_TIME);
+                }
+            }
+        }

-          case locale_time_style:
-            if (hard_locale (LC_TIME))
-              {
-                int i;
-                for (i = 0; i < 2; i++)
-                  long_time_format[i] =
-                    dcgettext (NULL, long_time_format[i], LC_TIME);
-              }
-          }
       /* Note we leave %5b etc. alone so user widths/flags are honored.  */
       if (strstr (long_time_format[0], "%b")
           || strstr (long_time_format[1], "%b"))
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 48c33cb..23cb70f 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -441,6 +441,7 @@ TESTS =						\
   ls/stat-free-symlinks				\
   ls/stat-vs-dirent				\
   ls/symlink-slash				\
+  ls/time-style-diag				\
   ls/x-option					\
   mkdir/p-1					\
   mkdir/p-2					\
diff --git a/tests/ls/time-style-diag b/tests/ls/time-style-diag
new file mode 100755
index 0000000..d756cfe
--- /dev/null
+++ b/tests/ls/time-style-diag
@@ -0,0 +1,39 @@
+#!/bin/sh
+# Ensure that an invalid --time-style=ARG is diagnosed the way we want.
+
+# Copyright (C) 2011 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/init.sh"; path_prepend_ ../src
+print_ver_ ls
+
+ls -l --time-style=XX > out 2> err
+test $? = 2 || fail=1
+
+cat <<\EOF > exp || fail=1
+ls: invalid argument `XX' for `time style'
+Valid arguments are:
+  - [posix-]full-iso
+  - [posix-]long-iso
+  - [posix-]iso
+  - [posix-]locale
+  - +FORMAT (e.g., +%H:%M) for a `date'-style format
+Try `ls --help' for more information.
+EOF
+
+compare exp err || fail=1
+compare /dev/null out || fail=1
+
+Exit $fail
--
1.7.8.163.g9859a

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

From: jidanni <at> jidanni.org
To: jim <at> meyering.net
Cc: 10253 <at> debbugs.gnu.org, eggert <at> cs.ucla.edu, eblake <at> redhat.com
Subject: Re: bug#10253: mention +FORMAT in ls time style reminder help blurb
Date: Mon, 12 Dec 2011 22:56:12 +0800

>>>>> "JM" == Jim Meyering <jim <at> meyering.net> writes:
JM> is misleading in that it might encourage someone to use the []'s.
Like You Know Who :-), who also recommends you figure out a way to get
rid of the
JM>     $ src/ls -l --time-style=x
"src/" even in these e-mails/commits, as it is bad for the eye, even though it
yes surely disappears in production.
Having walked from the airport... I'll be dead soon
http://www.youtube.com/watch?v=Tp8XcAKYsKo&list=PL6E40919035151385

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

From: Jim Meyering <jim <at> meyering.net>
To: jidanni <at> jidanni.org
Cc: 10253 <at> debbugs.gnu.org, eggert <at> cs.ucla.edu, eblake <at> redhat.com
Subject: Re: bug#10253: mention +FORMAT in ls time style reminder help blurb
Date: Mon, 12 Dec 2011 16:55:06 +0100

jidanni <at> jidanni.org wrote:
> JM>     $ src/ls -l --time-style=x
> "src/" even in these e-mails/commits, as it is bad for the eye, even though it
> yes surely disappears in production.

Actually, using the "src/" prefix (or some prefix, like "./")
is important.  Otherwise, I'm not testing what I've just built.
Hence, including that prefix shows what I've done.  Without it,
the reader would wonder if I'd simply used whatever ls is in my path,
which could easily represent a mistake.

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

From: jidanni <at> jidanni.org
To: jim <at> meyering.net
Cc: 10253 <at> debbugs.gnu.org, eggert <at> cs.ucla.edu, eblake <at> redhat.com
Subject: Re: bug#10253: mention +FORMAT in ls time style reminder help blurb
Date: Tue, 13 Dec 2011 00:07:37 +0800

>>>>> "JM" == Jim Meyering <jim <at> meyering.net> writes:
JM> Hence, including that prefix shows what I've done.  Without it,
JM> the reader would wonder if I'd simply used whatever ls is in my path,
JM> which could easily represent a mistake.
Well OK then.

Previous Next

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