Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 1 commit into from Aug 3, 2022
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions examples/multicall-busybox.rs
View file
Open in desktop
Expand Up @@ -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()),
)
Expand Down
109 changes: 13 additions & 96 deletions src/builder/arg.rs
View file
Open in desktop
Expand Up @@ -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
Expand All @@ -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]
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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.
///
Expand All @@ -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![
Expand All @@ -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",
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand Down
3 changes: 0 additions & 3 deletions src/builder/arg_settings.rs
View file
Open in desktop
Expand Up @@ -32,7 +32,6 @@ pub(crate) enum ArgSettings {
Global,
Hidden,
TakesValue,
UseValueDelimiter,
NextLineHelp,
RequireDelimiter,
HidePossibleValues,
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions src/builder/command.rs
View file
Open in desktop
Expand Up @@ -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.
Expand All @@ -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 {
Expand Down
1 change: 0 additions & 1 deletion src/builder/debug_asserts.rs
View file
Open in desktop
Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion src/error/kind.rs
View file
Open in desktop
Expand Up @@ -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());
Expand Down
4 changes: 2 additions & 2 deletions src/parser/matches/arg_matches.rs
View file
Open in desktop
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
12 changes: 4 additions & 8 deletions tests/builder/app_settings.rs
View file
Open in desktop
Expand Up @@ -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());
Expand All @@ -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"));
Expand Down Expand Up @@ -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),
)
Expand Down Expand Up @@ -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"));
Expand Down
4 changes: 2 additions & 2 deletions tests/builder/default_vals.rs
View file
Open in desktop
Expand Up @@ -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"),
)
Expand All @@ -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"),
)
Expand Down
2 changes: 1 addition & 1 deletion tests/builder/delimiters.rs
View file
Open in desktop
Expand Up @@ -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"]);

Expand Down
4 changes: 2 additions & 2 deletions tests/builder/env.rs
View file
Open in desktop
Expand Up @@ -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![""]);
Expand All @@ -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![""]);
Expand Down