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 <meyer...@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
