Repository: clap-rs/clap
Pull request #4028 (2022-08-04 14:40:10)

fix!: Replace takes_value with number_of_values
This is a part of #2688


Patch:
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<S: Into<&'help str>>(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::<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 {
@@ -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::<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 {
@@ -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()
@@ -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::<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`,
@@ -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]
@@ -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::<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>(
@@ -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>(
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::<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)
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/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]


Test patch:
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 139e7c98..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")
@@ -2533,7 +2537,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_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"),
     );
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(