diff --git a/docs/src/extensions-errors.md b/docs/src/extensions-errors.md index b6b4b5c9a7..fb0a2f8ca3 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) | | `seq` | the failing conversion in the format given to `-f`/`--format` | [`seq -f %5.2c 1 3`](https://uutils.org/playground/?cmd=seq+-f+%255.2c+1+3) | | `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) | +| `dd` | the failing key, value or flag of a `KEY=VALUE` operand | [`dd conv=ucase,zap`](https://uutils.org/playground/?cmd=dd+conv%3Ducase%2Czap) | | `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) | diff --git a/src/uu/dd/locales/en-US.ftl b/src/uu/dd/locales/en-US.ftl index 02962654c4..09728e07c8 100644 --- a/src/uu/dd/locales/en-US.ftl +++ b/src/uu/dd/locales/en-US.ftl @@ -160,3 +160,9 @@ dd-progress-bytes-copied-si-iec = { $bytes } bytes ({ $si }, { $iec }) copied, { # Warnings dd-warning-zero-multiplier = { $zero } is a zero multiplier; use { $alternative } if that is intended dd-warning-signal-handler = Internal dd Warning: Unable to register signal handler + +# Diagnostics +dd-diag-help-operand = an operand is KEY=VALUE, as in if=file bs=4k count=10 +dd-diag-help-flags = conv=, iflag= and oflag= take flags separated by commas, as in conv=ucase,sync +dd-diag-help-status = status= is one of none, noxfer or progress +dd-diag-help-number = a number may be followed by a multiplier: c, w, b, then K, M, G and so on for 1024, kB, MB, GB for 1000 diff --git a/src/uu/dd/locales/fr-FR.ftl b/src/uu/dd/locales/fr-FR.ftl index 952acdffae..8b49d9a044 100644 --- a/src/uu/dd/locales/fr-FR.ftl +++ b/src/uu/dd/locales/fr-FR.ftl @@ -160,3 +160,9 @@ dd-progress-bytes-copied-si-iec = { $bytes } octets ({ $si }, { $iec }) copiés, # Warnings dd-warning-zero-multiplier = { $zero } est un multiplicateur zéro ; utilisez { $alternative } si c'est voulu dd-warning-signal-handler = Avertissement dd interne : Impossible d'enregistrer le gestionnaire de signal + +# Diagnostics +dd-diag-help-operand = un opérande s'écrit CLÉ=VALEUR, comme dans if=fichier bs=4k count=10 +dd-diag-help-flags = conv=, iflag= et oflag= acceptent des indicateurs séparés par des virgules, comme dans conv=ucase,sync +dd-diag-help-status = status= vaut none, noxfer ou progress +dd-diag-help-number = un nombre peut être suivi d'un multiplicateur : c, w, b, puis K, M, G et ainsi de suite pour 1024, kB, MB, GB pour 1000 diff --git a/src/uu/dd/src/dd.rs b/src/uu/dd/src/dd.rs index 1ba8a1b618..e462bf1bae 100644 --- a/src/uu/dd/src/dd.rs +++ b/src/uu/dd/src/dd.rs @@ -9,6 +9,7 @@ mod blocks; mod bufferedoutput; mod conversion_tables; mod datastructures; +mod diagnostics; mod numbers; mod parseargs; mod progress; @@ -1493,12 +1494,16 @@ fn is_fifo(filename: &str) -> bool { #[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 operand diagnostics, which echoes the command line. + let diag_args = uucore::diagnostics::capture(&raw_args); + let matches = uucore::clap_localization::handle_clap_result(uu_app(), raw_args)?; - let settings: Settings = Parser::new().parse( + let settings: Settings = Parser::new().parse_with_diagnostics( matches .get_many::(options::OPERANDS) .unwrap_or_default(), + diag_args.as_deref(), )?; #[cfg(unix)] diff --git a/src/uu/dd/src/diagnostics.rs b/src/uu/dd/src/diagnostics.rs new file mode 100644 index 0000000000..cc285b275d --- /dev/null +++ b/src/uu/dd/src/diagnostics.rs @@ -0,0 +1,105 @@ +// This file is part of the uutils coreutils package. +// +// For the full copyright and license information, please view the LICENSE +// file that was distributed with this source code. + +// spell-checker:ignore parseargs + +//! Maps a [`ParseError`] onto the part of the operand it came from, so that +//! [`uucore::diagnostics`] can render it with a caret. +//! +//! Every dd operand is a `KEY=VALUE` pair, so an error is about the key, the +//! value, or one flag inside a comma-separated value; the caret says which. + +use std::ffi::{OsStr, OsString}; +use std::ops::Range; + +use uucore::diagnostics::Snapshot; +use uucore::error::{UError, quiet_if_reported}; +use uucore::translate; + +use crate::parseargs::ParseError; + +/// The error to raise for an operand dd rejected. +/// +/// Draws the caret when the arguments as typed were kept, and quiets `error` +/// when it did: the report has already said everything the one-line message +/// would, and the exit code is all that is left to carry. +/// +/// # Arguments +/// +/// * `diag_args` - The arguments as typed, program name included, or `None` +/// when they were not kept. +/// * `operand` - The `KEY=VALUE` operand at fault, as typed. +/// * `error` - What the parser made of it. +pub fn operand_error( + diag_args: Option<&[OsString]>, + operand: &str, + error: ParseError, +) -> Box { + let reported = diag_args.is_some_and(|args| render(args, operand, &error)); + quiet_if_reported(reported, error) +} + +/// Render `error` against `args`, with a caret under the part of `operand` +/// that is at fault. +/// +/// # Returns +/// +/// `false` when the error is not about a part of the operand, or when the +/// operand cannot be found among the arguments. +fn render(args: &[OsString], operand: &str, error: &ParseError) -> bool { + let key_end = operand.find('=').unwrap_or(operand.len()); + // The value starts past the `=`, or ends the operand when there is none. + let value_start = operand.len().min(key_end + 1); + let value = || value_start..operand.len(); + // A flag inside a comma-separated value. The list is walked the way the + // parser walks it rather than searched for the flag's text, which would + // match inside an earlier flag the failing one is a prefix of — the `noc` + // of `nocache,noc`. + let flag = |flag: &str| { + let mut at = value_start; + for part in operand[value_start..].split(',') { + if part == flag { + return Some(at..at + part.len()); + } + // Every separator is one byte wide. + at += part.len() + 1; + } + None + }; + + let (span, help): (Range, &str) = match error { + ParseError::UnrecognizedOperand(_) => (0..key_end, "dd-diag-help-operand"), + ParseError::FlagNoMatch(name) | ParseError::ConvFlagNoMatch(name) => { + (flag(name).unwrap_or_else(value), "dd-diag-help-flags") + } + ParseError::StatusLevelNotRecognized(_) => (value(), "dd-diag-help-status"), + ParseError::MultiplierStringParseFailure(_) + | ParseError::MultiplierStringOverflow(_) + | ParseError::InvalidNumber(_) + | ParseError::InvalidNumberWithErrMsg(_, _) + | ParseError::BsOutOfRange(_) => (value(), "dd-diag-help-number"), + // The rest is about how operands combine rather than about one of + // them, so there is nothing to point a caret at. + _ => return false, + }; + + let snapshot = Snapshot::with_program(args); + let Some(index) = snapshot.index_of(OsStr::new(operand)) else { + return false; + }; + // Some messages end with a "Try --help" hint of their own; the report + // closes with advice about the very syntax that failed, so only the + // headline is kept. + let message = error.to_string(); + let headline = message.lines().next().unwrap_or_default(); + snapshot.render_inside_at( + index, + operand, + span, + headline, + None, + Some(&translate!(help)), + ) +} diff --git a/src/uu/dd/src/parseargs.rs b/src/uu/dd/src/parseargs.rs index a2b710404d..52ea6109c2 100644 --- a/src/uu/dd/src/parseargs.rs +++ b/src/uu/dd/src/parseargs.rs @@ -9,6 +9,7 @@ mod unit_tests; use super::{ConversionMode, IConvFlags, IFlags, Num, OConvFlags, OFlags, Settings, StatusLevel}; use crate::conversion_tables::ConversionTable; +use std::ffi::OsString; use thiserror::Error; use uucore::display::Quotable; use uucore::error::UError; @@ -129,19 +130,55 @@ impl Parser { Self::default() } + /// Parse the operands, keeping only the error itself. + /// + /// The utility itself goes through [`Self::parse_with_diagnostics`], which + /// also knows which operand failed; this is the plain form the unit tests + /// compare against. + #[cfg(test)] pub(crate) fn parse( self, operands: impl IntoIterator>, ) -> Result { - self.read(operands)?.validate() + self.read(operands).map_err(|(_, error)| error)?.validate() } + /// Parse the operands, pointing a caret at the one that is at fault. + /// + /// # Arguments + /// + /// * `operands` - The operands as typed. + /// * `diag_args` - The whole argument list, program name included, or + /// `None` when it was not kept. + pub(crate) fn parse_with_diagnostics( + self, + operands: impl IntoIterator>, + diag_args: Option<&[OsString]>, + ) -> Result> { + match self.read(operands) { + Err((operand, error)) => Err(crate::diagnostics::operand_error( + diag_args, &operand, error, + )), + // A validation error is about how the operands combine rather than + // about any one of them, so it keeps its plain message. + Ok(parser) => parser.validate().map_err(Into::into), + } + } + + /// Read the operands into the parser state. + /// + /// # Returns + /// + /// The operand at fault along with the error, so that a caller with the + /// command line at hand can point a caret inside it. pub(crate) fn read( mut self, operands: impl IntoIterator>, - ) -> Result { + ) -> Result { for operand in operands { - self.parse_operand(operand.as_ref())?; + let operand = operand.as_ref(); + self.parse_operand(operand) + .map_err(|error| (operand.to_string(), error))?; } Ok(self) diff --git a/tests/by-util/test_dd.rs b/tests/by-util/test_dd.rs index 0967e2fb0c..138f57aaf9 100644 --- a/tests/by-util/test_dd.rs +++ b/tests/by-util/test_dd.rs @@ -2255,3 +2255,106 @@ fn test_stats_are_reported_when_a_write_fails() { result.stderr_contains("786432 bytes"); assert_eq!(at.metadata("capped.bin").len(), CAP); } + +#[cfg(all(feature = "feat_diagnostics", not(wasi_runner)))] +mod diagnostics { + use super::*; + + #[cfg(unix)] + #[test] + fn test_snippet_points_at_the_unrecognized_key() { + let result = new_ucmd!() + .terminal_sim_stderr() + .args(&["bsx=1"]) + .pipe_in("") + .fails_with_code(1); + + // The caret takes the key alone: the value is fine, it is the operand + // name that dd does not know. + assert_eq!( + result.stderr_as_displayed(), + "\ +dd: Unrecognized operand 'bsx=1' + ╭─[ dd:1:4 ] + │ + 1 │ dd bsx=1 + │ ─── + │ + │ Help: an operand is KEY=VALUE, as in if=file bs=4k count=10 +───╯" + ); + } + + #[cfg(unix)] + #[test] + fn test_snippet_points_at_the_failing_flag_of_a_list() { + let result = new_ucmd!() + .terminal_sim_stderr() + .args(&["conv=ucase,zap"]) + .pipe_in("") + .fails_with_code(1); + let stderr = result.stderr_as_displayed(); + + // Only the second flag is wrong, and the caret says so. + assert!(stderr.contains("dd:1:15"), "{stderr}"); + assert!(stderr.contains("1 │ dd conv=ucase,zap"), "{stderr}"); + } + + #[cfg(unix)] + #[test] + fn test_snippet_points_at_the_failing_flag_and_not_the_one_it_starts() { + let result = new_ucmd!() + .terminal_sim_stderr() + .args(&["conv=notrunc,not"]) + .pipe_in("") + .fails_with_code(1); + let stderr = result.stderr_as_displayed(); + + // `not` also opens the `notrunc` in front of it, and the caret belongs + // on the flag that failed rather than on the one that parsed. + assert!(stderr.contains("dd:1:17"), "{stderr}"); + assert!( + stderr.contains("1 \u{2502} dd conv=notrunc,not"), + "{stderr}" + ); + } + + #[cfg(unix)] + #[test] + fn test_snippet_points_at_the_value_of_a_count() { + let result = new_ucmd!() + .terminal_sim_stderr() + .args(&["count=8x"]) + .pipe_in("") + .fails_with_code(1); + let stderr = result.stderr_as_displayed(); + + assert!(stderr.contains("dd:1:10"), "{stderr}"); + assert!(stderr.contains("a number may be followed by"), "{stderr}"); + } + + #[cfg(unix)] + #[test] + fn test_snippet_drops_the_try_help_hint_of_a_flag_message() { + let result = new_ucmd!() + .terminal_sim_stderr() + .args(&["iflag=nope"]) + .pipe_in("") + .fails_with_code(1); + let stderr = result.stderr_as_displayed(); + + // The report ends with advice of its own, so the hint would be noise + // in the middle of it. + assert!(!stderr.contains("--help"), "{stderr}"); + assert!(stderr.contains("dd:1:10"), "{stderr}"); + } + + #[test] + fn test_plain_message_when_stderr_is_a_pipe() { + new_ucmd!() + .args(&["bsx=1"]) + .pipe_in("") + .fails_with_code(1) + .stderr_is("dd: Unrecognized operand 'bsx=1'\n"); + } +}