diff --git a/src/uu/wc/locales/en-US.ftl b/src/uu/wc/locales/en-US.ftl index c6d2262b950..48e0c70dd00 100644 --- a/src/uu/wc/locales/en-US.ftl +++ b/src/uu/wc/locales/en-US.ftl @@ -23,6 +23,17 @@ wc-error-cannot-open-for-reading = cannot open { $path } for reading wc-error-read-error = { $path }: read error wc-error-failed-to-print-result = failed to print result for { $title } wc-error-failed-to-print-total = failed to print total +wc-error-invalid-total-argument = invalid argument '{ $arg }' for '--total' + { wc-error-total-valid-arguments } +wc-error-ambiguous-total-argument = ambiguous argument '{ $arg }' for '--total' + { wc-error-total-valid-arguments } +wc-error-total-valid-arguments = + Valid arguments are: + - 'auto' + - 'always' + - 'only' + - 'never' + Try 'wc --help' for more information. # Decoder error messages decoder-error-invalid-byte-sequence = invalid byte sequence: { $bytes } diff --git a/src/uu/wc/src/wc.rs b/src/uu/wc/src/wc.rs index a7fea0519e9..8dcdcffd0ab 100644 --- a/src/uu/wc/src/wc.rs +++ b/src/uu/wc/src/wc.rs @@ -32,7 +32,6 @@ use uucore::{ error::{FromIo, UError, UResult}, format_usage, hardware::{HardwareFeature, HasHardwareFeatures as _, SimdPolicy}, - parser::shortcut_value_parser::ShortcutValueParser, quoting_style::{self, QuotingStyle}, show, }; @@ -74,15 +73,20 @@ impl Default for Settings<'_> { } impl<'a> Settings<'a> { - fn new(matches: &'a ArgMatches) -> Self { + fn new(matches: &'a ArgMatches) -> UResult { let files0_from = matches .get_one::(options::FILES0_FROM) .map(Into::into); - let total_when = matches - .get_one::(options::TOTAL) - .map(Into::into) - .unwrap_or_default(); + // The last `--total` wins, but every one of them is still checked: + // GNU rejects a bad value even where a later one overrides it. + let mut total_when = TotalWhen::default(); + for value in matches + .get_many::(options::TOTAL) + .unwrap_or_default() + { + total_when = TotalWhen::parse(value)?; + } let settings = Self { show_bytes: matches.get_flag(options::BYTES), @@ -95,7 +99,7 @@ impl<'a> Settings<'a> { total_when, }; - if settings.number_enabled() > 0 { + Ok(if settings.number_enabled() > 0 { settings } else { Self { @@ -104,7 +108,7 @@ impl<'a> Settings<'a> { debug: settings.debug, ..Default::default() } - } + }) } fn number_enabled(&self) -> u32 { @@ -321,19 +325,34 @@ enum TotalWhen { Never, } -impl> From for TotalWhen { - fn from(s: T) -> Self { - match s.as_ref() { - "auto" => Self::Auto, - "always" => Self::Always, - "only" => Self::Only, - "never" => Self::Never, - _ => unreachable!("Should have been caught by clap"), +/// The values `--total` accepts, in the order GNU lists them. +const TOTAL_CHOICES: &[(&str, TotalWhen)] = &[ + ("auto", TotalWhen::Auto), + ("always", TotalWhen::Always), + ("only", TotalWhen::Only), + ("never", TotalWhen::Never), +]; + +impl TotalWhen { + /// The choice `value` names, accepting any unambiguous abbreviation the + /// way GNU does: `--total=o` is `only`, while `--total=a` names both + /// `auto` and `always` and so names neither. An empty value abbreviates + /// all four, which GNU reports as ambiguous rather than invalid. + fn parse(value: &str) -> Result { + let mut named = TOTAL_CHOICES.iter().filter(|(c, _)| c.starts_with(value)); + match (named.next(), named.next()) { + // No choice abbreviates another, so a single match is the answer + // whether or not it is the whole word. + (Some((_, when)), None) if !value.is_empty() => Ok(*when), + (Some(_), Some(_)) => Err(WcError::AmbiguousTotalArgument { + arg: value.to_string(), + }), + _ => Err(WcError::InvalidTotalArgument { + arg: value.to_string(), + }), } } -} -impl TotalWhen { fn is_total_row_visible(self, num_inputs: usize) -> bool { match self { Self::Auto => num_inputs > 1, @@ -353,6 +372,10 @@ enum WcError { ZeroLengthFileName, #[error("{}", translate!("wc-error-zero-length-filename-ctx", "path" => path, "idx" => idx))] ZeroLengthFileNameCtx { path: Cow<'static, str>, idx: usize }, + #[error("{}", translate!("wc-error-invalid-total-argument", "arg" => arg.clone()))] + InvalidTotalArgument { arg: String }, + #[error("{}", translate!("wc-error-ambiguous-total-argument", "arg" => arg.clone()))] + AmbiguousTotalArgument { arg: String }, } impl WcError { @@ -384,7 +407,7 @@ impl UError for WcError { pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = uucore::clap_localization::handle_clap_result(uu_app(), args)?; - let settings = Settings::new(&matches); + let settings = Settings::new(&matches)?; let inputs = Inputs::new(&matches)?; wc(&inputs, &settings) @@ -437,11 +460,10 @@ pub fn uu_app() -> Command { .arg( Arg::new(options::TOTAL) .long(options::TOTAL) - .value_parser(ShortcutValueParser::new([ - "auto", "always", "only", "never", - ])) .value_name("WHEN") - .hide_possible_values(true) + // Appended rather than overwritten so that a value overridden + // by a later `--total` is still there to be checked. + .action(ArgAction::Append) .help(translate!("wc-help-total")), ) .arg( diff --git a/tests/by-util/test_wc.rs b/tests/by-util/test_wc.rs index 00a542fe0d9..382043cbb7f 100644 --- a/tests/by-util/test_wc.rs +++ b/tests/by-util/test_wc.rs @@ -639,6 +639,73 @@ fn test_total_never() { )); } +/// GNU names the option that was wrong and lists what it takes. +#[test] +fn test_total_invalid_argument() { + const VALID: &str = concat!( + "Valid arguments are:\n", + " - 'auto'\n", + " - 'always'\n", + " - 'only'\n", + " - 'never'\n", + "Try 'wc --help' for more information.\n", + ); + + new_ucmd!() + .args(&["lorem_ipsum.txt", "--total=bogus"]) + .fails_with_code(1) + .stderr_is(format!( + "wc: invalid argument 'bogus' for '--total'\n{VALID}" + )); +} + +/// A value that abbreviates more than one choice is ambiguous, not invalid, +/// and an empty one abbreviates all four. +#[test] +fn test_total_ambiguous_argument() { + for arg in ["--total=a", "--total="] { + let value = arg.strip_prefix("--total=").unwrap(); + new_ucmd!() + .args(&["lorem_ipsum.txt", arg]) + .fails_with_code(1) + .stderr_contains(format!("wc: ambiguous argument '{value}' for '--total'")); + } +} + +/// The last `--total` decides, but an earlier bad one is still an error: +/// GNU rejects it rather than letting the override excuse it. +#[test] +fn test_total_checks_every_occurrence() { + new_ucmd!() + .args(&["lorem_ipsum.txt", "--total=bogus", "--total=only"]) + .fails_with_code(1) + .stderr_contains("wc: invalid argument 'bogus' for '--total'"); + + new_ucmd!() + .args(&["lorem_ipsum.txt", "--total=only", "--total=never"]) + .succeeds() + .stdout_is(" 13 109 772 lorem_ipsum.txt\n"); +} + +/// Any unambiguous abbreviation still resolves, which is what GNU accepts. +#[test] +fn test_total_unambiguous_abbreviations() { + for (arg, expected) in [ + ("--total=au", " 13 109 772 lorem_ipsum.txt\n"), + ( + "--total=al", + " 13 109 772 lorem_ipsum.txt\n 13 109 772 total\n", + ), + ("--total=o", "13 109 772\n"), + ("--total=onl", "13 109 772\n"), + ] { + new_ucmd!() + .args(&["lorem_ipsum.txt", arg]) + .succeeds() + .stdout_is(expected); + } +} + #[test] fn test_total_only() { new_ucmd!()