Skip to content

Commit

Permalink
Merge pull request #4385 from epage/errors
Browse files Browse the repository at this point in the history
fix(error): Be consistent with rustc diagnostic guidelines
  • Loading branch information
epage committed Jan 3, 2023
2 parents 7229047 + f1ffc63 commit e7d58b3
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 45 deletions.
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
49 changes: 37 additions & 12 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 @@ -409,15 +426,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 @@ -426,7 +447,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 @@ -659,9 +660,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 @@ -677,12 +678,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 @@ -714,11 +715,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/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
4 changes: 2 additions & 2 deletions tests/builder/opts.rs
Expand Up @@ -448,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 @@ -546,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 @@ -519,9 +517,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

0 comments on commit e7d58b3

Please sign in to comment.