diff --git a/docs/src/extensions-errors.md b/docs/src/extensions-errors.md index 49ea57245fc..f06b6aab8c0 100644 --- a/docs/src/extensions-errors.md +++ b/docs/src/extensions-errors.md @@ -282,6 +282,7 @@ the difference: | `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) | +| `shred` | the failing part of the SIZE given to `-s`/`--size` | [`shred -s 4vv fruits.txt`](https://uutils.org/playground/?cmd=shred+-s+4vv+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) | @@ -332,8 +333,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`, `truncate` and - `split` today, and available to the other callers of the parser. +- **Sizes** (`uucore::parser::parse_size`), for `head`, `tail`, `truncate`, + `split` and `shred` 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/shred/src/shred.rs b/src/uu/shred/src/shred.rs index 25a12e87307..52a9c1662d2 100644 --- a/src/uu/shred/src/shred.rs +++ b/src/uu/shred/src/shred.rs @@ -244,7 +244,10 @@ impl BytesWriter { #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let matches = uucore::clap_localization::handle_clap_result(uu_app(), args)?; + let raw_args: Vec = args.collect(); + // Kept for the caret in size diagnostics, which needs the size as typed. + let diag_args = uucore::diagnostics::capture(&raw_args); + let matches = uucore::clap_localization::handle_clap_result(uu_app(), raw_args)?; if !matches.contains_id(options::FILE) { return Err(UUsageError::new( @@ -290,7 +293,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let size_arg = matches .get_one::(options::SIZE) .map(ToOwned::to_owned); - let size = get_size(size_arg); + let size = get_size(size_arg, diag_args.as_deref())?; let exact = matches.get_flag(options::EXACT) || size.is_some(); let zero = matches.get_flag(options::ZERO); let verbose = matches.get_flag(options::VERBOSE); @@ -399,21 +402,33 @@ pub fn uu_app() -> Command { ) } -fn get_size(size_str_opt: Option) -> Option { - size_str_opt - .as_ref() - .and_then(|size| parse_size_u64(size.as_str()).ok()) - .or_else(|| { - if let Some(size) = size_str_opt { - show_error!( - "{}", - translate!("shred-invalid-file-size", "size" => size.quote()) - ); - // TODO: replace with our error management - std::process::exit(1); - } - None - }) +/// The value of `-s`/`--size` as a number of bytes. +/// +/// # Arguments +/// +/// * `size_str_opt` - The value as typed, or `None` when the option was not +/// given. +/// * `diag_args` - The arguments as typed, for the caret, or `None` when they +/// were not kept. +fn get_size(size_str_opt: Option, diag_args: Option<&[OsString]>) -> UResult> { + let Some(size) = size_str_opt else { + return Ok(None); + }; + match parse_size_u64(&size) { + Ok(bytes) => Ok(Some(bytes)), + Err(error) => { + let message = translate!("shred-invalid-file-size", "size" => size.quote()); + Err(error.size_value_error( + diag_args, + &size, + 0, + 's', + options::SIZE, + &message, + USimpleError::new(1, message.clone()), + )) + } + } } fn pass_name(pass_type: &PassType) -> String { diff --git a/tests/by-util/test_shred.rs b/tests/by-util/test_shred.rs index 2938c324707..d5d4e641e73 100644 --- a/tests/by-util/test_shred.rs +++ b/tests/by-util/test_shred.rs @@ -498,3 +498,65 @@ fn test_shred_inaccessible_file_reports_real_error() { // Restore search permission so the fixture directory can be cleaned up. set_permissions(at.plus_as_string("locked"), Permissions::from_mode(0o755)).unwrap(); } + +#[cfg(all(feature = "feat_diagnostics", not(wasi_runner)))] +mod diagnostics { + use super::*; + + #[cfg(unix)] + #[test] + fn test_snippet_points_at_the_unknown_unit() { + let (at, mut ucmd) = at_and_ucmd!(); + at.touch("wipe_me"); + + let result = ucmd + .terminal_sim_stderr() + .args(&["-s", "4vv", "wipe_me"]) + .fails_with_code(1); + + // The number parsed; only the unit did not. + assert_eq!( + result.stderr_as_displayed(), + "\ +shred: invalid file size: '4vv' + ╭─[ shred:1:11 ] + │ + 1 │ shred -s 4vv wipe_me + │ ─┬ + │ ╰── 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 +───╯" + ); + + // The file must be left alone when the size does not parse. + assert!(at.file_exists("wipe_me")); + } + + #[cfg(unix)] + #[test] + fn test_snippet_underlines_a_size_with_no_number() { + let (at, mut ucmd) = at_and_ucmd!(); + at.touch("wipe_me"); + + let result = ucmd + .terminal_sim_stderr() + .args(&["--size=vv", "wipe_me"]) + .fails_with_code(1); + let stderr = result.stderr_as_displayed(); + + // Nothing usable was read, so the whole value is underlined. + assert!(stderr.contains("shred:1:14"), "{stderr}"); + assert!(!stderr.contains("not a known unit"), "{stderr}"); + } + + #[test] + fn test_plain_message_when_stderr_is_a_pipe() { + let (at, mut ucmd) = at_and_ucmd!(); + at.touch("wipe_me"); + + ucmd.args(&["-s", "4vv", "wipe_me"]) + .fails_with_code(1) + .stderr_is("shred: invalid file size: '4vv'\n"); + } +}