diff --git a/docs/src/extensions-errors.md b/docs/src/extensions-errors.md index a2d11c0d39..5c0f70bcaa 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) | +| `od` | the failing part of the SIZE given to `-j`, `-N`, `-S` or `-w` | [`od -N 3zz fruits.txt`](https://uutils.org/playground/?cmd=od+-N+3zz+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 @@ -335,7 +336,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`, `shred`, `stdbuf` and `sort` today, and available to the other callers of the parser. + `split`, `shred`, `stdbuf`, `sort` and `od` 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/od/src/od.rs b/src/uu/od/src/od.rs index 4398b0089b..1fea9c0feb 100644 --- a/src/uu/od/src/od.rs +++ b/src/uu/od/src/od.rs @@ -40,8 +40,9 @@ use crate::peek_reader::{PeekRead, PeekReader}; use crate::prn_char::format_ascii_dump; use clap::ArgAction; use clap::{Arg, ArgMatches, Command, parser::ValueSource}; +use std::ffi::OsString; use uucore::display::Quotable; -use uucore::error::{UResult, USimpleError}; +use uucore::error::{UError, UResult, USimpleError, quiet_if_reported}; use uucore::translate; use uucore::parser::parse_size::ParseSizeError; @@ -77,6 +78,35 @@ struct OdOptions { string_min_length: Option, } +/// The error to raise for a SIZE that does not parse. +/// +/// Draws a caret under the part of the value at fault when stderr is a +/// terminal, and quiets the message when it did, since the report has already +/// said everything it would. +/// +/// # Arguments +/// +/// * `error` - What the size parser made of the value. +/// * `args` - The whole argument list, program name included. +/// * `value` - The value as typed. +/// * `short` - The short name of the option it was given to, if it has one. +/// * `long` - Its long name. +/// * `message` - The headline, already localized. +fn size_error( + error: &ParseSizeError, + args: &[String], + value: &str, + short: Option, + long: &str, + message: String, +) -> Box { + let reported = uucore::diagnostics::enabled() && { + let diag_args: Vec = args.iter().map(OsString::from).collect(); + error.render_size_value(&diag_args, value, 0, short, Some(long), &message) + }; + quiet_if_reported(reported, USimpleError::new(1, message)) +} + /// Helper function to parse bytes with error handling fn parse_bytes_option( matches: &ArgMatches, @@ -88,10 +118,11 @@ fn parse_bytes_option( None => Ok(None), Some(s) => match parse_number_of_bytes(s) { Ok(n) => Ok(Some(n)), - Err(e) => Err(USimpleError::new( - 1, - format_error_message(&e, s, &option_display_name(args, option_name, short)), - )), + Err(e) => { + let message = + format_error_message(&e, s, &option_display_name(args, option_name, short)); + Err(size_error(&e, args, s, short, option_name, message)) + } }, } } @@ -135,8 +166,10 @@ impl OdOptions { matches.value_source(options::WIDTH), ) { let width_display = option_display_name(args, options::WIDTH, Some('w')); - let parsed = parse_number_of_bytes(s) - .map_err(|e| USimpleError::new(1, format_error_message(&e, s, &width_display)))?; + let parsed = parse_number_of_bytes(s).map_err(|e| { + let message = format_error_message(&e, s, &width_display); + size_error(&e, args, s, Some('w'), options::WIDTH, message) + })?; if parsed == 0 { return Err(USimpleError::new( 1, diff --git a/tests/by-util/test_od.rs b/tests/by-util/test_od.rs index d260e04083..ae6ca8af40 100644 --- a/tests/by-util/test_od.rs +++ b/tests/by-util/test_od.rs @@ -1433,3 +1433,68 @@ fn test_od_strings_with_n_flag() { .success() .stdout_only("0000000 foo\n0000004 bar\n"); } + +#[cfg(all(feature = "feat_diagnostics", not(wasi_runner)))] +mod diagnostics { + use super::*; + + #[cfg(unix)] + #[test] + fn test_snippet_points_at_the_unknown_unit_of_read_bytes() { + let result = new_ucmd!() + .terminal_sim_stderr() + .args(&["-N", "3zz", "/dev/null"]) + .fails_with_code(1); + + // The number parsed; only the unit did not. The headline keeps the + // option spelled the way it was typed. + assert_eq!( + result.stderr_as_displayed(), + "\ +od: invalid suffix in -N argument '3zz' + ╭─[ od:1:8 ] + │ + 1 │ od -N 3zz /dev/null + │ ─┬ + │ ╰── 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 +───╯" + ); + } + + #[cfg(unix)] + #[test] + fn test_snippet_points_inside_a_width_value() { + let result = new_ucmd!() + .terminal_sim_stderr() + .args(&["--width=4qq", "/dev/null"]) + .fails_with_code(1); + let stderr = result.stderr_as_displayed(); + + assert!(stderr.contains("od:1:13"), "{stderr}"); + assert!(stderr.contains("not a known unit"), "{stderr}"); + } + + #[cfg(unix)] + #[test] + fn test_snippet_underlines_a_hexadecimal_offset_that_does_not_parse() { + let result = new_ucmd!() + .terminal_sim_stderr() + .args(&["-j", "0x1zz", "/dev/null"]) + .fails_with_code(1); + let stderr = result.stderr_as_displayed(); + + // Nothing usable was read, so the whole value is underlined. + assert!(stderr.contains("od:1:7"), "{stderr}"); + assert!(!stderr.contains("not a known unit"), "{stderr}"); + } + + #[test] + fn test_plain_message_when_stderr_is_a_pipe() { + new_ucmd!() + .args(&["-N", "3zz", "/dev/null"]) + .fails_with_code(1) + .stderr_is("od: invalid suffix in -N argument '3zz'\n"); + } +}