Subject: Re: bug#79113: du --time-styles inconsistencies between manual and behaviour

From: Pádraig Brady <[email-masked]>
Date: Mon, 28 Jul 2025 23:11:58 +0100

On 28/07/2025 20:13, Pádraig Brady wrote:
> On 28/07/2025 18:49, Nicolas Boichat wrote:
>> I could have been clearer for this last one, I mean that the command
>> line error text for `du` could mention that +FORMAT is supported:
>>
>> Comparing du and ls output with a bad timestyle:
>> ```
>> $ du --time --time-style=xyz blob
>> du: invalid argument ‘xyz’ for ‘time style’
>> Valid arguments are:
>>     - ‘full-iso’
>>     - ‘long-iso’
>>     - ‘iso’
>>>>>>> it would be nice to add `- +FORMAT (e.g., +%H:%M) for a 'date'-style format` here
>> Try 'du --help' for more information.
>> $ ls -l --time-style=xyz blob
>> ls: invalid argument ‘xyz’ 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.
>> ```
> 
> Oh right.
> I'll refactor the code that prints the ls and du errors in this case.
> 

Will push the attached later.

cheers,
Padraig

From f00fa5e1cbf9cec6ff76c594d0ef8edba7c81a3a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email-masked]>
Date: Mon, 28 Jul 2025 23:05:49 +0100
Subject: [PATCH] du: improve diagnostics for --time-style

* src/system.h (x_timestyle_args): A new function refactored from ...
* src/ls.c (main): ... here.
* src/du.c: Use refactored x_timestyle_args() to output a custom error.
Addresses https://bugs.gnu.org/79113
---
 src/du.c     |  8 +++++---
 src/ls.c     | 30 +++++-------------------------
 src/system.h | 38 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+), 28 deletions(-)

diff --git a/src/du.c b/src/du.c
index bac372a04..c93d7f71b 100644
--- a/src/du.c
+++ b/src/du.c
@@ -26,8 +26,8 @@
 #include <config.h>
 #include <getopt.h>
 #include <sys/types.h>
-#include "system.h"
 #include "argmatch.h"
+#include "system.h"
 #include "argv-iter.h"
 #include "assure.h"
 #include "di-set.h"
@@ -974,8 +974,10 @@ main (int argc, char **argv)
         time_format = time_style + 1;
       else
         {
-          switch (XARGMATCH ("time style", time_style,
-                             time_style_args, time_style_types))
+          switch (x_timestyle_match (time_style, /*allow_posix=*/ false,
+                                     time_style_args,
+                                     (char const *) time_style_types,
+                                     sizeof (*time_style_types), EXIT_FAILURE))
             {
             case full_iso_time_style:
               time_format = "%Y-%m-%d %H:%M:%S.%N %z";
diff --git a/src/ls.c b/src/ls.c
index e84e0facf..d9faddee4 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -80,11 +80,11 @@
 # define SA_RESTART 0
 #endif
 
-#include "system.h"
 #include <fnmatch.h>
 
 #include "acl.h"
 #include "argmatch.h"
+#include "system.h"
 #include "assure.h"
 #include "c-strcase.h"
 #include "dev-ino.h"
@@ -2464,30 +2464,10 @@ decode_switches (int argc, char **argv)
         }
       else
         {
-          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)
+          switch (x_timestyle_match (style, /*allow_posix=*/ true,
+                                     time_style_args,
+                                     (char const *) time_style_types,
+                                     sizeof (*time_style_types), LS_FAILURE))
             {
             case full_iso_time_style:
               long_time_format[0] = long_time_format[1] =
diff --git a/src/system.h b/src/system.h
index ca223d547..5cb751cc8 100644
--- a/src/system.h
+++ b/src/system.h
@@ -794,3 +794,41 @@ is_ENOTSUP (int err)
   quotearg_style (shell_escape_always_quoting_style, arg)
 #define quoteaf_n(n, arg) \
   quotearg_n_style (n, shell_escape_always_quoting_style, arg)
+
+/* Used instead of XARGMATCH() to provide a custom error message.  */
+#ifdef XARGMATCH
+static inline ptrdiff_t
+x_timestyle_match (char const * style, bool allow_posix,
+                   char const *const * timestyle_args,
+                   char const * timestyle_types,
+                   size_t timestyle_types_size,
+                   int fail_status)
+{
+  ptrdiff_t res = argmatch (style, timestyle_args,
+                            (char const *) timestyle_types,
+                            timestyle_types_size);
+  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 = timestyle_args;
+      char const *posix_prefix = allow_posix ? "[posix-]" : "";
+      while (*p)
+        fprintf (stderr, "  - %s%s\n", posix_prefix, *p++);
+      fputs (_("  - +FORMAT (e.g., +%H:%M) for a 'date'-style"
+               " format\n"), stderr);
+      usage (fail_status);
+    }
+
+  return res;
+}
+#endif
-- 
2.50.0