diff --git a/src/uu/fold/src/fold.rs b/src/uu/fold/src/fold.rs index d950c762c0..b7f1591fe9 100644 --- a/src/uu/fold/src/fold.rs +++ b/src/uu/fold/src/fold.rs @@ -54,16 +54,13 @@ struct FoldContext<'a, W: Write> { pub fn uumain(args: impl uucore::Args) -> UResult<()> { let args = args.collect_lossy(); - let (args, obs_width) = handle_obsolete(&args[..]); + let args = handle_obsolete(&args[..]); let matches = uucore::clap_localization::handle_clap_result(uu_app(), args)?; let bytes = matches.get_flag(options::BYTES); let characters = matches.get_flag(options::CHARACTERS); let spaces = matches.get_flag(options::SPACES); - let poss_width = match matches.get_one::(options::WIDTH) { - Some(v) => Some(v.clone()), - None => obs_width, - }; + let poss_width = matches.get_one::(options::WIDTH).cloned(); let width = match poss_width { Some(inp_width) => match inp_width.parse::() { @@ -105,6 +102,8 @@ pub fn uu_app() -> Command { .override_usage(format_usage(&translate!("fold-usage"))) .about(translate!("fold-about")) .infer_long_args(true) + // GNU lets a later width override an earlier one, e.g. `fold -w3 -w5`. + .args_override_self(true) .arg( Arg::new(options::BYTES) .long(options::BYTES) @@ -143,16 +142,72 @@ pub fn uu_app() -> Command { ) } -fn handle_obsolete(args: &[String]) -> (Vec, Option) { - for (i, arg) in args.iter().enumerate() { - let slice = &arg; - if slice.starts_with('-') && slice.chars().nth(1).is_some_and(|c| c.is_ascii_digit()) { - let mut v = args.to_vec(); - v.remove(i); - return (v, Some(slice[1..].to_owned())); +/// Whether `arg` is the obsolete `-WIDTH` form, such as `-5`. +/// +/// The check is deliberately loose: GNU rejects `-5x` with +/// `invalid number of columns: '5x'` rather than treating it as a file, so +/// anything starting with a digit is taken as a (possibly invalid) width. +fn is_obsolete_width(arg: &str) -> bool { + arg.strip_prefix('-') + .and_then(|rest| rest.chars().next()) + .is_some_and(|c| c.is_ascii_digit()) +} + +/// Whether `arg` is a `-w`/`--width` spelling that consumes the *next* +/// argument as its value. +fn takes_width_value(arg: &str) -> bool { + if let Some(long) = arg.strip_prefix("--") { + // `--width=5` carries its own value. A bare `--width`, or an + // unambiguous abbreviation of it (`infer_long_args` is enabled), + // takes the following argument. + !long.is_empty() && !long.contains('=') && options::WIDTH.starts_with(long) + } else if let Some(short) = arg.strip_prefix('-') { + // Only a trailing `w` takes the next argument: in `-sw` it does, but + // in `-w3` and `-wb` the value is attached to the flag instead. + short.find('w') == Some(short.len() - 1) + } else { + false + } +} + +/// Rewrite the obsolete `-WIDTH` syntax into the equivalent `--width=WIDTH`. +/// +/// GNU processes options left to right, so the last width given wins no matter +/// which spelling was used (`fold -w3 -5` folds at 5). Rewriting in place keeps +/// that ordering and lets clap apply the same last-one-wins rule, via +/// `args_override_self`. +/// +/// An argument is only the obsolete form when it is not already being consumed +/// as the value of `-w`/`--width`, and has not been placed after a `--` +/// terminator. Otherwise `fold -w -1` would misreport the file name as the +/// invalid width, and `fold -- -3` would silently read stdin instead of +/// failing to open the file named `-3`. +fn handle_obsolete(args: &[String]) -> Vec { + let mut result = Vec::with_capacity(args.len()); + let mut iter = args.iter(); + + // argv[0] is the program name, never an option. + if let Some(program) = iter.next() { + result.push(program.clone()); + } + + let mut end_of_options = false; + let mut expecting_width = false; + for arg in iter { + if end_of_options || expecting_width { + expecting_width = false; + } else if arg == "--" { + end_of_options = true; + } else if is_obsolete_width(arg) { + result.push(format!("--{}={}", options::WIDTH, &arg[1..])); + continue; + } else { + expecting_width = takes_width_value(arg); } + result.push(arg.clone()); } - (args.to_vec(), None) + + result } fn fold( diff --git a/tests/by-util/test_fold.rs b/tests/by-util/test_fold.rs index fca8866e4b..acba6391ef 100644 --- a/tests/by-util/test_fold.rs +++ b/tests/by-util/test_fold.rs @@ -1109,3 +1109,59 @@ fn test_width_overflow() { "fold: invalid number of columns: '999999999999999999999': Value too large to be stored in data type\n", ); } + +#[test] +fn test_negative_width_as_separate_value() { + // GNU consumes the argument after `-w` as its value even when it looks + // like an option, and reports that value as the invalid one. Passing a + // file too, because the width used to be mistaken for the obsolete + // `-WIDTH` form, leaving the file name to be read as the width. + for args in [ + vec!["-w", "-1"], + vec!["--width", "-1"], + vec!["-bw", "-1"], + vec!["-sw", "-1"], + ] { + new_ucmd!() + .args(&args) + .arg("lorem_ipsum.txt") + .fails_with_code(1) + .stderr_is("fold: invalid number of columns: '-1'\n"); + } +} + +#[test] +fn test_obsolete_width_after_double_dash_is_a_file() { + // After `--` there are no more options, so `-3` names a file. + new_ucmd!() + .arg("--") + .arg("-3") + .fails_with_code(1) + .stderr_is("fold: -3: No such file or directory\n"); +} + +#[test] +fn test_last_width_wins() { + // GNU processes options left to right, so the last width given wins, + // regardless of which spelling each one used. + for args in [ + vec!["-w3", "-5"], + vec!["-3", "-5"], + vec!["-w", "3", "-w", "5"], + vec!["-w3", "-w5"], + vec!["-5", "-w", "5"], + ] { + new_ucmd!() + .args(&args) + .pipe_in("aaaaaaaaaa\n") + .succeeds() + .stdout_is("aaaaa\naaaaa\n"); + } + + // ...and an obsolete width is overridden by a later `-w` just the same. + new_ucmd!() + .args(&["-5", "-w", "3"]) + .pipe_in("aaaaaa\n") + .succeeds() + .stdout_is("aaa\naaa\n"); +}