From 5aea503bddac6c824c077413b4ab3ac2935248b9 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 16 Aug 2026 20:05:00 +0200 Subject: [PATCH] stdbuf: point the caret at the failing part of a buffering mode --- docs/src/extensions-errors.md | 3 +- src/uu/stdbuf/src/stdbuf.rs | 86 +++++++++++++++++++++++++++++------ tests/by-util/test_stdbuf.rs | 50 ++++++++++++++++++++ 3 files changed, 125 insertions(+), 14 deletions(-) diff --git a/docs/src/extensions-errors.md b/docs/src/extensions-errors.md index f06b6aab8c0..385f38b1742 100644 --- a/docs/src/extensions-errors.md +++ b/docs/src/extensions-errors.md @@ -286,6 +286,7 @@ the difference: | `head` | the failing part of the SIZE given to `-c` or `-n` | [`head -c 1fb fruits.txt`](https://uutils.org/playground/?cmd=head+-c+1fb+fruits.txt) | | `tail` | the failing part of the SIZE given to `-c` or `-n` | [`tail -c 1fb fruits.txt`](https://uutils.org/playground/?cmd=tail+-c+1fb+fruits.txt) | | `truncate` | the failing part of the SIZE given to `-s`/`--size` | [`truncate -s 10fb fruits.txt`](https://uutils.org/playground/?cmd=truncate+-s+10fb+fruits.txt) | +| `stdbuf` | the failing part of the buffering mode given to `-i`, `-o` or `-e` | [`stdbuf -o 6pq head`](https://uutils.org/playground/?cmd=stdbuf+-o+6pq+head) | ## How it works @@ -334,7 +335,7 @@ repeated per utility. Three parsers work this way: `numfmt --field`. `Range::from_list` reports which item of the list failed and where it sat. - **Sizes** (`uucore::parser::parse_size`), for `head`, `tail`, `truncate`, - `split` and `shred` today, and available to the other callers of the parser. + `split`, `shred` and `stdbuf` today, and available to the other callers of the parser. `ParseSizeError::span` works out from the operand which of its two parts — the number or the unit — was rejected, so the error type keeps the shape its callers build by hand. diff --git a/src/uu/stdbuf/src/stdbuf.rs b/src/uu/stdbuf/src/stdbuf.rs index ead5c055511..e25b4dae90b 100644 --- a/src/uu/stdbuf/src/stdbuf.rs +++ b/src/uu/stdbuf/src/stdbuf.rs @@ -15,9 +15,9 @@ use tempfile::TempDir; use tempfile::tempdir; use thiserror::Error; use uucore::display::Quotable; -use uucore::error::{UResult, USimpleError, UUsageError, strip_errno}; +use uucore::error::{UResult, USimpleError, UUsageError, quiet_if_reported, strip_errno}; use uucore::format_usage; -use uucore::parser::parse_size::parse_size_u64; +use uucore::parser::parse_size::{ParseSizeError, parse_size_u64}; use uucore::translate; mod options { @@ -64,23 +64,64 @@ impl TryFrom<&ArgMatches> for ProgramOptions { fn try_from(matches: &ArgMatches) -> Result { Ok(Self { - stdin: check_option(matches, options::INPUT)?, - stdout: check_option(matches, options::OUTPUT)?, - stderr: check_option(matches, options::ERROR)?, + stdin: check_option(matches, options::INPUT, options::INPUT_SHORT)?, + stdout: check_option(matches, options::OUTPUT, options::OUTPUT_SHORT)?, + stderr: check_option(matches, options::ERROR, options::ERROR_SHORT)?, }) } } +/// A buffering mode that did not parse as a size, and where it came from. +/// +/// The message is built where it always was; the rest is what a caret needs: +/// the mode as typed, the option it was given to, and what the size parser +/// made of it. +#[derive(Debug)] +struct ModeError { + value: String, + short: char, + long: &'static str, + error: ParseSizeError, +} + #[derive(Debug, Error)] enum ProgramOptionsError { #[error("{}", translate!("stdbuf-error-line-buffering-stdin-meaningless"))] LineBufferingStdinMeaningless, - #[error("{}", translate!("stdbuf-error-invalid-mode", "error" => _0.clone()))] - InvalidMode(String), + #[error("{}", translate!("stdbuf-error-invalid-mode", "error" => _0.error.to_string()))] + InvalidMode(Box), #[error("{}", translate!("stdbuf-error-value-too-large", "value" => _0.clone()))] ValueTooLarge(String), } +impl ProgramOptionsError { + /// Draw a caret under the part of the mode that is at fault. + /// + /// # Arguments + /// + /// * `diag_args` - The arguments as typed, program name included. + /// * `message` - The headline, already localized. + /// + /// # Returns + /// + /// `false` when this error is not about a mode that failed to parse as a + /// size, or when nothing could be drawn; the caller then falls back to the + /// plain one-line message. + fn render(&self, diag_args: &[OsString], message: &str) -> bool { + let Self::InvalidMode(mode) = self else { + return false; + }; + mode.error.render_size_value( + diag_args, + &mode.value, + 0, + Some(mode.short), + Some(mode.long), + message, + ) + } +} + #[cfg(all(unix, not(target_vendor = "apple"), not(target_os = "cygwin")))] fn preload_strings() -> (&'static str, &'static str) { ("LD_PRELOAD", "so") @@ -96,7 +137,11 @@ fn preload_strings() -> (&'static str, &'static str) { ("LD_PRELOAD", "dll") } -fn check_option(matches: &ArgMatches, name: &str) -> Result { +fn check_option( + matches: &ArgMatches, + name: &'static str, + short: char, +) -> Result { match matches.get_one::(name) { Some(value) => match value.as_str() { "L" => { @@ -107,7 +152,14 @@ fn check_option(matches: &ArgMatches, name: &str) -> Result parse_size_u64(x).map_or_else( - |e| Err(ProgramOptionsError::InvalidMode(e.to_string())), + |error| { + Err(ProgramOptionsError::InvalidMode(Box::new(ModeError { + value: x.to_string(), + short, + long: name, + error, + }))) + }, |m| { Ok(BufferType::Size(m.try_into().map_err(|_| { ProgramOptionsError::ValueTooLarge(x.to_string()) @@ -191,11 +243,19 @@ fn get_preload_env(_tmp_dir: &TempDir) -> UResult<(String, PathBuf)> { #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { + let raw_args: Vec = args.collect(); + // Kept for the caret in mode diagnostics, which needs the mode as typed. + let diag_args = uucore::diagnostics::capture(&raw_args); let matches = - uucore::clap_localization::handle_clap_result_with_exit_code(uu_app(), args, 125)?; - - let options = - ProgramOptions::try_from(&matches).map_err(|e| UUsageError::new(125, e.to_string()))?; + uucore::clap_localization::handle_clap_result_with_exit_code(uu_app(), raw_args, 125)?; + + let options = ProgramOptions::try_from(&matches).map_err(|e| { + let message = e.to_string(); + let reported = diag_args + .as_deref() + .is_some_and(|args| e.render(args, &message)); + quiet_if_reported(reported, UUsageError::new(125, message)) + })?; let mut command_values = matches .get_many::(options::COMMAND) diff --git a/tests/by-util/test_stdbuf.rs b/tests/by-util/test_stdbuf.rs index cbfd4e9694c..beedb00dddc 100644 --- a/tests/by-util/test_stdbuf.rs +++ b/tests/by-util/test_stdbuf.rs @@ -433,3 +433,53 @@ fn test_stdbuf_no_fork_regression() { child.kill().ok(); child.wait().ok(); } + +#[cfg(unix)] +#[cfg(all(feature = "feat_diagnostics", not(wasi_runner)))] +mod diagnostics { + use super::*; + + #[test] + fn test_snippet_points_at_the_unknown_unit() { + let result = new_ucmd!() + .terminal_sim_stderr() + .args(&["-o", "6pq", "head"]) + .fails_with_code(125); + + // The number parsed; only the unit did not. + assert_eq!( + result.stderr_as_displayed(), + "\ +stdbuf: invalid mode '6pq' + ╭─[ stdbuf:1:12 ] + │ + 1 │ stdbuf -o 6pq head + │ ─┬ + │ ╰── not a known unit + │ + │ Help: a size is a number and an optional unit: K, M, G and so on for 1024, KB, MB, GB for 1000 +───╯" + ); + } + + #[test] + fn test_snippet_points_inside_a_long_option_value() { + let result = new_ucmd!() + .terminal_sim_stderr() + .args(&["--error=pq", "head"]) + .fails_with_code(125); + let stderr = result.stderr_as_displayed(); + + // Nothing usable was read, so the whole value is underlined. + assert!(stderr.contains("stdbuf:1:16"), "{stderr}"); + assert!(!stderr.contains("not a known unit"), "{stderr}"); + } + + #[test] + fn test_plain_message_when_stderr_is_a_pipe() { + new_ucmd!() + .args(&["-o", "6pq", "head"]) + .fails_with_code(125) + .usage_error("invalid mode '6pq'"); + } +}