bug#10253: mention +FORMAT in ls time style reminder help blurb

[Jim Meyering]

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 <address@hidden>
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