Merge pull request #4026 from epage/require

fix!: Remove `Arg::rwquire_value_delimiter`

CHANGELOG.md

@@ -31,6 +31,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - 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::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
 - *(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)

src/builder/arg.rs

@@ -1409,88 +1409,6 @@ impl<'help> Arg<'help> {
         self.takes_value(true)
     }
 
-    /// Specifies that *multiple values* may only be set using the delimiter.
-    ///
-    /// This means if an option is encountered, and no delimiter is found, it is assumed that no
-    /// additional values for that option follow. This is unlike the default, where it is generally
-    /// assumed that more values will follow regardless of whether or not a delimiter is used.
-    ///
-    /// **NOTE:** It's a good idea to inform the user that use of a delimiter is required, either
-    /// through help text or other means.
-    ///
-    /// # Examples
-    ///
-    /// These examples demonstrate what happens when `require_delimiter(true)` is used. Notice
-    /// everything works in this first example, as we use a delimiter, as expected.
-    ///
-    /// ```rust
-    /// # use clap::{Command, Arg, ArgAction};
-    /// let delims = Command::new("prog")
-    ///     .arg(Arg::new("opt")
-    ///         .short('o')
-    ///         .action(ArgAction::Set)
-    ///         .value_delimiter(',')
-    ///         .require_value_delimiter(true)
-    ///         .num_args(1..))
-    ///     .get_matches_from(vec![
-    ///         "prog", "-o", "val1,val2,val3",
-    ///     ]);
-    ///
-    /// assert!(delims.contains_id("opt"));
-    /// assert_eq!(delims.get_many::<String>("opt").unwrap().collect::<Vec<_>>(), ["val1", "val2", "val3"]);
-    /// ```
-    ///
-    /// In this next example, we will *not* use a delimiter. Notice it's now an error.
-    ///
-    /// ```rust
-    /// # use clap::{Command, Arg, error::ErrorKind, ArgAction};
-    /// let res = Command::new("prog")
-    ///     .arg(Arg::new("opt")
-    ///         .short('o')
-    ///         .action(ArgAction::Set)
-    ///         .require_value_delimiter(true))
-    ///     .try_get_matches_from(vec![
-    ///         "prog", "-o", "val1", "val2", "val3",
-    ///     ]);
-    ///
-    /// assert!(res.is_err());
-    /// let err = res.unwrap_err();
-    /// assert_eq!(err.kind(), ErrorKind::UnknownArgument);
-    /// ```
-    ///
-    /// What's happening is `-o` is getting `val1`, and because delimiters are required yet none
-    /// were present, it stops parsing `-o`. At this point it reaches `val2` and because no
-    /// positional arguments have been defined, it's an error of an unexpected argument.
-    ///
-    /// In this final example, we contrast the above with `clap`'s default behavior where the above
-    /// is *not* an error.
-    ///
-    /// ```rust
-    /// # use clap::{Command, Arg, ArgAction};
-    /// let delims = Command::new("prog")
-    ///     .arg(Arg::new("opt")
-    ///         .short('o')
-    ///         .action(ArgAction::Set)
-    ///         .num_args(1..))
-    ///     .get_matches_from(vec![
-    ///         "prog", "-o", "val1", "val2", "val3",
-    ///     ]);
-    ///
-    /// assert!(delims.contains_id("opt"));
-    /// assert_eq!(delims.get_many::<String>("opt").unwrap().collect::<Vec<_>>(), ["val1", "val2", "val3"]);
-    /// ```
-    #[inline]
-    #[must_use]
-    pub fn require_value_delimiter(mut self, yes: bool) -> Self {
-        if yes {
-            self.val_delim.get_or_insert(',');
-            self.setting(ArgSettings::RequireDelimiter)
-                .takes_value(true)
-        } else {
-            self.unset_setting(ArgSettings::RequireDelimiter)
-        }
-    }
-
     /// Sentinel to **stop** parsing multiple values of a given argument.
     ///
     /// By default when
@@ -3989,11 +3907,6 @@ impl<'help> Arg<'help> {
         self.is_set(ArgSettings::HiddenLongHelp)
     }
 
-    /// Report whether [`Arg::require_value_delimiter`] is set
-    pub fn is_require_value_delimiter_set(&self) -> bool {
-        self.is_set(ArgSettings::RequireDelimiter)
-    }
-
     /// Report whether [`Arg::require_equals`] is set
     pub fn is_require_equals_set(&self) -> bool {
         self.is_set(ArgSettings::RequireEquals)
@@ -4098,12 +4011,7 @@ impl<'help> Arg<'help> {
     // Used for positionals when printing
     pub(crate) fn name_no_brackets(&self) -> Cow<str> {
         debug!("Arg::name_no_brackets:{}", self.name);
-        let delim = if self.is_require_value_delimiter_set() {
-            self.val_delim.expect(INTERNAL_ERROR_MSG)
-        } else {
-            ' '
-        }
-        .to_string();
+        let delim = " ";
         if !self.val_names.is_empty() {
             debug!("Arg::name_no_brackets: val_names={:#?}", self.val_names);
 
@@ -4268,13 +4176,7 @@ impl Default for ArgProvider {
 pub(crate) fn render_arg_val(arg: &Arg) -> String {
     let mut rendered = String::new();
 
-    let delim_storage;
-    let delim = if arg.is_require_value_delimiter_set() {
-        delim_storage = arg.val_delim.expect(INTERNAL_ERROR_MSG).to_string();
-        &delim_storage
-    } else {
-        " "
-    };
+    let delim = " ";
 
     let arg_name_storage;
     let val_names = if arg.val_names.is_empty() {

src/builder/arg_settings.rs

@@ -33,7 +33,6 @@ pub(crate) enum ArgSettings {
     Hidden,
     TakesValue,
     NextLineHelp,
-    RequireDelimiter,
     HidePossibleValues,
     AllowHyphenValues,
     RequireEquals,
@@ -56,7 +55,6 @@ bitflags! {
         const HIDDEN           = 1 << 4;
         const TAKES_VAL        = 1 << 5;
         const NEXT_LINE_HELP   = 1 << 7;
-        const REQ_DELIM        = 1 << 9;
         const DELIM_NOT_SET    = 1 << 10;
         const HIDE_POS_VALS    = 1 << 11;
         const ALLOW_TAC_VALS   = 1 << 12;
@@ -83,7 +81,6 @@ impl_settings! { ArgSettings, ArgFlags,
     Hidden => Flags::HIDDEN,
     TakesValue => Flags::TAKES_VAL,
     NextLineHelp => Flags::NEXT_LINE_HELP,
-    RequireDelimiter => Flags::REQ_DELIM,
     HidePossibleValues => Flags::HIDE_POS_VALS,
     AllowHyphenValues => Flags::ALLOW_TAC_VALS,
     RequireEquals => Flags::REQUIRE_EQUALS,

src/builder/debug_asserts.rs

@@ -768,7 +768,6 @@ fn assert_arg_flags(arg: &Arg) {
         }
     }
 
-    checker!(is_require_value_delimiter_set requires is_takes_value_set);
     checker!(is_hide_possible_values_set requires is_takes_value_set);
     checker!(is_allow_hyphen_values_set requires is_takes_value_set);
     checker!(is_require_equals_set requires is_takes_value_set);

src/parser/arg_matcher.rs

@@ -202,9 +202,7 @@ impl ArgMatcher {
             "ArgMatcher::needs_more_vals: o={}, pending={}",
             o.name, num_pending
         );
-        if num_pending == 1 && o.is_require_value_delimiter_set() {
-            false
-        } else if let Some(expected) = o.get_num_args() {
+        if let Some(expected) = o.get_num_args() {
             debug!(
                 "ArgMatcher::needs_more_vals: expected={}, actual={}",
                 expected, num_pending

tests/builder/app_settings.rs

@@ -1151,7 +1151,6 @@ fn aaos_opts_mult_req_delims() {
             arg!(--opt <val> ... "some option")
                 .action(ArgAction::Set)
                 .value_delimiter(',')
-                .require_value_delimiter(true)
                 .action(ArgAction::Append),
         )
         .try_get_matches_from(vec!["", "--opt=some", "--opt=other", "--opt=one,two"]);

tests/builder/default_vals.rs

@@ -818,15 +818,14 @@ fn invalid_default_values() {
 fn valid_delimited_default_values() {
     use clap::{Arg, Command};
 
-    let _ = Command::new("test")
+    Command::new("test")
         .arg(
             Arg::new("arg")
                 .value_parser(clap::value_parser!(u32))
                 .value_delimiter(',')
-                .require_value_delimiter(true)
                 .default_value("1,2,3"),
         )
-        .try_get_matches();
+        .debug_assert();
 }
 
 #[cfg(debug_assertions)]
@@ -835,15 +834,14 @@ fn valid_delimited_default_values() {
 fn invalid_delimited_default_values() {
     use clap::{Arg, Command};
 
-    let _ = Command::new("test")
+    Command::new("test")
         .arg(
             Arg::new("arg")
                 .value_parser(clap::value_parser!(u32))
                 .value_delimiter(',')
-                .require_value_delimiter(true)
                 .default_value("one,two"),
         )
-        .try_get_matches();
+        .debug_assert();
 }
 
 #[test]

tests/builder/help.rs

@@ -1580,10 +1580,10 @@ Kevin K.
 tests stuff
 
 USAGE:
-    test --fake <some>:<val>
+    test --fake <some> <val>
 
 OPTIONS:
-    -f, --fake <some>:<val>    some help
+    -f, --fake <some> <val>    some help
     -h, --help                 Print help information
     -V, --version              Print version information
 ";
@@ -1597,8 +1597,6 @@ OPTIONS:
                 .required(true)
                 .value_names(&["some", "val"])
                 .action(ArgAction::Set)
-                .value_delimiter(',')
-                .require_value_delimiter(true)
                 .value_delimiter(':'),
         );
 
@@ -1612,10 +1610,10 @@ Will M.
 does stuff
 
 USAGE:
-    test [OPTIONS] --fake <some>:<val>
+    test [OPTIONS] --fake <some> <val>
 
 OPTIONS:
-    -f, --fake <some>:<val>    some help
+    -f, --fake <some> <val>    some help
     -h, --help                 Print help information
     -V, --version              Print version information
 
@@ -1633,8 +1631,6 @@ NETWORKING:
                 .required(true)
                 .value_names(&["some", "val"])
                 .action(ArgAction::Set)
-                .value_delimiter(',')
-                .require_value_delimiter(true)
                 .value_delimiter(':'),
         )
         .next_help_heading(Some("NETWORKING"))
@@ -1655,10 +1651,10 @@ Will M.
 does stuff
 
 USAGE:
-    test [OPTIONS] --fake <some>:<val> --birthday-song <song> --birthday-song-volume <volume>
+    test [OPTIONS] --fake <some> <val> --birthday-song <song> --birthday-song-volume <volume>
 
 OPTIONS:
-    -f, --fake <some>:<val>    some help
+    -f, --fake <some> <val>    some help
         --style <style>        Choose musical style to play the song
     -s, --speed <SPEED>        How fast? [possible values: fast, slow]
     -h, --help                 Print help information
@@ -1686,8 +1682,6 @@ fn multiple_custom_help_headers() {
                 .required(true)
                 .value_names(&["some", "val"])
                 .action(ArgAction::Set)
-                .value_delimiter(',')
-                .require_value_delimiter(true)
                 .value_delimiter(':'),
         )
         .next_help_heading(Some("NETWORKING"))

tests/builder/multiple_values.rs

@@ -837,9 +837,8 @@ fn req_delimiter_long() {
         .arg(
             Arg::new("option")
                 .long("option")
-                .num_args(1..)
-                .value_delimiter(',')
-                .require_value_delimiter(true),
+                .num_args(1)
+                .value_delimiter(','),
         )
         .arg(
             Arg::new("args")
@@ -875,9 +874,8 @@ fn req_delimiter_long_with_equal() {
         .arg(
             Arg::new("option")
                 .long("option")
-                .num_args(1..)
-                .value_delimiter(',')
-                .require_value_delimiter(true),
+                .num_args(1)
+                .value_delimiter(','),
         )
         .arg(
             Arg::new("args")
@@ -913,9 +911,8 @@ fn req_delimiter_short_with_space() {
         .arg(
             Arg::new("option")
                 .short('o')
-                .num_args(1..)
-                .value_delimiter(',')
-                .require_value_delimiter(true),
+                .num_args(1)
+                .value_delimiter(','),
         )
         .arg(
             Arg::new("args")
@@ -951,9 +948,8 @@ fn req_delimiter_short_with_no_space() {
         .arg(
             Arg::new("option")
                 .short('o')
-                .num_args(1..)
-                .value_delimiter(',')
-                .require_value_delimiter(true),
+                .num_args(1)
+                .value_delimiter(','),
         )
         .arg(
             Arg::new("args")
@@ -989,9 +985,8 @@ fn req_delimiter_short_with_equal() {
         .arg(
             Arg::new("option")
                 .short('o')
-                .num_args(1..)
-                .value_delimiter(',')
-                .require_value_delimiter(true),
+                .num_args(1)
+                .value_delimiter(','),
         )
         .arg(
             Arg::new("args")
@@ -1028,10 +1023,9 @@ fn req_delimiter_complex() {
             Arg::new("option")
                 .long("option")
                 .short('o')
-                .num_args(1..)
+                .num_args(1)
                 .action(ArgAction::Append)
-                .value_delimiter(',')
-                .require_value_delimiter(true),
+                .value_delimiter(','),
         )
         .arg(Arg::new("args").num_args(1..).index(1))
         .try_get_matches_from(vec![

tests/builder/opts.rs

@@ -336,11 +336,7 @@ fn multiple_vals_pos_arg_equals() {
 #[test]
 fn require_delims_no_delim() {
     let r = Command::new("mvae")
-        .arg(
-            arg!(o: -o [opt] ... "some opt")
-                .value_delimiter(',')
-                .require_value_delimiter(true),
-        )
+        .arg(arg!(o: -o [opt] ... "some opt").value_delimiter(','))
         .arg(arg!([file] "some file"))
         .try_get_matches_from(vec!["mvae", "-o", "1", "2", "some"]);
     assert!(r.is_err());
@@ -351,12 +347,7 @@ fn require_delims_no_delim() {
 #[test]
 fn require_delims() {
     let r = Command::new("mvae")
-        .arg(
-            arg!(o: -o <opt> "some opt")
-                .num_args(1..)
-                .value_delimiter(',')
-                .require_value_delimiter(true),
-        )
+        .arg(arg!(o: -o <opt> "some opt").value_delimiter(','))
         .arg(arg!([file] "some file"))
         .try_get_matches_from(vec!["", "-o", "1,2", "some"]);
     assert!(r.is_ok(), "{}", r.unwrap_err());