Skip to content

Commit

Permalink
Merge pull request #3667 from epage/exe
Browse files Browse the repository at this point in the history 
fix(help): Make help output more consistent
  • Loading branch information
@epage
epage committed Apr 30, 2022
2 parents d411e7a + 8df2047 commit bd653b9
Show file tree
Hide file tree
Showing 26 changed files with 571 additions and 1,031 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
View file
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
View file

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/error/mod.rs
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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,
);
}

0 comments on commit bd653b9

Please sign in to comment.