Merge pull request #3998 from epage/missing

fix(parser)!: Apply default_missing_value per occurrence
clap-rs · Jul 28, 2022 · 586b49a · 586b49a
2 parents ee06707 + 67adc4a
commit 586b49a
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 69 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -37,6 +37,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Disallow more `value_names` than `number_of_values` (#2695)
 - Replaced `cmd.allow_invalid_for_utf8_external_subcommands` with `cmd.external_subcommand_value_parser` (#3733)
 - Changed the default type of `allow_external_subcommands` from `String` to `OsString`
+- `Arg::default_missing_value` now applies per occurrence rather than if a value is missing across all occurrences
 - *(assert)* Always enforce that version is specified when the `ArgAction::Version` is used
 - *(assert)* Add missing `#[track_caller]`s to make it easier to debug asserts
 - *(assert)* Ensure `overrides_with` IDs are valid

diff --git a/src/parser/parser.rs b/src/parser/parser.rs
@@ -964,25 +964,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
         if arg.is_require_equals_set() && !has_eq {
             if arg.min_vals == Some(0) {
                 debug!("Requires equals, but min_vals == 0");
-                let mut arg_values = Vec::new();
-                // We assume this case is valid: require equals, but min_vals == 0.
-                if !arg.default_missing_vals.is_empty() {
-                    debug!("Parser::parse_opt_value: has default_missing_vals");
-                    for v in arg.default_missing_vals.iter() {
-                        let trailing_values = false; // CLI should not be affecting default_missing_values
-                        let _parse_result = self.split_arg_values(
-                            arg,
-                            &RawOsStr::new(v),
-                            trailing_values,
-                            &mut arg_values,
-                        );
-                        if let Some(_parse_result) = _parse_result {
-                            if _parse_result != ParseResult::ValuesDone {
-                                debug!("Parser::parse_opt_value: Ignoring state {:?}; no values accepted after default_missing_values", _parse_result);
-                            }
-                        }
-                    }
-                };
+                let arg_values = Vec::new();
                 let react_result = self.react(
                     Some(ident),
                     ValueSource::CommandLine,
@@ -1133,11 +1115,32 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
         ident: Option<Identifier>,
         source: ValueSource,
         arg: &Arg<'help>,
-        raw_vals: Vec<OsString>,
+        mut raw_vals: Vec<OsString>,
         matcher: &mut ArgMatcher,
     ) -> ClapResult<ParseResult> {
         self.resolve_pending(matcher)?;
 
+        if raw_vals.is_empty() {
+            // We assume this case is valid: require equals, but min_vals == 0.
+            if !arg.default_missing_vals.is_empty() {
+                debug!("Parser::react: has default_missing_vals");
+                for v in arg.default_missing_vals.iter() {
+                    let trailing_values = false; // CLI should not be affecting default_missing_values
+                    let _parse_result = self.split_arg_values(
+                        arg,
+                        &RawOsStr::new(v),
+                        trailing_values,
+                        &mut raw_vals,
+                    );
+                    if let Some(_parse_result) = _parse_result {
+                        if _parse_result != ParseResult::ValuesDone {
+                            debug!("Parser::parse_opt_value: Ignoring state {:?}; no values accepted after default_missing_values", _parse_result);
+                        }
+                    }
+                }
+            };
+        }
+
         debug!(
             "Parser::react action={:?}, identifier={:?}, source={:?}",
             arg.get_action(),
@@ -1350,55 +1353,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
     fn add_default_value(&self, arg: &Arg<'help>, matcher: &mut ArgMatcher) -> ClapResult<()> {
         let trailing_values = false; // defaults are independent of the commandline
 
-        if !arg.default_missing_vals.is_empty() {
-            debug!(
-                "Parser::add_default_value:iter:{}: has default missing vals",
-                arg.name
-            );
-            match matcher.get(&arg.id) {
-                Some(ma) if ma.all_val_groups_empty() => {
-                    debug!(
-                        "Parser::add_default_value:iter:{}: has no user defined vals",
-                        arg.name
-                    );
-                    // The flag occurred, we just want to add the val groups
-                    let mut arg_values = Vec::new();
-                    for v in arg.default_missing_vals.iter() {
-                        let _parse_result = self.split_arg_values(
-                            arg,
-                            &RawOsStr::new(v),
-                            trailing_values,
-                            &mut arg_values,
-                        );
-                        if let Some(_parse_result) = _parse_result {
-                            if _parse_result != ParseResult::ValuesDone {
-                                debug!("Parser::add_default_value: Ignoring state {:?}; defaults are outside of the parse loop", _parse_result);
-                            }
-                        }
-                    }
-                    self.start_custom_arg(matcher, arg, ValueSource::CommandLine);
-                    self.push_arg_values(arg, arg_values, matcher)?;
-                }
-                None => {
-                    debug!("Parser::add_default_value:iter:{}: wasn't used", arg.name);
-                    // do nothing
-                }
-                _ => {
-                    debug!(
-                        "Parser::add_default_value:iter:{}: has user defined vals",
-                        arg.name
-                    );
-                    // do nothing
-                }
-            }
-        } else {
-            debug!(
-                "Parser::add_default_value:iter:{}: doesn't have default missing vals",
-                arg.name
-            );
-            // do nothing
-        }
-
         if !arg.default_vals_ifs.is_empty() {
             debug!("Parser::add_default_value: has conditional defaults");
             if !matcher.contains(&arg.id) {

diff --git a/tests/builder/default_missing_vals.rs b/tests/builder/default_missing_vals.rs
@@ -143,6 +143,28 @@ fn opt_default_user_override() {
     );
 }
 
+#[test]
+fn default_missing_value_per_occurrence() {
+    // assert no change to usual argument handling when adding default_missing_value()
+    let r = Command::new("cmd")
+        .arg(
+            arg!(o: -o <opt> ... "some opt")
+                .min_values(0)
+                .default_value("default")
+                .default_missing_value("default_missing"),
+        )
+        .try_get_matches_from(vec!["", "-o", "-o=value", "-o"]);
+    assert!(r.is_ok(), "{}", r.unwrap_err());
+    let m = r.unwrap();
+    assert_eq!(
+        m.get_many::<String>("o")
+            .unwrap()
+            .map(|v| v.as_str())
+            .collect::<Vec<_>>(),
+        vec!["default_missing", "value", "default_missing"]
+    );
+}
+
 #[test]
 #[allow(clippy::bool_assert_comparison)]
 fn default_missing_value_flag_value() {