fix: Make arg!(--flag <value>) optional by default

This was ported over from the usage parser which modeled after docopt. We just never got around to implementing the rest of the syntax. However, when considering this as a standalone feature, an `arg!(--flag <value>)`, outside of other context, should be optional. This is how the help would display it. Fixes clap-rs#4206
davidrhp · Sep 12, 2022 · c9eef44 · c9eef44
1 parent 75a73f3
commit c9eef44
Show file tree

Hide file tree

Showing 31 changed files with 177 additions and 301 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -183,6 +183,7 @@ Subtle changes (i.e. compiler won't catch):
 - By default, an `Arg`s default action is `ArgAction::Set`, rather than `ArgAction::IncOccurrence` to reduce confusing magic through consistency (#2687, #4032, see also #3977)
 - `mut_arg` can no longer be used to customize help and version arguments, instead disable them (`Command::disable_help_flag`, `Command::disable_version_flag`) and provide your own (#4056)
 - Removed lifetimes from `Command`, `Arg`, `ArgGroup`, and `PossibleValue`
+- `arg!(--flag <value>)` is now optional, instead of required.  Add `.required(true)` at the end to restore the original behavior (#4206)
 - *(parser)* Always fill in `""` argument for external subcommands to make it easier to distinguish them from built-in commands (#3263)
 - *(parser)* Short flags now have higher precedence than hyphen values with `Arg::allow_hyphen_values`, to be consistent with `Command::allow_hyphen_values` (#4187)
 - *(parser)* `Arg::value_terminator` must be its own argument on the CLI rather than being in a delimited list (#4025)

diff --git a/clap_bench/benches/02_simple.rs b/clap_bench/benches/02_simple.rs
@@ -8,7 +8,7 @@ macro_rules! create_app {
             .about("tests clap library")
             .author("Kevin K. <[email protected]>")
             .arg(arg!(-f --flag         "tests flags"))
-            .arg(arg!(-o --option <opt> "tests options").required(false))
+            .arg(arg!(-o --option <opt> "tests options"))
             .arg(arg!([positional]      "tests positional"))
     }};
 }
@@ -34,14 +34,14 @@ pub fn build_with_flag_ref(c: &mut Criterion) {
 
 pub fn build_with_opt(c: &mut Criterion) {
     c.bench_function("build_with_opt", |b| {
-        b.iter(|| Command::new("claptests").arg(arg!(-s --some <FILE> "something")))
+        b.iter(|| Command::new("claptests").arg(arg!(-s --some <FILE> "something").required(true)))
     });
 }
 
 pub fn build_with_opt_ref(c: &mut Criterion) {
     c.bench_function("build_with_opt_ref", |b| {
         b.iter(|| {
-            let arg = arg!(-s --some <FILE> "something");
+            let arg = arg!(-s --some <FILE> "something").required(true);
             Command::new("claptests").arg(&arg)
         })
     });

diff --git a/clap_bench/benches/03_complex.rs b/clap_bench/benches/03_complex.rs
@@ -10,36 +10,34 @@ macro_rules! create_app {
             .version("0.1")
             .about("tests clap library")
             .author("Kevin K. <[email protected]>")
-            .arg(arg!(-o --option <opt> ... "tests options").required(false))
+            .arg(arg!(-o --option <opt> ... "tests options"))
             .arg(arg!([positional] "tests positionals"))
             .arg(arg!(-f --flag ... "tests flags").global(true))
             .args([
                 arg!(flag2: -F "tests flags with exclusions")
                     .conflicts_with("flag")
                     .requires("option2"),
                 arg!(option2: --"long-option-2" <option2> "tests long options with exclusions")
-                    .required(false)
                     .conflicts_with("option")
                     .requires("positional2"),
                 arg!([positional2] "tests positionals with exclusions"),
                 arg!(-O --Option <option3> "tests options with specific value sets")
-                .required(false)
                     .value_parser(OPT3_VALS),
                 arg!([positional3] ... "tests positionals with specific values")
                     .value_parser(POS3_VALS),
-                arg!(--multvals <s> "Tests multiple values not mult occs").required(false).value_names(["one", "two"]),
+                arg!(--multvals <s> "Tests multiple values not mult occs").value_names(["one", "two"]),
                 arg!(
                     --multvalsmo <s> "Tests multiple values, not mult occs"
                 ).required(false).value_names(["one", "two"]),
-                arg!(--minvals2 <minvals> ... "Tests 2 min vals").num_args(2..).required(false),
-                arg!(--maxvals3 <maxvals> ... "Tests 3 max vals").num_args(1..=3).required(false),
+                arg!(--minvals2 <minvals> ... "Tests 2 min vals").num_args(2..),
+                arg!(--maxvals3 <maxvals> ... "Tests 3 max vals").num_args(1..=3),
             ])
             .subcommand(
                 Command::new("subcmd")
                     .about("tests subcommands")
                     .version("0.1")
                     .author("Kevin K. <[email protected]>")
-                    .arg(arg!(-o --option <scoption> ... "tests options").required(false))
+                    .arg(arg!(-o --option <scoption> ... "tests options"))
                     .arg(arg!([scpositional] "tests positionals"))
             )
     }};

diff --git a/clap_mangen/examples/man.rs b/clap_mangen/examples/man.rs
@@ -19,7 +19,6 @@ And a few newlines.",
         .arg(
             arg!(-c --config <FILE> "Sets a custom config file")
                 .long_help("Some more text about how to set a custom config file")
-                .required(false)
                 .default_value("config.toml")
                 .env("CONFIG_FILE"),
         )

diff --git a/examples/cargo-example.rs b/examples/cargo-example.rs
@@ -5,7 +5,6 @@ fn main() {
         .subcommand(
             clap::command!("example").arg(
                 clap::arg!(--"manifest-path" <PATH>)
-                    .required(false)
                     .value_parser(clap::value_parser!(std::path::PathBuf)),
             ),
         );

diff --git a/examples/escaped-positional.rs b/examples/escaped-positional.rs
@@ -3,11 +3,7 @@ use clap::{arg, command, value_parser, ArgAction};
 fn main() {
     let matches = command!() // requires `cargo` feature
         .arg(arg!(eff: -f).action(ArgAction::SetTrue))
-        .arg(
-            arg!(pea: -p <PEAR>)
-                .required(false)
-                .value_parser(value_parser!(String)),
-        )
+        .arg(arg!(pea: -p <PEAR>).value_parser(value_parser!(String)))
         .arg(
             // Indicates that `slop` is only accessible after `--`.
             arg!(slop: [SLOP])

diff --git a/examples/find.md b/examples/find.md
@@ -4,7 +4,7 @@
 $ find --help
 A simple to use, efficient, and full-featured Command Line Argument Parser
 
-Usage: find[EXE] [OPTIONS] --name <NAME>
+Usage: find[EXE] [OPTIONS]
 
 Options:
   -h, --help     Print help information

diff --git a/examples/git.rs b/examples/git.rs
@@ -45,7 +45,7 @@ fn cli() -> Command {
 }
 
 fn push_args() -> Vec<clap::Arg> {
-    vec![arg!(-m --message <MESSAGE>).required(false)]
+    vec![arg!(-m --message <MESSAGE>)]
 }
 
 fn main() {

diff --git a/examples/tutorial_builder/02_app_settings.rs b/examples/tutorial_builder/02_app_settings.rs
@@ -3,8 +3,8 @@ use clap::{arg, command, ArgAction};
 fn main() {
     let matches = command!() // requires `cargo` feature
         .next_line_help(true)
-        .arg(arg!(--two <VALUE>).action(ArgAction::Set))
-        .arg(arg!(--one <VALUE>).action(ArgAction::Set))
+        .arg(arg!(--two <VALUE>).required(true).action(ArgAction::Set))
+        .arg(arg!(--one <VALUE>).required(true).action(ArgAction::Set))
         .get_matches();
 
     println!(

diff --git a/examples/tutorial_builder/02_apps.rs b/examples/tutorial_builder/02_apps.rs
@@ -5,8 +5,8 @@ fn main() {
         .version("1.0")
         .author("Kevin K. <[email protected]>")
         .about("Does awesome things")
-        .arg(arg!(--two <VALUE>))
-        .arg(arg!(--one <VALUE>))
+        .arg(arg!(--two <VALUE>).required(true))
+        .arg(arg!(--one <VALUE>).required(true))
         .get_matches();
 
     println!(

diff --git a/examples/tutorial_builder/02_crate.rs b/examples/tutorial_builder/02_crate.rs
@@ -3,8 +3,8 @@ use clap::{arg, command};
 fn main() {
     // requires `cargo` feature, reading name, version, author, and description from `Cargo.toml`
     let matches = command!()
-        .arg(arg!(--two <VALUE>))
-        .arg(arg!(--one <VALUE>))
+        .arg(arg!(--two <VALUE>).required(true))
+        .arg(arg!(--one <VALUE>).required(true))
         .get_matches();
 
     println!(

diff --git a/examples/tutorial_builder/04_03_relations.rs b/examples/tutorial_builder/04_03_relations.rs
@@ -6,7 +6,7 @@ fn main() {
     // Create application like normal
     let matches = command!() // requires `cargo` feature
         // Add the version arguments
-        .arg(arg!(--"set-ver" <VER> "set version manually").required(false))
+        .arg(arg!(--"set-ver" <VER> "set version manually"))
         .arg(arg!(--major         "auto inc major").action(ArgAction::SetTrue))
         .arg(arg!(--minor         "auto inc minor").action(ArgAction::SetTrue))
         .arg(arg!(--patch         "auto inc patch").action(ArgAction::SetTrue))
@@ -25,15 +25,13 @@ fn main() {
         )
         .arg(
             arg!(--"spec-in" <SPEC_IN> "some special input argument")
-                .required(false)
                 .value_parser(value_parser!(PathBuf))
                 .group("input"),
         )
         // Now let's assume we have a -c [config] argument which requires one of
         // (but **not** both) the "input" arguments
         .arg(
             arg!(config: -c <CONFIG>)
-                .required(false)
                 .value_parser(value_parser!(PathBuf))
                 .requires("input"),
         )

diff --git a/examples/tutorial_builder/04_04_custom.rs b/examples/tutorial_builder/04_04_custom.rs
@@ -7,7 +7,7 @@ fn main() {
     // Create application like normal
     let mut cmd = command!() // requires `cargo` feature
         // Add the version arguments
-        .arg(arg!(--"set-ver" <VER> "set version manually").required(false))
+        .arg(arg!(--"set-ver" <VER> "set version manually"))
         .arg(arg!(--major         "auto inc major").action(ArgAction::SetTrue))
         .arg(arg!(--minor         "auto inc minor").action(ArgAction::SetTrue))
         .arg(arg!(--patch         "auto inc patch").action(ArgAction::SetTrue))
@@ -16,16 +16,11 @@ fn main() {
         .arg(arg!([INPUT_FILE] "some regular input").value_parser(value_parser!(PathBuf)))
         .arg(
             arg!(--"spec-in" <SPEC_IN> "some special input argument")
-                .required(false)
                 .value_parser(value_parser!(PathBuf)),
         )
         // Now let's assume we have a -c [config] argument which requires one of
         // (but **not** both) the "input" arguments
-        .arg(
-            arg!(config: -c <CONFIG>)
-                .required(false)
-                .value_parser(value_parser!(PathBuf)),
-        );
+        .arg(arg!(config: -c <CONFIG>).value_parser(value_parser!(PathBuf)));
     let matches = cmd.get_matches_mut();
 
     // Let's assume the old version 1.2.3

diff --git a/src/builder/arg_group.rs b/src/builder/arg_group.rs
@@ -36,7 +36,7 @@ use crate::util::Id;
 /// ```rust
 /// # use clap::{Command, arg, ArgGroup, error::ErrorKind};
 /// let result = Command::new("cmd")
-///     .arg(arg!(--"set-ver" <ver> "set the version manually").required(false))
+///     .arg(arg!(--"set-ver" <ver> "set the version manually"))
 ///     .arg(arg!(--major           "auto increase major"))
 ///     .arg(arg!(--minor           "auto increase minor"))
 ///     .arg(arg!(--patch           "auto increase patch"))
@@ -54,7 +54,7 @@ use crate::util::Id;
 /// ```rust
 /// # use clap::{Command, arg, ArgGroup, Id};
 /// let result = Command::new("cmd")
-///     .arg(arg!(--"set-ver" <ver> "set the version manually").required(false))
+///     .arg(arg!(--"set-ver" <ver> "set the version manually"))
 ///     .arg(arg!(--major           "auto increase major"))
 ///     .arg(arg!(--minor           "auto increase minor"))
 ///     .arg(arg!(--patch           "auto increase patch"))

diff --git a/src/builder/command.rs b/src/builder/command.rs
@@ -923,8 +923,8 @@ impl Command {
     /// # use clap::{Command, arg};
     /// let cmd = Command::new("cmd")
     ///   .ignore_errors(true)
-    ///   .arg(arg!(-c --config <FILE> "Sets a custom config file").required(false))
-    ///   .arg(arg!(-x --stuff <FILE> "Sets a custom stuff file").required(false))
+    ///   .arg(arg!(-c --config <FILE> "Sets a custom config file"))
+    ///   .arg(arg!(-x --stuff <FILE> "Sets a custom stuff file"))
     ///   .arg(arg!(f: -f "Flag"));
     ///
     /// let r = cmd.try_get_matches_from(vec!["cmd", "-c", "file", "-f", "-x"]);

diff --git a/src/macros.rs b/src/macros.rs
@@ -295,7 +295,9 @@ macro_rules! arg_impl {
 
                 let mut arg = $arg;
 
-                arg = arg.required(true);
+                if arg.get_long().is_none() && arg.get_short().is_none() {
+                    arg = arg.required(true);
+                }
 
                 let value_name = $crate::arg_impl! { @string $value_name };
                 if arg.get_id() == "" {
@@ -322,7 +324,9 @@ macro_rules! arg_impl {
 
                 let mut arg = $arg;
 
-                arg = arg.required(true);
+                if arg.get_long().is_none() && arg.get_short().is_none() {
+                    arg = arg.required(true);
+                }
 
                 let value_name = $crate::arg_impl! { @string $value_name };
                 if arg.get_id() == "" {

diff --git a/src/parser/matches/arg_matches.rs b/src/parser/matches/arg_matches.rs
@@ -376,11 +376,9 @@ impl ArgMatches {
     ///
     /// let m = Command::new("myprog")
     ///     .arg(arg!(--color <when>)
-    ///         .value_parser(["auto", "always", "never"])
-    ///         .required(false))
+    ///         .value_parser(["auto", "always", "never"]))
     ///     .arg(arg!(--config <path>)
-    ///         .value_parser(value_parser!(std::path::PathBuf))
-    ///         .required(false))
+    ///         .value_parser(value_parser!(std::path::PathBuf)))
     ///     .get_matches_from(["myprog", "--config=config.toml", "--color=auto"]);
     /// assert_eq!(m.ids().len(), 2);
     /// assert_eq!(
@@ -1168,11 +1166,9 @@ pub(crate) struct SubCommand {
 ///
 /// let m = Command::new("myprog")
 ///     .arg(arg!(--color <when>)
-///         .value_parser(["auto", "always", "never"])
-///         .required(false))
+///         .value_parser(["auto", "always", "never"]))
 ///     .arg(arg!(--config <path>)
-///         .value_parser(value_parser!(std::path::PathBuf))
-///         .required(false))
+///         .value_parser(value_parser!(std::path::PathBuf)))
 ///     .get_matches_from(["myprog", "--config=config.toml", "--color=auto"]);
 /// assert_eq!(
 ///     m.ids()

diff --git a/tests/builder/app_settings.rs b/tests/builder/app_settings.rs
@@ -108,7 +108,7 @@ fn arg_required_else_help_over_req_subcommand() {
 fn arg_required_else_help_with_default() {
     let result = Command::new("arg_required")
         .arg_required_else_help(true)
-        .arg(arg!(--input <PATH>).required(false).default_value("-"))
+        .arg(arg!(--input <PATH>).default_value("-"))
         .try_get_matches_from(vec![""]);
 
     assert!(result.is_err());
@@ -298,9 +298,7 @@ Options:
         .version("1.3")
         .hide_possible_values(true)
         .args(&[
-            arg!(-o --opt <opt> "some option")
-                .required(false)
-                .value_parser(["one", "two"]),
+            arg!(-o --opt <opt> "some option").value_parser(["one", "two"]),
             arg!([arg1] "some pos arg").value_parser(["three", "four"]),
         ]);
 
@@ -311,10 +309,7 @@ Options:
 fn stop_delim_values_only_pos_follows() {
     let r = Command::new("onlypos")
         .dont_delimit_trailing_values(true)
-        .args(&[
-            arg!(f: -f <flag> "some opt").required(false),
-            arg!([arg] ... "some arg"),
-        ])
+        .args(&[arg!(f: -f <flag> "some opt"), arg!([arg] ... "some arg")])
         .try_get_matches_from(vec!["", "--", "-f", "-g,x"]);
     assert!(r.is_ok(), "{}", r.unwrap_err());
     let m = r.unwrap();
@@ -843,7 +838,11 @@ fn missing_positional_hyphen_req_error() {
 #[test]
 fn issue_1066_allow_leading_hyphen_and_unknown_args_option() {
     let res = Command::new("prog")
-        .arg(arg!(--"some-argument" <val>).allow_hyphen_values(true))
+        .arg(
+            arg!(--"some-argument" <val>)
+                .required(true)
+                .allow_hyphen_values(true),
+        )
         .try_get_matches_from(vec!["prog", "-fish"]);
 
     assert!(res.is_err());
@@ -1027,7 +1026,11 @@ fn aaos_flags_mult() {
 fn aaos_opts() {
     // opts
     let res = Command::new("posix")
-        .arg(arg!(--opt <val> "some option").action(ArgAction::Set))
+        .arg(
+            arg!(--opt <val> "some option")
+                .required(true)
+                .action(ArgAction::Set),
+        )
         .try_get_matches_from(vec!["", "--opt=some", "--opt=other"]);
     assert!(res.is_ok(), "{}", res.unwrap_err());
     let m = res.unwrap();
@@ -1042,14 +1045,9 @@ fn aaos_opts() {
 fn aaos_opts_w_other_overrides() {
     // opts with other overrides
     let res = Command::new("posix")
-        .arg(
-            arg!(--opt <val> "some option")
-                .required(false)
-                .action(ArgAction::Set),
-        )
+        .arg(arg!(--opt <val> "some option").action(ArgAction::Set))
         .arg(
             arg!(--other <val> "some other option")
-                .required(false)
                 .overrides_with("opt")
                 .action(ArgAction::Set),
         )
@@ -1096,15 +1094,10 @@ fn aaos_opts_w_other_overrides_2() {
     let res = Command::new("posix")
         .arg(
             arg!(--opt <val> "some option")
-                .required(false)
                 .overrides_with("other")
                 .action(ArgAction::Set),
         )
-        .arg(
-            arg!(--other <val> "some other option")
-                .required(false)
-                .action(ArgAction::Set),
-        )
+        .arg(arg!(--other <val> "some other option").action(ArgAction::Set))
         .try_get_matches_from(vec!["", "--opt=some", "--other=test", "--opt=other"]);
     assert!(res.is_ok(), "{}", res.unwrap_err());
     let m = res.unwrap();
@@ -1266,7 +1259,11 @@ fn aaos_pos_mult() {
 #[test]
 fn aaos_option_use_delim_false() {
     let m = Command::new("posix")
-        .arg(arg!(--opt <val> "some option").action(ArgAction::Set))
+        .arg(
+            arg!(--opt <val> "some option")
+                .required(true)
+                .action(ArgAction::Set),
+        )
         .try_get_matches_from(vec!["", "--opt=some,other", "--opt=one,two"])
         .unwrap();
     assert!(m.contains_id("opt"));