From 95c812b4112f4bb6e63d6501af2298d53d30c0b2 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 1 Jun 2022 10:12:44 -0500 Subject: [PATCH] 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. --- src/builder/action.rs | 53 +++++++++++++++++++++++++++++ src/parser/parser.rs | 75 +++++++++++++++++++++++++++++++++-------- tests/builder/action.rs | 71 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 185 insertions(+), 14 deletions(-) diff --git a/src/builder/action.rs b/src/builder/action.rs index 40b67d427bb..a25c513865e 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` 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::("flag").unwrap_or_default().map(|v| v.as_str()).collect::>(), + /// 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::("flag").unwrap_or_default().map(|v| v.as_str()).collect::>(), + /// 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 { 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::()), diff --git a/src/parser/parser.rs b/src/parser/parser.rs index c3a407f7e87..4a695c26638 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 => { diff --git a/tests/builder/action.rs b/tests/builder/action.rs index bdac85b7ddb..25abb1de4f1 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::("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::("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::("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::("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::("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![2] + ); + + let matches = cmd + .clone() + .try_get_matches_from(["test", "--mammal", "dog", "--mammal", "cat"]) + .unwrap(); + assert_eq!( + matches + .get_many::("mammal") + .unwrap() + .map(|s| s.as_str()) + .collect::>(), + 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![2, 4] + ); +} + #[test] fn set_true() { let cmd =