From 3ca27f08794fc70a21d29bd86ae411268eeaa022 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 22 Jul 2022 19:42:49 -0500 Subject: [PATCH 1/6] refactor: Remove dead code for NoAutoHelp/NoAutoVersion --- src/builder/app_settings.rs | 8 -------- src/builder/command.rs | 8 ++------ src/builder/mod.rs | 1 - src/parser/parser.rs | 14 +------------- 4 files changed, 3 insertions(+), 28 deletions(-) diff --git a/src/builder/app_settings.rs b/src/builder/app_settings.rs index 3d86b9efce8..1aac4769255 100644 --- a/src/builder/app_settings.rs +++ b/src/builder/app_settings.rs @@ -57,8 +57,6 @@ pub(crate) enum AppSettings { HidePossibleValues, HelpExpected, NoBinaryName, - NoAutoHelp, - NoAutoVersion, #[allow(dead_code)] ColorAuto, ColorAlways, @@ -75,8 +73,6 @@ bitflags! { const PROPAGATE_VERSION = 1 << 3; const DISABLE_VERSION_FOR_SC = 1 << 4; const WAIT_ON_ERROR = 1 << 6; - const NO_AUTO_HELP = 1 << 8; - const NO_AUTO_VERSION = 1 << 9; const DISABLE_VERSION_FLAG = 1 << 10; const HIDDEN = 1 << 11; const TRAILING_VARARG = 1 << 12; @@ -158,10 +154,6 @@ impl_settings! { AppSettings, AppFlags, => Flags::HIDDEN, Multicall => Flags::MULTICALL, - NoAutoHelp - => Flags::NO_AUTO_HELP, - NoAutoVersion - => Flags::NO_AUTO_VERSION, NoBinaryName => Flags::NO_BIN_NAME, SubcommandsNegateReqs diff --git a/src/builder/command.rs b/src/builder/command.rs index 6adfd44ad22..98a868aed92 100644 --- a/src/builder/command.rs +++ b/src/builder/command.rs @@ -3789,10 +3789,6 @@ impl<'help> Command<'help> { let mut pos_counter = 1; let hide_pv = self.is_set(AppSettings::HidePossibleValues); - let auto_help = - !self.is_set(AppSettings::NoAutoHelp) && !self.is_disable_help_flag_set(); - let auto_version = - !self.is_set(AppSettings::NoAutoVersion) && !self.is_disable_version_flag_set(); for a in self.args.args_mut() { // Fill in the groups for g in &a.groups { @@ -3819,10 +3815,10 @@ impl<'help> Command<'help> { // required. Otherwise, most of this won't be needed because when we can break // compat, actions will reign supreme (default to `Store`) if a.action.is_none() { - if a.get_id() == "help" && auto_help && !a.is_takes_value_set() { + if a.get_id() == "help" && !a.is_takes_value_set() { let action = super::ArgAction::Help; a.action = Some(action); - } else if a.get_id() == "version" && auto_version && !a.is_takes_value_set() { + } else if a.get_id() == "version" && !a.is_takes_value_set() { let action = super::ArgAction::Version; a.action = Some(action); } else if a.is_takes_value_set() { diff --git a/src/builder/mod.rs b/src/builder/mod.rs index bb0d14751da..09b92e60817 100644 --- a/src/builder/mod.rs +++ b/src/builder/mod.rs @@ -42,7 +42,6 @@ pub use value_parser::OsStringValueParser; pub use value_parser::PathBufValueParser; pub(crate) use action::CountType; -pub(crate) use app_settings::AppSettings; pub(crate) use arg::display_arg_val; pub(crate) use arg_predicate::ArgPredicate; pub(crate) use arg_settings::{ArgFlags, ArgSettings}; diff --git a/src/parser/parser.rs b/src/parser/parser.rs index 3529e848f9b..a4dbd6de168 100644 --- a/src/parser/parser.rs +++ b/src/parser/parser.rs @@ -8,7 +8,6 @@ use std::{ use clap_lex::RawOsStr; // Internal -use crate::builder::AppSettings as AS; use crate::builder::{Arg, Command}; use crate::error::Error as ClapError; use crate::error::Result as ClapResult; @@ -107,11 +106,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { let sc_name = self.possible_subcommand(arg_os.to_value(), valid_arg_found); debug!("Parser::get_matches_with: sc={:?}", sc_name); if let Some(sc_name) = sc_name { - #[allow(deprecated)] - if sc_name == "help" - && !self.is_set(AS::NoAutoHelp) - && !self.cmd.is_disable_help_subcommand_set() - { + if sc_name == "help" && !self.cmd.is_disable_help_subcommand_set() { self.parse_help_subcommand(raw_args.remaining(&mut args_cursor))?; unreachable!("`parse_help_subcommand` always errors"); } else { @@ -1665,13 +1660,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { } } -// Query Methods -impl<'help, 'cmd> Parser<'help, 'cmd> { - pub(crate) fn is_set(&self, s: AS) -> bool { - self.cmd.is_set(s) - } -} - #[derive(Debug, PartialEq, Eq)] pub(crate) enum ParseState { ValuesDone, From 3356e243603e2fa2f041b45c00be0dca4de9d0ac Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 22 Jul 2022 19:46:07 -0500 Subject: [PATCH 2/6] refactor: Move action defaulting to where it belongs --- src/builder/arg.rs | 15 +++++++++++++++ src/builder/command.rs | 18 ------------------ 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/builder/arg.rs b/src/builder/arg.rs index 32de7eae00a..b1d537dddba 100644 --- a/src/builder/arg.rs +++ b/src/builder/arg.rs @@ -4361,6 +4361,21 @@ impl<'help> Arg<'help> { if self.is_positional() { self.settings.set(ArgSettings::TakesValue); } + if self.action.is_none() { + if self.get_id() == "help" && !self.is_takes_value_set() { + let action = super::ArgAction::Help; + self.action = Some(action); + } else if self.get_id() == "version" && !self.is_takes_value_set() { + let action = super::ArgAction::Version; + self.action = Some(action); + } else if self.is_takes_value_set() { + let action = super::ArgAction::StoreValue; + self.action = Some(action); + } else { + let action = super::ArgAction::IncOccurrence; + self.action = Some(action); + } + } if let Some(action) = self.action.as_ref() { if let Some(default_value) = action.default_value() { if self.default_vals.is_empty() { diff --git a/src/builder/command.rs b/src/builder/command.rs index 98a868aed92..ac5b5469edf 100644 --- a/src/builder/command.rs +++ b/src/builder/command.rs @@ -3811,24 +3811,6 @@ impl<'help> Command<'help> { a.settings.set(ArgSettings::HidePossibleValues); } a._build(); - // HACK: Setting up action at this level while auto-help / disable help flag is - // required. Otherwise, most of this won't be needed because when we can break - // compat, actions will reign supreme (default to `Store`) - if a.action.is_none() { - if a.get_id() == "help" && !a.is_takes_value_set() { - let action = super::ArgAction::Help; - a.action = Some(action); - } else if a.get_id() == "version" && !a.is_takes_value_set() { - let action = super::ArgAction::Version; - a.action = Some(action); - } else if a.is_takes_value_set() { - let action = super::ArgAction::StoreValue; - a.action = Some(action); - } else { - let action = super::ArgAction::IncOccurrence; - a.action = Some(action); - } - } if a.is_positional() && a.index.is_none() { a.index = Some(pos_counter); pos_counter += 1; From 0039ef91fa1a132ebe5ccb15e078c8795ee686b1 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 22 Jul 2022 16:30:40 -0500 Subject: [PATCH 3/6] fix: Have arg! collection across flags for positionals --- src/macros.rs | 2 ++ tests/macros.rs | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/macros.rs b/src/macros.rs index a73d83e5e52..18c78a18447 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -393,6 +393,8 @@ macro_rules! arg_impl { ({ if $arg.get_long().is_none() && $arg.get_short().is_none() { $arg.multiple_values(true) + // Allow collecting arguments interleaved with flags + .action($crate::ArgAction::Append) } else if $arg.is_takes_value_set() { $arg.action($crate::ArgAction::Append) } else { diff --git a/tests/macros.rs b/tests/macros.rs index 119721b5cff..84b620a8532 100644 --- a/tests/macros.rs +++ b/tests/macros.rs @@ -184,7 +184,7 @@ mod arg { let arg = clap::arg!( ...); assert_eq!(arg.get_id(), "NUM"); assert_eq!(arg.get_value_names(), Some(vec!["NUM"].as_slice())); - assert!(matches!(arg.get_action(), clap::ArgAction::Set)); + assert!(matches!(arg.get_action(), clap::ArgAction::Append)); assert!(arg.is_multiple_values_set()); assert!(arg.is_required_set()); assert_eq!(arg.get_help(), None); From 4b499ac024d5bc0ac0791bfef410f4f688effbe8 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 22 Jul 2022 16:56:04 -0500 Subject: [PATCH 4/6] docs(env): Fix bool example --- src/builder/arg.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/builder/arg.rs b/src/builder/arg.rs index b1d537dddba..de9cbcfffab 100644 --- a/src/builder/arg.rs +++ b/src/builder/arg.rs @@ -2153,7 +2153,7 @@ impl<'help> Arg<'help> { /// "prog" /// ]); /// - /// assert_eq!(m.get_one::("true_flag"), None); + /// assert!(m.is_present("true_flag")); /// assert!(!m.is_present("false_flag")); /// assert!(!m.is_present("absent_flag")); /// ``` From 122b562e6bc40544ad7cb38d9500399f17926736 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 22 Jul 2022 13:37:18 -0500 Subject: [PATCH 5/6] fix!: Change default actions to Set/SetTrue This is in prep for removing StoreValue/IncOccurrences --- CHANGELOG.md | 1 + clap_complete/tests/snapshots/aliases.zsh | 16 +++---- clap_complete/tests/snapshots/basic.zsh | 8 ++-- .../tests/snapshots/feature_sample.zsh | 2 +- clap_complete/tests/snapshots/quoting.zsh | 12 ++--- .../tests/snapshots/special_commands.zsh | 4 +- .../tests/snapshots/sub_subcommands.zsh | 4 +- clap_complete/tests/snapshots/value_hint.zsh | 40 ++++++++-------- .../tests/snapshots/aliases.fig.js | 1 + .../tests/snapshots/feature_sample.fig.js | 1 + .../tests/snapshots/special_commands.fig.js | 2 + .../tests/snapshots/sub_subcommands.fig.js | 2 + .../tests/snapshots/value_hint.fig.js | 13 +++++ clap_mangen/tests/snapshots/aliases.bash.roff | 2 +- clap_mangen/tests/snapshots/basic.bash.roff | 4 +- .../tests/snapshots/feature_sample.bash.roff | 2 +- clap_mangen/tests/snapshots/quoting.bash.roff | 12 ++--- .../snapshots/special_commands.bash.roff | 2 +- .../tests/snapshots/sub_subcommands.bash.roff | 2 +- src/builder/arg.rs | 48 ++++++++++++------- src/error/kind.rs | 13 ----- tests/builder/grouped_values.rs | 4 +- tests/builder/opts.rs | 4 -- 23 files changed, 107 insertions(+), 92 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a3320b79366..72bd3ab9d43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - `ErrorKind::EmptyValue` replaced with `ErrorKind::InvalidValue` - `ErrorKind::UnrecognizedSubcommand` replaced with `ErrorKind::InvalidSubcommand` (#3676) - `arg!` now sets `ArgAction::SetTrue`, `ArgAction::Count`, `ArgAction::Set`, or `ArgAction::Append` as appropriate (#3795) +- Default actions are now `Set` and `SetTrue` - *(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) - *(env)* Parse `--help` and `--version` like any `ArgAction::SetTrue` flag (#3776) diff --git a/clap_complete/tests/snapshots/aliases.zsh b/clap_complete/tests/snapshots/aliases.zsh index d98875ed074..37984c2f1c9 100644 --- a/clap_complete/tests/snapshots/aliases.zsh +++ b/clap_complete/tests/snapshots/aliases.zsh @@ -15,18 +15,18 @@ _my-app() { local context curcontext="$curcontext" state line _arguments "${_arguments_options[@]}" / -'-o+[cmd option]: : ' / -'-O+[cmd option]: : ' / -'--option=[cmd option]: : ' / -'--opt=[cmd option]: : ' / +'*-o+[cmd option]: : ' / +'*-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]' / -'-f[cmd flag]' / -'-F[cmd flag]' / -'--flag[cmd flag]' / -'--flg[cmd flag]' / +'*-f[cmd flag]' / +'*-F[cmd flag]' / +'*--flag[cmd flag]' / +'*--flg[cmd flag]' / '::positional:' / && ret=0 } diff --git a/clap_complete/tests/snapshots/basic.zsh b/clap_complete/tests/snapshots/basic.zsh index cd50968d461..3ba7c7f43de 100644 --- a/clap_complete/tests/snapshots/basic.zsh +++ b/clap_complete/tests/snapshots/basic.zsh @@ -17,8 +17,8 @@ _my-app() { _arguments "${_arguments_options[@]}" / '-h[Print help information]' / '--help[Print help information]' / -'-c[]' / -'(-c)-v[]' / +'*-c[]' / +'(-c)*-v[]' / ":: :_my-app_commands" / "*::: :->my-app" / && ret=0 @@ -33,12 +33,12 @@ _arguments "${_arguments_options[@]}" / '*-d[]' / '-h[Print help information]' / '--help[Print help information]' / -'-c[]' / +'*-c[]' / && ret=0 ;; (help) _arguments "${_arguments_options[@]}" / -'-c[]' / +'*-c[]' / '*::subcommand -- The subcommand whose help message to display:' / && ret=0 ;; diff --git a/clap_complete/tests/snapshots/feature_sample.zsh b/clap_complete/tests/snapshots/feature_sample.zsh index 96b44ef461d..f0fd07aa7b8 100644 --- a/clap_complete/tests/snapshots/feature_sample.zsh +++ b/clap_complete/tests/snapshots/feature_sample.zsh @@ -36,7 +36,7 @@ _my-app() { case $line[3] in (test) _arguments "${_arguments_options[@]}" / -'--case=[the case to test]: : ' / +'*--case=[the case to test]: : ' / '-h[Print help information]' / '--help[Print help information]' / '-V[Print version information]' / diff --git a/clap_complete/tests/snapshots/quoting.zsh b/clap_complete/tests/snapshots/quoting.zsh index b31471ae3a1..fe8587e5762 100644 --- a/clap_complete/tests/snapshots/quoting.zsh +++ b/clap_complete/tests/snapshots/quoting.zsh @@ -19,12 +19,12 @@ _my-app() { '--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`]' / -'--backslash[Avoid '/''//n'/'']' / -'--brackets[List packages /[filter/]]' / -'--expansions[Execute the shell command with $SHELL]' / +'*--single-quotes[Can be '/''always'/'', '/''auto'/'', or '/''never'/'']' / +'*--double-quotes[Can be "always", "auto", or "never"]' / +'*--backticks[For more information see `echo test`]' / +'*--backslash[Avoid '/''//n'/'']' / +'*--brackets[List packages /[filter/]]' / +'*--expansions[Execute the shell command with $SHELL]' / ":: :_my-app_commands" / "*::: :->my-app" / && ret=0 diff --git a/clap_complete/tests/snapshots/special_commands.zsh b/clap_complete/tests/snapshots/special_commands.zsh index c906b334284..5a7d1911ef7 100644 --- a/clap_complete/tests/snapshots/special_commands.zsh +++ b/clap_complete/tests/snapshots/special_commands.zsh @@ -36,7 +36,7 @@ _my-app() { case $line[3] in (test) _arguments "${_arguments_options[@]}" / -'--case=[the case to test]: : ' / +'*--case=[the case to test]: : ' / '-h[Print help information]' / '--help[Print help information]' / '-V[Print version information]' / @@ -45,7 +45,7 @@ _arguments "${_arguments_options[@]}" / ;; (some_cmd) _arguments "${_arguments_options[@]}" / -'--config=[the other case to test]: : ' / +'*--config=[the other case to test]: : ' / '-h[Print help information]' / '--help[Print help information]' / '-V[Print version information]' / diff --git a/clap_complete/tests/snapshots/sub_subcommands.zsh b/clap_complete/tests/snapshots/sub_subcommands.zsh index 947d0cb4ec8..ac408b602c1 100644 --- a/clap_complete/tests/snapshots/sub_subcommands.zsh +++ b/clap_complete/tests/snapshots/sub_subcommands.zsh @@ -36,7 +36,7 @@ _my-app() { case $line[3] in (test) _arguments "${_arguments_options[@]}" / -'--case=[the case to test]: : ' / +'*--case=[the case to test]: : ' / '-h[Print help information]' / '--help[Print help information]' / '-V[Print version information]' / @@ -61,7 +61,7 @@ _arguments "${_arguments_options[@]}" / case $line[1] in (sub_cmd) _arguments "${_arguments_options[@]}" / -'--config=[the other case to test]: :(Lest quotes aren't escaped.)' / +'*--config=[the other case to test]: :(Lest quotes aren't escaped.)' / '-h[Print help information]' / '--help[Print help information]' / '-V[Print version information]' / diff --git a/clap_complete/tests/snapshots/value_hint.zsh b/clap_complete/tests/snapshots/value_hint.zsh index 1bb1411e86f..a2cc5ee8d34 100644 --- a/clap_complete/tests/snapshots/value_hint.zsh +++ b/clap_complete/tests/snapshots/value_hint.zsh @@ -15,26 +15,26 @@ _my-app() { local context curcontext="$curcontext" state line _arguments "${_arguments_options[@]}" / -'--choice=[]: :(bash fish zsh)' / -'--unknown=[]: : ' / -'--other=[]: :( )' / -'-p+[]: :_files' / -'--path=[]: :_files' / -'-f+[]: :_files' / -'--file=[]: :_files' / -'-d+[]: :_files -/' / -'--dir=[]: :_files -/' / -'-e+[]: :_absolute_command_paths' / -'--exe=[]: :_absolute_command_paths' / -'--cmd-name=[]: :_command_names -e' / -'-c+[]: :_cmdstring' / -'--cmd=[]: :_cmdstring' / -'-u+[]: :_users' / -'--user=[]: :_users' / -'-h+[]: :_hosts' / -'--host=[]: :_hosts' / -'--url=[]: :_urls' / -'--email=[]: :_email_addresses' / +'*--choice=[]: :(bash fish zsh)' / +'*--unknown=[]: : ' / +'*--other=[]: :( )' / +'*-p+[]: :_files' / +'*--path=[]: :_files' / +'*-f+[]: :_files' / +'*--file=[]: :_files' / +'*-d+[]: :_files -/' / +'*--dir=[]: :_files -/' / +'*-e+[]: :_absolute_command_paths' / +'*--exe=[]: :_absolute_command_paths' / +'*--cmd-name=[]: :_command_names -e' / +'*-c+[]: :_cmdstring' / +'*--cmd=[]: :_cmdstring' / +'*-u+[]: :_users' / +'*--user=[]: :_users' / +'*-h+[]: :_hosts' / +'*--host=[]: :_hosts' / +'*--url=[]: :_urls' / +'*--email=[]: :_email_addresses' / '--help[Print help information]' / '*::command_with_args:_cmdambivalent' / && ret=0 diff --git a/clap_complete_fig/tests/snapshots/aliases.fig.js b/clap_complete_fig/tests/snapshots/aliases.fig.js index 16f61717f27..8d5378b76a1 100644 --- a/clap_complete_fig/tests/snapshots/aliases.fig.js +++ b/clap_complete_fig/tests/snapshots/aliases.fig.js @@ -5,6 +5,7 @@ const completion: Fig.Spec = { { name: ["-o", "-O", "--option", "--opt"], description: "cmd option", + isRepeatable: true, args: { name: "option", 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 5ab7eb30ae3..69d343d156b 100644 --- a/clap_complete_fig/tests/snapshots/feature_sample.fig.js +++ b/clap_complete_fig/tests/snapshots/feature_sample.fig.js @@ -9,6 +9,7 @@ const completion: Fig.Spec = { { name: "--case", description: "the case to test", + isRepeatable: true, args: { name: "case", 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 1982cac0070..de9dcf6fc82 100644 --- a/clap_complete_fig/tests/snapshots/special_commands.fig.js +++ b/clap_complete_fig/tests/snapshots/special_commands.fig.js @@ -9,6 +9,7 @@ const completion: Fig.Spec = { { name: "--case", description: "the case to test", + isRepeatable: true, args: { name: "case", isOptional: true, @@ -32,6 +33,7 @@ const completion: Fig.Spec = { name: "--config", description: "the other case to test", hidden: true, + isRepeatable: true, requiresEquals: true, args: { name: "config", diff --git a/clap_complete_fig/tests/snapshots/sub_subcommands.fig.js b/clap_complete_fig/tests/snapshots/sub_subcommands.fig.js index 8d6dbca78eb..c2dced153f4 100644 --- a/clap_complete_fig/tests/snapshots/sub_subcommands.fig.js +++ b/clap_complete_fig/tests/snapshots/sub_subcommands.fig.js @@ -9,6 +9,7 @@ const completion: Fig.Spec = { { name: "--case", description: "the case to test", + isRepeatable: true, args: { name: "case", isOptional: true, @@ -35,6 +36,7 @@ const completion: Fig.Spec = { { name: "--config", description: "the other case to test", + isRepeatable: true, args: { name: "config", isOptional: true, diff --git a/clap_complete_fig/tests/snapshots/value_hint.fig.js b/clap_complete_fig/tests/snapshots/value_hint.fig.js index 7c1860b0368..67fa2801be4 100644 --- a/clap_complete_fig/tests/snapshots/value_hint.fig.js +++ b/clap_complete_fig/tests/snapshots/value_hint.fig.js @@ -4,6 +4,7 @@ const completion: Fig.Spec = { options: [ { name: "--choice", + isRepeatable: true, args: { name: "choice", isOptional: true, @@ -16,6 +17,7 @@ const completion: Fig.Spec = { }, { name: "--unknown", + isRepeatable: true, args: { name: "unknown", isOptional: true, @@ -23,6 +25,7 @@ const completion: Fig.Spec = { }, { name: "--other", + isRepeatable: true, args: { name: "other", isOptional: true, @@ -30,6 +33,7 @@ const completion: Fig.Spec = { }, { name: ["-p", "--path"], + isRepeatable: true, args: { name: "path", isOptional: true, @@ -38,6 +42,7 @@ const completion: Fig.Spec = { }, { name: ["-f", "--file"], + isRepeatable: true, args: { name: "file", isOptional: true, @@ -46,6 +51,7 @@ const completion: Fig.Spec = { }, { name: ["-d", "--dir"], + isRepeatable: true, args: { name: "dir", isOptional: true, @@ -54,6 +60,7 @@ const completion: Fig.Spec = { }, { name: ["-e", "--exe"], + isRepeatable: true, args: { name: "exe", isOptional: true, @@ -62,6 +69,7 @@ const completion: Fig.Spec = { }, { name: "--cmd-name", + isRepeatable: true, args: { name: "cmd_name", isOptional: true, @@ -70,6 +78,7 @@ const completion: Fig.Spec = { }, { name: ["-c", "--cmd"], + isRepeatable: true, args: { name: "cmd", isOptional: true, @@ -78,6 +87,7 @@ const completion: Fig.Spec = { }, { name: ["-u", "--user"], + isRepeatable: true, args: { name: "user", isOptional: true, @@ -85,6 +95,7 @@ const completion: Fig.Spec = { }, { name: ["-h", "--host"], + isRepeatable: true, args: { name: "host", isOptional: true, @@ -92,6 +103,7 @@ const completion: Fig.Spec = { }, { name: "--url", + isRepeatable: true, args: { name: "url", isOptional: true, @@ -99,6 +111,7 @@ const completion: Fig.Spec = { }, { name: "--email", + isRepeatable: true, args: { name: "email", isOptional: true, diff --git a/clap_mangen/tests/snapshots/aliases.bash.roff b/clap_mangen/tests/snapshots/aliases.bash.roff index 63a5dad8505..2f22d2234eb 100644 --- a/clap_mangen/tests/snapshots/aliases.bash.roff +++ b/clap_mangen/tests/snapshots/aliases.bash.roff @@ -15,7 +15,7 @@ Print help information /fB/-V/fR, /fB/-/-version/fR Print version information .TP -/fB/-f/fR, /fB/-/-flag/fR +/fB/-f/fR, /fB/-/-flag/fR [default: false] cmd flag .TP /fB/-o/fR, /fB/-/-option/fR diff --git a/clap_mangen/tests/snapshots/basic.bash.roff b/clap_mangen/tests/snapshots/basic.bash.roff index 94da963b70c..5655191eed8 100644 --- a/clap_mangen/tests/snapshots/basic.bash.roff +++ b/clap_mangen/tests/snapshots/basic.bash.roff @@ -11,10 +11,10 @@ my/-app /fB/-h/fR, /fB/-/-help/fR Print help information .TP -/fB/-c/fR +/fB/-c/fR [default: false] .TP -/fB/-v/fR +/fB/-v/fR [default: false] .SH SUBCOMMANDS .TP diff --git a/clap_mangen/tests/snapshots/feature_sample.bash.roff b/clap_mangen/tests/snapshots/feature_sample.bash.roff index ae9be971f3a..814b63d3941 100644 --- a/clap_mangen/tests/snapshots/feature_sample.bash.roff +++ b/clap_mangen/tests/snapshots/feature_sample.bash.roff @@ -15,7 +15,7 @@ Print help information /fB/-V/fR, /fB/-/-version/fR Print version information .TP -/fB/-c/fR, /fB/-/-config/fR +/fB/-c/fR, /fB/-/-config/fR [default: false] some config file .TP [/fIfile/fR] diff --git a/clap_mangen/tests/snapshots/quoting.bash.roff b/clap_mangen/tests/snapshots/quoting.bash.roff index 454126134a0..f0e0e1b5b09 100644 --- a/clap_mangen/tests/snapshots/quoting.bash.roff +++ b/clap_mangen/tests/snapshots/quoting.bash.roff @@ -14,22 +14,22 @@ Print help information /fB/-V/fR, /fB/-/-version/fR Print version information .TP -/fB/-/-single/-quotes/fR +/fB/-/-single/-quotes/fR [default: false] Can be /*(Aqalways/*(Aq, /*(Aqauto/*(Aq, or /*(Aqnever/*(Aq .TP -/fB/-/-double/-quotes/fR +/fB/-/-double/-quotes/fR [default: false] Can be "always", "auto", or "never" .TP -/fB/-/-backticks/fR +/fB/-/-backticks/fR [default: false] For more information see `echo test` .TP -/fB/-/-backslash/fR +/fB/-/-backslash/fR [default: false] Avoid /*(Aq//n/*(Aq .TP -/fB/-/-brackets/fR +/fB/-/-brackets/fR [default: false] List packages [filter] .TP -/fB/-/-expansions/fR +/fB/-/-expansions/fR [default: false] Execute the shell command with $SHELL .SH SUBCOMMANDS .TP diff --git a/clap_mangen/tests/snapshots/special_commands.bash.roff b/clap_mangen/tests/snapshots/special_commands.bash.roff index 08d364d8bc3..3191e55894b 100644 --- a/clap_mangen/tests/snapshots/special_commands.bash.roff +++ b/clap_mangen/tests/snapshots/special_commands.bash.roff @@ -15,7 +15,7 @@ Print help information /fB/-V/fR, /fB/-/-version/fR Print version information .TP -/fB/-c/fR, /fB/-/-config/fR +/fB/-c/fR, /fB/-/-config/fR [default: false] some config file .TP [/fIfile/fR] diff --git a/clap_mangen/tests/snapshots/sub_subcommands.bash.roff b/clap_mangen/tests/snapshots/sub_subcommands.bash.roff index e364240fe71..c3d89db9bbc 100644 --- a/clap_mangen/tests/snapshots/sub_subcommands.bash.roff +++ b/clap_mangen/tests/snapshots/sub_subcommands.bash.roff @@ -15,7 +15,7 @@ Print help information /fB/-V/fR, /fB/-/-version/fR Print version information .TP -/fB/-c/fR, /fB/-/-config/fR +/fB/-c/fR, /fB/-/-config/fR [default: false] some config file .TP [/fIfile/fR] diff --git a/src/builder/arg.rs b/src/builder/arg.rs index de9cbcfffab..de8daa4f4b3 100644 --- a/src/builder/arg.rs +++ b/src/builder/arg.rs @@ -995,21 +995,22 @@ impl<'help> Arg<'help> { /// assert_eq!(files, ["file1", "file2", "file3"]); /// ``` /// - /// Although `multiple_values` has been specified, we cannot use the argument more than once. + /// Although `multiple_values` has been specified, the last argument still wins /// /// ```rust /// # use clap::{Command, Arg, ErrorKind}; - /// let res = Command::new("prog") + /// let m = Command::new("prog") /// .arg(Arg::new("file") /// .takes_value(true) /// .multiple_values(true) /// .short('F')) - /// .try_get_matches_from(vec![ + /// .get_matches_from(vec![ /// "prog", "-F", "file1", "-F", "file2", "-F", "file3" /// ]); /// - /// assert!(res.is_err()); - /// assert_eq!(res.unwrap_err().kind(), ErrorKind::UnexpectedMultipleUsage) + /// assert!(m.contains_id("file")); + /// let files: Vec<_> = m.get_many::("file").unwrap().collect(); + /// assert_eq!(files, ["file3"]); /// ``` /// /// A common mistake is to define an option which allows multiple values, and a positional @@ -2128,13 +2129,16 @@ impl<'help> Arg<'help> { /// /// In this example, because [`Arg::takes_value(false)`] (by default), /// `prog` is a flag that accepts an optional, case-insensitive boolean literal. - /// A `false` literal is `n`, `no`, `f`, `false`, `off` or `0`. - /// An absent environment variable will also be considered as `false`. - /// Anything else will considered as `true`. + /// + /// Note that the value parser controls how flags are parsed. In this case we've selected + /// [`FalseyValueParser`][crate::builder::FalseyValueParser]. A `false` literal is `n`, `no`, + /// `f`, `false`, `off` or `0`. An absent environment variable will also be considered as + /// `false`. Anything else will considered as `true`. /// /// ```rust /// # use std::env; /// # use clap::{Command, Arg}; + /// # use clap::builder::FalseyValueParser; /// /// env::set_var("TRUE_FLAG", "true"); /// env::set_var("FALSE_FLAG", "0"); @@ -2142,20 +2146,23 @@ impl<'help> Arg<'help> { /// let m = Command::new("prog") /// .arg(Arg::new("true_flag") /// .long("true_flag") + /// .value_parser(FalseyValueParser::new()) /// .env("TRUE_FLAG")) /// .arg(Arg::new("false_flag") /// .long("false_flag") + /// .value_parser(FalseyValueParser::new()) /// .env("FALSE_FLAG")) /// .arg(Arg::new("absent_flag") /// .long("absent_flag") + /// .value_parser(FalseyValueParser::new()) /// .env("ABSENT_FLAG")) /// .get_matches_from(vec![ /// "prog" /// ]); /// - /// assert!(m.is_present("true_flag")); - /// assert!(!m.is_present("false_flag")); - /// assert!(!m.is_present("absent_flag")); + /// assert!(*m.get_one::("true_flag").unwrap()); + /// assert!(!*m.get_one::("false_flag").unwrap()); + /// assert!(!*m.get_one::("absent_flag").unwrap()); /// ``` /// /// In this example, we show the variable coming from an option on the CLI: @@ -2920,7 +2927,7 @@ impl<'help> Arg<'help> { /// .long("flag")) /// .arg(Arg::new("other") /// .long("other") - /// .default_value_if("flag", None, Some("default"))) + /// .default_value_if("flag", Some("true"), Some("default"))) /// .get_matches_from(vec![ /// "prog" /// ]); @@ -2976,7 +2983,7 @@ impl<'help> Arg<'help> { /// .arg(Arg::new("other") /// .long("other") /// .default_value("default") - /// .default_value_if("flag", None, None)) + /// .default_value_if("flag", Some("true"), None)) /// .get_matches_from(vec![ /// "prog", "--flag" /// ]); @@ -3034,7 +3041,7 @@ impl<'help> Arg<'help> { /// .arg(Arg::new("other") /// .long("other") /// .default_value_ifs(&[ - /// ("flag", None, Some("default")), + /// ("flag", Some("true"), Some("default")), /// ("opt", Some("channal"), Some("chan")), /// ])) /// .get_matches_from(vec![ @@ -3054,7 +3061,7 @@ impl<'help> Arg<'help> { /// .arg(Arg::new("other") /// .long("other") /// .default_value_ifs(&[ - /// ("flag", None, Some("default")), + /// ("flag", Some("true"), Some("default")), /// ("opt", Some("channal"), Some("chan")), /// ])) /// .get_matches_from(vec![ @@ -4249,7 +4256,7 @@ impl<'help> Arg<'help> { /// Behavior when parsing the argument pub fn get_action(&self) -> &super::ArgAction { - const DEFAULT: super::ArgAction = super::ArgAction::StoreValue; + const DEFAULT: super::ArgAction = super::ArgAction::Set; self.action.as_ref().unwrap_or(&DEFAULT) } @@ -4369,10 +4376,15 @@ impl<'help> Arg<'help> { let action = super::ArgAction::Version; self.action = Some(action); } else if self.is_takes_value_set() { - let action = super::ArgAction::StoreValue; + let action = if self.is_positional() && self.is_multiple_values_set() { + // Allow collecting arguments interleaved with flags + super::ArgAction::Append + } else { + super::ArgAction::Set + }; self.action = Some(action); } else { - let action = super::ArgAction::IncOccurrence; + let action = super::ArgAction::SetTrue; self.action = Some(action); } } diff --git a/src/error/kind.rs b/src/error/kind.rs index 9f5c2bc4f86..a29e7d871f4 100644 --- a/src/error/kind.rs +++ b/src/error/kind.rs @@ -208,19 +208,6 @@ pub enum ErrorKind { MissingSubcommand, /// Occurs when the user provides multiple values to an argument which doesn't allow that. - /// - /// # Examples - /// - /// ```rust - /// # use clap::{Command, Arg, ErrorKind}; - /// let result = Command::new("prog") - /// .arg(Arg::new("debug") - /// .long("debug") - /// .multiple_occurrences(false)) - /// .try_get_matches_from(vec!["prog", "--debug", "--debug"]); - /// assert!(result.is_err()); - /// assert_eq!(result.unwrap_err().kind(), ErrorKind::UnexpectedMultipleUsage); - /// ``` UnexpectedMultipleUsage, /// Occurs when the user provides a value containing invalid UTF-8. diff --git a/tests/builder/grouped_values.rs b/tests/builder/grouped_values.rs index f289a0f7228..c44f0dd257b 100644 --- a/tests/builder/grouped_values.rs +++ b/tests/builder/grouped_values.rs @@ -191,7 +191,7 @@ fn grouped_interleaved_positional_values() { .unwrap(); let pos: Vec<_> = m.grouped_values_of("pos").unwrap().collect(); - assert_eq!(pos, vec![vec!["1", "2", "3", "4"]]); + assert_eq!(pos, vec![vec!["1", "2"], vec!["3"], vec!["4"]]); let flag: Vec<_> = m.grouped_values_of("flag").unwrap().collect(); assert_eq!(flag, vec![vec!["a"], vec!["b"]]); @@ -214,7 +214,7 @@ fn grouped_interleaved_positional_occurrences() { .unwrap(); let pos: Vec<_> = m.grouped_values_of("pos").unwrap().collect(); - assert_eq!(pos, vec![vec!["1", "2", "3", "4"]]); + assert_eq!(pos, vec![vec!["1", "2"], vec!["3"], vec!["4"]]); let flag: Vec<_> = m.grouped_values_of("flag").unwrap().collect(); assert_eq!(flag, vec![vec!["a"], vec!["b"]]); diff --git a/tests/builder/opts.rs b/tests/builder/opts.rs index 1dbe6c39c89..3690c596453 100644 --- a/tests/builder/opts.rs +++ b/tests/builder/opts.rs @@ -520,10 +520,6 @@ fn issue_1047_min_zero_vals_default_val() { ) .try_get_matches_from(vec!["foo", "-d"]) .unwrap(); - #[allow(deprecated)] - { - assert_eq!(m.occurrences_of("del"), 1); - } assert_eq!( m.get_one::("del").map(|v| v.as_str()), Some("default") From 50259e51d52090ce2cb93ba7e1ea41c3d4f0ad11 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 22 Jul 2022 13:36:10 -0500 Subject: [PATCH 6/6] fix!: Remove deprecated ArgActions --- clap_complete_fig/src/fig.rs | 26 --------- src/builder/action.rs | 34 ----------- src/builder/arg.rs | 5 +- src/parser/matches/matched_arg.rs | 5 -- src/parser/parser.rs | 95 +++---------------------------- 5 files changed, 8 insertions(+), 157 deletions(-) diff --git a/clap_complete_fig/src/fig.rs b/clap_complete_fig/src/fig.rs index f2292d14fd8..32e426aa5ec 100644 --- a/clap_complete_fig/src/fig.rs +++ b/clap_complete_fig/src/fig.rs @@ -216,19 +216,6 @@ fn gen_options(cmd: &Command, indent: usize) -> String { buffer.push_str(&format!("{:indent$}],\n", "", indent = indent + 4)); } - #[allow(deprecated)] - if matches!( - option.get_action(), - ArgAction::StoreValue | ArgAction::IncOccurrence - ) && option.is_multiple_occurrences_set() - { - buffer.push_str(&format!( - "{:indent$}isRepeatable: true,\n", - "", - indent = indent + 4 - )); - } - if let ArgAction::Set | ArgAction::Append | ArgAction::Count = option.get_action() { buffer.push_str(&format!( "{:indent$}isRepeatable: true,\n", @@ -316,19 +303,6 @@ fn gen_options(cmd: &Command, indent: usize) -> String { buffer.push_str(&format!("{:indent$}],\n", "", indent = indent + 4)); } - #[allow(deprecated)] - if matches!( - flag.get_action(), - ArgAction::StoreValue | ArgAction::IncOccurrence - ) && flag.is_multiple_occurrences_set() - { - buffer.push_str(&format!( - "{:indent$}isRepeatable: true,\n", - "", - indent = indent + 4 - )); - } - if let ArgAction::Set | ArgAction::Append | ArgAction::Count = flag.get_action() { buffer.push_str(&format!( "{:indent$}isRepeatable: true,\n", diff --git a/src/builder/action.rs b/src/builder/action.rs index 382d7cef532..8d755927327 100644 --- a/src/builder/action.rs +++ b/src/builder/action.rs @@ -70,24 +70,6 @@ pub enum ArgAction { /// ); /// ``` Append, - /// Deprecated, replaced with [`ArgAction::Set`] or [`ArgAction::Append`] - #[cfg_attr( - feature = "deprecated", - deprecated( - since = "3.2.0", - note = "Replaced with `ArgAction::Set` or `ArgAction::Append`" - ) - )] - StoreValue, - /// Deprecated, replaced with [`ArgAction::SetTrue`] or [`ArgAction::Count`] - #[cfg_attr( - feature = "deprecated", - deprecated( - since = "3.2.0", - note = "Replaced with `ArgAction::SetTrue` or `ArgAction::Count`" - ) - )] - IncOccurrence, /// When encountered, act as if `"true"` was encountered on the command-line /// /// If no [`default_value`][super::Arg::default_value] is set, it will be `false`. @@ -258,10 +240,6 @@ impl ArgAction { match self { Self::Set => true, Self::Append => true, - #[allow(deprecated)] - Self::StoreValue => true, - #[allow(deprecated)] - Self::IncOccurrence => false, Self::SetTrue => false, Self::SetFalse => false, Self::Count => false, @@ -274,10 +252,6 @@ impl ArgAction { match self { Self::Set => None, Self::Append => None, - #[allow(deprecated)] - Self::StoreValue => None, - #[allow(deprecated)] - Self::IncOccurrence => None, Self::SetTrue => Some(std::ffi::OsStr::new("false")), Self::SetFalse => Some(std::ffi::OsStr::new("true")), Self::Count => Some(std::ffi::OsStr::new("0")), @@ -290,10 +264,6 @@ impl ArgAction { match self { Self::Set => None, Self::Append => None, - #[allow(deprecated)] - Self::StoreValue => None, - #[allow(deprecated)] - Self::IncOccurrence => None, Self::SetTrue => Some(super::ValueParser::bool()), Self::SetFalse => Some(super::ValueParser::bool()), Self::Count => Some(crate::value_parser!(u8).into()), @@ -309,10 +279,6 @@ impl ArgAction { match self { Self::Set => None, Self::Append => None, - #[allow(deprecated)] - Self::StoreValue => None, - #[allow(deprecated)] - Self::IncOccurrence => None, Self::SetTrue => Some(AnyValueId::of::()), Self::SetFalse => Some(AnyValueId::of::()), Self::Count => Some(AnyValueId::of::()), diff --git a/src/builder/arg.rs b/src/builder/arg.rs index de8daa4f4b3..24ec02d21c4 100644 --- a/src/builder/arg.rs +++ b/src/builder/arg.rs @@ -4400,10 +4400,7 @@ impl<'help> Arg<'help> { self.settings.unset(ArgSettings::TakesValue); } match action { - ArgAction::StoreValue - | ArgAction::IncOccurrence - | ArgAction::Help - | ArgAction::Version => {} + ArgAction::Help | ArgAction::Version => {} ArgAction::Set | ArgAction::Append | ArgAction::SetTrue diff --git a/src/parser/matches/matched_arg.rs b/src/parser/matches/matched_arg.rs index fde6d37f3d8..b66b5616c9d 100644 --- a/src/parser/matches/matched_arg.rs +++ b/src/parser/matches/matched_arg.rs @@ -72,11 +72,6 @@ impl MatchedArg { self.occurs += 1; } - #[cfg_attr(feature = "deprecated", deprecated(since = "3.2.0"))] - pub(crate) fn set_occurrences(&mut self, occurs: u64) { - self.occurs = occurs - } - #[cfg_attr(feature = "deprecated", deprecated(since = "3.2.0"))] pub(crate) fn get_occurrences(&self) -> u64 { self.occurs diff --git a/src/parser/parser.rs b/src/parser/parser.rs index a4dbd6de168..b3936dc8a7f 100644 --- a/src/parser/parser.rs +++ b/src/parser/parser.rs @@ -1180,53 +1180,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { } Ok(ParseResult::ValuesDone) } - #[allow(deprecated)] - ArgAction::StoreValue => { - if ident == Some(Identifier::Index) - && arg.is_multiple_values_set() - && matcher.contains(&arg.id) - { - // HACK: Reuse existing occurrence - } else if source == ValueSource::CommandLine { - if matches!(ident, Some(Identifier::Short) | Some(Identifier::Long)) { - // Record flag's index - self.cur_idx.set(self.cur_idx.get() + 1); - debug!("Parser::react: cur_idx:={}", self.cur_idx.get()); - } - self.start_occurrence_of_arg(matcher, arg); - } else { - self.start_custom_arg(matcher, arg, source); - } - self.push_arg_values(arg, raw_vals, matcher)?; - if ident == Some(Identifier::Index) && arg.is_multiple_values_set() { - // HACK: Maintain existing occurrence behavior - let matched = matcher.get_mut(&arg.id).unwrap(); - #[allow(deprecated)] - matched.set_occurrences(matched.num_vals() as u64); - } - if cfg!(debug_assertions) && matcher.needs_more_vals(arg) { - debug!( - "Parser::react not enough values passed in, leaving it to the validator to complain", - ); - } - Ok(ParseResult::ValuesDone) - } - #[allow(deprecated)] - ArgAction::IncOccurrence => { - debug_assert_eq!(raw_vals, Vec::::new()); - if source == ValueSource::CommandLine { - if matches!(ident, Some(Identifier::Short) | Some(Identifier::Long)) { - // Record flag's index - self.cur_idx.set(self.cur_idx.get() + 1); - debug!("Parser::react: cur_idx:={}", self.cur_idx.get()); - } - self.start_occurrence_of_arg(matcher, arg); - } else { - self.start_custom_arg(matcher, arg, source); - } - matcher.add_index_to(&arg.id, self.cur_idx.get()); - Ok(ParseResult::ValuesDone) - } ArgAction::SetTrue => { let raw_vals = match raw_vals.len() { 0 => { @@ -1338,7 +1291,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { #[cfg(feature = "env")] fn add_env(&mut self, matcher: &mut ArgMatcher) -> ClapResult<()> { debug!("Parser::add_env"); - use crate::util::str_to_bool; let trailing_values = false; // defaults are independent of the commandline for arg in self.cmd.get_arguments() { @@ -1368,46 +1320,13 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { } } } else { - match arg.get_action() { - #[allow(deprecated)] - ArgAction::StoreValue => unreachable!("{:?} is not a flag", arg.get_id()), - #[allow(deprecated)] - ArgAction::IncOccurrence => { - debug!("Parser::add_env: Found a flag with value `{:?}`", val); - let predicate = str_to_bool(val.to_str_lossy()); - debug!("Parser::add_env: Found boolean literal `{:?}`", predicate); - if predicate.unwrap_or(true) { - let _ = self.react( - None, - ValueSource::EnvVariable, - arg, - vec![], - matcher, - )?; - } - } - ArgAction::Set - | ArgAction::Append - | ArgAction::SetTrue - | ArgAction::SetFalse - | ArgAction::Count - | ArgAction::Help - | ArgAction::Version => { - let mut arg_values = Vec::new(); - let _parse_result = - self.split_arg_values(arg, &val, trailing_values, &mut arg_values); - let _ = self.react( - None, - ValueSource::EnvVariable, - arg, - arg_values, - matcher, - )?; - if let Some(_parse_result) = _parse_result { - if _parse_result != ParseResult::ValuesDone { - debug!("Parser::add_env: Ignoring state {:?}; env variables are outside of the parse loop", _parse_result); - } - } + let mut arg_values = Vec::new(); + let _parse_result = + self.split_arg_values(arg, &val, trailing_values, &mut arg_values); + let _ = self.react(None, ValueSource::EnvVariable, arg, arg_values, matcher)?; + if let Some(_parse_result) = _parse_result { + if _parse_result != ParseResult::ValuesDone { + debug!("Parser::add_env: Ignoring state {:?}; env variables are outside of the parse loop", _parse_result); } } }