Mirrors/clap
2
0
Fork 0
mirror of https://github.com/clap-rs/clap synced 2025-03-04 07:17:26 +00:00
Code Issues Projects Releases Packages Wiki Activity

fix(usage): Make dont_collapse_args_in_usage the default

Browse source 
The setting was added to resolve #769.  The reason it was optional is out
of concern for applications with a lot of positional arguments.  I think
those cases are rare enough that we should just push people to override
the usage.  Positional arguments are generally important enough, even if
optional, to show.

As a side effect, this fixed some bugs with
`dont_collapse_args_in_usage` where it would repeat an argument in a
smart usage.

As a side effect, smart usage now shows `--` when it should
This commit is contained in:
Ed Page 2022-08-30 09:54:30 -05:00
parent c22b78ba61
commit 02d27b5ce3
12 changed files with 176 additions and 228 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
CHANGELOG.md
View file

@ -61,6 +61,7 @@ MSRV is now 1.60.0
Deprecated
- `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
### Features
@ -92,7 +93,8 @@ Deprecated
- *(help)* Don't rely on ALL CAPS for help headers
- *(help)* List subcommands first, focusing the emphasis on them
- *(help)* Do not include global args in `cmd help help` (#4131)
- *(help)* Use `[positional]` in argument list when relevant (#4145)
- *(help)* Use `[positional]` in list when relevant (#4145)
- *(help)* Show all `[positional]` in usage
- *(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)

4
examples/tutorial_builder/04_03_relations.md
View file

@ -25,7 +25,7 @@ error: The following required arguments were not provided:
<--set-ver <VER>|--major|--minor|--patch>
Usage:
04_03_relations[EXE] <--set-ver <VER>|--major|--minor|--patch>
04_03_relations[EXE] <--set-ver <VER>|--major|--minor|--patch> [INPUT_FILE]
For more information try --help
@ -37,7 +37,7 @@ $ 04_03_relations --major --minor
error: The argument '--major' cannot be used with '--minor'
Usage:
04_03_relations[EXE] <--set-ver <VER>|--major|--minor|--patch>
04_03_relations[EXE] <--set-ver <VER>|--major|--minor|--patch> [INPUT_FILE]
For more information try --help

4
examples/tutorial_derive/04_03_relations.md
View file

@ -25,7 +25,7 @@ error: The following required arguments were not provided:
<--set-ver <VER>|--major|--minor|--patch>
Usage:
04_03_relations_derive[EXE] <--set-ver <VER>|--major|--minor|--patch>
04_03_relations_derive[EXE] <--set-ver <VER>|--major|--minor|--patch> [INPUT_FILE]
For more information try --help
@ -37,7 +37,7 @@ $ 04_03_relations_derive --major --minor
error: The argument '--major' cannot be used with '--minor'
Usage:
04_03_relations_derive[EXE] <--set-ver <VER>|--major|--minor|--patch>
04_03_relations_derive[EXE] <--set-ver <VER>|--major|--minor|--patch> [INPUT_FILE]
For more information try --help

4
src/builder/app_settings.rs
View file

@ -43,7 +43,6 @@ pub(crate) enum AppSettings {
ArgsNegateSubcommands,
SubcommandPrecedenceOverArg,
ArgRequiredElseHelp,
DontCollapseArgsInUsage,
NextLineHelp,
DisableColoredHelp,
DisableHelpFlag,
@ -86,7 +85,6 @@ bitflags! {
const DONT_DELIM_TRAIL = 1 << 24;
const ALLOW_NEG_NUMS = 1 << 25;
const DISABLE_HELP_SC = 1 << 27;
const DONT_COLLAPSE_ARGS = 1 << 28;
const ARGS_NEGATE_SCS = 1 << 29;
const PROPAGATE_VALS_DOWN = 1 << 30;
const ALLOW_MISSING_POS = 1 << 31;
@ -130,8 +128,6 @@ impl_settings! { AppSettings, AppFlags,
=> Flags::COLOR_NEVER,
DontDelimitTrailingValues
=> Flags::DONT_DELIM_TRAIL,
DontCollapseArgsInUsage
=> Flags::DONT_COLLAPSE_ARGS,
DisableColoredHelp
=> Flags::DISABLE_COLORED_HELP,
DisableHelpSubcommand

43
src/builder/command.rs
View file

@ -1242,25 +1242,13 @@ impl Command {
}
}
/// Disables the automatic collapsing of positional args into `[ARGS]` inside the usage string.
///
/// **NOTE:** This choice is propagated to all child subcommands.
///
/// # Examples
///
/// ```no_run
/// # use clap::{Command, Arg};
/// Command::new("myprog")
/// .dont_collapse_args_in_usage(true)
/// .get_matches();
/// ```
#[inline]
pub fn dont_collapse_args_in_usage(self, yes: bool) -> Self {
if yes {
self.global_setting(AppSettings::DontCollapseArgsInUsage)
} else {
self.unset_global_setting(AppSettings::DontCollapseArgsInUsage)
}
#[doc(hidden)]
#[cfg_attr(
feature = "deprecated",
deprecated(since = "4.0.0", note = "This is now the default")
)]
pub fn dont_collapse_args_in_usage(self, _yes: bool) -> Self {
self
}
/// Tells `clap` *not* to print possible values when displaying help information.
@ -3574,9 +3562,13 @@ impl Command {
self.is_set(AppSettings::HelpExpected)
}
/// Report whether [`Command::dont_collapse_args_in_usage`] is set
#[doc(hidden)]
#[cfg_attr(
feature = "deprecated",
deprecated(since = "4.0.0", note = "This is now the default")
)]
pub fn is_dont_collapse_args_in_usage_set(&self) -> bool {
self.is_set(AppSettings::DontCollapseArgsInUsage)
true
}
/// Report whether [`Command::infer_long_args`] is set
@ -3808,11 +3800,6 @@ impl Command {
}
// Figure out implied settings
if a.is_last_set() {
// if an arg has `Last` set, we need to imply DontCollapseArgsInUsage so that args
// in the usage string don't get confused or left out.
self.settings.set(AppSettings::DontCollapseArgsInUsage);
}
a._build();
if hide_pv && a.is_takes_value_set() {
a.settings.set(ArgSettings::HidePossibleValues);
@ -4305,10 +4292,6 @@ impl Command {
!self.args.is_empty()
}
pub(crate) fn has_positionals(&self) -> bool {
self.args.keys().any(|x| x.is_position())
}
pub(crate) fn has_visible_subcommands(&self) -> bool {
self.subcommands
.iter()

289
src/output/usage.rs
View file

@ -1,11 +1,10 @@
// Internal
use crate::builder::StyledStr;
use crate::builder::{Arg, ArgPredicate, Command};
use crate::builder::{ArgPredicate, Command};
use crate::parser::ArgMatcher;
use crate::util::ChildGraph;
use crate::util::FlatSet;
use crate::util::Id;
use crate::INTERNAL_ERROR_MSG;
static DEFAULT_SUB_VALUE_NAME: &str = "SUBCOMMAND";
@ -65,46 +64,7 @@ impl<'cmd> Usage<'cmd> {
styled.placeholder(" [OPTIONS]");
}
let allow_missing_positional = self.cmd.is_allow_missing_positional_set();
if !allow_missing_positional && incl_reqs {
self.write_required_usage_from(&[], None, false, &mut styled);
}
let not_req_or_hidden =
|p: &Arg| (!p.is_required_set() || p.is_last_set()) && !p.is_hide_set();
if self.cmd.get_positionals().any(not_req_or_hidden) {
if let Some(args_tag) = self.get_optional_args(incl_reqs) {
styled.placeholder(&*args_tag);
} else {
styled.placeholder(" [ARGS]");
}
let has_last = self.cmd.get_positionals().any(|p| p.is_last_set());
if has_last && incl_reqs {
let pos = self
.cmd
.get_positionals()
.find(|p| p.is_last_set())
.expect(INTERNAL_ERROR_MSG);
debug!(
"Usage::create_help_usage: '{}' has .last(true)",
pos.get_id()
);
let req = pos.is_required_set();
if req {
styled.literal(" -- ");
} else {
styled.placeholder(" [-- ");
}
styled.extend(pos.stylized(Some(true)).into_iter());
if !req {
styled.placeholder(']');
}
}
}
if allow_missing_positional && incl_reqs {
self.write_required_usage_from(&[], None, false, &mut styled);
}
self.write_args(&[], !incl_reqs, &mut styled);
// incl_reqs is only false when this function is called recursively
if self.cmd.has_visible_subcommands() && incl_reqs
@ -155,7 +115,7 @@ impl<'cmd> Usage<'cmd> {
.unwrap_or_else(|| self.cmd.get_name()),
);
self.write_required_usage_from(used, None, true, &mut styled);
self.write_args(used, false, &mut styled);
if self.cmd.is_subcommand_required_set() {
styled.placeholder(" <");
@ -169,114 +129,6 @@ impl<'cmd> Usage<'cmd> {
styled
}
// Gets the `[ARGS]` tag for the usage string
fn get_optional_args(&self, incl_reqs: bool) -> Option<String> {
debug!("Usage::get_optional_args; incl_reqs = {:?}", incl_reqs);
let mut count = 0;
for pos in self
.cmd
.get_positionals()
.filter(|pos| !pos.is_required_set())
.filter(|pos| !pos.is_hide_set())
.filter(|pos| !pos.is_last_set())
{
debug!("Usage::get_optional_args:iter:{}", pos.get_id());
let required = self.cmd.groups_for_arg(&pos.id).any(|grp_s| {
debug!(
"Usage::get_optional_args:iter:{:?}:iter:{:?}",
pos.get_id(),
grp_s
);
// if it's part of a required group we don't want to count it
self.cmd.get_groups().any(|g| g.required && (g.id == grp_s))
});
if !required {
count += 1;
debug!(
"Usage::get_optional_args:iter: {} Args not required or hidden",
count
);
}
}
if !self.cmd.is_dont_collapse_args_in_usage_set() && count > 1 {
debug!("Usage::get_optional_args:iter: More than one, returning [ARGS]");
// [ARGS]
None
} else if count == 1 && incl_reqs {
let pos = self
.cmd
.get_positionals()
.find(|pos| {
!pos.is_required_set()
&& !pos.is_hide_set()
&& !pos.is_last_set()
&& !self.cmd.groups_for_arg(&pos.id).any(|grp_s| {
debug!(
"Usage::get_optional_args:iter:{:?}:iter:{:?}",
pos.get_id(),
grp_s
);
// if it's part of a required group we don't want to count it
self.cmd.get_groups().any(|g| g.required && (g.id == grp_s))
})
})
.expect(INTERNAL_ERROR_MSG);
debug!(
"Usage::get_optional_args:iter: Exactly one, returning '{}'",
pos.get_id()
);
Some(format!(" {}", pos.stylized(Some(false))))
} else if self.cmd.is_dont_collapse_args_in_usage_set()
&& self.cmd.has_positionals()
&& incl_reqs
{
debug!("Usage::get_optional_args:iter: Don't collapse returning all");
Some(
self.cmd
.get_positionals()
.filter(|pos| !pos.is_required_set())
.filter(|pos| !pos.is_hide_set())
.filter(|pos| !pos.is_last_set())
.map(|pos| format!(" {}", pos.stylized(Some(false))))
.collect::<Vec<_>>()
.join(""),
)
} else if !incl_reqs {
debug!(
"Usage::get_optional_args:iter: incl_reqs=false, building secondary usage string"
);
let highest_req_pos = self
.cmd
.get_positionals()
.filter_map(|pos| {
if pos.is_required_set() && !pos.is_last_set() {
Some(pos.index)
} else {
None
}
})
.max()
.unwrap_or_else(|| Some(self.cmd.get_positionals().count()));
Some(
self.cmd
.get_positionals()
.filter(|pos| pos.index <= highest_req_pos)
.filter(|pos| !pos.is_required_set())
.filter(|pos| !pos.is_hide_set())
.filter(|pos| !pos.is_last_set())
.map(|pos| format!(" {}", pos.stylized(Some(false))))
.collect::<Vec<_>>()
.join(""),
)
} else {
Some("".into())
}
}
// Determines if we need the `[OPTIONS]` tag in the usage string
fn needs_options_tag(&self) -> bool {
debug!("Usage::needs_options_tag");
@ -314,22 +166,135 @@ impl<'cmd> Usage<'cmd> {
}
// Returns the required args in usage string form by fully unrolling all groups
// `incl_last`: should we include args that are Arg::Last? (i.e. `prog [foo] -- [last]). We
// can't do that for required usages being built for subcommands because it would look like:
// `prog [foo] -- [last] <subcommand>` which is totally wrong.
pub(crate) fn write_required_usage_from(
&self,
incls: &[Id],
matcher: Option<&ArgMatcher>,
incl_last: bool,
styled: &mut StyledStr,
) {
for required in self.get_required_usage_from(incls, matcher, incl_last) {
pub(crate) fn write_args(&self, incls: &[Id], force_optional: bool, styled: &mut StyledStr) {
for required in self.get_args(incls, force_optional) {
styled.none(" ");
styled.extend(required.into_iter());
}
}
pub(crate) fn get_args(&self, incls: &[Id], force_optional: bool) -> Vec<StyledStr> {
debug!("Usage::get_required_usage_from: incls={:?}", incls,);
let required_owned;
let required = if let Some(required) = self.required {
required
} else {
required_owned = self.cmd.required_graph();
&required_owned
};
let mut unrolled_reqs = Vec::new();
for a in required.iter() {
let is_relevant = |(val, req_arg): &(ArgPredicate, Id)| -> Option<Id> {
let required = match val {
ArgPredicate::Equals(_) => false,
ArgPredicate::IsPresent => true,
};
required.then(|| req_arg.clone())
};
for aa in self.cmd.unroll_arg_requires(is_relevant, a) {
// if we don't check for duplicates here this causes duplicate error messages
// see https://github.com/clap-rs/clap/issues/2770
unrolled_reqs.push(aa);
}
// always include the required arg itself. it will not be enumerated
// by unroll_requirements_for_arg.
unrolled_reqs.push(a.clone());
}
debug!(
"Usage::get_required_usage_from: unrolled_reqs={:?}",
unrolled_reqs
);
let mut required_groups_members = FlatSet::new();
let mut required_groups = FlatSet::new();
for req in unrolled_reqs.iter().chain(incls.iter()) {
if self.cmd.find_group(req).is_some() {
let group_members = self.cmd.unroll_args_in_group(req);
let elem = self.cmd.format_group(req);
required_groups.insert(elem);
required_groups_members.extend(group_members);
} else {
debug_assert!(self.cmd.find(req).is_some());
}
}
let mut required_opts = FlatSet::new();
let mut required_positionals = Vec::new();
for req in unrolled_reqs.iter().chain(incls.iter()) {
if let Some(arg) = self.cmd.find(req) {
if required_groups_members.contains(arg.get_id()) {
continue;
}
let stylized = arg.stylized(Some(!force_optional));
if let Some(index) = arg.get_index() {
let new_len = index + 1;
if required_positionals.len() < new_len {
required_positionals.resize(new_len, None);
}
required_positionals[index] = Some(stylized);
} else {
required_opts.insert(stylized);
}
} else {
debug_assert!(self.cmd.find_group(req).is_some());
}
}
for pos in self.cmd.get_positionals() {
if pos.is_hide_set() {
continue;
}
if required_groups_members.contains(pos.get_id()) {
continue;
}
let index = pos.get_index().unwrap();
let new_len = index + 1;
if required_positionals.len() < new_len {
required_positionals.resize(new_len, None);
}
if required_positionals[index].is_some() {
if pos.is_last_set() {
let styled = required_positionals[index].take().unwrap();
let mut new = StyledStr::new();
new.literal("-- ");
new.extend(styled.into_iter());
required_positionals[index] = Some(new);
}
} else {
let mut styled;
if pos.is_last_set() {
styled = StyledStr::new();
styled.literal("[-- ");
styled.extend(pos.stylized(Some(true)).into_iter());
styled.literal("]");
} else {
styled = pos.stylized(Some(false));
}
required_positionals[index] = Some(styled);
}
if pos.is_last_set() && force_optional {
required_positionals[index] = None;
}
}
let mut ret_val = Vec::new();
if !force_optional {
ret_val.extend(required_opts);
ret_val.extend(required_groups);
}
for pos in required_positionals.into_iter().flatten() {
ret_val.push(pos);
}
debug!("Usage::get_required_usage_from: ret_val={:?}", ret_val);
ret_val
}
pub(crate) fn get_required_usage_from(
&self,
incls: &[Id],

13
tests/builder/app_settings.rs
View file

@ -552,14 +552,11 @@ fn disable_help_subcommand() {
#[test]
fn dont_collapse_args() {
let cmd = Command::new("clap-test")
.version("v1.4.8")
.dont_collapse_args_in_usage(true)
.args(&[
Arg::new("arg1").help("some"),
Arg::new("arg2").help("some"),
Arg::new("arg3").help("some"),
]);
let cmd = Command::new("clap-test").version("v1.4.8").args(&[
Arg::new("arg1").help("some"),
Arg::new("arg2").help("some"),
Arg::new("arg3").help("some"),
]);
utils::assert_output(cmd, "clap-test --help", DONT_COLLAPSE_ARGS, false);
}

8
tests/builder/conflicts.rs
View file

@ -293,7 +293,7 @@ fn conflict_output() {
static CONFLICT_ERR: &str = "error: The argument '--flag...' cannot be used with '-F'
Usage:
clap-test --flag... --long-option-2 <option2> <positional> <positional2>
clap-test --flag... --long-option-2 <option2> <positional> <positional2> [positional3]...
For more information try --help
";
@ -311,7 +311,7 @@ fn conflict_output_rev() {
static CONFLICT_ERR_REV: &str = "error: The argument '-F' cannot be used with '--flag...'
Usage:
clap-test -F --long-option-2 <option2> <positional> <positional2>
clap-test -F --long-option-2 <option2> <positional> <positional2> [positional3]...
For more information try --help
";
@ -329,7 +329,7 @@ fn conflict_output_with_required() {
static CONFLICT_ERR: &str = "error: The argument '--flag...' cannot be used with '-F'
Usage:
clap-test --flag... --long-option-2 <option2> <positional> <positional2>
clap-test --flag... --long-option-2 <option2> <positional> <positional2> [positional3]...
For more information try --help
";
@ -347,7 +347,7 @@ fn conflict_output_rev_with_required() {
static CONFLICT_ERR_REV: &str = "error: The argument '-F' cannot be used with '--flag...'
Usage:
clap-test -F --long-option-2 <option2> <positional> <positional2>
clap-test -F --long-option-2 <option2> <positional> <positional2> [positional3]...
For more information try --help
";

10
tests/builder/help.rs
View file

@ -205,7 +205,7 @@ Kevin K. <kbknapp@gmail.com>
tests clap library
Usage:
clap-test [OPTIONS] [ARGS] [SUBCOMMAND]
clap-test [OPTIONS] [positional] [positional2] [positional3]... [SUBCOMMAND]
Subcommands:
subcmd tests subcommands
@ -954,7 +954,7 @@ foo
bar
Usage:
myapp [OPTIONS] [ARGS]
myapp [OPTIONS] [arg1] [arg2]...
Arguments:
[arg1] some option
@ -1389,7 +1389,7 @@ fn last_arg_mult_usage_req_with_sc() {
Usage:
last <TARGET> [CORPUS] -- <ARGS>...
last <SUBCOMMAND>
last [TARGET] [CORPUS] <SUBCOMMAND>
Subcommands:
test some
@ -2295,7 +2295,7 @@ Options:
static CUSTOM_HEADING_POS: &str = "test 1.4
Usage:
test [ARGS]
test [gear] [speed]
Arguments:
[gear] Which gear
@ -2428,7 +2428,7 @@ fn missing_positional_final_multiple() {
"test
Usage:
test [ARGS]
test [foo] [bar] [baz]...
Arguments:
[foo]

2
tests/builder/opts.rs
View file

@ -11,7 +11,7 @@ static DYM: &str =
\tIf you tried to supply `--optio` as a value rather than a flag, use `-- --optio`
Usage:
clap-test --option <opt>...
clap-test --option <opt>... [positional] [positional2] [positional3]...
For more information try --help
";

15
tests/builder/positionals.rs
View file

@ -211,13 +211,13 @@ fn positional_hyphen_does_not_panic() {
#[test]
fn single_positional_usage_string() {
let mut cmd = Command::new("test").arg(arg!([FILE] "some file"));
assert_eq!(cmd.render_usage(), "Usage:\n test [FILE]");
crate::utils::assert_eq(cmd.render_usage(), "Usage:\n test [FILE]");
}
#[test]
fn single_positional_multiple_usage_string() {
let mut cmd = Command::new("test").arg(arg!([FILE]... "some file"));
assert_eq!(cmd.render_usage(), "Usage:\n test [FILE]...");
crate::utils::assert_eq(cmd.render_usage(), "Usage:\n test [FILE]...");
}
#[test]
@ -225,7 +225,12 @@ fn multiple_positional_usage_string() {
let mut cmd = Command::new("test")
.arg(arg!([FILE] "some file"))
.arg(arg!([FILES]... "some file"));
assert_eq!(cmd.render_usage(), "Usage:\n test [ARGS]");
crate::utils::assert_eq(
cmd.render_usage(),
"\
Usage:
test [FILE] [FILES]...",
);
}
#[test]
@ -233,13 +238,13 @@ fn multiple_positional_one_required_usage_string() {
let mut cmd = Command::new("test")
.arg(arg!(<FILE> "some file"))
.arg(arg!([FILES]... "some file"));
assert_eq!(cmd.render_usage(), "Usage:\n test <FILE> [FILES]...");
crate::utils::assert_eq(cmd.render_usage(), "Usage:\n test <FILE> [FILES]...");
}
#[test]
fn single_positional_required_usage_string() {
let mut cmd = Command::new("test").arg(arg!(<FILE> "some file"));
assert_eq!(cmd.render_usage(), "Usage:\n test <FILE>");
crate::utils::assert_eq(cmd.render_usage(), "Usage:\n test <FILE>");
}
// This tests a programmer error and will only succeed with debug_assertions

8
tests/builder/require.rs
View file

@ -37,7 +37,7 @@ static MISSING_REQ: &str = "error: The following required arguments were not pro
<positional2>
Usage:
clap-test --long-option-2 <option2> -F <positional> <positional2>
clap-test --long-option-2 <option2> -F <positional> <positional2> [positional3]...
For more information try --help
";
@ -142,7 +142,7 @@ static POSITIONAL_REQ: &str = "error: The following required arguments were not
<opt>
Usage:
clap-test <flag> <opt>
clap-test <flag> <opt> [bar]
For more information try --help
";
@ -161,7 +161,7 @@ static POSITIONAL_REQ_IF_NO_VAL: &str = "error: The following required arguments
<flag>
Usage:
clap-test <flag>
clap-test <flag> [opt] [bar]
For more information try --help
";
@ -182,7 +182,7 @@ static POSITIONAL_REQ_IF_VAL: &str = "error: The following required arguments we
<opt>
Usage:
clap-test <flag> <foo> <opt>
clap-test <flag> <foo> <opt> [bar]
For more information try --help
";