join: support -o field lists supplied as separate arguments

src/uu/join/src/join.rs

@@ -6,6 +6,7 @@
 // spell-checker:ignore (ToDO) autoformat FILENUM whitespaces nocheck memmem
 
 use clap::builder::ValueParser;
+use clap::parser::ValueSource;
 use clap::{Arg, ArgAction, Command};
 use memchr::{Memchr3, memchr_iter, memmem::Finder};
 use std::cmp::Ordering;
@@ -771,7 +772,11 @@ fn get_and_parse_field_number(matches: &clap::ArgMatches, key: &str) -> UResult<
 /// This function takes the matches from the command-line arguments, processes them,
 /// and returns a `Settings` struct that encapsulates the configuration for the program.
 #[allow(clippy::field_reassign_with_default)]
-fn parse_settings(matches: &clap::ArgMatches, diag_args: Option<&[OsString]>) -> UResult<Settings> {
+fn parse_settings(
+    matches: &clap::ArgMatches,
+    formats: &[FieldList<'_>],
+    diag_args: Option<&[OsString]>,
+) -> UResult<Settings> {
     let keys = get_and_parse_field_number(matches, "j")?;
     let key1 = get_and_parse_field_number(matches, "1")?;
     let key2 = get_and_parse_field_number(matches, "2")?;
@@ -790,16 +795,28 @@ fn parse_settings(matches: &clap::ArgMatches, diag_args: Option<&[OsString]>) ->
     if let Some(value_os) = matches.get_one::<OsString>("t") {
         settings.separator = parse_separator(value_os)?;
     }
-    if let Some(formats) = matches.get_many::<String>("o") {
-        let formats: Vec<&String> = formats.collect();
+    if !formats.is_empty() {
         // GNU lets `-o` repeat and accumulates the fields. Auto mode applies only
         // when every value is exactly `auto`; otherwise each `auto` is ignored.
-        if formats.iter().all(|f| *f == "auto") {
+        if formats
+            .iter()
+            .all(|format| matches!(format, FieldList::Option(format) if *format == "auto"))
+        {
             settings.autoformat = true;
         } else {
             let mut specs = vec![];
             for format in formats {
-                if format == "auto" {
+                let format = match format {
+                    FieldList::Option(format) => *format,
+                    FieldList::Operand(list) => {
+                        let list = list.to_string_lossy();
+                        for (part, _) in uucore::diagnostics::list_items(&list, &[' ', ',', '\t']) {
+                            specs.push(Spec::parse(part)?);
+                        }
+                        continue;
+                    }
+                };
+                if *format == "auto" {
                     continue;
                 }
                 // `-o` has no long form.
@@ -846,6 +863,103 @@ fn parse_settings(matches: &clap::ArgMatches, diag_args: Option<&[OsString]>) ->
     Ok(settings)
 }
 
+/// A `-o` field list, given as the option's value or as an operand.
+enum FieldList<'a> {
+    Option(&'a String),
+    Operand(&'a OsString),
+}
+
+/// The two file operands, and the `-o` field lists in the order GNU join adds them.
+struct Operands<'a> {
+    files: UResult<(&'a OsString, &'a OsString)>,
+    formats: Vec<FieldList<'a>>,
+}
+
+#[derive(Clone, Copy, PartialEq, Eq)]
+enum OperandStatus {
+    MustBeOperand,
+    MightBeFieldList,
+}
+
+/// Splits the operands like GNU join: an operand that directly follows a `-o`
+/// field list, or another such operand, might be more of that list. When a
+/// third operand comes, the older pending operand that might be a field list is
+/// taken as one, so that `join -o 1.1 2.2 file1 file2` works.
+fn split_operands(matches: &clap::ArgMatches) -> Operands<'_> {
+    enum Token<'a> {
+        Option(Option<&'a String>),
+        Operand(&'a OsString),
+    }
+
+    let mut tokens: Vec<(usize, Token<'_>)> = vec![];
+    for id in matches.ids().map(clap::Id::as_str) {
+        if matches.value_source(id) != Some(ValueSource::CommandLine) {
+            continue;
+        }
+        let indices = matches.indices_of(id).into_iter().flatten();
+        match id {
+            "file1" | "file2" | "extra" => {
+                let names = matches.get_many::<OsString>(id).into_iter().flatten();
+                tokens.extend(indices.zip(names.map(Token::Operand)));
+            }
+            "o" => {
+                let formats = matches.get_many::<String>(id).into_iter().flatten();
+                tokens.extend(indices.zip(formats.map(|format| Token::Option(Some(format)))));
+            }
+            _ => tokens.extend(indices.map(|index| (index, Token::Option(None)))),
+        }
+    }
+    tokens.sort_by_key(|(index, _)| *index);
+
+    let mut status = OperandStatus::MustBeOperand;
+    let mut pending: Vec<(&OsString, OperandStatus)> = Vec::with_capacity(2);
+    let mut formats = vec![];
+    let mut error = None;
+    for (_, token) in tokens {
+        match token {
+            Token::Option(format) => {
+                status = OperandStatus::MustBeOperand;
+                if let Some(format) = format {
+                    formats.push(FieldList::Option(format));
+                    if format != "auto" {
+                        status = OperandStatus::MightBeFieldList;
+                    }
+                }
+            }
+            Token::Operand(name) => {
+                if pending.len() == 2 {
+                    let taken = usize::from(pending[0].1 == OperandStatus::MustBeOperand);
+                    if pending[taken].1 == OperandStatus::MustBeOperand {
+                        error.get_or_insert_with(|| {
+                            UUsageError::new(
+                                1,
+                                translate!("join-error-extra-operand", "operand" => name.quote()),
+                            )
+                        });
+                        continue;
+                    }
+                    formats.push(FieldList::Operand(pending.remove(taken).0));
+                }
+                pending.push((name, status));
+            }
+        }
+    }
+
+    let files = match (error, &pending[..]) {
+        (Some(error), _) => Err(error),
+        (None, [(file1, _), (file2, _)]) => Ok((*file1, *file2)),
+        (None, [(file1, _)]) => Err(UUsageError::new(
+            1,
+            translate!("join-error-missing-operand-after", "file" => file1.quote()),
+        )),
+        (None, _) => Err(UUsageError::new(
+            1,
+            translate!("join-error-missing-operand"),
+        )),
+    };
+    Operands { files, formats }
+}
+
 #[uucore::main]
 pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     // The command line is kept for the caret in `-o` diagnostics, which needs
@@ -860,32 +974,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     opts.alternate_handling = Some(AlternateHandling::Shifted);
     let _ = try_init_collator(opts);
 
-    let settings = parse_settings(&matches, diag_args.as_deref())?;
-
-    let (file1, file2) = match (
-        matches.get_one::<OsString>("file1"),
-        matches.get_one::<OsString>("file2"),
-    ) {
-        (Some(file1), Some(file2)) => (file1, file2),
-        (Some(file1), None) => {
-            return Err(UUsageError::new(
-                1,
-                translate!("join-error-missing-operand-after", "file" => file1.quote()),
-            ));
-        }
-        _ => {
-            return Err(UUsageError::new(
-                1,
-                translate!("join-error-missing-operand"),
-            ));
-        }
-    };
-    if let Some(extra) = matches.get_one::<OsString>("extra") {
-        return Err(UUsageError::new(
-            1,
-            translate!("join-error-extra-operand", "operand" => extra.quote()),
-        ));
-    }
+    let operands = split_operands(&matches);
+    let settings = parse_settings(&matches, &operands.formats, diag_args.as_deref())?;
+    let (file1, file2) = operands.files?;
 
     if file1 == "-" && file2 == "-" {
         return Err(USimpleError::new(

tests/by-util/test_join.rs

@@ -861,3 +861,82 @@ fn test_operand_count() {
         .stderr_contains("join: extra operand 'fields_3.txt'\n")
         .stderr_contains("--help");
 }
+
+#[test]
+fn test_format_list_in_separate_operands() {
+    let ts = TestScenario::new(util_name!());
+    let at = &ts.fixtures;
+
+    at.write("a", "1 a x\n2 b y\n");
+    at.write("b", "1 d u\n2 e v\n");
+
+    // The fields of -o may be given as separate operands before the files.
+    ts.ucmd()
+        .args(&["-o", "1.1", "1.2", "1.3", "2.1", "2.2", "2.3", "a", "b"])
+        .succeeds()
+        .stdout_only("1 a x 1 d u\n2 b y 2 e v\n");
+
+    // An operand that follows the -o fields is one too, even after a file.
+    ts.ucmd()
+        .args(&["a", "-o", "1.1", "1.2", "b"])
+        .succeeds()
+        .stdout_only("1 a\n2 b\n");
+
+    // Only as many operands as needed to leave two files are taken as fields.
+    #[cfg(not(windows))]
+    let missing = "join: 2.2: No such file or directory\n";
+    #[cfg(windows)]
+    let missing = "join: '2.2': No such file or directory\n";
+    ts.ucmd()
+        .args(&["-o", "1.1", "1.2", "2.2", "a"])
+        .fails_with_code(1)
+        .stderr_only(missing);
+
+    // The operand taken as fields must be a field list, as with GNU join.
+    ts.ucmd()
+        .args(&["-o", "1.1", "a", "b", "c"])
+        .fails_with_code(1)
+        .stderr_only("join: invalid file number in field spec: 'a'\n");
+
+    // An operand that doesn't follow -o is still an extra operand.
+    ts.ucmd()
+        .args(&["-o", "auto", "1.1", "a", "b"])
+        .fails_with_code(1)
+        .stderr_contains("join: extra operand 'b'\n");
+}
+
+#[test]
+fn test_format_list_operands_around_other_options() {
+    let ts = TestScenario::new(util_name!());
+    let at = &ts.fixtures;
+
+    at.write("a", "1,a\n2,b\n");
+    at.write("b", "1,d\n2,e\n");
+
+    // The field operand only has to follow -o, not to come right before the files.
+    ts.ucmd()
+        .args(&["-o", "1.1", "1.2", "-t", ",", "a", "b"])
+        .succeeds()
+        .stdout_only("1,a\n2,b\n");
+
+    // `1.2` comes before -o, so the operand taken as fields is `a`.
+    ts.ucmd()
+        .args(&["-t", ",", "1.2", "-o", "1.1", "a", "b"])
+        .fails_with_code(1)
+        .stderr_only("join: invalid file number in field spec: 'a'\n");
+}
+
+#[test]
+fn test_format_list_operand_order() {
+    let ts = TestScenario::new(util_name!());
+    let at = &ts.fixtures;
+
+    at.write("a", "1 a\n2 b\n");
+    at.write("b", "1 d\n2 e\n");
+
+    // `2.2` is taken as fields when `b` comes, so before the later `-o 1.2`.
+    ts.ucmd()
+        .args(&["-o", "1.1", "2.2", "a", "b", "-o", "1.2"])
+        .succeeds()
+        .stdout_only("1 d a\n2 e b\n");
+}