From 29753b67980665102fb7d453e50dd9f6a9098e9c Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 4 Aug 2022 09:08:43 -0500 Subject: [PATCH 1/2] fix: Remove references to number_of_values --- src/builder/command.rs | 4 ++-- src/builder/debug_asserts.rs | 16 ++++++++-------- tests/builder/help.rs | 2 +- tests/builder/multiple_values.rs | 6 +++--- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/builder/command.rs b/src/builder/command.rs index f202956a..a38ba62f 100644 --- a/src/builder/command.rs +++ b/src/builder/command.rs @@ -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 /// @@ -2030,7 +2030,7 @@ impl<'help> Command<'help> { /// let trail: Vec<_> = m.get_many::("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) diff --git a/src/builder/debug_asserts.rs b/src/builder/debug_asserts.rs index 483b961c..dc378084 100644 --- a/src/builder/debug_asserts.rs +++ b/src/builder/debug_asserts.rs @@ -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, @@ -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." ); @@ -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 @@ -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" ); } @@ -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 ); } @@ -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, ); @@ -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 ); } diff --git a/tests/builder/help.rs b/tests/builder/help.rs index 139e7c98..6f9be8fd 100644 --- a/tests/builder/help.rs +++ b/tests/builder/help.rs @@ -2533,7 +2533,7 @@ OPTIONS: } #[test] -#[should_panic = "Argument foo: Too many value names (2) compared to number_of_values (1)"] +#[should_panic = "Argument foo: Too many value names (2) compared to `num_args` (1)"] fn too_many_value_names_panics() { Command::new("test") .arg( diff --git a/tests/builder/multiple_values.rs b/tests/builder/multiple_values.rs index fe497f6e..f31bdfda 100644 --- a/tests/builder/multiple_values.rs +++ b/tests/builder/multiple_values.rs @@ -1087,7 +1087,7 @@ fn req_delimiter_complex() { #[cfg(debug_assertions)] #[test] #[should_panic = "When using a positional argument with \ -.num_args(1..) that is *not the last* positional argument, the last \ +`.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."] fn low_index_positional_not_required() { @@ -1106,7 +1106,7 @@ fn low_index_positional_not_required() { // This tests a programmer error and will only succeed with debug_assertions #[cfg(debug_assertions)] #[test] -#[should_panic = "Only one positional argument with .num_args(1..) \ +#[should_panic = "Only one positional argument with `.num_args(1..)` \ set is allowed per command, unless the second one also has .last(true) set"] fn low_index_positional_last_multiple_too() { let _ = Command::new("lip") @@ -1131,7 +1131,7 @@ fn low_index_positional_last_multiple_too() { #[cfg(debug_assertions)] #[test] #[should_panic = "Only the last positional argument, or second to \ -last positional argument may be set to .num_args(1..)"] +last positional argument may be set to `.num_args(1..)`"] fn low_index_positional_too_far_back() { let _ = Command::new("lip") .arg( From c62d3f0cfd66206fd28b96b0b2f1a73b8272e7ae Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 29 Jul 2022 16:36:03 -0500 Subject: [PATCH 2/2] fix!: Replace `takes_value` with `number_of_values` --- CHANGELOG.md | 1 + src/builder/arg.rs | 69 +++++++++------------------ src/macros.rs | 8 ---- src/parser/matches/arg_matches.rs | 6 +-- tests/builder/groups.rs | 2 +- tests/builder/help.rs | 8 +++- tests/builder/multiple_occurrences.rs | 2 - 7 files changed, 32 insertions(+), 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f7ae047b..de5c445e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/builder/arg.rs b/src/builder/arg.rs index 1c8639fa..72a113f9 100644 --- a/src/builder/arg.rs +++ b/src/builder/arg.rs @@ -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>(n: S) -> Self { Arg::default().id(n) } @@ -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 @@ -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::("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 { @@ -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 { @@ -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 { @@ -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. /// @@ -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 @@ -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 /// @@ -1400,8 +1375,8 @@ impl<'help> Arg<'help> { /// /// assert_eq!(m.get_many::("config").unwrap().collect::>(), ["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>) -> Self { @@ -1451,7 +1426,7 @@ impl<'help> Arg<'help> { /// assert_eq!(&cmds, &["find", "-type", "f", "-name", "special"]); /// assert_eq!(m.get_one::("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() @@ -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() @@ -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] @@ -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] @@ -1761,8 +1736,8 @@ impl<'help> Arg<'help> { /// assert_eq!(m.get_one::("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`, @@ -1862,8 +1837,8 @@ impl<'help> Arg<'help> { /// /// assert_eq!(m.get_many::("flag").unwrap().collect::>(), 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] @@ -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]. @@ -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 /// @@ -2633,7 +2608,7 @@ impl<'help> Arg<'help> { /// /// assert_eq!(m.get_one::("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( @@ -2740,7 +2715,7 @@ impl<'help> Arg<'help> { /// /// assert_eq!(m.get_one::("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( diff --git a/src/macros.rs b/src/macros.rs index 880ea747..30463cae 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -210,7 +210,6 @@ macro_rules! arg_impl { arg .long(long) .action($crate::ArgAction::SetTrue) - .takes_value(false) }) $($tail)* } @@ -235,7 +234,6 @@ macro_rules! arg_impl { arg .long(long) .action($crate::ArgAction::SetTrue) - .takes_value(false) }) $($tail)* } @@ -256,7 +254,6 @@ macro_rules! arg_impl { $arg .short($crate::arg_impl! { @char $short }) .action($crate::ArgAction::SetTrue) - .takes_value(false) }) $($tail)* } @@ -277,7 +274,6 @@ macro_rules! arg_impl { $arg .short($crate::arg_impl! { @char $short }) .action($crate::ArgAction::SetTrue) - .takes_value(false) }) $($tail)* } @@ -305,7 +301,6 @@ macro_rules! arg_impl { arg .value_name(value_name) .action($crate::ArgAction::Set) - .takes_value(true) }) $($tail)* } @@ -333,7 +328,6 @@ macro_rules! arg_impl { arg .value_name(value_name) .action($crate::ArgAction::Set) - .takes_value(true) }) $($tail)* } @@ -365,7 +359,6 @@ macro_rules! arg_impl { arg .value_name(value_name) .action($crate::ArgAction::Set) - .takes_value(true) }) $($tail)* } @@ -397,7 +390,6 @@ macro_rules! arg_impl { arg .value_name(value_name) .action($crate::ArgAction::Set) - .takes_value(true) }) $($tail)* } diff --git a/src/parser/matches/arg_matches.rs b/src/parser/matches/arg_matches.rs index 72eb4902..79249c85 100644 --- a/src/parser/matches/arg_matches.rs +++ b/src/parser/matches/arg_matches.rs @@ -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. /// @@ -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] @@ -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. /// @@ -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] diff --git a/tests/builder/groups.rs b/tests/builder/groups.rs index 4fe603a1..30981ac8 100644 --- a/tests/builder/groups.rs +++ b/tests/builder/groups.rs @@ -307,7 +307,7 @@ fn group_acts_like_arg() { fn issue_1794() { let cmd = clap::Command::new("hello") .bin_name("deno") - .arg(Arg::new("option1").long("option1").takes_value(false).action(ArgAction::SetTrue)) + .arg(Arg::new("option1").long("option1").action(ArgAction::SetTrue)) .arg(Arg::new("pos1").action(ArgAction::Set)) .arg(Arg::new("pos2").action(ArgAction::Set)) .group( diff --git a/tests/builder/help.rs b/tests/builder/help.rs index 6f9be8fd..93c4006c 100644 --- a/tests/builder/help.rs +++ b/tests/builder/help.rs @@ -839,7 +839,7 @@ fn dont_wrap_urls() { Arg::new("force-non-host") .help("Install toolchains that require an emulator. See https://github.com/rust-lang/rustup/wiki/Non-host-toolchains") .long("force-non-host") - .takes_value(false)) + .action(ArgAction::SetTrue)) ); const EXPECTED: &str = "\ @@ -2244,7 +2244,11 @@ OPTIONS: let cmd = clap::Command::new("hello") .bin_name("deno") - .arg(Arg::new("option1").long("option1").takes_value(false)) + .arg( + Arg::new("option1") + .long("option1") + .action(ArgAction::SetTrue), + ) .arg(Arg::new("pos1").action(ArgAction::Set)) .group( ArgGroup::new("arg1") diff --git a/tests/builder/multiple_occurrences.rs b/tests/builder/multiple_occurrences.rs index 0719b95e..0b3d043a 100644 --- a/tests/builder/multiple_occurrences.rs +++ b/tests/builder/multiple_occurrences.rs @@ -94,7 +94,6 @@ fn multiple_occurrences_of_before_env() { .env("VERBOSE") .short('v') .long("verbose") - .takes_value(false) .action(ArgAction::Count), ); @@ -126,7 +125,6 @@ fn multiple_occurrences_of_after_env() { Arg::new("verbose") .short('v') .long("verbose") - .takes_value(false) .action(ArgAction::Count) .env("VERBOSE"), );