diff --git a/docs/src/extensions-errors.md b/docs/src/extensions-errors.md index 3878378b6da..fdfbc94e86e 100644 --- a/docs/src/extensions-errors.md +++ b/docs/src/extensions-errors.md @@ -276,14 +276,22 @@ the difference: | `mknod` | the failing part of the mode given to `-m`/`--mode` | [`mknod -m u+q mydev c 1 3`](https://uutils.org/playground/?cmd=mknod+-m+u%2Bq+mydev+c+1+3) | | `install`| the failing part of the mode given to `-m`/`--mode` | [`install -m u+q fruits.txt dest`](https://uutils.org/playground/?cmd=install+-m+u%2Bq+fruits.txt+dest) | | `tr` | the part of a set that is at fault (bad class, backwards range, bad repeat count, …) | [`tr 'qw[y-b]' x`](https://uutils.org/playground/?cmd=tr+%27qw%5By-b%5D%27+x) | -| `sort` | the failing part of a `-k`/`--key` or field specification | [`sort -k2.3x fruits.txt`](https://uutils.org/playground/?cmd=sort+-k2.3x+fruits.txt) | +| `sort` | the failing part of a `-k`/`--key` or field specification, or of the SIZE given to `-S` | [`sort -k2.3x fruits.txt`](https://uutils.org/playground/?cmd=sort+-k2.3x+fruits.txt) | | `numfmt` | the failing part of a `--field` or `--format` specification | [`numfmt --format=%q 1000`](https://uutils.org/playground/?cmd=numfmt+--format%3D%25q+1000) | | `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) | +| `stat` | the failing directive of a `-c`/`--format` or `--printf` format | [`stat -c %d%.3 fruits.txt`](https://uutils.org/playground/?cmd=stat+-c+%25d%25.3+fruits.txt) | | `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) | +| `join` | the failing field of the output format given to `-o` | [`join -o 1.2,2.x fruits.txt fruits.txt`](https://uutils.org/playground/?cmd=join+-o+1.2%2C2.x+fruits.txt+fruits.txt) | | `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) | +| `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 @@ -331,8 +339,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`, + `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/cut/src/cut.rs b/src/uu/cut/src/cut.rs index e68e177d9c6..45d5cea1ca2 100644 --- a/src/uu/cut/src/cut.rs +++ b/src/uu/cut/src/cut.rs @@ -1097,17 +1097,20 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // The list is the value of the option that selected the mode, so the // caret can be put under the one range that is at fault. let (short, long) = mode_arg_names(mode_arg); - let reported = diag_args.as_ref().is_some_and(|args| { - e.render_option_value( - args, - list, - Some(short), - long, - &translate!("cut-diag-label-zero-bound"), - &translate!("cut-diag-help-list-syntax"), - ) - }); - uucore::error::quiet_if_reported(reported, UUsageError::new(1, e.message)) + uucore::diagnostics::error_after_report( + diag_args.as_deref(), + UUsageError::new(1, e.message.clone()), + |args, _| { + e.render_option_value( + args, + list, + Some(short), + long, + &translate!("cut-diag-label-zero-bound"), + &translate!("cut-diag-help-list-syntax"), + ) + }, + ) })?; let mode = match mode_arg { diff --git a/src/uu/dd/locales/en-US.ftl b/src/uu/dd/locales/en-US.ftl index 02962654c40..e1fbcb219df 100644 --- a/src/uu/dd/locales/en-US.ftl +++ b/src/uu/dd/locales/en-US.ftl @@ -135,7 +135,6 @@ dd-error-multiple-case = Only one of conv=lcase or conv=ucase may be specified dd-error-multiple-block = Only one of conv=block or conv=unblock may be specified dd-error-multiple-excl = Only one ov conv=excl or conv=nocreat may be specified dd-error-invalid-flag = invalid input flag: ‘{ $flag }’ - Try '{ $cmd } --help' for more information. dd-error-conv-flag-no-match = Unrecognized conv=CONV -> { $flag } dd-error-multiplier-parse-failure = invalid number: '{ $input }' dd-error-multiplier-overflow = Multiplier string would overflow on current system -> { $input } @@ -160,3 +159,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 952acdffae1..ba584371354 100644 --- a/src/uu/dd/locales/fr-FR.ftl +++ b/src/uu/dd/locales/fr-FR.ftl @@ -135,7 +135,6 @@ dd-error-multiple-case = Seul un seul de conv=lcase ou conv=ucase peut être sp dd-error-multiple-block = Seul un seul de conv=block ou conv=unblock peut être spécifié dd-error-multiple-excl = Seul un seul de conv=excl ou conv=nocreat peut être spécifié dd-error-invalid-flag = indicateur d'entrée invalide : '{ $flag }' - Essayez '{ $cmd } --help' pour plus d'informations. dd-error-conv-flag-no-match = conv=CONV non reconnu -> { $flag } dd-error-multiplier-parse-failure = nombre invalide : ‘{ $input }‘ dd-error-multiplier-overflow = La chaîne de multiplicateur déborderait sur le système actuel -> { $input } @@ -160,3 +159,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 1ba8a1b6189..723add1f14b 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,15 @@ 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)?; + // The command line is kept for the caret in operand diagnostics. + let (matches, diag_args) = + uucore::clap_localization::handle_clap_result_with_diagnostics(uu_app(), args.collect())?; - 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 00000000000..eb234fabf7d --- /dev/null +++ b/src/uu/dd/src/diagnostics.rs @@ -0,0 +1,93 @@ +// 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, list_items}; +use uucore::error::UError; +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 { + uucore::diagnostics::error_after_report(diag_args, error, |args, error| { + render(args, operand, 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, at its place in the list rather + // than wherever its text first turns up. + let flag = |flag: &str| { + list_items(&operand[value_start..], &[',']) + .find(|&(part, _)| part == flag) + .map(|(_, span)| value_start + span.start..value_start + span.end) + }; + + 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; + }; + snapshot.render_inside_at( + index, + operand, + span, + &error.to_string(), + None, + Some(&translate!(help)), + ) +} diff --git a/src/uu/dd/src/parseargs.rs b/src/uu/dd/src/parseargs.rs index a2b710404db..8acf1830ddc 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; @@ -29,7 +30,7 @@ pub enum ParseError { MultipleBlockUnblock, #[error("{}", translate!("dd-error-multiple-excl"))] MultipleExclNoCreate, - #[error("{}", translate!("dd-error-invalid-flag", "flag" => .0.clone(), "cmd" => uucore::execution_phrase()))] + #[error("{}", translate!("dd-error-invalid-flag", "flag" => .0.clone()))] FlagNoMatch(String), #[error("{}", translate!("dd-error-conv-flag-no-match", "flag" => .0.clone()))] ConvFlagNoMatch(String), @@ -129,19 +130,42 @@ impl Parser { Self::default() } - pub(crate) fn parse( + /// 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>, - ) -> Result { - self.read(operands)?.validate() + 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) @@ -447,6 +471,13 @@ impl UError for ParseError { fn code(&self) -> i32 { 1 } + + /// The one message that ends on a hint about the syntax it rejected. The + /// hint is left to this, rather than written into the message, so that it + /// survives a caret report replacing the message. + fn usage(&self) -> bool { + matches!(self, Self::FlagNoMatch(_)) + } } fn show_zero_multiplier_warning() { diff --git a/src/uu/dd/src/parseargs/unit_tests.rs b/src/uu/dd/src/parseargs/unit_tests.rs index cde0ef0cc1f..9e5c9871b10 100644 --- a/src/uu/dd/src/parseargs/unit_tests.rs +++ b/src/uu/dd/src/parseargs/unit_tests.rs @@ -12,6 +12,19 @@ use crate::conversion_tables::{ }; use crate::parseargs::Parser; +impl Parser { + /// Parse the operands, keeping only the error itself. + /// + /// The utility goes through `parse_with_diagnostics`, which also knows + /// which operand failed; this is the plain form the tests compare against. + pub(crate) fn parse( + self, + operands: impl IntoIterator>, + ) -> Result { + self.read(operands).map_err(|(_, error)| error)?.validate() + } +} + #[cfg(not(any(target_os = "linux", target_os = "android")))] #[allow(clippy::useless_vec)] #[test] diff --git a/src/uu/head/src/head.rs b/src/uu/head/src/head.rs index 7392046f336..7f3049c15d9 100644 --- a/src/uu/head/src/head.rs +++ b/src/uu/head/src/head.rs @@ -17,6 +17,7 @@ use std::os::fd::AsFd; use std::path::Path; use std::path::PathBuf; use thiserror::Error; +use uucore::diagnostics::OptionValue; use uucore::display::{Quotable, print_verbatim}; use uucore::error::{FromIo, UError, UResult, USimpleError}; use uucore::line_ending::LineEnding; @@ -85,9 +86,7 @@ impl Default for Mode { /// made of it. pub struct SizeError { pub message: String, - value: String, - short: char, - long: &'static str, + option: OptionValue, error: ParseSizeError, } @@ -97,11 +96,9 @@ impl SizeError { fn into_error(self, diag_args: Option<&[OsString]>) -> Box { self.error.size_value_error( diag_args, - &self.value, + &self.option, // The parser never saw the sign; the caret has to count it back in. - number_offset(&self.value), - self.short, - self.long, + number_offset(&self.option.value), &self.message, HeadError::MatchOption(self.message.clone()), ) @@ -116,12 +113,10 @@ impl Mode { long: &'static str, key: &'static str, ) -> impl FnOnce(ParseSizeError) -> SizeError { - let value = value.to_string(); + let option = OptionValue::new(value, short, long); move |error| SizeError { message: translate!(key, "err" => &error), - value, - short, - long, + option, error, } } diff --git a/src/uu/join/locales/en-US.ftl b/src/uu/join/locales/en-US.ftl index 81a339a2e76..04cc94fd013 100644 --- a/src/uu/join/locales/en-US.ftl +++ b/src/uu/join/locales/en-US.ftl @@ -33,3 +33,6 @@ join-error-invalid-field-number = invalid field number: { $value } join-error-incompatible-fields = incompatible join fields { $field1 }, { $field2 } join-error-not-sorted = { $file }:{ $line_num }: is not sorted: { $content } join-error-input-not-sorted = input is not in sorted order + +# Diagnostics +join-diag-help-format = an output field is FILENUM.FIELD, as in -o 1.2,2.1; 0 stands for the join field diff --git a/src/uu/join/locales/fr-FR.ftl b/src/uu/join/locales/fr-FR.ftl index 75ed4235532..eac430b5d55 100644 --- a/src/uu/join/locales/fr-FR.ftl +++ b/src/uu/join/locales/fr-FR.ftl @@ -33,3 +33,6 @@ join-error-invalid-field-number = numéro de champ invalide : { $value } join-error-incompatible-fields = champs de jointure incompatibles { $field1 }, { $field2 } join-error-not-sorted = { $file }:{ $line_num } : n'est pas trié : { $content } join-error-input-not-sorted = l'entrée n'est pas dans l'ordre trié + +# Diagnostics +join-diag-help-format = un champ de sortie s'écrit NUMFICHIER.CHAMP, comme dans -o 1.2,2.1 ; 0 désigne le champ de jointure diff --git a/src/uu/join/src/join.rs b/src/uu/join/src/join.rs index ec3dd8690d0..52129b4aed4 100644 --- a/src/uu/join/src/join.rs +++ b/src/uu/join/src/join.rs @@ -16,6 +16,7 @@ use std::num::IntErrorKind; #[cfg(unix)] use std::os::unix::ffi::OsStrExt; use thiserror::Error; +use uucore::diagnostics::OptionValue; use uucore::display::Quotable; use uucore::error::{FromIo, UError, UResult, USimpleError, set_exit_code}; use uucore::i18n::collator::{ @@ -764,7 +765,7 @@ fn get_and_parse_field_number(matches: &clap::ArgMatches, key: &str) -> UResult< /// This function takes the matches from the command-line arguments, processes them, /// and returns a `Settings` struct that encapsulates the configuration for the program. #[allow(clippy::field_reassign_with_default)] -fn parse_settings(matches: &clap::ArgMatches) -> UResult { +fn parse_settings(matches: &clap::ArgMatches, diag_args: Option<&[OsString]>) -> UResult { let keys = get_and_parse_field_number(matches, "j")?; let key1 = get_and_parse_field_number(matches, "1")?; let key2 = get_and_parse_field_number(matches, "2")?; @@ -788,8 +789,23 @@ fn parse_settings(matches: &clap::ArgMatches) -> UResult { settings.autoformat = true; } else { let mut specs = vec![]; - for part in format.split([' ', ',', '\t']) { - specs.push(Spec::parse(part)?); + // `-o` has no long form. + let option = OptionValue::with_names(format.clone(), Some('o'), None); + // Each field carries its place in the value, so that the caret can + // take the one that is at fault out of a long list. + for (part, span) in uucore::diagnostics::list_items(format, &[' ', ',', '\t']) { + specs.push(Spec::parse(part).map_err(|error| { + let message = error.to_string(); + uucore::diagnostics::error_after_report(diag_args, error, |args, _| { + uucore::diagnostics::Snapshot::with_program(args).render_option( + &option, + span, + &message, + None, + Some(&translate!("join-diag-help-format")), + ) + }) + })?); } settings.format = specs; } @@ -818,13 +834,16 @@ fn parse_settings(matches: &clap::ArgMatches) -> UResult { #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let matches = uucore::clap_localization::handle_clap_result(uu_app(), args)?; + // The command line is kept for the caret in `-o` diagnostics, which needs + // the list as typed. + let (matches, diag_args) = + uucore::clap_localization::handle_clap_result_with_diagnostics(uu_app(), args.collect())?; let mut opts = CollatorOptions::default(); opts.alternate_handling = Some(AlternateHandling::Shifted); let _ = try_init_collator(opts); - let settings = parse_settings(&matches)?; + let settings = parse_settings(&matches, diag_args.as_deref())?; let file1 = matches.get_one::("file1").unwrap(); let file2 = matches.get_one::("file2").unwrap(); diff --git a/src/uu/numfmt/src/numfmt.rs b/src/uu/numfmt/src/numfmt.rs index ac1567c2501..e6844d84ad8 100644 --- a/src/uu/numfmt/src/numfmt.rs +++ b/src/uu/numfmt/src/numfmt.rs @@ -19,7 +19,7 @@ use std::io::{BufRead, Write as _, stderr}; use std::str::FromStr; use uucore::display::Quotable; -use uucore::error::{UResult, quiet_if_reported}; +use uucore::error::UResult; use uucore::i18n::decimal::locale_grouping_separator; use uucore::parser::parse_size::{IEC_BASES, SI_BASES}; use uucore::parser::shortcut_value_parser::ShortcutValueParser; @@ -132,10 +132,13 @@ fn handle_args<'a>( // Only this mode stops on the first bad number; the others carry // on, where a report per line would bury the output. Err(error) => { - let reported = snapshot.is_some_and(|args| { - diagnostics::render_input(args, l, n, &error.to_string(), options) - }); - return Err(quiet_if_reported(reported, error)); + return Err(uucore::diagnostics::error_after_report( + snapshot, + error, + |args, error| { + diagnostics::render_input(args, l, n, &error.to_string(), options) + }, + )); } } } @@ -443,34 +446,34 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // A format error still knows where in the format string it happened, // so it is the one error worth a caret. Err(ParseError::Format(error)) => { - let reported = format_args - .as_ref() - .zip(matches.get_one::(FORMAT)) - .is_some_and(|(args, format)| diagnostics::render(args, format, &error)); - return Err(quiet_if_reported( - reported, - NumfmtError::IllegalArgument(error.message), + return Err(uucore::diagnostics::error_after_report( + format_args.as_deref(), + NumfmtError::IllegalArgument(error.message.clone()), + |args, _| { + matches + .get_one::(FORMAT) + .is_some_and(|format| diagnostics::render(args, format, &error)) + }, )); } // As for a format, a field list knows which of its ranges is at fault. Err(ParseError::Field(error)) => { - let reported = format_args - .as_ref() - .zip(matches.get_one::(FIELD)) - .is_some_and(|(args, fields)| diagnostics::render_field(args, fields, &error)); - return Err(quiet_if_reported( - reported, - NumfmtError::IllegalArgument(error.message), + return Err(uucore::diagnostics::error_after_report( + format_args.as_deref(), + NumfmtError::IllegalArgument(error.message.clone()), + |args, _| { + matches + .get_one::(FIELD) + .is_some_and(|fields| diagnostics::render_field(args, fields, &error)) + }, )); } // An option value that is wrong as a whole: underline it where typed. Err(ParseError::Value(error)) => { - let reported = format_args - .as_ref() - .is_some_and(|args| diagnostics::render_value(args, &error)); - return Err(quiet_if_reported( - reported, - NumfmtError::IllegalArgument(error.message), + return Err(uucore::diagnostics::error_after_report( + format_args.as_deref(), + NumfmtError::IllegalArgument(error.message.clone()), + |args, _| diagnostics::render_value(args, &error), )); } Err(ParseError::Other(message)) => { diff --git a/src/uu/od/src/od.rs b/src/uu/od/src/od.rs index 4398b0089b7..41363668bd6 100644 --- a/src/uu/od/src/od.rs +++ b/src/uu/od/src/od.rs @@ -40,6 +40,8 @@ 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::diagnostics::OptionValue; use uucore::display::Quotable; use uucore::error::{UResult, USimpleError}; use uucore::translate; @@ -81,23 +83,32 @@ struct OdOptions { fn parse_bytes_option( matches: &ArgMatches, args: &[String], - option_name: &str, + diag_args: Option<&[OsString]>, + option_name: &'static str, short: Option, ) -> UResult> { match matches.get_one::(option_name) { 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)); + let option = OptionValue::with_names(s.clone(), short, Some(option_name)); + Err(e.size_value_error( + diag_args, + &option, + 0, + &message, + USimpleError::new(1, message.clone()), + )) + } }, } } impl OdOptions { - fn new(matches: &ArgMatches, args: &[String]) -> UResult { + fn new(matches: &ArgMatches, args: &[String], diag_args: Option<&[OsString]>) -> UResult { let byte_order = if let Some(s) = matches.get_one::(options::ENDIAN) { match s.as_str() { "little" => ByteOrder::Little, @@ -114,7 +125,8 @@ impl OdOptions { }; let mut skip_bytes = - parse_bytes_option(matches, args, options::SKIP_BYTES, Some('j'))?.unwrap_or(0); + parse_bytes_option(matches, args, diag_args, options::SKIP_BYTES, Some('j'))? + .unwrap_or(0); let mut label: Option = None; @@ -135,8 +147,16 @@ 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); + e.size_value_error( + diag_args, + &OptionValue::new(s, 'w', options::WIDTH), + 0, + &message, + USimpleError::new(1, message.clone()), + ) + })?; if parsed == 0 { return Err(USimpleError::new( 1, @@ -174,9 +194,11 @@ impl OdOptions { let output_duplicates = matches.get_flag(options::OUTPUT_DUPLICATES); - let read_bytes = parse_bytes_option(matches, args, options::READ_BYTES, Some('N'))?; + let read_bytes = + parse_bytes_option(matches, args, diag_args, options::READ_BYTES, Some('N'))?; - let string_min_length = match parse_bytes_option(matches, args, options::STRINGS, Some('S'))? { + let strings = parse_bytes_option(matches, args, diag_args, options::STRINGS, Some('S'))?; + let string_min_length = match strings { None => None, Some(n) => Some(usize::try_from(n).map_err(|_| { USimpleError::new( @@ -235,12 +257,15 @@ impl OdOptions { #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { let args = args.collect_ignore(); + let raw_args: Vec = args.iter().map(OsString::from).collect(); let clap_opts = uu_app(); let clap_matches = uucore::clap_localization::handle_clap_result(clap_opts, &args)?; - let od_options = OdOptions::new(&clap_matches, &args)?; + // Kept for the caret in SIZE diagnostics, which echoes the command line. + let diag_args = uucore::diagnostics::capture(&raw_args); + let od_options = OdOptions::new(&clap_matches, &args, diag_args.as_deref())?; let mut out = std::io::stdout().lock(); // Check if we're in strings mode diff --git a/src/uu/printf/src/printf.rs b/src/uu/printf/src/printf.rs index 8e13a3d1e79..3666400ea0a 100644 --- a/src/uu/printf/src/printf.rs +++ b/src/uu/printf/src/printf.rs @@ -7,7 +7,7 @@ use std::ffi::OsString; use std::io::{Write, stdout}; use std::ops::ControlFlow; use uucore::display::Quotable; -use uucore::error::{FromIo, UError, UResult, UUsageError, quiet_if_reported}; +use uucore::error::{FromIo, UError, UResult, UUsageError}; use uucore::format::{ FormatArgument, FormatArguments, FormatError, FormatItem, parse_spec_and_escape, }; @@ -63,10 +63,9 @@ fn print_formatted(args: impl uucore::Args) -> UResult<()> { // A parse error is rendered against the argument list when stderr is a // terminal; the plain one-line message is kept anywhere else. let raise = |error: FormatError| -> Box { - let reported = diag_args - .as_ref() - .is_some_and(|args| diagnostics::render(args, format, &error)); - quiet_if_reported(reported, error) + uucore::diagnostics::error_after_report(diag_args.as_deref(), error, |args, error| { + diagnostics::render(args, format, error) + }) }; let mut format_seen = false; diff --git a/src/uu/seq/locales/en-US.ftl b/src/uu/seq/locales/en-US.ftl index 34a97d43868..285727f24af 100644 --- a/src/uu/seq/locales/en-US.ftl +++ b/src/uu/seq/locales/en-US.ftl @@ -18,3 +18,6 @@ seq-error-format-and-equal-width = format string may not be specified when print # Parse error types seq-parse-error-type-float = floating point seq-parse-error-type-nan = 'not-a-number' + +# Diagnostics +seq-diag-help-format = a format holds exactly one float conversion: %f, %e, %g or %a, as in -f%.3f diff --git a/src/uu/seq/locales/fr-FR.ftl b/src/uu/seq/locales/fr-FR.ftl index ed804982051..df98846079d 100644 --- a/src/uu/seq/locales/fr-FR.ftl +++ b/src/uu/seq/locales/fr-FR.ftl @@ -18,3 +18,6 @@ seq-error-format-and-equal-width = la chaîne de format ne peut pas être spéci # Types d'erreur d'analyse seq-parse-error-type-float = nombre à virgule flottante seq-parse-error-type-nan = 'non-un-nombre' + +# Diagnostics +seq-diag-help-format = un format contient exactement une conversion flottante : %f, %e, %g ou %a, comme dans -f%.3f diff --git a/src/uu/seq/src/diagnostics.rs b/src/uu/seq/src/diagnostics.rs new file mode 100644 index 00000000000..6cdb09205b4 --- /dev/null +++ b/src/uu/seq/src/diagnostics.rs @@ -0,0 +1,46 @@ +// 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. + +//! Maps a [`FormatError`] onto the part of the `-f` format it came from, so +//! that [`uucore::diagnostics`] can render it with a caret. + +use std::ffi::OsString; +use std::ops::Range; + +use uucore::diagnostics::{OptionValue, Snapshot}; +use uucore::format::FormatError; +use uucore::translate; + +/// Render `error` against `args` — the whole argument list, program name +/// included — where `format` is the value of `-f`/`--format` as typed. +/// +/// # Returns +/// +/// `false` when the error is not about the format string, or when the format +/// cannot be found among the arguments, in which case the caller should fall +/// back to the plain one-line message. +pub fn render(args: &[OsString], format: &str, error: &FormatError) -> bool { + let span: Range = match error { + FormatError::SpecError(_, span) + | FormatError::MissingHex(Some(span)) + | FormatError::InvalidCharacter(_, _, Some(span)) => span.clone(), + // These are about the format as a whole — it holds no directive, or + // more than one, or one seq cannot print a number with — so the caret + // takes all of it. + FormatError::TooManySpecs(_) + | FormatError::NeedAtLeastOneSpec(_) + | FormatError::EndsWithPercent(_) + | FormatError::WrongSpecType => 0..format.len(), + _ => return false, + }; + + Snapshot::with_program(args).render_option( + &OptionValue::new(format, 'f', crate::OPT_FORMAT), + span, + &error.to_string(), + None, + Some(&translate!("seq-diag-help-format")), + ) +} diff --git a/src/uu/seq/src/seq.rs b/src/uu/seq/src/seq.rs index 64ab7100c4c..2c5a82f4111 100644 --- a/src/uu/seq/src/seq.rs +++ b/src/uu/seq/src/seq.rs @@ -17,6 +17,7 @@ use uucore::format::num_format::FloatVariant; use uucore::format::{Format, num_format}; use uucore::{fast_inc::fast_inc, format_usage}; +mod diagnostics; mod error; // public to allow fuzzing @@ -35,7 +36,7 @@ use uucore::translate; const OPT_SEPARATOR: &str = "separator"; const OPT_TERMINATOR: &str = "terminator"; const OPT_EQUAL_WIDTH: &str = "equal-width"; -const OPT_FORMAT: &str = "format"; +pub(crate) const OPT_FORMAT: &str = "format"; const ARG_NUMBERS: &str = "numbers"; @@ -94,8 +95,14 @@ fn select_precision( #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let matches = - uucore::clap_localization::handle_clap_result(uu_app(), split_short_args_with_value(args))?; + let raw_args: Vec = args.collect(); + // Captured before `-f%q` is split into two arguments, so that the caret + // echoes the command line as it was typed. + let diag_args = uucore::diagnostics::capture(&raw_args); + let matches = uucore::clap_localization::handle_clap_result( + uu_app(), + split_short_args_with_value(raw_args.into_iter()), + )?; let numbers_option = matches.get_many::(ARG_NUMBERS); @@ -152,11 +159,15 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // If a format was passed on the command line, use that. // If not, use some default format based on parameters precision. let (format, padding, fast_allowed) = if let Some(str) = options.format { - ( - Format::::parse(str)?, - 0, - false, - ) + let format = + Format::::parse(str).map_err(|error| { + uucore::diagnostics::error_after_report( + diag_args.as_deref(), + error, + |args, error| diagnostics::render(args, str, error), + ) + })?; + (format, 0, false) } else { let precision = select_precision(&first, &increment, &last); diff --git a/src/uu/shred/src/shred.rs b/src/uu/shred/src/shred.rs index 25a12e87307..b18e33850df 100644 --- a/src/uu/shred/src/shred.rs +++ b/src/uu/shred/src/shred.rs @@ -16,6 +16,7 @@ use std::io::{self, Read, Seek, SeekFrom, Write}; #[cfg(unix)] use std::os::unix::prelude::PermissionsExt; use std::path::{Path, PathBuf}; +use uucore::diagnostics::OptionValue; use uucore::display::Quotable; use uucore::error::{FromIo, UResult, USimpleError, UUsageError}; use uucore::parser::parse_size::parse_size_u64; @@ -244,7 +245,10 @@ impl BytesWriter { #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let matches = uucore::clap_localization::handle_clap_result(uu_app(), args)?; + // The command line is kept for the caret in size diagnostics, which needs + // the size as typed. + let (matches, diag_args) = + uucore::clap_localization::handle_clap_result_with_diagnostics(uu_app(), args.collect())?; if !matches.contains_id(options::FILE) { return Err(UUsageError::new( @@ -290,7 +294,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 +403,31 @@ 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, + &OptionValue::new(&size, 's', options::SIZE), + 0, + &message, + USimpleError::new(1, message.clone()), + )) + } + } } fn pass_name(pass_type: &PassType) -> String { diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 6639b744d53..314868589d0 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -45,9 +45,10 @@ use std::path::PathBuf; use std::str::Utf8Error; use std::sync::OnceLock; use thiserror::Error; +use uucore::diagnostics::OptionValue; use uucore::display::Quotable; use uucore::error::{FromIo, strip_errno}; -use uucore::error::{UError, UResult, USimpleError, UUsageError, quiet_if_reported}; +use uucore::error::{UError, UResult, USimpleError, UUsageError}; use uucore::extendedbigdecimal::ExtendedBigDecimal; #[cfg(feature = "i18n-collator")] use uucore::i18n::collator::{compute_sort_key_utf8, locale_cmp}; @@ -2228,8 +2229,15 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } if let Some(size_str) = matches.get_one::(options::BUF_SIZE) { - settings.buffer_size = GlobalSettings::parse_byte_count(size_str).map_err(|e| { - USimpleError::new(2, format_error_message(&e, size_str, options::BUF_SIZE)) + settings.buffer_size = GlobalSettings::parse_byte_count(size_str).map_err(|error| { + let message = format_error_message(&error, size_str, options::BUF_SIZE); + error.size_value_error( + key_args.as_deref(), + &OptionValue::new(size_str, 'S', options::BUF_SIZE), + 0, + &message, + USimpleError::new(2, message.clone()), + ) })?; settings.buffer_size_is_explicit = true; } else { @@ -2376,10 +2384,11 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let selector = match FieldSelector::parse(value, &settings) { Ok(selector) => selector, Err(error) => { - let reported = key_args - .as_ref() - .is_some_and(|args| diagnostics::render(args, value, &error)); - return Err(quiet_if_reported(reported, error)); + return Err(uucore::diagnostics::error_after_report( + key_args.as_deref(), + error, + |args, error| diagnostics::render(args, value, error), + )); } }; settings.selectors.push(selector); diff --git a/src/uu/split/src/split.rs b/src/uu/split/src/split.rs index 227508b7810..20c6748a192 100644 --- a/src/uu/split/src/split.rs +++ b/src/uu/split/src/split.rs @@ -32,15 +32,27 @@ 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}")) - } else { - USimpleError::new(1, format!("{e}")) + return UUsageError::new(1, message); } + uucore::diagnostics::error_after_report( + diag_args.as_deref(), + USimpleError::new(1, message.clone()), + |args, _| match &e { + SettingsError::Strategy(error) => error.render(args, &message), + // The rest is about how the options combine rather than about + // one of them, so there is nothing to point a caret at. + _ => false, + }, + ) })?; // When using --filter, we write to a child process's stdin which may diff --git a/src/uu/split/src/strategy.rs b/src/uu/split/src/strategy.rs index e3d6093cc61..0388cb5de1b 100644 --- a/src/uu/split/src/strategy.rs +++ b/src/uu/split/src/strategy.rs @@ -7,8 +7,10 @@ use crate::cli::options; use clap::{ArgMatches, parser::ValueSource}; +use std::ffi::OsString; use thiserror::Error; use uucore::{ + diagnostics::OptionValue, display::Quotable, parser::parse_size::{ParseSizeError, parse_size_u64, parse_size_u64_max}, translate, @@ -202,15 +204,19 @@ pub enum Strategy { } /// An error when parsing a chunking strategy from command-line arguments. +/// +/// A bad size carries the option it was given to, so that a caret can point +/// inside it — `None` when it came from no option, as with the obsolete +/// `split -22` spelling. #[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 +227,46 @@ 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, Some(option)) | Self::Bytes(error, Some(option))) = self else { + return false; + }; + error.render_size_value(diag_args, option, 0, 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)?; + // `None` for a size that did not come from an option, such as the + // obsolete `split -22` spelling: there is nothing to point at. + let origin = || Some(OptionValue::new(s, short, 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 +282,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/src/uu/stat/locales/en-US.ftl b/src/uu/stat/locales/en-US.ftl index 1b14b1b9d96..fd81d42218c 100644 --- a/src/uu/stat/locales/en-US.ftl +++ b/src/uu/stat/locales/en-US.ftl @@ -114,3 +114,6 @@ stat-word-birth = Birth stat-selinux-failed-get-context = failed to get security context stat-selinux-unsupported-system = unsupported on this system stat-selinux-unsupported-os = unsupported for this operating system + +# Diagnostics +stat-diag-help-directive = a directive is %[FLAGS][WIDTH][.PRECISION]LETTER, as in %-10.2s; a literal % is written %% diff --git a/src/uu/stat/locales/fr-FR.ftl b/src/uu/stat/locales/fr-FR.ftl index 824b092bb6e..45693e4dd4e 100644 --- a/src/uu/stat/locales/fr-FR.ftl +++ b/src/uu/stat/locales/fr-FR.ftl @@ -113,3 +113,6 @@ stat-warning-unrecognized-escape = séquence d'échappement non reconnue '\{$esc stat-selinux-failed-get-context = impossible d'obtenir le contexte de sécurité stat-selinux-unsupported-system = non pris en charge sur ce système stat-selinux-unsupported-os = non pris en charge pour ce système d'exploitation + +# Diagnostics +stat-diag-help-directive = une directive s'écrit %[DRAPEAUX][LARGEUR][.PRÉCISION]LETTRE, comme dans %-10.2s ; un % littéral s'écrit %% diff --git a/src/uu/stat/src/stat.rs b/src/uu/stat/src/stat.rs index e1eaa4f3be1..8760f6e293b 100644 --- a/src/uu/stat/src/stat.rs +++ b/src/uu/stat/src/stat.rs @@ -4,6 +4,8 @@ // file that was distributed with this source code. // spell-checker:ignore datetime +use std::ops::Range; +use uucore::diagnostics::OptionValue; use uucore::error::{UError, UResult, USimpleError}; use uucore::i18n::UEncoding; use uucore::quoting_style::{QuotingStyle as UucoreQuotingStyle, escape_name}; @@ -82,22 +84,90 @@ struct Flags { /// checks if the string is within the specified bound, /// if it gets out of bound, error out by printing sub-string from index `beg` to`end`, /// where `beg` & `end` is the beginning and end index of sub-string, respectively -fn check_bound(slice: &str, bound: usize, beg: usize, end: usize) -> UResult<()> { +fn check_bound(slice: &str, bound: usize, beg: usize, end: usize) -> Result<(), DirectiveError> { if end >= bound { // `beg`/`end` are char indices, so take the directive by chars: byte-slicing // `slice` could land mid-UTF-8 when a multibyte char precedes the directive. let directive: String = slice.chars().skip(beg).take(end - beg).collect(); - return Err(USimpleError::new( - 1, - StatError::InvalidDirective { - directive: directive.quote().to_string(), - } - .to_string(), - )); + return Err(DirectiveError::new(slice, &directive, beg, end)); } Ok(()) } +/// Converts a character index to a byte index in a UTF-8 string +/// +/// This is necessary because Rust strings are UTF-8 encoded, so character +/// positions don't always align with byte positions for multi-byte characters. +/// An index past the last character gives the end of the string. +fn char_index_to_byte_index(format_str: &str, char_index: usize) -> usize { + format_str + .char_indices() + .nth(char_index) + .map_or(format_str.len(), |(byte_idx, _)| byte_idx) +} + +/// A directive stat does not know, and where it sat in the format string. +/// +/// The message is the one stat always printed; the byte range is what a caret +/// needs to point inside the format rather than at all of it. +#[derive(Debug)] +struct DirectiveError { + directive: String, + span: Range, +} + +impl DirectiveError { + /// # Arguments + /// + /// * `format_str` - The format string the directive came from. + /// * `directive` - The directive as written, without its quotes. + /// * `beg`, `end` - Its char indices in `format_str`; `end` may sit past + /// the end, for a directive the format stops in the middle of. + fn new(format_str: &str, directive: &str, beg: usize, end: usize) -> Self { + Self { + directive: directive.quote().to_string(), + span: char_index_to_byte_index(format_str, beg) + ..char_index_to_byte_index(format_str, end), + } + } + + /// The error to raise, a caret under the directive when the format was + /// given on the command line and stderr is a terminal. + /// + /// # Arguments + /// + /// * `diag_args` - The arguments as typed, or `None` when they were not + /// kept. + /// * `option` - The format as typed and the option it was given to, or + /// `None` for a format stat built itself, which is not on the command + /// line and has nothing to point at. + fn into_error( + self, + diag_args: Option<&[OsString]>, + option: Option<&OptionValue>, + ) -> Box { + let message = StatError::InvalidDirective { + directive: self.directive, + } + .to_string(); + uucore::diagnostics::error_after_report( + diag_args, + USimpleError::new(1, message.clone()), + |args, _| { + option.is_some_and(|option| { + uucore::diagnostics::Snapshot::with_program(args).render_option( + option, + self.span.clone(), + &message, + None, + Some(&translate!("stat-diag-help-directive")), + ) + }) + }, + ) + } +} + enum Padding { Zero, Space, @@ -760,22 +830,12 @@ impl Stater { } } - /// Converts a character index to a byte index in a UTF-8 string - /// This is necessary because Rust strings are UTF-8 encoded, so character positions - /// don't always align with byte positions for multi-byte characters - fn char_index_to_byte_index(format_str: &str, char_index: usize) -> usize { - format_str - .char_indices() - .nth(char_index) - .map_or(format_str.len(), |(byte_idx, _)| byte_idx) - } - fn handle_percent_case( chars: &[char], i: &mut usize, bound: usize, format_str: &str, - ) -> UResult { + ) -> Result { let old = *i; *i += 1; @@ -794,20 +854,20 @@ impl Stater { let mut precision = Precision::NotSpecified; let mut j = *i; - let j_byte = Self::char_index_to_byte_index(format_str, j); + let j_byte = char_index_to_byte_index(format_str, j); if let Some((field_width, offset)) = format_str[j_byte..].scan_num::() { width = field_width; j += offset; // Reject directives like `%` by checking if width has been parsed. if j >= bound || chars[j] == '%' { - let invalid_directive: String = chars[old..=j.min(bound - 1)].iter().collect(); - return Err(USimpleError::new( - 1, - StatError::InvalidDirective { - directive: invalid_directive.quote().to_string(), - } - .to_string(), + let end = j.min(bound - 1); + let invalid_directive: String = chars[old..=end].iter().collect(); + return Err(DirectiveError::new( + format_str, + &invalid_directive, + old, + end + 1, )); } } @@ -817,7 +877,7 @@ impl Stater { j += 1; check_bound(format_str, bound, old, j)?; - let j_byte = Self::char_index_to_byte_index(format_str, j); + let j_byte = char_index_to_byte_index(format_str, j); match format_str[j_byte..].scan_num::() { Some((value, offset)) => { if value >= 0 { @@ -898,7 +958,7 @@ impl Stater { // Parse hexadecimal escape sequence (\xNN format) // Uses UTF-8 safe byte indexing to handle multi-byte characters properly if *i + 1 < bound { - let byte_index = Self::char_index_to_byte_index(format_str, *i + 1); + let byte_index = char_index_to_byte_index(format_str, *i + 1); if let Some((c, offset)) = format_str[byte_index..].scan_char(16) { *i += offset; Token::Byte(c as u8) @@ -921,7 +981,7 @@ impl Stater { } } - fn generate_tokens(format_str: &str, use_printf: bool) -> UResult> { + fn generate_tokens(format_str: &str, use_printf: bool) -> Result, DirectiveError> { let mut tokens = Vec::new(); let chars = format_str.chars().collect::>(); let bound = chars.len(); @@ -973,7 +1033,7 @@ impl Stater { Ok(mount_list) } - fn new(matches: &ArgMatches) -> UResult { + fn new(matches: &ArgMatches, diag_args: Option<&[OsString]>) -> UResult { let files: Vec = matches .get_many::(options::FILES) .map(|v| v.map(OsString::from).collect()) @@ -995,13 +1055,30 @@ impl Stater { let terse = matches.get_flag(options::TERSE); let show_fs = matches.get_flag(options::FILE_SYSTEM); + // Only the format the user typed can be pointed at; the ones stat + // builds for itself never fail, and are not on the command line. + // `--printf` has no short form; `--format` also answers to `-c`. + let given_option = || { + OptionValue::with_names( + format_str, + if use_printf { None } else { Some('c') }, + Some(if use_printf { + options::PRINTF + } else { + options::FORMAT + }), + ) + }; let default_tokens = if format_str.is_empty() { - Self::generate_tokens(&Self::default_format(show_fs, terse, false), use_printf)? + Self::generate_tokens(&Self::default_format(show_fs, terse, false), use_printf) + .map_err(|e| e.into_error(diag_args, None))? } else { - Self::generate_tokens(format_str, use_printf)? + Self::generate_tokens(format_str, use_printf) + .map_err(|e| e.into_error(diag_args, Some(&given_option())))? }; let default_dev_tokens = - Self::generate_tokens(&Self::default_format(show_fs, terse, true), use_printf)?; + Self::generate_tokens(&Self::default_format(show_fs, terse, true), use_printf) + .map_err(|e| e.into_error(diag_args, None))?; // mount points aren't displayed when showing filesystem information, or // whenever the format string does not request the mount point. @@ -1379,9 +1456,12 @@ impl Stater { #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let matches = uucore::clap_localization::handle_clap_result(uu_app(), args)?; + // The command line is kept for the caret in format diagnostics, which + // needs the format as typed. + let (matches, diag_args) = + uucore::clap_localization::handle_clap_result_with_diagnostics(uu_app(), args.collect())?; - let stater = Stater::new(&matches)?; + let stater = Stater::new(&matches, diag_args.as_deref())?; let exit_status = stater.exec(); if exit_status == 0 { Ok(()) diff --git a/src/uu/stdbuf/src/stdbuf.rs b/src/uu/stdbuf/src/stdbuf.rs index ead5c055511..024ea672b5d 100644 --- a/src/uu/stdbuf/src/stdbuf.rs +++ b/src/uu/stdbuf/src/stdbuf.rs @@ -14,10 +14,11 @@ use std::process; use tempfile::TempDir; use tempfile::tempdir; use thiserror::Error; +use uucore::diagnostics::OptionValue; use uucore::display::Quotable; use uucore::error::{UResult, USimpleError, UUsageError, strip_errno}; use uucore::format_usage; -use uucore::parser::parse_size::parse_size_u64; +use uucore::parser::parse_size::{ParseSizeError, parse_size_u64}; use uucore::translate; mod options { @@ -64,19 +65,30 @@ impl TryFrom<&ArgMatches> for ProgramOptions { fn try_from(matches: &ArgMatches) -> Result { Ok(Self { - stdin: check_option(matches, options::INPUT)?, - stdout: check_option(matches, options::OUTPUT)?, - stderr: check_option(matches, options::ERROR)?, + stdin: check_option(matches, options::INPUT, options::INPUT_SHORT)?, + stdout: check_option(matches, options::OUTPUT, options::OUTPUT_SHORT)?, + stderr: check_option(matches, options::ERROR, options::ERROR_SHORT)?, }) } } +/// A buffering mode that did not parse as a size, and where it came from. +/// +/// The message is built where it always was; the rest is what a caret needs: +/// the mode as typed with the option it was given to, and what the size parser +/// made of it. +#[derive(Debug)] +struct ModeError { + option: OptionValue, + error: ParseSizeError, +} + #[derive(Debug, Error)] enum ProgramOptionsError { #[error("{}", translate!("stdbuf-error-line-buffering-stdin-meaningless"))] LineBufferingStdinMeaningless, - #[error("{}", translate!("stdbuf-error-invalid-mode", "error" => _0.clone()))] - InvalidMode(String), + #[error("{}", translate!("stdbuf-error-invalid-mode", "error" => _0.error.to_string()))] + InvalidMode(Box), #[error("{}", translate!("stdbuf-error-value-too-large", "value" => _0.clone()))] ValueTooLarge(String), } @@ -96,7 +108,11 @@ fn preload_strings() -> (&'static str, &'static str) { ("LD_PRELOAD", "dll") } -fn check_option(matches: &ArgMatches, name: &str) -> Result { +fn check_option( + matches: &ArgMatches, + name: &'static str, + short: char, +) -> Result { match matches.get_one::(name) { Some(value) => match value.as_str() { "L" => { @@ -107,7 +123,12 @@ fn check_option(matches: &ArgMatches, name: &str) -> Result parse_size_u64(x).map_or_else( - |e| Err(ProgramOptionsError::InvalidMode(e.to_string())), + |error| { + Err(ProgramOptionsError::InvalidMode(Box::new(ModeError { + option: OptionValue::new(x, short, name), + error, + }))) + }, |m| { Ok(BufferType::Size(m.try_into().map_err(|_| { ProgramOptionsError::ValueTooLarge(x.to_string()) @@ -191,11 +212,28 @@ fn get_preload_env(_tmp_dir: &TempDir) -> UResult<(String, PathBuf)> { #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { + let raw_args: Vec = args.collect(); + // Kept for the caret in mode diagnostics, which needs the mode as typed. + let diag_args = uucore::diagnostics::capture(&raw_args); let matches = - uucore::clap_localization::handle_clap_result_with_exit_code(uu_app(), args, 125)?; - - let options = - ProgramOptions::try_from(&matches).map_err(|e| UUsageError::new(125, e.to_string()))?; + uucore::clap_localization::handle_clap_result_with_exit_code(uu_app(), raw_args, 125)?; + + let options = ProgramOptions::try_from(&matches).map_err(|e| { + let message = e.to_string(); + uucore::diagnostics::error_after_report( + diag_args.as_deref(), + UUsageError::new(125, message.clone()), + |args, _| match &e { + ProgramOptionsError::InvalidMode(mode) => { + mode.error + .render_size_value(args, &mode.option, 0, &message) + } + // The rest is not about a mode that failed to parse, so there + // is nothing to point a caret at. + _ => false, + }, + ) + })?; let mut command_values = matches .get_many::(options::COMMAND) diff --git a/src/uu/tail/src/args.rs b/src/uu/tail/src/args.rs index 25a1f1dab7d..6f10f7c7041 100644 --- a/src/uu/tail/src/args.rs +++ b/src/uu/tail/src/args.rs @@ -12,6 +12,7 @@ use same_file::Handle; use std::ffi::OsString; use std::io::{IsTerminal, Write}; use std::time::Duration; +use uucore::diagnostics::OptionValue; use uucore::error::{UResult, USimpleError, UUsageError}; use uucore::parser::parse_signed_num::{SignPrefix, number_offset, parse_signed_num_max}; use uucore::parser::parse_size::ParseSizeError; @@ -76,11 +77,9 @@ impl FilterMode { let raise = |message: String, arg: &str, short, long, error: &ParseSizeError| { error.size_value_error( diag_args, - arg, + &OptionValue::new(arg, short, long), // The parser never saw the sign; the caret has to count it back in. number_offset(arg), - short, - long, &message, USimpleError::new(1, message.clone()), ) diff --git a/src/uu/test/src/test.rs b/src/uu/test/src/test.rs index 965ce713c16..9743ee45bf4 100644 --- a/src/uu/test/src/test.rs +++ b/src/uu/test/src/test.rs @@ -82,12 +82,11 @@ pub fn uumain(mut args: impl uucore::Args) -> UResult<()> { match parse(args).and_then(|mut stack| eval(&mut stack)) { Ok(true) => Ok(()), Ok(false) => Err(1.into()), - Err(e) => { - let reported = expression - .as_ref() - .is_some_and(|expression| diagnostics::render(expression, &e)); - Err(uucore::error::quiet_if_reported(reported, e)) - } + Err(e) => Err(uucore::diagnostics::error_after_report( + expression.as_deref(), + e, + diagnostics::render, + )), } } diff --git a/src/uu/tr/src/tr.rs b/src/uu/tr/src/tr.rs index d92551a52f9..a6bc546fb37 100644 --- a/src/uu/tr/src/tr.rs +++ b/src/uu/tr/src/tr.rs @@ -17,7 +17,7 @@ use simd::process_input; use std::ffi::OsString; use std::io::{stdin, stdout}; use uucore::display::Quotable; -use uucore::error::{UResult, USimpleError, UUsageError, quiet_if_reported}; +use uucore::error::{UResult, USimpleError, UUsageError}; use uucore::fs::is_stdin_directory; use uucore::translate; use uucore::{format_usage, os_str_as_bytes, show}; @@ -118,10 +118,11 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let (set1, set2) = match solved { Ok(sets_solved) => sets_solved, Err(error) => { - let reported = set_args - .as_ref() - .is_some_and(|args| diagnostics::render(args, &sets, &error)); - return Err(quiet_if_reported(reported, error)); + return Err(uucore::diagnostics::error_after_report( + set_args.as_deref(), + error, + |args, error| diagnostics::render(args, &sets, error), + )); } }; diff --git a/src/uu/truncate/src/truncate.rs b/src/uu/truncate/src/truncate.rs index 2fc9928cac7..a2f6652503c 100644 --- a/src/uu/truncate/src/truncate.rs +++ b/src/uu/truncate/src/truncate.rs @@ -12,6 +12,7 @@ use std::io::ErrorKind; #[cfg(unix)] use std::os::unix::fs::FileTypeExt; use std::path::Path; +use uucore::diagnostics::OptionValue; use uucore::display::Quotable; use uucore::error::{FromIo, UResult, USimpleError, UUsageError}; use uucore::format_usage; @@ -314,12 +315,10 @@ fn truncate( let message = translate!("truncate-error-invalid-number", "error" => &error); return Err(error.size_value_error( diag_args, - string, + &OptionValue::new(string, 's', "size"), // The parser never saw the mode character; the caret has // to count it back in. size_offset(string, is_modifier), - 's', - "size", &message, USimpleError::new(1, message.clone()), )); diff --git a/src/uucore/src/lib/features/diagnostics.rs b/src/uucore/src/lib/features/diagnostics.rs index 4cf6720815f..6ae44e15cd4 100644 --- a/src/uucore/src/lib/features/diagnostics.rs +++ b/src/uucore/src/lib/features/diagnostics.rs @@ -93,7 +93,35 @@ pub fn operands(args: &[OsString]) -> Option> { capture(args.get(1..).unwrap_or_default()) } -pub use crate::features::diagnostics_boundary::{char_span, floor_boundary}; +pub use crate::features::diagnostics_boundary::{ + OptionValue, char_span, floor_boundary, list_items, +}; + +/// The error to raise for something a caret may have just explained. +/// +/// Draws the report 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. Every caret +/// diagnostic ends this way, so it is written once here. +/// +/// # Arguments +/// +/// * `diag_args` - The arguments as typed, program name included, or `None` +/// when they were not kept — as [`capture`] returns them. +/// * `error` - The error to raise when nothing was drawn. It is lent to `draw` +/// rather than moved into it, since it is usually the error the report is +/// about as well. +/// * `draw` - Draws the report against the arguments, and returns `false` when +/// it could not — because the error is not about any one of them, or because +/// none of them turned out to carry what the caret would point at. +pub fn error_after_report>>( + diag_args: Option<&[OsString]>, + error: E, + draw: impl FnOnce(&[OsString], &E) -> bool, +) -> Box { + let reported = diag_args.is_some_and(|args| draw(args, &error)); + crate::error::quiet_if_reported(reported, error) +} /// An argument list rendered as a single line, with the position of every /// argument inside it. @@ -464,6 +492,39 @@ impl Snapshot { self.render_inside_at(index, operand, range, message, label, help) } + /// Write a report pointing at `range` inside the value of an option. + /// + /// As [`Snapshot::render_option_value`], for a value that travels with the + /// option it was given to. + /// + /// # Arguments + /// + /// * `option` - The value at fault and the option it came from. + /// * `range` - Byte range inside the value to point at. An empty range + /// marks the character it starts at. + /// * `message` - The error message, already localized. + /// * `label` - Text placed under the caret, already localized, or `None` + /// for a bare underline. + /// * `help` - An optional line of advice, already localized. + pub fn render_option( + &self, + option: &OptionValue, + range: Range, + message: &str, + label: Option<&str>, + help: Option<&str>, + ) -> bool { + self.render_option_value( + &option.value, + option.short, + option.long, + range, + message, + label, + help, + ) + } + /// Byte range covered by `range` — an offset inside `operand` — within the /// argument at `index`. fn locate_at(&self, index: usize, operand: &str, range: Range) -> Option> { diff --git a/src/uucore/src/lib/features/diagnostics_boundary.rs b/src/uucore/src/lib/features/diagnostics_boundary.rs index 3548c19baa8..8d4a19fe970 100644 --- a/src/uucore/src/lib/features/diagnostics_boundary.rs +++ b/src/uucore/src/lib/features/diagnostics_boundary.rs @@ -3,12 +3,14 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -//! The character-boundary arithmetic behind the caret diagnostics. +//! The part of the caret diagnostics that is real even without them. //! //! Both [`crate::diagnostics`] and its no-op stand-in re-export these, and both -//! do so for the same reason: a caller may floor an offset before it knows -//! whether anything will be drawn, so the arithmetic has to be real even when -//! the rendering is compiled out. Keeping it here means the two cannot drift. +//! do so for the same reason: a caller locates what a caret would point at — +//! flooring an offset, walking a list, keeping a value next to the option it +//! came from — before it knows whether anything will be drawn, so that much has +//! to work even when the rendering is compiled out. Keeping it here means the +//! two cannot drift. use std::ops::Range; @@ -52,9 +54,74 @@ pub fn char_span(text: &str, offset: usize) -> Range { } } +/// The value of an option, and the option it was given to. +/// +/// An option's value can be spelled many ways — `-S 1Q`, `-S1Q`, +/// `--buffer-size=1Q` — so a caret pointing inside one has to know which option +/// carried it before it can know which argument to draw under. A utility that +/// may want a caret keeps the value and the two names together from the moment +/// the parse fails until the report is drawn. +#[derive(Debug)] +pub struct OptionValue { + /// The value as typed. + pub value: String, + /// The option's short name, if it has one. + pub short: Option, + /// The option's long name, if it has one. + pub long: Option<&'static str>, +} + +impl OptionValue { + /// The value of an option answering to both a short and a long name. + pub fn new(value: impl Into, short: char, long: &'static str) -> Self { + Self::with_names(value, Some(short), Some(long)) + } + + /// The value of an option that is missing one of the two names, or whose + /// names are only known once the parse has failed — `stat` blames `-c` or + /// `--printf` depending on which one it was given. + pub fn with_names( + value: impl Into, + short: Option, + long: Option<&'static str>, + ) -> Self { + Self { + value: value.into(), + short, + long, + } + } +} + +/// The items of a separated list, each with its byte range inside `list`. +/// +/// A caret pointing at one item of a list — a `dd` conversion flag, a `join` +/// output field — needs to know where that item was written. The list is walked +/// rather than searched for the item's text, which would also match inside an +/// earlier item the wanted one is a prefix of: the `noc` of `nocache,noc`. +/// +/// # Arguments +/// +/// * `list` - The list as typed. +/// * `separators` - The characters it is split on, of any width. +pub fn list_items<'a>( + list: &'a str, + separators: &'a [char], +) -> impl DoubleEndedIterator)> { + let base = list.as_ptr() as usize; + list.split(separators).map(move |item| { + // Every item is a slice of `list`, so its address gives away where it + // was written: no running count to keep, which would have tied the + // spans to walking the list once, in order, past separators of a width + // the count assumed. + let start = item.as_ptr() as usize - base; + (item, start..start + item.len()) + }) +} + #[cfg(test)] mod tests { - use super::{char_span, floor_boundary}; + use super::{char_span, floor_boundary, list_items}; #[test] fn floors_into_a_multibyte_character() { @@ -78,4 +145,32 @@ mod tests { fn spans_nothing_at_the_end() { assert_eq!(char_span("ab", 2), 2..2); } + + #[test] + fn spans_every_item_of_a_list() { + let items: Vec<_> = list_items("ab,,cde", &[',']).collect(); + assert_eq!(items, vec![("ab", 0..2), ("", 3..3), ("cde", 4..7)]); + } + + /// The whole point of walking: "sy" also occurs at the start of "sync". + #[test] + fn spans_an_item_an_earlier_one_starts_with() { + let items: Vec<_> = list_items("sync sy", &[' ']).collect(); + assert_eq!(items[1], ("sy", 5..7)); + } + + /// The spans are a property of the list, not of the walk: taking the items + /// out of order, or splitting on a separator that is more than one byte + /// wide, points at the same text. + #[test] + fn spans_an_item_wherever_it_is_reached() { + let items: Vec<_> = list_items("aé§bb§c", &['\u{a7}']).rev().collect(); + assert_eq!(items, vec![("c", 9..10), ("bb", 5..7), ("aé", 0..3)]); + } + + #[test] + fn spans_a_list_of_one() { + let items: Vec<_> = list_items("solo", &[',', ' ']).collect(); + assert_eq!(items, vec![("solo", 0..4)]); + } } diff --git a/src/uucore/src/lib/features/diagnostics_stub.rs b/src/uucore/src/lib/features/diagnostics_stub.rs index 857c46c1f37..3c9c839f024 100644 --- a/src/uucore/src/lib/features/diagnostics_stub.rs +++ b/src/uucore/src/lib/features/diagnostics_stub.rs @@ -33,7 +33,18 @@ pub fn operands(_args: &[OsString]) -> Option> { // an offset before it knows whether anything will be drawn. It is the one part // of this module that is not a no-op, so it is shared with the real one rather // than restated here. -pub use crate::features::diagnostics_boundary::{char_span, floor_boundary}; +pub use crate::features::diagnostics_boundary::{ + OptionValue, char_span, floor_boundary, list_items, +}; + +/// Always the error itself: nothing is ever drawn to replace it. +pub fn error_after_report>>( + _diag_args: Option<&[OsString]>, + error: E, + _draw: impl FnOnce(&[OsString], &E) -> bool, +) -> Box { + error.into() +} /// A snapshot of nothing: it finds nothing and renders nothing. /// @@ -98,6 +109,17 @@ impl Snapshot { false } + pub fn render_option( + &self, + _option: &OptionValue, + _range: Range, + _message: &str, + _label: Option<&str>, + _help: Option<&str>, + ) -> bool { + false + } + #[allow(clippy::too_many_arguments)] pub fn render_option_value( &self, diff --git a/src/uucore/src/lib/features/parser/parse_size.rs b/src/uucore/src/lib/features/parser/parse_size.rs index b30a2f99a3e..97a714a5669 100644 --- a/src/uucore/src/lib/features/parser/parse_size.rs +++ b/src/uucore/src/lib/features/parser/parse_size.rs @@ -636,7 +636,7 @@ impl ParseSizeError { } } - /// Render this error against `args`, with a caret under the part of the + /// Render this error against `snapshot`, with a caret under the part of the /// SIZE that is at fault. /// /// Every utility taking a SIZE takes the same syntax, so the label and the @@ -646,32 +646,27 @@ impl ParseSizeError { /// /// * `args` - The whole argument list, program name included — as /// [`crate::diagnostics::capture`] returns it. - /// * `operand` - The option's value as typed. It may carry something in - /// front of the size — `truncate` takes a mode character, as in `+2K`, - /// `head` and `tail` a sign — which the caret has to count but the parser - /// never saw. - /// * `size_at` - Where the size itself starts inside `operand`, zero when + /// * `option` - The option's value as typed, and the option it was given + /// to. The value may carry something in front of the size — `truncate` + /// takes a mode character, as in `+2K`, `head` and `tail` a sign — which + /// the caret has to count but the parser never saw. + /// * `size_at` - Where the size itself starts inside the value, zero when /// the whole of it is the size. - /// * `short` - The short name of the option it was given to, if it has one. - /// * `long` - Its long name, if it has one. /// * `message` - The headline, already localized. It differs between /// utilities, so it is passed in rather than built here. /// /// # Returns /// - /// `false` when no argument carries `size` as that option's value, in + /// `false` when no argument carries the value as that option's value, in /// which case the caller should fall back to the plain one-line message. - #[allow(clippy::too_many_arguments)] pub fn render_size_value( &self, args: &[std::ffi::OsString], - operand: &str, + option: &crate::diagnostics::OptionValue, size_at: usize, - short: Option, - long: Option<&str>, message: &str, ) -> bool { - let Some(size) = operand.get(size_at..) else { + let Some(size) = option.value.get(size_at..) else { return false; }; // Labelled only where a label would add to the message, per the @@ -682,10 +677,8 @@ impl ParseSizeError { Self::ParseFailure(_) | Self::PhysicalMem(_) => None, }; let span = self.span(size); - crate::diagnostics::Snapshot::with_program(args).render_option_value( - operand, - short, - long, + crate::diagnostics::Snapshot::with_program(args).render_option( + option, size_at + span.start..size_at + span.end, message, label.as_deref(), @@ -704,24 +697,19 @@ impl ParseSizeError { /// /// * `diag_args` - The arguments as typed, or `None` when they were not /// kept — as [`crate::diagnostics::capture`] returns them. - /// * `operand`, `size_at`, `short`, `long`, `message` - As for - /// [`Self::render_size_value`]. + /// * `option`, `size_at`, `message` - As for [`Self::render_size_value`]. /// * `error` - The error to raise if nothing was drawn. - #[allow(clippy::too_many_arguments)] pub fn size_value_error( &self, diag_args: Option<&[std::ffi::OsString]>, - operand: &str, + option: &crate::diagnostics::OptionValue, size_at: usize, - short: char, - long: &str, message: &str, error: impl Into>, ) -> Box { - let reported = diag_args.is_some_and(|args| { - self.render_size_value(args, operand, size_at, Some(short), Some(long), message) - }); - crate::error::quiet_if_reported(reported, error) + crate::diagnostics::error_after_report(diag_args, error, |args, _| { + self.render_size_value(args, option, size_at, message) + }) } fn size_too_big(s: &str) -> Self { diff --git a/src/uucore/src/lib/mods/clap_localization.rs b/src/uucore/src/lib/mods/clap_localization.rs index 0249089c142..8198498ed78 100644 --- a/src/uucore/src/lib/mods/clap_localization.rs +++ b/src/uucore/src/lib/mods/clap_localization.rs @@ -399,6 +399,33 @@ where handle_clap_result_with_exit_code(cmd, itr, 1) } +/// Parses the command line as [`handle_clap_result`] does, keeping a copy of +/// it for a caret diagnostic first. +/// +/// Parsing consumes the argument list, and a caret echoes it as it was typed, +/// so the copy has to be taken before — which is what this saves every caller +/// from spelling out. A utility that rewrites its arguments before parsing +/// keeps capturing on its own, since only it knows which of the two lists the +/// caret should echo. +/// +/// # Arguments +/// +/// * `cmd` - The clap `Command` to parse arguments against +/// * `args` - The command line, program name included +/// +/// # Returns +/// +/// The parsed arguments, and the command line as typed — `None` when +/// diagnostics are off, so that nothing is copied for a report no one will +/// see. +pub fn handle_clap_result_with_diagnostics( + cmd: Command, + args: Vec, +) -> UResult<(ArgMatches, Option>)> { + let diag_args = crate::diagnostics::capture(&args); + Ok((handle_clap_result(cmd, args)?, diag_args)) +} + /// Handles clap command parsing with a custom exit code for errors. /// /// Similar to `handle_clap_result` but allows specifying a custom exit code diff --git a/src/uucore/src/lib/mods/error.rs b/src/uucore/src/lib/mods/error.rs index 9ff6f57e76d..0ef0509b9f4 100644 --- a/src/uucore/src/lib/mods/error.rs +++ b/src/uucore/src/lib/mods/error.rs @@ -694,7 +694,8 @@ impl ExitCode { /// # Returns /// /// A bare [`ExitCode`] carrying `error`'s code when `reported`, and `error` -/// itself otherwise. +/// itself otherwise. An error that asks for a usage hint still gets one: the +/// hint is not part of the message the report replaced. /// /// # Examples /// @@ -707,11 +708,13 @@ impl ExitCode { /// ``` pub fn quiet_if_reported>>(reported: bool, error: E) -> Box { let error = error.into(); - if reported { - ExitCode::new(error.code()) - } else { - error + if !reported { + return error; } + if error.usage() { + return UUsageError::new(error.code(), String::new()); + } + ExitCode::new(error.code()) } impl Error for ExitCode {} @@ -833,6 +836,34 @@ impl Display for ClapErrorWrapper { #[cfg(test)] mod tests { + use super::{USimpleError, UUsageError, quiet_if_reported}; + + /// A quieted error keeps its code but says nothing: the report already did. + #[test] + fn a_reported_error_carries_only_its_code() { + let error = quiet_if_reported(true, USimpleError::new(3, "bad size".to_string())); + assert_eq!(error.code(), 3); + assert_eq!(error.to_string(), ""); + assert!(!error.usage()); + } + + /// Quieting a usage error must not swallow its "Try --help" hint, or the + /// output would depend on whether a caret happened to be drawn. + #[test] + fn a_reported_usage_error_still_asks_for_the_hint() { + let error = quiet_if_reported(true, UUsageError::new(125, "bad mode".to_string())); + assert_eq!(error.code(), 125); + assert_eq!(error.to_string(), ""); + assert!(error.usage()); + } + + #[test] + fn an_unreported_error_is_left_alone() { + let error = quiet_if_reported(false, UUsageError::new(125, "bad mode".to_string())); + assert_eq!(error.to_string(), "bad mode"); + assert!(error.usage()); + } + #[test] #[cfg(unix)] fn test_nix_error_conversion() { diff --git a/tests/by-util/test_cut.rs b/tests/by-util/test_cut.rs index be7a85a2420..6345a744295 100644 --- a/tests/by-util/test_cut.rs +++ b/tests/by-util/test_cut.rs @@ -1228,9 +1228,10 @@ mod diagnostics { .fails_with_code(1); // One item of the list is at fault, not the whole of it. - assert_eq!( - result.stderr_as_displayed(), - "\ + let stderr = result.stderr_as_displayed(); + assert!( + stderr.starts_with( + "\ cut: invalid decreasing range ╭─[ cut:1:10 ] │ @@ -1240,6 +1241,14 @@ cut: invalid decreasing range │ │ Help: a list is N, N-M, N- or -M, separated by commas, as in -f1,4-6,9- ───╯" + ), + "{stderr}" + ); + // The caret replaces the message, not the usage hint: a pipe and a + // terminal must not disagree on whether one was printed. + assert!( + stderr.ends_with("cut --help' for more information."), + "{stderr}" ); } diff --git a/tests/by-util/test_dd.rs b/tests/by-util/test_dd.rs index 0967e2fb0cf..2ab7654af2d 100644 --- a/tests/by-util/test_dd.rs +++ b/tests/by-util/test_dd.rs @@ -2255,3 +2255,121 @@ 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_keeps_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(); + + assert!(stderr.contains("dd:1:10"), "{stderr}"); + // The caret replaces the message, not the usage hint: a pipe and a + // terminal must not disagree on whether one was printed. + assert!( + stderr + .trim_end() + .ends_with("dd --help' for more information."), + "{stderr}" + ); + } + + #[test] + fn test_plain_message_keeps_the_try_help_hint_of_a_flag_message() { + new_ucmd!() + .args(&["iflag=nope"]) + .pipe_in("") + .fails_with_code(1) + .stderr_contains("dd: invalid input flag: \u{2018}nope\u{2019}") + .stderr_contains("--help' for more information."); + } + + #[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"); + } +} diff --git a/tests/by-util/test_join.rs b/tests/by-util/test_join.rs index f7934dbbf6a..16c6a6afe0a 100644 --- a/tests/by-util/test_join.rs +++ b/tests/by-util/test_join.rs @@ -2,7 +2,7 @@ // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore (words) autoformat nocheck +// spell-checker:ignore (words) autoformat nocheck FILENUM #[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "netbsd"))] use std::fs::OpenOptions; @@ -662,3 +662,52 @@ fn test_locale_collation() { .stdout_contains("abc:d 2 y") .stdout_contains("ab:d 1 x"); } + +#[cfg(all(feature = "feat_diagnostics", not(wasi_runner)))] +mod diagnostics { + use super::*; + + #[cfg(unix)] + #[test] + fn test_snippet_points_at_the_failing_field_of_a_list() { + let result = new_ucmd!() + .terminal_sim_stderr() + .args(&["-o", "1.2,2.x", "/dev/null", "/dev/null"]) + .fails_with_code(1); + + // The first field is fine; only the second one is at fault. + assert_eq!( + result.stderr_as_displayed(), + "\ +join: invalid field number: 'x' + ╭─[ join:1:13 ] + │ + 1 │ join -o 1.2,2.x /dev/null /dev/null + │ ─── + │ + │ Help: an output field is FILENUM.FIELD, as in -o 1.2,2.1; 0 stands for the join field +───╯" + ); + } + + #[cfg(unix)] + #[test] + fn test_snippet_points_inside_a_glued_short_option() { + let result = new_ucmd!() + .terminal_sim_stderr() + .args(&["-o1.2,0.4", "/dev/null", "/dev/null"]) + .fails_with_code(1); + let stderr = result.stderr_as_displayed(); + + assert!(stderr.contains("join:1:12"), "{stderr}"); + assert!(stderr.contains("invalid field specifier"), "{stderr}"); + } + + #[test] + fn test_plain_message_when_stderr_is_a_pipe() { + new_ucmd!() + .args(&["-o", "1.2,2.x", "/dev/null", "/dev/null"]) + .fails_with_code(1) + .stderr_is("join: invalid field number: 'x'\n"); + } +} diff --git a/tests/by-util/test_od.rs b/tests/by-util/test_od.rs index d260e040837..ae6ca8af401 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"); + } +} diff --git a/tests/by-util/test_seq.rs b/tests/by-util/test_seq.rs index ec3c266bc4d..7d9c48e33fd 100644 --- a/tests/by-util/test_seq.rs +++ b/tests/by-util/test_seq.rs @@ -1172,3 +1172,65 @@ fn test_equalize_widths_corner_cases() { .succeeds() .stdout_is("1.0625\n2.06252\n"); } + +#[cfg(all(feature = "feat_diagnostics", not(wasi_runner)))] +mod diagnostics { + use super::*; + + #[cfg(unix)] + #[test] + fn test_snippet_points_at_the_failing_conversion() { + let result = new_ucmd!() + .terminal_sim_stderr() + .args(&["-f", "%5.2c", "1", "3"]) + .fails_with_code(1); + + assert_eq!( + result.stderr_as_displayed(), + "\ +seq: %5.2c: invalid conversion specification + ╭─[ seq:1:8 ] + │ + 1 │ seq -f %5.2c 1 3 + │ ───── + │ + │ Help: a format holds exactly one float conversion: %f, %e, %g or %a, as in -f%.3f +───╯" + ); + } + + #[cfg(unix)] + #[test] + fn test_snippet_points_inside_a_glued_short_option() { + // `-f%q` is split for clap, but the report echoes what was typed. + let result = new_ucmd!() + .terminal_sim_stderr() + .args(&["-f%q", "1", "3"]) + .fails_with_code(1); + let stderr = result.stderr_as_displayed(); + + assert!(stderr.contains("1 │ seq -f%q 1 3"), "{stderr}"); + assert!(stderr.contains("seq:1:7"), "{stderr}"); + } + + #[cfg(unix)] + #[test] + fn test_snippet_underlines_a_format_with_no_directive() { + let result = new_ucmd!() + .terminal_sim_stderr() + .args(&["--format=abc", "1", "3"]) + .fails_with_code(1); + let stderr = result.stderr_as_displayed(); + + assert!(stderr.contains("seq:1:14"), "{stderr}"); + assert!(stderr.contains("has no % directive"), "{stderr}"); + } + + #[test] + fn test_plain_message_when_stderr_is_a_pipe() { + new_ucmd!() + .args(&["-f", "%5.2c", "1", "3"]) + .fails_with_code(1) + .stderr_is("seq: %5.2c: invalid conversion specification\n"); + } +} 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"); + } +} diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index f12850f28e2..3e9f7e3d68f 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -3361,6 +3361,30 @@ sort: invalid number at field start: invalid count at start of 'sort' ); } + #[cfg(unix)] + #[test] + fn test_snippet_points_at_the_unknown_unit_of_a_buffer_size() { + let result = new_ucmd!() + .terminal_sim_stderr() + .args(&["-S", "8zz", "/dev/null"]) + .fails_with_code(2); + + // The number parsed; only the unit did not. + assert_eq!( + result.stderr_as_displayed(), + "\ +sort: invalid suffix in --buffer-size argument '8zz' + ╭─[ sort:1:10 ] + │ + 1 │ sort -S 8zz /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 +───╯" + ); + } + #[test] fn test_plain_message_when_stderr_is_not_a_terminal() { // The test harness pipes stderr, so the report must not appear. @@ -3370,6 +3394,10 @@ sort: invalid number at field start: invalid count at start of 'sort' .stderr_only( "sort: stray character in field spec: invalid field specification '2.3q'\n", ); + new_ucmd!() + .args(&["-S", "8zz", "/dev/null"]) + .fails_with_code(2) + .stderr_only("sort: invalid suffix in --buffer-size argument '8zz'\n"); } } 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"); + } +} diff --git a/tests/by-util/test_stat.rs b/tests/by-util/test_stat.rs index 6531f3379d2..67da88514a9 100644 --- a/tests/by-util/test_stat.rs +++ b/tests/by-util/test_stat.rs @@ -789,3 +789,52 @@ fn test_no_such_directory_message() { .fails_with_code(1) .stderr_is("stat: cannot statx 'a': No such file or directory\n"); } + +#[cfg(all(feature = "feat_diagnostics", not(wasi_runner)))] +mod diagnostics { + use super::*; + + #[cfg(unix)] + #[test] + fn test_snippet_points_at_the_failing_directive() { + let result = new_ucmd!() + .terminal_sim_stderr() + .args(&["-c", "%d%.3", "/dev/null"]) + .fails_with_code(1); + + // The first directive is fine; the caret takes the second one alone. + assert_eq!( + result.stderr_as_displayed(), + "\ +stat: '%.3': invalid directive + ╭─[ stat:1:11 ] + │ + 1 │ stat -c %d%.3 /dev/null + │ ─── + │ + │ Help: a directive is %[FLAGS][WIDTH][.PRECISION]LETTER, as in %-10.2s; a literal % is written %% +───╯" + ); + } + + #[cfg(unix)] + #[test] + fn test_snippet_points_inside_a_printf_format() { + let result = new_ucmd!() + .terminal_sim_stderr() + .args(&["--printf=%12", "/dev/null"]) + .fails_with_code(1); + let stderr = result.stderr_as_displayed(); + + assert!(stderr.contains("stat:1:15"), "{stderr}"); + assert!(stderr.contains("'%12': invalid directive"), "{stderr}"); + } + + #[test] + fn test_plain_message_when_stderr_is_a_pipe() { + new_ucmd!() + .args(&["-c", "%d%.3", "/dev/null"]) + .fails_with_code(1) + .stderr_is("stat: '%.3': invalid directive\n"); + } +} diff --git a/tests/by-util/test_stdbuf.rs b/tests/by-util/test_stdbuf.rs index cbfd4e9694c..fe398f84adc 100644 --- a/tests/by-util/test_stdbuf.rs +++ b/tests/by-util/test_stdbuf.rs @@ -433,3 +433,62 @@ fn test_stdbuf_no_fork_regression() { child.kill().ok(); child.wait().ok(); } + +#[cfg(unix)] +#[cfg(all(feature = "feat_diagnostics", not(wasi_runner)))] +mod diagnostics { + use super::*; + + #[test] + fn test_snippet_points_at_the_unknown_unit() { + let result = new_ucmd!() + .terminal_sim_stderr() + .args(&["-o", "6pq", "head"]) + .fails_with_code(125); + + // The number parsed; only the unit did not. + let stderr = result.stderr_as_displayed(); + assert!( + stderr.starts_with( + "\ +stdbuf: invalid mode '6pq' + ╭─[ stdbuf:1:12 ] + │ + 1 │ stdbuf -o 6pq head + │ ─┬ + │ ╰── 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 +───╯" + ), + "{stderr}" + ); + // The caret replaces the message, not the usage hint: a pipe and a + // terminal must not disagree on whether one was printed. + assert!( + stderr.ends_with("stdbuf --help' for more information."), + "{stderr}" + ); + } + + #[test] + fn test_snippet_points_inside_a_long_option_value() { + let result = new_ucmd!() + .terminal_sim_stderr() + .args(&["--error=pq", "head"]) + .fails_with_code(125); + let stderr = result.stderr_as_displayed(); + + // Nothing usable was read, so the whole value is underlined. + assert!(stderr.contains("stdbuf:1:16"), "{stderr}"); + assert!(!stderr.contains("not a known unit"), "{stderr}"); + } + + #[test] + fn test_plain_message_when_stderr_is_a_pipe() { + new_ucmd!() + .args(&["-o", "6pq", "head"]) + .fails_with_code(125) + .usage_error("invalid mode '6pq'"); + } +}