From ce4cf44f8e6e2504a74fea9ee0a1fd48b11f0092 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 28 Jun 2026 21:05:00 +0200 Subject: [PATCH] cut: match GNU diagnostics for invalid ranges and options Produce GNU's mode-aware messages (fields vs byte/character positions), distinguish empty/invalid/decreasing/no-endpoint ranges, and align the usage errors, all with the 'Try --help' hint. --- src/uu/cut/locales/en-US.ftl | 21 ++-- src/uu/cut/locales/fr-FR.ftl | 21 ++-- src/uu/cut/src/cut.rs | 141 +++++++++++++++++++++++--- src/uucore/src/lib/features/ranges.rs | 2 +- tests/by-util/test_cut.rs | 95 +++++++++++++++-- 5 files changed, 245 insertions(+), 35 deletions(-) diff --git a/src/uu/cut/locales/en-US.ftl b/src/uu/cut/locales/en-US.ftl index e2a9bce2a1a..0836378216a 100644 --- a/src/uu/cut/locales/en-US.ftl +++ b/src/uu/cut/locales/en-US.ftl @@ -106,10 +106,19 @@ cut-help-no-partial = with -b, don't output partial multi-byte characters # Error messages cut-error-is-directory = Is a directory cut-error-write-error = write error -cut-error-delimiter-and-whitespace-conflict = invalid input: Only one of --delimiter (-d) or -w option can be specified +cut-error-delimiter-and-whitespace-conflict = -d and -w are mutually exclusive cut-error-delimiter-must-be-single-character = the delimiter must be a single character -cut-error-multiple-mode-args = invalid usage: expects no more than one of --fields (-f), --chars (-c) or --bytes (-b) -cut-error-missing-mode-arg = invalid usage: expects one of --fields (-f), --chars (-c) or --bytes (-b) -cut-error-delimiter-only-with-fields = invalid input: The '--delimiter' ('-d') option can only be used when printing a sequence of fields -cut-error-whitespace-only-with-fields = invalid input: The '-w' option can only be used when printing a sequence of fields -cut-error-only-delimited-only-with-fields = invalid input: The '--only-delimited' ('-s') option can only be used when printing a sequence of fields +cut-error-multiple-mode-args = only one list may be specified +cut-error-missing-mode-arg = you must specify a list of bytes, characters, or fields +cut-error-delimiter-only-with-fields = an input delimiter makes sense{ "\u000A\u0009" }only when operating on fields +cut-error-only-delimited-only-with-fields = suppressing non-delimited lines makes sense{ "\u000A\u0009" }only when operating on fields +cut-error-field-numbered-from-1 = fields are numbered from 1 +cut-error-position-numbered-from-1 = byte/character positions are numbered from 1 +cut-error-invalid-field-range = invalid field range +cut-error-invalid-position-range = invalid byte or character range +cut-error-invalid-decreasing-range = invalid decreasing range +cut-error-invalid-range-no-endpoint = invalid range with no endpoint: { $range } +cut-error-invalid-field-value = invalid field value { $value } +cut-error-invalid-position-value = invalid byte/character position { $value } +cut-error-field-number-too-large = field number { $value } is too large +cut-error-position-too-large = byte/character offset { $value } is too large diff --git a/src/uu/cut/locales/fr-FR.ftl b/src/uu/cut/locales/fr-FR.ftl index be73c47b0ac..a5bbab45885 100644 --- a/src/uu/cut/locales/fr-FR.ftl +++ b/src/uu/cut/locales/fr-FR.ftl @@ -106,10 +106,19 @@ cut-help-no-partial = avec -b, ne pas afficher les caractères multi-octets part # Messages d'erreur cut-error-is-directory = Est un répertoire cut-error-write-error = erreur d'écriture -cut-error-delimiter-and-whitespace-conflict = entrée invalide : Seulement une des options --delimiter (-d) ou -w peut être spécifiée +cut-error-delimiter-and-whitespace-conflict = -d et -w sont mutuellement exclusifs cut-error-delimiter-must-be-single-character = le délimiteur doit être un caractère unique -cut-error-multiple-mode-args = usage invalide : attend au plus une des options --fields (-f), --chars (-c) ou --bytes (-b) -cut-error-missing-mode-arg = usage invalide : attend une des options --fields (-f), --chars (-c) ou --bytes (-b) -cut-error-delimiter-only-with-fields = entrée invalide : L'option '--delimiter' ('-d') n'est utilisable que si on affiche une séquence de champs -cut-error-whitespace-only-with-fields = entrée invalide : L'option '-w' n'est utilisable que si on affiche une séquence de champs -cut-error-only-delimited-only-with-fields = entrée invalide : L'option '--only-delimited' ('-s') n'est utilisable que si on affiche une séquence de champs +cut-error-multiple-mode-args = une seule liste peut être spécifiée +cut-error-missing-mode-arg = vous devez spécifier une liste d'octets, de caractères ou de champs +cut-error-delimiter-only-with-fields = un délimiteur d'entrée n'a de sens{ "\u000A\u0009" }que lors d'opérations sur des champs +cut-error-only-delimited-only-with-fields = supprimer les lignes sans délimiteur n'a de sens{ "\u000A\u0009" }que lors d'opérations sur des champs +cut-error-field-numbered-from-1 = les champs sont numérotés à partir de 1 +cut-error-position-numbered-from-1 = les positions d'octet/caractère sont numérotées à partir de 1 +cut-error-invalid-field-range = plage de champs invalide +cut-error-invalid-position-range = plage d'octets ou de caractères invalide +cut-error-invalid-decreasing-range = plage décroissante invalide +cut-error-invalid-range-no-endpoint = plage invalide sans borne : { $range } +cut-error-invalid-field-value = valeur de champ invalide { $value } +cut-error-invalid-position-value = position d'octet/caractère invalide { $value } +cut-error-field-number-too-large = le numéro de champ { $value } est trop grand +cut-error-position-too-large = le décalage d'octet/caractère { $value } est trop grand diff --git a/src/uu/cut/src/cut.rs b/src/uu/cut/src/cut.rs index 2a939729f3b..f46808bf1f4 100644 --- a/src/uu/cut/src/cut.rs +++ b/src/uu/cut/src/cut.rs @@ -12,7 +12,7 @@ use std::fs::File; use std::io::{BufRead, BufReader, BufWriter, IsTerminal, Read, Write, stdin, stdout}; use std::path::Path; use uucore::display::Quotable; -use uucore::error::{FromIo, UResult, USimpleError, set_exit_code}; +use uucore::error::{FromIo, UResult, USimpleError, UUsageError, set_exit_code}; use uucore::i18n::charmap::{Encoding, locale_encoding, mb_char_len}; use uucore::line_ending::LineEnding; use uucore::os_str_as_bytes; @@ -62,12 +62,127 @@ impl<'a> From<&'a OsString> for Delimiter<'a> { } } -fn list_to_ranges(list: &str, complement: bool) -> Result, String> { - if complement { - Range::from_list(list).map(|r| uucore::ranges::complement(&r)) +/// GNU's range-list diagnostics, worded for field mode (`-f`/`-F`) or for +/// byte/character mode (`-b`/`-c`). +struct RangeErrors { + numbered_from_1: &'static str, + invalid_range: &'static str, + invalid_value: &'static str, + too_large: &'static str, +} + +const FIELD_ERRORS: RangeErrors = RangeErrors { + numbered_from_1: "cut-error-field-numbered-from-1", + invalid_range: "cut-error-invalid-field-range", + invalid_value: "cut-error-invalid-field-value", + too_large: "cut-error-field-number-too-large", +}; + +const POSITION_ERRORS: RangeErrors = RangeErrors { + numbered_from_1: "cut-error-position-numbered-from-1", + invalid_range: "cut-error-invalid-position-range", + invalid_value: "cut-error-invalid-position-value", + too_large: "cut-error-position-too-large", +}; + +/// Fill in a message that names the offending text. Quoting it also keeps the +/// number away from Fluent's numeric formatting. +fn about(key: &str, text: &str) -> String { + translate!(key, "value" => text.quote()) +} + +/// Split the leading run of ASCII digits off `s`, returning it along with the +/// rest of `s`. The run is empty when `s` does not start with a digit. +fn split_digits(s: &str) -> (&str, &str) { + s.split_at(s.find(|c: char| !c.is_ascii_digit()).unwrap_or(s.len())) +} + +/// Parse one endpoint of a range, `default` standing in for an omitted one. +/// GNU rejects `usize::MAX` itself, so the highest value taken is `MAX - 1`. +fn parse_endpoint(digits: &str, default: usize, errors: &RangeErrors) -> Result { + if digits.is_empty() { + return Ok(default); + } + match digits.parse::() { + Ok(n) if n != usize::MAX => Ok(n), + _ => Err(about(errors.too_large, digits)), + } +} + +/// Parse a single `N`, `N-`, `-N` or `N-M` item into a range. +fn parse_range(item: &str, errors: &RangeErrors) -> Result { + let (low_digits, rest) = split_digits(item); + // GNU reports everything it could not consume, not just the first byte. + if low_digits.is_empty() && !rest.starts_with('-') { + return Err(about(errors.invalid_value, item)); + } + let Some(high_part) = rest.strip_prefix('-') else { + // No dash at all: a lone number, or trailing junk after it. + if !rest.is_empty() { + return Err(about(errors.invalid_value, rest)); + } + let n = parse_endpoint(low_digits, 0, errors)?; + return Ok(Range { low: n, high: n }); + }; + + let (high_digits, tail) = split_digits(high_part); + if !tail.is_empty() { + // A second dash makes the item a malformed range, anything else a bad + // value. + return Err(if tail.starts_with('-') { + translate!(errors.invalid_range) + } else { + about(errors.invalid_value, tail) + }); + } + if low_digits.is_empty() && high_digits.is_empty() { + return Err(translate!("cut-error-invalid-range-no-endpoint", "range" => item)); + } + + let low = parse_endpoint(low_digits, 1, errors)?; + let high = parse_endpoint(high_digits, usize::MAX - 1, errors)?; + if low > high { + return Err(translate!("cut-error-invalid-decreasing-range")); + } + Ok(Range { low, high }) +} + +/// Parse a range list, reporting GNU's exact diagnostics. +/// +/// GNU distinguishes its messages by mode and by the kind of problem, which the +/// shared [`Range::from_list`] parser does not, so the list is parsed here and +/// the resulting ranges are merged directly. +fn parse_range_list(list: &str, is_field: bool) -> Result, String> { + let errors = if is_field { + &FIELD_ERRORS } else { - Range::from_list(list) + &POSITION_ERRORS + }; + let mut ranges = Vec::new(); + + for item in list.split([',', ' ']) { + // A stray separator leaves an empty item, which GNU reads as position + // zero -- the same complaint as an explicit `0`. + if item.is_empty() { + return Err(translate!(errors.numbered_from_1)); + } + let range = parse_range(item, errors)?; + if range.low == 0 { + return Err(translate!(errors.numbered_from_1)); + } + ranges.push(range); } + + Ok(Range::merge(ranges)) +} + +fn list_to_ranges(list: &str, complement: bool, is_field: bool) -> Result, String> { + let ranges = parse_range_list(list, is_field)?; + Ok(if complement { + uucore::ranges::complement(&ranges) + } else { + ranges + }) } /// Write the parts of `line` selected by `ranges`, treating every byte as a @@ -683,7 +798,7 @@ fn get_delimiters(matches: &ArgMatches) -> UResult<(Delimiter<'_>, Option<&[u8]> let delim_opt = matches.get_one::(options::DELIMITER); let delim = match delim_opt { Some(_) if whitespace_delimited => { - return Err(USimpleError::new( + return Err(UUsageError::new( 1, translate!("cut-error-delimiter-and-whitespace-conflict"), )); @@ -700,7 +815,7 @@ fn get_delimiters(matches: &ArgMatches) -> UResult<(Delimiter<'_>, Option<&[u8]> let single_utf8_char = os_string.to_str().is_some_and(|s| s.chars().count() == 1); let single_locale_char = mb_char_len(bytes) == bytes.len(); if !single_utf8_char && !single_locale_char { - return Err(USimpleError::new( + return Err(UUsageError::new( 1, translate!("cut-error-delimiter-must-be-single-character"), )); @@ -769,7 +884,8 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let list = matches .get_one::(mode_arg) .expect("should be ensured by get_mode_arg"); - let ranges = list_to_ranges(list, complement).map_err(|e| USimpleError::new(1, e))?; + let ranges = list_to_ranges(list, complement, mode_arg == options::FIELDS) + .map_err(|e| UUsageError::new(1, e))?; let mode = match mode_arg { options::BYTES => Mode::Bytes( @@ -829,13 +945,13 @@ fn get_mode_arg(matches: &ArgMatches) -> UResult<&str> { let mode_arg = match mode_args_and_counts.as_slice() { [(arg, 1)] => *arg, [] => { - return Err(USimpleError::new( + return Err(UUsageError::new( 1, translate!("cut-error-missing-mode-arg"), )); } _ => { - return Err(USimpleError::new( + return Err(UUsageError::new( 1, translate!("cut-error-multiple-mode-args"), )); @@ -849,8 +965,9 @@ fn get_mode_arg(matches: &ArgMatches) -> UResult<&str> { "cut-error-delimiter-only-with-fields", ), ( + // GNU words `-w` as a delimiter too, so it shares the message. matches.get_flag(options::WHITESPACE_DELIMITED), - "cut-error-whitespace-only-with-fields", + "cut-error-delimiter-only-with-fields", ), ( matches.get_flag(options::ONLY_DELIMITED), @@ -860,7 +977,7 @@ fn get_mode_arg(matches: &ArgMatches) -> UResult<&str> { for (is_triggered, msg_key) in checks { if is_triggered { - return Err(USimpleError::new(1, translate!(msg_key))); + return Err(UUsageError::new(1, translate!(msg_key))); } } } diff --git a/src/uucore/src/lib/features/ranges.rs b/src/uucore/src/lib/features/ranges.rs index 0797a3bd5ec..777cc5be03f 100644 --- a/src/uucore/src/lib/features/ranges.rs +++ b/src/uucore/src/lib/features/ranges.rs @@ -94,7 +94,7 @@ impl Range { /// Merge any overlapping ranges. Adjacent ranges are *NOT* merged. /// /// Is guaranteed to return only disjoint ranges in a sorted order. - fn merge(mut ranges: Vec) -> Vec { + pub fn merge(mut ranges: Vec) -> Vec { ranges.sort(); // merge overlapping ranges diff --git a/tests/by-util/test_cut.rs b/tests/by-util/test_cut.rs index 85ee9bf6937..adc03ea79f2 100644 --- a/tests/by-util/test_cut.rs +++ b/tests/by-util/test_cut.rs @@ -45,9 +45,9 @@ const COMPLEX_SEQUENCE: &str = "9-,6-7,-2,4"; #[test] fn test_no_args() { - new_ucmd!().fails().stderr_is( - "cut: invalid usage: expects one of --fields (-f), --chars (-c) or --bytes (-b)\n", - ); + new_ucmd!() + .fails() + .stderr_contains("cut: you must specify a list of bytes, characters, or fields"); } #[test] @@ -55,6 +55,77 @@ fn test_invalid_arg() { new_ucmd!().arg("--definitely-invalid").fails_with_code(1); } +#[test] +fn test_range_error_messages() { + // Mode-aware diagnostics for invalid ranges. + let cases: &[(&[&str], &str)] = &[ + ( + &["-c0"], + "cut: byte/character positions are numbered from 1", + ), + ( + &["-b0-7"], + "cut: byte/character positions are numbered from 1", + ), + (&["-f0-9"], "cut: fields are numbered from 1"), + (&["-f", ""], "cut: fields are numbered from 1"), + ( + &["-c", ""], + "cut: byte/character positions are numbered from 1", + ), + (&["-f", "q"], "cut: invalid field value 'q'"), + (&["-c", "zz"], "cut: invalid byte/character position 'zz'"), + (&["-f", "9-4"], "cut: invalid decreasing range"), + (&["-c", "-"], "cut: invalid range with no endpoint: -"), + (&["-f", "8,-"], "cut: invalid range with no endpoint: -"), + // The offending text is what could not be consumed, not the whole item. + (&["-f", "7k"], "cut: invalid field value 'k'"), + (&["-f", "4-w2"], "cut: invalid field value 'w2'"), + ( + &["-c", "3x-5"], + "cut: invalid byte/character position 'x-5'", + ), + // A leading sign is not part of a number here. + (&["-f", "+6"], "cut: invalid field value '+6'"), + // A second dash makes it a malformed range instead. + (&["-f", "2-5-8"], "cut: invalid field range"), + (&["-c", "3--6"], "cut: invalid byte or character range"), + // `usize::MAX` itself is rejected, and so is anything above it. + ( + &["-f", "18446744073709551615"], + "cut: field number '18446744073709551615' is too large", + ), + ( + &["-c", "4-77777777777777777777777"], + "cut: byte/character offset '77777777777777777777777' is too large", + ), + ]; + for (args, expected) in cases { + new_ucmd!() + .args(args) + .fails_with_code(1) + .stderr_contains(*expected); + } +} + +#[test] +fn test_field_only_options_without_fields() { + new_ucmd!() + .args(&["-s", "-c7"]) + .fails_with_code(1) + .stderr_contains( + "cut: suppressing non-delimited lines makes sense\n\tonly when operating on fields", + ); +} + +#[test] +fn test_delimiter_and_whitespace_are_exclusive() { + new_ucmd!() + .args(&["-w", "-d,", "-f3"]) + .fails_with_code(1) + .stderr_contains("cut: -d and -w are mutually exclusive"); +} + #[test] fn test_byte_sequence() { for param in ["-b", "--bytes", "--byt"] { @@ -109,16 +180,19 @@ fn test_whitespace_with_explicit_delimiter() { #[test] fn test_whitespace_with_byte() { + // `-w` counts as an input delimiter for the purpose of this diagnostic. new_ucmd!() .args(&["-w", "-b", COMPLEX_SEQUENCE]) - .fails_with_code(1); + .fails_with_code(1) + .stderr_contains("cut: an input delimiter makes sense\n\tonly when operating on fields"); } #[test] fn test_whitespace_with_char() { new_ucmd!() .args(&["-c", COMPLEX_SEQUENCE, "-w"]) - .fails_with_code(1); + .fails_with_code(1) + .stderr_contains("cut: an input delimiter makes sense\n\tonly when operating on fields"); } #[test] @@ -127,8 +201,9 @@ fn test_delimiter_with_byte_and_char() { new_ucmd!() .args(&[conflicting_arg, COMPLEX_SEQUENCE, "-d="]) .fails_with_code(1) - .stderr_is("cut: invalid input: The '--delimiter' ('-d') option can only be used when printing a sequence of fields\n") -; + .stderr_contains( + "cut: an input delimiter makes sense\n\tonly when operating on fields", + ); } } @@ -562,9 +637,9 @@ fn test_multiple_mode_args() { vec!["-b1", "-c2", "-f3"], ] { new_ucmd!() - .args(&args) - .fails() - .stderr_is("cut: invalid usage: expects no more than one of --fields (-f), --chars (-c) or --bytes (-b)\n"); + .args(&args) + .fails() + .stderr_contains("cut: only one list may be specified"); } }