From 6327784150b4cc65dc67d62b4a379602ab3b50b3 Mon Sep 17 00:00:00 2001 From: arbelonson-source <269032023+arbelonson-source@users.noreply.github.com> Date: Mon, 31 Aug 2026 07:15:44 +0300 Subject: [PATCH] ls: match GNU's error for a bad --format/--sort/--time/etc. value Eight options here validate an enumerated choice with a ShortcutValueParser: --format, --hyperlink, --quoting-style, --time, --sort, --color, --indicator-style, and --classify. An unrecognized or ambiguous value on any of them produces clap's own generic wording instead of GNU's grouped-alias one. Unlike the other options fixed in this series (#14293, #14303-#14312), none of these are resolved by hand: each keeps its ShortcutValueParser completely unchanged, since that's still the source of truth clap uses both to validate the value and to render --format's/etc.'s own --help text. Instead, clap errors are now parsed through a small wrapper (get_matches) that, on an ErrorKind::InvalidValue naming one of these eight options, reconstructs GNU's wording from a metadata table (CHOICE_OPTIONS) describing each option's own choice groups in GNU's display order; every other clap error -- unknown flags, --help, --version, etc. -- still goes through the exact same shared formatter as before, unchanged. Two of these options have their own uutils-only choice not present in GNU's real --format/--sort at all: --format=columns (GNU's own 'vertical' choice has no separate word for it) is listed as its own group, same as du's 'creation'/'birth' precedent (#14306) -- --quoting-style is unusual in the other direction: GNU lists each of its choices, including 'c' and 'c-maybe' (a real alias pair), on its own separate line rather than grouping aliases together the way every other option in this series does. AI-assisted-by: Claude Opus 5, via Claude Code --- src/uu/ls/locales/en-US.ftl | 8 ++ src/uu/ls/locales/fr-FR.ftl | 8 ++ src/uu/ls/src/ls.rs | 159 ++++++++++++++++++++++++++++++++++-- tests/by-util/test_ls.rs | 128 ++++++++++++++++++++++++++++- 4 files changed, 293 insertions(+), 10 deletions(-) diff --git a/src/uu/ls/locales/en-US.ftl b/src/uu/ls/locales/en-US.ftl index 72380e3a356..d381ff67310 100644 --- a/src/uu/ls/locales/en-US.ftl +++ b/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. diff --git a/src/uu/ls/locales/fr-FR.ftl b/src/uu/ls/locales/fr-FR.ftl index 06090f76393..c64f4da9d11 100644 --- a/src/uu/ls/locales/fr-FR.ftl +++ b/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. diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 6c861aa409f..dd0daf6fc3a 100644 --- a/src/uu/ls/src/ls.rs +++ b/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> { + 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::>() + .join(", ") + ) + }) + .collect::>() + .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) -> UResult<(clap::ArgMatches, Option>)> { + 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(); diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 08233e3ebe3..b268a38e67a 100644 --- a/tests/by-util/test_ls.rs +++ b/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: