split: point the caret at the failing part of a SIZE

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.

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<OsString> = 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!(

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<SizeOrigin>),
 
     /// Invalid number of bytes.
     #[error("{}", translate!("split-error-invalid-number-of-bytes", "error" => .0))]
-    Bytes(ParseSizeError),
+    Bytes(ParseSizeError, Option<SizeOrigin>),
 
     /// 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<Self, StrategyError> {
         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<SizeOrigin>) -> StrategyError,
         ) -> Result<Strategy, StrategyError> {
             let s = matches.get_one::<String>(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,
             ),

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");
+    }
+}