From 1fa0b032e55cacd3f864eeb4f5e5869f25a5ee7c Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Sat, 23 Mar 2024 19:28:14 +0100 Subject: [PATCH 1/4] comm: permit and test separators that contain a hyphen --- src/uu/comm/src/comm.rs | 1 + tests/by-util/test_comm.rs | 16 ++++++++++++++++ .../comm/ab_delimiter_hyphen_help.expected | 3 +++ .../comm/ab_delimiter_hyphen_one.expected | 3 +++ 4 files changed, 23 insertions(+) create mode 100644 tests/fixtures/comm/ab_delimiter_hyphen_help.expected create mode 100644 tests/fixtures/comm/ab_delimiter_hyphen_one.expected diff --git a/src/uu/comm/src/comm.rs b/src/uu/comm/src/comm.rs index dd49ef53b02..d6366e4e53f 100644 --- a/src/uu/comm/src/comm.rs +++ b/src/uu/comm/src/comm.rs @@ -186,6 +186,7 @@ pub fn uu_app() -> Command { .help("separate columns with STR") .value_name("STR") .default_value(options::DELIMITER_DEFAULT) + .allow_hyphen_values(true) .hide_default_value(true), ) .arg( diff --git a/tests/by-util/test_comm.rs b/tests/by-util/test_comm.rs index e2bcc1c443f..f6608c7ebc3 100644 --- a/tests/by-util/test_comm.rs +++ b/tests/by-util/test_comm.rs @@ -91,6 +91,22 @@ fn output_delimiter() { .stdout_only_fixture("ab_delimiter_word.expected"); } +#[test] +fn output_delimiter_hyphen_one() { + new_ucmd!() + .args(&["--output-delimiter", "-1", "a", "b"]) + .succeeds() + .stdout_only_fixture("ab_delimiter_hyphen_one.expected"); +} + +#[test] +fn output_delimiter_hyphen_help() { + new_ucmd!() + .args(&["--output-delimiter", "--help", "a", "b"]) + .succeeds() + .stdout_only_fixture("ab_delimiter_hyphen_help.expected"); +} + #[test] fn output_delimiter_nul() { new_ucmd!() diff --git a/tests/fixtures/comm/ab_delimiter_hyphen_help.expected b/tests/fixtures/comm/ab_delimiter_hyphen_help.expected new file mode 100644 index 00000000000..c245aa27f13 --- /dev/null +++ b/tests/fixtures/comm/ab_delimiter_hyphen_help.expected @@ -0,0 +1,3 @@ +a +--helpb +--help--helpz diff --git a/tests/fixtures/comm/ab_delimiter_hyphen_one.expected b/tests/fixtures/comm/ab_delimiter_hyphen_one.expected new file mode 100644 index 00000000000..458f98dd42f --- /dev/null +++ b/tests/fixtures/comm/ab_delimiter_hyphen_one.expected @@ -0,0 +1,3 @@ +a +-1b +-1-1z From 801edbbcb4777ef404bb3a1af181815c92e2ebc5 Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Sat, 23 Mar 2024 20:52:58 +0100 Subject: [PATCH 2/4] comm: implement and test correct handling of repeated --output-delimiter --- src/uu/comm/src/comm.rs | 33 +++++++++++++++++++------- tests/by-util/test_comm.rs | 47 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 8 deletions(-) diff --git a/src/uu/comm/src/comm.rs b/src/uu/comm/src/comm.rs index d6366e4e53f..931d0a0544b 100644 --- a/src/uu/comm/src/comm.rs +++ b/src/uu/comm/src/comm.rs @@ -9,7 +9,7 @@ use std::cmp::Ordering; use std::fs::File; use std::io::{self, stdin, BufRead, BufReader, Stdin}; use std::path::Path; -use uucore::error::{FromIo, UResult}; +use uucore::error::{FromIo, UResult, USimpleError}; use uucore::line_ending::LineEnding; use uucore::{format_usage, help_about, help_usage}; @@ -61,12 +61,7 @@ impl LineReader { } } -fn comm(a: &mut LineReader, b: &mut LineReader, opts: &ArgMatches) { - let delim = match opts.get_one::(options::DELIMITER).unwrap().as_str() { - "" => "\0", - delim => delim, - }; - +fn comm(a: &mut LineReader, b: &mut LineReader, delim: &str, opts: &ArgMatches) { let width_col_1 = usize::from(!opts.get_flag(options::COLUMN_1)); let width_col_2 = usize::from(!opts.get_flag(options::COLUMN_2)); @@ -152,7 +147,28 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let mut f1 = open_file(filename1, line_ending).map_err_context(|| filename1.to_string())?; let mut f2 = open_file(filename2, line_ending).map_err_context(|| filename2.to_string())?; - comm(&mut f1, &mut f2, &matches); + // Due to default_value(), there must be at least one value here, thus unwrap() must not panic. + let all_delimiters = matches + .get_many::(options::DELIMITER) + .unwrap() + .map(String::from) + .collect::>(); + for delim in &all_delimiters[1..] { + // Note that this check is very different from ".conflicts_with_self(true).action(ArgAction::Set)", + // as this accepts duplicate *identical* arguments. + if delim != &all_delimiters[0] { + // Note: This intentionally deviate from the GNU error message by inserting the word "conflicting". + return Err(USimpleError::new( + 1, + "comm: multiple conflicting output delimiters specified", + )); + } + } + let delim = match &*all_delimiters[0] { + "" => "\0", + delim => delim, + }; + comm(&mut f1, &mut f2, delim, &matches); Ok(()) } @@ -187,6 +203,7 @@ pub fn uu_app() -> Command { .value_name("STR") .default_value(options::DELIMITER_DEFAULT) .allow_hyphen_values(true) + .action(ArgAction::Append) .hide_default_value(true), ) .arg( diff --git a/tests/by-util/test_comm.rs b/tests/by-util/test_comm.rs index f6608c7ebc3..ce3814eac83 100644 --- a/tests/by-util/test_comm.rs +++ b/tests/by-util/test_comm.rs @@ -107,6 +107,53 @@ fn output_delimiter_hyphen_help() { .stdout_only_fixture("ab_delimiter_hyphen_help.expected"); } +#[test] +fn output_delimiter_multiple_identical() { + new_ucmd!() + .args(&[ + "--output-delimiter=word", + "--output-delimiter=word", + "a", + "b", + ]) + .succeeds() + .stdout_only_fixture("ab_delimiter_word.expected"); +} + +#[test] +fn output_delimiter_multiple_different() { + new_ucmd!() + .args(&[ + "--output-delimiter=word", + "--output-delimiter=other", + "a", + "b", + ]) + .fails() + .no_stdout() + .stderr_contains("multiple") + .stderr_contains("output") + .stderr_contains("delimiters"); +} + +#[test] +#[ignore = "This is too weird; deviate intentionally."] +fn output_delimiter_multiple_different_prevents_help() { + new_ucmd!() + .args(&[ + "--output-delimiter=word", + "--output-delimiter=other", + "--help", + "a", + "b", + ]) + .fails() + .no_stdout() + .stderr_contains("multiple") + .stderr_contains("output") + .stderr_contains("delimiters"); +} + #[test] fn output_delimiter_nul() { new_ucmd!() From 884ef1f54b9d340285ce9c378973e1da9e29d6fc Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Sat, 23 Mar 2024 21:04:42 +0100 Subject: [PATCH 3/4] comm: implement and test correct handling of repeated flags --- src/uu/comm/src/comm.rs | 1 + tests/by-util/test_comm.rs | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/src/uu/comm/src/comm.rs b/src/uu/comm/src/comm.rs index 931d0a0544b..ee15a070ee9 100644 --- a/src/uu/comm/src/comm.rs +++ b/src/uu/comm/src/comm.rs @@ -178,6 +178,7 @@ pub fn uu_app() -> Command { .about(ABOUT) .override_usage(format_usage(USAGE)) .infer_long_args(true) + .args_override_self(true) .arg( Arg::new(options::COLUMN_1) .short('1') diff --git a/tests/by-util/test_comm.rs b/tests/by-util/test_comm.rs index ce3814eac83..2dc385ef3f2 100644 --- a/tests/by-util/test_comm.rs +++ b/tests/by-util/test_comm.rs @@ -75,6 +75,14 @@ fn total_with_suppressed_regular_output() { .stdout_is_fixture("ab_total_suppressed_regular_output.expected"); } +#[test] +fn repeated_flags() { + new_ucmd!() + .args(&["--total", "-123123", "--total", "a", "b"]) + .succeeds() + .stdout_is_fixture("ab_total_suppressed_regular_output.expected"); +} + #[test] fn total_with_output_delimiter() { new_ucmd!() From 5803d3b6836778756023b06ca16d1d20849277a8 Mon Sep 17 00:00:00 2001 From: Daniel Hofstetter Date: Sun, 24 Mar 2024 16:09:15 +0100 Subject: [PATCH 4/4] comm: remove "comm" from error msg --- src/uu/comm/src/comm.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/comm/src/comm.rs b/src/uu/comm/src/comm.rs index ee15a070ee9..f8371729284 100644 --- a/src/uu/comm/src/comm.rs +++ b/src/uu/comm/src/comm.rs @@ -160,7 +160,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // Note: This intentionally deviate from the GNU error message by inserting the word "conflicting". return Err(USimpleError::new( 1, - "comm: multiple conflicting output delimiters specified", + "multiple conflicting output delimiters specified", )); } }