Merge pull request #4022 from epage/delim

fix!: Remove Arg::use_value_delimiter in favor of Arg::value_delimiter

CHANGELOG.md

@@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - 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)
 - Remove `Arg::multiple_values(true)` with `Arg::number_of_values(1..)` and `Arg::multiple_values(false)` with `Arg::number_of_values(0)`
+- Remove `Arg::use_value_delimiter` in favor of `Arg::value_delimiter`
 - `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)

examples/multicall-busybox.rs

@@ -25,8 +25,7 @@ fn main() {
                         .exclusive(true)
                         .action(ArgAction::Set)
                         .default_missing_value("/usr/local/bin")
-                        .value_parser(value_parser!(PathBuf))
-                        .use_value_delimiter(false),
+                        .value_parser(value_parser!(PathBuf)),
                 )
                 .subcommands(applet_commands()),
         )

src/builder/arg.rs

@@ -786,12 +786,6 @@ impl<'help> Arg<'help> {
     /// - Using an equals and no space such as `-o=value` or `--option=value`
     /// - Use a short and no space such as `-ovalue`
     ///
-    /// **NOTE:** By default, args which allow [multiple values] are delimited by commas, meaning
-    /// `--option=val1,val2,val3` is three values for the `--option` argument. If you wish to
-    /// change the delimiter to another character you can use [`Arg::value_delimiter(char)`],
-    /// alternatively you can turn delimiting values **OFF** by using
-    /// [`Arg::use_value_delimiter(false)`][Arg::use_value_delimiter]
-    ///
     /// # Examples
     ///
     /// ```rust
@@ -807,7 +801,6 @@ impl<'help> Arg<'help> {
     /// assert!(m.contains_id("mode"));
     /// assert_eq!(m.get_one::<String>("mode").unwrap(), "fast");
     /// ```
-    /// [`Arg::value_delimiter(char)`]: Arg::value_delimiter()
     /// [multiple values]: Arg::number_of_values
     #[inline]
     #[must_use]
@@ -1060,7 +1053,6 @@ impl<'help> Arg<'help> {
     /// assert_eq!(files, ["file1", "file2", "file3"]);
     /// assert_eq!(m.get_one::<String>("word").unwrap(), "word");
     /// ```
-    /// [`Arg::value_delimiter(char)`]: Arg::value_delimiter()
     #[inline]
     #[must_use]
     pub fn number_of_values(mut self, qty: impl Into<ValuesRange>) -> Self {
@@ -1385,72 +1377,12 @@ impl<'help> Arg<'help> {
         }
     }
 
-    /// Specifies that an argument should allow grouping of multiple values via a
-    /// delimiter.
+    /// Allow grouping of multiple values via a delimiter.
     ///
     /// i.e. should `--option=val1,val2,val3` be parsed as three values (`val1`, `val2`,
     /// and `val3`) or as a single value (`val1,val2,val3`). Defaults to using `,` (comma) as the
     /// value delimiter for all arguments that accept values (options and positional arguments)
     ///
-    /// **NOTE:** When this setting is used, it will default [`Arg::value_delimiter`]
-    /// to the comma `,`.
-    ///
-    /// **NOTE:** Implicitly sets [`Arg::takes_value`]
-    ///
-    /// # Examples
-    ///
-    /// The following example shows the default behavior.
-    ///
-    /// ```rust
-    /// # use clap::{Command, Arg, ArgAction};
-    /// let delims = Command::new("prog")
-    ///     .arg(Arg::new("option")
-    ///         .long("option")
-    ///         .use_value_delimiter(true)
-    ///         .action(ArgAction::Set))
-    ///     .get_matches_from(vec![
-    ///         "prog", "--option=val1,val2,val3",
-    ///     ]);
-    ///
-    /// assert!(delims.contains_id("option"));
-    /// assert_eq!(delims.get_many::<String>("option").unwrap().collect::<Vec<_>>(), ["val1", "val2", "val3"]);
-    /// ```
-    /// The next example shows the difference when turning delimiters off. This is the default
-    /// behavior
-    ///
-    /// ```rust
-    /// # use clap::{Command, Arg, ArgAction};
-    /// let nodelims = Command::new("prog")
-    ///     .arg(Arg::new("option")
-    ///         .long("option")
-    ///         .action(ArgAction::Set))
-    ///     .get_matches_from(vec![
-    ///         "prog", "--option=val1,val2,val3",
-    ///     ]);
-    ///
-    /// assert!(nodelims.contains_id("option"));
-    /// assert_eq!(nodelims.get_one::<String>("option").unwrap(), "val1,val2,val3");
-    /// ```
-    /// [`Arg::value_delimiter`]: Arg::value_delimiter()
-    #[inline]
-    #[must_use]
-    pub fn use_value_delimiter(mut self, yes: bool) -> Self {
-        if yes {
-            if self.val_delim.is_none() {
-                self.val_delim = Some(',');
-            }
-            self.takes_value(true)
-                .setting(ArgSettings::UseValueDelimiter)
-        } else {
-            self.val_delim = None;
-            self.unset_setting(ArgSettings::UseValueDelimiter)
-        }
-    }
-
-    /// Separator between the arguments values, defaults to `,` (comma).
-    ///
-    /// **NOTE:** implicitly sets [`Arg::use_value_delimiter(true)`]
-    ///
     /// **NOTE:** implicitly sets [`Arg::action(ArgAction::Set)`]
     ///
     /// # Examples
@@ -1461,20 +1393,20 @@ impl<'help> Arg<'help> {
     ///     .arg(Arg::new("config")
     ///         .short('c')
     ///         .long("config")
-    ///         .value_delimiter(';'))
+    ///         .value_delimiter(','))
     ///     .get_matches_from(vec![
-    ///         "prog", "--config=val1;val2;val3"
+    ///         "prog", "--config=val1,val2,val3"
     ///     ]);
     ///
     /// assert_eq!(m.get_many::<String>("config").unwrap().collect::<Vec<_>>(), ["val1", "val2", "val3"])
     /// ```
-    /// [`Arg::use_value_delimiter(true)`]: Arg::use_value_delimiter()
+    /// [`Arg::value_delimiter(',')`]: Arg::use_value_delimiter()
     /// [`Arg::action(ArgAction::Set)`]: Arg::takes_value()
     #[inline]
     #[must_use]
-    pub fn value_delimiter(mut self, d: char) -> Self {
-        self.val_delim = Some(d);
-        self.takes_value(true).use_value_delimiter(true)
+    pub fn value_delimiter(mut self, d: impl Into<Option<char>>) -> Self {
+        self.val_delim = d.into();
+        self.takes_value(true)
     }
 
     /// Specifies that *multiple values* may only be set using the delimiter.
@@ -1483,11 +1415,6 @@ impl<'help> Arg<'help> {
     /// 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:** The default is `false`.
-    ///
-    /// **NOTE:** Setting this requires [`Arg::use_value_delimiter`] and
-    /// [`Arg::takes_value`]
-    ///
     /// **NOTE:** It's a good idea to inform the user that use of a delimiter is required, either
     /// through help text or other means.
     ///
@@ -1502,7 +1429,7 @@ impl<'help> Arg<'help> {
     ///     .arg(Arg::new("opt")
     ///         .short('o')
     ///         .action(ArgAction::Set)
-    ///         .use_value_delimiter(true)
+    ///         .value_delimiter(',')
     ///         .require_value_delimiter(true)
     ///         .number_of_values(1..))
     ///     .get_matches_from(vec![
@@ -1521,7 +1448,6 @@ impl<'help> Arg<'help> {
     ///     .arg(Arg::new("opt")
     ///         .short('o')
     ///         .action(ArgAction::Set)
-    ///         .use_value_delimiter(true)
     ///         .require_value_delimiter(true))
     ///     .try_get_matches_from(vec![
     ///         "prog", "-o", "val1", "val2", "val3",
@@ -1555,9 +1481,11 @@ impl<'help> Arg<'help> {
     /// ```
     #[inline]
     #[must_use]
-    pub fn require_value_delimiter(self, yes: bool) -> Self {
+    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)
         }
@@ -2009,15 +1937,15 @@ impl<'help> Arg<'help> {
     ///         .env("MY_FLAG_MULTI")
     ///         .action(ArgAction::Set)
     ///         .number_of_values(1..)
-    ///         .use_value_delimiter(true))
+    ///         .value_delimiter(','))
     ///     .get_matches_from(vec![
     ///         "prog"
     ///     ]);
     ///
     /// assert_eq!(m.get_many::<String>("flag").unwrap().collect::<Vec<_>>(), vec!["env1", "env2"]);
     /// ```
     /// [`Arg::action(ArgAction::Set)`]: Arg::takes_value()
-    /// [`Arg::use_value_delimiter(true)`]: Arg::use_value_delimiter()
+    /// [`Arg::value_delimiter(',')`]: Arg::use_value_delimiter()
     #[cfg(feature = "env")]
     #[inline]
     #[must_use]
@@ -4061,11 +3989,6 @@ impl<'help> Arg<'help> {
         self.is_set(ArgSettings::HiddenLongHelp)
     }
 
-    /// Report whether [`Arg::use_value_delimiter`] is set
-    pub fn is_use_value_delimiter_set(&self) -> bool {
-        self.is_set(ArgSettings::UseValueDelimiter)
-    }
-
     /// Report whether [`Arg::require_value_delimiter`] is set
     pub fn is_require_value_delimiter_set(&self) -> bool {
         self.is_set(ArgSettings::RequireDelimiter)
@@ -4144,12 +4067,6 @@ impl<'help> Arg<'help> {
             }
         }
 
-        if (self.is_use_value_delimiter_set() || self.is_require_value_delimiter_set())
-            && self.val_delim.is_none()
-        {
-            self.val_delim = Some(',');
-        }
-
         let val_names_len = self.val_names.len();
         if val_names_len > 1 {
             self.settings.set(ArgSettings::MultipleValues);

src/builder/arg_settings.rs

@@ -32,7 +32,6 @@ pub(crate) enum ArgSettings {
     Global,
     Hidden,
     TakesValue,
-    UseValueDelimiter,
     NextLineHelp,
     RequireDelimiter,
     HidePossibleValues,
@@ -56,7 +55,6 @@ bitflags! {
         const GLOBAL           = 1 << 3;
         const HIDDEN           = 1 << 4;
         const TAKES_VAL        = 1 << 5;
-        const USE_DELIM        = 1 << 6;
         const NEXT_LINE_HELP   = 1 << 7;
         const REQ_DELIM        = 1 << 9;
         const DELIM_NOT_SET    = 1 << 10;
@@ -84,7 +82,6 @@ impl_settings! { ArgSettings, ArgFlags,
     Global => Flags::GLOBAL,
     Hidden => Flags::HIDDEN,
     TakesValue => Flags::TAKES_VAL,
-    UseValueDelimiter => Flags::USE_DELIM,
     NextLineHelp => Flags::NEXT_LINE_HELP,
     RequireDelimiter => Flags::REQ_DELIM,
     HidePossibleValues => Flags::HIDE_POS_VALS,

src/builder/command.rs

@@ -972,7 +972,7 @@ impl<'help> Command<'help> {
     /// was used.
     ///
     /// **NOTE:** The same thing can be done manually by setting the final positional argument to
-    /// [`Arg::use_value_delimiter(false)`]. Using this setting is safer, because it's easier to locate
+    /// [`Arg::value_delimiter(None)`]. Using this setting is safer, because it's easier to locate
     /// when making changes.
     ///
     /// **NOTE:** This choice is propagated to all child subcommands.
@@ -986,7 +986,7 @@ impl<'help> Command<'help> {
     ///     .get_matches();
     /// ```
     ///
-    /// [`Arg::use_value_delimiter(false)`]: crate::Arg::use_value_delimiter()
+    /// [`Arg::value_delimiter(None)`]: crate::Arg::value_delimiter()
     #[inline]
     pub fn dont_delimit_trailing_values(self, yes: bool) -> Self {
         if yes {

src/builder/debug_asserts.rs

@@ -759,7 +759,6 @@ fn assert_arg_flags(arg: &Arg) {
     }
 
     checker!(is_require_value_delimiter_set requires is_takes_value_set);
-    checker!(is_require_value_delimiter_set requires is_use_value_delimiter_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/error/kind.rs

@@ -105,7 +105,6 @@ pub enum ErrorKind {
     /// let result = Command::new("prog")
     ///     .arg(Arg::new("arg")
     ///         .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());

src/parser/matches/arg_matches.rs

@@ -543,7 +543,7 @@ impl ArgMatches {
     /// let m = Command::new("myapp")
     ///     .arg(Arg::new("option")
     ///         .short('o')
-    ///         .use_value_delimiter(true)
+    ///         .value_delimiter(',')
     ///         .number_of_values(1..))
     ///     .get_matches_from(vec!["myapp", "-o=val1,val2,val3"]);
     ///            // ARGV indices: ^0       ^1
@@ -585,7 +585,7 @@ impl ArgMatches {
     /// let m = Command::new("myapp")
     ///     .arg(Arg::new("option")
     ///         .short('o')
-    ///         .use_value_delimiter(true))
+    ///         .value_delimiter(','))
     ///     .get_matches_from(vec!["myapp", "-o=val1,val2,val3"]);
     ///            // ARGV indices: ^0       ^1
     ///            // clap indices:             ^2   ^3   ^4

tests/builder/app_settings.rs

@@ -395,7 +395,7 @@ fn delim_values_only_pos_follows_with_delim() {
     let r = Command::new("onlypos")
         .args(&[
             arg!(f: -f [flag] "some opt"),
-            arg!([arg] ... "some arg").use_value_delimiter(true),
+            arg!([arg] ... "some arg").value_delimiter(','),
         ])
         .try_get_matches_from(vec!["", "--", "-f", "-g,x"]);
     assert!(r.is_ok(), "{}", r.unwrap_err());
@@ -415,7 +415,7 @@ fn delim_values_only_pos_follows_with_delim() {
 fn delim_values_trailingvararg_with_delim() {
     let m = Command::new("positional")
         .trailing_var_arg(true)
-        .arg(arg!([opt] ... "some pos").use_value_delimiter(true))
+        .arg(arg!([opt] ... "some pos").value_delimiter(','))
         .try_get_matches_from(vec!["", "test", "--foo", "-Wl,-bar"])
         .unwrap();
     assert!(m.contains_id("opt"));
@@ -1150,7 +1150,7 @@ fn aaos_opts_mult_req_delims() {
         .arg(
             arg!(--opt <val> ... "some option")
                 .action(ArgAction::Set)
-                .use_value_delimiter(true)
+                .value_delimiter(',')
                 .require_value_delimiter(true)
                 .action(ArgAction::Append),
         )
@@ -1219,11 +1219,7 @@ fn aaos_pos_mult() {
 #[test]
 fn aaos_option_use_delim_false() {
     let m = Command::new("posix")
-        .arg(
-            arg!(--opt <val> "some option")
-                .use_value_delimiter(false)
-                .action(ArgAction::Set),
-        )
+        .arg(arg!(--opt <val> "some option").action(ArgAction::Set))
         .try_get_matches_from(vec!["", "--opt=some,other", "--opt=one,two"])
         .unwrap();
     assert!(m.contains_id("opt"));

tests/builder/default_vals.rs

@@ -822,7 +822,7 @@ fn valid_delimited_default_values() {
         .arg(
             Arg::new("arg")
                 .value_parser(clap::value_parser!(u32))
-                .use_value_delimiter(true)
+                .value_delimiter(',')
                 .require_value_delimiter(true)
                 .default_value("1,2,3"),
         )
@@ -839,7 +839,7 @@ fn invalid_delimited_default_values() {
         .arg(
             Arg::new("arg")
                 .value_parser(clap::value_parser!(u32))
-                .use_value_delimiter(true)
+                .value_delimiter(',')
                 .require_value_delimiter(true)
                 .default_value("one,two"),
         )

tests/builder/delimiters.rs

@@ -109,7 +109,7 @@ fn opt_eq_mult_def_delim() {
                 .long("opt")
                 .action(ArgAction::Set)
                 .number_of_values(1..)
-                .use_value_delimiter(true),
+                .value_delimiter(','),
         )
         .try_get_matches_from(vec!["", "--opt=val1,val2,val3"]);
 

tests/builder/env.rs

@@ -229,7 +229,7 @@ fn multiple_one() {
             arg!([arg] "some opt")
                 .env("CLP_TEST_ENV_MO")
                 .action(ArgAction::Set)
-                .use_value_delimiter(true)
+                .value_delimiter(',')
                 .number_of_values(1..),
         )
         .try_get_matches_from(vec![""]);
@@ -255,7 +255,7 @@ fn multiple_three() {
             arg!([arg] "some opt")
                 .env("CLP_TEST_ENV_MULTI1")
                 .action(ArgAction::Set)
-                .use_value_delimiter(true)
+                .value_delimiter(',')
                 .number_of_values(1..),
         )
         .try_get_matches_from(vec![""]);