Skip to content

Commit

Permalink
Merge pull request clap-rs#4028 from epage/value
Browse files Browse the repository at this point in the history
fix!: Replace takes_value with number_of_values
  • Loading branch information
epage committed Aug 4, 2022
2 parents 14c8f33 + c62d3f0 commit 91b1055
Show file tree
Hide file tree
Showing 10 changed files with 46 additions and 78 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Replace `Arg::min_values` (across all occurrences) with `Arg::num_args(N..)` (per occurrence)
- Replace `Arg::max_values` (across all occurrences) with `Arg::num_args(1..=M)` (per occurrence)
- Replace `Arg::multiple_values(true)` with `Arg::num_args(1..)` and `Arg::multiple_values(false)` with `Arg::num_args(0)`
- Remove `Arg::takes_value(true)` with `Arg::number_of_values(1)` and `Arg::takes_value(false)` with `Arg::number_of_values(0)`
- Remove `Arg::use_value_delimiter` in favor of `Arg::value_delimiter`
- Remove `Arg::require_value_delimiter`, either users could use `Arg::value_delimiter` or implement a custom parser with `TypedValueParser`
- `ArgAction::SetTrue` and `ArgAction::SetFalse` now prioritize `Arg::default_missing_value` over their standard behavior
Expand Down
69 changes: 22 additions & 47 deletions src/builder/arg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ impl<'help> Arg<'help> {
/// Arg::new("config")
/// # ;
/// ```
/// [`Arg::action(ArgAction::Set)`]: Arg::takes_value()
/// [`Arg::action(ArgAction::Set)`]: Arg::action()
pub fn new<S: Into<&'help str>>(n: S) -> Self {
Arg::default().id(n)
}
Expand Down Expand Up @@ -474,7 +474,7 @@ impl<'help> Arg<'help> {
///
/// **NOTE**: This setting only applies to positional arguments, and has no effect on OPTIONS
///
/// **NOTE:** Setting this requires [`Arg::takes_value`]
/// **NOTE:** Setting this requires [taking values][Arg::num_args]
///
/// **CAUTION:** Using this setting *and* having child subcommands is not
/// recommended with the exception of *also* using
Expand Down Expand Up @@ -778,33 +778,9 @@ impl<'help> Arg<'help> {

/// # Value Handling
impl<'help> Arg<'help> {
/// Specifies that the argument takes a value at run time.
///
/// **NOTE:** values for arguments may be specified in any of the following methods
///
/// - Using a space such as `-o value` or `--option value`
/// - Using an equals and no space such as `-o=value` or `--option=value`
/// - Use a short and no space such as `-ovalue`
///
/// # Examples
///
/// ```rust
/// # use clap::{Command, Arg, ArgAction};
/// let m = Command::new("prog")
/// .arg(Arg::new("mode")
/// .long("mode")
/// .action(ArgAction::Set))
/// .get_matches_from(vec![
/// "prog", "--mode", "fast"
/// ]);
///
/// assert!(m.contains_id("mode"));
/// assert_eq!(m.get_one::<String>("mode").unwrap(), "fast");
/// ```
/// [multiple values]: Arg::num_args
#[inline]
#[must_use]
pub fn takes_value(self, yes: bool) -> Self {
fn takes_value(self, yes: bool) -> Self {
if yes {
self.setting(ArgSettings::TakesValue)
} else {
Expand Down Expand Up @@ -1105,9 +1081,8 @@ impl<'help> Arg<'help> {
/// -h, --help Print help information
/// -V, --version Print version information
/// ```
/// [option]: Arg::takes_value()
/// [positional]: Arg::index()
/// [`Arg::action(ArgAction::Set)`]: Arg::takes_value()
/// [`Arg::action(ArgAction::Set)`]: Arg::action()
#[inline]
#[must_use]
pub fn value_name(self, name: &'help str) -> Self {
Expand Down Expand Up @@ -1165,7 +1140,7 @@ impl<'help> Arg<'help> {
/// ```
/// [`Arg::next_line_help(true)`]: Arg::next_line_help()
/// [`Arg::num_args`]: Arg::num_args()
/// [`Arg::action(ArgAction::Set)`]: Arg::takes_value()
/// [`Arg::action(ArgAction::Set)`]: Arg::action()
/// [`Arg::num_args(1..)`]: Arg::num_args()
#[must_use]
pub fn value_names(mut self, names: &[&'help str]) -> Self {
Expand Down Expand Up @@ -1216,7 +1191,7 @@ impl<'help> Arg<'help> {
/// [`Arg::required_if_eq_all`] is case-insensitive.
///
///
/// **NOTE:** Setting this requires [`Arg::takes_value`]
/// **NOTE:** Setting this requires [taking values][Arg::num_args]
///
/// **NOTE:** To do unicode case folding, enable the `unicode` feature flag.
///
Expand Down Expand Up @@ -1268,7 +1243,7 @@ impl<'help> Arg<'help> {

/// Allows values which start with a leading hyphen (`-`)
///
/// **NOTE:** Setting this requires [`Arg::takes_value`]
/// **NOTE:** Setting this requires [taking values][Arg::num_args]
///
/// **WARNING**: Take caution when using this setting combined with
/// [`Arg::num_args`], as this becomes ambiguous `$ prog --arg -- -- val`. All
Expand Down Expand Up @@ -1329,7 +1304,7 @@ impl<'help> Arg<'help> {
///
/// i.e. an equals between the option and associated value.
///
/// **NOTE:** Setting this requires [`Arg::takes_value`]
/// **NOTE:** Setting this requires [taking values][Arg::num_args]
///
/// # Examples
///
Expand Down Expand Up @@ -1400,8 +1375,8 @@ impl<'help> Arg<'help> {
///
/// assert_eq!(m.get_many::<String>("config").unwrap().collect::<Vec<_>>(), ["val1", "val2", "val3"])
/// ```
/// [`Arg::value_delimiter(',')`]: Arg::use_value_delimiter()
/// [`Arg::action(ArgAction::Set)`]: Arg::takes_value()
/// [`Arg::value_delimiter(',')`]: Arg::value_delimiter()
/// [`Arg::action(ArgAction::Set)`]: Arg::action()
#[inline]
#[must_use]
pub fn value_delimiter(mut self, d: impl Into<Option<char>>) -> Self {
Expand Down Expand Up @@ -1451,7 +1426,7 @@ impl<'help> Arg<'help> {
/// assert_eq!(&cmds, &["find", "-type", "f", "-name", "special"]);
/// assert_eq!(m.get_one::<String>("location").unwrap(), "/home/clap");
/// ```
/// [options]: Arg::takes_value()
/// [options]: Arg::action
/// [positional arguments]: Arg::index()
/// [`num_args(1..)`]: Arg::num_args()
/// [`num_args`]: Arg::num_args()
Expand Down Expand Up @@ -1480,7 +1455,7 @@ impl<'help> Arg<'help> {
/// **NOTE:** Implicitly sets [`Arg::action(ArgAction::Set)`] [`Arg::num_args(1..)`],
/// [`Arg::allow_hyphen_values(true)`], and [`Arg::last(true)`] when set to `true`.
///
/// [`Arg::action(ArgAction::Set)`]: Arg::takes_value()
/// [`Arg::action(ArgAction::Set)`]: Arg::action()
/// [`Arg::num_args(1..)`]: Arg::num_args()
/// [`Arg::allow_hyphen_values(true)`]: Arg::allow_hyphen_values()
/// [`Arg::last(true)`]: Arg::last()
Expand Down Expand Up @@ -1544,7 +1519,7 @@ impl<'help> Arg<'help> {
/// assert!(m.contains_id("opt"));
/// assert_eq!(m.value_source("opt"), Some(ValueSource::CommandLine));
/// ```
/// [`Arg::action(ArgAction::Set)`]: Arg::takes_value()
/// [`Arg::action(ArgAction::Set)`]: Arg::action()
/// [`ArgMatches::contains_id`]: crate::ArgMatches::contains_id()
/// [`Arg::default_value_if`]: Arg::default_value_if()
#[inline]
Expand Down Expand Up @@ -1677,7 +1652,7 @@ impl<'help> Arg<'help> {
/// assert_eq!(m.value_source("create"), Some(ValueSource::CommandLine));
/// ```
///
/// [`Arg::action(ArgAction::Set)`]: Arg::takes_value()
/// [`Arg::action(ArgAction::Set)`]: Arg::action()
/// [`Arg::default_value`]: Arg::default_value()
#[inline]
#[must_use]
Expand Down Expand Up @@ -1761,8 +1736,8 @@ impl<'help> Arg<'help> {
/// assert_eq!(m.get_one::<String>("flag").unwrap(), "env");
/// ```
///
/// In this example, because [`Arg::takes_value(false)`] (by default),
/// `prog` is a flag that accepts an optional, case-insensitive boolean literal.
/// In this example, because `prog` is a flag that accepts an optional, case-insensitive
/// boolean literal.
///
/// Note that the value parser controls how flags are parsed. In this case we've selected
/// [`FalseyValueParser`][crate::builder::FalseyValueParser]. A `false` literal is `n`, `no`,
Expand Down Expand Up @@ -1862,8 +1837,8 @@ impl<'help> Arg<'help> {
///
/// assert_eq!(m.get_many::<String>("flag").unwrap().collect::<Vec<_>>(), vec!["env1", "env2"]);
/// ```
/// [`Arg::action(ArgAction::Set)`]: Arg::takes_value()
/// [`Arg::value_delimiter(',')`]: Arg::use_value_delimiter()
/// [`Arg::action(ArgAction::Set)`]: Arg::action()
/// [`Arg::value_delimiter(',')`]: Arg::value_delimiter()
#[cfg(feature = "env")]
#[inline]
#[must_use]
Expand Down Expand Up @@ -2169,7 +2144,7 @@ impl<'help> Arg<'help> {
/// This is useful for args with many values, or ones which are explained elsewhere in the
/// help text.
///
/// **NOTE:** Setting this requires [`Arg::takes_value`]
/// **NOTE:** Setting this requires [taking values][Arg::num_args]
///
/// To set this for all arguments, see
/// [`Command::hide_possible_values`][crate::Command::hide_possible_values].
Expand Down Expand Up @@ -2201,7 +2176,7 @@ impl<'help> Arg<'help> {
///
/// This is useful when default behavior of an arg is explained elsewhere in the help text.
///
/// **NOTE:** Setting this requires [`Arg::takes_value`]
/// **NOTE:** Setting this requires [taking values][Arg::num_args]
///
/// # Examples
///
Expand Down Expand Up @@ -2633,7 +2608,7 @@ impl<'help> Arg<'help> {
///
/// assert_eq!(m.get_one::<String>("other"), None);
/// ```
/// [`Arg::action(ArgAction::Set)`]: Arg::takes_value()
/// [`Arg::action(ArgAction::Set)`]: Arg::action()
/// [`Arg::default_value`]: Arg::default_value()
#[must_use]
pub fn default_value_if<T: Key>(
Expand Down Expand Up @@ -2740,7 +2715,7 @@ impl<'help> Arg<'help> {
///
/// assert_eq!(m.get_one::<String>("other").unwrap(), "default");
/// ```
/// [`Arg::action(ArgAction::Set)`]: Arg::takes_value()
/// [`Arg::action(ArgAction::Set)`]: Arg::action()
/// [`Arg::default_value_if`]: Arg::default_value_if()
#[must_use]
pub fn default_value_ifs<T: Key>(
Expand Down
4 changes: 2 additions & 2 deletions src/builder/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2016,7 +2016,7 @@ impl<'help> Command<'help> {
///
/// The values of the trailing positional argument will contain all args from itself on.
///
/// **NOTE:** The final positional argument **must** have [`Arg::number_of_values(..)`].
/// **NOTE:** The final positional argument **must** have [`Arg::num_args(..)`].
///
/// # Examples
///
Expand All @@ -2030,7 +2030,7 @@ impl<'help> Command<'help> {
/// let trail: Vec<_> = m.get_many::<String>("cmd").unwrap().collect();
/// assert_eq!(trail, ["arg1", "-r", "val1"]);
/// ```
/// [`Arg::number_of_values(true)`]: crate::Arg::number_of_values()
/// [`Arg::num_args(..)`]: crate::Arg::num_args()
pub fn trailing_var_arg(self, yes: bool) -> Self {
if yes {
self.setting(AppSettings::TrailingVarArg)
Expand Down
16 changes: 8 additions & 8 deletions src/builder/debug_asserts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ pub(crate) fn assert_app(cmd: &Command) {
panic!(
"Command {}: Argument '{}' has the same index as '{}' \
and they are both positional arguments\n\n\t \
Use Arg::number_of_values(1..) to allow one \
Use `Arg::num_args(1..)` to allow one \
positional argument to take multiple values",
cmd.get_name(),
first.name,
Expand Down Expand Up @@ -517,7 +517,7 @@ fn _verify_positionals(cmd: &Command) -> bool {
|| last.is_last_set();
assert!(
ok,
"When using a positional argument with .num_args(1..) that is *not the \
"When using a positional argument with `.num_args(1..)` that is *not the \
last* positional argument, the last positional argument (i.e. the one \
with the highest index) *must* have .required(true) or .last(true) set."
);
Expand All @@ -527,7 +527,7 @@ fn _verify_positionals(cmd: &Command) -> bool {
assert!(
ok,
"Only the last positional argument, or second to last positional \
argument may be set to .num_args(1..)"
argument may be set to `.num_args(1..)`"
);

// Next we check how many have both Multiple and not a specific number of values set
Expand All @@ -545,7 +545,7 @@ fn _verify_positionals(cmd: &Command) -> bool {
&& count == 2);
assert!(
ok,
"Only one positional argument with .num_args(1..) set is allowed per \
"Only one positional argument with `.num_args(1..)` set is allowed per \
command, unless the second one also has .last(true) set"
);
}
Expand Down Expand Up @@ -697,7 +697,7 @@ fn assert_arg(arg: &Arg) {
let num_val_names = arg.get_value_names().unwrap_or(&[]).len();
if num_vals.max_values() < num_val_names {
panic!(
"Argument {}: Too many value names ({}) compared to number_of_values ({})",
"Argument {}: Too many value names ({}) compared to `num_args` ({})",
arg.name, num_val_names, num_vals
);
}
Expand All @@ -706,14 +706,14 @@ fn assert_arg(arg: &Arg) {
assert_eq!(
num_vals.takes_values(),
arg.is_takes_value_set(),
"Argument {}: mismatch between `number_of_values` ({}) and `takes_value`",
"Argument {}: mismatch between `num_args` ({}) and `takes_value`",
arg.name,
num_vals,
);
assert_eq!(
num_vals.is_multiple(),
arg.is_multiple_values_set(),
"Argument {}: mismatch between `number_of_values` ({}) and `multiple_values`",
"Argument {}: mismatch between `num_args` ({}) and `multiple_values`",
arg.name,
num_vals,
);
Expand All @@ -730,7 +730,7 @@ fn assert_arg(arg: &Arg) {
if arg.get_num_args() == Some(1.into()) {
assert!(
!arg.is_multiple_values_set(),
"Argument {}: mismatch between `number_of_values` and `multiple_values`",
"Argument {}: mismatch between `num_args` and `multiple_values`",
arg.name
);
}
Expand Down
8 changes: 0 additions & 8 deletions src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,6 @@ macro_rules! arg_impl {
arg
.long(long)
.action($crate::ArgAction::SetTrue)
.takes_value(false)
})
$($tail)*
}
Expand All @@ -235,7 +234,6 @@ macro_rules! arg_impl {
arg
.long(long)
.action($crate::ArgAction::SetTrue)
.takes_value(false)
})
$($tail)*
}
Expand All @@ -256,7 +254,6 @@ macro_rules! arg_impl {
$arg
.short($crate::arg_impl! { @char $short })
.action($crate::ArgAction::SetTrue)
.takes_value(false)
})
$($tail)*
}
Expand All @@ -277,7 +274,6 @@ macro_rules! arg_impl {
$arg
.short($crate::arg_impl! { @char $short })
.action($crate::ArgAction::SetTrue)
.takes_value(false)
})
$($tail)*
}
Expand Down Expand Up @@ -305,7 +301,6 @@ macro_rules! arg_impl {
arg
.value_name(value_name)
.action($crate::ArgAction::Set)
.takes_value(true)
})
$($tail)*
}
Expand Down Expand Up @@ -333,7 +328,6 @@ macro_rules! arg_impl {
arg
.value_name(value_name)
.action($crate::ArgAction::Set)
.takes_value(true)
})
$($tail)*
}
Expand Down Expand Up @@ -365,7 +359,6 @@ macro_rules! arg_impl {
arg
.value_name(value_name)
.action($crate::ArgAction::Set)
.takes_value(true)
})
$($tail)*
}
Expand Down Expand Up @@ -397,7 +390,6 @@ macro_rules! arg_impl {
arg
.value_name(value_name)
.action($crate::ArgAction::Set)
.takes_value(true)
})
$($tail)*
}
Expand Down
6 changes: 2 additions & 4 deletions src/parser/matches/arg_matches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ pub struct ArgMatches {
impl ArgMatches {
/// Gets the value of a specific option or positional argument.
///
/// i.e. an argument that [takes an additional value][crate::Arg::takes_value] at runtime.
/// i.e. an argument that [takes an additional value][crate::Arg::num_args] at runtime.
///
/// Returns an error if the wrong type was used.
///
Expand Down Expand Up @@ -106,7 +106,6 @@ impl ArgMatches {
/// .expect("`port`is required");
/// assert_eq!(port, 2020);
/// ```
/// [option]: crate::Arg::takes_value()
/// [positional]: crate::Arg::index()
/// [`default_value`]: crate::Arg::default_value()
#[track_caller]
Expand Down Expand Up @@ -205,7 +204,7 @@ impl ArgMatches {

/// Returns the value of a specific option or positional argument.
///
/// i.e. an argument that [takes an additional value][crate::Arg::takes_value] at runtime.
/// i.e. an argument that [takes an additional value][crate::Arg::num_args] at runtime.
///
/// Returns an error if the wrong type was used. No item will have been removed.
///
Expand Down Expand Up @@ -234,7 +233,6 @@ impl ArgMatches {
/// .expect("`file`is required");
/// assert_eq!(vals, "file.txt");
/// ```
/// [option]: crate::Arg::takes_value()
/// [positional]: crate::Arg::index()
/// [`default_value`]: crate::Arg::default_value()
#[track_caller]
Expand Down

0 comments on commit 91b1055

Please sign in to comment.