From f37e0180c5ba850ebd5093d637d5b163102614aa Mon Sep 17 00:00:00 2001 From: arbelonson-source <269032023+arbelonson-source@users.noreply.github.com> Date: Mon, 31 Aug 2026 06:23:36 +0300 Subject: [PATCH] du: match GNU's error for a bad --time value, fix --time-style's help hint --time validated its value with a plain ShortcutValueParser, so an unrecognized or ambiguous value (e.g. --time=bogus) produced clap's own generic wording instead of GNU's grouped-alias 'invalid argument ... for --time / Valid arguments are: / - 'atime', 'access', 'use' / ...'. Resolve the value against the option's own alias groups by hand, accepting any unambiguous abbreviation the way GNU does (including one ambiguous only between aliases of the same choice), and report GNU's wording -- extended with this implementation's own 'creation'/'birth' choice, which GNU's --time does not support at all -- when it does not resolve. Also fixes an unrelated, adjacent bug in the same error family: --time-style's own invalid-argument message already had GNU's wording, but its 'Try ... for more information' hint used uucore::execution_phrase() (just the invoked binary, e.g. 'target/debug/du') without appending '--help', and quoted full-iso/long-iso/iso where GNU does not. AI-assisted-by: Claude Opus 5, via Claude Code --- src/uu/du/locales/en-US.ftl | 16 ++++++-- src/uu/du/locales/fr-FR.ftl | 16 ++++++-- src/uu/du/src/du.rs | 77 +++++++++++++++++++++++++++++++------ tests/by-util/test_du.rs | 54 ++++++++++++++++++++++++++ 4 files changed, 143 insertions(+), 20 deletions(-) diff --git a/src/uu/du/locales/en-US.ftl b/src/uu/du/locales/en-US.ftl index cf69ccdbb99..364de9f045e 100644 --- a/src/uu/du/locales/en-US.ftl +++ b/src/uu/du/locales/en-US.ftl @@ -50,12 +50,20 @@ du-error-invalid-max-depth = invalid maximum depth { $depth } du-error-summarize-depth-conflict = summarizing conflicts with --max-depth={ $depth } du-error-invalid-time-style = invalid argument { $style } for 'time style' Valid arguments are: - - 'full-iso' - - 'long-iso' - - 'iso' + - full-iso + - long-iso + - iso - +FORMAT (e.g., +%H:%M) for a 'date'-style format - Try '{ $help }' for more information. + Try '{ $help } --help' for more information. du-error-invalid-time-arg = 'birth' and 'creation' arguments for --time are not supported on this platform. +du-error-invalid-time-choice = invalid argument '{ $arg }' for '--time' + Valid arguments are: + { $choices } + Try 'du --help' for more information. +du-error-ambiguous-time-choice = ambiguous argument '{ $arg }' for '--time' + Valid arguments are: + { $choices } + Try 'du --help' for more information. du-error-invalid-glob = Invalid exclude syntax: { $error } du-error-cannot-read-directory = cannot read directory { $path } du-error-cannot-access = cannot access { $path } diff --git a/src/uu/du/locales/fr-FR.ftl b/src/uu/du/locales/fr-FR.ftl index 27bed339a41..7007ba6789a 100644 --- a/src/uu/du/locales/fr-FR.ftl +++ b/src/uu/du/locales/fr-FR.ftl @@ -50,12 +50,20 @@ du-error-invalid-max-depth = profondeur maximale invalide { $depth } du-error-summarize-depth-conflict = la synthèse entre en conflit avec --max-depth={ $depth } du-error-invalid-time-style = argument invalide { $style } pour 'style de temps' Les arguments valides sont : - - 'full-iso' - - 'long-iso' - - 'iso' + - full-iso + - long-iso + - iso - +FORMAT (e.g., +%H:%M) pour un format de type 'date' - Essayez '{ $help }' pour plus d'informations. + Essayez '{ $help } --help' pour plus d'informations. du-error-invalid-time-arg = les arguments 'birth' et 'creation' pour --time ne sont pas pris en charge sur cette plateforme. +du-error-invalid-time-choice = argument '{ $arg }' invalide pour '--time' + Les arguments valides sont : + { $choices } + Essayez 'du --help' pour plus d'informations. +du-error-ambiguous-time-choice = argument '{ $arg }' ambigu pour '--time' + Les arguments valides sont : + { $choices } + Essayez 'du --help' pour plus d'informations. du-error-invalid-glob = Syntaxe d'exclusion invalide : { $error } du-error-cannot-read-directory = impossible de lire le répertoire { $path } du-error-cannot-access = impossible d'accéder à { $path } diff --git a/src/uu/du/src/du.rs b/src/uu/du/src/du.rs index e0968556acb..83b66ab066d 100644 --- a/src/uu/du/src/du.rs +++ b/src/uu/du/src/du.rs @@ -5,7 +5,7 @@ // // spell-checker:ignore fstatat openat dirfd -use clap::{Arg, ArgAction, ArgMatches, Command, builder::PossibleValue}; +use clap::{Arg, ArgAction, ArgMatches, Command}; use glob::{Pattern, PatternError}; use rustc_hash::FxHashSet as HashSet; use std::env; @@ -35,7 +35,6 @@ use uucore::translate; use uucore::parser::parse_block_size; use uucore::parser::parse_glob; use uucore::parser::parse_size::{ParseSizeError, parse_size_u64}; -use uucore::parser::shortcut_value_parser::ShortcutValueParser; use uucore::time::{FormatSystemTimeFallback, format, format_system_time}; use uucore::{format_usage, show, show_error, show_warning}; #[cfg(windows)] @@ -1095,11 +1094,13 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { vec![PathBuf::from(".")] }; - let time = matches.contains_id(options::TIME).then(|| { - matches - .get_one::(options::TIME) - .map_or(MetadataTimeField::Modification, |s| s.as_str().into()) - }); + let time = matches + .contains_id(options::TIME) + .then(|| match matches.get_one::(options::TIME) { + Some(s) => resolve_time_choice(s).map(Into::into), + None => Ok(MetadataTimeField::Modification), + }) + .transpose()?; let size_format = parse_size_format(&matches, diag_args.as_deref())?; @@ -1276,6 +1277,63 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { Ok(()) } +/// The choices `--time`'s value accepts. GNU groups each choice's aliases +/// on a single line in its error message; `creation`/`birth` is this +/// implementation's own extension, listed last since GNU's `--time` does +/// not support it at all. +const TIME_CHOICE_GROUPS: &[&[&str]] = &[ + &["atime", "access", "use"], + &["ctime", "status"], + &["creation", "birth"], +]; + +/// The name `value` names among `TIME_CHOICE_GROUPS`, accepting any +/// unambiguous abbreviation the way GNU does (including one that is +/// ambiguous between aliases of the *same* choice). +fn resolve_time_choice(value: &str) -> UResult<&'static str> { + let list = || { + TIME_CHOICE_GROUPS + .iter() + .map(|group| { + format!( + " - {}", + group + .iter() + .map(|name| format!("'{name}'")) + .collect::>() + .join(", ") + ) + }) + .collect::>() + .join("\n") + }; + let matches: Vec<(usize, &'static str)> = TIME_CHOICE_GROUPS + .iter() + .enumerate() + .flat_map(|(i, group)| group.iter().map(move |name| (i, *name))) + .filter(|(_, name)| name.starts_with(value)) + .collect(); + + if !value.is_empty() + && let Some(&(_, exact)) = matches.iter().find(|(_, name)| *name == value) + { + return Ok(exact); + } + match matches.first() { + Some(&(group, name)) if !value.is_empty() && matches.iter().all(|(g, _)| *g == group) => { + Ok(name) + } + Some(_) => Err(USimpleError::new( + 1, + translate!("du-error-ambiguous-time-choice", "arg" => value.to_string(), "choices" => list()), + )), + None => Err(USimpleError::new( + 1, + translate!("du-error-invalid-time-choice", "arg" => value.to_string(), "choices" => list()), + )), + } +} + // Parse --time-style argument, falling back to environment variable if necessary. fn parse_time_style(s: Option<&String>) -> UResult { let s = match s { @@ -1540,11 +1598,6 @@ pub fn uu_app() -> Command { .value_name("WORD") .require_equals(true) .num_args(0..) - .value_parser(ShortcutValueParser::new([ - PossibleValue::new("atime").alias("access").alias("use"), - PossibleValue::new("ctime").alias("status"), - PossibleValue::new("creation").alias("birth"), - ])) .help(translate!("du-help-time")) .overrides_with(options::TIME), ) diff --git a/tests/by-util/test_du.rs b/tests/by-util/test_du.rs index 470382848fe..b3dd53b93d5 100644 --- a/tests/by-util/test_du.rs +++ b/tests/by-util/test_du.rs @@ -1137,6 +1137,40 @@ fn test_du_time() { } } +#[test] +fn test_du_time_invalid_arg_message() { + new_ucmd!() + .arg("--time=bogus") + .arg(".") + .fails() + .stderr_is(concat!( + "du: invalid argument 'bogus' for '--time'\n", + "Valid arguments are:\n", + " - 'atime', 'access', 'use'\n", + " - 'ctime', 'status'\n", + " - 'creation', 'birth'\n", + "Try 'du --help' for more information.\n", + )); +} + +#[test] +fn test_du_time_ambiguous_arg_message() { + // 'c' is ambiguous between 'ctime' and this implementation's own + // 'creation' extension, which GNU's --time does not accept at all. + new_ucmd!() + .arg("--time=c") + .arg(".") + .fails() + .stderr_is(concat!( + "du: ambiguous argument 'c' for '--time'\n", + "Valid arguments are:\n", + " - 'atime', 'access', 'use'\n", + " - 'ctime', 'status'\n", + " - 'creation', 'birth'\n", + "Try 'du --help' for more information.\n", + )); +} + #[cfg(feature = "touch")] fn birth_supported() -> bool { let ts = TestScenario::new(util_name!()); @@ -2003,6 +2037,26 @@ fn test_invalid_time_style() { .stdout_does_not_contain("du: invalid argument 'banana' for 'time style'"); } +#[test] +fn test_invalid_time_style_with_time_message() { + let result = new_ucmd!() + .arg("--time") + .arg("--time-style=banana") + .arg(".") + .fails(); + result.stderr_contains(concat!( + "du: invalid argument 'banana' for 'time style'\n", + "Valid arguments are:\n", + " - full-iso\n", + " - long-iso\n", + " - iso\n", + " - +FORMAT (e.g., +%H:%M) for a 'date'-style format\n", + )); + // The binary path `execution_phrase()` reports varies by how the test is + // invoked; only the "du --help" suffix is stable. + result.stderr_contains("du --help' for more information.\n"); +} + #[test] fn test_human_size() { use std::fs::File;