Skip to content

Commit

Permalink
fix: Make arg!(--flag <value>) optional by default
Browse files Browse the repository at this point in the history
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
  • Loading branch information
epage committed Sep 12, 2022
1 parent 75a73f3 commit c9eef44
Show file tree
Hide file tree
Showing 31 changed files with 177 additions and 301 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions clap_bench/benches/02_simple.rs
Expand Up @@ -8,7 +8,7 @@ macro_rules! create_app {
.about("tests clap library")
.author("Kevin K. <kbknapp@gmail.com>")
.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"))
}};
}
Expand All @@ -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)
})
});
Expand Down
12 changes: 5 additions & 7 deletions clap_bench/benches/03_complex.rs
Expand Up @@ -10,36 +10,34 @@ macro_rules! create_app {
.version("0.1")
.about("tests clap library")
.author("Kevin K. <kbknapp@gmail.com>")
.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. <kbknapp@gmail.com>")
.arg(arg!(-o --option <scoption> ... "tests options").required(false))
.arg(arg!(-o --option <scoption> ... "tests options"))
.arg(arg!([scpositional] "tests positionals"))
)
}};
Expand Down
1 change: 0 additions & 1 deletion clap_mangen/examples/man.rs
Expand Up @@ -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"),
)
Expand Down
1 change: 0 additions & 1 deletion examples/cargo-example.rs
Expand Up @@ -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)),
),
);
Expand Down
6 changes: 1 addition & 5 deletions examples/escaped-positional.rs
Expand Up @@ -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])
Expand Down
2 changes: 1 addition & 1 deletion examples/find.md
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion examples/git.rs
Expand Up @@ -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() {
Expand Down
4 changes: 2 additions & 2 deletions examples/tutorial_builder/02_app_settings.rs
Expand Up @@ -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!(
Expand Down
4 changes: 2 additions & 2 deletions examples/tutorial_builder/02_apps.rs
Expand Up @@ -5,8 +5,8 @@ fn main() {
.version("1.0")
.author("Kevin K. <kbknapp@gmail.com>")
.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!(
Expand Down
4 changes: 2 additions & 2 deletions examples/tutorial_builder/02_crate.rs
Expand Up @@ -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!(
Expand Down
4 changes: 1 addition & 3 deletions examples/tutorial_builder/04_03_relations.rs
Expand Up @@ -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))
Expand All @@ -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"),
)
Expand Down
9 changes: 2 additions & 7 deletions examples/tutorial_builder/04_04_custom.rs
Expand Up @@ -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))
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/builder/arg_group.rs
Expand Up @@ -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"))
Expand All @@ -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"))
Expand Down
4 changes: 2 additions & 2 deletions src/builder/command.rs
Expand Up @@ -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"]);
Expand Down
8 changes: 6 additions & 2 deletions src/macros.rs
Expand Up @@ -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() == "" {
Expand All @@ -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() == "" {
Expand Down
12 changes: 4 additions & 8 deletions src/parser/matches/arg_matches.rs
Expand Up @@ -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!(
Expand Down Expand Up @@ -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()
Expand Down
43 changes: 20 additions & 23 deletions tests/builder/app_settings.rs
Expand Up @@ -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());
Expand Down Expand Up @@ -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"]),
]);

Expand All @@ -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();
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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();
Expand All @@ -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),
)
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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"));
Expand Down

0 comments on commit c9eef44

Please sign in to comment.