From dbc812484fd39b83f44e717e9ec40a301a91ca12 Mon Sep 17 00:00:00 2001 From: arbelonson-source <269032023+arbelonson-source@users.noreply.github.com> Date: Mon, 31 Aug 2026 06:34:31 +0300 Subject: [PATCH] tee: match GNU's error for a bad --output-error value An unrecognized or ambiguous --output-error value produced clap's own generic wording instead of GNU's ('invalid argument ... for --output-error / Valid arguments are: / - 'warn' / ...'). Unlike the other options fixed in this series (#14293, #14303, #14304, #14305, #14306, #14307), --output-error keeps its ShortcutValueParser rather than being resolved by hand: each of its choices carries its own description (clap's 'Possible values:' block in --help), which only a value_parser can supply, and hand-rolling that metadata a second time to drop the parser would only recreate what it already provides. Instead, intercept a rejection specifically on this argument after parsing and re-report it with GNU's wording (checking for an exact match first, since 'warn' and 'exit' are themselves prefixes of 'warn-nopipe' and 'exit-nopipe'); every other clap error still goes through the shared formatter unchanged. AI-assisted-by: Claude Opus 5, via Claude Code --- src/uu/tee/locales/en-US.ftl | 8 +++++ src/uu/tee/locales/fr-FR.ftl | 8 +++++ src/uu/tee/src/tee.rs | 69 ++++++++++++++++++++++++++++++++++-- tests/by-util/test_tee.rs | 34 ++++++++++++++++++ 4 files changed, 116 insertions(+), 3 deletions(-) diff --git a/src/uu/tee/locales/en-US.ftl b/src/uu/tee/locales/en-US.ftl index 47bd49c74a0..5f7aa249b12 100644 --- a/src/uu/tee/locales/en-US.ftl +++ b/src/uu/tee/locales/en-US.ftl @@ -15,6 +15,14 @@ tee-help-output-error-exit-nopipe = exit on write errors to any output that are # Error messages tee-error-stdin = read error: { $error } +tee-error-invalid-output-error-choice = invalid argument '{ $arg }' for '--output-error' + Valid arguments are: + { $choices } + Try 'tee --help' for more information. +tee-error-ambiguous-output-error-choice = ambiguous argument '{ $arg }' for '--output-error' + Valid arguments are: + { $choices } + Try 'tee --help' for more information. # Other messages tee-standard-output = 'standard output' diff --git a/src/uu/tee/locales/fr-FR.ftl b/src/uu/tee/locales/fr-FR.ftl index e86faf9b6df..46b936ca050 100644 --- a/src/uu/tee/locales/fr-FR.ftl +++ b/src/uu/tee/locales/fr-FR.ftl @@ -15,6 +15,14 @@ tee-help-output-error-exit-nopipe = quitter en cas d'erreurs d'écriture vers to # Messages d'erreur tee-error-stdin = erreur de lecture: { $error } +tee-error-invalid-output-error-choice = argument '{ $arg }' invalide pour '--output-error' + Les arguments valides sont : + { $choices } + Essayez 'tee --help' pour plus d'informations. +tee-error-ambiguous-output-error-choice = argument '{ $arg }' ambigu pour '--output-error' + Les arguments valides sont : + { $choices } + Essayez 'tee --help' pour plus d'informations. # Autres messages tee-standard-output = 'sortie standard' diff --git a/src/uu/tee/src/tee.rs b/src/uu/tee/src/tee.rs index 278bc1d3f94..0e628242ff7 100644 --- a/src/uu/tee/src/tee.rs +++ b/src/uu/tee/src/tee.rs @@ -10,7 +10,7 @@ use std::fs::OpenOptions; use std::io::{self, Error, ErrorKind, Write}; use std::path::PathBuf; use uucore::display::Quotable; -use uucore::error::{UResult, strip_errno}; +use uucore::error::{UResult, USimpleError, strip_errno}; use uucore::{show_error, translate}; mod cli; @@ -22,9 +22,72 @@ use uucore::signals::ensure_stdout_not_broken; #[cfg(all(unix, not(target_os = "fuchsia")))] use uucore::signals::{disable_pipe_errors, ignore_interrupts}; +/// The choices `--output-error`'s value accepts, in the order GNU lists them. +const OUTPUT_ERROR_CHOICES: &[&str] = &["warn", "warn-nopipe", "exit", "exit-nopipe"]; + +/// The choice `value` names among `OUTPUT_ERROR_CHOICES`, accepting any +/// unambiguous abbreviation the way GNU does. Checks for an exact match +/// first since e.g. 'warn' is itself a prefix of 'warn-nopipe'. +fn resolve_output_error_choice(value: &str) -> UResult<&'static str> { + let list = || { + OUTPUT_ERROR_CHOICES + .iter() + .map(|name| format!(" - '{name}'")) + .collect::>() + .join("\n") + }; + if !value.is_empty() + && let Some(&exact) = OUTPUT_ERROR_CHOICES.iter().find(|name| **name == value) + { + return Ok(exact); + } + let mut named = OUTPUT_ERROR_CHOICES + .iter() + .filter(|name| name.starts_with(value)); + match (named.next(), named.next()) { + (Some(name), None) if !value.is_empty() => Ok(*name), + (Some(_), Some(_)) => Err(USimpleError::new( + 1, + translate!("tee-error-ambiguous-output-error-choice", "arg" => value.to_string(), "choices" => list()), + )), + _ => Err(USimpleError::new( + 1, + translate!("tee-error-invalid-output-error-choice", "arg" => value.to_string(), "choices" => list()), + )), + } +} + +/// `--output-error`'s value keeps its `ShortcutValueParser` (rather than +/// being resolved by hand like the other options in this series) so that +/// `--help` keeps rendering each choice's own description. Clap's own +/// wording for a value it rejects doesn't match GNU's, though, so a +/// rejection specifically on that argument is intercepted and reported with +/// `resolve_output_error_choice`'s message instead; everything else still +/// goes through the shared formatter. +fn get_matches(args: impl uucore::Args) -> UResult { + match uu_app().try_get_matches_from(args) { + Ok(matches) => Ok(matches), + Err(clap_error) => { + if clap_error.exit_code() == 0 { + return Err(clap_error.into()); + } + if clap_error.kind() == clap::error::ErrorKind::InvalidValue + && clap_error + .get(clap::error::ContextKind::InvalidArg) + .is_some_and(|arg| arg.to_string().contains(options::OUTPUT_ERROR)) + && let Some(value) = clap_error.get(clap::error::ContextKind::InvalidValue) + { + return Err(resolve_output_error_choice(&value.to_string()).unwrap_err()); + } + let formatter = uucore::clap_localization::ErrorFormatter::new(uucore::util_name()); + formatter.print_error_and_exit_with_callback(&clap_error, 1, || {}); + } + } +} + #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let matches = uucore::clap_localization::handle_clap_result(uu_app(), args)?; + let matches = get_matches(args)?; let append = matches.get_flag(options::APPEND); let ignore_interrupts = matches.get_flag(options::IGNORE_INTERRUPTS); @@ -36,7 +99,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { "warn-nopipe" => OutputErrorMode::WarnNoPipe, "exit" => OutputErrorMode::Exit, "exit-nopipe" => OutputErrorMode::ExitNoPipe, - _ => unreachable!("clap excluded it"), + _ => unreachable!("ShortcutValueParser already restricted it"), }) .or_else(|| ignore_pipe_errors.then_some(OutputErrorMode::WarnNoPipe)); diff --git a/tests/by-util/test_tee.rs b/tests/by-util/test_tee.rs index 4de4bc84022..a4a88a7955f 100644 --- a/tests/by-util/test_tee.rs +++ b/tests/by-util/test_tee.rs @@ -727,6 +727,40 @@ fn test_output_error_flag_without_value_defaults_warn_nopipe() { assert!(at.file_exists(file_out)); assert_eq!(at.read(file_out), content); } + +#[test] +fn test_output_error_invalid_arg_message() { + new_ucmd!() + .arg("--output-error=bogus") + .pipe_in("") + .fails_with_code(1) + .stderr_is(concat!( + "tee: invalid argument 'bogus' for '--output-error'\n", + "Valid arguments are:\n", + " - 'warn'\n", + " - 'warn-nopipe'\n", + " - 'exit'\n", + " - 'exit-nopipe'\n", + "Try 'tee --help' for more information.\n", + )); +} + +#[test] +fn test_output_error_ambiguous_arg_message() { + new_ucmd!() + .arg("--output-error=wa") + .pipe_in("") + .fails_with_code(1) + .stderr_is(concat!( + "tee: ambiguous argument 'wa' for '--output-error'\n", + "Valid arguments are:\n", + " - 'warn'\n", + " - 'warn-nopipe'\n", + " - 'exit'\n", + " - 'exit-nopipe'\n", + "Try 'tee --help' for more information.\n", + )); +} // Unix-only: presence-only --output-error should not crash on broken pipe. // Current implementation may exit zero; we only assert the process exits to avoid flakiness. // TODO: When semantics are aligned with GNU warn-nopipe, strengthen assertions here.