Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(help): Make help output more consistent #3667

Merged
merged 11 commits into from Apr 30, 2022
1 change: 1 addition & 0 deletions Cargo.toml
Expand Up @@ -142,6 +142,7 @@ rustversion = "1"
# Cutting out `filesystem` feature
trycmd = { version = "0.13", default-features = false, features = ["color-auto", "diff", "examples"] }
humantime = "2"
snapbox = "0.2.9"

[[example]]
name = "demo"
Expand Down
297 changes: 172 additions & 125 deletions src/build/command.rs

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/error/mod.rs
Expand Up @@ -77,7 +77,7 @@ impl Error {
/// Format the existing message with the Command's context
#[must_use]
pub fn format(mut self, cmd: &mut Command) -> Self {
cmd._build();
cmd._build_self();
let usage = cmd.render_usage();
if let Some(message) = self.inner.message.as_mut() {
message.format(cmd, usage);
Expand Down
37 changes: 12 additions & 25 deletions src/parse/parser.rs
Expand Up @@ -613,42 +613,29 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
) -> ClapResult<ParseResult> {
debug!("Parser::parse_help_subcommand");

let mut bin_name = self
.cmd
.get_bin_name()
.unwrap_or_else(|| self.cmd.get_name())
.to_owned();

let mut sc = {
let mut sc = self.cmd.clone();
let mut cmd = self.cmd.clone();
let sc = {
let mut sc = &mut cmd;

for cmd in cmds {
sc = if let Some(c) = sc.find_subcommand(cmd) {
c
} else if let Some(c) = sc.find_subcommand(&cmd.to_string_lossy()) {
c
sc = if let Some(sc_name) =
sc.find_subcommand(cmd).map(|sc| sc.get_name().to_owned())
{
sc._build_subcommand(&sc_name).unwrap()
} else {
return Err(ClapError::unrecognized_subcommand(
self.cmd,
sc,
cmd.to_string_lossy().into_owned(),
self.cmd
.get_bin_name()
.unwrap_or_else(|| self.cmd.get_name())
sc.get_bin_name()
.unwrap_or_else(|| sc.get_name())
.to_owned(),
));
}
.clone();

sc._build_self();
bin_name.push(' ');
bin_name.push_str(sc.get_name());
};
}

sc
};
sc = sc.bin_name(bin_name);

let parser = Parser::new(&mut sc);
let parser = Parser::new(sc);

Err(parser.help_err(true, Stream::Stdout))
}
Expand Down
40 changes: 10 additions & 30 deletions tests/builder/app_settings.rs
Expand Up @@ -250,12 +250,12 @@ fn arg_required_else_help_error_message() {
.short('i')
.long("info"),
);
assert!(utils::compare_output(
utils::assert_output(
cmd,
"test",
ARG_REQUIRED_ELSE_HELP,
true // Unlike normal displaying of help, we should provide a fatal exit code
));
true, // Unlike normal displaying of help, we should provide a fatal exit code
);
}

#[test]
Expand All @@ -281,12 +281,12 @@ fn subcommand_required_else_help_error_message() {
.setting(AppSettings::SubcommandRequiredElseHelp)
.version("1.0")
.subcommand(Command::new("info").arg(Arg::new("filename")));
assert!(utils::compare_output(
utils::assert_output(
cmd,
"test",
SUBCOMMAND_REQUIRED_ELSE_HELP,
true // Unlike normal displaying of help, we should provide a fatal exit code
));
true, // Unlike normal displaying of help, we should provide a fatal exit code
);
}

#[cfg(not(feature = "suggestions"))]
Expand Down Expand Up @@ -419,12 +419,7 @@ fn skip_possible_values() {
arg!([arg1] "some pos arg").possible_values(["three", "four"]),
]);

assert!(utils::compare_output(
cmd,
"test --help",
SKIP_POS_VALS,
false
));
utils::assert_output(cmd, "test --help", SKIP_POS_VALS, false);
}

#[test]
Expand Down Expand Up @@ -624,12 +619,7 @@ fn dont_collapse_args() {
Arg::new("arg2").help("some"),
Arg::new("arg3").help("some"),
]);
assert!(utils::compare_output(
cmd,
"clap-test --help",
DONT_COLLAPSE_ARGS,
false
));
utils::assert_output(cmd, "clap-test --help", DONT_COLLAPSE_ARGS, false);
}

#[test]
Expand All @@ -643,12 +633,7 @@ fn require_eq() {
.value_name("FILE")
.help("some"),
);
assert!(utils::compare_output(
cmd,
"clap-test --help",
REQUIRE_EQUALS,
false
));
utils::assert_output(cmd, "clap-test --help", REQUIRE_EQUALS, false);
}

#[test]
Expand Down Expand Up @@ -863,12 +848,7 @@ fn issue_1093_allow_ext_sc() {
let cmd = Command::new("clap-test")
.version("v1.4.8")
.allow_external_subcommands(true);
assert!(utils::compare_output(
cmd,
"clap-test --help",
ALLOW_EXT_SC,
false
));
utils::assert_output(cmd, "clap-test --help", ALLOW_EXT_SC, false);
}

#[test]
Expand Down
14 changes: 2 additions & 12 deletions tests/builder/arg_aliases.rs
Expand Up @@ -168,12 +168,7 @@ fn invisible_arg_aliases_help_output() {
)
.arg(arg!(-f - -flag).aliases(&["unseeable", "flg1", "anyway"])),
);
assert!(utils::compare_output(
cmd,
"ct test --help",
SC_INVISIBLE_ALIAS_HELP,
false
));
utils::assert_output(cmd, "ct test --help", SC_INVISIBLE_ALIAS_HELP, false);
}

#[test]
Expand All @@ -197,10 +192,5 @@ fn visible_arg_aliases_help_output() {
.visible_aliases(&["v_flg", "flag2", "flg3"]),
),
);
assert!(utils::compare_output(
cmd,
"ct test --help",
SC_VISIBLE_ALIAS_HELP,
false
));
utils::assert_output(cmd, "ct test --help", SC_VISIBLE_ALIAS_HELP, false);
}
14 changes: 2 additions & 12 deletions tests/builder/arg_aliases_short.rs
Expand Up @@ -164,12 +164,7 @@ fn invisible_short_arg_aliases_help_output() {
)
.arg(arg!(-f - -flag).short_aliases(&['x', 'y', 'z'])),
);
assert!(utils::compare_output(
cmd,
"ct test --help",
SC_INVISIBLE_ALIAS_HELP,
false
));
utils::assert_output(cmd, "ct test --help", SC_INVISIBLE_ALIAS_HELP, false);
}

#[test]
Expand All @@ -194,10 +189,5 @@ fn visible_short_arg_aliases_help_output() {
.visible_short_aliases(&['a', 'b', '🦆']),
),
);
assert!(utils::compare_output(
cmd,
"ct test --help",
SC_VISIBLE_ALIAS_HELP,
false
));
utils::assert_output(cmd, "ct test --help", SC_VISIBLE_ALIAS_HELP, false);
}
20 changes: 10 additions & 10 deletions tests/builder/conflicts.rs
Expand Up @@ -248,42 +248,42 @@ fn required_group_conflicts_with_arg() {

#[test]
fn conflict_output() {
assert!(utils::compare_output(
utils::assert_output(
utils::complex_app(),
"clap-test val1 fa --flag --long-option-2 val2 -F",
CONFLICT_ERR,
true,
));
);
}

#[test]
fn conflict_output_rev() {
assert!(utils::compare_output(
utils::assert_output(
utils::complex_app(),
"clap-test val1 fa -F --long-option-2 val2 --flag",
CONFLICT_ERR_REV,
true,
));
);
}

#[test]
fn conflict_output_with_required() {
assert!(utils::compare_output(
utils::assert_output(
utils::complex_app(),
"clap-test val1 --flag --long-option-2 val2 -F",
CONFLICT_ERR,
true,
));
);
}

#[test]
fn conflict_output_rev_with_required() {
assert!(utils::compare_output(
utils::assert_output(
utils::complex_app(),
"clap-test val1 -F --long-option-2 val2 --flag",
CONFLICT_ERR_REV,
true,
));
);
}

#[test]
Expand All @@ -304,12 +304,12 @@ fn conflict_output_three_conflicting() {
.long("three")
.conflicts_with_all(&["one", "two"]),
);
assert!(utils::compare_output(
utils::assert_output(
cmd,
"three_conflicting_arguments --one --two --three",
CONFLICT_ERR_THREE,
true,
));
);
}

#[test]
Expand Down
4 changes: 2 additions & 2 deletions tests/builder/default_vals.rs
Expand Up @@ -577,7 +577,7 @@ fn default_vals_donnot_show_in_smart_usage() {
)
.arg(Arg::new("input").required(true));

assert!(utils::compare_output(
utils::assert_output(
cmd,
"bug",
"error: The following required arguments were not provided:
Expand All @@ -589,7 +589,7 @@ USAGE:
For more information try --help
",
true,
));
);
}

#[test]
Expand Down
44 changes: 12 additions & 32 deletions tests/builder/derive_order.rs
Expand Up @@ -99,12 +99,7 @@ fn no_derive_order() {
.help("second option"),
]);

assert!(utils::compare_output(
cmd,
"test --help",
NO_DERIVE_ORDER,
false
));
utils::assert_output(cmd, "test --help", NO_DERIVE_ORDER, false);
}

#[test]
Expand All @@ -125,12 +120,7 @@ fn derive_order() {
.help("second option"),
]);

assert!(utils::compare_output(
cmd,
"test --help",
UNIFIED_HELP_AND_DERIVE,
false
));
utils::assert_output(cmd, "test --help", UNIFIED_HELP_AND_DERIVE, false);
}

#[test]
Expand Down Expand Up @@ -169,7 +159,7 @@ OPTIONS:
.help("first option"),
);

assert!(utils::compare_output(cmd, "test --help", HELP, false));
utils::assert_output(cmd, "test --help", HELP, false);
}

#[test]
Expand Down Expand Up @@ -207,7 +197,7 @@ OPTIONS:
.help("second option"),
);

assert!(utils::compare_output(cmd, "test --help", HELP, false));
utils::assert_output(cmd, "test --help", HELP, false);
}

#[test]
Expand All @@ -229,12 +219,7 @@ fn derive_order_subcommand_propagate() {
]),
);

assert!(utils::compare_output(
cmd,
"test sub --help",
UNIFIED_DERIVE_SC_PROP,
false
));
utils::assert_output(cmd, "test sub --help", UNIFIED_DERIVE_SC_PROP, false);
}

#[test]
Expand All @@ -259,12 +244,12 @@ fn derive_order_subcommand_propagate_with_explicit_display_order() {
]),
);

assert!(utils::compare_output(
utils::assert_output(
cmd,
"test sub --help",
UNIFIED_DERIVE_SC_PROP_EXPLICIT_ORDER,
false
));
false,
);
}

#[test]
Expand All @@ -281,12 +266,7 @@ fn prefer_user_help_with_derive_order() {
Arg::new("flag_a").long("flag_a").help("second flag"),
]);

assert!(utils::compare_output(
cmd,
"test --help",
PREFER_USER_HELP_DERIVE_ORDER,
false
));
utils::assert_output(cmd, "test --help", PREFER_USER_HELP_DERIVE_ORDER, false);
}

#[test]
Expand All @@ -304,10 +284,10 @@ fn prefer_user_help_in_subcommand_with_derive_order() {
]),
);

assert!(utils::compare_output(
utils::assert_output(
cmd,
"test sub --help",
PREFER_USER_HELP_SUBCMD_DERIVE_ORDER,
false
));
false,
);
}