diff --git a/src/uu/join/src/join.rs b/src/uu/join/src/join.rs index 4c2e6d52ab7..aa4b95120b5 100644 --- a/src/uu/join/src/join.rs +++ b/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 { +fn parse_settings( + matches: &clap::ArgMatches, + formats: &[FieldList<'_>], + diag_args: Option<&[OsString]>, +) -> UResult { 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::("t") { settings.separator = parse_separator(value_os)?; } - if let Some(formats) = matches.get_many::("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>, +} + +#[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::(id).into_iter().flatten(); + tokens.extend(indices.zip(names.map(Token::Operand))); + } + "o" => { + let formats = matches.get_many::(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::("file1"), - matches.get_one::("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::("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( diff --git a/tests/by-util/test_join.rs b/tests/by-util/test_join.rs index bb7c9d95aab..8859589fd6f 100644 --- a/tests/by-util/test_join.rs +++ b/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"); +}