Skip to content

Commit

Permalink
Merge pull request #4383 from epage/error
Browse files Browse the repository at this point in the history 
refactor(error): Generalize how we report suggestions
  • Loading branch information
@epage
epage authored Oct 13, 2022
2 parents bae951b + b9d2980 commit 6422046
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 86 deletions.
15 changes: 13 additions & 2 deletions src/error/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub enum ContextKind {
/// Trailing argument
TrailingArg,
/// Potential fix for the user
SuggestedTrailingArg,
Suggested,
/// A usage string
Usage,
/// An opaque message to the user
Expand All @@ -53,8 +53,8 @@ impl ContextKind {
Self::SuggestedSubcommand => Some("Suggested Subcommand"),
Self::SuggestedArg => Some("Suggested Argument"),
Self::SuggestedValue => Some("Suggested Value"),
Self::SuggestedTrailingArg => Some("Suggested Trailing Argument"),
Self::TrailingArg => Some("Trailing Argument"),
Self::Suggested => Some("Suggested"),
Self::Usage => None,
Self::Custom => None,
}
Expand Down Expand Up @@ -82,6 +82,8 @@ pub enum ContextValue {
Strings(Vec<String>),
/// A single value
StyledStr(crate::builder::StyledStr),
/// many value
StyledStrs(Vec<crate::builder::StyledStr>),
/// A single value
Number(isize),
}
Expand All @@ -94,6 +96,15 @@ impl std::fmt::Display for ContextValue {
Self::String(v) => v.fmt(f),
Self::Strings(v) => v.join(", ").fmt(f),
Self::StyledStr(v) => v.fmt(f),
Self::StyledStrs(v) => {
for (i, v) in v.iter().enumerate() {
if i != 0 {
", ".fmt(f)?;
}
v.fmt(f)?;
}
Ok(())
}
Self::Number(v) => v.fmt(f),
}
}
Expand Down
90 changes: 21 additions & 69 deletions src/error/format.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,27 @@ impl ErrorFormatter for RichFormatter {
}
}

if let Some(valid) = error.get(ContextKind::SuggestedSubcommand) {
styled.none("\n\n");
did_you_mean(&mut styled, valid);
}
if let Some(valid) = error.get(ContextKind::SuggestedArg) {
styled.none("\n\n");
did_you_mean(&mut styled, valid);
}
if let Some(valid) = error.get(ContextKind::SuggestedValue) {
styled.none("\n\n");
did_you_mean(&mut styled, valid);
}
let suggestions = error.get(ContextKind::Suggested);
if let Some(ContextValue::StyledStrs(suggestions)) = suggestions {
for suggestion in suggestions {
styled.none("\n\n");
styled.none(TAB);
styled.extend(suggestion.iter());
}
}

let usage = error.get(ContextKind::Usage);
if let Some(ContextValue::StyledStr(usage)) = usage {
put_usage(&mut styled, usage.clone());
Expand Down Expand Up @@ -170,11 +191,6 @@ fn write_dynamic_context(error: &crate::error::Error, styled: &mut StyledStr) ->
styled.none("]");
}
}

if let Some(valid) = error.get(ContextKind::SuggestedValue) {
styled.none("\n\n");
did_you_mean(styled, valid);
}
true
} else {
false
Expand All @@ -186,22 +202,6 @@ fn write_dynamic_context(error: &crate::error::Error, styled: &mut StyledStr) ->
styled.none("The subcommand '");
styled.warning(invalid_sub);
styled.none("' wasn't recognized");

if let Some(valid) = error.get(ContextKind::SuggestedSubcommand) {
styled.none("\n\n");
did_you_mean(styled, valid);
}

let suggestion = error.get(ContextKind::SuggestedCommand);
if let Some(ContextValue::String(suggestion)) = suggestion {
styled.none("\n\n");
styled.none(TAB);
styled.none(
"If you believe you received this message in error, try re-running with '",
);
styled.good(suggestion);
styled.none("'");
}
true
} else {
false
Expand Down Expand Up @@ -326,54 +326,6 @@ fn write_dynamic_context(error: &crate::error::Error, styled: &mut StyledStr) ->
styled.none("Found argument '");
styled.warning(invalid_arg.to_string());
styled.none("' which wasn't expected, or isn't valid in this context");

let valid_sub = error.get(ContextKind::SuggestedSubcommand);
let valid_arg = error.get(ContextKind::SuggestedArg);
match (valid_sub, valid_arg) {
(
Some(ContextValue::String(valid_sub)),
Some(ContextValue::String(valid_arg)),
) => {
styled.none("\n\n");
styled.none(TAB);
styled.none("Did you mean to put '");
styled.good(valid_arg);
styled.none("' after the subcommand '");
styled.good(valid_sub);
styled.none("'?");
}
(None, Some(valid)) => {
styled.none("\n\n");
did_you_mean(styled, valid);
}
(_, _) => {}
}

let invalid_arg = error.get(ContextKind::InvalidArg);
if let Some(ContextValue::String(invalid_arg)) = invalid_arg {
let suggested_trailing_arg = error.get(ContextKind::SuggestedTrailingArg);
if suggested_trailing_arg == Some(&ContextValue::Bool(true)) {
styled.none("\n\n");
styled.none(TAB);
styled.none("If you tried to supply '");
styled.warning(invalid_arg);
styled.none("' as a value rather than a flag, use '");
styled.good("-- ");
styled.good(invalid_arg);
styled.none("'");
}

let trailing_arg = error.get(ContextKind::TrailingArg);
if trailing_arg == Some(&ContextValue::Bool(true)) {
styled.none("\n\n");
styled.none(TAB);
styled.none("If you tried to supply '");
styled.warning(invalid_arg);
styled.none("' as a subcommand, remove the '");
styled.warning("--");
styled.none("' before it.");
}
}
true
} else {
false
Expand Down
66 changes: 51 additions & 15 deletions src/error/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -426,20 +426,27 @@ impl<F: ErrorFormatter> Error<F> {
name: String,
usage: Option<StyledStr>,
) -> Self {
let suggestion = format!("{} -- {}", name, subcmd);
let mut err = Self::new(ErrorKind::InvalidSubcommand).with_cmd(cmd);

#[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.good(name);
styled_suggestion.good(" -- ");
styled_suggestion.good(&subcmd);
styled_suggestion.none("'");

err = err.extend_context_unchecked([
(ContextKind::InvalidSubcommand, ContextValue::String(subcmd)),
(
ContextKind::SuggestedSubcommand,
ContextValue::Strings(did_you_mean),
),
(
ContextKind::SuggestedCommand,
ContextValue::String(suggestion),
ContextKind::Suggested,
ContextValue::StyledStrs(vec![styled_suggestion]),
),
]);
if let Some(usage) = usage {
Expand Down Expand Up @@ -645,28 +652,47 @@ impl<F: ErrorFormatter> Error<F> {

#[cfg(feature = "error-context")]
{
let mut suggestions = vec![];
if suggested_trailing_arg {
let mut styled_suggestion = StyledStr::new();
styled_suggestion.none("If you tried to supply '");
styled_suggestion.warning(&arg);
styled_suggestion.none("' as a value rather than a flag, use '");
styled_suggestion.good("-- ");
styled_suggestion.good(&arg);
styled_suggestion.none("'");
suggestions.push(styled_suggestion);
}

err = err
.extend_context_unchecked([(ContextKind::InvalidArg, ContextValue::String(arg))]);
if let Some(usage) = usage {
err = err
.insert_context_unchecked(ContextKind::Usage, ContextValue::StyledStr(usage));
}
if let Some((flag, sub)) = did_you_mean {
err = err.insert_context_unchecked(
ContextKind::SuggestedArg,
ContextValue::String(format!("--{}", flag)),
);
if let Some(sub) = sub {
match did_you_mean {
Some((flag, Some(sub))) => {
let mut styled_suggestion = StyledStr::new();
styled_suggestion.none("Did you mean to put '");
styled_suggestion.good("--");
styled_suggestion.good(flag);
styled_suggestion.none("' after the subcommand '");
styled_suggestion.good(sub);
styled_suggestion.none("'?");
suggestions.push(styled_suggestion);
}
Some((flag, None)) => {
err = err.insert_context_unchecked(
ContextKind::SuggestedSubcommand,
ContextValue::String(sub),
ContextKind::SuggestedArg,
ContextValue::String(format!("--{}", flag)),
);
}
None => {}
}
if suggested_trailing_arg {
if !suggestions.is_empty() {
err = err.insert_context_unchecked(
ContextKind::SuggestedTrailingArg,
ContextValue::Bool(true),
ContextKind::Suggested,
ContextValue::StyledStrs(suggestions),
);
}
}
Expand All @@ -683,9 +709,19 @@ 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.warning("--");
styled_suggestion.none("' before it.");

err = err.extend_context_unchecked([
(ContextKind::InvalidArg, ContextValue::String(arg)),
(ContextKind::TrailingArg, ContextValue::Bool(true)),
(
ContextKind::Suggested,
ContextValue::StyledStrs(vec![styled_suggestion]),
),
]);
if let Some(usage) = usage {
err = err
Expand Down

0 comments on commit 6422046

Please sign in to comment.