From f41250acfbb5ff82b1e7f48b5651d7c93dc3d9d6 Mon Sep 17 00:00:00 2001 From: NdekoCode Date: Thu, 8 Oct 2026 00:15:28 +0200 Subject: [PATCH 1/2] pr: report invalid numeric options and page ranges as GNU does The value of -l, -w, -W, -N and -o was dropped by the legacy operand scan when it looked like one: `pr -l -1 f` read `-1` as a column count and `f` as the page length, and `pr -l +5 f` took `+5` for a page range. A page range that was not one (`--pages=0`, `2:1`) was reported as a bad argument, a `+` operand that was not a range was taken for a file, and a zero or negative length, width or column count got a message of its own. Keep the value of an option out of the operand scan, and let clap take a value that starts with `-`. Report the ranges and numbers the way GNU 9.12 does: `invalid page range '0'`, `--pages argument '...' too large`, `invalid + argument 'x'`, `invalid suffix in + argument '1:3x'`, and for the numbers `'-l PAGE_LENGTH' invalid number of lines: '0': Numerical result out of range`. A `+` operand that GNU takes for a file name, digits followed by other characters or a range that is not one, stays a file name. --- src/uu/pr/locales/en-US.ftl | 2 +- src/uu/pr/locales/fr-FR.ftl | 2 +- src/uu/pr/src/pr.rs | 349 +++++++++++++++++++----------------- tests/by-util/test_pr.rs | 110 ++++++++++-- 4 files changed, 283 insertions(+), 180 deletions(-) diff --git a/src/uu/pr/locales/en-US.ftl b/src/uu/pr/locales/en-US.ftl index ff9b341955a..14e1006d023 100644 --- a/src/uu/pr/locales/en-US.ftl +++ b/src/uu/pr/locales/en-US.ftl @@ -100,7 +100,7 @@ pr-error-socket-not-supported = pr: cannot open {$file}, Operation not supported pr-error-no-such-file = pr: cannot open {$file}, No such file or directory pr-error-column-merge-conflict = cannot specify number of columns when printing in parallel pr-error-across-merge-conflict = cannot specify both printing across and printing in parallel -pr-error-invalid-pages-range = invalid --pages argument '{$start}:{$end}' +pr-error-invalid-pages-range = invalid page range {$range} pr-error-starting-page-exceeds-page-count = starting page number {$start} exceeds page count {$count} pr-error-invalid-expand-tab-argument ='-e' extra characters or invalid number in the argument: ‘{$arg}’ pr-error-invalid-number-argument ='-n' extra characters or invalid number in the argument: ‘{$arg}’ diff --git a/src/uu/pr/locales/fr-FR.ftl b/src/uu/pr/locales/fr-FR.ftl index 116088df38b..9e38ebe770c 100644 --- a/src/uu/pr/locales/fr-FR.ftl +++ b/src/uu/pr/locales/fr-FR.ftl @@ -99,7 +99,7 @@ pr-error-socket-not-supported = pr : impossible d'ouvrir {$file}, Opération non pr-error-no-such-file = pr : impossible d'ouvrir {$file}, Aucun fichier ou répertoire de ce type pr-error-column-merge-conflict = impossible de spécifier le nombre de colonnes lors de l'impression en parallèle pr-error-across-merge-conflict = impossible de spécifier à la fois l'impression transversale et l'impression en parallèle -pr-error-invalid-pages-range = argument --pages invalide '{$start}:{$end}' +pr-error-invalid-pages-range = plage de pages invalide {$range} pr-error-starting-page-exceeds-page-count = le numéro de page de début {$start} dépasse le nombre de pages {$count} pr-error-invalid-expand-tab-argument = Caractères supplémentaires ou nombre invalide dans l'argument de '-e': '{$arg}' pr-error-invalid-number-argument = Caractères supplémentaires ou nombre invalide dans l'argument de '-n': '{$arg}' diff --git a/src/uu/pr/src/pr.rs b/src/uu/pr/src/pr.rs index 5e92d444ba4..20f261bce8d 100644 --- a/src/uu/pr/src/pr.rs +++ b/src/uu/pr/src/pr.rs @@ -244,6 +244,7 @@ pub fn uu_app() -> Command { .short('N') .long(options::FIRST_LINE_NUMBER) .help(translate!("pr-help-first-line-number")) + .allow_hyphen_values(true) .value_name("NUMBER"), ) .arg( @@ -265,6 +266,7 @@ pub fn uu_app() -> Command { .short('l') .long(options::PAGE_LENGTH) .help(translate!("pr-help-page-length")) + .allow_hyphen_values(true) .value_name("PAGE_LENGTH"), ) .arg( @@ -287,6 +289,7 @@ pub fn uu_app() -> Command { .short('w') .long(options::COLUMN_WIDTH) .help(translate!("pr-help-column-width")) + .allow_hyphen_values(true) .value_name("width"), ) .arg( @@ -294,6 +297,7 @@ pub fn uu_app() -> Command { .short('W') .long(options::PAGE_WIDTH) .help(translate!("pr-help-page-width")) + .allow_hyphen_values(true) .value_name("width"), ) .arg( @@ -346,6 +350,7 @@ pub fn uu_app() -> Command { .short('o') .long(options::INDENT) .help(translate!("pr-help-indent")) + .allow_hyphen_values(true) .value_name("margin"), ) .arg( @@ -459,6 +464,7 @@ fn recreate_arguments(args: &[OsString]) -> Vec { // Remove only whole-token legacy operands before clap parsing. let mut past_terminator = false; + let mut value_follows = false; arguments .into_iter() .filter(|arg| { @@ -469,13 +475,41 @@ fn recreate_arguments(args: &[OsString]) -> Vec { past_terminator = true; return true; } - arg.to_str().is_none_or(|arg| { - as_column_operand(arg).is_none() && as_page_operand(arg).is_none() - }) + let is_value = value_follows; + value_follows = takes_next_argument(arg); + is_value + || arg.to_str().is_none_or(|arg| { + as_column_operand(arg).is_none() && as_page_operand(arg).is_none() + }) }) .collect() } +/// Whether `arg` is an option whose value is the next argument, which is +/// then not an operand however it looks (`-l -1`, `-w +5`). +fn takes_next_argument(arg: &OsStr) -> bool { + matches!( + arg.to_str(), + Some( + "-l" | "--length" + | "-w" + | "--width" + | "-W" + | "--page-width" + | "-N" + | "--first-line-number" + | "-o" + | "--indent" + | "-h" + | "--header" + | "-D" + | "--date-format" + | "--columns" + | "--pages" + ) + ) +} + #[derive(Default)] struct ColumnPageOperands { column: Option, @@ -485,9 +519,12 @@ struct ColumnPageOperands { /// Extract legacy `-COLUMN` and `+FIRST[:LAST]` operands before `--`. fn parse_column_page_operands(args: &[OsString]) -> ColumnPageOperands { let mut operands = ColumnPageOperands::default(); - for arg in args { - if arg == "--" { - break; + let mut value_follows = false; + for arg in args.iter().take_while(|arg| *arg != "--") { + let is_value = value_follows; + value_follows = takes_next_argument(arg); + if is_value { + continue; } let Some(arg) = arg.to_str() else { continue; @@ -514,14 +551,31 @@ fn as_column_operand(arg: &str) -> Option<&str> { } /// Return the range from a whole-token `+FIRST[:LAST]` operand. +/// +/// GNU takes two forms for file names instead: digits followed by other +/// characters with no `:`, and numbers that do not make a page range (a 0, +/// or a last page before the first). Everything else that starts with `+` +/// is a page operand, valid or not. fn as_page_operand(arg: &str) -> Option<&str> { let spec = arg.strip_prefix('+')?; let is_digits = |s: &str| !s.is_empty() && s.bytes().all(|b| b.is_ascii_digit()); - let valid = match spec.split_once(':') { - Some((first, last)) => is_digits(first) && is_digits(last), - None => is_digits(spec), + let (first, last) = match spec.split_once(':') { + Some((first, last)) => (first, Some(last)), + None => (spec, None), }; - valid.then_some(spec) + if last.is_none() && first.starts_with(|c: char| c.is_ascii_digit()) && !is_digits(first) { + return None; + } + if is_digits(first) && last.is_none_or(is_digits) { + let first = first.parse::(); + let last = last.map(str::parse::); + match (first, last) { + (Ok(0), _) | (Ok(_), Some(Ok(0))) => return None, + (Ok(first), Some(Ok(last))) if last < first => return None, + _ => (), + } + } + Some(spec) } fn print_error(matches: &ArgMatches, err: &PrError) { @@ -537,29 +591,105 @@ fn value_too_large(option_message: &str, raw: &str) -> String { ) } +fn out_of_range(option_message: &str, raw: &str) -> String { + format!( + "{option_message}: {}: Numerical result out of range", + raw.quote() + ) +} + +/// Parse the value of a numeric option, reported as GNU does: a value that +/// is not a number is quoted after `option_message`, one that does not fit +/// gets the errno text. A leading `+` is accepted. +fn parse_number(raw: &str, option_message: &str) -> Result { + match raw.strip_prefix('+').unwrap_or(raw).parse::() { + Ok(n @ ..=MAX_INT_VALUE) => Ok(n), + Ok(_) => Err(PrError::EncounteredErrors { + msg: value_too_large(option_message, raw), + }), + Err(e) if *e.kind() == IntErrorKind::PosOverflow => Err(PrError::EncounteredErrors { + msg: value_too_large(option_message, raw), + }), + Err(_) => Err(PrError::EncounteredErrors { + msg: format!("{option_message}: {}", raw.quote()), + }), + } +} + +/// Like [`parse_number`], for an option that wants a number from 1 up: +/// zero and negative numbers are numbers out of range. +fn parse_positive(raw: &str, option_message: &str) -> Result { + let negative = raw + .strip_prefix('-') + .is_some_and(|digits| !digits.is_empty() && digits.bytes().all(|b| b.is_ascii_digit())); + match parse_number(raw, option_message) { + Ok(0) => Err(PrError::EncounteredErrors { + msg: out_of_range(option_message, raw), + }), + Err(_) if negative => Err(PrError::EncounteredErrors { + msg: out_of_range(option_message, raw), + }), + result => result, + } +} + fn parse_usize( matches: &ArgMatches, opt: &str, - too_large_error_message: &str, + option_message: &str, ) -> Option> { - matches.get_one::(opt).map(|i| { - let raw = i.as_str(); - match raw.parse::() { - Ok(n @ ..=MAX_INT_VALUE) => Ok(n), - Ok(_) => Err(PrError::EncounteredErrors { - msg: value_too_large(too_large_error_message, raw), - }), - Err(e) if *e.kind() == IntErrorKind::PosOverflow => Err(PrError::EncounteredErrors { - msg: value_too_large(too_large_error_message, raw), - }), - Err(_) => { - let option = format!("-{opt}"); - Err(PrError::EncounteredErrors { - msg: format!("invalid -{option} argument {}", raw.quote()), - }) - } + matches + .get_one::(opt) + .map(|raw| parse_number(raw, option_message)) +} + +fn parse_positive_option( + matches: &ArgMatches, + opt: &str, + option_message: &str, +) -> Option> { + matches + .get_one::(opt) + .map(|raw| parse_positive(raw, option_message)) +} + +/// Parse `FIRST[:LAST]` from `--pages` or a `+` operand, named by `option` +/// in the messages, as GNU reports it. +fn parse_page_range(spec: &str, option: &str) -> Result<(usize, Option), PrError> { + let error = |msg: String| PrError::EncounteredErrors { msg }; + let (first, last) = match spec.split_once(':') { + Some((first, last)) => (first, Some(last)), + None => (spec, None), + }; + let page = |part: &str| match part.parse::() { + Ok(n) => Ok(Some(n)), + Err(e) if *e.kind() == IntErrorKind::PosOverflow => Err(error(format!( + "{option} argument {} too large", + spec.quote() + ))), + Err(_) => Ok(None), + }; + let (first_page, last_page) = (page(first)?, last.map(page).transpose()?); + match (first_page, last_page) { + (Some(first), None) if first >= 1 => Ok((first, None)), + (Some(first), Some(Some(last))) if first >= 1 && last >= first => Ok((first, Some(last))), + (Some(_), None | Some(Some(_))) => Err(error( + translate!("pr-error-invalid-pages-range", "range" => spec.quote()), + )), + // A last page that is digits followed by other characters. + _ if option == "+" + && last.is_some_and(|last| { + last.starts_with(|c: char| c.is_ascii_digit()) + && !last.bytes().all(|b| b.is_ascii_digit()) + }) => + { + Err(error(format!( + "invalid suffix in {option} argument {}", + spec.quote() + ))) } - }) + _ => Err(error(format!("invalid {option} argument {}", spec.quote()))), + } } fn get_date_format(matches: &ArgMatches) -> String { @@ -775,98 +905,29 @@ fn build_options( }; // +page option is less priority than --pages - let plus_page = operands.page.as_deref(); - let res = plus_page.map(|unparsed_num| { - let x: Vec<_> = unparsed_num.split(':').collect(); - x[0].to_string() - .parse::() - .map_err(|_e| PrError::EncounteredErrors { - msg: format!("invalid {} argument {}", "+", unparsed_num.quote()), - }) - }); - let start_page_in_plus_option = match res { - Some(res) => res?, - None => 1, - }; - - let res = plus_page.filter(|i| i.contains(':')).map(|unparsed_num| { - let x: Vec<_> = unparsed_num.split(':').collect(); - x[1].to_string() - .parse::() - .map_err(|_e| PrError::EncounteredErrors { - msg: format!("invalid {} argument {}", "+", unparsed_num.quote()), - }) - }); - let end_page_in_plus_option = match res { - Some(res) => Some(res?), - None => None, - }; - - let invalid_pages_map = |i: String| { - let unparsed_value = matches.get_one::(options::PAGES).unwrap(); - let parsed_value = i.parse::().map_err(|_e| PrError::EncounteredErrors { - msg: format!("invalid --pages argument {}", unparsed_value.quote()), - }); - - match parsed_value { - Ok(0) => Err(PrError::EncounteredErrors { - msg: "invalid --pages argument '0'".to_string(), - }), - Ok(res) => Ok(res), - Err(e) => Err(e), - } - }; - - let res = matches - .get_one::(options::PAGES) - .map(|i| { - let x: Vec<_> = i.split(':').collect(); - x[0].to_string() - }) - .map(invalid_pages_map); - let start_page = match res { - Some(res) => res?, - None => start_page_in_plus_option, - }; - - let res = matches - .get_one::(options::PAGES) - .filter(|i| i.contains(':')) - .map(|i| { - let x: Vec<_> = i.split(':').collect(); - x[1].to_string() - }) - .map(invalid_pages_map); - let end_page = match res { - Some(res) => Some(res?), - None => end_page_in_plus_option, + let plus_range = operands + .page + .as_deref() + .map(|spec| parse_page_range(spec, "+")) + .transpose()?; + let (start_page, end_page) = match matches.get_one::(options::PAGES) { + Some(spec) => parse_page_range(spec, "--pages")?, + None => plus_range.unwrap_or((1, None)), }; - if let Some(end_page) = end_page.filter(|end| start_page > *end) { - return Err(PrError::EncounteredErrors { - msg: translate!("pr-error-invalid-pages-range", "start" => start_page, "end" => end_page), - }); - } - let default_lines_per_page = if form_feed_used { LINES_PER_PAGE_FOR_FORM_FEED } else { LINES_PER_PAGE }; - let page_length = parse_usize( + let page_length = parse_positive_option( matches, options::PAGE_LENGTH, "'-l PAGE_LENGTH' invalid number of lines", ) .unwrap_or(Ok(default_lines_per_page))?; - if page_length == 0 { - return Err(PrError::EncounteredErrors { - msg: "invalid --length argument '0'".to_string(), - }); - } - // `pr --help` states the rule twice: a page length of 10 or less implies // `-t`. At exactly 10 the old `<` left the header and trailer in place and // subtracted them from the page, so the page had no room for content at all. @@ -905,75 +966,33 @@ fn build_options( DEFAULT_COLUMN_WIDTH }; - let column_width = parse_usize( + let column_width = parse_positive_option( matches, options::COLUMN_WIDTH, "'-w PAGE_WIDTH' invalid number of characters", ) .unwrap_or(Ok(default_column_width))?; - if column_width == 0 { - return Err(PrError::EncounteredErrors { - msg: "invalid --width argument '0'".to_string(), - }); - } - let page_width = if matches.get_flag(options::JOIN_LINES) { None } else { - match parse_usize( + parse_positive_option( matches, options::PAGE_WIDTH, "'-W PAGE_WIDTH' invalid number of characters", - ) { - Some(res) => Some(res?), - None => None, - } - }; - - if page_width == Some(0) { - return Err(PrError::EncounteredErrors { - msg: "invalid --page-width argument '0'".to_string(), - }); - } - - let res = operands - .column - .as_deref() - .map(|unparsed_num| match unparsed_num.parse::() { - Ok(n @ ..=MAX_INT_VALUE) => Ok(n), - Ok(_) => Err(PrError::EncounteredErrors { - msg: value_too_large("invalid number of columns", unparsed_num), - }), - Err(e) if *e.kind() == IntErrorKind::PosOverflow => Err(PrError::EncounteredErrors { - msg: value_too_large("invalid number of columns", unparsed_num), - }), - Err(_) => Err(PrError::EncounteredErrors { - msg: format!("invalid {} argument {}", "-", unparsed_num.quote()), - }), - }); - let start_column_option = match res { - Some(Ok(0)) => { - return Err(PrError::EncounteredErrors { - msg: "invalid --columns argument '0'".to_string(), - }); - } - Some(res) => Some(res?), - None => None, + ) + .transpose()? }; // --columns has more priority than -column - - let column_option_value = - match parse_usize(matches, options::COLUMNS, "invalid number of columns") { - Some(Ok(0)) => { - return Err(PrError::EncounteredErrors { - msg: "invalid --columns argument '0'".to_string(), - }); - } - Some(res) => Some(res?), - None => start_column_option, - }; + let column_option_value = match matches.get_one::(options::COLUMNS) { + Some(raw) => Some(parse_positive(raw, "invalid number of columns")?), + None => operands + .column + .as_deref() + .map(|digits| parse_positive(digits, "invalid number of columns")) + .transpose()?, + }; let column_mode_options = column_option_value.map(|columns| ColumnModeOptions { columns, @@ -988,6 +1007,16 @@ fn build_options( // Store the count. Spaces are streamed at print time to avoid huge allocations. Some(raw) => match raw.parse::() { Ok(n @ 0..) => n as usize, + Ok(_) | Err(_) + if raw.strip_prefix('-').is_some_and(|digits| { + !digits.is_empty() && digits.bytes().all(|b| b.is_ascii_digit()) + }) => + { + // A negative margin, reported like one that does not fit. + return Err(PrError::EncounteredErrors { + msg: value_too_large("'-o MARGIN' invalid line offset", raw), + }); + } Err(e) if *e.kind() == IntErrorKind::PosOverflow => { return Err(PrError::EncounteredErrors { msg: value_too_large("'-o MARGIN' invalid line offset", raw), diff --git a/tests/by-util/test_pr.rs b/tests/by-util/test_pr.rs index 02fdcefee12..25cb035f696 100644 --- a/tests/by-util/test_pr.rs +++ b/tests/by-util/test_pr.rs @@ -210,7 +210,7 @@ fn test_with_valid_page_ranges() { scenario .args(&["--pages=20:5", test_file_path]) .fails() - .stderr_only("pr: invalid --pages argument '20:5'\n"); + .stderr_only("pr: invalid page range '20:5'\n"); new_ucmd!() .args(&["--pages=1:5", test_file_path]) .succeeds(); @@ -226,7 +226,7 @@ fn test_with_valid_page_ranges() { new_ucmd!() .args(&["--pages=5:1", test_file_path]) .fails() - .stderr_only("pr: invalid --pages argument '5:1'\n"); + .stderr_only("pr: invalid page range '5:1'\n"); } #[test] @@ -581,7 +581,9 @@ fn test_offset_invalid() { new_ucmd!() .args(&["--indent=-5"]) .fails_with_code(1) - .stderr_is("pr: '-o MARGIN' invalid line offset: '-5'\n"); + .stderr_is( + "pr: '-o MARGIN' invalid line offset: '-5': Value too large for defined data type\n", + ); new_ucmd!() .args(&["-o", "abc"]) @@ -1218,7 +1220,7 @@ fn test_zero_columns() { new_ucmd!() .arg("--columns=0") .fails_with_code(1) - .stderr_contains("pr: invalid --columns argument '0'"); + .stderr_contains("pr: invalid number of columns: '0': Numerical result out of range"); } #[test] @@ -1226,7 +1228,7 @@ fn test_zero_columns_shortcut() { new_ucmd!() .arg("-0") .fails_with_code(1) - .stderr_contains("pr: invalid --columns argument '0'"); + .stderr_contains("pr: invalid number of columns: '0': Numerical result out of range"); } #[test] @@ -1296,18 +1298,16 @@ fn test_zero_expand_tab_width() { #[test] fn test_zero_column_width() { - new_ucmd!() - .args(&["-w", "0"]) - .fails_with_code(1) - .stderr_is("pr: invalid --width argument '0'\n"); + new_ucmd!().args(&["-w", "0"]).fails_with_code(1).stderr_is( + "pr: '-w PAGE_WIDTH' invalid number of characters: '0': Numerical result out of range\n", + ); } #[test] fn test_zero_page_width() { - new_ucmd!() - .args(&["-W", "0"]) - .fails_with_code(1) - .stderr_is("pr: invalid --page-width argument '0'\n"); + new_ucmd!().args(&["-W", "0"]).fails_with_code(1).stderr_is( + "pr: '-W PAGE_WIDTH' invalid number of characters: '0': Numerical result out of range\n", + ); } #[test] @@ -1340,10 +1340,9 @@ fn test_page_length_eleven_keeps_header() { #[test] fn test_zero_length() { - new_ucmd!() - .args(&["-l", "0"]) - .fails_with_code(1) - .stderr_is("pr: invalid --length argument '0'\n"); + new_ucmd!().args(&["-l", "0"]).fails_with_code(1).stderr_is( + "pr: '-l PAGE_LENGTH' invalid number of lines: '0': Numerical result out of range\n", + ); } #[test] @@ -1351,7 +1350,7 @@ fn test_zero_pages() { new_ucmd!() .args(&["--pages", "0"]) .fails_with_code(1) - .stderr_is("pr: invalid --pages argument '0'\n"); + .stderr_is("pr: invalid page range '0'\n"); } #[test] @@ -1486,3 +1485,78 @@ fn test_non_utf8_input_in_columns() { .succeeds() .stdout_is_bytes(b"abcd\xff|c\n"); } + +#[test] +fn test_numeric_option_diagnostics() { + // A value that is not a number is quoted after the option; a number + // that is out of range gets the errno text, as GNU reports them. + let cases = [ + ( + &["-l", "x"][..], + "pr: '-l PAGE_LENGTH' invalid number of lines: 'x'\n", + ), + ( + &["-l", "-1"], + "pr: '-l PAGE_LENGTH' invalid number of lines: '-1': Numerical result out of range\n", + ), + ( + &["-w", "+0"], + "pr: '-w PAGE_WIDTH' invalid number of characters: '+0': Numerical result out of range\n", + ), + ( + &["-W", "x"], + "pr: '-W PAGE_WIDTH' invalid number of characters: 'x'\n", + ), + ( + &["-N", "x"], + "pr: '-N NUMBER' invalid starting line number: 'x'\n", + ), + (&["--columns=x"], "pr: invalid number of columns: 'x'\n"), + (&["--pages=1:x"], "pr: invalid --pages argument '1:x'\n"), + (&["--pages=1:0"], "pr: invalid page range '1:0'\n"), + ( + &["--pages=99999999999999999999"], + "pr: --pages argument '99999999999999999999' too large\n", + ), + (&["+x"], "pr: invalid + argument 'x'\n"), + (&["+1:"], "pr: invalid + argument '1:'\n"), + (&["+1:3x"], "pr: invalid suffix in + argument '1:3x'\n"), + ( + &["+99999999999999999999"], + "pr: + argument '99999999999999999999' too large\n", + ), + ]; + for (args, stderr) in cases { + new_ucmd!() + .args(args) + .arg("test_one_page.log") + .fails() + .stderr_only(stderr); + } +} + +#[test] +fn test_option_value_is_not_an_operand() { + // `-l -1` holds no column operand, and `-l +5` no page operand: the + // value goes with the option, however it looks. + new_ucmd!() + .args(&["-l", "+5", "-t", "test_one_page.log"]) + .succeeds() + .stdout_is_fixture("test_one_page.log"); + new_ucmd!() + .args(&["-N", "-1", "test_one_page.log"]) + .fails() + .stderr_only("pr: '-N NUMBER' invalid starting line number: '-1'\n"); +} + +#[test] +fn test_plus_operand_that_is_not_a_page_range_names_a_file() { + // GNU takes a first page of 0, a last page before the first, or digits + // followed by other characters for a file name. + for operand in ["+0", "+2:1", "+1x"] { + new_ucmd!() + .args(&[operand, "test_one_page.log"]) + .fails() + .stderr_contains("No such file or directory"); + } +} From 16e7ca0fa906942ab48ab2449a7bdbd2be60e9da Mon Sep 17 00:00:00 2001 From: NdekoCode Date: Fri, 9 Oct 2026 23:34:14 +0200 Subject: [PATCH 2/2] pr: take the value-taking options from uu_app, share the page parse The list of options whose value is the next argument duplicated what clap knows and missed a cluster (`-tl -1`) and an abbreviation (`--len -1`). Ask the built Command instead: a long option, exact or the one option a prefix fits, or the last short option of a cluster, that takes at least one value and does not require `=`. The scan runs once, on the arguments after `-e` got its attached value, so clap and the operands see the same thing. `-h` and `-D` take a value that starts with `-`, as GNU does. One parse of `FIRST[:LAST]` serves both the `+` operand check and the messages. A number is the digits up to whatever follows them, so `--pages=1x` is an invalid page range and `+1x:3` a file name, and a number too large with characters after it is an invalid suffix, as GNU reports them. The numeric diagnostics go through translate!(). Drop the xstrtol.pl wording patch from build-gnu.sh, since pr now prints what the test expects, and make the file name test independent of the OS error text. --- src/uu/pr/locales/en-US.ftl | 12 ++ src/uu/pr/locales/fr-FR.ftl | 12 ++ src/uu/pr/src/pr.rs | 365 +++++++++++++++++++----------------- tests/by-util/test_pr.rs | 44 ++++- util/build-gnu.sh | 5 - 5 files changed, 255 insertions(+), 183 deletions(-) diff --git a/src/uu/pr/locales/en-US.ftl b/src/uu/pr/locales/en-US.ftl index 14e1006d023..09e08c00dde 100644 --- a/src/uu/pr/locales/en-US.ftl +++ b/src/uu/pr/locales/en-US.ftl @@ -101,6 +101,18 @@ pr-error-no-such-file = pr: cannot open {$file}, No such file or directory pr-error-column-merge-conflict = cannot specify number of columns when printing in parallel pr-error-across-merge-conflict = cannot specify both printing across and printing in parallel pr-error-invalid-pages-range = invalid page range {$range} +pr-error-invalid-pages-argument = invalid {$option} argument {$spec} +pr-error-invalid-pages-suffix = invalid suffix in {$option} argument {$spec} +pr-error-pages-argument-too-large = {$option} argument {$spec} too large +pr-error-invalid-page-length = '-l PAGE_LENGTH' invalid number of lines +pr-error-invalid-column-width = '-w PAGE_WIDTH' invalid number of characters +pr-error-invalid-page-width = '-W PAGE_WIDTH' invalid number of characters +pr-error-invalid-first-line-number = '-N NUMBER' invalid starting line number +pr-error-invalid-number-of-columns = invalid number of columns +pr-error-invalid-line-offset = '-o MARGIN' invalid line offset +pr-error-invalid-value = {$message}: {$value} +pr-error-value-too-large = {$message}: {$value}: Value too large for defined data type +pr-error-value-out-of-range = {$message}: {$value}: Numerical result out of range pr-error-starting-page-exceeds-page-count = starting page number {$start} exceeds page count {$count} pr-error-invalid-expand-tab-argument ='-e' extra characters or invalid number in the argument: ‘{$arg}’ pr-error-invalid-number-argument ='-n' extra characters or invalid number in the argument: ‘{$arg}’ diff --git a/src/uu/pr/locales/fr-FR.ftl b/src/uu/pr/locales/fr-FR.ftl index 9e38ebe770c..bbf96e8819b 100644 --- a/src/uu/pr/locales/fr-FR.ftl +++ b/src/uu/pr/locales/fr-FR.ftl @@ -100,6 +100,18 @@ pr-error-no-such-file = pr : impossible d'ouvrir {$file}, Aucun fichier ou répe pr-error-column-merge-conflict = impossible de spécifier le nombre de colonnes lors de l'impression en parallèle pr-error-across-merge-conflict = impossible de spécifier à la fois l'impression transversale et l'impression en parallèle pr-error-invalid-pages-range = plage de pages invalide {$range} +pr-error-invalid-pages-argument = argument {$option} invalide {$spec} +pr-error-invalid-pages-suffix = suffixe invalide dans l'argument {$option} {$spec} +pr-error-pages-argument-too-large = argument {$option} {$spec} trop grand +pr-error-invalid-page-length = '-l PAGE_LENGTH' nombre de lignes invalide +pr-error-invalid-column-width = '-w PAGE_WIDTH' nombre de caractères invalide +pr-error-invalid-page-width = '-W PAGE_WIDTH' nombre de caractères invalide +pr-error-invalid-first-line-number = '-N NUMBER' numéro de ligne de départ invalide +pr-error-invalid-number-of-columns = nombre de colonnes invalide +pr-error-invalid-line-offset = '-o MARGIN' décalage de ligne invalide +pr-error-invalid-value = {$message} : {$value} +pr-error-value-too-large = {$message} : {$value} : Valeur trop grande pour le type de données défini +pr-error-value-out-of-range = {$message} : {$value} : Résultat numérique hors limites pr-error-starting-page-exceeds-page-count = le numéro de page de début {$start} dépasse le nombre de pages {$count} pr-error-invalid-expand-tab-argument = Caractères supplémentaires ou nombre invalide dans l'argument de '-e': '{$arg}' pr-error-invalid-number-argument = Caractères supplémentaires ou nombre invalide dans l'argument de '-n': '{$arg}' diff --git a/src/uu/pr/src/pr.rs b/src/uu/pr/src/pr.rs index 20f261bce8d..27e07a618c8 100644 --- a/src/uu/pr/src/pr.rs +++ b/src/uu/pr/src/pr.rs @@ -213,12 +213,14 @@ pub fn uu_app() -> Command { .short('h') .long(options::HEADER) .help(translate!("pr-help-header")) + .allow_hyphen_values(true) .value_name("STRING"), ) .arg( Arg::new(options::DATE_FORMAT) .short('D') .long(options::DATE_FORMAT) + .allow_hyphen_values(true) .value_name("FORMAT") .help(translate!("pr-help-date-format")), ) @@ -387,9 +389,12 @@ pub fn uu_app() -> Command { pub fn uumain(args: impl uucore::Args) -> UResult<()> { let args: Vec<_> = args.collect(); - let opt_args = recreate_arguments(&args); + let mut command = uu_app(); + // Settle what the options leave to clap's defaults, such as how many + // values each takes, before the arguments are read against them. + command.build(); + let (opt_args, operands) = recreate_arguments(&args, &command); - let command = uu_app(); let matches = uucore::clap_localization::handle_clap_result(command, opt_args)?; #[allow(clippy::unwrap_used, reason = "default value is set by clap")] @@ -404,8 +409,6 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { files.into_iter().map(|i| vec![i]).collect() }; - let operands = parse_column_page_operands(&args); - for file_group in file_groups { let result_options = build_options(&matches, &file_group, &operands); let options = match result_options { @@ -429,8 +432,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { Ok(()) } -/// Rewrite arguments before clap parsing, preserving legacy numeric operands. -fn recreate_arguments(args: &[OsString]) -> Vec { +/// Rewrite arguments before clap parsing, taking out the legacy numeric +/// operands that clap would not know. +fn recreate_arguments(args: &[OsString], cmd: &Command) -> (Vec, ColumnPageOperands) { let num_regex = Regex::new(r"^[^-]\d*$").unwrap(); // `-e` ends a cluster of short flags that take no value of their own, as in `-tre`. // Options that do take a value are excluded so that `-se` keeps meaning `-s e`. @@ -462,52 +466,83 @@ fn recreate_arguments(args: &[OsString]) -> Vec { arguments[pos] = format!("{}\t8", value.trim()).into(); } - // Remove only whole-token legacy operands before clap parsing. - let mut past_terminator = false; + // Take out the whole-token legacy operands before `--`, keeping the first + // of each kind. The value of an option is not one, however it looks + // (`-l -1`, `-w +5`). + let mut operands = ColumnPageOperands::default(); + let mut clap_args = Vec::with_capacity(arguments.len()); + let mut arguments = arguments.into_iter(); let mut value_follows = false; - arguments - .into_iter() - .filter(|arg| { - if past_terminator { - return true; - } - if arg == "--" { - past_terminator = true; - return true; - } - let is_value = value_follows; - value_follows = takes_next_argument(arg); - is_value - || arg.to_str().is_none_or(|arg| { - as_column_operand(arg).is_none() && as_page_operand(arg).is_none() - }) - }) - .collect() + for arg in arguments.by_ref() { + if arg == "--" { + clap_args.push(arg); + break; + } + let is_value = value_follows; + let text = arg.to_str(); + value_follows = text.is_some_and(|text| takes_next_argument(cmd, text)); + if is_value { + clap_args.push(arg); + } else if let Some(digits) = text.and_then(as_column_operand) { + operands.column.get_or_insert_with(|| digits.to_string()); + } else if let Some(spec) = text.and_then(as_page_operand) { + operands.page.get_or_insert_with(|| spec.to_string()); + } else { + clap_args.push(arg); + } + } + clap_args.extend(arguments); + (clap_args, operands) } -/// Whether `arg` is an option whose value is the next argument, which is -/// then not an operand however it looks (`-l -1`, `-w +5`). -fn takes_next_argument(arg: &OsStr) -> bool { - matches!( - arg.to_str(), - Some( - "-l" | "--length" - | "-w" - | "--width" - | "-W" - | "--page-width" - | "-N" - | "--first-line-number" - | "-o" - | "--indent" - | "-h" - | "--header" - | "-D" - | "--date-format" - | "--columns" - | "--pages" - ) - ) +/// Whether `arg` is an option whose value is the next argument, going by +/// what `cmd` knows of its options. +fn takes_next_argument(cmd: &Command, arg: &str) -> bool { + let takes_value = |opt: &Arg| { + opt.get_action().takes_values() + && opt.get_num_args().is_some_and(|n| n.min_values() >= 1) + && !opt.is_require_equals_set() + }; + if let Some(name) = arg.strip_prefix("--") { + // `--NAME=VALUE` carries its value. `--NAME` may be cut short to a + // prefix of one option only, as `uu_app()` sets `infer_long_args`, + // which clap has no getter for. + if name.is_empty() || name.contains('=') { + return false; + } + let named = |opt: &&Arg| { + opt.get_long_and_visible_aliases() + .is_some_and(|longs| longs.contains(&name)) + }; + let prefixed = |opt: &&Arg| { + opt.get_long_and_visible_aliases() + .is_some_and(|longs| longs.iter().any(|long| long.starts_with(name))) + }; + let option = cmd.get_arguments().find(named).or_else(|| { + let mut options = cmd.get_arguments().filter(prefixed); + options.next().filter(|_| options.next().is_none()) + }); + return option.is_some_and(takes_value); + } + // In a cluster of short options, only the last one can take the next + // argument: an earlier one has its value attached (`-tl5`). + let Some(cluster) = arg.strip_prefix('-') else { + return false; + }; + let mut chars = cluster.chars(); + while let Some(c) = chars.next() { + let short = |opt: &&Arg| { + opt.get_short_and_visible_aliases() + .is_some_and(|shorts| shorts.contains(&c)) + }; + let Some(opt) = cmd.get_arguments().find(short) else { + return false; + }; + if takes_value(opt) { + return chars.as_str().is_empty(); + } + } + false } #[derive(Default)] @@ -516,66 +551,70 @@ struct ColumnPageOperands { page: Option, } -/// Extract legacy `-COLUMN` and `+FIRST[:LAST]` operands before `--`. -fn parse_column_page_operands(args: &[OsString]) -> ColumnPageOperands { - let mut operands = ColumnPageOperands::default(); - let mut value_follows = false; - for arg in args.iter().take_while(|arg| *arg != "--") { - let is_value = value_follows; - value_follows = takes_next_argument(arg); - if is_value { - continue; - } - let Some(arg) = arg.to_str() else { - continue; - }; - if operands.column.is_none() - && let Some(digits) = as_column_operand(arg) - { - operands.column = Some(digits.to_string()); - continue; - } - if operands.page.is_none() - && let Some(spec) = as_page_operand(arg) - { - operands.page = Some(spec.to_string()); - } - } - operands -} - /// Return the digits from a whole-token `-COLUMN` operand. fn as_column_operand(arg: &str) -> Option<&str> { arg.strip_prefix('-') .filter(|digits| !digits.is_empty() && digits.bytes().all(|b| b.is_ascii_digit())) } -/// Return the range from a whole-token `+FIRST[:LAST]` operand. +/// Return the spec from a whole-token `+FIRST[:LAST]` operand. /// -/// GNU takes two forms for file names instead: digits followed by other -/// characters with no `:`, and numbers that do not make a page range (a 0, -/// or a last page before the first). Everything else that starts with `+` -/// is a page operand, valid or not. +/// GNU takes a spec that is not a page range for a file name, and +/// everything else that starts with `+` for a page operand, valid or not. fn as_page_operand(arg: &str) -> Option<&str> { - let spec = arg.strip_prefix('+')?; - let is_digits = |s: &str| !s.is_empty() && s.bytes().all(|b| b.is_ascii_digit()); - let (first, last) = match spec.split_once(':') { - Some((first, last)) => (first, Some(last)), - None => (spec, None), - }; - if last.is_none() && first.starts_with(|c: char| c.is_ascii_digit()) && !is_digits(first) { - return None; - } - if is_digits(first) && last.is_none_or(is_digits) { - let first = first.parse::(); - let last = last.map(str::parse::); - match (first, last) { - (Ok(0), _) | (Ok(_), Some(Ok(0))) => return None, - (Ok(first), Some(Ok(last))) if last < first => return None, - _ => (), + arg.strip_prefix('+') + .filter(|spec| !matches!(parse_page_spec(spec), Err(PageSpecError::NotARange))) +} + +/// Why a `FIRST[:LAST]` page spec gives no page range. The first three are +/// errors in a number; a spec that is not a range is a file name as a `+` +/// operand and an error as `--pages`. +enum PageSpecError { + NotANumber, + InvalidSuffix, + TooLarge, + NotARange, +} + +/// Read `FIRST[:LAST]` the way GNU does: a number is the digits up to the +/// end or to what follows them, and a page range starts at 1 and does not +/// go backwards. +fn parse_page_spec(spec: &str) -> Result<(usize, Option), PageSpecError> { + use PageSpecError::{InvalidSuffix, NotANumber, NotARange, TooLarge}; + fn number(part: &str) -> Result<(usize, &str), PageSpecError> { + let digits_end = part + .bytes() + .position(|b| !b.is_ascii_digit()) + .unwrap_or(part.len()); + let (digits, rest) = part.split_at(digits_end); + if digits.is_empty() { + return Err(NotANumber); + } + match digits.parse::() { + Ok(n) => Ok((n, rest)), + Err(_) if rest.is_empty() => Err(TooLarge), + Err(_) => Err(InvalidSuffix), } } - Some(spec) + let (first, rest) = number(spec)?; + if first == 0 { + return Err(NotARange); + } + let Some(rest) = rest.strip_prefix(':') else { + return if rest.is_empty() { + Ok((first, None)) + } else { + Err(NotARange) + }; + }; + let (last, rest) = number(rest)?; + if !rest.is_empty() { + Err(InvalidSuffix) + } else if last < first { + Err(NotARange) + } else { + Ok((first, Some(last))) + } } fn print_error(matches: &ArgMatches, err: &PrError) { @@ -585,17 +624,17 @@ fn print_error(matches: &ArgMatches, err: &PrError) { } fn value_too_large(option_message: &str, raw: &str) -> String { - format!( - "{option_message}: {}: Value too large for defined data type", - raw.quote() - ) + translate!("pr-error-value-too-large", "message" => option_message, "value" => raw.quote()) } fn out_of_range(option_message: &str, raw: &str) -> String { - format!( - "{option_message}: {}: Numerical result out of range", - raw.quote() - ) + translate!("pr-error-value-out-of-range", "message" => option_message, "value" => raw.quote()) +} + +/// Whether `raw` is a `-` followed by digits. +fn is_negative_number(raw: &str) -> bool { + raw.strip_prefix('-') + .is_some_and(|digits| !digits.is_empty() && digits.bytes().all(|b| b.is_ascii_digit())) } /// Parse the value of a numeric option, reported as GNU does: a value that @@ -611,7 +650,7 @@ fn parse_number(raw: &str, option_message: &str) -> Result { msg: value_too_large(option_message, raw), }), Err(_) => Err(PrError::EncounteredErrors { - msg: format!("{option_message}: {}", raw.quote()), + msg: translate!("pr-error-invalid-value", "message" => option_message, "value" => raw.quote()), }), } } @@ -619,14 +658,11 @@ fn parse_number(raw: &str, option_message: &str) -> Result { /// Like [`parse_number`], for an option that wants a number from 1 up: /// zero and negative numbers are numbers out of range. fn parse_positive(raw: &str, option_message: &str) -> Result { - let negative = raw - .strip_prefix('-') - .is_some_and(|digits| !digits.is_empty() && digits.bytes().all(|b| b.is_ascii_digit())); match parse_number(raw, option_message) { Ok(0) => Err(PrError::EncounteredErrors { msg: out_of_range(option_message, raw), }), - Err(_) if negative => Err(PrError::EncounteredErrors { + Err(_) if is_negative_number(raw) => Err(PrError::EncounteredErrors { msg: out_of_range(option_message, raw), }), result => result, @@ -656,40 +692,22 @@ fn parse_positive_option( /// Parse `FIRST[:LAST]` from `--pages` or a `+` operand, named by `option` /// in the messages, as GNU reports it. fn parse_page_range(spec: &str, option: &str) -> Result<(usize, Option), PrError> { - let error = |msg: String| PrError::EncounteredErrors { msg }; - let (first, last) = match spec.split_once(':') { - Some((first, last)) => (first, Some(last)), - None => (spec, None), - }; - let page = |part: &str| match part.parse::() { - Ok(n) => Ok(Some(n)), - Err(e) if *e.kind() == IntErrorKind::PosOverflow => Err(error(format!( - "{option} argument {} too large", - spec.quote() - ))), - Err(_) => Ok(None), - }; - let (first_page, last_page) = (page(first)?, last.map(page).transpose()?); - match (first_page, last_page) { - (Some(first), None) if first >= 1 => Ok((first, None)), - (Some(first), Some(Some(last))) if first >= 1 && last >= first => Ok((first, Some(last))), - (Some(_), None | Some(Some(_))) => Err(error( - translate!("pr-error-invalid-pages-range", "range" => spec.quote()), - )), - // A last page that is digits followed by other characters. - _ if option == "+" - && last.is_some_and(|last| { - last.starts_with(|c: char| c.is_ascii_digit()) - && !last.bytes().all(|b| b.is_ascii_digit()) - }) => - { - Err(error(format!( - "invalid suffix in {option} argument {}", - spec.quote() - ))) - } - _ => Err(error(format!("invalid {option} argument {}", spec.quote()))), - } + parse_page_spec(spec).map_err(|err| PrError::EncounteredErrors { + msg: match err { + PageSpecError::NotANumber => { + translate!("pr-error-invalid-pages-argument", "option" => option, "spec" => spec.quote()) + } + PageSpecError::InvalidSuffix => { + translate!("pr-error-invalid-pages-suffix", "option" => option, "spec" => spec.quote()) + } + PageSpecError::TooLarge => { + translate!("pr-error-pages-argument-too-large", "option" => option, "spec" => spec.quote()) + } + PageSpecError::NotARange => { + translate!("pr-error-invalid-pages-range", "range" => spec.quote()) + } + }, + }) } fn get_date_format(matches: &ArgMatches) -> String { @@ -754,7 +772,7 @@ fn build_options( let first_number = parse_usize( matches, options::FIRST_LINE_NUMBER, - "'-N NUMBER' invalid starting line number", + &translate!("pr-error-invalid-first-line-number"), ) .unwrap_or(Ok(default_first_number))?; @@ -924,7 +942,7 @@ fn build_options( let page_length = parse_positive_option( matches, options::PAGE_LENGTH, - "'-l PAGE_LENGTH' invalid number of lines", + &translate!("pr-error-invalid-page-length"), ) .unwrap_or(Ok(default_lines_per_page))?; @@ -969,7 +987,7 @@ fn build_options( let column_width = parse_positive_option( matches, options::COLUMN_WIDTH, - "'-w PAGE_WIDTH' invalid number of characters", + &translate!("pr-error-invalid-column-width"), ) .unwrap_or(Ok(default_column_width))?; @@ -979,18 +997,21 @@ fn build_options( parse_positive_option( matches, options::PAGE_WIDTH, - "'-W PAGE_WIDTH' invalid number of characters", + &translate!("pr-error-invalid-page-width"), ) .transpose()? }; // --columns has more priority than -column let column_option_value = match matches.get_one::(options::COLUMNS) { - Some(raw) => Some(parse_positive(raw, "invalid number of columns")?), + Some(raw) => Some(parse_positive( + raw, + &translate!("pr-error-invalid-number-of-columns"), + )?), None => operands .column .as_deref() - .map(|digits| parse_positive(digits, "invalid number of columns")) + .map(|digits| parse_positive(digits, &translate!("pr-error-invalid-number-of-columns"))) .transpose()?, }; @@ -1001,33 +1022,25 @@ fn build_options( across_mode, }); + // Store the count. Spaces are streamed at print time to avoid huge allocations. let offset_spaces = match matches.get_one::(options::INDENT) { None => 0, - // Parse as i32 to match GNU pr's behavior - // Store the count. Spaces are streamed at print time to avoid huge allocations. - Some(raw) => match raw.parse::() { - Ok(n @ 0..) => n as usize, - Ok(_) | Err(_) - if raw.strip_prefix('-').is_some_and(|digits| { - !digits.is_empty() && digits.bytes().all(|b| b.is_ascii_digit()) - }) => - { - // A negative margin, reported like one that does not fit. - return Err(PrError::EncounteredErrors { - msg: value_too_large("'-o MARGIN' invalid line offset", raw), - }); - } - Err(e) if *e.kind() == IntErrorKind::PosOverflow => { - return Err(PrError::EncounteredErrors { - msg: value_too_large("'-o MARGIN' invalid line offset", raw), - }); - } - _ => { - return Err(PrError::EncounteredErrors { - msg: format!("'-o MARGIN' invalid line offset: {}", raw.quote()), - }); + Some(raw) => { + let option_message = translate!("pr-error-invalid-line-offset"); + match parse_number(raw, &option_message) { + // A negative margin is reported like one that does not fit; + // a negative zero is zero. + Err(_) if is_negative_number(raw) => { + if raw.parse::() != Ok(0) { + return Err(PrError::EncounteredErrors { + msg: value_too_large(&option_message, raw), + }); + } + 0 + } + result => result?, } - }, + } }; let join_lines = matches.get_flag(options::JOIN_LINES); diff --git a/tests/by-util/test_pr.rs b/tests/by-util/test_pr.rs index 25cb035f696..dfb7a7eb7a0 100644 --- a/tests/by-util/test_pr.rs +++ b/tests/by-util/test_pr.rs @@ -589,6 +589,13 @@ fn test_offset_invalid() { .args(&["-o", "abc"]) .fails_with_code(1) .stderr_is("pr: '-o MARGIN' invalid line offset: 'abc'\n"); + + // A negative zero is a margin of zero, as in GNU. + new_ucmd!() + .args(&["-t", "-o", "-0"]) + .pipe_in("a\n") + .succeeds() + .stdout_only("a\n"); } #[test] @@ -1499,6 +1506,10 @@ fn test_numeric_option_diagnostics() { &["-l", "-1"], "pr: '-l PAGE_LENGTH' invalid number of lines: '-1': Numerical result out of range\n", ), + ( + &["-l", "-0"], + "pr: '-l PAGE_LENGTH' invalid number of lines: '-0': Numerical result out of range\n", + ), ( &["-w", "+0"], "pr: '-w PAGE_WIDTH' invalid number of characters: '+0': Numerical result out of range\n", @@ -1514,6 +1525,7 @@ fn test_numeric_option_diagnostics() { (&["--columns=x"], "pr: invalid number of columns: 'x'\n"), (&["--pages=1:x"], "pr: invalid --pages argument '1:x'\n"), (&["--pages=1:0"], "pr: invalid page range '1:0'\n"), + (&["--pages=1x"], "pr: invalid page range '1x'\n"), ( &["--pages=99999999999999999999"], "pr: --pages argument '99999999999999999999' too large\n", @@ -1525,6 +1537,10 @@ fn test_numeric_option_diagnostics() { &["+99999999999999999999"], "pr: + argument '99999999999999999999' too large\n", ), + ( + &["+99999999999999999999:3"], + "pr: invalid suffix in + argument '99999999999999999999:3'\n", + ), ]; for (args, stderr) in cases { new_ucmd!() @@ -1547,16 +1563,40 @@ fn test_option_value_is_not_an_operand() { .args(&["-N", "-1", "test_one_page.log"]) .fails() .stderr_only("pr: '-N NUMBER' invalid starting line number: '-1'\n"); + // The option may end a cluster, or be cut short. + for args in [&["-tl", "-1"][..], &["--len", "-1"]] { + new_ucmd!() + .args(args) + .arg("test_one_page.log") + .fails() + .stderr_only( + "pr: '-l PAGE_LENGTH' invalid number of lines: '-1': Numerical result out of range\n", + ); + } + new_ucmd!() + .args(&["-h", "-1"]) + .pipe_in("a") + .succeeds() + .stdout_contains(" -1 "); + // A value attached to the option leaves the next argument an operand, + // also for `-e`, whose value is filled in before the scan. + for args in [&["-tl1", "+2"][..], &["-e", "+2", "-l", "1"]] { + new_ucmd!() + .args(args) + .pipe_in("a\nb\nc\n") + .succeeds() + .stdout_only("b\nc\n"); + } } #[test] fn test_plus_operand_that_is_not_a_page_range_names_a_file() { // GNU takes a first page of 0, a last page before the first, or digits // followed by other characters for a file name. - for operand in ["+0", "+2:1", "+1x"] { + for operand in ["+0", "+2:1", "+1x", "+1x:3"] { new_ucmd!() .args(&[operand, "test_one_page.log"]) .fails() - .stderr_contains("No such file or directory"); + .stderr_contains(format!("pr: {operand}: ")); } } diff --git a/util/build-gnu.sh b/util/build-gnu.sh index 70889387dea..155c36178ee 100755 --- a/util/build-gnu.sh +++ b/util/build-gnu.sh @@ -273,11 +273,6 @@ sed -i -e "s|framework_failure_ 'no inotify_add_watch';|fail=1;|" tests/tail/ino # This prevents the test from overwhelming logs while still reporting failures sed -i '/^my $fail = run_tests/i no warnings "redefine"; *Coreutils::_compare_files = sub { my ($p, $t, $io, $a, $e) = @_; my $d = File::Compare::compare($a, $e); warn "$p: test $t: mismatch\\n" if $d; return $d; };' tests/pr/pr-tests.pl -# We don't have the same error message and no need to be that specific -sed -i -e "s|invalid suffix in --pages argument|invalid --pages argument|" \ - -e "s|--pages argument '\$too_big' too large|invalid --pages argument '\$too_big'|" \ - -e "s|invalid page range|invalid --pages argument|" tests/misc/xstrtol.pl - # When decoding an invalid base32/64 string, gnu writes everything it was able to decode until # it hit the decode error, while we don't write anything if the input is invalid. sed -i "s/\(baddecode.*OUT=>\"\).*\"/\1\"/g" tests/basenc/base64.pl