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::rwquire_value_delimiter #4026

Merged
merged 1 commit into from Aug 4, 2022
Merged

fix!: Remove Arg::rwquire_value_delimiter #4026

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 @@ -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)
Expand Down
102 changes: 2 additions & 100 deletions src/builder/arg.rs
View file
Open in desktop
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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() {
Expand Down
3 changes: 0 additions & 3 deletions src/builder/arg_settings.rs
View file
Open in desktop
Expand Up @@ -33,7 +33,6 @@ pub(crate) enum ArgSettings {
Hidden,
TakesValue,
NextLineHelp,
RequireDelimiter,
HidePossibleValues,
AllowHyphenValues,
RequireEquals,
Expand All @@ -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;
Expand All @@ -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,
Expand Down
1 change: 0 additions & 1 deletion src/builder/debug_asserts.rs
View file
Open in desktop
Expand Up @@ -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);
Expand Down
4 changes: 1 addition & 3 deletions src/parser/arg_matcher.rs
View file
Open in desktop
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion tests/builder/app_settings.rs
View file
Open in desktop
Expand Up @@ -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"]);
Expand Down
10 changes: 4 additions & 6 deletions tests/builder/default_vals.rs
View file
Open in desktop
Expand Up @@ -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)]
Expand All @@ -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]
Expand Down
18 changes: 6 additions & 12 deletions tests/builder/help.rs
View file
Open in desktop
Expand Up @@ -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
";
Expand All @@ -1597,8 +1597,6 @@ OPTIONS:
.required(true)
.value_names(&["some", "val"])
.action(ArgAction::Set)
.value_delimiter(',')
.require_value_delimiter(true)
.value_delimiter(':'),
);

Expand All @@ -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

Expand All @@ -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"))
Expand All @@ -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
Expand Down Expand Up @@ -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"))
Expand Down
30 changes: 12 additions & 18 deletions tests/builder/multiple_values.rs
View file
Open in desktop
Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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![
Expand Down
13 changes: 2 additions & 11 deletions tests/builder/opts.rs
View file
Open in desktop
Expand Up @@ -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());
Expand All @@ -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());
Expand Down