Merge pull request #6112 from BenWiederhake/dev-comm-all-args

comm: Handle duplicated flags and output-delimiter correctly

Author: cakebaker

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::<String>(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::<String>(options::DELIMITER)
+        .unwrap()
+        .map(String::from)
+        .collect::<Vec<_>>();
+    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,
+                "multiple conflicting output delimiters specified",
+            ));
+        }
+    }
+    let delim = match &*all_delimiters[0] {
+        "" => "\0",
+        delim => delim,
+    };
+    comm(&mut f1, &mut f2, delim, &matches);
     Ok(())
 }
 
@@ -162,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')
@@ -186,6 +203,8 @@ pub fn uu_app() -> Command {
                 .help("separate columns with STR")
                 .value_name("STR")
                 .default_value(options::DELIMITER_DEFAULT)
+                .allow_hyphen_values(true)
+                .action(ArgAction::Append)
                 .hide_default_value(true),
         )
         .arg(

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!()
@@ -91,6 +99,69 @@ 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_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!()

tests/fixtures/comm/ab_delimiter_hyphen_help.expected

@@ -0,0 +1,3 @@
+a
+--helpb
+--help--helpz

tests/fixtures/comm/ab_delimiter_hyphen_one.expected

@@ -0,0 +1,3 @@
+a
+-1b
+-1-1z