Merge pull request #4924 from epage/term2

fix(parser): Allow multiple value terminated positionals

Author: epage

clap_builder/src/builder/debug_asserts.rs

@@ -603,6 +603,7 @@ fn _verify_positionals(cmd: &Command) -> bool {
             .get_positionals()
             .filter(|p| {
                 p.is_multiple_values_set()
+                    && p.get_value_terminator().is_none()
                     && !p.get_num_args().expect(INTERNAL_ERROR_MSG).is_fixed()
             })
             .count();

clap_builder/src/parser/parser.rs

@@ -372,26 +372,27 @@ impl<'cmd> Parser<'cmd> {
                 if matcher.pending_arg_id() != Some(arg.get_id()) || !arg.is_multiple_values_set() {
                     ok!(self.resolve_pending(matcher));
                 }
-                if let Some(_parse_result) = self.check_terminator(arg, arg_os.to_value_os()) {
-                    debug!(
-                        "Parser::get_matches_with: ignoring terminator result {_parse_result:?}"
-                    );
-                } else {
-                    let arg_values = matcher.pending_values_mut(
-                        arg.get_id(),
-                        Some(Identifier::Index),
-                        trailing_values,
-                    );
-                    arg_values.push(arg_os.to_value_os().to_owned());
-                }
+                parse_state =
+                    if let Some(parse_result) = self.check_terminator(arg, arg_os.to_value_os()) {
+                        debug_assert_eq!(parse_result, ParseResult::ValuesDone);
+                        pos_counter += 1;
+                        ParseState::ValuesDone
+                    } else {
+                        let arg_values = matcher.pending_values_mut(
+                            arg.get_id(),
+                            Some(Identifier::Index),
+                            trailing_values,
+                        );
+                        arg_values.push(arg_os.to_value_os().to_owned());
 
-                // Only increment the positional counter if it doesn't allow multiples
-                if !arg.is_multiple() {
-                    pos_counter += 1;
-                    parse_state = ParseState::ValuesDone;
-                } else {
-                    parse_state = ParseState::Pos(arg.get_id().clone());
-                }
+                        // Only increment the positional counter if it doesn't allow multiples
+                        if !arg.is_multiple() {
+                            pos_counter += 1;
+                            ParseState::ValuesDone
+                        } else {
+                            ParseState::Pos(arg.get_id().clone())
+                        }
+                    };
                 valid_arg_found = true;
             } else if let Some(external_parser) =
                 self.cmd.get_external_subcommand_value_parser().cloned()

tests/builder/multiple_values.rs

@@ -1411,6 +1411,51 @@ fn multiple_vals_with_hyphen() {
     );
 }
 
+#[test]
+fn multiple_positional_multiple_values() {
+    let res = Command::new("do")
+        .arg(
+            Arg::new("cmd1")
+                .action(ArgAction::Set)
+                .num_args(1..)
+                .allow_hyphen_values(true)
+                .value_terminator(";"),
+        )
+        .arg(
+            Arg::new("cmd2")
+                .action(ArgAction::Set)
+                .num_args(1..)
+                .allow_hyphen_values(true)
+                .value_terminator(";"),
+        )
+        .try_get_matches_from(vec![
+            "do",
+            "find",
+            "-type",
+            "f",
+            "-name",
+            "special",
+            ";",
+            "/home/clap",
+            "foo",
+        ]);
+    assert!(res.is_ok(), "{:?}", res.unwrap_err().kind());
+
+    let m = res.unwrap();
+    let cmd1: Vec<_> = m
+        .get_many::<String>("cmd1")
+        .unwrap()
+        .map(|v| v.as_str())
+        .collect();
+    assert_eq!(&cmd1, &["find", "-type", "f", "-name", "special"]);
+    let cmd2: Vec<_> = m
+        .get_many::<String>("cmd2")
+        .unwrap()
+        .map(|v| v.as_str())
+        .collect();
+    assert_eq!(&cmd2, &["/home/clap", "foo"]);
+}
+
 #[test]
 fn issue_1480_max_values_consumes_extra_arg_1() {
     let res = Command::new("prog")