From b90b59c0035f3296b3defc0fc1196660f1e3ade6 Mon Sep 17 00:00:00 2001 From: Leviticoh <67531886+Leviticoh@users.noreply.github.com> Date: Sun, 24 Sep 2023 14:44:44 +0200 Subject: [PATCH] uniq: added support for deprecated `-N` option (#4228) --- src/uu/uniq/src/uniq.rs | 80 +++++++++++++++++- tests/by-util/test_uniq.rs | 92 +++++++++++++++------ tests/fixtures/uniq/skip-2-fields.expected | 3 + tests/fixtures/uniq/skip-21-fields.expected | 3 + tests/fixtures/uniq/skip-fields.txt | 3 + 5 files changed, 150 insertions(+), 31 deletions(-) create mode 100644 tests/fixtures/uniq/skip-21-fields.expected diff --git a/src/uu/uniq/src/uniq.rs b/src/uu/uniq/src/uniq.rs index 3aac7b834..72338bf96 100644 --- a/src/uu/uniq/src/uniq.rs +++ b/src/uu/uniq/src/uniq.rs @@ -3,7 +3,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -use clap::{builder::ValueParser, crate_version, Arg, ArgAction, ArgMatches, Command}; +use clap::{builder::ValueParser, crate_version, Arg, ArgAction, ArgGroup, ArgMatches, Command}; use std::ffi::{OsStr, OsString}; use std::fs::File; use std::io::{self, stdin, stdout, BufRead, BufReader, BufWriter, Write}; @@ -23,6 +23,7 @@ pub mod options { pub static IGNORE_CASE: &str = "ignore-case"; pub static REPEATED: &str = "repeated"; pub static SKIP_FIELDS: &str = "skip-fields"; + pub static OBSOLETE_SKIP_FIELDS: &str = "obsolete_skip_field"; pub static SKIP_CHARS: &str = "skip-chars"; pub static UNIQUE: &str = "unique"; pub static ZERO_TERMINATED: &str = "zero-terminated"; @@ -53,6 +54,8 @@ struct Uniq { zero_terminated: bool, } +const OBSOLETE_SKIP_FIELDS_DIGITS: [&str; 10] = ["0", "1", "2", "3", "4", "5", "6", "7", "8", "9"]; + macro_rules! write_line_terminator { ($writer:expr, $line_terminator:expr) => { $writer @@ -236,6 +239,41 @@ fn opt_parsed(opt_name: &str, matches: &ArgMatches) -> UResult UResult> { + for opt_text in OBSOLETE_SKIP_FIELDS_DIGITS { + let argument = matches.get_one::(opt_text); + if matches.contains_id(opt_text) { + let mut full = opt_text.to_owned(); + if let Some(ar) = argument { + full.push_str(ar); + } + let value = full.parse::(); + + if let Ok(val) = value { + return Ok(Some(val)); + } else { + return Err(USimpleError { + code: 1, + message: format!("Invalid argument for skip-fields: {}", full), + } + .into()); + } + } + } + Ok(None) +} + #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = uu_app().after_help(AFTER_HELP).try_get_matches_from(args)?; @@ -247,6 +285,10 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .map(|mut fi| (fi.next(), fi.next())) .unwrap_or_default(); + let skip_fields_modern: Option = opt_parsed(options::SKIP_FIELDS, &matches)?; + + let skip_fields_old: Option = obsolete_skip_field(&matches)?; + let uniq = Uniq { repeats_only: matches.get_flag(options::REPEATED) || matches.contains_id(options::ALL_REPEATED), @@ -255,7 +297,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { || matches.contains_id(options::GROUP), delimiters: get_delimiter(&matches), show_counts: matches.get_flag(options::COUNT), - skip_fields: opt_parsed(options::SKIP_FIELDS, &matches)?, + skip_fields: skip_fields_modern.or(skip_fields_old), slice_start: opt_parsed(options::SKIP_CHARS, &matches)?, slice_stop: opt_parsed(options::CHECK_CHARS, &matches)?, ignore_case: matches.get_flag(options::IGNORE_CASE), @@ -276,7 +318,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } pub fn uu_app() -> Command { - Command::new(uucore::util_name()) + let mut cmd = Command::new(uucore::util_name()) .version(crate_version!()) .about(ABOUT) .override_usage(format_usage(USAGE)) @@ -355,6 +397,7 @@ pub fn uu_app() -> Command { Arg::new(options::SKIP_FIELDS) .short('f') .long(options::SKIP_FIELDS) + .overrides_with_all(OBSOLETE_SKIP_FIELDS_DIGITS) .help("avoid comparing the first N fields") .value_name("N"), ) @@ -372,13 +415,42 @@ pub fn uu_app() -> Command { .help("end lines with 0 byte, not newline") .action(ArgAction::SetTrue), ) + .group( + // in GNU `uniq` every every digit of these arguments + // would be interpreted as a simple flag, + // these flags then are concatenated to get + // the number of fields to skip. + // in this way `uniq -1 -z -2` would be + // equal to `uniq -12 -q`, since this behavior + // is counterintuitive and it's hard to do in clap + // we handle it more like GNU `fold`: we have a flag + // for each possible initial digit, that takes the + // rest of the value as argument. + // we disallow explicitly multiple occurrences + // because then it would have a different behavior + // from GNU + ArgGroup::new(options::OBSOLETE_SKIP_FIELDS) + .multiple(false) + .args(OBSOLETE_SKIP_FIELDS_DIGITS) + ) .arg( Arg::new(ARG_FILES) .action(ArgAction::Append) .value_parser(ValueParser::os_string()) .num_args(0..=2) .value_hint(clap::ValueHint::FilePath), - ) + ); + + for i in OBSOLETE_SKIP_FIELDS_DIGITS { + cmd = cmd.arg( + Arg::new(i) + .short(i.chars().next().unwrap()) + .num_args(0..=1) + .hide(true), + ); + } + + cmd } fn get_delimiter(matches: &ArgMatches) -> Delimiters { diff --git a/tests/by-util/test_uniq.rs b/tests/by-util/test_uniq.rs index 97a829b9f..aa41de827 100644 --- a/tests/by-util/test_uniq.rs +++ b/tests/by-util/test_uniq.rs @@ -80,7 +80,7 @@ fn test_stdin_skip_and_check_2_chars() { } #[test] -fn test_stdin_skip_1_field() { +fn test_stdin_skip_2_fields() { new_ucmd!() .args(&["-f2"]) .pipe_in_fixture(SKIP_FIELDS) @@ -88,6 +88,42 @@ fn test_stdin_skip_1_field() { .stdout_is_fixture("skip-2-fields.expected"); } +#[test] +fn test_stdin_skip_2_fields_obsolete() { + new_ucmd!() + .args(&["-2"]) + .pipe_in_fixture(SKIP_FIELDS) + .run() + .stdout_is_fixture("skip-2-fields.expected"); +} + +#[test] +fn test_stdin_skip_21_fields() { + new_ucmd!() + .args(&["-f21"]) + .pipe_in_fixture(SKIP_FIELDS) + .run() + .stdout_is_fixture("skip-21-fields.expected"); +} + +#[test] +fn test_stdin_skip_21_fields_obsolete() { + new_ucmd!() + .args(&["-21"]) + .pipe_in_fixture(SKIP_FIELDS) + .run() + .stdout_is_fixture("skip-21-fields.expected"); +} + +#[test] +fn test_stdin_skip_invalid_fields_obsolete() { + new_ucmd!() + .args(&["-5deadbeef"]) + .run() + .failure() + .stderr_only("uniq: Invalid argument for skip-fields: 5deadbeef\n"); +} + #[test] fn test_stdin_all_repeated() { new_ucmd!() @@ -436,15 +472,15 @@ fn gnu_tests() { stderr: None, exit: None, }, - // // Obsolete syntax for "-f 1" - // TestCase { - // name: "obs30", - // args: &["-1"], - // input: "a a\nb a\n", - // stdout: Some("a a\n"), - // stderr: None, - // exit: None, - // }, + // Obsolete syntax for "-f 1" + TestCase { + name: "obs30", + args: &["-1"], + input: "a a\nb a\n", + stdout: Some("a a\n"), + stderr: None, + exit: None, + }, TestCase { name: "31", args: &["-f", "1"], @@ -518,23 +554,25 @@ fn gnu_tests() { stderr: None, exit: None, }, - // // Obsolete syntax for "-s 1" - // TestCase { - // name: "obs-plus44", - // args: &["+1", "--"], - // input: "aaa\naaa\n", - // stdout: Some("aaa\n"), - // stderr: None, - // exit: None, - // }, - // TestCase { - // name: "obs-plus45", - // args: &["+1", "--"], - // input: "baa\naaa\n", - // stdout: Some("baa\n"), - // stderr: None, - // exit: None, - // }, + /* + // Obsolete syntax for "-s 1" + TestCase { + name: "obs-plus44", + args: &["+1", "--"], + input: "aaa\naaa\n", + stdout: Some("aaa\n"), + stderr: None, + exit: None, + }, + TestCase { + name: "obs-plus45", + args: &["+1", "--"], + input: "baa\naaa\n", + stdout: Some("baa\n"), + stderr: None, + exit: None, + }, + */ TestCase { name: "50", args: &["-f", "1", "-s", "1"], diff --git a/tests/fixtures/uniq/skip-2-fields.expected b/tests/fixtures/uniq/skip-2-fields.expected index b971c2b2b..c7f9bde9d 100644 --- a/tests/fixtures/uniq/skip-2-fields.expected +++ b/tests/fixtures/uniq/skip-2-fields.expected @@ -1,2 +1,5 @@ aaa ⟪⟫ a aa a +a a a a a a a a a a a a a a a a a a a b a a a +a a a a a a a a a a a a a a a a a a a a b a a +a a a a a a a a a a a a a a a a a a a a a b a diff --git a/tests/fixtures/uniq/skip-21-fields.expected b/tests/fixtures/uniq/skip-21-fields.expected new file mode 100644 index 000000000..1f5295afa --- /dev/null +++ b/tests/fixtures/uniq/skip-21-fields.expected @@ -0,0 +1,3 @@ + aaa ⟪⟫ a +a a a a a a a a a a a a a a a a a a a b a a a +a a a a a a a a a a a a a a a a a a a a a b a diff --git a/tests/fixtures/uniq/skip-fields.txt b/tests/fixtures/uniq/skip-fields.txt index 4ec2744c6..0ca708a74 100644 --- a/tests/fixtures/uniq/skip-fields.txt +++ b/tests/fixtures/uniq/skip-fields.txt @@ -6,3 +6,6 @@ ZZZ aa a aa a a +a a a a a a a a a a a a a a a a a a a b a a a +a a a a a a a a a a a a a a a a a a a a b a a +a a a a a a a a a a a a a a a a a a a a a b a