Merge pull request #4187 from epage/exp

fix(parser): Resolve problems around `allow_hyphen_values`

CHANGELOG.md

@@ -44,6 +44,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - `Arg::short_aliases` and other builder functions that took `&[]` need the `&` dropped
 - Changed `Arg::requires_ifs` and `Arg::default_value*_ifs*` to taking an `ArgPredicate`, removing ambiguity with `None` when accepting owned and borrowed types
 - Removed lifetimes from `Command`, `Arg`, `ArgGroup`, and `PossibleValue`
+- *(parser)* Short flags now have higher precedence than hyphen values with `Arg::allow_hyphen_values`, like `Command::allow_hyphen_values`
 - *(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)
 - *(help)* Subcommands are now listed before arguments.  To get the old behavior, see `Command::help_template`
@@ -66,6 +67,9 @@ Deprecated
 - `Arg::number_of_values` in favor of `Arg::num_args`
 - `default_value_os`, `default_values_os`, `default_value_if_os`, and `default_value_ifs_os` as the non `_os` variants now accept either a `str` or an `OsStr`
 - `Command::dont_collapse_args_in_usage` is now the default and is deprecated
+- `Command::trailing_var_arg` in favor of `Arg::trailing_var_arg`
+- `Command::allow_hyphen_values` in favor of `Arg::allow_hyphen_values`
+- `Command::allow_negative_numbers` in favor of `Arg::allow_negative_numbers`
 - *(derive)* `structopt` and `clap` attributes in favor of the more specific `command`, `arg`, and `value`
 
 ### Features
@@ -108,6 +112,7 @@ Deprecated
 - *(version)* Use `Command::display_name` rather than `Command::bin_name`
 - *(parser)* Assert on unknown args when using external subcommands (#3703)
 - *(parser)* Always fill in `""` argument for external subcommands (#3263)
+- *(parser)* Short flags now have higher precedence than hyphen values with `Arg::allow_hyphen_values`, like `Command::allow_hyphen_values`
 - *(derive)* Detect escaped external subcommands that look like built-in subcommands (#3703)
 - *(derive)* Leave `Arg::id` as `verbatim` casing (#3282)
 - *(derive)* Default to `#[clap(value_parser, action)]` instead of `#[clap(parse)]` (#3827)

clap_bench/benches/06_rustup.rs

@@ -238,9 +238,13 @@ fn build_cli() -> Command {
             Command::new("run")
                 .about("Run a command with an environment configured for a given toolchain")
                 .after_help(RUN_HELP)
-                .trailing_var_arg(true)
                 .arg(Arg::new("toolchain").required(true))
-                .arg(Arg::new("command").required(true).num_args(1..)),
+                .arg(
+                    Arg::new("command")
+                        .required(true)
+                        .num_args(1..)
+                        .trailing_var_arg(true),
+                ),
         )
         .subcommand(
             Command::new("which")

clap_complete/examples/completion-derive.rs

@@ -19,11 +19,7 @@ use std::io;
 use std::path::PathBuf;
 
 #[derive(Parser, Debug, PartialEq)]
-#[command(
-    name = "value_hints_derive",
-    // Command::trailing_var_ar is required to use ValueHint::CommandWithArguments
-    trailing_var_arg = true,
-)]
+#[command(name = "value_hints_derive")]
 struct Opt {
     /// If provided, outputs the completion file for given shell
     #[arg(long = "generate", value_enum)]
@@ -45,7 +41,8 @@ struct Opt {
     cmd_name: Option<OsString>,
     #[arg(short, long, value_hint = ValueHint::CommandString)]
     cmd: Option<String>,
-    #[arg(value_hint = ValueHint::CommandWithArguments)]
+    // Command::trailing_var_ar is required to use ValueHint::CommandWithArguments
+    #[arg(trailing_var_arg = true, value_hint = ValueHint::CommandWithArguments)]
     command_with_args: Vec<String>,
     #[arg(short, long, value_hint = ValueHint::Username)]
     user: Option<String>,

clap_complete/examples/completion.rs

@@ -18,8 +18,6 @@ use std::io;
 
 fn build_cli() -> Command {
     Command::new("value_hints")
-        // AppSettings::TrailingVarArg is required to use ValueHint::CommandWithArguments
-        .trailing_var_arg(true)
         .arg(
             Arg::new("generator")
                 .long("generate")
@@ -69,6 +67,8 @@ fn build_cli() -> Command {
         .arg(
             Arg::new("command_with_args")
                 .num_args(1..)
+                // AppSettings::TrailingVarArg is required to use ValueHint::CommandWithArguments
+                .trailing_var_arg(true)
                 .value_hint(ValueHint::CommandWithArguments),
         )
         .arg(

clap_complete/tests/common.rs

@@ -169,7 +169,6 @@ pub fn sub_subcommands_command(name: &'static str) -> clap::Command {
 
 pub fn value_hint_command(name: &'static str) -> clap::Command {
     clap::Command::new(name)
-        .trailing_var_arg(true)
         .arg(
             clap::Arg::new("choice")
                 .long("choice")
@@ -225,6 +224,7 @@ pub fn value_hint_command(name: &'static str) -> clap::Command {
             clap::Arg::new("command_with_args")
                 .action(clap::ArgAction::Set)
                 .num_args(1..)
+                .trailing_var_arg(true)
                 .value_hint(clap::ValueHint::CommandWithArguments),
         )
         .arg(

clap_complete_fig/tests/common.rs

@@ -165,7 +165,6 @@ pub fn sub_subcommands_command(name: &'static str) -> clap::Command {
 
 pub fn value_hint_command(name: &'static str) -> clap::Command {
     clap::Command::new(name)
-        .trailing_var_arg(true)
         .arg(
             clap::Arg::new("choice")
                 .long("choice")
@@ -221,6 +220,7 @@ pub fn value_hint_command(name: &'static str) -> clap::Command {
             clap::Arg::new("command_with_args")
                 .action(clap::ArgAction::Set)
                 .num_args(1..)
+                .trailing_var_arg(true)
                 .value_hint(clap::ValueHint::CommandWithArguments),
         )
         .arg(

clap_mangen/tests/common.rs

@@ -163,7 +163,6 @@ pub fn sub_subcommands_command(name: &'static str) -> clap::Command {
 
 pub fn value_hint_command(name: &'static str) -> clap::Command {
     clap::Command::new(name)
-        .trailing_var_arg(true)
         .arg(
             clap::Arg::new("choice")
                 .long("choice")
@@ -219,6 +218,7 @@ pub fn value_hint_command(name: &'static str) -> clap::Command {
             clap::Arg::new("command_with_args")
                 .action(clap::ArgAction::Set)
                 .num_args(1..)
+                .trailing_var_arg(true)
                 .value_hint(clap::ValueHint::CommandWithArguments),
         )
         .arg(
@@ -283,7 +283,6 @@ pub fn assert_matches_path(expected_path: impl AsRef<std::path::Path>, cmd: clap
 
 pub fn possible_values_command(name: &'static str) -> clap::Command {
     clap::Command::new(name)
-        .trailing_var_arg(true)
         .arg(
             clap::Arg::new("choice")
                 .long("choice")

examples/tutorial_builder/02_app_settings.md

@@ -6,13 +6,16 @@ Usage:
     02_app_settings[EXE] --two <VALUE> --one <VALUE>
 
 Options:
-        --two <VALUE>    
-        --one <VALUE>    
-    -h, --help           Print help information
-    -V, --version        Print version information
+        --two <VALUE>
+            
 
-$ 02_app_settings --one -1 --one -3 --two 10
-two: "10"
-one: "-3"
+        --one <VALUE>
+            
+
+    -h, --help
+            Print help information
+
+    -V, --version
+            Print version information
 
 ```

examples/tutorial_builder/02_app_settings.rs

@@ -2,7 +2,7 @@ use clap::{arg, command, ArgAction};
 
 fn main() {
     let matches = command!() // requires `cargo` feature
-        .allow_negative_numbers(true)
+        .next_line_help(true)
         .arg(arg!(--two <VALUE>).action(ArgAction::Set))
         .arg(arg!(--one <VALUE>).action(ArgAction::Set))
         .get_matches();

examples/tutorial_derive/02_app_settings.md

@@ -6,13 +6,16 @@ Usage:
     02_app_settings_derive[EXE] --two <TWO> --one <ONE>
 
 Options:
-        --two <TWO>    
-        --one <ONE>    
-    -h, --help         Print help information
-    -V, --version      Print version information
+        --two <TWO>
+            
 
-$ 02_app_settings_derive --one -1 --one -3 --two 10
-two: "10"
-one: "-3"
+        --one <ONE>
+            
+
+    -h, --help
+            Print help information
+
+    -V, --version
+            Print version information
 
 ```

examples/tutorial_derive/02_app_settings.rs

@@ -2,7 +2,7 @@ use clap::Parser;
 
 #[derive(Parser)]
 #[command(author, version, about, long_about = None)]
-#[command(allow_negative_numbers = true)]
+#[command(next_line_help = true)]
 struct Cli {
     #[arg(long)]
     two: String,

src/builder/arg.rs

@@ -459,6 +459,37 @@ impl Arg {
         self
     }
 
+    /// This is a "VarArg" and everything that follows should be captured by it, as if the user had
+    /// used a `--`.
+    ///
+    /// **NOTE:** To start the trailing "VarArg" on unknown flags (and not just a positional
+    /// value), set [`allow_hyphen_values`][Arg::allow_hyphen_values].  Either way, users still
+    /// have the option to explicitly escape ambiguous arguments with `--`.
+    ///
+    /// **NOTE:** [`Arg::value_delimiter`] still applies if set.
+    ///
+    /// **NOTE:** Setting this requires [`Arg::num_args(..)`].
+    ///
+    /// # Examples
+    ///
+    /// ```rust
+    /// # use clap::{Command, arg};
+    /// let m = Command::new("myprog")
+    ///     .arg(arg!(<cmd> ... "commands to run").trailing_var_arg(true))
+    ///     .get_matches_from(vec!["myprog", "arg1", "-r", "val1"]);
+    ///
+    /// let trail: Vec<_> = m.get_many::<String>("cmd").unwrap().collect();
+    /// assert_eq!(trail, ["arg1", "-r", "val1"]);
+    /// ```
+    /// [`Arg::num_args(..)`]: crate::Arg::num_args()
+    pub fn trailing_var_arg(self, yes: bool) -> Self {
+        if yes {
+            self.setting(ArgSettings::TrailingVarArg)
+        } else {
+            self.unset_setting(ArgSettings::TrailingVarArg)
+        }
+    }
+
     /// This arg is the last, or final, positional argument (i.e. has the highest
     /// index) and is *only* able to be accessed via the `--` syntax (i.e. `$ prog args --
     /// last_arg`).
@@ -1242,20 +1273,21 @@ impl Arg {
 
     /// Allows values which start with a leading hyphen (`-`)
     ///
+    /// To limit values to just numbers, see
+    /// [`allow_negative_numbers`][Arg::allow_negative_numbers].
+    ///
     /// **NOTE:** Setting this requires [taking values][Arg::num_args]
     ///
-    /// **WARNING**: Take caution when using this setting combined with
+    /// **NOTE:** If a positional argument has `allow_hyphen_values` and is followed by a known
+    /// flag, it will be treated as a flag (see [`trailing_var_arg`][Arg::trailing_var_arg] for
+    /// consuming known flags).  If an option has `allow_hyphen_values` and is followed by a known
+    /// flag, it will be treated as the value for the option.
+    ///
+    /// **WARNING**: Take caution when using this setting combined with another argument using
     /// [`Arg::num_args`], 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.
     ///
-    /// **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
-    /// `-F` is supposed to be a value, yet `-F` is *also* a valid short for another arg.
-    /// Care should be taken when designing these args. This is compounded by the ability to "stack"
-    /// short args. I.e. if `-val` is supposed to be a value, but `-v`, `-a`, and `-l` are all valid
-    /// shorts.
-    ///
     /// # Examples
     ///
     /// ```rust
@@ -1299,6 +1331,35 @@ impl Arg {
         }
     }
 
+    /// Allows negative numbers to pass as values.
+    ///
+    /// This is similar to [`Arg::allow_hyphen_values`] except that it only allows numbers,
+    /// all other undefined leading hyphens will fail to parse.
+    ///
+    /// **NOTE:** Setting this requires [taking values][Arg::num_args]
+    ///
+    /// # Examples
+    ///
+    /// ```rust
+    /// # use clap::{Command, Arg};
+    /// let res = Command::new("myprog")
+    ///     .arg(Arg::new("num").allow_negative_numbers(true))
+    ///     .try_get_matches_from(vec![
+    ///         "myprog", "-20"
+    ///     ]);
+    /// assert!(res.is_ok());
+    /// let m = res.unwrap();
+    /// assert_eq!(m.get_one::<String>("num").unwrap(), "-20");
+    /// ```
+    #[inline]
+    pub fn allow_negative_numbers(self, yes: bool) -> Self {
+        if yes {
+            self.setting(ArgSettings::AllowNegativeNumbers)
+        } else {
+            self.unset_setting(ArgSettings::AllowNegativeNumbers)
+        }
+    }
+
     /// Requires that options use the `--option=val` syntax
     ///
     /// i.e. an equals between the option and associated value.
@@ -3807,6 +3868,11 @@ impl Arg {
         self.is_set(ArgSettings::AllowHyphenValues)
     }
 
+    /// Report whether [`Arg::allow_negative_numbers`] is set
+    pub fn is_allow_negative_numbers_set(&self) -> bool {
+        self.is_set(ArgSettings::AllowNegativeNumbers)
+    }
+
     /// Behavior when parsing the argument
     pub fn get_action(&self) -> &super::ArgAction {
         const DEFAULT: super::ArgAction = super::ArgAction::Set;
@@ -3894,6 +3960,11 @@ impl Arg {
         self.is_set(ArgSettings::Exclusive)
     }
 
+    /// Report whether [`Arg::trailing_var_arg`] is set
+    pub fn is_trailing_var_arg_set(&self) -> bool {
+        self.is_set(ArgSettings::TrailingVarArg)
+    }
+
     /// Reports whether [`Arg::last`] is set
     pub fn is_last_set(&self) -> bool {
         self.is_set(ArgSettings::Last)

src/builder/arg_settings.rs

@@ -33,8 +33,10 @@ pub(crate) enum ArgSettings {
     NextLineHelp,
     HidePossibleValues,
     AllowHyphenValues,
+    AllowNegativeNumbers,
     RequireEquals,
     Last,
+    TrailingVarArg,
     HideDefaultValue,
     IgnoreCase,
     #[cfg(feature = "env")]
@@ -51,6 +53,8 @@ bitflags! {
         const REQUIRED         = 1;
         const GLOBAL           = 1 << 3;
         const HIDDEN           = 1 << 4;
+        const TRAILING_VARARG  = 1 << 5;
+        const ALLOW_NEG_NUMS   = 1 << 6;
         const NEXT_LINE_HELP   = 1 << 7;
         const DELIM_NOT_SET    = 1 << 10;
         const HIDE_POS_VALS    = 1 << 11;
@@ -77,8 +81,10 @@ impl_settings! { ArgSettings, ArgFlags,
     NextLineHelp => Flags::NEXT_LINE_HELP,
     HidePossibleValues => Flags::HIDE_POS_VALS,
     AllowHyphenValues => Flags::ALLOW_TAC_VALS,
+    AllowNegativeNumbers => Flags::ALLOW_NEG_NUMS,
     RequireEquals => Flags::REQUIRE_EQUALS,
     Last => Flags::LAST,
+    TrailingVarArg => Flags::TRAILING_VARARG,
     IgnoreCase => Flags::CASE_INSENSITIVE,
     #[cfg(feature = "env")]
     HideEnv => Flags::HIDE_ENV,