Repository: clap-rs/clap
Pull request #3777 (2022-06-01 15:16:24)

feat(builder): Set/Append Actions
This round out the new style actions and allow us to start deprecating
occurrences.

As part of an effort to unify code paths, this does change flag parsing
to do splits.  This will only be a problem if the user enables splits
but we'll at least not crash.  Once we also address #3776, we'll be able
to have envs all work the same.

<!--
Thanks for helping out!

Please link the appropriate issue from your PR.

If you don't have an issue, we'd recommend starting with one first so the PR can focus on the
implementation (unless its an obvious bug or documentation fix that will have
little conversation).
-->



Patch:
diff --git a/src/builder/action.rs b/src/builder/action.rs
index 40b67d42..a25c5138 100644
--- a/src/builder/action.rs
+++ b/src/builder/action.rs
@@ -24,6 +24,53 @@
 #[non_exhaustive]
 #[allow(missing_copy_implementations)] // In the future, we may accept `Box<dyn ...>`
 pub enum ArgAction {
+    /// When encountered, store the associated value(s) in [`ArgMatches`][crate::ArgMatches]
+    ///
+    /// # Examples
+    ///
+    /// ```rust
+    /// # use clap::Command;
+    /// # use clap::Arg;
+    /// let cmd = Command::new("mycmd")
+    ///     .arg(
+    ///         Arg::new("flag")
+    ///             .long("flag")
+    ///             .action(clap::builder::ArgAction::Set)
+    ///     );
+    ///
+    /// let matches = cmd.try_get_matches_from(["mycmd", "--flag", "value"]).unwrap();
+    /// assert!(matches.is_present("flag"));
+    /// assert_eq!(matches.occurrences_of("flag"), 0);
+    /// assert_eq!(
+    ///     matches.get_many::<String>("flag").unwrap_or_default().map(|v| v.as_str()).collect::<Vec<_>>(),
+    ///     vec!["value"]
+    /// );
+    /// ```
+    Set,
+    /// When encountered, store the associated value(s) in [`ArgMatches`][crate::ArgMatches]
+    ///
+    /// # Examples
+    ///
+    /// ```rust
+    /// # use clap::Command;
+    /// # use clap::Arg;
+    /// let cmd = Command::new("mycmd")
+    ///     .arg(
+    ///         Arg::new("flag")
+    ///             .long("flag")
+    ///             .multiple_occurrences(true)
+    ///             .action(clap::builder::ArgAction::Append)
+    ///     );
+    ///
+    /// let matches = cmd.try_get_matches_from(["mycmd", "--flag", "value1", "--flag", "value2"]).unwrap();
+    /// assert!(matches.is_present("flag"));
+    /// assert_eq!(matches.occurrences_of("flag"), 0);
+    /// assert_eq!(
+    ///     matches.get_many::<String>("flag").unwrap_or_default().map(|v| v.as_str()).collect::<Vec<_>>(),
+    ///     vec!["value1", "value2"]
+    /// );
+    /// ```
+    Append,
     /// When encountered, store the associated value(s) in [`ArgMatches`][crate::ArgMatches]
     ///
     /// # Examples
@@ -201,6 +248,8 @@ pub enum ArgAction {
 impl ArgAction {
     pub(crate) fn takes_value(&self) -> bool {
         match self {
+            Self::Set => true,
+            Self::Append => true,
             Self::StoreValue => true,
             Self::IncOccurrence => false,
             Self::SetTrue => false,
@@ -213,6 +262,8 @@ impl ArgAction {
 
     pub(crate) fn default_value_parser(&self) -> Option<super::ValueParser> {
         match self {
+            Self::Set => None,
+            Self::Append => None,
             Self::StoreValue => None,
             Self::IncOccurrence => None,
             Self::SetTrue => Some(super::ValueParser::bool()),
@@ -228,6 +279,8 @@ impl ArgAction {
         use crate::parser::AnyValueId;
 
         match self {
+            Self::Set => None,
+            Self::Append => None,
             Self::StoreValue => None,
             Self::IncOccurrence => None,
             Self::SetTrue => Some(AnyValueId::of::<bool>()),
diff --git a/src/parser/parser.rs b/src/parser/parser.rs
index c3a407f7..4a695c26 100644
--- a/src/parser/parser.rs
+++ b/src/parser/parser.rs
@@ -1138,6 +1138,41 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
             source
         );
         match arg.get_action() {
+            ArgAction::Set => {
+                if source == ValueSource::CommandLine
+                    && matches!(ident, Some(Identifier::Short) | Some(Identifier::Long))
+                {
+                    // Record flag's index
+                    self.cur_idx.set(self.cur_idx.get() + 1);
+                    debug!("Parser::react: cur_idx:={}", self.cur_idx.get());
+                }
+                matcher.remove(&arg.id);
+                self.start_custom_arg(matcher, arg, source);
+                self.push_arg_values(arg, raw_vals, matcher)?;
+                if cfg!(debug_assertions) && matcher.needs_more_vals(arg) {
+                    debug!(
+                        "Parser::react not enough values passed in, leaving it to the validator to complain",
+                    );
+                }
+                Ok(ParseResult::ValuesDone)
+            }
+            ArgAction::Append => {
+                if source == ValueSource::CommandLine
+                    && matches!(ident, Some(Identifier::Short) | Some(Identifier::Long))
+                {
+                    // Record flag's index
+                    self.cur_idx.set(self.cur_idx.get() + 1);
+                    debug!("Parser::react: cur_idx:={}", self.cur_idx.get());
+                }
+                self.start_custom_arg(matcher, arg, source);
+                self.push_arg_values(arg, raw_vals, matcher)?;
+                if cfg!(debug_assertions) && matcher.needs_more_vals(arg) {
+                    debug!(
+                        "Parser::react not enough values passed in, leaving it to the validator to complain",
+                    );
+                }
+                Ok(ParseResult::ValuesDone)
+            }
             ArgAction::StoreValue => {
                 if ident == Some(Identifier::Index)
                     && arg.is_multiple_values_set()
@@ -1189,10 +1224,10 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
                     }
                     1 => raw_vals,
                     _ => {
-                        panic!(
-                            "Argument {:?} received too many values: {:?}",
-                            arg.id, raw_vals
-                        )
+                        debug!("Parser::react ignoring trailing values: {:?}", raw_vals);
+                        let mut raw_vals = raw_vals;
+                        raw_vals.resize(1, Default::default());
+                        raw_vals
                     }
                 };
 
@@ -1208,10 +1243,10 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
                     }
                     1 => raw_vals,
                     _ => {
-                        panic!(
-                            "Argument {:?} received too many values: {:?}",
-                            arg.id, raw_vals
-                        )
+                        debug!("Parser::react ignoring trailing values: {:?}", raw_vals);
+                        let mut raw_vals = raw_vals;
+                        raw_vals.resize(1, Default::default());
+                        raw_vals
                     }
                 };
 
@@ -1231,10 +1266,10 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
                     }
                     1 => raw_vals,
                     _ => {
-                        panic!(
-                            "Argument {:?} received too many values: {:?}",
-                            arg.id, raw_vals
-                        )
+                        debug!("Parser::react ignoring trailing values: {:?}", raw_vals);
+                        let mut raw_vals = raw_vals;
+                        raw_vals.resize(1, Default::default());
+                        raw_vals
                     }
                 };
 
@@ -1339,14 +1374,26 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
                                 )?;
                             }
                         }
-                        ArgAction::SetTrue | ArgAction::SetFalse | ArgAction::Count => {
+                        ArgAction::Set
+                        | ArgAction::Append
+                        | ArgAction::SetTrue
+                        | ArgAction::SetFalse
+                        | ArgAction::Count => {
+                            let mut arg_values = Vec::new();
+                            let _parse_result =
+                                self.split_arg_values(arg, &val, trailing_values, &mut arg_values);
                             let _ = self.react(
                                 None,
                                 ValueSource::EnvVariable,
                                 arg,
-                                vec![val.to_os_str().into_owned()],
+                                arg_values,
                                 matcher,
                             )?;
+                            if let Some(_parse_result) = _parse_result {
+                                if _parse_result != ParseResult::ValuesDone {
+                                    debug!("Parser::add_env: Ignoring state {:?}; env variables are outside of the parse loop", _parse_result);
+                                }
+                            }
                         }
                         // Early return on `Help` or `Version`.
                         ArgAction::Help | ArgAction::Version => {


Test patch:
diff --git a/tests/builder/action.rs b/tests/builder/action.rs
index bdac85b7..25abb1de 100644
--- a/tests/builder/action.rs
+++ b/tests/builder/action.rs
@@ -4,6 +4,77 @@ use clap::builder::ArgAction;
 use clap::Arg;
 use clap::Command;
 
+#[test]
+fn set() {
+    let cmd = Command::new("test").arg(Arg::new("mammal").long("mammal").action(ArgAction::Set));
+
+    let matches = cmd.clone().try_get_matches_from(["test"]).unwrap();
+    assert_eq!(matches.get_one::<String>("mammal"), None);
+    assert_eq!(matches.is_present("mammal"), false);
+    assert_eq!(matches.occurrences_of("mammal"), 0);
+    assert_eq!(matches.index_of("mammal"), None);
+
+    let matches = cmd
+        .clone()
+        .try_get_matches_from(["test", "--mammal", "dog"])
+        .unwrap();
+    assert_eq!(matches.get_one::<String>("mammal").unwrap(), "dog");
+    assert_eq!(matches.is_present("mammal"), true);
+    assert_eq!(matches.occurrences_of("mammal"), 0);
+    assert_eq!(matches.index_of("mammal"), Some(2));
+
+    let matches = cmd
+        .clone()
+        .try_get_matches_from(["test", "--mammal", "dog", "--mammal", "cat"])
+        .unwrap();
+    assert_eq!(matches.get_one::<String>("mammal").unwrap(), "cat");
+    assert_eq!(matches.is_present("mammal"), true);
+    assert_eq!(matches.occurrences_of("mammal"), 0);
+    assert_eq!(matches.index_of("mammal"), Some(4));
+}
+
+#[test]
+fn append() {
+    let cmd = Command::new("test").arg(Arg::new("mammal").long("mammal").action(ArgAction::Append));
+
+    let matches = cmd.clone().try_get_matches_from(["test"]).unwrap();
+    assert_eq!(matches.get_one::<String>("mammal"), None);
+    assert_eq!(matches.is_present("mammal"), false);
+    assert_eq!(matches.occurrences_of("mammal"), 0);
+    assert_eq!(matches.index_of("mammal"), None);
+
+    let matches = cmd
+        .clone()
+        .try_get_matches_from(["test", "--mammal", "dog"])
+        .unwrap();
+    assert_eq!(matches.get_one::<String>("mammal").unwrap(), "dog");
+    assert_eq!(matches.is_present("mammal"), true);
+    assert_eq!(matches.occurrences_of("mammal"), 0);
+    assert_eq!(
+        matches.indices_of("mammal").unwrap().collect::<Vec<_>>(),
+        vec![2]
+    );
+
+    let matches = cmd
+        .clone()
+        .try_get_matches_from(["test", "--mammal", "dog", "--mammal", "cat"])
+        .unwrap();
+    assert_eq!(
+        matches
+            .get_many::<String>("mammal")
+            .unwrap()
+            .map(|s| s.as_str())
+            .collect::<Vec<_>>(),
+        vec!["dog", "cat"]
+    );
+    assert_eq!(matches.is_present("mammal"), true);
+    assert_eq!(matches.occurrences_of("mammal"), 0);
+    assert_eq!(
+        matches.indices_of("mammal").unwrap().collect::<Vec<_>>(),
+        vec![2, 4]
+    );
+}
+
 #[test]
 fn set_true() {
     let cmd =