Repository: clap-rs/clap
Pull request #4001 (2022-07-29 02:48:57)

fix!: Simplify min_values/max_values into number_of_values (per occurrence)
Currently
- `min_values` / `max_values` are across all occurrences
  - This prevents proper `--flag [value]` behavior with multiple occurrences
- `number_of_values` is sort of per occurrence on average
- it is confusing how these interact with `multiple_values` and `takes_value`

This is a major steps towards simplifying all of that (#2688) by creating `number_of_values(range)`.
- It is always per occurrence
- Reduces the number of user-visible settings involved, reducing confusion on interactions
- Displaying of args can more easily take all of the settings into account

Still remaining is to look into dropping `multiple_values` and `takes_value` in favor of this.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 95f3adad..8257bd88 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -20,8 +20,11 @@ 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`
+- `Arg::number_of_values` now applies per occurrence rather than the average across all occurrences
 - `number_of_values(0)` no longer implies `takes_value(true).multiple_values(true)`
 - `number_of_values(1)` no longer implies `multiple_values(true)`
+- Remove `Arg::min_values` (across all occurrences) with `Arg::number_of_values(N..)` (per occurrence)
+- Remove `Arg::max_values` (across all occurrences) with `Arg::number_of_values(1..=M)` (per occurrence)
 - `ArgAction::SetTrue` and `ArgAction::SetFalse` now prioritize `Arg::default_missing_value` over their standard behavior
 - *(help)* Make `DeriveDisplayOrder` the default and removed the setting.  To sort help, set `next_display_order(None)` (#2808)
 - *(help)* Subcommand display order respects `Command::next_display_order` instead of `DeriveDisplayOrder` and using its own initial display order value (#2808)
@@ -31,6 +34,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Features
 
+- `Arg::number_of_values` now accepts ranges, allowing setting both the minimum and maximum number of values per occurrence
 - *(help)* Show `PossibleValue::help` in long help (`--help`) (#3312)
 
 ### Fixes
@@ -41,6 +45,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Replaced `cmd.allow_invalid_for_utf8_external_subcommands` with `cmd.external_subcommand_value_parser` (#3733)
 - Changed the default type of `allow_external_subcommands` from `String` to `OsString`
 - `Arg::default_missing_value` now applies per occurrence rather than if a value is missing across all occurrences
+- `arg!(--long [value])` to accept `0..=1` per occurrence rather than across all occurrences, making it safe to use with `ArgAction::Append`
 - *(assert)* Always enforce that version is specified when the `ArgAction::Version` is used
 - *(assert)* Add missing `#[track_caller]`s to make it easier to debug asserts
 - *(assert)* Ensure `overrides_with` IDs are valid
diff --git a/clap_bench/benches/03_complex.rs b/clap_bench/benches/03_complex.rs
index a1dcebfd..c22c279e 100644
--- a/clap_bench/benches/03_complex.rs
+++ b/clap_bench/benches/03_complex.rs
@@ -31,8 +31,8 @@ macro_rules! create_app {
                 arg!(
                     --multvalsmo 
 "Tests multiple values, not mult occs"
                 ).multiple_values(true).required(false).value_names(&["one", "two"]),
-                arg!(--minvals2 <minvals> ... "Tests 2 min vals").min_values(2).multiple_values(true).required(false),
-                arg!(--maxvals3 <maxvals> ... "Tests 3 max vals").max_values(3).multiple_values(true).required(false),
+                arg!(--minvals2 <minvals> ... "Tests 2 min vals").number_of_values(2..).multiple_values(true).required(false),
+                arg!(--maxvals3 <maxvals> ... "Tests 3 max vals").number_of_values(1..=3).multiple_values(true).required(false),
             ])
             .subcommand(
                 Command::new("subcmd")
@@ -124,7 +124,7 @@ pub fn build_from_builder(c: &mut Criterion) {
                         .multiple_values(true)
                         .action(ArgAction::Append)
                         .help("Tests 2 min vals")
-                        .min_values(2),
+                        .number_of_values(2..),
                 )
                 .arg(
                     Arg::new("maxvals")
@@ -132,7 +132,7 @@ pub fn build_from_builder(c: &mut Criterion) {
                         .multiple_values(true)
                         .action(ArgAction::Append)
                         .help("Tests 3 max vals")
-                        .max_values(3),
+                        .number_of_values(1..=3),
                 )
                 .subcommand(
                     Command::new("subcmd")
diff --git a/clap_complete/src/shells/zsh.rs b/clap_complete/src/shells/zsh.rs
index ccc527fa..b7129e43 100644
--- a/clap_complete/src/shells/zsh.rs
+++ b/clap_complete/src/shells/zsh.rs
@@ -462,7 +462,7 @@ fn write_opts_of(p: &Command, p_global: Option<&Command>) -> String {
             None => format!(":{}: ", vn),
         };
         let vc = match o.get_num_vals() {
-            Some(num_vals) => vc.repeat(num_vals),
+            Some(num_vals) => vc.repeat(num_vals.min_values()),
             None => vc,
         };
 
diff --git a/clap_derive/src/derives/args.rs b/clap_derive/src/derives/args.rs
index fecb72f4..73d56f6e 100644
--- a/clap_derive/src/derives/args.rs
+++ b/clap_derive/src/derives/args.rs
@@ -258,9 +258,7 @@ pub fn gen_augment(
 
                     Ty::OptionOption => quote_spanned! { ty.span()=>
                         .value_name(#value_name)
-                        .min_values(0)
-                        .max_values(1)
-                        .multiple_values(false)
+                        .number_of_values(0..=1)
                         #value_parser
                         #action
                     },
diff --git a/src/builder/arg.rs b/src/builder/arg.rs
index 6388fca8..ff5a96f1 100644
--- a/src/builder/arg.rs
+++ b/src/builder/arg.rs
@@ -14,6 +14,7 @@ use std::{env, ffi::OsString};
 // Internal
 use super::{ArgFlags, ArgSettings};
 use crate::builder::ArgPredicate;
+use crate::builder::ValuesRange;
 use crate::util::{Id, Key};
 use crate::ArgAction;
 use crate::PossibleValue;
@@ -72,9 +73,7 @@ pub struct Arg<'help> {
     pub(crate) short_aliases: Vec<(char, bool)>, // (name, visible)
     pub(crate) disp_ord: Option<usize>,
     pub(crate) val_names: Vec<&'help str>,
-    pub(crate) num_vals: Option<usize>,
-    pub(crate) max_vals: Option<usize>,
-    pub(crate) min_vals: Option<usize>,
+    pub(crate) num_vals: Option<ValuesRange>,
     pub(crate) val_delim: Option<char>,
     pub(crate) default_vals: Vec<&'help OsStr>,
     pub(crate) default_vals_ifs: Vec<(Id, ArgPredicate<'help>, Option<&'help OsStr>)>,
@@ -1066,9 +1065,9 @@ impl<'help> Arg<'help> {
     ///
     /// [`subcommands`]: crate::Command::subcommand()
     /// [`Arg::number_of_values(1)`]: Arg::number_of_values()
-    /// [maximum number of values]: Arg::max_values()
+    /// [maximum number of values]: Arg::number_of_values()
     /// [specific number of values]: Arg::number_of_values()
-    /// [maximum]: Arg::max_values()
+    /// [maximum]: Arg::number_of_values()
     /// [specific]: Arg::number_of_values()
     #[inline]
     #[must_use]
@@ -1080,166 +1079,160 @@ impl<'help> Arg<'help> {
         }
     }
 
-    /// The number of values allowed for this argument.
+    /// Specifies the number of values allowed per occurrence of this argument
     ///
-    /// For example, if you had a
-    /// `-f <file>` argument where you wanted exactly 3 'files' you would set
-    /// `.number_of_values(3)`, and this argument wouldn't be satisfied unless the user provided
-    /// 3 and only 3 values.
+    /// For example, if you had a `-f <file>` argument where you wanted exactly 3 'files' you would
+    /// set `.number_of_values(3)`, and this argument wouldn't be satisfied unless the user
+    /// provided 3 and only 3 values.
     ///
-    /// **NOTE:** implicitly sets [`Arg::action(ArgAction::Set)`] and [`Arg::multiple_values(true)`].
+    /// **NOTE:** Users may specify values for arguments in any of the following methods
     ///
-    /// # Examples
+    /// - 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`
     ///
-    /// ```rust
-    /// # use clap::{Command, Arg};
-    /// Arg::new("file")
-    ///     .short('f')
-    ///     .number_of_values(3);
-    /// ```
+    /// **WARNING:**
     ///
-    /// Not supplying the correct number of values is an error
+    /// Setting a variable number of values (e.g. `1..=10`) for an argument without
+    /// other details can be dangerous in some circumstances. Because multiple values are
+    /// allowed, `--option val1 val2 val3` is perfectly valid. Be careful when designing a CLI
+    /// where **positional arguments** or **subcommands** are *also* expected as `clap` will continue
+    /// parsing *values* until one of the following happens:
     ///
-    /// ```rust
-    /// # use clap::{Command, Arg, ErrorKind, ArgAction};
-    /// let res = Command::new("prog")
-    ///     .arg(Arg::new("file")
-    ///         .action(ArgAction::Set)
-    ///         .number_of_values(2)
-    ///         .short('F'))
-    ///     .try_get_matches_from(vec![
-    ///         "prog", "-F", "file1"
-    ///     ]);
-    ///
-    /// assert!(res.is_err());
-    /// assert_eq!(res.unwrap_err().kind(), ErrorKind::WrongNumberOfValues);
-    /// ```
-    #[inline]
-    #[must_use]
-    pub fn number_of_values(mut self, qty: usize) -> Self {
-        self.num_vals = Some(qty);
-        self.takes_value(qty != 0).multiple_values(1 < qty)
-    }
-
-    /// The *maximum* number of values are for this argument.
+    /// - It reaches the maximum number of values
+    /// - It reaches a specific number of values
+    /// - It finds another flag or option (i.e. something that starts with a `-`)
+    /// - It reaches the [`Arg::value_terminator`] if set
     ///
-    /// For example, if you had a
-    /// `-f <file>` argument where you wanted up to 3 'files' you would set `.max_values(3)`, and
-    /// this argument would be satisfied if the user provided, 1, 2, or 3 values.
+    /// Alternatively,
+    /// - Require a delimiter between values with [Arg::require_value_delimiter]
+    /// - Require a flag occurrence per value with [`ArgAction::Append`]
+    /// - Require positional arguments to appear after `--` with [`Arg::last`]
     ///
     /// # Examples
     ///
+    /// Option:
     /// ```rust
     /// # use clap::{Command, Arg};
-    /// Arg::new("file")
-    ///     .short('f')
-    ///     .max_values(3);
-    /// ```
+    /// let m = Command::new("prog")
+    ///     .arg(Arg::new("mode")
+    ///         .long("mode")
+    ///         .number_of_values(1))
+    ///     .get_matches_from(vec![
+    ///         "prog", "--mode", "fast"
+    ///     ]);
     ///
-    /// Supplying less than the maximum number of values is allowed
+    /// assert_eq!(m.get_one::<String>("mode").unwrap(), "fast");
+    /// ```
     ///
+    /// Flag/option hybrid (see also [default_missing_value][Arg::default_missing_value])
     /// ```rust
-    /// # use clap::{Command, Arg, ArgAction};
-    /// let res = Command::new("prog")
-    ///     .arg(Arg::new("file")
-    ///         .action(ArgAction::Set)
-    ///         .max_values(3)
-    ///         .short('F'))
-    ///     .try_get_matches_from(vec![
-    ///         "prog", "-F", "file1", "file2"
+    /// # use clap::{Command, Arg, ErrorKind, ArgAction};
+    /// let cmd = Command::new("prog")
+    ///     .arg(Arg::new("mode")
+    ///         .long("mode")
+    ///         .default_missing_value("slow")
+    ///         .default_value("plaid")
+    ///         .number_of_values(0..=1));
+    ///
+    /// let m = cmd.clone()
+    ///     .get_matches_from(vec![
+    ///         "prog", "--mode", "fast"
     ///     ]);
+    /// assert_eq!(m.get_one::<String>("mode").unwrap(), "fast");
     ///
-    /// assert!(res.is_ok());
-    /// let m = res.unwrap();
-    /// let files: Vec<_> = m.get_many::<String>("file").unwrap().collect();
-    /// assert_eq!(files, ["file1", "file2"]);
-    /// ```
+    /// let m = cmd.clone()
+    ///     .get_matches_from(vec![
+    ///         "prog", "--mode",
+    ///     ]);
+    /// assert_eq!(m.get_one::<String>("mode").unwrap(), "slow");
     ///
-    /// Supplying more than the maximum number of values is an error
+    /// let m = cmd.clone()
+    ///     .get_matches_from(vec![
+    ///         "prog",
+    ///     ]);
+    /// assert_eq!(m.get_one::<String>("mode").unwrap(), "plaid");
+    /// ```
     ///
+    /// Tuples
     /// ```rust
     /// # use clap::{Command, Arg, ErrorKind, ArgAction};
-    /// let res = Command::new("prog")
+    /// let cmd = Command::new("prog")
     ///     .arg(Arg::new("file")
     ///         .action(ArgAction::Set)
-    ///         .max_values(2)
-    ///         .short('F'))
-    ///     .try_get_matches_from(vec![
-    ///         "prog", "-F", "file1", "file2", "file3"
-    ///     ]);
-    ///
-    /// assert!(res.is_err());
-    /// assert_eq!(res.unwrap_err().kind(), ErrorKind::UnknownArgument);
-    /// ```
-    #[inline]
-    #[must_use]
-    pub fn max_values(mut self, qty: usize) -> Self {
-        self.max_vals = Some(qty);
-        self.takes_value(true).multiple_values(true)
-    }
-
-    /// The *minimum* number of values for this argument.
-    ///
-    /// For example, if you had a
-    /// `-f <file>` argument where you wanted at least 2 'files' you would set
-    /// `.min_values(2)`, and this argument would be satisfied if the user provided, 2 or more
-    /// values.
-    ///
-    /// **NOTE:** Passing a non-zero value is not the same as specifying [`Arg::required(true)`].
-    /// This is due to min and max validation only being performed for present arguments,
-    /// marking them as required will thus perform validation and a min value of 1
-    /// is unnecessary, ignored if not required.
+    ///         .number_of_values(2)
+    ///         .short('F'));
     ///
-    /// # Examples
+    /// let m = cmd.clone()
+    ///     .get_matches_from(vec![
+    ///         "prog", "-F", "in-file", "out-file"
+    ///     ]);
+    /// assert_eq!(
+    ///     m.get_many::<String>("file").unwrap_or_default().map(|v| v.as_str()).collect::<Vec<_>>(),
+    ///     vec!["in-file", "out-file"]
+    /// );
     ///
-    /// ```rust
-    /// # use clap::{Command, Arg};
-    /// Arg::new("file")
-    ///     .short('f')
-    ///     .min_values(3);
+    /// let res = cmd.clone()
+    ///     .try_get_matches_from(vec![
+    ///         "prog", "-F", "file1"
+    ///     ]);
+    /// assert_eq!(res.unwrap_err().kind(), ErrorKind::WrongNumberOfValues);
     /// ```
     ///
-    /// Supplying more than the minimum number of values is allowed
-    ///
+    /// A common mistake is to define an option which allows multiple values and a positional
+    /// argument.
     /// ```rust
     /// # use clap::{Command, Arg, ArgAction};
-    /// let res = Command::new("prog")
+    /// let cmd = Command::new("prog")
     ///     .arg(Arg::new("file")
     ///         .action(ArgAction::Set)
-    ///         .min_values(2)
+    ///         .number_of_values(0..)
     ///         .short('F'))
-    ///     .try_get_matches_from(vec![
-    ///         "prog", "-F", "file1", "file2", "file3"
-    ///     ]);
+    ///     .arg(Arg::new("word"));
     ///
-    /// assert!(res.is_ok());
-    /// let m = res.unwrap();
+    /// let m = cmd.clone().get_matches_from(vec![
+    ///     "prog", "-F", "file1", "file2", "file3", "word"
+    /// ]);
+    /// let files: Vec<_> = m.get_many::<String>("file").unwrap().collect();
+    /// assert_eq!(files, ["file1", "file2", "file3", "word"]); // wait...what?!
+    /// assert!(!m.contains_id("word")); // but we clearly used word!
+    ///
+    /// // but this works
+    /// let m = cmd.clone().get_matches_from(vec![
+    ///     "prog", "word", "-F", "file1", "file2", "file3",
+    /// ]);
     /// let files: Vec<_> = m.get_many::<String>("file").unwrap().collect();
     /// assert_eq!(files, ["file1", "file2", "file3"]);
+    /// assert_eq!(m.get_one::<String>("word").unwrap(), "word");
     /// ```
+    /// The problem is `clap` doesn't know when to stop parsing values for "file".
     ///
-    /// Supplying less than the minimum number of values is an error
-    ///
+    /// A solution for the example above is to limit how many values with a maximum, or specific
+    /// number, or to say [`ArgAction::Append`] is ok, but multiple values are not.
     /// ```rust
-    /// # use clap::{Command, Arg, ErrorKind, ArgAction};
-    /// let res = Command::new("prog")
+    /// # use clap::{Command, Arg, ArgAction};
+    /// let m = Command::new("prog")
     ///     .arg(Arg::new("file")
-    ///         .action(ArgAction::Set)
-    ///         .min_values(2)
+    ///         .action(ArgAction::Append)
     ///         .short('F'))
-    ///     .try_get_matches_from(vec![
-    ///         "prog", "-F", "file1"
+    ///     .arg(Arg::new("word"))
+    ///     .get_matches_from(vec![
+    ///         "prog", "-F", "file1", "-F", "file2", "-F", "file3", "word"
     ///     ]);
     ///
-    /// assert!(res.is_err());
-    /// assert_eq!(res.unwrap_err().kind(), ErrorKind::TooFewValues);
+    /// let files: Vec<_> = m.get_many::<String>("file").unwrap().collect();
+    /// assert_eq!(files, ["file1", "file2", "file3"]);
+    /// assert_eq!(m.get_one::<String>("word").unwrap(), "word");
     /// ```
-    /// [`Arg::required(true)`]: Arg::required()
+    /// [`Arg::value_delimiter(char)`]: Arg::value_delimiter()
+    /// [multiple values]: Arg::multiple_values
     #[inline]
     #[must_use]
-    pub fn min_values(mut self, qty: usize) -> Self {
-        self.min_vals = Some(qty);
-        self.takes_value(true).multiple_values(true)
+    pub fn number_of_values(mut self, qty: impl Into<ValuesRange>) -> Self {
+        let qty = qty.into();
+        self.num_vals = Some(qty);
+        self.takes_value(qty.takes_values())
+            .multiple_values(qty.is_multiple())
     }
 
     /// Placeholder for the argument's value in the help message / usage.
@@ -1740,8 +1733,7 @@ impl<'help> Arg<'help> {
     /// By default when
     /// one sets [`multiple_values(true)`] on an argument, clap will continue parsing values for that
     /// argument until it reaches another valid argument, or one of the other more specific settings
-    /// for multiple values is used (such as [`min_values`], [`max_values`] or
-    /// [`number_of_values`]).
+    /// for multiple values is used (such as [`number_of_values`]).
     ///
     /// **NOTE:** This setting only applies to [options] and [positional arguments]
     ///
@@ -1781,9 +1773,7 @@ impl<'help> Arg<'help> {
     /// [options]: Arg::takes_value()
     /// [positional arguments]: Arg::index()
     /// [`multiple_values(true)`]: Arg::multiple_values()
-    /// [`min_values`]: Arg::min_values()
     /// [`number_of_values`]: Arg::number_of_values()
-    /// [`max_values`]: Arg::max_values()
     #[inline]
     #[must_use]
     pub fn value_terminator(mut self, term: &'help str) -> Self {
@@ -1931,9 +1921,10 @@ impl<'help> Arg<'help> {
     /// argument is a common example. By, supplying an default, such as `default_missing_value("always")`,
     /// the user can quickly just add `--color` to the command line to produce the desired color output.
     ///
-    /// **NOTE:** using this configuration option requires the use of the `.min_values(0)` and the
-    /// `.require_equals(true)` configuration option. These are required in order to unambiguously
-    /// determine what, if any, value was supplied for the argument.
+    /// **NOTE:** using this configuration option requires the use of the
+    /// [`.number_of_values(0..N)`][Arg::number_of_values] and the
+    /// [`.require_equals(true)`][Arg::require_equals] configuration option. These are required in
+    /// order to unambiguously determine what, if any, value was supplied for the argument.
     ///
     /// # Examples
     ///
@@ -1946,7 +1937,7 @@ impl<'help> Arg<'help> {
     ///             .value_name("WHEN")
     ///             .value_parser(["always", "auto", "never"])
     ///             .default_value("auto")
-    ///             .min_values(0)
+    ///             .number_of_values(0..=1)
     ///             .require_equals(true)
     ///             .default_missing_value("always")
     ///             .help("Specify WHEN to colorize output.")
@@ -1983,7 +1974,7 @@ impl<'help> Arg<'help> {
     ///         .arg(Arg::new("create").long("create")
     ///             .value_name("BOOL")
     ///             .value_parser(value_parser!(bool))
-    ///             .min_values(0)
+    ///             .number_of_values(0..=1)
     ///             .require_equals(true)
     ///             .default_missing_value("true")
     ///         )
@@ -4062,13 +4053,13 @@ impl<'help> Arg<'help> {
 
     /// Get the number of values for this argument.
     #[inline]
-    pub fn get_num_vals(&self) -> Option<usize> {
+    pub fn get_num_vals(&self) -> Option<ValuesRange> {
         self.num_vals
     }
 
     #[inline]
     pub(crate) fn get_min_vals(&self) -> Option<usize> {
-        self.min_vals
+        self.get_num_vals().map(|r| r.min_values())
     }
 
     /// Get the delimiter between multiple values
@@ -4330,13 +4321,9 @@ impl<'help> Arg<'help> {
         }
 
         let val_names_len = self.val_names.len();
-
         if val_names_len > 1 {
             self.settings.set(ArgSettings::MultipleValues);
-
-            if self.num_vals.is_none() {
-                self.num_vals = Some(val_names_len);
-            }
+            self.num_vals.get_or_insert(val_names_len.into());
         }
     }
 
@@ -4497,8 +4484,6 @@ impl<'help> fmt::Debug for Arg<'help> {
             .field("disp_ord", &self.disp_ord)
             .field("val_names", &self.val_names)
             .field("num_vals", &self.num_vals)
-            .field("max_vals", &self.max_vals)
-            .field("min_vals", &self.min_vals)
             .field("val_delim", &self.val_delim)
             .field("default_vals", &self.default_vals)
             .field("default_vals_ifs", &self.default_vals_ifs)
@@ -4552,10 +4537,17 @@ pub(crate) fn render_arg_val(arg: &Arg) -> String {
 
     let mut extra_values = false;
     debug_assert!(arg.is_takes_value_set());
-    let num_vals = arg.num_vals.unwrap_or(1);
+    let num_vals = arg.num_vals.unwrap_or_else(|| {
+        if arg.is_multiple_values_set() {
+            (1..).into()
+        } else {
+            1.into()
+        }
+    });
     if val_names.len() == 1 {
         let arg_name = format!("<{}>", val_names[0]);
-        for n in 1..=num_vals {
+        let min = num_vals.min_values().max(1);
+        for n in 1..=min {
             if n != 1 {
                 rendered.push_str(delim);
             }
@@ -4571,8 +4563,9 @@ pub(crate) fn render_arg_val(arg: &Arg) -> String {
             }
             rendered.push_str(&arg_name);
         }
-        extra_values |= val_names.len() < num_vals;
+        extra_values |= val_names.len() < num_vals.max_values();
     }
+    extra_values |= !num_vals.is_fixed();
     if arg.is_positional() {
         if matches!(*arg.get_action(), ArgAction::Append) {
             extra_values = true;
@@ -4678,7 +4671,7 @@ mod test {
         let mut o = Arg::new("opt")
             .long("option")
             .action(ArgAction::Set)
-            .min_values(0);
+            .number_of_values(0..);
         o._build();
 
         assert_eq!(o.to_string(), "--option [<opt>...]");
@@ -4689,7 +4682,7 @@ mod test {
         let mut o = Arg::new("opt")
             .long("option")
             .action(ArgAction::Set)
-            .min_values(1);
+            .number_of_values(1..);
         o._build();
 
         assert_eq!(o.to_string(), "--option <opt>...");
@@ -4700,7 +4693,7 @@ mod test {
         let mut o = Arg::new("opt")
             .short('o')
             .action(ArgAction::Set)
-            .min_values(0)
+            .number_of_values(0..)
             .value_names(&["file"]);
         o._build();
 
@@ -4712,7 +4705,7 @@ mod test {
         let mut o = Arg::new("opt")
             .short('o')
             .action(ArgAction::Set)
-            .min_values(1)
+            .number_of_values(1..)
             .value_names(&["file"]);
         o._build();
 
@@ -4800,7 +4793,7 @@ mod test {
 
     #[test]
     fn positional_display_zero_or_more_values() {
-        let mut p = Arg::new("pos").index(1).min_values(0);
+        let mut p = Arg::new("pos").index(1).number_of_values(0..);
         p._build();
 
         assert_eq!(p.to_string(), "[<pos>...]");
@@ -4808,7 +4801,7 @@ mod test {
 
     #[test]
     fn positional_display_one_or_more_values() {
-        let mut p = Arg::new("pos").index(1).min_values(1);
+        let mut p = Arg::new("pos").index(1).number_of_values(1..);
         p._build();
 
         assert_eq!(p.to_string(), "<pos>...");
diff --git a/src/builder/debug_asserts.rs b/src/builder/debug_asserts.rs
index 4bb3f26b..552000d9 100644
--- a/src/builder/debug_asserts.rs
+++ b/src/builder/debug_asserts.rs
@@ -687,15 +687,32 @@ fn assert_arg(arg: &Arg) {
         );
     }
 
-    let num_vals = arg.get_num_vals().unwrap_or(usize::MAX);
-    let num_val_names = arg.get_value_names().unwrap_or(&[]).len();
-    if num_vals < num_val_names {
-        panic!(
-            "Argument {}: Too many value names ({}) compared to number_of_values ({})",
-            arg.name, num_val_names, num_vals
+    if let Some(num_vals) = arg.get_num_vals() {
+        // This can be the cause of later asserts, so put this first
+        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 ({})",
+                arg.name, num_val_names, num_vals
+            );
+        }
+
+        assert_eq!(
+            num_vals.takes_values(),
+            arg.is_takes_value_set(),
+            "Argument {}: mismatch between `number_of_values` ({}) 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`",
+            arg.name,
+            num_vals,
         );
     }
-    if arg.get_num_vals() == Some(1) {
+    if arg.get_num_vals() == Some(1.into()) {
         assert!(
             !arg.is_multiple_values_set(),
             "Argument {}: mismatch between `number_of_values` and `multiple_values`",
diff --git a/src/builder/mod.rs b/src/builder/mod.rs
index f66fb485..2ee326d9 100644
--- a/src/builder/mod.rs
+++ b/src/builder/mod.rs
@@ -8,6 +8,7 @@ mod arg_predicate;
 mod arg_settings;
 mod command;
 mod possible_value;
+mod range;
 mod value_hint;
 mod value_parser;
 
@@ -22,15 +23,8 @@ pub use arg::Arg;
 pub use arg_group::ArgGroup;
 pub use command::Command;
 pub use possible_value::PossibleValue;
+pub use range::ValuesRange;
 pub use value_hint::ValueHint;
-pub use value_parser::PossibleValuesParser;
-pub use value_parser::RangedI64ValueParser;
-pub use value_parser::RangedU64ValueParser;
-pub use value_parser::StringValueParser;
-pub use value_parser::TypedValueParser;
-pub use value_parser::ValueParser;
-pub use value_parser::ValueParserFactory;
-pub use value_parser::_AnonymousValueParser;
 pub use value_parser::_AutoValueParser;
 pub use value_parser::via_prelude;
 pub use value_parser::BoolValueParser;
@@ -40,6 +34,14 @@ pub use value_parser::FalseyValueParser;
 pub use value_parser::NonEmptyStringValueParser;
 pub use value_parser::OsStringValueParser;
 pub use value_parser::PathBufValueParser;
+pub use value_parser::PossibleValuesParser;
+pub use value_parser::RangedI64ValueParser;
+pub use value_parser::RangedU64ValueParser;
+pub use value_parser::StringValueParser;
+pub use value_parser::TypedValueParser;
+pub use value_parser::ValueParser;
+pub use value_parser::ValueParserFactory;
+pub use value_parser::_AnonymousValueParser;
 
 pub(crate) use action::CountType;
 pub(crate) use arg::render_arg_val;
diff --git a/src/builder/range.rs b/src/builder/range.rs
new file mode 100644
index 00000000..c634f068
--- /dev/null
+++ b/src/builder/range.rs
@@ -0,0 +1,267 @@
+/// Values per occurrence for an argument
+#[derive(Copy, Clone, PartialEq, Eq, Hash)]
+pub struct ValuesRange {
+    start_inclusive: usize,
+    end_inclusive: usize,
+}
+
+impl ValuesRange {
+    /// Create a range
+    ///
+    /// # Panics
+    ///
+    /// If the end is less than the start
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use clap::builder::ValuesRange;
+    /// let range = ValuesRange::new(5);
+    /// let range = ValuesRange::new(5..10);
+    /// let range = ValuesRange::new(5..=10);
+    /// let range = ValuesRange::new(5..);
+    /// let range = ValuesRange::new(..10);
+    /// let range = ValuesRange::new(..=10);
+    /// ```
+    ///
+    /// While this will panic:
+    /// ```should_panic
+    /// # use clap::builder::ValuesRange;
+    /// let range = ValuesRange::new(10..5);  // Panics!
+    /// ```
+    pub fn new(range: impl Into<Self>) -> Self {
+        range.into()
+    }
+
+    pub(crate) fn raw(start_inclusive: usize, end_inclusive: usize) -> Self {
+        debug_assert!(start_inclusive <= end_inclusive);
+        Self {
+            start_inclusive,
+            end_inclusive,
+        }
+    }
+
+    /// Fewest number of values the argument accepts
+    pub fn min_values(&self) -> usize {
+        self.start_inclusive
+    }
+
+    /// Most number of values the argument accepts
+    pub fn max_values(&self) -> usize {
+        self.end_inclusive
+    }
+
+    /// Report whether the argument takes any values (ie is a flag)
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use clap::builder::ValuesRange;
+    /// let range = ValuesRange::new(5);
+    /// assert!(range.takes_values());
+    ///
+    /// let range = ValuesRange::new(0);
+    /// assert!(!range.takes_values());
+    /// ```
+    pub fn takes_values(&self) -> bool {
+        self.end_inclusive != 0
+    }
+
+    pub(crate) fn is_fixed(&self) -> bool {
+        self.start_inclusive == self.end_inclusive
+    }
+
+    pub(crate) fn is_multiple(&self) -> bool {
+        self.start_inclusive != self.end_inclusive || 1 < self.start_inclusive
+    }
+
+    pub(crate) fn num_values(&self) -> Option<usize> {
+        self.is_fixed().then(|| self.start_inclusive)
+    }
+
+    pub(crate) fn accepts_more(&self, current: usize) -> bool {
+        current < self.end_inclusive
+    }
+}
+
+impl std::ops::RangeBounds<usize> for ValuesRange {
+    fn start_bound(&self) -> std::ops::Bound<&usize> {
+        std::ops::Bound::Included(&self.start_inclusive)
+    }
+
+    fn end_bound(&self) -> std::ops::Bound<&usize> {
+        std::ops::Bound::Included(&self.end_inclusive)
+    }
+}
+
+impl Default for ValuesRange {
+    fn default() -> Self {
+        0.into()
+    }
+}
+
+impl From<usize> for ValuesRange {
+    fn from(fixed: usize) -> Self {
+        (fixed..=fixed).into()
+    }
+}
+
+impl From<std::ops::Range<usize>> for ValuesRange {
+    fn from(range: std::ops::Range<usize>) -> Self {
+        let start_inclusive = range.start;
+        let end_inclusive = range.end.saturating_sub(1);
+        Self::raw(start_inclusive, end_inclusive)
+    }
+}
+
+impl From<std::ops::RangeFull> for ValuesRange {
+    fn from(_: std::ops::RangeFull) -> Self {
+        let start_inclusive = 0;
+        let end_inclusive = usize::MAX;
+        Self::raw(start_inclusive, end_inclusive)
+    }
+}
+
+impl From<std::ops::RangeFrom<usize>> for ValuesRange {
+    fn from(range: std::ops::RangeFrom<usize>) -> Self {
+        let start_inclusive = range.start;
+        let end_inclusive = usize::MAX;
+        Self::raw(start_inclusive, end_inclusive)
+    }
+}
+
+impl From<std::ops::RangeTo<usize>> for ValuesRange {
+    fn from(range: std::ops::RangeTo<usize>) -> Self {
+        let start_inclusive = 0;
+        let end_inclusive = range.end.saturating_sub(1);
+        Self::raw(start_inclusive, end_inclusive)
+    }
+}
+
+impl From<std::ops::RangeInclusive<usize>> for ValuesRange {
+    fn from(range: std::ops::RangeInclusive<usize>) -> Self {
+        let start_inclusive = *range.start();
+        let end_inclusive = *range.end();
+        Self::raw(start_inclusive, end_inclusive)
+    }
+}
+
+impl From<std::ops::RangeToInclusive<usize>> for ValuesRange {
+    fn from(range: std::ops::RangeToInclusive<usize>) -> Self {
+        let start_inclusive = 0;
+        let end_inclusive = range.end;
+        Self::raw(start_inclusive, end_inclusive)
+    }
+}
+
+impl std::fmt::Display for ValuesRange {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        self.start_inclusive.fmt(f)?;
+        if !self.is_fixed() {
+            "..=".fmt(f)?;
+            self.end_inclusive.fmt(f)?;
+        }
+        Ok(())
+    }
+}
+
+impl std::fmt::Debug for ValuesRange {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        write!(f, "{}", self)
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    use std::ops::RangeBounds;
+
+    #[test]
+    fn from_fixed() {
+        let range: ValuesRange = 5.into();
+        assert_eq!(range.start_bound(), std::ops::Bound::Included(&5));
+        assert_eq!(range.end_bound(), std::ops::Bound::Included(&5));
+        assert!(range.is_fixed());
+        assert!(range.is_multiple());
+        assert_eq!(range.num_values(), Some(5));
+        assert!(range.takes_values());
+    }
+
+    #[test]
+    fn from_fixed_empty() {
+        let range: ValuesRange = 0.into();
+        assert_eq!(range.start_bound(), std::ops::Bound::Included(&0));
+        assert_eq!(range.end_bound(), std::ops::Bound::Included(&0));
+        assert!(range.is_fixed());
+        assert!(!range.is_multiple());
+        assert_eq!(range.num_values(), Some(0));
+        assert!(!range.takes_values());
+    }
+
+    #[test]
+    fn from_range() {
+        let range: ValuesRange = (5..10).into();
+        assert_eq!(range.start_bound(), std::ops::Bound::Included(&5));
+        assert_eq!(range.end_bound(), std::ops::Bound::Included(&9));
+        assert!(!range.is_fixed());
+        assert!(range.is_multiple());
+        assert_eq!(range.num_values(), None);
+        assert!(range.takes_values());
+    }
+
+    #[test]
+    fn from_range_inclusive() {
+        let range: ValuesRange = (5..=10).into();
+        assert_eq!(range.start_bound(), std::ops::Bound::Included(&5));
+        assert_eq!(range.end_bound(), std::ops::Bound::Included(&10));
+        assert!(!range.is_fixed());
+        assert!(range.is_multiple());
+        assert_eq!(range.num_values(), None);
+        assert!(range.takes_values());
+    }
+
+    #[test]
+    fn from_range_full() {
+        let range: ValuesRange = (..).into();
+        assert_eq!(range.start_bound(), std::ops::Bound::Included(&0));
+        assert_eq!(range.end_bound(), std::ops::Bound::Included(&usize::MAX));
+        assert!(!range.is_fixed());
+        assert!(range.is_multiple());
+        assert_eq!(range.num_values(), None);
+        assert!(range.takes_values());
+    }
+
+    #[test]
+    fn from_range_from() {
+        let range: ValuesRange = (5..).into();
+        assert_eq!(range.start_bound(), std::ops::Bound::Included(&5));
+        assert_eq!(range.end_bound(), std::ops::Bound::Included(&usize::MAX));
+        assert!(!range.is_fixed());
+        assert!(range.is_multiple());
+        assert_eq!(range.num_values(), None);
+        assert!(range.takes_values());
+    }
+
+    #[test]
+    fn from_range_to() {
+        let range: ValuesRange = (..10).into();
+        assert_eq!(range.start_bound(), std::ops::Bound::Included(&0));
+        assert_eq!(range.end_bound(), std::ops::Bound::Included(&9));
+        assert!(!range.is_fixed());
+        assert!(range.is_multiple());
+        assert_eq!(range.num_values(), None);
+        assert!(range.takes_values());
+    }
+
+    #[test]
+    fn from_range_to_inclusive() {
+        let range: ValuesRange = (..=10).into();
+        assert_eq!(range.start_bound(), std::ops::Bound::Included(&0));
+        assert_eq!(range.end_bound(), std::ops::Bound::Included(&10));
+        assert!(!range.is_fixed());
+        assert!(range.is_multiple());
+        assert_eq!(range.num_values(), None);
+        assert!(range.takes_values());
+    }
+}
diff --git a/src/error/kind.rs b/src/error/kind.rs
index e25e01b9..d0ce745c 100644
--- a/src/error/kind.rs
+++ b/src/error/kind.rs
@@ -96,7 +96,7 @@ pub enum ErrorKind {
     ValueValidation,
 
     /// Occurs when a user provides more values for an argument than were defined by setting
-    /// [`Arg::max_values`].
+    /// [`Arg::number_of_values`].
     ///
     /// # Examples
     ///
@@ -104,16 +104,18 @@ pub enum ErrorKind {
     /// # use clap::{Command, Arg, ErrorKind};
     /// let result = Command::new("prog")
     ///     .arg(Arg::new("arg")
-    ///         .max_values(2))
-    ///     .try_get_matches_from(vec!["prog", "too", "many", "values"]);
+    ///         .number_of_values(1..=2)
+    ///         .use_value_delimiter(true)
+    ///         .require_value_delimiter(true))
+    ///     .try_get_matches_from(vec!["prog", "too,many,values"]);
     /// assert!(result.is_err());
     /// assert_eq!(result.unwrap_err().kind(), ErrorKind::TooManyValues);
     /// ```
-    /// [`Arg::max_values`]: crate::Arg::max_values()
+    /// [`Arg::number_of_values`]: crate::Arg::number_of_values()
     TooManyValues,
 
     /// Occurs when the user provides fewer values for an argument than were defined by setting
-    /// [`Arg::min_values`].
+    /// [`Arg::number_of_values`].
     ///
     /// # Examples
     ///
@@ -122,12 +124,12 @@ pub enum ErrorKind {
     /// let result = Command::new("prog")
     ///     .arg(Arg::new("some_opt")
     ///         .long("opt")
-    ///         .min_values(3))
+    ///         .number_of_values(3..))
     ///     .try_get_matches_from(vec!["prog", "--opt", "too", "few"]);
     /// assert!(result.is_err());
     /// assert_eq!(result.unwrap_err().kind(), ErrorKind::TooFewValues);
     /// ```
-    /// [`Arg::min_values`]: crate::Arg::min_values()
+    /// [`Arg::number_of_values`]: crate::Arg::number_of_values()
     TooFewValues,
 
     /// Occurs when the user provides a different number of values for an argument than what's
diff --git a/src/error/mod.rs b/src/error/mod.rs
index 648b894e..68956149 100644
--- a/src/error/mod.rs
+++ b/src/error/mod.rs
@@ -545,15 +545,17 @@ impl Error {
 
                     let possible_values = self.get_context(ContextKind::ValidValue);
                     if let Some(ContextValue::Strings(possible_values)) = possible_values {
-                        c.none("\n\t[possible values: ");
-                        if let Some((last, elements)) = possible_values.split_last() {
-                            for v in elements {
-                                c.good(escape(v));
-                                c.none(", ");
+                        if !possible_values.is_empty() {
+                            c.none("\n\t[possible values: ");
+                            if let Some((last, elements)) = possible_values.split_last() {
+                                for v in elements {
+                                    c.good(escape(v));
+                                    c.none(", ");
+                                }
+                                c.good(escape(last));
                             }
-                            c.good(escape(last));
+                            c.none("]");
                         }
-                        c.none("]");
                     }
 
                     let suggestion = self.get_context(ContextKind::SuggestedValue);
diff --git a/src/macros.rs b/src/macros.rs
index ae228713..cb3d6d2e 100644
--- a/src/macros.rs
+++ b/src/macros.rs
@@ -355,7 +355,7 @@ macro_rules! arg_impl {
                 if arg.get_long().is_none() && arg.get_short().is_none() {
                     arg = arg.required(false);
                 } else {
-                    arg = arg.min_values(0).max_values(1);
+                    arg = arg.number_of_values(0..=1);
                 }
 
                 let value_name = $crate::arg_impl! { @string $value_name };
@@ -387,7 +387,7 @@ macro_rules! arg_impl {
                 if arg.get_long().is_none() && arg.get_short().is_none() {
                     arg = arg.required(false);
                 } else {
-                    arg = arg.min_values(0).max_values(1);
+                    arg = arg.number_of_values(0..=1);
                 }
 
                 let value_name = $crate::arg_impl! { @string $value_name };
diff --git a/src/parser/arg_matcher.rs b/src/parser/arg_matcher.rs
index bf306b26..5856d6e0 100644
--- a/src/parser/arg_matcher.rs
+++ b/src/parser/arg_matcher.rs
@@ -11,7 +11,6 @@ use crate::parser::Identifier;
 use crate::parser::PendingArg;
 use crate::parser::{ArgMatches, MatchedArg, SubCommand, ValueSource};
 use crate::util::Id;
-use crate::ArgAction;
 use crate::INTERNAL_ERROR_MSG;
 
 #[derive(Debug, Default)]
@@ -217,19 +216,12 @@ impl ArgMatcher {
         );
         if current_num == 0 {
             true
-        } else if let Some(num) = o.num_vals {
-            debug!("ArgMatcher::needs_more_vals: num_vals...{}", num);
-            if matches!(o.get_action(), ArgAction::Append) && !o.is_positional() {
-                (current_num % num) != 0
-            } else {
-                num != current_num
-            }
-        } else if let Some(num) = o.max_vals {
-            debug!("ArgMatcher::needs_more_vals: max_vals...{}", num);
-            current_num < num
-        } else if o.get_min_vals().is_some() {
-            debug!("ArgMatcher::needs_more_vals: min_vals...true");
-            true
+        } else if let Some(expected) = o.num_vals {
+            debug!(
+                "ArgMatcher::needs_more_vals: expected={}, actual={}",
+                expected, num_pending
+            );
+            expected.accepts_more(num_pending)
         } else {
             o.is_multiple_values_set()
         }
diff --git a/src/parser/matches/arg_matches.rs b/src/parser/matches/arg_matches.rs
index 978c745c..0c8dcb0d 100644
--- a/src/parser/matches/arg_matches.rs
+++ b/src/parser/matches/arg_matches.rs
@@ -360,7 +360,7 @@ impl ArgMatches {
     /// let m = Command::new("myprog")
     ///     .arg(Arg::new("exec")
     ///         .short('x')
-    ///         .min_values(1)
+    ///         .number_of_values(1..)
     ///         .action(ArgAction::Append)
     ///         .value_terminator(";"))
     ///     .get_matches_from(vec![
diff --git a/src/parser/matches/matched_arg.rs b/src/parser/matches/matched_arg.rs
index b66b5616..d031ebd7 100644
--- a/src/parser/matches/matched_arg.rs
+++ b/src/parser/matches/matched_arg.rs
@@ -89,7 +89,6 @@ impl MatchedArg {
         self.indices.push(index)
     }
 
-    #[cfg(feature = "unstable-grouped")]
     pub(crate) fn vals(&self) -> Iter<Vec<AnyValue>> {
         self.vals.iter()
     }
diff --git a/src/parser/validator.rs b/src/parser/validator.rs
index 7e58cda7..4f64215c 100644
--- a/src/parser/validator.rs
+++ b/src/parser/validator.rs
@@ -6,7 +6,6 @@ use crate::output::Usage;
 use crate::parser::{ArgMatcher, MatchedArg, ParseState};
 use crate::util::ChildGraph;
 use crate::util::Id;
-use crate::ArgAction;
 use crate::{INTERNAL_ERROR_MSG, INVALID_UTF8};
 
 pub(crate) struct Validator<'help, 'cmd> {
@@ -249,83 +248,71 @@ impl<'help, 'cmd> Validator<'help, 'cmd> {
 
     fn validate_arg_num_vals(&self, a: &Arg, ma: &MatchedArg) -> ClapResult<()> {
         debug!("Validator::validate_arg_num_vals");
-        if let Some(num) = a.num_vals {
-            let total_num = ma.num_vals();
+        for (_i, group) in ma.vals().enumerate() {
+            let actual = group.len();
             debug!(
-                "Validator::validate_arg_num_vals: num_vals={}, actual={}",
-                num, total_num
+                "Validator::validate_arg_num_vals: group={}, actual={}",
+                _i, actual
             );
-            let should_err = if matches!(a.get_action(), ArgAction::Append) && !a.is_positional() {
-                total_num % num != 0
-            } else {
-                num != total_num
-            };
-            if should_err {
-                debug!("Validator::validate_arg_num_vals: Sending error WrongNumberOfValues");
-                return Err(Error::wrong_number_of_values(
-                    self.cmd,
-                    a.to_string(),
-                    num,
-                    if matches!(a.get_action(), ArgAction::Append) && !a.is_positional() {
-                        total_num % num
-                    } else {
-                        total_num
-                    },
-                    Usage::new(self.cmd)
-                        .required(&self.required)
-                        .create_usage_with_title(&[]),
-                ));
-            }
-        }
-        if let Some(num) = a.max_vals {
-            debug!("Validator::validate_arg_num_vals: max_vals set...{}", num);
-            if ma.num_vals() > num {
-                debug!("Validator::validate_arg_num_vals: Sending error TooManyValues");
-                return Err(Error::too_many_values(
+
+            let min_vals = a.get_min_vals().unwrap_or(1);
+            if a.is_takes_value_set() && 0 < min_vals && actual == 0 {
+                // Issue 665 (https://github.com/clap-rs/clap/issues/665)
+                // Issue 1105 (https://github.com/clap-rs/clap/issues/1105)
+                return Err(Error::empty_value(
                     self.cmd,
-                    ma.raw_vals_flatten()
-                        .last()
-                        .expect(INTERNAL_ERROR_MSG)
-                        .to_str()
-                        .expect(INVALID_UTF8)
-                        .to_string(),
+                    &get_possible_values(a)
+                        .iter()
+                        .filter(|pv| !pv.is_hide_set())
+                        .map(PossibleValue::get_name)
+                        .collect::<Vec<_>>(),
                     a.to_string(),
-                    Usage::new(self.cmd)
-                        .required(&self.required)
-                        .create_usage_with_title(&[]),
                 ));
             }
-        }
-        let min_vals_zero = if let Some(num) = a.get_min_vals() {
-            debug!("Validator::validate_arg_num_vals: min_vals set: {}", num);
-            if ma.num_vals() < num && num != 0 {
-                debug!("Validator::validate_arg_num_vals: Sending error TooFewValues");
-                return Err(Error::too_few_values(
-                    self.cmd,
-                    a.to_string(),
-                    num,
-                    ma.num_vals(),
-                    Usage::new(self.cmd)
-                        .required(&self.required)
-                        .create_usage_with_title(&[]),
-                ));
+
+            if let Some(expected) = a.num_vals {
+                if let Some(expected) = expected.num_values() {
+                    if expected != actual {
+                        debug!(
+                            "Validator::validate_arg_num_vals: Sending error WrongNumberOfValues"
+                        );
+                        return Err(Error::wrong_number_of_values(
+                            self.cmd,
+                            a.to_string(),
+                            expected,
+                            actual,
+                            Usage::new(self.cmd)
+                                .required(&self.required)
+                                .create_usage_with_title(&[]),
+                        ));
+                    }
+                } else if actual < expected.min_values() {
+                    return Err(Error::too_few_values(
+                        self.cmd,
+                        a.to_string(),
+                        expected.min_values(),
+                        actual,
+                        Usage::new(self.cmd)
+                            .required(&self.required)
+                            .create_usage_with_title(&[]),
+                    ));
+                } else if expected.max_values() < actual {
+                    debug!("Validator::validate_arg_num_vals: Sending error TooManyValues");
+                    return Err(Error::too_many_values(
+                        self.cmd,
+                        ma.raw_vals_flatten()
+                            .last()
+                            .expect(INTERNAL_ERROR_MSG)
+                            .to_str()
+                            .expect(INVALID_UTF8)
+                            .to_string(),
+                        a.to_string(),
+                        Usage::new(self.cmd)
+                            .required(&self.required)
+                            .create_usage_with_title(&[]),
+                    ));
+                }
             }
-            num == 0
-        } else {
-            false
-        };
-        // Issue 665 (https://github.com/clap-rs/clap/issues/665)
-        // Issue 1105 (https://github.com/clap-rs/clap/issues/1105)
-        if a.is_takes_value_set() && !min_vals_zero && ma.all_val_groups_empty() {
-            return Err(Error::empty_value(
-                self.cmd,
-                &get_possible_values(a)
-                    .iter()
-                    .filter(|pv| !pv.is_hide_set())
-                    .map(PossibleValue::get_name)
-                    .collect::<Vec<_>>(),
-                a.to_string(),
-            ));
         }
         Ok(())
     }


Test patch:
diff --git a/tests/builder/default_missing_vals.rs b/tests/builder/default_missing_vals.rs
index dad62f2e..f00fb007 100644
--- a/tests/builder/default_missing_vals.rs
+++ b/tests/builder/default_missing_vals.rs
@@ -7,7 +7,7 @@ fn opt_missing() {
             Arg::new("color")
                 .long("color")
                 .default_value("auto")
-                .min_values(0)
+                .number_of_values(0..=1)
                 .require_equals(true)
                 .default_missing_value("always"),
         )
@@ -33,7 +33,7 @@ fn opt_present_with_missing_value() {
             Arg::new("color")
                 .long("color")
                 .default_value("auto")
-                .min_values(0)
+                .number_of_values(0..=1)
                 .require_equals(true)
                 .default_missing_value("always"),
         )
@@ -59,7 +59,7 @@ fn opt_present_with_value() {
             Arg::new("color")
                 .long("color")
                 .default_value("auto")
-                .min_values(0)
+                .number_of_values(0..=1)
                 .require_equals(true)
                 .default_missing_value("always"),
         )
@@ -148,8 +148,7 @@ fn default_missing_value_per_occurrence() {
     // assert no change to usual argument handling when adding default_missing_value()
     let r = Command::new("cmd")
         .arg(
-            arg!(o: -o <opt> ... "some opt")
-                .min_values(0)
+            arg!(o: -o [opt] ... "some opt")
                 .default_value("default")
                 .default_missing_value("default_missing"),
         )
@@ -234,7 +233,7 @@ fn delimited_missing_value() {
             .long("flag")
             .default_value("one,two")
             .default_missing_value("three,four")
-            .min_values(0)
+            .number_of_values(0..)
             .value_delimiter(',')
             .require_equals(true),
     );
@@ -295,7 +294,7 @@ fn valid_index() {
             Arg::new("color")
                 .long("color")
                 .default_value("auto")
-                .min_values(0)
+                .number_of_values(0..=1)
                 .require_equals(true)
                 .default_missing_value("always"),
         )
diff --git a/tests/builder/help.rs b/tests/builder/help.rs
index 64d08b0b..188969e8 100644
--- a/tests/builder/help.rs
+++ b/tests/builder/help.rs
@@ -218,19 +218,19 @@ ARGS:
     <positional3>...    tests specific values [possible values: vi, emacs]
 
 OPTIONS:
-    -o, --option <opt>...             tests options
-    -f, --flag                        tests flags
-    -F                                tests flags with exclusions
-        --long-option-2 <option2>     tests long options with exclusions
-    -O, --option3 <option3>           specific vals [possible values: fast, slow]
-        --multvals <one> <two>        Tests multiple values, not mult occs
-        --multvalsmo <one> <two>      Tests multiple values, and mult occs
-        --minvals2 <minvals>...       Tests 2 min vals
-        --maxvals3 <maxvals>...       Tests 3 max vals
-        --optvaleq[=<optval>...]      Tests optional value, require = sign
-        --optvalnoeq [<optval>...]    Tests optional value
-    -h, --help                        Print help information
-    -V, --version                     Print version information
+    -o, --option <opt>...                    tests options
+    -f, --flag                               tests flags
+    -F                                       tests flags with exclusions
+        --long-option-2 <option2>            tests long options with exclusions
+    -O, --option3 <option3>                  specific vals [possible values: fast, slow]
+        --multvals <one> <two>               Tests multiple values, not mult occs
+        --multvalsmo <one> <two>             Tests multiple values, and mult occs
+        --minvals2 <minvals> <minvals>...    Tests 2 min vals
+        --maxvals3 <maxvals>...              Tests 3 max vals
+        --optvaleq[=<optval>...]             Tests optional value, require = sign
+        --optvalnoeq [<optval>...]           Tests optional value
+    -h, --help                               Print help information
+    -V, --version                            Print version information
 
 SUBCOMMANDS:
     subcmd    tests subcommands
@@ -1044,7 +1044,7 @@ OPTIONS:
 }
 
 #[test]
-#[should_panic = "Argument option: mismatch between `number_of_values` and `multiple_values`"]
+#[should_panic = "Argument option: mismatch between `number_of_values` (1) and `multiple_values`"]
 fn number_of_values_conflicts_with_multiple_values() {
     Command::new("ctest")
         .version("0.1")
diff --git a/tests/builder/multiple_values.rs b/tests/builder/multiple_values.rs
index 197f98ca..8efba4f9 100644
--- a/tests/builder/multiple_values.rs
+++ b/tests/builder/multiple_values.rs
@@ -93,7 +93,9 @@ fn option_exact_exact() {
                 .number_of_values(3)
                 .action(ArgAction::Append),
         )
-        .try_get_matches_from(vec!["", "-o", "val1", "-o", "val2", "-o", "val3"]);
+        .try_get_matches_from(vec![
+            "", "-o", "val1", "val2", "val3", "-o", "val4", "val5", "val6",
+        ]);
 
     assert!(m.is_ok(), "{}", m.unwrap_err());
     let m = m.unwrap();
@@ -104,7 +106,7 @@ fn option_exact_exact() {
             .unwrap()
             .map(|v| v.as_str())
             .collect::<Vec<_>>(),
-        ["val1", "val2", "val3"]
+        ["val1", "val2", "val3", "val4", "val5", "val6"]
     );
 }
 
@@ -200,10 +202,10 @@ fn option_min_exact() {
             Arg::new("option")
                 .short('o')
                 .help("multiple options")
-                .min_values(3)
-                .action(ArgAction::Append),
+                .number_of_values(3..)
+                .action(ArgAction::Set),
         )
-        .try_get_matches_from(vec!["", "-o", "val1", "-o", "val2", "-o", "val3"]);
+        .try_get_matches_from(vec!["", "-o", "val1", "val2", "val3"]);
 
     assert!(m.is_ok(), "{}", m.unwrap_err());
     let m = m.unwrap();
@@ -225,10 +227,10 @@ fn option_min_less() {
             Arg::new("option")
                 .short('o')
                 .help("multiple options")
-                .min_values(3)
-                .action(ArgAction::Append),
+                .number_of_values(3..)
+                .action(ArgAction::Set),
         )
-        .try_get_matches_from(vec!["", "-o", "val1", "-o", "val2"]);
+        .try_get_matches_from(vec!["", "-o", "val1", "val2"]);
 
     assert!(m.is_err());
     assert_eq!(m.unwrap_err().kind(), ErrorKind::TooFewValues);
@@ -242,12 +244,10 @@ fn option_short_min_more_mult_occurs() {
             Arg::new("option")
                 .short('o')
                 .help("multiple options")
-                .min_values(3)
-                .action(ArgAction::Append),
+                .number_of_values(3..)
+                .action(ArgAction::Set),
         )
-        .try_get_matches_from(vec![
-            "", "pos", "-o", "val1", "-o", "val2", "-o", "val3", "-o", "val4",
-        ]);
+        .try_get_matches_from(vec!["", "pos", "-o", "val1", "val2", "val3", "val4"]);
 
     assert!(m.is_ok(), "{}", m.unwrap_err());
     let m = m.unwrap();
@@ -272,7 +272,7 @@ fn option_short_min_more_single_occur() {
             Arg::new("option")
                 .short('o')
                 .help("multiple options")
-                .min_values(3),
+                .number_of_values(3..),
         )
         .try_get_matches_from(vec!["", "pos", "-o", "val1", "val2", "val3", "val4"]);
 
@@ -298,10 +298,10 @@ fn option_max_exact() {
             Arg::new("option")
                 .short('o')
                 .help("multiple options")
-                .max_values(3)
-                .action(ArgAction::Append),
+                .number_of_values(1..=3)
+                .action(ArgAction::Set),
         )
-        .try_get_matches_from(vec!["", "-o", "val1", "-o", "val2", "-o", "val3"]);
+        .try_get_matches_from(vec!["", "-o", "val1", "val2", "val3"]);
 
     assert!(m.is_ok(), "{}", m.unwrap_err());
     let m = m.unwrap();
@@ -323,10 +323,10 @@ fn option_max_less() {
             Arg::new("option")
                 .short('o')
                 .help("multiple options")
-                .max_values(3)
-                .action(ArgAction::Append),
+                .number_of_values(1..=3)
+                .action(ArgAction::Set),
         )
-        .try_get_matches_from(vec!["", "-o", "val1", "-o", "val2"]);
+        .try_get_matches_from(vec!["", "-o", "val1", "val2"]);
 
     assert!(m.is_ok(), "{}", m.unwrap_err());
     let m = m.unwrap();
@@ -341,6 +341,47 @@ fn option_max_less() {
     );
 }
 
+#[test]
+fn option_max_zero() {
+    let m = Command::new("multiple_values")
+        .arg(
+            Arg::new("option")
+                .short('o')
+                .help("multiple options")
+                .number_of_values(1..=3)
+                .action(ArgAction::Set),
+        )
+        .try_get_matches_from(vec!["", "-o"]);
+
+    assert!(m.is_err());
+    assert_eq!(m.unwrap_err().kind(), ErrorKind::InvalidValue);
+}
+
+#[test]
+fn option_max_zero_eq() {
+    let m = Command::new("multiple_values")
+        .arg(
+            Arg::new("option")
+                .short('o')
+                .help("multiple options")
+                .number_of_values(1..=3)
+                .action(ArgAction::Set),
+        )
+        .try_get_matches_from(vec!["", "-o="]);
+
+    assert!(m.is_ok(), "{}", m.unwrap_err());
+    let m = m.unwrap();
+
+    assert!(m.contains_id("option"));
+    assert_eq!(
+        m.get_many::<String>("option")
+            .unwrap()
+            .map(|v| v.as_str())
+            .collect::<Vec<_>>(),
+        [""]
+    );
+}
+
 #[test]
 fn option_max_more() {
     let m = Command::new("multiple_values")
@@ -348,15 +389,56 @@ fn option_max_more() {
             Arg::new("option")
                 .short('o')
                 .help("multiple options")
-                .max_values(3)
-                .action(ArgAction::Append),
+                .number_of_values(1..=3)
+                .action(ArgAction::Set),
         )
-        .try_get_matches_from(vec![
-            "", "-o", "val1", "-o", "val2", "-o", "val3", "-o", "val4",
-        ]);
+        .try_get_matches_from(vec!["", "-o", "val1", "val2", "val3", "val4"]);
 
     assert!(m.is_err());
-    assert_eq!(m.unwrap_err().kind(), ErrorKind::TooManyValues);
+    // Can end up being TooManyValues or UnknownArgument
+    assert_eq!(m.unwrap_err().kind(), ErrorKind::UnknownArgument);
+}
+
+#[test]
+fn optional_value() {
+    let mut cmd = Command::new("test").arg(
+        Arg::new("port")
+            .short('p')
+            .value_name("NUM")
+            .number_of_values(0..=1),
+    );
+
+    let r = cmd.try_get_matches_from_mut(["test", "-p42"]);
+    assert!(r.is_ok(), "{}", r.unwrap_err());
+    let m = r.unwrap();
+    assert!(m.contains_id("port"));
+    assert_eq!(m.get_one::<String>("port").unwrap(), "42");
+
+    let r = cmd.try_get_matches_from_mut(["test", "-p"]);
+    assert!(r.is_ok(), "{}", r.unwrap_err());
+    let m = r.unwrap();
+    assert!(m.contains_id("port"));
+    assert!(m.get_one::<String>("port").is_none());
+
+    let r = cmd.try_get_matches_from_mut(["test", "-p", "24", "-p", "42"]);
+    assert!(r.is_ok(), "{}", r.unwrap_err());
+    let m = r.unwrap();
+    assert!(m.contains_id("port"));
+    assert_eq!(m.get_one::<String>("port").unwrap(), "42");
+
+    let mut help = Vec::new();
+    cmd.write_help(&mut help).unwrap();
+    const HELP: &str = "\
+test 
+
+USAGE:
+    test [OPTIONS]
+
+OPTIONS:
+    -p [<NUM>...]        
+    -h, --help           Print help information
+";
+    snapbox::assert_eq(HELP, help);
 }
 
 #[test]
@@ -437,7 +519,11 @@ fn positional_exact_more() {
 #[test]
 fn positional_min_exact() {
     let m = Command::new("multiple_values")
-        .arg(Arg::new("pos").help("multiple positionals").min_values(3))
+        .arg(
+            Arg::new("pos")
+                .help("multiple positionals")
+                .number_of_values(3..),
+        )
         .try_get_matches_from(vec!["myprog", "val1", "val2", "val3"]);
 
     assert!(m.is_ok(), "{}", m.unwrap_err());
@@ -456,7 +542,11 @@ fn positional_min_exact() {
 #[test]
 fn positional_min_less() {
     let m = Command::new("multiple_values")
-        .arg(Arg::new("pos").help("multiple positionals").min_values(3))
+        .arg(
+            Arg::new("pos")
+                .help("multiple positionals")
+                .number_of_values(3..),
+        )
         .try_get_matches_from(vec!["myprog", "val1", "val2"]);
 
     assert!(m.is_err());
@@ -466,7 +556,11 @@ fn positional_min_less() {
 #[test]
 fn positional_min_more() {
     let m = Command::new("multiple_values")
-        .arg(Arg::new("pos").help("multiple positionals").min_values(3))
+        .arg(
+            Arg::new("pos")
+                .help("multiple positionals")
+                .number_of_values(3..),
+        )
         .try_get_matches_from(vec!["myprog", "val1", "val2", "val3", "val4"]);
 
     assert!(m.is_ok(), "{}", m.unwrap_err());
@@ -485,7 +579,11 @@ fn positional_min_more() {
 #[test]
 fn positional_max_exact() {
     let m = Command::new("multiple_values")
-        .arg(Arg::new("pos").help("multiple positionals").max_values(3))
+        .arg(
+            Arg::new("pos")
+                .help("multiple positionals")
+                .number_of_values(1..=3),
+        )
         .try_get_matches_from(vec!["myprog", "val1", "val2", "val3"]);
 
     assert!(m.is_ok(), "{}", m.unwrap_err());
@@ -504,7 +602,11 @@ fn positional_max_exact() {
 #[test]
 fn positional_max_less() {
     let m = Command::new("multiple_values")
-        .arg(Arg::new("pos").help("multiple positionals").max_values(3))
+        .arg(
+            Arg::new("pos")
+                .help("multiple positionals")
+                .number_of_values(1..=3),
+        )
         .try_get_matches_from(vec!["myprog", "val1", "val2"]);
 
     assert!(m.is_ok(), "{}", m.unwrap_err());
@@ -523,7 +625,11 @@ fn positional_max_less() {
 #[test]
 fn positional_max_more() {
     let m = Command::new("multiple_values")
-        .arg(Arg::new("pos").help("multiple positionals").max_values(3))
+        .arg(
+            Arg::new("pos")
+                .help("multiple positionals")
+                .number_of_values(1..=3),
+        )
         .try_get_matches_from(vec!["myprog", "val1", "val2", "val3", "val4"]);
 
     assert!(m.is_err());
@@ -1359,7 +1465,7 @@ fn multiple_vals_with_hyphen() {
 #[test]
 fn issue_1480_max_values_consumes_extra_arg_1() {
     let res = Command::new("prog")
-        .arg(Arg::new("field").max_values(1).long("field"))
+        .arg(Arg::new("field").number_of_values(..=1).long("field"))
         .arg(Arg::new("positional").required(true).index(1))
         .try_get_matches_from(vec!["prog", "--field", "1", "file"]);
 
@@ -1369,7 +1475,7 @@ fn issue_1480_max_values_consumes_extra_arg_1() {
 #[test]
 fn issue_1480_max_values_consumes_extra_arg_2() {
     let res = Command::new("prog")
-        .arg(Arg::new("field").max_values(1).long("field"))
+        .arg(Arg::new("field").number_of_values(..=1).long("field"))
         .try_get_matches_from(vec!["prog", "--field", "1", "2"]);
 
     assert!(res.is_err());
@@ -1379,7 +1485,7 @@ fn issue_1480_max_values_consumes_extra_arg_2() {
 #[test]
 fn issue_1480_max_values_consumes_extra_arg_3() {
     let res = Command::new("prog")
-        .arg(Arg::new("field").max_values(1).long("field"))
+        .arg(Arg::new("field").number_of_values(..=1).long("field"))
         .try_get_matches_from(vec!["prog", "--field", "1", "2", "3"]);
 
     assert!(res.is_err());
diff --git a/tests/builder/opts.rs b/tests/builder/opts.rs
index 1d9b8900..0c6ba9f0 100644
--- a/tests/builder/opts.rs
+++ b/tests/builder/opts.rs
@@ -71,7 +71,7 @@ fn require_equals_min_values_zero() {
             Arg::new("cfg")
                 .action(ArgAction::Set)
                 .require_equals(true)
-                .min_values(0)
+                .number_of_values(0..)
                 .long("config"),
         )
         .arg(Arg::new("cmd"))
@@ -514,7 +514,7 @@ fn issue_1047_min_zero_vals_default_val() {
                 .long("del")
                 .action(ArgAction::Set)
                 .require_equals(true)
-                .min_values(0)
+                .number_of_values(0..)
                 .default_missing_value("default"),
         )
         .try_get_matches_from(vec!["foo", "-d"])
diff --git a/tests/builder/require.rs b/tests/builder/require.rs
index 55a477b8..c97d3ae6 100644
--- a/tests/builder/require.rs
+++ b/tests/builder/require.rs
@@ -344,7 +344,7 @@ fn required_unless_present_err() {
 #[test]
 fn required_unless_present_with_optional_value() {
     let res = Command::new("unlesstest")
-        .arg(Arg::new("opt").long("opt").min_values(0).max_values(1))
+        .arg(Arg::new("opt").long("opt").number_of_values(0..=1))
         .arg(
             Arg::new("cfg")
                 .required_unless_present("dbg")
@@ -1144,7 +1144,7 @@ fn short_flag_require_equals_with_minvals_zero() {
         .arg(
             Arg::new("check")
                 .short('c')
-                .min_values(0)
+                .number_of_values(0..)
                 .require_equals(true),
         )
         .arg(Arg::new("unique").short('u').action(ArgAction::SetTrue))
@@ -1162,7 +1162,7 @@ fn issue_2624() {
                 .short('c')
                 .long("check")
                 .require_equals(true)
-                .min_values(0)
+                .number_of_values(0..)
                 .value_parser(["silent", "quiet", "diagnose-first"]),
         )
         .arg(
diff --git a/tests/builder/utils.rs b/tests/builder/utils.rs
index 3ee91acb..55ab1ebe 100644
--- a/tests/builder/utils.rs
+++ b/tests/builder/utils.rs
@@ -80,19 +80,17 @@ pub fn complex_app() -> Command<'static> {
                 .required(false),
             arg!(--minvals2 <minvals> "Tests 2 min vals")
                 .required(false)
-                .min_values(2),
+                .number_of_values(2..),
             arg!(--maxvals3 <maxvals> "Tests 3 max vals")
                 .required(false)
-                .max_values(3),
+                .number_of_values(1..=3),
             arg!(--optvaleq <optval> "Tests optional value, require = sign")
                 .required(false)
-                .min_values(0)
-                .max_values(1)
+                .number_of_values(0..=1)
                 .require_equals(true),
             arg!(--optvalnoeq <optval> "Tests optional value")
                 .required(false)
-                .min_values(0)
-                .max_values(1),
+                .number_of_values(0..=1),
         ])
         .subcommand(
             Command::new("subcmd")
diff --git a/tests/derive/options.rs b/tests/derive/options.rs
index c15cf950..7e2dd4ef 100644
--- a/tests/derive/options.rs
+++ b/tests/derive/options.rs
@@ -271,8 +271,7 @@ fn option_option_type_help() {
         arg: Option<Option<i32>>,
     }
     let help = utils::get_help::<Opt>();
-    assert!(help.contains("--arg [<val>]"));
-    assert!(!help.contains("--arg [<val>]..."));
+    assert!(help.contains("--arg [<val>...]"));
 }
 
 #[test]
@@ -429,7 +428,7 @@ fn option_vec_type() {
 fn option_vec_type_structopt_behavior() {
     #[derive(Parser, PartialEq, Debug)]
     struct Opt {
-        #[clap(short, long, multiple_values(true), min_values(0))]
+        #[clap(short, long, multiple_values(true), number_of_values(0..))]
         arg: Option<Vec<i32>>,
     }
     assert_eq!(