From 059cb31e5b327d2803ac1a8928d4e093786e260a Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 15 Aug 2026 21:12:00 +0200 Subject: [PATCH] split: point the caret at the failing part of a SIZE --- docs/src/extensions-errors.md | 5 +- src/uu/split/src/split.rs | 26 ++++++++-- src/uu/split/src/strategy.rs | 94 +++++++++++++++++++++++++++++------ tests/by-util/test_split.rs | 65 ++++++++++++++++++++++++ 4 files changed, 168 insertions(+), 22 deletions(-) diff --git a/docs/src/extensions-errors.md b/docs/src/extensions-errors.md index 3878378b6da..49ea57245fc 100644 --- a/docs/src/extensions-errors.md +++ b/docs/src/extensions-errors.md @@ -281,6 +281,7 @@ the difference: | `printf` | the failing conversion or escape in the format string | [`printf %5.2c q`](https://uutils.org/playground/?cmd=printf+%255.2c+q) | | `env` | the failing part of a `-S`/`--split-string` string | [`env -S 'echo ${1FOO}'`](https://uutils.org/playground/?cmd=env+-S+%27echo+%24%7B1FOO%7D%27) | | `cut` | the failing range in the list given to `-b`, `-c`, `-f` or `-F` | [`cut -f 1,4-2 fruits.txt`](https://uutils.org/playground/?cmd=cut+-f+1%2C4-2+fruits.txt) | +| `split` | the failing part of the SIZE given to `-b`, `-C` or `-l` | [`split -b 7zq fruits.txt`](https://uutils.org/playground/?cmd=split+-b+7zq+fruits.txt) | | `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) | @@ -331,8 +332,8 @@ repeated per utility. Three parsers work this way: - **Range lists** (`uucore::ranges`), for `cut`'s `-b`, `-c` and `-f` and for `numfmt --field`. `Range::from_list` reports which item of the list failed and where it sat. -- **Sizes** (`uucore::parser::parse_size`), for `head`, `tail` and `truncate` - today, and available to the other nine callers of the parser. +- **Sizes** (`uucore::parser::parse_size`), for `head`, `tail`, `truncate` and + `split` 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/split/src/split.rs b/src/uu/split/src/split.rs index 227508b7810..7b154971b7f 100644 --- a/src/uu/split/src/split.rs +++ b/src/uu/split/src/split.rs @@ -26,20 +26,29 @@ use std::io::{BufRead, BufReader, ErrorKind, Read, Seek, SeekFrom, Write, stdin} use std::path::Path; use thiserror::Error; use uucore::display::Quotable; -use uucore::error::{FromIo, UResult, USimpleError, UUsageError, set_exit_code, strip_errno}; +use uucore::error::{ + FromIo, UResult, USimpleError, UUsageError, quiet_if_reported, set_exit_code, strip_errno, +}; use uucore::parser::parse_size::parse_size_u64; use uucore::translate; #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let (args, obs_lines) = handle_obsolete(args); + let raw_args: Vec = args.collect(); + // Capture before the obsolete `-22` spelling is rewritten to `-l 22`. + let diag_args = uucore::diagnostics::capture(&raw_args); + let (args, obs_lines) = handle_obsolete(raw_args.into_iter()); let matches = uucore::clap_localization::handle_clap_result(uu_app(), args)?; let settings = Settings::from(&matches, obs_lines.as_deref()).map_err(|e| { + let message = format!("{e}"); if e.requires_usage() { - UUsageError::new(1, format!("{e}")) + UUsageError::new(1, message) } else { - USimpleError::new(1, format!("{e}")) + let reported = diag_args + .as_deref() + .is_some_and(|args| e.render(args, &message)); + quiet_if_reported(reported, USimpleError::new(1, message)) } })?; @@ -271,6 +280,15 @@ enum SettingsError { } impl SettingsError { + /// Draw a caret under the part of the argument that is at fault, when this + /// error is one that knows where it came from. + fn render(&self, diag_args: &[OsString], message: &str) -> bool { + match self { + Self::Strategy(error) => error.render(diag_args, message), + _ => false, + } + } + /// Whether the error demands a usage message. fn requires_usage(&self) -> bool { matches!( diff --git a/src/uu/split/src/strategy.rs b/src/uu/split/src/strategy.rs index e3d6093cc61..01ad2f5aaaf 100644 --- a/src/uu/split/src/strategy.rs +++ b/src/uu/split/src/strategy.rs @@ -7,6 +7,7 @@ use crate::cli::options; use clap::{ArgMatches, parser::ValueSource}; +use std::ffi::OsString; use thiserror::Error; use uucore::{ display::Quotable, @@ -201,16 +202,28 @@ pub enum Strategy { Number(NumberType), } +/// The option a failing SIZE was given to, and the value as typed. +/// +/// Kept next to the error so that a caret knows which argument to point at; +/// `None` for a size that did not come from an option, such as the obsolete +/// `split -22` spelling. +#[derive(Debug)] +pub struct SizeOrigin { + value: String, + short: char, + long: &'static str, +} + /// An error when parsing a chunking strategy from command-line arguments. #[derive(Debug, Error)] pub enum StrategyError { /// Invalid number of lines. #[error("{}", translate!("split-error-invalid-number-of-lines", "error" => .0))] - Lines(ParseSizeError), + Lines(ParseSizeError, Option), /// Invalid number of bytes. #[error("{}", translate!("split-error-invalid-number-of-bytes", "error" => .0))] - Bytes(ParseSizeError), + Bytes(ParseSizeError, Option), /// Invalid number type. #[error("{0}")] @@ -221,21 +234,60 @@ pub enum StrategyError { MultipleWays, } +impl StrategyError { + /// Draw a caret under the part of the SIZE 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 SIZE given to an option, or when + /// nothing could be drawn; the caller then falls back to the plain + /// one-line message. + pub fn render(&self, diag_args: &[OsString], message: &str) -> bool { + let (Self::Lines(error, origin) | Self::Bytes(error, origin)) = self else { + return false; + }; + let Some(origin) = origin else { + return false; + }; + error.render_size_value( + diag_args, + &origin.value, + 0, + Some(origin.short), + Some(origin.long), + message, + ) + } +} + impl Strategy { /// Parse a strategy from the command-line arguments. pub fn from(matches: &ArgMatches, obs_lines: Option<&str>) -> Result { fn get_and_parse( matches: &ArgMatches, - option: &str, + option: &'static str, + short: char, strategy: fn(u64) -> Strategy, - error: fn(ParseSizeError) -> StrategyError, + error: fn(ParseSizeError, Option) -> StrategyError, ) -> Result { let s = matches.get_one::(option).unwrap(); - let n = parse_size_u64_max(s).map_err(error)?; + let origin = || { + Some(SizeOrigin { + value: s.clone(), + short, + long: option, + }) + }; + let n = parse_size_u64_max(s).map_err(|e| error(e, origin()))?; if n > 0 { Ok(strategy(n)) } else { - Err(error(ParseSizeError::ParseFailure(s.to_owned()))) + Err(error(ParseSizeError::ParseFailure(s.to_owned()), origin())) } } // Check that the user is not specifying more than one strategy. @@ -251,26 +303,36 @@ impl Strategy { ) { (Some(v), false, false, false, false) => { let v = parse_size_u64_max(v).map_err(|_| { - StrategyError::Lines(ParseSizeError::ParseFailure(v.to_string())) + StrategyError::Lines(ParseSizeError::ParseFailure(v.to_string()), None) })?; if v > 0 { Ok(Self::Lines(v)) } else { - Err(StrategyError::Lines(ParseSizeError::ParseFailure( - v.to_string(), - ))) + Err(StrategyError::Lines( + ParseSizeError::ParseFailure(v.to_string()), + None, + )) } } (None, false, false, false, false) => Ok(Self::Lines(1000)), - (None, true, false, false, false) => { - get_and_parse(matches, options::LINES, Self::Lines, StrategyError::Lines) - } - (None, false, true, false, false) => { - get_and_parse(matches, options::BYTES, Self::Bytes, StrategyError::Bytes) - } + (None, true, false, false, false) => get_and_parse( + matches, + options::LINES, + 'l', + Self::Lines, + StrategyError::Lines, + ), + (None, false, true, false, false) => get_and_parse( + matches, + options::BYTES, + 'b', + Self::Bytes, + StrategyError::Bytes, + ), (None, false, false, true, false) => get_and_parse( matches, options::LINE_BYTES, + 'C', Self::LineBytes, StrategyError::Bytes, ), diff --git a/tests/by-util/test_split.rs b/tests/by-util/test_split.rs index bb76dceb31f..f5b98771a7e 100644 --- a/tests/by-util/test_split.rs +++ b/tests/by-util/test_split.rs @@ -2138,3 +2138,68 @@ fn test_write_error_on_full_device() { // split must not have moved on to the next chunk. assert!(!at.file_exists("xab")); } + +#[cfg(all(feature = "feat_diagnostics", not(wasi_runner)))] +mod diagnostics { + use super::*; + + #[cfg(unix)] + #[test] + fn test_snippet_points_at_the_unknown_unit() { + let result = new_ucmd!() + .terminal_sim_stderr() + .args(&["-b", "7zq", "/dev/null"]) + .fails_with_code(1); + + // The number parsed; only the unit did not. + assert_eq!( + result.stderr_as_displayed(), + "\ +split: invalid number of bytes: '7zq' + ╭─[ split:1:11 ] + │ + 1 │ split -b 7zq /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_line_bytes_value() { + let result = new_ucmd!() + .terminal_sim_stderr() + .args(&["--line-bytes=3qq", "/dev/null"]) + .fails_with_code(1); + let stderr = result.stderr_as_displayed(); + + assert!(stderr.contains("split:1:21"), "{stderr}"); + assert!(stderr.contains("not a known unit"), "{stderr}"); + } + + #[cfg(unix)] + #[test] + fn test_snippet_underlines_a_count_with_no_number() { + let result = new_ucmd!() + .terminal_sim_stderr() + .args(&["-l", "qq", "/dev/null"]) + .fails_with_code(1); + let stderr = result.stderr_as_displayed(); + + // Nothing usable was read, so the whole value is underlined and the + // message says the rest. + assert!(stderr.contains("invalid number of lines"), "{stderr}"); + assert!(!stderr.contains("not a known unit"), "{stderr}"); + } + + #[test] + fn test_plain_message_when_stderr_is_a_pipe() { + new_ucmd!() + .args(&["-b", "7zq", "/dev/null"]) + .fails_with_code(1) + .stderr_is("split: invalid number of bytes: '7zq'\n"); + } +}