diff --git a/CHANGELOG.md b/CHANGELOG.md index de5c445ea1bf..4884293d0264 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - `arg!` now sets `ArgAction::SetTrue`, `ArgAction::Count`, `ArgAction::Set`, or `ArgAction::Append` as appropriate (#3795) - Default actions are now `Set` and `SetTrue` - Removed `PartialEq` and `Eq` from `Command` +- By default, an `Arg`s default action is `ArgAction::Set`, rather than `ArgAction::SetTrue` - Replace `Arg::number_of_values` (average across occurrences) with `Arg::num_args` (per occurrence, raw CLI args, not parsed values) - `num_args(0)` no longer implies `takes_value(true).multiple_values(true)` - `num_args(1)` no longer implies `multiple_values(true)` diff --git a/src/builder/arg.rs b/src/builder/arg.rs index fa7bb4af5c6c..9cad874d2b92 100644 --- a/src/builder/arg.rs +++ b/src/builder/arg.rs @@ -778,16 +778,6 @@ impl<'help> Arg<'help> { /// # Value Handling impl<'help> Arg<'help> { - #[inline] - #[must_use] - fn takes_value(self, yes: bool) -> Self { - if yes { - self.setting(ArgSettings::TakesValue) - } else { - self.unset_setting(ArgSettings::TakesValue) - } - } - /// Specify the behavior when parsing an argument /// /// # Examples @@ -1024,7 +1014,7 @@ impl<'help> Arg<'help> { pub fn num_args(mut self, qty: impl Into) -> Self { let qty = qty.into(); self.num_vals = Some(qty); - self.takes_value(qty.takes_values()) + self } /// Placeholder for the argument's value in the help message / usage. @@ -1134,7 +1124,7 @@ impl<'help> Arg<'help> { #[must_use] pub fn value_names(mut self, names: &[&'help str]) -> Self { self.val_names = names.to_vec(); - self.takes_value(true) + self } /// Provide the shell a hint about how to complete this argument. @@ -1169,7 +1159,7 @@ impl<'help> Arg<'help> { #[must_use] pub fn value_hint(mut self, value_hint: ValueHint) -> Self { self.value_hint = Some(value_hint); - self.takes_value(true) + self } /// Match values against [`PossibleValuesParser`][crate::builder::PossibleValuesParser] without matching case. @@ -1370,7 +1360,7 @@ impl<'help> Arg<'help> { #[must_use] pub fn value_delimiter(mut self, d: impl Into>) -> Self { self.val_delim = d.into(); - self.takes_value(true) + self } /// Sentinel to **stop** parsing multiple values of a given argument. @@ -1423,7 +1413,7 @@ impl<'help> Arg<'help> { #[must_use] pub fn value_terminator(mut self, term: &'help str) -> Self { self.terminator = Some(term); - self.takes_value(true) + self } /// Consume all following arguments. @@ -1451,8 +1441,10 @@ impl<'help> Arg<'help> { #[inline] #[must_use] pub fn raw(mut self, yes: bool) -> Self { - self.num_vals.get_or_insert_with(|| (1..).into()); - self.takes_value(yes).allow_hyphen_values(yes).last(yes) + if yes { + self.num_vals.get_or_insert_with(|| (1..).into()); + } + self.allow_hyphen_values(yes).last(yes) } /// Value for the argument when not present. @@ -1549,7 +1541,7 @@ impl<'help> Arg<'help> { #[must_use] pub fn default_values_os(mut self, vals: &[&'help OsStr]) -> Self { self.default_vals = vals.to_vec(); - self.takes_value(true) + self } /// Value for the argument when the flag is present but no value is specified. @@ -1681,7 +1673,7 @@ impl<'help> Arg<'help> { #[must_use] pub fn default_missing_values_os(mut self, vals: &[&'help OsStr]) -> Self { self.default_missing_vals = vals.to_vec(); - self.takes_value(true) + self } /// Read from `name` environment variable when argument is not present. @@ -1733,7 +1725,7 @@ impl<'help> Arg<'help> { /// /// ```rust /// # use std::env; - /// # use clap::{Command, Arg}; + /// # use clap::{Command, Arg, ArgAction}; /// # use clap::builder::FalseyValueParser; /// /// env::set_var("TRUE_FLAG", "true"); @@ -1742,14 +1734,17 @@ impl<'help> Arg<'help> { /// let m = Command::new("prog") /// .arg(Arg::new("true_flag") /// .long("true_flag") + /// .action(ArgAction::SetTrue) /// .value_parser(FalseyValueParser::new()) /// .env("TRUE_FLAG")) /// .arg(Arg::new("false_flag") /// .long("false_flag") + /// .action(ArgAction::SetTrue) /// .value_parser(FalseyValueParser::new()) /// .env("FALSE_FLAG")) /// .arg(Arg::new("absent_flag") /// .long("absent_flag") + /// .action(ArgAction::SetTrue) /// .value_parser(FalseyValueParser::new()) /// .env("ABSENT_FLAG")) /// .get_matches_from(vec![ @@ -2621,7 +2616,7 @@ impl<'help> Arg<'help> { ) -> Self { self.default_vals_ifs .push((arg_id.into(), val.into(), default)); - self.takes_value(true) + self } /// Specifies multiple values and conditions in the same manner as [`Arg::default_value_if`]. @@ -3401,7 +3396,8 @@ impl<'help> Arg<'help> { /// .conflicts_with("debug") /// .long("config")) /// .arg(Arg::new("debug") - /// .long("debug")) + /// .long("debug") + /// .action(ArgAction::SetTrue)) /// .try_get_matches_from(vec![ /// "prog", "--debug", "--config", "file.conf" /// ]); @@ -3782,7 +3778,7 @@ impl<'help> Arg<'help> { } pub(crate) fn is_takes_value_set(&self) -> bool { - self.is_set(ArgSettings::TakesValue) + self.get_action().takes_values() } /// Report whether [`Arg::allow_hyphen_values`] is set @@ -3891,17 +3887,21 @@ impl<'help> Arg<'help> { /// # Internally used only impl<'help> Arg<'help> { pub(crate) fn _build(&mut self) { - if self.is_positional() { - self.settings.set(ArgSettings::TakesValue); - } if self.action.is_none() { - if self.get_id() == "help" && !self.is_takes_value_set() { + if self.get_id() == "help" + && matches!(self.get_num_args(), Some(ValueRange::EMPTY) | None) + { let action = super::ArgAction::Help; self.action = Some(action); - } else if self.get_id() == "version" && !self.is_takes_value_set() { + } else if self.get_id() == "version" + && matches!(self.get_num_args(), Some(ValueRange::EMPTY) | None) + { let action = super::ArgAction::Version; self.action = Some(action); - } else if self.is_takes_value_set() { + } else if self.num_vals == Some(ValueRange::EMPTY) { + let action = super::ArgAction::SetTrue; + self.action = Some(action); + } else { let action = if self.is_positional() && self.is_multiple_values_set() { // Allow collecting arguments interleaved with flags super::ArgAction::Append @@ -3909,9 +3909,6 @@ impl<'help> Arg<'help> { super::ArgAction::Set }; self.action = Some(action); - } else { - let action = super::ArgAction::SetTrue; - self.action = Some(action); } } if let Some(action) = self.action.as_ref() { @@ -3925,11 +3922,6 @@ impl<'help> Arg<'help> { self.default_missing_vals = vec![default_value]; } } - if action.takes_values() { - self.settings.set(ArgSettings::TakesValue); - } else { - self.settings.unset(ArgSettings::TakesValue); - } } if self.value_parser.is_none() { @@ -3944,11 +3936,12 @@ impl<'help> Arg<'help> { if val_names_len > 1 { self.num_vals.get_or_insert(val_names_len.into()); } else { - if self.is_takes_value_set() { - self.num_vals.get_or_insert(ValueRange::SINGLE); + let nargs = if self.get_action().takes_values() { + ValueRange::SINGLE } else { - self.num_vals.get_or_insert(ValueRange::EMPTY); - } + ValueRange::EMPTY + }; + self.num_vals.get_or_insert(nargs); } } diff --git a/src/builder/arg_settings.rs b/src/builder/arg_settings.rs index 693b8b6db51e..7ea5e6f8b575 100644 --- a/src/builder/arg_settings.rs +++ b/src/builder/arg_settings.rs @@ -30,7 +30,6 @@ pub(crate) enum ArgSettings { Required, Global, Hidden, - TakesValue, NextLineHelp, HidePossibleValues, AllowHyphenValues, @@ -52,7 +51,6 @@ bitflags! { const REQUIRED = 1; const GLOBAL = 1 << 3; const HIDDEN = 1 << 4; - const TAKES_VAL = 1 << 5; const NEXT_LINE_HELP = 1 << 7; const DELIM_NOT_SET = 1 << 10; const HIDE_POS_VALS = 1 << 11; @@ -76,7 +74,6 @@ impl_settings! { ArgSettings, ArgFlags, Required => Flags::REQUIRED, Global => Flags::GLOBAL, Hidden => Flags::HIDDEN, - TakesValue => Flags::TAKES_VAL, NextLineHelp => Flags::NEXT_LINE_HELP, HidePossibleValues => Flags::HIDE_POS_VALS, AllowHyphenValues => Flags::ALLOW_TAC_VALS, diff --git a/src/builder/range.rs b/src/builder/range.rs index 808a6f34ce15..54d19e322558 100644 --- a/src/builder/range.rs +++ b/src/builder/range.rs @@ -108,7 +108,7 @@ impl std::ops::RangeBounds for ValueRange { impl Default for ValueRange { fn default() -> Self { - Self::EMPTY + Self::SINGLE } } diff --git a/tests/builder/help.rs b/tests/builder/help.rs index 93c4006cc0f3..e61d5c714e47 100644 --- a/tests/builder/help.rs +++ b/tests/builder/help.rs @@ -401,21 +401,25 @@ OPTIONS: Arg::new("all") .short('a') .long("all") + .action(ArgAction::SetTrue) .help("Also do versioning for private crates (will not be published)"), ) .arg( Arg::new("exact") .long("exact") + .action(ArgAction::SetTrue) .help("Specify inter dependency version numbers exactly with `=`"), ) .arg( Arg::new("no_git_commit") .long("no-git-commit") + .action(ArgAction::SetTrue) .help("Do not commit version changes"), ) .arg( Arg::new("no_git_push") .long("no-git-push") + .action(ArgAction::SetTrue) .help("Do not push generated commit and tags to git remote"), ); utils::assert_output(cmd, "test --help", WRAPPED_HELP, false); @@ -445,21 +449,25 @@ OPTIONS: Arg::new("all") .short('a') .long("all") + .action(ArgAction::SetTrue) .help("Also do versioning for private crates (will not be published)"), ) .arg( Arg::new("exact") .long("exact") + .action(ArgAction::SetTrue) .help("Specify inter dependency version numbers exactly with `=`"), ) .arg( Arg::new("no_git_commit") .long("no-git-commit") + .action(ArgAction::SetTrue) .help("Do not commit version changes"), ) .arg( Arg::new("no_git_push") .long("no-git-push") + .action(ArgAction::SetTrue) .help("Do not push generated commit and tags to git remote"), ); utils::assert_output(cmd, "test --help", UNWRAPPED_HELP, false); diff --git a/tests/builder/help_env.rs b/tests/builder/help_env.rs index 8ab74c7d7dc0..4407709c0e99 100644 --- a/tests/builder/help_env.rs +++ b/tests/builder/help_env.rs @@ -172,6 +172,7 @@ fn hide_env_flag() { Arg::new("cafe") .short('c') .long("cafe") + .action(ArgAction::SetTrue) .hide_env(true) .env("ENVVAR") .help("A coffeehouse, coffee shop, or café."), @@ -188,6 +189,7 @@ fn show_env_flag() { Arg::new("cafe") .short('c') .long("cafe") + .action(ArgAction::SetTrue) .env("ENVVAR") .help("A coffeehouse, coffee shop, or café."), ); @@ -203,6 +205,7 @@ fn hide_env_vals_flag() { Arg::new("cafe") .short('c') .long("cafe") + .action(ArgAction::SetTrue) .hide_env_values(true) .env("ENVVAR") .help("A coffeehouse, coffee shop, or café."), @@ -219,6 +222,7 @@ fn show_env_vals_flag() { Arg::new("cafe") .short('c') .long("cafe") + .action(ArgAction::SetTrue) .env("ENVVAR") .help("A coffeehouse, coffee shop, or café."), );