ls: match GNU's error for a bad --format/--sort/--time/etc. value

src/uu/ls/locales/en-US.ftl

@@ -34,6 +34,14 @@ ls-error-invalid-time-style = invalid --time-style argument {$style}
     - +FORMAT (e.g., +%H:%M) for a 'date'-style format
 
   For more information try --help
+ls-error-invalid-choice = invalid argument '{$arg}' for '--{$option}'
+  Valid arguments are:
+  {$choices}
+  Try 'ls --help' for more information.
+ls-error-ambiguous-choice = ambiguous argument '{$arg}' for '--{$option}'
+  Valid arguments are:
+  {$choices}
+  Try 'ls --help' for more information.
 
 # Help messages
 ls-help-print-help = Print help information.

src/uu/ls/locales/fr-FR.ftl

@@ -32,6 +32,14 @@ ls-error-invalid-time-style = argument --time-style invalide {$style}
     - +FORMAT (e.g., +%H:%M) pour un format de type 'date'
 
   Pour plus d'informations, essayez --help
+ls-error-invalid-choice = argument '{$arg}' invalide pour '--{$option}'
+  Les arguments valides sont :
+  {$choices}
+  Essayez 'ls --help' pour plus d'informations.
+ls-error-ambiguous-choice = argument '{$arg}' ambigu pour '--{$option}'
+  Les arguments valides sont :
+  {$choices}
+  Essayez 'ls --help' pour plus d'informations.
 
 # Messages d'aide
 ls-help-print-help = Afficher les informations d'aide.

src/uu/ls/src/ls.rs

@@ -34,7 +34,7 @@ use thiserror::Error;
 use uucore::libc::{S_IXGRP, S_IXOTH, S_IXUSR};
 use uucore::{
     display::Quotable,
-    error::{UError, UResult, set_exit_code, strip_errno},
+    error::{UError, UResult, USimpleError, set_exit_code, strip_errno},
     format_usage,
     fs::FileInformation,
     fsext::metadata_get_time,
@@ -115,15 +115,162 @@ impl UError for LsError {
     }
 }
 
+/// The choices each of `WHEN`-style option's value accepts, in the order
+/// GNU lists them. Each choice's aliases are grouped on a single line the
+/// way GNU does; `columns` on `--format` is this implementation's own
+/// extension, which GNU's `--format` does not support at all.
+const WHEN_CHOICES: &[&[&str]] = &[
+    &["always", "yes", "force"],
+    &["never", "no", "none"],
+    &["auto", "tty", "if-tty"],
+];
+
+/// The options in this file that validate an enumerated choice with a
+/// `ShortcutValueParser`. That parser reports an invalid or ambiguous
+/// value with clap's own generic wording, not GNU's; each entry here is
+/// the metadata this module's [`invalid_choice_error`] needs to
+/// reconstruct GNU's wording after clap has already rejected a value.
+struct ChoiceOption {
+    long: &'static str,
+    groups: &'static [&'static [&'static str]],
+}
+
+const CHOICE_OPTIONS: &[ChoiceOption] = &[
+    ChoiceOption {
+        long: options::FORMAT,
+        groups: &[
+            &["verbose", "long"],
+            &["commas"],
+            &["horizontal", "across"],
+            &["vertical"],
+            &["single-column"],
+            &["columns"],
+        ],
+    },
+    ChoiceOption {
+        long: options::HYPERLINK,
+        groups: WHEN_CHOICES,
+    },
+    ChoiceOption {
+        long: QUOTING_STYLE,
+        groups: &[
+            &["literal"],
+            &["shell"],
+            &["shell-always"],
+            &["shell-escape"],
+            &["shell-escape-always"],
+            &["c"],
+            &["c-maybe"],
+            &["escape"],
+            &["locale"],
+            &["clocale"],
+        ],
+    },
+    ChoiceOption {
+        long: options::TIME,
+        groups: &[
+            &["atime", "access", "use"],
+            &["ctime", "status"],
+            &["mtime", "modification"],
+            &["birth", "creation"],
+        ],
+    },
+    ChoiceOption {
+        long: options::SORT,
+        groups: &[
+            &["none"],
+            &["size"],
+            &["time"],
+            &["version"],
+            &["extension"],
+            &["name"],
+            &["width"],
+        ],
+    },
+    ChoiceOption {
+        long: options::COLOR,
+        groups: WHEN_CHOICES,
+    },
+    ChoiceOption {
+        long: options::INDICATOR_STYLE,
+        groups: &[&["none"], &["slash"], &["file-type"], &["classify"]],
+    },
+    ChoiceOption {
+        long: options::indicator_style::CLASSIFY,
+        groups: WHEN_CHOICES,
+    },
+];
+
+/// Rebuilds `clap_error` -- already known to be an `ErrorKind::InvalidValue`
+/// on one of `CHOICE_OPTIONS` -- as GNU's own wording instead of clap's,
+/// or returns `None` if it names none of them (letting the caller fall
+/// back to the shared formatter for everything else).
+fn invalid_choice_error(clap_error: &clap::Error) -> Option<Box<dyn UError>> {
+    let arg = clap_error
+        .get(clap::error::ContextKind::InvalidArg)?
+        .to_string();
+    let value = clap_error
+        .get(clap::error::ContextKind::InvalidValue)?
+        .to_string();
+    let option = CHOICE_OPTIONS
+        .iter()
+        .find(|opt| arg.contains(&format!("--{}", opt.long)))?;
+
+    let list = || {
+        option
+            .groups
+            .iter()
+            .map(|group| {
+                format!(
+                    "  - {}",
+                    group
+                        .iter()
+                        .map(|name| format!("'{name}'"))
+                        .collect::<Vec<_>>()
+                        .join(", ")
+                )
+            })
+            .collect::<Vec<_>>()
+            .join("\n")
+    };
+    let matched_groups = option
+        .groups
+        .iter()
+        .filter(|group| group.iter().any(|name| name.starts_with(&value)))
+        .count();
+    let message = if matched_groups > 1 {
+        translate!("ls-error-ambiguous-choice", "option" => option.long, "arg" => value, "choices" => list())
+    } else {
+        translate!("ls-error-invalid-choice", "option" => option.long, "arg" => value, "choices" => list())
+    };
+    Some(USimpleError::new(1, message))
+}
+
+/// Parses the command line the way
+/// [`uucore::clap_localization::handle_clap_result_with_diagnostics`] does,
+/// except that an `ErrorKind::InvalidValue` naming one of `CHOICE_OPTIONS`
+/// is reported with GNU's wording via [`invalid_choice_error`] instead of
+/// the shared formatter's.
+fn get_matches(args: Vec<OsString>) -> UResult<(clap::ArgMatches, Option<Vec<OsString>>)> {
+    let diag_args = uucore::diagnostics::capture(&args);
+    match uu_app().try_get_matches_from(args) {
+        Ok(matches) => Ok((matches, diag_args)),
+        Err(clap_error) => {
+            if clap_error.kind() == clap::error::ErrorKind::InvalidValue
+                && let Some(err) = invalid_choice_error(&clap_error)
+            {
+                return Err(err);
+            }
+            uucore::clap_localization::handle_clap_error_with_exit_code(clap_error, 2);
+        }
+    }
+}
+
 #[uucore::main]
 pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     // The arguments are kept for the caret in SIZE diagnostics, which echoes
     // the command line.
-    let (matches, diag_args) = uucore::clap_localization::handle_clap_result_with_diagnostics(
-        uu_app(),
-        args.collect(),
-        2,
-    )?;
+    let (matches, diag_args) = get_matches(args.collect())?;
 
     uucore::i18n::collator::init_locale_collation();
 

tests/by-util/test_ls.rs

@@ -104,15 +104,15 @@ fn test_localized_possible_values() {
         (
             "en_US.UTF-8",
             vec![
-                "error: invalid value 'invalid_test_value' for '--color",
-                "[possible values:",
+                "invalid argument 'invalid_test_value' for '--color'",
+                "Valid arguments are:",
             ],
         ),
         (
             "fr_FR.UTF-8",
             vec![
-                "erreur : valeur invalide 'invalid_test_value' pour '--color",
-                "[valeurs possibles:",
+                "argument 'invalid_test_value' invalide pour '--color'",
+                "Les arguments valides sont :",
             ],
         ),
     ];
@@ -133,6 +133,126 @@ fn test_localized_possible_values() {
 }
 /* spellchecker: enable */
 
+#[test]
+fn test_format_invalid_arg_message() {
+    // `columns` is this implementation's own extra --format choice; GNU's
+    // --format does not accept it at all.
+    new_ucmd!()
+        .arg("--format=bogus")
+        .fails_with_code(1)
+        .stderr_is(concat!(
+            "ls: invalid argument 'bogus' for '--format'\n",
+            "Valid arguments are:\n",
+            "  - 'verbose', 'long'\n",
+            "  - 'commas'\n",
+            "  - 'horizontal', 'across'\n",
+            "  - 'vertical'\n",
+            "  - 'single-column'\n",
+            "  - 'columns'\n",
+            "Try 'ls --help' for more information.\n",
+        ));
+}
+
+#[test]
+fn test_time_ambiguous_arg_message() {
+    new_ucmd!()
+        .arg("--time=bogus")
+        .fails_with_code(1)
+        .stderr_is(concat!(
+            "ls: invalid argument 'bogus' for '--time'\n",
+            "Valid arguments are:\n",
+            "  - 'atime', 'access', 'use'\n",
+            "  - 'ctime', 'status'\n",
+            "  - 'mtime', 'modification'\n",
+            "  - 'birth', 'creation'\n",
+            "Try 'ls --help' for more information.\n",
+        ));
+}
+
+#[test]
+fn test_quoting_style_invalid_arg_message() {
+    // Unlike every other option here, GNU lists each of --quoting-style's
+    // choices on its own line, even 'c' and 'c-maybe', which are aliases
+    // for the same style.
+    new_ucmd!()
+        .arg("--quoting-style=bogus")
+        .fails_with_code(1)
+        .stderr_is(concat!(
+            "ls: invalid argument 'bogus' for '--quoting-style'\n",
+            "Valid arguments are:\n",
+            "  - 'literal'\n",
+            "  - 'shell'\n",
+            "  - 'shell-always'\n",
+            "  - 'shell-escape'\n",
+            "  - 'shell-escape-always'\n",
+            "  - 'c'\n",
+            "  - 'c-maybe'\n",
+            "  - 'escape'\n",
+            "  - 'locale'\n",
+            "  - 'clocale'\n",
+            "Try 'ls --help' for more information.\n",
+        ));
+}
+
+#[test]
+fn test_sort_invalid_arg_message() {
+    new_ucmd!()
+        .arg("--sort=bogus")
+        .fails_with_code(1)
+        .stderr_is(concat!(
+            "ls: invalid argument 'bogus' for '--sort'\n",
+            "Valid arguments are:\n",
+            "  - 'none'\n",
+            "  - 'size'\n",
+            "  - 'time'\n",
+            "  - 'version'\n",
+            "  - 'extension'\n",
+            "  - 'name'\n",
+            "  - 'width'\n",
+            "Try 'ls --help' for more information.\n",
+        ));
+}
+
+#[test]
+fn test_indicator_style_invalid_arg_message() {
+    new_ucmd!()
+        .arg("--indicator-style=bogus")
+        .fails_with_code(1)
+        .stderr_is(concat!(
+            "ls: invalid argument 'bogus' for '--indicator-style'\n",
+            "Valid arguments are:\n",
+            "  - 'none'\n",
+            "  - 'slash'\n",
+            "  - 'file-type'\n",
+            "  - 'classify'\n",
+            "Try 'ls --help' for more information.\n",
+        ));
+}
+
+#[test]
+fn test_hyperlink_ambiguous_arg_message() {
+    new_ucmd!()
+        .arg("--hyperlink=a")
+        .fails_with_code(1)
+        .stderr_is(concat!(
+            "ls: ambiguous argument 'a' for '--hyperlink'\n",
+            "Valid arguments are:\n",
+            "  - 'always', 'yes', 'force'\n",
+            "  - 'never', 'no', 'none'\n",
+            "  - 'auto', 'tty', 'if-tty'\n",
+            "Try 'ls --help' for more information.\n",
+        ));
+}
+
+#[test]
+fn test_classify_when_still_works() {
+    // The --classify=WHEN interception must not disturb normal parsing.
+    new_ucmd!()
+        .arg("--classify=never")
+        .arg("--classify=always")
+        .succeeds();
+}
+
 #[test]
 fn test_invalid_value_returns_2() {
     // Invalid values to flags *sometimes* result in error code 2: