From 67f47c5618aa4b05cd96aa8bbb07d8c2f3bc1c92 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 31 May 2022 16:48:34 -0500 Subject: [PATCH 1/5] feat(parser): SetTrue/SetFalse/Count Actions This is the minimum set of actions for the derive to move off of `parse`. These are inspired by Python's native actions. These new actions have a "unified" behavior with defaults/envs. This mostly means that occurrences aren't tracked. Occurrences were used as a substitute for `ValueSource` or for counting values. Both cases shouldn't be needed anymore but we can re-evaluate this later if needed. --- src/builder/action.rs | 106 +++++++++++++++++++++++++++++++++++ src/builder/arg.rs | 4 +- src/builder/debug_asserts.rs | 10 ++++ src/builder/mod.rs | 1 + src/parser/parser.rs | 91 ++++++++++++++++++++++++++++++ 5 files changed, 211 insertions(+), 1 deletion(-) diff --git a/src/builder/action.rs b/src/builder/action.rs index 0f58738dbfd..0bbeb14956b 100644 --- a/src/builder/action.rs +++ b/src/builder/action.rs @@ -1,3 +1,5 @@ +use crate::parser::AnyValueId; + /// Behavior of arguments when they are encountered while parsing /// /// # Examples @@ -70,6 +72,81 @@ pub enum ArgAction { /// assert_eq!(matches.get_many::("flag").unwrap_or_default().count(), 0); /// ``` IncOccurrence, + /// When encountered, act as if `"true"` was encountered on the command-line + /// + /// No value is allowed + /// + /// # Examples + /// + /// ```rust + /// # use clap::Command; + /// # use clap::Arg; + /// let cmd = Command::new("mycmd") + /// .arg( + /// Arg::new("flag") + /// .long("flag") + /// .action(clap::builder::ArgAction::SetTrue) + /// ); + /// + /// let matches = cmd.try_get_matches_from(["mycmd", "--flag", "--flag"]).unwrap(); + /// assert!(matches.is_present("flag")); + /// assert_eq!(matches.occurrences_of("flag"), 0); + /// assert_eq!( + /// matches.get_one::("flag").copied(), + /// Some(true) + /// ); + /// ``` + SetTrue, + /// When encountered, act as if `"false"` was encountered on the command-line + /// + /// No value is allowed + /// + /// # Examples + /// + /// ```rust + /// # use clap::Command; + /// # use clap::Arg; + /// let cmd = Command::new("mycmd") + /// .arg( + /// Arg::new("flag") + /// .long("flag") + /// .action(clap::builder::ArgAction::SetFalse) + /// ); + /// + /// let matches = cmd.try_get_matches_from(["mycmd", "--flag", "--flag"]).unwrap(); + /// assert!(matches.is_present("flag")); + /// assert_eq!(matches.occurrences_of("flag"), 0); + /// assert_eq!( + /// matches.get_one::("flag").copied(), + /// Some(false) + /// ); + /// ``` + SetFalse, + /// When encountered, increment a counter + /// + /// No value is allowed + /// + /// # Examples + /// + /// ```rust + /// # use clap::Command; + /// # use clap::Arg; + /// let cmd = Command::new("mycmd") + /// .arg( + /// Arg::new("flag") + /// .long("flag") + /// .action(clap::builder::ArgAction::Count) + /// ); + /// + /// let matches = cmd.try_get_matches_from(["mycmd", "--flag", "--flag"]).unwrap(); + /// assert!(matches.is_present("flag")); + /// assert_eq!(matches.occurrences_of("flag"), 0); + /// assert_eq!( + /// matches.get_one::("flag").copied(), + /// Some(2) + /// ); + /// ``` + Count, /// When encountered, display [`Command::print_help`][super::App::print_help] /// /// Depending on the flag, [`Command::print_long_help`][super::App::print_long_help] may be shown @@ -128,8 +205,37 @@ impl ArgAction { match self { Self::StoreValue => true, Self::IncOccurrence => false, + Self::SetTrue => false, + Self::SetFalse => false, + Self::Count => false, Self::Help => false, Self::Version => false, } } + + pub(crate) fn default_value_parser(&self) -> Option { + match self { + Self::StoreValue => None, + Self::IncOccurrence => None, + Self::SetTrue => Some(super::ValueParser::bool()), + Self::SetFalse => Some(super::ValueParser::bool()), + Self::Count => Some(crate::value_parser!(u64)), + Self::Help => None, + Self::Version => None, + } + } + + pub(crate) fn value_type_id(&self) -> Option { + match self { + Self::StoreValue => None, + Self::IncOccurrence => None, + Self::SetTrue => Some(AnyValueId::of::()), + Self::SetFalse => Some(AnyValueId::of::()), + Self::Count => Some(AnyValueId::of::()), + Self::Help => None, + Self::Version => None, + } + } } + +pub(crate) type CountType = u64; diff --git a/src/builder/arg.rs b/src/builder/arg.rs index d8892cef9d6..9f62e06241c 100644 --- a/src/builder/arg.rs +++ b/src/builder/arg.rs @@ -4900,7 +4900,9 @@ impl<'help> Arg<'help> { } if self.value_parser.is_none() { - if self.is_allow_invalid_utf8_set() { + if let Some(default) = self.action.as_ref().and_then(|a| a.default_value_parser()) { + self.value_parser = Some(default); + } else if self.is_allow_invalid_utf8_set() { self.value_parser = Some(super::ValueParser::os_string()); } else { self.value_parser = Some(super::ValueParser::string()); diff --git a/src/builder/debug_asserts.rs b/src/builder/debug_asserts.rs index 327887c4ed4..fd691c46760 100644 --- a/src/builder/debug_asserts.rs +++ b/src/builder/debug_asserts.rs @@ -649,6 +649,16 @@ fn assert_arg(arg: &Arg) { arg.name, arg.get_action() ); + if let Some(action_type_id) = arg.get_action().value_type_id() { + assert_eq!( + action_type_id, + arg.get_value_parser().type_id(), + "Argument `{}`'s selected action {:?} contradicts `value_parser` ({:?})", + arg.name, + arg.get_action(), + arg.get_value_parser() + ); + } if arg.get_value_hint() != ValueHint::Unknown { assert!( diff --git a/src/builder/mod.rs b/src/builder/mod.rs index 3c59b37350a..1f18ad51b0e 100644 --- a/src/builder/mod.rs +++ b/src/builder/mod.rs @@ -55,6 +55,7 @@ pub use command::App; #[cfg(feature = "regex")] pub use self::regex::RegexRef; +pub(crate) use action::CountType; pub(crate) use arg::display_arg_val; pub(crate) use arg_predicate::ArgPredicate; pub(crate) use value_parser::ValueParserInner; diff --git a/src/parser/parser.rs b/src/parser/parser.rs index 61b17eacf13..1105e1e36fd 100644 --- a/src/parser/parser.rs +++ b/src/parser/parser.rs @@ -1182,6 +1182,88 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { matcher.add_index_to(&arg.id, self.cur_idx.get()); Ok(ParseResult::ValuesDone) } + ArgAction::SetTrue => { + 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()); + } + let raw_vals = match raw_vals.len() { + 0 => { + vec![OsString::from("true")] + } + 1 => raw_vals, + _ => { + panic!( + "Argument {:?} received too many values: {:?}", + arg.id, raw_vals + ) + } + }; + + matcher.remove(&arg.id); + self.start_custom_arg(matcher, arg, source); + self.push_arg_values(arg, raw_vals, matcher)?; + Ok(ParseResult::ValuesDone) + } + ArgAction::SetFalse => { + 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()); + } + let raw_vals = match raw_vals.len() { + 0 => { + vec![OsString::from("false")] + } + 1 => raw_vals, + _ => { + panic!( + "Argument {:?} received too many values: {:?}", + arg.id, raw_vals + ) + } + }; + + matcher.remove(&arg.id); + self.start_custom_arg(matcher, arg, source); + self.push_arg_values(arg, raw_vals, matcher)?; + Ok(ParseResult::ValuesDone) + } + ArgAction::Count => { + 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()); + } + let raw_vals = match raw_vals.len() { + 0 => { + let existing_value = *matcher + .get_one::(arg.get_id()) + .unwrap_or(&0); + let next_value = existing_value + 1; + vec![OsString::from(next_value.to_string())] + } + 1 => raw_vals, + _ => { + panic!( + "Argument {:?} received too many values: {:?}", + arg.id, raw_vals + ) + } + }; + + matcher.remove(&arg.id); + self.start_custom_arg(matcher, arg, source); + self.push_arg_values(arg, raw_vals, matcher)?; + Ok(ParseResult::ValuesDone) + } ArgAction::Help => { debug_assert_eq!(raw_vals, Vec::::new()); let use_long = match ident { @@ -1278,6 +1360,15 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { )?; } } + ArgAction::SetTrue | ArgAction::SetFalse | ArgAction::Count => { + let _ = self.react( + None, + ValueSource::EnvVariable, + arg, + vec![val.to_os_str().into_owned()], + matcher, + )?; + } // Early return on `Help` or `Version`. ArgAction::Help | ArgAction::Version => { let _ = From 06ea57277051f08275cf445c079ef54eb5fd0039 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 31 May 2022 20:44:25 -0500 Subject: [PATCH 2/5] fix(parser): Apply conditional defaults Now that we can store constants for flags, we can apply defaults for flags too. Fixes #3294 --- src/parser/parser.rs | 7 +----- tests/builder/action.rs | 49 +++++++++++++++++++++++++++++++++++++++++ tests/builder/main.rs | 1 + 3 files changed, 51 insertions(+), 6 deletions(-) create mode 100644 tests/builder/action.rs diff --git a/src/parser/parser.rs b/src/parser/parser.rs index 1105e1e36fd..001a2e26ef8 100644 --- a/src/parser/parser.rs +++ b/src/parser/parser.rs @@ -1385,12 +1385,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { fn add_defaults(&self, matcher: &mut ArgMatcher) -> ClapResult<()> { debug!("Parser::add_defaults"); - for arg in self.cmd.get_opts() { - debug!("Parser::add_defaults:iter:{}:", arg.name); - self.add_default_value(arg, matcher)?; - } - - for arg in self.cmd.get_positionals() { + for arg in self.cmd.get_arguments() { debug!("Parser::add_defaults:iter:{}:", arg.name); self.add_default_value(arg, matcher)?; } diff --git a/tests/builder/action.rs b/tests/builder/action.rs new file mode 100644 index 00000000000..a86baad2020 --- /dev/null +++ b/tests/builder/action.rs @@ -0,0 +1,49 @@ +use clap::builder::ArgAction; +use clap::Arg; +use clap::Command; + +#[test] +fn set_true_with_default_value_if_present() { + let cmd = Command::new("test") + .arg( + Arg::new("mammal") + .long("mammal") + .action(ArgAction::SetTrue) + .default_value_if("dog", None, Some("true")), + ) + .arg(Arg::new("dog").long("dog").action(ArgAction::SetTrue)); + + let matches = cmd.clone().try_get_matches_from(["test", "--dog"]).unwrap(); + assert_eq!(*matches.get_one::("dog").unwrap(), true); + assert_eq!(*matches.get_one::("mammal").unwrap(), true); + + let matches = cmd + .clone() + .try_get_matches_from(["test", "--mammal"]) + .unwrap(); + assert_eq!(matches.get_one::("dog"), None); + assert_eq!(*matches.get_one::("mammal").unwrap(), true); +} + +#[test] +fn set_true_with_default_value_if_value() { + let cmd = Command::new("test") + .arg( + Arg::new("mammal") + .long("mammal") + .action(ArgAction::SetTrue) + .default_value_if("dog", Some("true"), Some("true")), + ) + .arg(Arg::new("dog").long("dog").action(ArgAction::SetTrue)); + + let matches = cmd.clone().try_get_matches_from(["test", "--dog"]).unwrap(); + assert_eq!(*matches.get_one::("dog").unwrap(), true); + assert_eq!(*matches.get_one::("mammal").unwrap(), true); + + let matches = cmd + .clone() + .try_get_matches_from(["test", "--mammal"]) + .unwrap(); + assert_eq!(matches.get_one::("dog"), None); + assert_eq!(*matches.get_one::("mammal").unwrap(), true); +} diff --git a/tests/builder/main.rs b/tests/builder/main.rs index 124111f1011..b034bba7af7 100644 --- a/tests/builder/main.rs +++ b/tests/builder/main.rs @@ -1,3 +1,4 @@ +mod action; mod app_from_crate; mod app_settings; mod arg_aliases; From 4afd1aafe5d7b16c729dd9df8782acd2437a2e37 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 31 May 2022 20:57:32 -0500 Subject: [PATCH 3/5] fix(parser): Don't double-increment index on flags --- src/parser/parser.rs | 21 ---- tests/builder/action.rs | 266 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 266 insertions(+), 21 deletions(-) diff --git a/src/parser/parser.rs b/src/parser/parser.rs index 001a2e26ef8..c3a407f7e87 100644 --- a/src/parser/parser.rs +++ b/src/parser/parser.rs @@ -1183,13 +1183,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { Ok(ParseResult::ValuesDone) } ArgAction::SetTrue => { - 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()); - } let raw_vals = match raw_vals.len() { 0 => { vec![OsString::from("true")] @@ -1209,13 +1202,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { Ok(ParseResult::ValuesDone) } ArgAction::SetFalse => { - 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()); - } let raw_vals = match raw_vals.len() { 0 => { vec![OsString::from("false")] @@ -1235,13 +1221,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { Ok(ParseResult::ValuesDone) } ArgAction::Count => { - 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()); - } let raw_vals = match raw_vals.len() { 0 => { let existing_value = *matcher diff --git a/tests/builder/action.rs b/tests/builder/action.rs index a86baad2020..8e634b1c6bd 100644 --- a/tests/builder/action.rs +++ b/tests/builder/action.rs @@ -2,6 +2,61 @@ use clap::builder::ArgAction; use clap::Arg; use clap::Command; +#[test] +fn set_true() { + let cmd = + Command::new("test").arg(Arg::new("mammal").long("mammal").action(ArgAction::SetTrue)); + + 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"]) + .unwrap(); + assert_eq!(*matches.get_one::("mammal").unwrap(), true); + assert_eq!(matches.is_present("mammal"), true); + assert_eq!(matches.occurrences_of("mammal"), 0); + assert_eq!(matches.index_of("mammal"), Some(1)); + + let matches = cmd + .clone() + .try_get_matches_from(["test", "--mammal", "--mammal"]) + .unwrap(); + assert_eq!(*matches.get_one::("mammal").unwrap(), true); + assert_eq!(matches.is_present("mammal"), true); + assert_eq!(matches.occurrences_of("mammal"), 0); + assert_eq!(matches.index_of("mammal"), Some(2)); +} + +#[test] +fn set_true_with_default_value() { + let cmd = Command::new("test").arg( + Arg::new("mammal") + .long("mammal") + .action(ArgAction::SetTrue) + .default_value("false"), + ); + + let matches = cmd + .clone() + .try_get_matches_from(["test", "--mammal"]) + .unwrap(); + assert_eq!(*matches.get_one::("mammal").unwrap(), true); + assert_eq!(matches.is_present("mammal"), true); + assert_eq!(matches.occurrences_of("mammal"), 0); + assert_eq!(matches.index_of("mammal"), Some(1)); + + let matches = cmd.clone().try_get_matches_from(["test"]).unwrap(); + assert_eq!(*matches.get_one::("mammal").unwrap(), false); + assert_eq!(matches.is_present("mammal"), true); + assert_eq!(matches.occurrences_of("mammal"), 0); + assert_eq!(matches.index_of("mammal"), Some(1)); +} + #[test] fn set_true_with_default_value_if_present() { let cmd = Command::new("test") @@ -47,3 +102,214 @@ fn set_true_with_default_value_if_value() { assert_eq!(matches.get_one::("dog"), None); assert_eq!(*matches.get_one::("mammal").unwrap(), true); } + +#[test] +fn set_false() { + let cmd = Command::new("test").arg( + Arg::new("mammal") + .long("mammal") + .action(ArgAction::SetFalse), + ); + + 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"]) + .unwrap(); + assert_eq!(*matches.get_one::("mammal").unwrap(), false); + assert_eq!(matches.is_present("mammal"), true); + assert_eq!(matches.occurrences_of("mammal"), 0); + assert_eq!(matches.index_of("mammal"), Some(1)); + + let matches = cmd + .clone() + .try_get_matches_from(["test", "--mammal", "--mammal"]) + .unwrap(); + assert_eq!(*matches.get_one::("mammal").unwrap(), false); + assert_eq!(matches.is_present("mammal"), true); + assert_eq!(matches.occurrences_of("mammal"), 0); + assert_eq!(matches.index_of("mammal"), Some(2)); +} + +#[test] +fn set_false_with_default_value() { + let cmd = Command::new("test").arg( + Arg::new("mammal") + .long("mammal") + .action(ArgAction::SetFalse) + .default_value("true"), + ); + + let matches = cmd + .clone() + .try_get_matches_from(["test", "--mammal"]) + .unwrap(); + assert_eq!(*matches.get_one::("mammal").unwrap(), false); + assert_eq!(matches.is_present("mammal"), true); + assert_eq!(matches.occurrences_of("mammal"), 0); + assert_eq!(matches.index_of("mammal"), Some(1)); + + let matches = cmd.clone().try_get_matches_from(["test"]).unwrap(); + assert_eq!(*matches.get_one::("mammal").unwrap(), true); + assert_eq!(matches.is_present("mammal"), true); + assert_eq!(matches.occurrences_of("mammal"), 0); + assert_eq!(matches.index_of("mammal"), Some(1)); +} + +#[test] +fn set_false_with_default_value_if_present() { + let cmd = Command::new("test") + .arg( + Arg::new("mammal") + .long("mammal") + .action(ArgAction::SetFalse) + .default_value_if("dog", None, Some("false")), + ) + .arg(Arg::new("dog").long("dog").action(ArgAction::SetFalse)); + + let matches = cmd.clone().try_get_matches_from(["test", "--dog"]).unwrap(); + assert_eq!(*matches.get_one::("dog").unwrap(), false); + assert_eq!(*matches.get_one::("mammal").unwrap(), false); + + let matches = cmd + .clone() + .try_get_matches_from(["test", "--mammal"]) + .unwrap(); + assert_eq!(matches.get_one::("dog"), None); + assert_eq!(*matches.get_one::("mammal").unwrap(), false); +} + +#[test] +fn set_false_with_default_value_if_value() { + let cmd = Command::new("test") + .arg( + Arg::new("mammal") + .long("mammal") + .action(ArgAction::SetFalse) + .default_value_if("dog", Some("false"), Some("false")), + ) + .arg(Arg::new("dog").long("dog").action(ArgAction::SetFalse)); + + let matches = cmd.clone().try_get_matches_from(["test", "--dog"]).unwrap(); + assert_eq!(*matches.get_one::("dog").unwrap(), false); + assert_eq!(*matches.get_one::("mammal").unwrap(), false); + + let matches = cmd + .clone() + .try_get_matches_from(["test", "--mammal"]) + .unwrap(); + assert_eq!(matches.get_one::("dog"), None); + assert_eq!(*matches.get_one::("mammal").unwrap(), false); +} + +#[test] +fn count() { + let cmd = Command::new("test").arg(Arg::new("mammal").long("mammal").action(ArgAction::Count)); + + 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"]) + .unwrap(); + assert_eq!(*matches.get_one::("mammal").unwrap(), 1); + assert_eq!(matches.is_present("mammal"), true); + assert_eq!(matches.occurrences_of("mammal"), 0); + assert_eq!(matches.index_of("mammal"), Some(1)); + + let matches = cmd + .clone() + .try_get_matches_from(["test", "--mammal", "--mammal"]) + .unwrap(); + assert_eq!(*matches.get_one::("mammal").unwrap(), 2); + assert_eq!(matches.is_present("mammal"), true); + assert_eq!(matches.occurrences_of("mammal"), 0); + assert_eq!(matches.index_of("mammal"), Some(2)); +} + +#[test] +fn count_with_default_value() { + let cmd = Command::new("test").arg( + Arg::new("mammal") + .long("mammal") + .action(ArgAction::Count) + .default_value("10"), + ); + + let matches = cmd + .clone() + .try_get_matches_from(["test", "--mammal"]) + .unwrap(); + assert_eq!(*matches.get_one::("mammal").unwrap(), 1); + assert_eq!(matches.is_present("mammal"), true); + assert_eq!(matches.occurrences_of("mammal"), 0); + assert_eq!(matches.index_of("mammal"), Some(1)); + + let matches = cmd.clone().try_get_matches_from(["test"]).unwrap(); + assert_eq!(*matches.get_one::("mammal").unwrap(), 10); + assert_eq!(matches.is_present("mammal"), true); + assert_eq!(matches.occurrences_of("mammal"), 0); + assert_eq!(matches.index_of("mammal"), Some(1)); +} + +#[test] +fn count_with_default_value_if_present() { + let cmd = Command::new("test") + .arg( + Arg::new("mammal") + .long("mammal") + .action(ArgAction::Count) + .default_value_if("dog", None, Some("10")), + ) + .arg(Arg::new("dog").long("dog").action(ArgAction::Count)); + + let matches = cmd.clone().try_get_matches_from(["test", "--dog"]).unwrap(); + assert_eq!(*matches.get_one::("dog").unwrap(), 1); + assert_eq!(*matches.get_one::("mammal").unwrap(), 10); + + let matches = cmd + .clone() + .try_get_matches_from(["test", "--mammal"]) + .unwrap(); + assert_eq!(matches.get_one::("dog"), None); + assert_eq!(*matches.get_one::("mammal").unwrap(), 1); +} + +#[test] +fn count_with_default_value_if_value() { + let cmd = Command::new("test") + .arg( + Arg::new("mammal") + .long("mammal") + .action(ArgAction::Count) + .default_value_if("dog", Some("2"), Some("10")), + ) + .arg(Arg::new("dog").long("dog").action(ArgAction::Count)); + + let matches = cmd.clone().try_get_matches_from(["test", "--dog"]).unwrap(); + assert_eq!(*matches.get_one::("dog").unwrap(), 1); + assert_eq!(matches.get_one::("mammal"), None); + + let matches = cmd + .clone() + .try_get_matches_from(["test", "--dog", "--dog"]) + .unwrap(); + assert_eq!(*matches.get_one::("dog").unwrap(), 2); + assert_eq!(*matches.get_one::("mammal").unwrap(), 10); + + let matches = cmd + .clone() + .try_get_matches_from(["test", "--mammal"]) + .unwrap(); + assert_eq!(matches.get_one::("dog"), None); + assert_eq!(*matches.get_one::("mammal").unwrap(), 1); +} From 2e9e5563596d6c8826630bf24fa12961da56e1ee Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 31 May 2022 21:07:20 -0500 Subject: [PATCH 4/5] test(parser): Ensure conditional requirements work with new Actions --- tests/builder/action.rs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/builder/action.rs b/tests/builder/action.rs index 8e634b1c6bd..4edde7c729f 100644 --- a/tests/builder/action.rs +++ b/tests/builder/action.rs @@ -103,6 +103,36 @@ fn set_true_with_default_value_if_value() { assert_eq!(*matches.get_one::("mammal").unwrap(), true); } +#[test] +fn set_true_with_required_if_eq() { + let cmd = Command::new("test") + .arg( + Arg::new("mammal") + .long("mammal") + .action(ArgAction::SetTrue) + .required_if_eq("dog", "true"), + ) + .arg(Arg::new("dog").long("dog").action(ArgAction::SetTrue)); + + let matches = cmd + .clone() + .try_get_matches_from(["test", "--mammal"]) + .unwrap(); + assert_eq!(matches.get_one::("dog"), None); + assert_eq!(*matches.get_one::("mammal").unwrap(), true); + + cmd.clone() + .try_get_matches_from(["test", "--dog"]) + .unwrap_err(); + + let matches = cmd + .clone() + .try_get_matches_from(["test", "--dog", "--mammal"]) + .unwrap(); + assert_eq!(*matches.get_one::("dog").unwrap(), true); + assert_eq!(*matches.get_one::("mammal").unwrap(), true); +} + #[test] fn set_false() { let cmd = Command::new("test").arg( From c58a802a1db62c0521dc8d27065666f824dd81ce Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 1 Jun 2022 06:37:53 -0500 Subject: [PATCH 5/5] style: Make clippy happy --- src/builder/action.rs | 7 ++++--- tests/builder/action.rs | 2 ++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/builder/action.rs b/src/builder/action.rs index 0bbeb14956b..40b67d427bb 100644 --- a/src/builder/action.rs +++ b/src/builder/action.rs @@ -1,5 +1,3 @@ -use crate::parser::AnyValueId; - /// Behavior of arguments when they are encountered while parsing /// /// # Examples @@ -225,7 +223,10 @@ impl ArgAction { } } - pub(crate) fn value_type_id(&self) -> Option { + #[cfg(debug_assertions)] + pub(crate) fn value_type_id(&self) -> Option { + use crate::parser::AnyValueId; + match self { Self::StoreValue => None, Self::IncOccurrence => None, diff --git a/tests/builder/action.rs b/tests/builder/action.rs index 4edde7c729f..bdac85b7ddb 100644 --- a/tests/builder/action.rs +++ b/tests/builder/action.rs @@ -1,3 +1,5 @@ +#![allow(clippy::bool_assert_comparison)] + use clap::builder::ArgAction; use clap::Arg; use clap::Command;