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.

Sign up for GitHub

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

Jump to bottom

refactor(error): Generalize how we report suggestions #4383

Merged
merged 7 commits into from Oct 13, 2022
Merged

refactor(error): Generalize how we report suggestions #4383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4ec79b1
refactor(error): Centralize suggestions
epage Oct 13, 2022
c8cb5bb
feat(error): General suggestion mechanism
epage Oct 13, 2022
7417c75
refactor(error): Move escaped-subcmd suggestion to general suggestions
epage Oct 13, 2022
813060e
refactor(error): Move bad-escape suggestion to general suggestion
epage Oct 13, 2022
5275660
refactor(error): Move escape suggestion to general suggestion
epage Oct 13, 2022
63eec40
refactor(error): Clarify distinct suggestion cases
epage Oct 13, 2022
b9d2980
refactor(error): Move subcommand suggestion to general suggestions
epage Oct 13, 2022
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
15 changes: 13 additions & 2 deletions src/error/context.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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