From 6f03b4f948af4d862db3343a1fd8b48454404a2f Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 21 Jul 2022 16:36:54 -0500 Subject: [PATCH] fix!: Remove multiple occurrences in favor of Append/Count For num_vals and friends, this only implements hacks until #2688 Fixes #3021 --- clap_complete/src/shells/zsh.rs | 17 +---- clap_complete/tests/snapshots/aliases.zsh | 8 +- clap_complete/tests/snapshots/basic.zsh | 8 +- .../tests/snapshots/feature_sample.zsh | 16 ++-- clap_complete/tests/snapshots/quoting.zsh | 32 ++++---- .../tests/snapshots/special_commands.zsh | 40 +++++----- .../tests/snapshots/sub_subcommands.zsh | 32 ++++---- clap_complete/tests/snapshots/value_hint.zsh | 2 +- .../tests/snapshots/basic.fig.js | 1 + .../tests/snapshots/feature_sample.fig.js | 1 + .../tests/snapshots/quoting.fig.js | 1 + .../tests/snapshots/special_commands.fig.js | 1 + .../tests/snapshots/sub_subcommands.fig.js | 2 + src/builder/arg.rs | 73 +++---------------- src/builder/arg_settings.rs | 4 - src/builder/command.rs | 2 +- src/builder/debug_asserts.rs | 8 +- src/error/kind.rs | 6 -- src/error/mod.rs | 20 ----- src/parser/arg_matcher.rs | 4 +- src/parser/validator.rs | 36 ++------- tests/builder/multiple_values.rs | 2 +- tests/derive/custom_string_parsers.rs | 2 +- 23 files changed, 101 insertions(+), 217 deletions(-) diff --git a/clap_complete/src/shells/zsh.rs b/clap_complete/src/shells/zsh.rs index 0ff4703202b..ccc527fae23 100644 --- a/clap_complete/src/shells/zsh.rs +++ b/clap_complete/src/shells/zsh.rs @@ -451,12 +451,7 @@ fn write_opts_of(p: &Command, p_global: Option<&Command>) -> String { let help = o.get_help().map_or(String::new(), escape_help); let conflicts = arg_conflicts(p, o, p_global); - #[allow(deprecated)] - let multiple = if o.is_multiple_occurrences_set() { - "*" - } else { - "" - }; + let multiple = "*"; let vn = match o.get_value_names() { None => " ".to_string(), @@ -555,12 +550,7 @@ fn write_flags_of(p: &Command, p_global: Option<&Command>) -> String { let help = f.get_help().map_or(String::new(), escape_help); let conflicts = arg_conflicts(p, &f, p_global); - #[allow(deprecated)] - let multiple = if f.is_multiple_occurrences_set() { - "*" - } else { - "" - }; + let multiple = "*"; if let Some(short) = f.get_short() { let s = format!( @@ -634,8 +624,7 @@ fn write_positionals_of(p: &Command) -> String { for arg in p.get_positionals() { debug!("write_positionals_of:iter: arg={}", arg.get_id()); - #[allow(deprecated)] - let cardinality = if arg.is_multiple_values_set() || arg.is_multiple_occurrences_set() { + let cardinality = if arg.is_multiple_values_set() { "*:" } else if !arg.is_required_set() { ":" diff --git a/clap_complete/tests/snapshots/aliases.zsh b/clap_complete/tests/snapshots/aliases.zsh index 37984c2f1c9..2168079668a 100644 --- a/clap_complete/tests/snapshots/aliases.zsh +++ b/clap_complete/tests/snapshots/aliases.zsh @@ -19,10 +19,10 @@ _my-app() { '*-O+[cmd option]: : ' / '*--option=[cmd option]: : ' / '*--opt=[cmd option]: : ' / -'-h[Print help information]' / -'--help[Print help information]' / -'-V[Print version information]' / -'--version[Print version information]' / +'*-h[Print help information]' / +'*--help[Print help information]' / +'*-V[Print version information]' / +'*--version[Print version information]' / '*-f[cmd flag]' / '*-F[cmd flag]' / '*--flag[cmd flag]' / diff --git a/clap_complete/tests/snapshots/basic.zsh b/clap_complete/tests/snapshots/basic.zsh index 3ba7c7f43de..3274279fae8 100644 --- a/clap_complete/tests/snapshots/basic.zsh +++ b/clap_complete/tests/snapshots/basic.zsh @@ -15,8 +15,8 @@ _my-app() { local context curcontext="$curcontext" state line _arguments "${_arguments_options[@]}" / -'-h[Print help information]' / -'--help[Print help information]' / +'*-h[Print help information]' / +'*--help[Print help information]' / '*-c[]' / '(-c)*-v[]' / ":: :_my-app_commands" / @@ -31,8 +31,8 @@ _my-app() { (test) _arguments "${_arguments_options[@]}" / '*-d[]' / -'-h[Print help information]' / -'--help[Print help information]' / +'*-h[Print help information]' / +'*--help[Print help information]' / '*-c[]' / && ret=0 ;; diff --git a/clap_complete/tests/snapshots/feature_sample.zsh b/clap_complete/tests/snapshots/feature_sample.zsh index f0fd07aa7b8..d745ce162fb 100644 --- a/clap_complete/tests/snapshots/feature_sample.zsh +++ b/clap_complete/tests/snapshots/feature_sample.zsh @@ -15,10 +15,10 @@ _my-app() { local context curcontext="$curcontext" state line _arguments "${_arguments_options[@]}" / -'-h[Print help information]' / -'--help[Print help information]' / -'-V[Print version information]' / -'--version[Print version information]' / +'*-h[Print help information]' / +'*--help[Print help information]' / +'*-V[Print version information]' / +'*--version[Print version information]' / '*-c[some config file]' / '*-C[some config file]' / '*--config[some config file]' / @@ -37,10 +37,10 @@ _my-app() { (test) _arguments "${_arguments_options[@]}" / '*--case=[the case to test]: : ' / -'-h[Print help information]' / -'--help[Print help information]' / -'-V[Print version information]' / -'--version[Print version information]' / +'*-h[Print help information]' / +'*--help[Print help information]' / +'*-V[Print version information]' / +'*--version[Print version information]' / && ret=0 ;; (help) diff --git a/clap_complete/tests/snapshots/quoting.zsh b/clap_complete/tests/snapshots/quoting.zsh index fe8587e5762..9953862c85d 100644 --- a/clap_complete/tests/snapshots/quoting.zsh +++ b/clap_complete/tests/snapshots/quoting.zsh @@ -15,10 +15,10 @@ _my-app() { local context curcontext="$curcontext" state line _arguments "${_arguments_options[@]}" / -'-h[Print help information]' / -'--help[Print help information]' / -'-V[Print version information]' / -'--version[Print version information]' / +'*-h[Print help information]' / +'*--help[Print help information]' / +'*-V[Print version information]' / +'*--version[Print version information]' / '*--single-quotes[Can be '/''always'/'', '/''auto'/'', or '/''never'/'']' / '*--double-quotes[Can be "always", "auto", or "never"]' / '*--backticks[For more information see `echo test`]' / @@ -36,38 +36,38 @@ _my-app() { case $line[1] in (cmd-single-quotes) _arguments "${_arguments_options[@]}" / -'-h[Print help information]' / -'--help[Print help information]' / +'*-h[Print help information]' / +'*--help[Print help information]' / && ret=0 ;; (cmd-double-quotes) _arguments "${_arguments_options[@]}" / -'-h[Print help information]' / -'--help[Print help information]' / +'*-h[Print help information]' / +'*--help[Print help information]' / && ret=0 ;; (cmd-backticks) _arguments "${_arguments_options[@]}" / -'-h[Print help information]' / -'--help[Print help information]' / +'*-h[Print help information]' / +'*--help[Print help information]' / && ret=0 ;; (cmd-backslash) _arguments "${_arguments_options[@]}" / -'-h[Print help information]' / -'--help[Print help information]' / +'*-h[Print help information]' / +'*--help[Print help information]' / && ret=0 ;; (cmd-brackets) _arguments "${_arguments_options[@]}" / -'-h[Print help information]' / -'--help[Print help information]' / +'*-h[Print help information]' / +'*--help[Print help information]' / && ret=0 ;; (cmd-expansions) _arguments "${_arguments_options[@]}" / -'-h[Print help information]' / -'--help[Print help information]' / +'*-h[Print help information]' / +'*--help[Print help information]' / && ret=0 ;; (help) diff --git a/clap_complete/tests/snapshots/special_commands.zsh b/clap_complete/tests/snapshots/special_commands.zsh index 5a7d1911ef7..3785cd2845e 100644 --- a/clap_complete/tests/snapshots/special_commands.zsh +++ b/clap_complete/tests/snapshots/special_commands.zsh @@ -15,10 +15,10 @@ _my-app() { local context curcontext="$curcontext" state line _arguments "${_arguments_options[@]}" / -'-h[Print help information]' / -'--help[Print help information]' / -'-V[Print version information]' / -'--version[Print version information]' / +'*-h[Print help information]' / +'*--help[Print help information]' / +'*-V[Print version information]' / +'*--version[Print version information]' / '*-c[some config file]' / '*-C[some config file]' / '*--config[some config file]' / @@ -37,36 +37,36 @@ _my-app() { (test) _arguments "${_arguments_options[@]}" / '*--case=[the case to test]: : ' / -'-h[Print help information]' / -'--help[Print help information]' / -'-V[Print version information]' / -'--version[Print version information]' / +'*-h[Print help information]' / +'*--help[Print help information]' / +'*-V[Print version information]' / +'*--version[Print version information]' / && ret=0 ;; (some_cmd) _arguments "${_arguments_options[@]}" / '*--config=[the other case to test]: : ' / -'-h[Print help information]' / -'--help[Print help information]' / -'-V[Print version information]' / -'--version[Print version information]' / +'*-h[Print help information]' / +'*--help[Print help information]' / +'*-V[Print version information]' / +'*--version[Print version information]' / '*::path:' / && ret=0 ;; (some-cmd-with-hyphens) _arguments "${_arguments_options[@]}" / -'-h[Print help information]' / -'--help[Print help information]' / -'-V[Print version information]' / -'--version[Print version information]' / +'*-h[Print help information]' / +'*--help[Print help information]' / +'*-V[Print version information]' / +'*--version[Print version information]' / && ret=0 ;; (some-hidden-cmd) _arguments "${_arguments_options[@]}" / -'-h[Print help information]' / -'--help[Print help information]' / -'-V[Print version information]' / -'--version[Print version information]' / +'*-h[Print help information]' / +'*--help[Print help information]' / +'*-V[Print version information]' / +'*--version[Print version information]' / && ret=0 ;; (help) diff --git a/clap_complete/tests/snapshots/sub_subcommands.zsh b/clap_complete/tests/snapshots/sub_subcommands.zsh index ac408b602c1..78249b82ef6 100644 --- a/clap_complete/tests/snapshots/sub_subcommands.zsh +++ b/clap_complete/tests/snapshots/sub_subcommands.zsh @@ -15,10 +15,10 @@ _my-app() { local context curcontext="$curcontext" state line _arguments "${_arguments_options[@]}" / -'-h[Print help information]' / -'--help[Print help information]' / -'-V[Print version information]' / -'--version[Print version information]' / +'*-h[Print help information]' / +'*--help[Print help information]' / +'*-V[Print version information]' / +'*--version[Print version information]' / '*-c[some config file]' / '*-C[some config file]' / '*--config[some config file]' / @@ -37,18 +37,18 @@ _my-app() { (test) _arguments "${_arguments_options[@]}" / '*--case=[the case to test]: : ' / -'-h[Print help information]' / -'--help[Print help information]' / -'-V[Print version information]' / -'--version[Print version information]' / +'*-h[Print help information]' / +'*--help[Print help information]' / +'*-V[Print version information]' / +'*--version[Print version information]' / && ret=0 ;; (some_cmd) _arguments "${_arguments_options[@]}" / -'-h[Print help information]' / -'--help[Print help information]' / -'-V[Print version information]' / -'--version[Print version information]' / +'*-h[Print help information]' / +'*--help[Print help information]' / +'*-V[Print version information]' / +'*--version[Print version information]' / ":: :_my-app__some_cmd_commands" / "*::: :->some_cmd" / && ret=0 @@ -62,10 +62,10 @@ _arguments "${_arguments_options[@]}" / (sub_cmd) _arguments "${_arguments_options[@]}" / '*--config=[the other case to test]: :(Lest quotes aren't escaped.)' / -'-h[Print help information]' / -'--help[Print help information]' / -'-V[Print version information]' / -'--version[Print version information]' / +'*-h[Print help information]' / +'*--help[Print help information]' / +'*-V[Print version information]' / +'*--version[Print version information]' / && ret=0 ;; (help) diff --git a/clap_complete/tests/snapshots/value_hint.zsh b/clap_complete/tests/snapshots/value_hint.zsh index a2cc5ee8d34..068ff008aa7 100644 --- a/clap_complete/tests/snapshots/value_hint.zsh +++ b/clap_complete/tests/snapshots/value_hint.zsh @@ -35,7 +35,7 @@ _my-app() { '*--host=[]: :_hosts' / '*--url=[]: :_urls' / '*--email=[]: :_email_addresses' / -'--help[Print help information]' / +'*--help[Print help information]' / '*::command_with_args:_cmdambivalent' / && ret=0 } diff --git a/clap_complete_fig/tests/snapshots/basic.fig.js b/clap_complete_fig/tests/snapshots/basic.fig.js index b951ee3dfe2..8502480d37e 100644 --- a/clap_complete_fig/tests/snapshots/basic.fig.js +++ b/clap_complete_fig/tests/snapshots/basic.fig.js @@ -29,6 +29,7 @@ const completion: Fig.Spec = { ], args: { name: "subcommand", + isVariadic: true, isOptional: true, }, }, diff --git a/clap_complete_fig/tests/snapshots/feature_sample.fig.js b/clap_complete_fig/tests/snapshots/feature_sample.fig.js index 69d343d156b..07e48b5cb6e 100644 --- a/clap_complete_fig/tests/snapshots/feature_sample.fig.js +++ b/clap_complete_fig/tests/snapshots/feature_sample.fig.js @@ -30,6 +30,7 @@ const completion: Fig.Spec = { description: "Print this message or the help of the given subcommand(s)", args: { name: "subcommand", + isVariadic: true, isOptional: true, }, }, diff --git a/clap_complete_fig/tests/snapshots/quoting.fig.js b/clap_complete_fig/tests/snapshots/quoting.fig.js index 2063430b334..c878edef3cf 100644 --- a/clap_complete_fig/tests/snapshots/quoting.fig.js +++ b/clap_complete_fig/tests/snapshots/quoting.fig.js @@ -67,6 +67,7 @@ const completion: Fig.Spec = { description: "Print this message or the help of the given subcommand(s)", args: { name: "subcommand", + isVariadic: true, isOptional: true, }, }, diff --git a/clap_complete_fig/tests/snapshots/special_commands.fig.js b/clap_complete_fig/tests/snapshots/special_commands.fig.js index de9dcf6fc82..632e2bf690b 100644 --- a/clap_complete_fig/tests/snapshots/special_commands.fig.js +++ b/clap_complete_fig/tests/snapshots/special_commands.fig.js @@ -87,6 +87,7 @@ const completion: Fig.Spec = { description: "Print this message or the help of the given subcommand(s)", args: { name: "subcommand", + isVariadic: true, isOptional: true, }, }, diff --git a/clap_complete_fig/tests/snapshots/sub_subcommands.fig.js b/clap_complete_fig/tests/snapshots/sub_subcommands.fig.js index c2dced153f4..8d17a992628 100644 --- a/clap_complete_fig/tests/snapshots/sub_subcommands.fig.js +++ b/clap_complete_fig/tests/snapshots/sub_subcommands.fig.js @@ -60,6 +60,7 @@ const completion: Fig.Spec = { description: "Print this message or the help of the given subcommand(s)", args: { name: "subcommand", + isVariadic: true, isOptional: true, }, }, @@ -80,6 +81,7 @@ const completion: Fig.Spec = { description: "Print this message or the help of the given subcommand(s)", args: { name: "subcommand", + isVariadic: true, isOptional: true, }, }, diff --git a/src/builder/arg.rs b/src/builder/arg.rs index beb2815d564..f90fcd3a5ec 100644 --- a/src/builder/arg.rs +++ b/src/builder/arg.rs @@ -752,21 +752,6 @@ impl<'help> Arg<'help> { } } - /// Deprecated, replaced with [`Arg::action`] ([Issue #3772](https://github.com/clap-rs/clap/issues/3772)) - #[inline] - #[must_use] - #[cfg_attr( - feature = "deprecated", - deprecated(since = "3.2.0", note = "Replaced with `Arg::action` (Issue #3772)") - )] - pub fn multiple_occurrences(self, yes: bool) -> Self { - if yes { - self.setting(ArgSettings::MultipleOccurrences) - } else { - self.unset_setting(ArgSettings::MultipleOccurrences) - } - } - #[inline] pub(crate) fn is_set(&self, s: ArgSettings) -> bool { self.settings.is_set(s) @@ -1100,10 +1085,6 @@ impl<'help> Arg<'help> { /// `.number_of_values(3)`, and this argument wouldn't be satisfied unless the user provided /// 3 and only 3 values. /// - /// **NOTE:** Does *not* require [`Arg::multiple_occurrences(true)`] to be set. Setting - /// [`Arg::multiple_occurrences(true)`] would allow `-f -f ` where - /// as *not* setting it would only allow one occurrence of this argument. - /// /// **NOTE:** implicitly sets [`Arg::takes_value(true)`] and [`Arg::multiple_values(true)`]. /// /// # Examples @@ -1131,7 +1112,6 @@ impl<'help> Arg<'help> { /// assert!(res.is_err()); /// assert_eq!(res.unwrap_err().kind(), ErrorKind::WrongNumberOfValues); /// ``` - /// [`Arg::multiple_occurrences(true)`]: Arg::multiple_occurrences() #[inline] #[must_use] pub fn number_of_values(mut self, qty: usize) -> Self { @@ -1145,12 +1125,6 @@ impl<'help> Arg<'help> { /// `-f ` 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. /// - /// **NOTE:** This does *not* implicitly set [`Arg::multiple_occurrences(true)`]. This is because - /// `-o val -o val` is multiple occurrences but a single value and `-o val1 val2` is a single - /// occurrence with multiple values. For positional arguments this **does** set - /// [`Arg::multiple_occurrences(true)`] because there is no way to determine the difference between multiple - /// occurrences and multiple values. - /// /// # Examples /// /// ```rust @@ -1195,7 +1169,6 @@ impl<'help> Arg<'help> { /// assert!(res.is_err()); /// assert_eq!(res.unwrap_err().kind(), ErrorKind::UnknownArgument); /// ``` - /// [`Arg::multiple_occurrences(true)`]: Arg::multiple_occurrences() #[inline] #[must_use] pub fn max_values(mut self, qty: usize) -> Self { @@ -1210,12 +1183,6 @@ impl<'help> Arg<'help> { /// `.min_values(2)`, and this argument would be satisfied if the user provided, 2 or more /// values. /// - /// **NOTE:** This does not implicitly set [`Arg::multiple_occurrences(true)`]. This is because - /// `-o val -o val` is multiple occurrences but a single value and `-o val1 val2` is a single - /// occurrence with multiple values. For positional arguments this **does** set - /// [`Arg::multiple_occurrences(true)`] because there is no way to determine the difference between multiple - /// occurrences and multiple 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 @@ -1265,7 +1232,6 @@ impl<'help> Arg<'help> { /// assert!(res.is_err()); /// assert_eq!(res.unwrap_err().kind(), ErrorKind::TooFewValues); /// ``` - /// [`Arg::multiple_occurrences(true)`]: Arg::multiple_occurrences() /// [`Arg::required(true)`]: Arg::required() #[inline] #[must_use] @@ -1485,8 +1451,7 @@ impl<'help> Arg<'help> { /// **WARNING**: Take caution when using this setting combined with /// [`Arg::multiple_values`], as this becomes ambiguous `$ prog --arg -- -- val`. All /// three `--, --, val` will be values when the user may have thought the second `--` would - /// constitute the normal, "Only positional args follow" idiom. To fix this, consider using - /// [`Arg::multiple_occurrences`] which only allows a single value at a time. + /// constitute the normal, "Only positional args follow" idiom. /// /// **WARNING**: When building your CLIs, consider the effects of allowing leading hyphens and /// the user passing in a value that matches a valid short. For example, `prog -opt -F` where @@ -4164,15 +4129,6 @@ impl<'help> Arg<'help> { self.is_set(ArgSettings::MultipleValues) } - /// [`Arg::multiple_occurrences`] is going away ([Issue #3772](https://github.com/clap-rs/clap/issues/3772)) - #[cfg_attr( - feature = "deprecated", - deprecated(since = "3.2.0", note = "`multiple_occurrences` away (Issue #3772)") - )] - pub fn is_multiple_occurrences_set(&self) -> bool { - self.is_set(ArgSettings::MultipleOccurrences) - } - /// Report whether [`Arg::is_takes_value_set`] is set pub fn is_takes_value_set(&self) -> bool { self.is_set(ArgSettings::TakesValue) @@ -4328,18 +4284,6 @@ impl<'help> Arg<'help> { } else { self.settings.unset(ArgSettings::TakesValue); } - match action { - ArgAction::Help | ArgAction::Version => {} - ArgAction::Set - | ArgAction::Append - | ArgAction::SetTrue - | ArgAction::SetFalse - | ArgAction::Count => { - if !self.is_positional() { - self.settings.set(ArgSettings::MultipleOccurrences); - } - } - } } if self.value_parser.is_none() { @@ -4379,7 +4323,9 @@ impl<'help> Arg<'help> { // Used for positionals when printing pub(crate) fn multiple_str(&self) -> &str { let mult_vals = self.val_names.len() > 1; - if (self.is_multiple_values_set() || self.is_multiple_occurrences_set()) && !mult_vals { + if (self.is_multiple_values_set() || matches!(*self.get_action(), ArgAction::Append)) + && !mult_vals + { "..." } else { "" @@ -4417,7 +4363,7 @@ impl<'help> Arg<'help> { /// Either multiple values or occurrences pub(crate) fn is_multiple(&self) -> bool { - self.is_multiple_values_set() | self.is_multiple_occurrences_set() + self.is_multiple_values_set() || matches!(*self.get_action(), ArgAction::Append) } pub(crate) fn get_display_order(&self) -> usize { @@ -4554,7 +4500,7 @@ where F: FnMut(&str, bool) -> Result, { let mult_val = arg.is_multiple_values_set(); - let mult_occ = arg.is_multiple_occurrences_set(); + let mult_occ = matches!(*arg.get_action(), ArgAction::Append); let delim = if arg.is_require_value_delimiter_set() { arg.val_delim.expect(INTERNAL_ERROR_MSG) } else { @@ -4622,10 +4568,11 @@ where #[cfg(test)] mod test { use super::Arg; + use super::ArgAction; #[test] fn flag_display() { - let mut f = Arg::new("flg").multiple_occurrences(true); + let mut f = Arg::new("flg").action(ArgAction::Append); f.long = Some("flag"); assert_eq!(f.to_string(), "--flag"); @@ -4682,7 +4629,7 @@ mod test { let o = Arg::new("opt") .long("option") .takes_value(true) - .multiple_occurrences(true); + .action(ArgAction::Append); assert_eq!(o.to_string(), "--option "); } @@ -4774,7 +4721,7 @@ mod test { let p = Arg::new("pos") .index(1) .takes_value(true) - .multiple_occurrences(true); + .action(ArgAction::Append); assert_eq!(p.to_string(), "..."); } diff --git a/src/builder/arg_settings.rs b/src/builder/arg_settings.rs index e532de96292..87dccd2d845 100644 --- a/src/builder/arg_settings.rs +++ b/src/builder/arg_settings.rs @@ -31,7 +31,6 @@ impl Default for ArgFlags { pub(crate) enum ArgSettings { Required, MultipleValues, - MultipleOccurrences, Global, Hidden, TakesValue, @@ -56,7 +55,6 @@ pub(crate) enum ArgSettings { bitflags! { struct Flags: u32 { const REQUIRED = 1; - const MULTIPLE_OCC = 1 << 1; const GLOBAL = 1 << 3; const HIDDEN = 1 << 4; const TAKES_VAL = 1 << 5; @@ -75,7 +73,6 @@ bitflags! { const HIDDEN_SHORT_H = 1 << 18; const HIDDEN_LONG_H = 1 << 19; const MULTIPLE_VALS = 1 << 20; - const MULTIPLE = Self::MULTIPLE_OCC.bits | Self::MULTIPLE_VALS.bits; #[cfg(feature = "env")] const HIDE_ENV = 1 << 21; const EXCLUSIVE = 1 << 23; @@ -85,7 +82,6 @@ bitflags! { impl_settings! { ArgSettings, ArgFlags, Required => Flags::REQUIRED, - MultipleOccurrences => Flags::MULTIPLE_OCC, MultipleValues => Flags::MULTIPLE_VALS, Global => Flags::GLOBAL, Hidden => Flags::HIDDEN, diff --git a/src/builder/command.rs b/src/builder/command.rs index 7f14c716b48..707f9f7e106 100644 --- a/src/builder/command.rs +++ b/src/builder/command.rs @@ -4239,7 +4239,7 @@ To change `help`s short, call `cmd.arg(Arg::new(\"help\")...)`.", Arg::new("subcommand") .index(1) .takes_value(true) - .multiple_occurrences(true) + .multiple_values(true) .value_name("SUBCOMMAND") .help("The subcommand whose help message to display"), ); diff --git a/src/builder/debug_asserts.rs b/src/builder/debug_asserts.rs index 0e8a8212eeb..7c2aeb39037 100644 --- a/src/builder/debug_asserts.rs +++ b/src/builder/debug_asserts.rs @@ -515,13 +515,7 @@ fn _verify_positionals(cmd: &Command) -> bool { // Next we check how many have both Multiple and not a specific number of values set let count = cmd .get_positionals() - .filter(|p| { - #[allow(deprecated)] - { - p.is_multiple_occurrences_set() - || (p.is_multiple_values_set() && p.num_vals.is_none()) - } - }) + .filter(|p| p.is_multiple_values_set() && p.num_vals.is_none()) .count(); let ok = count <= 1 || (last.is_last_set() diff --git a/src/error/kind.rs b/src/error/kind.rs index a29e7d871f4..da74bc95b10 100644 --- a/src/error/kind.rs +++ b/src/error/kind.rs @@ -207,9 +207,6 @@ pub enum ErrorKind { /// [`Command::subcommand_required`]: crate::Command::subcommand_required MissingSubcommand, - /// Occurs when the user provides multiple values to an argument which doesn't allow that. - UnexpectedMultipleUsage, - /// Occurs when the user provides a value containing invalid UTF-8. /// /// To allow arbitrary data @@ -333,9 +330,6 @@ impl ErrorKind { Some("One or more required arguments were not provided") } Self::MissingSubcommand => Some("A subcommand is required but one was not provided"), - Self::UnexpectedMultipleUsage => { - Some("An argument was provided more than once but cannot be used multiple times") - } Self::InvalidUtf8 => Some("Invalid UTF-8 was detected in one or more arguments"), Self::DisplayHelp => None, Self::DisplayHelpOnMissingArgumentOrSubcommand => None, diff --git a/src/error/mod.rs b/src/error/mod.rs index 3104d98c229..648b894e1aa 100644 --- a/src/error/mod.rs +++ b/src/error/mod.rs @@ -412,15 +412,6 @@ impl Error { ]) } - pub(crate) fn unexpected_multiple_usage(cmd: &Command, arg: String, usage: String) -> Self { - Self::new(ErrorKind::UnexpectedMultipleUsage) - .with_cmd(cmd) - .extend_context_unchecked([ - (ContextKind::InvalidArg, ContextValue::String(arg)), - (ContextKind::Usage, ContextValue::String(usage)), - ]) - } - pub(crate) fn unknown_argument( cmd: &Command, arg: String, @@ -715,17 +706,6 @@ impl Error { false } } - ErrorKind::UnexpectedMultipleUsage => { - let invalid_arg = self.get_context(ContextKind::InvalidArg); - if let Some(ContextValue::String(invalid_arg)) = invalid_arg { - c.none("The argument '"); - c.warning(invalid_arg.to_string()); - c.none("' was provided more than once, but cannot be used multiple times"); - true - } else { - false - } - } ErrorKind::UnknownArgument => { let invalid_arg = self.get_context(ContextKind::InvalidArg); if let Some(ContextValue::String(invalid_arg)) = invalid_arg { diff --git a/src/parser/arg_matcher.rs b/src/parser/arg_matcher.rs index 08a3815327e..e09f6112856 100644 --- a/src/parser/arg_matcher.rs +++ b/src/parser/arg_matcher.rs @@ -11,6 +11,7 @@ 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)] @@ -218,8 +219,7 @@ impl ArgMatcher { true } else if let Some(num) = o.num_vals { debug!("ArgMatcher::needs_more_vals: num_vals...{}", num); - #[allow(deprecated)] - if o.is_multiple_occurrences_set() { + if matches!(o.get_action(), ArgAction::Append) && !o.is_positional() { (current_num % num) != 0 } else { num != current_num diff --git a/src/parser/validator.rs b/src/parser/validator.rs index 069152b47d6..06c67c21e93 100644 --- a/src/parser/validator.rs +++ b/src/parser/validator.rs @@ -6,6 +6,7 @@ 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> { @@ -241,42 +242,20 @@ impl<'help, 'cmd> Validator<'help, 'cmd> { ); if let Some(arg) = self.cmd.find(name) { self.validate_arg_num_vals(arg, ma)?; - self.validate_arg_num_occurs(arg, ma)?; } Ok(()) }) } - fn validate_arg_num_occurs(&self, a: &Arg, ma: &MatchedArg) -> ClapResult<()> { - #![allow(deprecated)] - debug!( - "Validator::validate_arg_num_occurs: {:?}={}", - a.name, - ma.get_occurrences() - ); - // Occurrence of positional argument equals to number of values rather - // than number of grouped values. - if ma.get_occurrences() > 1 && !a.is_multiple_occurrences_set() && !a.is_positional() { - // Not the first time, and we don't allow multiples - return Err(Error::unexpected_multiple_usage( - self.cmd, - a.to_string(), - Usage::new(self.cmd) - .required(&self.required) - .create_usage_with_title(&[]), - )); - } - - Ok(()) - } - 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(); - debug!("Validator::validate_arg_num_vals: num_vals set...{}", num); - #[allow(deprecated)] - let should_err = if a.is_multiple_occurrences_set() { + debug!( + "Validator::validate_arg_num_vals: num_vals={}, actual={}", + num, total_num + ); + let should_err = if matches!(a.get_action(), ArgAction::Append) && !a.is_positional() { total_num % num != 0 } else { num != total_num @@ -287,8 +266,7 @@ impl<'help, 'cmd> Validator<'help, 'cmd> { self.cmd, a.to_string(), num, - #[allow(deprecated)] - if a.is_multiple_occurrences_set() { + if matches!(a.get_action(), ArgAction::Append) && !a.is_positional() { total_num % num } else { total_num diff --git a/tests/builder/multiple_values.rs b/tests/builder/multiple_values.rs index 2715c5a032e..7e17e2f5891 100644 --- a/tests/builder/multiple_values.rs +++ b/tests/builder/multiple_values.rs @@ -1403,7 +1403,7 @@ fn issue_2229() { "myprog", "val1", "val2", "val3", "val4", "val5", "val6", ]); - assert!(m.is_err()); // This panics, because `m.is_err() == false`. + assert!(m.is_err()); assert_eq!(m.unwrap_err().kind(), ErrorKind::WrongNumberOfValues); } diff --git a/tests/derive/custom_string_parsers.rs b/tests/derive/custom_string_parsers.rs index 6f5d54354ea..4ef5f81659a 100644 --- a/tests/derive/custom_string_parsers.rs +++ b/tests/derive/custom_string_parsers.rs @@ -27,7 +27,7 @@ struct PathOpt { #[clap(short, default_value = "../")] default_path: PathBuf, - #[clap(short, multiple_occurrences(true))] + #[clap(short)] vector_path: Vec, #[clap(short)]