Merge pull request #4000 from epage/missing

fix(parser): Rely on default_missing_value for flag actions
clap-rs · Jul 28, 2022 · 81faa8b · 81faa8b
2 parents 4887695 + 8e20782
commit 81faa8b
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 37 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Removed `PartialEq` and `Eq` from `Command`
 - `number_of_values(0)` no longer implies `takes_value(true).multiple_values(true)`
 - `number_of_values(1)` no longer implies `multiple_values(true)`
+- `ArgAction::SetTrue` and `ArgAction::SetFalse` now prioritize `Arg::default_missing_value` over their standard behavior
 - *(help)* Make `DeriveDisplayOrder` the default and removed the setting.  To sort help, set `next_display_order(None)` (#2808)
 - *(help)* Subcommand display order respects `Command::next_display_order` instead of `DeriveDisplayOrder` and using its own initial display order value (#2808)
 - *(env)* Parse `--help` and `--version` like any `ArgAction::SetTrue` flag (#3776)

diff --git a/src/builder/action.rs b/src/builder/action.rs
@@ -260,6 +260,18 @@ impl ArgAction {
         }
     }
 
+    pub(crate) fn default_missing_value(&self) -> Option<&'static std::ffi::OsStr> {
+        match self {
+            Self::Set => None,
+            Self::Append => None,
+            Self::SetTrue => Some(std::ffi::OsStr::new("true")),
+            Self::SetFalse => Some(std::ffi::OsStr::new("false")),
+            Self::Count => None,
+            Self::Help => None,
+            Self::Version => None,
+        }
+    }
+
     pub(crate) fn default_value_parser(&self) -> Option<super::ValueParser> {
         match self {
             Self::Set => None,

diff --git a/src/builder/arg.rs b/src/builder/arg.rs
@@ -4303,6 +4303,11 @@ impl<'help> Arg<'help> {
                     self.default_vals = vec![default_value];
                 }
             }
+            if let Some(default_value) = action.default_missing_value() {
+                if self.default_missing_vals.is_empty() {
+                    self.default_missing_vals = vec![default_value];
+                }
+            }
             if action.takes_values() {
                 self.settings.set(ArgSettings::TakesValue);
             } else {

diff --git a/src/parser/parser.rs b/src/parser/parser.rs
@@ -1184,17 +1184,10 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
                 Ok(ParseResult::ValuesDone)
             }
             ArgAction::SetTrue => {
-                let raw_vals = match raw_vals.len() {
-                    0 => {
-                        vec![OsString::from("true")]
-                    }
-                    1 => raw_vals,
-                    _ => {
-                        debug!("Parser::react ignoring trailing values: {:?}", raw_vals);
-                        let mut raw_vals = raw_vals;
-                        raw_vals.resize(1, Default::default());
-                        raw_vals
-                    }
+                let raw_vals = if raw_vals.is_empty() {
+                    vec![OsString::from("true")]
+                } else {
+                    raw_vals
                 };
 
                 matcher.remove(&arg.id);
@@ -1203,17 +1196,10 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
                 Ok(ParseResult::ValuesDone)
             }
             ArgAction::SetFalse => {
-                let raw_vals = match raw_vals.len() {
-                    0 => {
-                        vec![OsString::from("false")]
-                    }
-                    1 => raw_vals,
-                    _ => {
-                        debug!("Parser::react ignoring trailing values: {:?}", raw_vals);
-                        let mut raw_vals = raw_vals;
-                        raw_vals.resize(1, Default::default());
-                        raw_vals
-                    }
+                let raw_vals = if raw_vals.is_empty() {
+                    vec![OsString::from("false")]
+                } else {
+                    raw_vals
                 };
 
                 matcher.remove(&arg.id);
@@ -1222,21 +1208,14 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
                 Ok(ParseResult::ValuesDone)
             }
             ArgAction::Count => {
-                let raw_vals = match raw_vals.len() {
-                    0 => {
-                        let existing_value = *matcher
-                            .get_one::<crate::builder::CountType>(arg.get_id())
-                            .unwrap_or(&0);
-                        let next_value = existing_value.saturating_add(1);
-                        vec![OsString::from(next_value.to_string())]
-                    }
-                    1 => raw_vals,
-                    _ => {
-                        debug!("Parser::react ignoring trailing values: {:?}", raw_vals);
-                        let mut raw_vals = raw_vals;
-                        raw_vals.resize(1, Default::default());
-                        raw_vals
-                    }
+                let raw_vals = if raw_vals.is_empty() {
+                    let existing_value = *matcher
+                        .get_one::<crate::builder::CountType>(arg.get_id())
+                        .unwrap_or(&0);
+                    let next_value = existing_value.saturating_add(1);
+                    vec![OsString::from(next_value.to_string())]
+                } else {
+                    raw_vals
                 };
 
                 matcher.remove(&arg.id);