From 51b181844b5b45e6ac1390cb596294ddca0ac5ce Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Mon, 17 Aug 2026 21:12:00 +0200 Subject: [PATCH] uucore: keep the usage hint when a caret replaced the message A quieted UUsageError became a bare ExitCode, whose usage() is false, so stdbuf and cut printed "Try '... --help'" on a pipe but not on a terminal. --- src/uu/dd/locales/en-US.ftl | 1 - src/uu/dd/locales/fr-FR.ftl | 1 - src/uu/dd/src/diagnostics.rs | 7 +----- src/uu/dd/src/parseargs.rs | 9 ++++++- src/uucore/src/lib/mods/error.rs | 41 ++++++++++++++++++++++++++++---- tests/by-util/test_cut.rs | 15 +++++++++--- tests/by-util/test_dd.rs | 23 ++++++++++++++---- tests/by-util/test_stdbuf.rs | 15 +++++++++--- 8 files changed, 88 insertions(+), 24 deletions(-) diff --git a/src/uu/dd/locales/en-US.ftl b/src/uu/dd/locales/en-US.ftl index 09728e07c8..e1fbcb219d 100644 --- a/src/uu/dd/locales/en-US.ftl +++ b/src/uu/dd/locales/en-US.ftl @@ -135,7 +135,6 @@ dd-error-multiple-case = Only one of conv=lcase or conv=ucase may be specified dd-error-multiple-block = Only one of conv=block or conv=unblock may be specified dd-error-multiple-excl = Only one ov conv=excl or conv=nocreat may be specified dd-error-invalid-flag = invalid input flag: ‘{ $flag }’ - Try '{ $cmd } --help' for more information. dd-error-conv-flag-no-match = Unrecognized conv=CONV -> { $flag } dd-error-multiplier-parse-failure = invalid number: '{ $input }' dd-error-multiplier-overflow = Multiplier string would overflow on current system -> { $input } diff --git a/src/uu/dd/locales/fr-FR.ftl b/src/uu/dd/locales/fr-FR.ftl index 8b49d9a044..ba58437135 100644 --- a/src/uu/dd/locales/fr-FR.ftl +++ b/src/uu/dd/locales/fr-FR.ftl @@ -135,7 +135,6 @@ dd-error-multiple-case = Seul un seul de conv=lcase ou conv=ucase peut être sp dd-error-multiple-block = Seul un seul de conv=block ou conv=unblock peut être spécifié dd-error-multiple-excl = Seul un seul de conv=excl ou conv=nocreat peut être spécifié dd-error-invalid-flag = indicateur d'entrée invalide : '{ $flag }' - Essayez '{ $cmd } --help' pour plus d'informations. dd-error-conv-flag-no-match = conv=CONV non reconnu -> { $flag } dd-error-multiplier-parse-failure = nombre invalide : ‘{ $input }‘ dd-error-multiplier-overflow = La chaîne de multiplicateur déborderait sur le système actuel -> { $input } diff --git a/src/uu/dd/src/diagnostics.rs b/src/uu/dd/src/diagnostics.rs index cc285b275d..fa498859d4 100644 --- a/src/uu/dd/src/diagnostics.rs +++ b/src/uu/dd/src/diagnostics.rs @@ -89,16 +89,11 @@ fn render(args: &[OsString], operand: &str, error: &ParseError) -> bool { let Some(index) = snapshot.index_of(OsStr::new(operand)) else { return false; }; - // Some messages end with a "Try --help" hint of their own; the report - // closes with advice about the very syntax that failed, so only the - // headline is kept. - let message = error.to_string(); - let headline = message.lines().next().unwrap_or_default(); snapshot.render_inside_at( index, operand, span, - headline, + &error.to_string(), None, Some(&translate!(help)), ) diff --git a/src/uu/dd/src/parseargs.rs b/src/uu/dd/src/parseargs.rs index 52ea6109c2..cf123f250c 100644 --- a/src/uu/dd/src/parseargs.rs +++ b/src/uu/dd/src/parseargs.rs @@ -30,7 +30,7 @@ pub enum ParseError { MultipleBlockUnblock, #[error("{}", translate!("dd-error-multiple-excl"))] MultipleExclNoCreate, - #[error("{}", translate!("dd-error-invalid-flag", "flag" => .0.clone(), "cmd" => uucore::execution_phrase()))] + #[error("{}", translate!("dd-error-invalid-flag", "flag" => .0.clone()))] FlagNoMatch(String), #[error("{}", translate!("dd-error-conv-flag-no-match", "flag" => .0.clone()))] ConvFlagNoMatch(String), @@ -484,6 +484,13 @@ impl UError for ParseError { fn code(&self) -> i32 { 1 } + + /// The one message that ends on a hint about the syntax it rejected. The + /// hint is left to this, rather than written into the message, so that it + /// survives a caret report replacing the message. + fn usage(&self) -> bool { + matches!(self, Self::FlagNoMatch(_)) + } } fn show_zero_multiplier_warning() { diff --git a/src/uucore/src/lib/mods/error.rs b/src/uucore/src/lib/mods/error.rs index 9ff6f57e76..0ef0509b9f 100644 --- a/src/uucore/src/lib/mods/error.rs +++ b/src/uucore/src/lib/mods/error.rs @@ -694,7 +694,8 @@ impl ExitCode { /// # Returns /// /// A bare [`ExitCode`] carrying `error`'s code when `reported`, and `error` -/// itself otherwise. +/// itself otherwise. An error that asks for a usage hint still gets one: the +/// hint is not part of the message the report replaced. /// /// # Examples /// @@ -707,11 +708,13 @@ impl ExitCode { /// ``` pub fn quiet_if_reported>>(reported: bool, error: E) -> Box { let error = error.into(); - if reported { - ExitCode::new(error.code()) - } else { - error + if !reported { + return error; } + if error.usage() { + return UUsageError::new(error.code(), String::new()); + } + ExitCode::new(error.code()) } impl Error for ExitCode {} @@ -833,6 +836,34 @@ impl Display for ClapErrorWrapper { #[cfg(test)] mod tests { + use super::{USimpleError, UUsageError, quiet_if_reported}; + + /// A quieted error keeps its code but says nothing: the report already did. + #[test] + fn a_reported_error_carries_only_its_code() { + let error = quiet_if_reported(true, USimpleError::new(3, "bad size".to_string())); + assert_eq!(error.code(), 3); + assert_eq!(error.to_string(), ""); + assert!(!error.usage()); + } + + /// Quieting a usage error must not swallow its "Try --help" hint, or the + /// output would depend on whether a caret happened to be drawn. + #[test] + fn a_reported_usage_error_still_asks_for_the_hint() { + let error = quiet_if_reported(true, UUsageError::new(125, "bad mode".to_string())); + assert_eq!(error.code(), 125); + assert_eq!(error.to_string(), ""); + assert!(error.usage()); + } + + #[test] + fn an_unreported_error_is_left_alone() { + let error = quiet_if_reported(false, UUsageError::new(125, "bad mode".to_string())); + assert_eq!(error.to_string(), "bad mode"); + assert!(error.usage()); + } + #[test] #[cfg(unix)] fn test_nix_error_conversion() { diff --git a/tests/by-util/test_cut.rs b/tests/by-util/test_cut.rs index be7a85a242..6345a74429 100644 --- a/tests/by-util/test_cut.rs +++ b/tests/by-util/test_cut.rs @@ -1228,9 +1228,10 @@ mod diagnostics { .fails_with_code(1); // One item of the list is at fault, not the whole of it. - assert_eq!( - result.stderr_as_displayed(), - "\ + let stderr = result.stderr_as_displayed(); + assert!( + stderr.starts_with( + "\ cut: invalid decreasing range ╭─[ cut:1:10 ] │ @@ -1240,6 +1241,14 @@ cut: invalid decreasing range │ │ Help: a list is N, N-M, N- or -M, separated by commas, as in -f1,4-6,9- ───╯" + ), + "{stderr}" + ); + // The caret replaces the message, not the usage hint: a pipe and a + // terminal must not disagree on whether one was printed. + assert!( + stderr.ends_with("cut --help' for more information."), + "{stderr}" ); } diff --git a/tests/by-util/test_dd.rs b/tests/by-util/test_dd.rs index 138f57aaf9..2ab7654af2 100644 --- a/tests/by-util/test_dd.rs +++ b/tests/by-util/test_dd.rs @@ -2335,7 +2335,7 @@ dd: Unrecognized operand 'bsx=1' #[cfg(unix)] #[test] - fn test_snippet_drops_the_try_help_hint_of_a_flag_message() { + fn test_snippet_keeps_the_try_help_hint_of_a_flag_message() { let result = new_ucmd!() .terminal_sim_stderr() .args(&["iflag=nope"]) @@ -2343,10 +2343,25 @@ dd: Unrecognized operand 'bsx=1' .fails_with_code(1); let stderr = result.stderr_as_displayed(); - // The report ends with advice of its own, so the hint would be noise - // in the middle of it. - assert!(!stderr.contains("--help"), "{stderr}"); assert!(stderr.contains("dd:1:10"), "{stderr}"); + // The caret replaces the message, not the usage hint: a pipe and a + // terminal must not disagree on whether one was printed. + assert!( + stderr + .trim_end() + .ends_with("dd --help' for more information."), + "{stderr}" + ); + } + + #[test] + fn test_plain_message_keeps_the_try_help_hint_of_a_flag_message() { + new_ucmd!() + .args(&["iflag=nope"]) + .pipe_in("") + .fails_with_code(1) + .stderr_contains("dd: invalid input flag: \u{2018}nope\u{2019}") + .stderr_contains("--help' for more information."); } #[test] diff --git a/tests/by-util/test_stdbuf.rs b/tests/by-util/test_stdbuf.rs index beedb00ddd..fe398f84ad 100644 --- a/tests/by-util/test_stdbuf.rs +++ b/tests/by-util/test_stdbuf.rs @@ -447,9 +447,10 @@ mod diagnostics { .fails_with_code(125); // The number parsed; only the unit did not. - assert_eq!( - result.stderr_as_displayed(), - "\ + let stderr = result.stderr_as_displayed(); + assert!( + stderr.starts_with( + "\ stdbuf: invalid mode '6pq' ╭─[ stdbuf:1:12 ] │ @@ -459,6 +460,14 @@ stdbuf: invalid mode '6pq' │ │ Help: a size is a number and an optional unit: K, M, G and so on for 1024, KB, MB, GB for 1000 ───╯" + ), + "{stderr}" + ); + // The caret replaces the message, not the usage hint: a pipe and a + // terminal must not disagree on whether one was printed. + assert!( + stderr.ends_with("stdbuf --help' for more information."), + "{stderr}" ); }