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(error): Be consistent with rustc diagnostic guidelines #4385

Merged
merged 2 commits into from Jan 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/derive_ref/interop_tests.md
Expand Up @@ -142,7 +142,7 @@ $ interop_hand_subcommand add --unknown
? failed
error: Found argument '--unknown' which wasn't expected, or isn't valid in this context

If you tried to supply '--unknown' as a value rather than a flag, use '-- --unknown'
note: to pass '--unknown' as a value, use '-- --unknown'

Usage: interop_hand_subcommand[EXE] add [NAME]...

Expand Down
51 changes: 38 additions & 13 deletions src/error/format.rs
Expand Up @@ -64,23 +64,40 @@ impl ErrorFormatter for RichFormatter {
}
}

let mut suggested = false;
if let Some(valid) = error.get(ContextKind::SuggestedSubcommand) {
styled.none("\n\n");
did_you_mean(&mut styled, valid);
styled.none("\n");
if !suggested {
styled.none("\n");
suggested = true;
}
did_you_mean(&mut styled, "subcommand", valid);
}
if let Some(valid) = error.get(ContextKind::SuggestedArg) {
styled.none("\n\n");
did_you_mean(&mut styled, valid);
styled.none("\n");
if !suggested {
styled.none("\n");
suggested = true;
}
did_you_mean(&mut styled, "argument", valid);
}
if let Some(valid) = error.get(ContextKind::SuggestedValue) {
styled.none("\n\n");
did_you_mean(&mut styled, valid);
styled.none("\n");
if !suggested {
styled.none("\n");
suggested = true;
}
did_you_mean(&mut styled, "value", valid);
}
let suggestions = error.get(ContextKind::Suggested);
if let Some(ContextValue::StyledStrs(suggestions)) = suggestions {
if !suggested {
styled.none("\n");
}
for suggestion in suggestions {
styled.none("\n\n");
styled.none("\n");
styled.none(TAB);
styled.good("note: ");
styled.extend(suggestion.iter());
}
}
Expand Down Expand Up @@ -149,7 +166,7 @@ fn write_dynamic_context(error: &crate::error::Error, styled: &mut StyledStr) ->
if let Some(ContextValue::String(invalid_arg)) = invalid_arg {
styled.none("Equal sign is needed when assigning values to '");
styled.warning(invalid_arg);
styled.none("'.");
styled.none("'");
true
} else {
false
Expand Down Expand Up @@ -391,15 +408,19 @@ fn try_help(styled: &mut StyledStr, help: Option<&str>) {
}

#[cfg(feature = "error-context")]
fn did_you_mean(styled: &mut StyledStr, valid: &ContextValue) {
fn did_you_mean(styled: &mut StyledStr, context: &str, valid: &ContextValue) {
if let ContextValue::String(valid) = valid {
styled.none(TAB);
styled.none("Did you mean '");
styled.good("note: ");
styled.none(context);
styled.none(" '");
styled.good(valid);
styled.none("'?");
styled.none("' exists");
} else if let ContextValue::Strings(valid) = valid {
styled.none(TAB);
styled.none("Did you mean ");
styled.good("note: ");
styled.none(context);
styled.none(" ");
for (i, valid) in valid.iter().enumerate() {
if i != 0 {
styled.none(", ");
Expand All @@ -408,7 +429,11 @@ fn did_you_mean(styled: &mut StyledStr, valid: &ContextValue) {
styled.good(valid);
styled.none("'");
}
styled.none("?");
if valid.len() == 1 {
styled.none(" exists");
} else {
styled.none(" exist");
}
}
}

Expand Down
25 changes: 13 additions & 12 deletions src/error/mod.rs
Expand Up @@ -431,8 +431,9 @@ impl<F: ErrorFormatter> Error<F> {
#[cfg(feature = "error-context")]
{
let mut styled_suggestion = StyledStr::new();
styled_suggestion
.none("If you believe you received this message in error, try re-running with '");
styled_suggestion.none("to pass '");
styled_suggestion.warning(&subcmd);
styled_suggestion.none("' as a value, use '");
styled_suggestion.good(name);
styled_suggestion.good(" -- ");
styled_suggestion.good(&subcmd);
Expand Down Expand Up @@ -655,9 +656,9 @@ impl<F: ErrorFormatter> Error<F> {
let mut suggestions = vec![];
if suggested_trailing_arg {
let mut styled_suggestion = StyledStr::new();
styled_suggestion.none("If you tried to supply '");
styled_suggestion.none("to pass '");
styled_suggestion.warning(&arg);
styled_suggestion.none("' as a value rather than a flag, use '");
styled_suggestion.none("' as a value, use '");
styled_suggestion.good("-- ");
styled_suggestion.good(&arg);
styled_suggestion.none("'");
Expand All @@ -673,12 +674,12 @@ impl<F: ErrorFormatter> Error<F> {
match did_you_mean {
Some((flag, Some(sub))) => {
let mut styled_suggestion = StyledStr::new();
styled_suggestion.none("Did you mean to put '");
styled_suggestion.none("'");
styled_suggestion.good(sub);
styled_suggestion.none(" ");
styled_suggestion.good("--");
styled_suggestion.good(flag);
styled_suggestion.none("' after the subcommand '");
styled_suggestion.good(sub);
styled_suggestion.none("'?");
styled_suggestion.none("' exists");
suggestions.push(styled_suggestion);
}
Some((flag, None)) => {
Expand Down Expand Up @@ -710,11 +711,11 @@ impl<F: ErrorFormatter> Error<F> {
#[cfg(feature = "error-context")]
{
let mut styled_suggestion = StyledStr::new();
styled_suggestion.none("If you tried to supply '");
styled_suggestion.warning(&arg);
styled_suggestion.none("' as a subcommand, remove the '");
styled_suggestion.none("subcommand '");
styled_suggestion.good(&arg);
styled_suggestion.none("' exists; to use it, remove the '");
styled_suggestion.warning("--");
styled_suggestion.none("' before it.");
styled_suggestion.none("' before it");

err = err.extend_context_unchecked([
(ContextKind::InvalidArg, ContextValue::String(arg)),
Expand Down
2 changes: 1 addition & 1 deletion tests/builder/empty_values.rs
Expand Up @@ -118,7 +118,7 @@ fn no_empty_values_without_equals_but_requires_equals() {
assert_eq!(m.unwrap_err().kind(), ErrorKind::NoEquals);

static NO_EUQALS_ERROR: &str =
"error: Equal sign is needed when assigning values to '--config=<config>'.
"error: Equal sign is needed when assigning values to '--config=<config>'

Usage: config [OPTIONS]

Expand Down
2 changes: 1 addition & 1 deletion tests/builder/error.rs
Expand Up @@ -141,7 +141,7 @@ fn suggest_trailing() {
static MESSAGE: &str = "\
error: Found argument '--foo' which wasn't expected, or isn't valid in this context

If you tried to supply '--foo' as a value rather than a flag, use '-- --foo'
note: to pass '--foo' as a value, use '-- --foo'

Usage: rg [PATTERN]

Expand Down
4 changes: 2 additions & 2 deletions tests/builder/flags.rs
Expand Up @@ -160,7 +160,7 @@ fn issue_1284_argument_in_flag_style() {
const USE_FLAG_AS_ARGUMENT: &str = "\
error: Found argument '--another-flag' which wasn't expected, or isn't valid in this context

If you tried to supply '--another-flag' as a value rather than a flag, use '-- --another-flag'
note: to pass '--another-flag' as a value, use '-- --another-flag'

Usage: mycat [OPTIONS] [filename]

Expand Down Expand Up @@ -204,7 +204,7 @@ fn issue_2308_multiple_dashes() {
static MULTIPLE_DASHES: &str = "\
error: Found argument '-----' which wasn't expected, or isn't valid in this context

If you tried to supply '-----' as a value rather than a flag, use '-- -----'
note: to pass '-----' as a value, use '-- -----'

Usage: test <arg>

Expand Down
7 changes: 3 additions & 4 deletions tests/builder/opts.rs
Expand Up @@ -21,8 +21,7 @@ fn require_equals_fail() {
#[test]
#[cfg(feature = "error-context")]
fn require_equals_fail_message() {
static NO_EQUALS: &str =
"error: Equal sign is needed when assigning values to '--config=<cfg>'.
static NO_EQUALS: &str = "error: Equal sign is needed when assigning values to '--config=<cfg>'

Usage: prog [OPTIONS]

Expand Down Expand Up @@ -449,7 +448,7 @@ fn did_you_mean() {
static DYM: &str = "\
error: Found argument '--optio' which wasn't expected, or isn't valid in this context

Did you mean '--option'?
note: argument '--option' exists

Usage: clap-test --option <opt>... [positional] [positional2] [positional3]...

Expand Down Expand Up @@ -547,7 +546,7 @@ fn issue_1073_suboptimal_flag_suggestion() {
static DYM_ISSUE_1073: &str = "\
error: Found argument '--files-without-matches' which wasn't expected, or isn't valid in this context

Did you mean '--files-without-match'?
note: argument '--files-without-match' exists

Usage: ripgrep-616 --files-without-match

Expand Down
8 changes: 4 additions & 4 deletions tests/builder/possible_values.rs
Expand Up @@ -181,7 +181,7 @@ fn possible_values_output() {
error: 'slo' isn't a valid value for '-O <option>'
[possible values: slow, fast, \"ludicrous speed\"]

Did you mean 'slow'?
note: value 'slow' exists

For more information try '--help'
";
Expand Down Expand Up @@ -215,7 +215,7 @@ fn possible_values_alias_output() {
error: 'slo' isn't a valid value for '-O <option>'
[possible values: slow, fast, \"ludicrous speed\"]

Did you mean 'slow'?
note: value 'slow' exists

For more information try '--help'
";
Expand Down Expand Up @@ -253,7 +253,7 @@ fn possible_values_hidden_output() {
error: 'slo' isn't a valid value for '-O <option>'
[possible values: slow, fast, \"ludicrous speed\"]

Did you mean 'slow'?
note: value 'slow' exists

For more information try '--help'
";
Expand Down Expand Up @@ -292,7 +292,7 @@ fn escaped_possible_values_output() {
error: 'ludicrous' isn't a valid value for '-O <option>'
[possible values: slow, fast, \"ludicrous speed\"]

Did you mean 'ludicrous speed'?
note: value 'ludicrous speed' exists

For more information try '--help'
";
Expand Down
19 changes: 8 additions & 11 deletions tests/builder/subcommands.rs
Expand Up @@ -100,9 +100,8 @@ fn subcmd_did_you_mean_output() {
static DYM_SUBCMD: &str = "\
error: The subcommand 'subcm' wasn't recognized

Did you mean 'subcmd'?

If you believe you received this message in error, try re-running with 'dym -- subcm'
note: subcommand 'subcmd' exists
note: to pass 'subcm' as a value, use 'dym -- subcm'

Usage: dym [COMMAND]

Expand All @@ -121,9 +120,8 @@ fn subcmd_did_you_mean_output_ambiguous() {
static DYM_SUBCMD_AMBIGUOUS: &str = "\
error: The subcommand 'te' wasn't recognized

Did you mean 'test', 'temp'?

If you believe you received this message in error, try re-running with 'dym -- te'
note: subcommand 'test', 'temp' exist
note: to pass 'te' as a value, use 'dym -- te'

Usage: dym [COMMAND]

Expand All @@ -143,7 +141,7 @@ fn subcmd_did_you_mean_output_arg() {
static EXPECTED: &str = "\
error: Found argument '--subcmarg' which wasn't expected, or isn't valid in this context

Did you mean to put '--subcmdarg' after the subcommand 'subcmd'?
note: 'subcmd --subcmdarg' exists

Usage: dym [COMMAND]

Expand Down Expand Up @@ -355,7 +353,7 @@ fn subcommand_used_after_double_dash() {
static SUBCMD_AFTER_DOUBLE_DASH: &str = "\
error: Found argument 'subcmd' which wasn't expected, or isn't valid in this context

If you tried to supply 'subcmd' as a subcommand, remove the '--' before it.
note: subcommand 'subcmd' exists; to use it, remove the '--' before it

Usage: cmd [COMMAND]

Expand Down Expand Up @@ -522,9 +520,8 @@ For more information try 'help'
static BAZ_EXPECTED: &str = "\
error: The subcommand 'baz' wasn't recognized

Did you mean 'bar'?

If you believe you received this message in error, try re-running with ' -- baz'
note: subcommand 'bar' exists
note: to pass 'baz' as a value, use ' -- baz'

Usage: <COMMAND>

Expand Down