From 4ec79b1448b16781bbe9748d5eccdb44205c1d9d Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 13 Oct 2022 11:19:31 -0500 Subject: [PATCH 1/7] refactor(error): Centralize suggestions --- src/error/format.rs | 128 ++++++++++++++++++++------------------------ 1 file changed, 59 insertions(+), 69 deletions(-) diff --git a/src/error/format.rs b/src/error/format.rs index 4442a8e5386..c815227ee92 100644 --- a/src/error/format.rs +++ b/src/error/format.rs @@ -64,6 +64,65 @@ impl ErrorFormatter for RichFormatter { } } + if let Some(valid) = error.get(ContextKind::SuggestedValue) { + styled.none("\n\n"); + did_you_mean(&mut styled, valid); + } + + 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("'?"); + } + (Some(valid), None) | (None, Some(valid)) => { + styled.none("\n\n"); + did_you_mean(&mut 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."); + } + } + + 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("'"); + } + let usage = error.get(ContextKind::Usage); if let Some(ContextValue::StyledStr(usage)) = usage { put_usage(&mut styled, usage.clone()); @@ -170,11 +229,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 @@ -186,22 +240,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 @@ -326,54 +364,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 From c8cb5bba385c33f9cba5baaa0962373135e74d43 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 13 Oct 2022 12:20:04 -0500 Subject: [PATCH 2/7] feat(error): General suggestion mechanism --- src/error/context.rs | 16 +++++++++++++++- src/error/format.rs | 9 +++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/error/context.rs b/src/error/context.rs index e0055bb2ad0..5fe5f4f2239 100644 --- a/src/error/context.rs +++ b/src/error/context.rs @@ -31,6 +31,8 @@ pub enum ContextKind { TrailingArg, /// Potential fix for the user SuggestedTrailingArg, + /// Potential fix for the user + Suggested, /// A usage string Usage, /// An opaque message to the user @@ -53,8 +55,9 @@ 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::SuggestedTrailingArg => Some("Suggested Trailing Argument"), + Self::Suggested => Some("Suggested"), Self::Usage => None, Self::Custom => None, } @@ -82,6 +85,8 @@ pub enum ContextValue { Strings(Vec), /// A single value StyledStr(crate::builder::StyledStr), + /// many value + StyledStrs(Vec), /// A single value Number(isize), } @@ -94,6 +99,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), } } diff --git a/src/error/format.rs b/src/error/format.rs index c815227ee92..a95bebddb12 100644 --- a/src/error/format.rs +++ b/src/error/format.rs @@ -123,6 +123,15 @@ impl ErrorFormatter for RichFormatter { styled.none("'"); } + 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()); From 7417c75c57b8a7f0fd9563237c2b829021b52d5f Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 13 Oct 2022 12:59:13 -0500 Subject: [PATCH 3/7] refactor(error): Move escaped-subcmd suggestion to general suggestions --- src/error/format.rs | 9 --------- src/error/mod.rs | 13 ++++++++++--- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/error/format.rs b/src/error/format.rs index a95bebddb12..b39e5650d1f 100644 --- a/src/error/format.rs +++ b/src/error/format.rs @@ -114,15 +114,6 @@ impl ErrorFormatter for RichFormatter { } } - 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("'"); - } - let suggestions = error.get(ContextKind::Suggested); if let Some(ContextValue::StyledStrs(suggestions)) = suggestions { for suggestion in suggestions { diff --git a/src/error/mod.rs b/src/error/mod.rs index 6d8502f56f6..91aa9e046b6 100644 --- a/src/error/mod.rs +++ b/src/error/mod.rs @@ -426,11 +426,18 @@ impl Error { name: String, usage: Option, ) -> 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)), ( @@ -438,8 +445,8 @@ impl Error { ContextValue::Strings(did_you_mean), ), ( - ContextKind::SuggestedCommand, - ContextValue::String(suggestion), + ContextKind::Suggested, + ContextValue::StyledStrs(vec![styled_suggestion]), ), ]); if let Some(usage) = usage { From 813060ea82d5b090e5b9effb31910f032ec8469d Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 13 Oct 2022 13:03:14 -0500 Subject: [PATCH 4/7] refactor(error): Move bad-escape suggestion to general suggestion --- src/error/format.rs | 11 ----------- src/error/mod.rs | 12 +++++++++++- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/error/format.rs b/src/error/format.rs index b39e5650d1f..58526dd46ef 100644 --- a/src/error/format.rs +++ b/src/error/format.rs @@ -101,17 +101,6 @@ impl ErrorFormatter for RichFormatter { 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."); - } } let suggestions = error.get(ContextKind::Suggested); diff --git a/src/error/mod.rs b/src/error/mod.rs index 91aa9e046b6..928261ac70f 100644 --- a/src/error/mod.rs +++ b/src/error/mod.rs @@ -690,9 +690,19 @@ impl Error { #[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 From 5275660967d3b27587f307f952b57f97d808983b Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 13 Oct 2022 13:10:15 -0500 Subject: [PATCH 5/7] refactor(error): Move escape suggestion to general suggestion --- src/error/context.rs | 3 --- src/error/format.rs | 15 --------------- src/error/mod.rs | 18 +++++++++++++++--- 3 files changed, 15 insertions(+), 21 deletions(-) diff --git a/src/error/context.rs b/src/error/context.rs index 5fe5f4f2239..5cc31097f12 100644 --- a/src/error/context.rs +++ b/src/error/context.rs @@ -30,8 +30,6 @@ pub enum ContextKind { /// Trailing argument TrailingArg, /// Potential fix for the user - SuggestedTrailingArg, - /// Potential fix for the user Suggested, /// A usage string Usage, @@ -56,7 +54,6 @@ impl ContextKind { Self::SuggestedArg => Some("Suggested Argument"), Self::SuggestedValue => Some("Suggested Value"), Self::TrailingArg => Some("Trailing Argument"), - Self::SuggestedTrailingArg => Some("Suggested Trailing Argument"), Self::Suggested => Some("Suggested"), Self::Usage => None, Self::Custom => None, diff --git a/src/error/format.rs b/src/error/format.rs index 58526dd46ef..5e78e08fdaf 100644 --- a/src/error/format.rs +++ b/src/error/format.rs @@ -88,21 +88,6 @@ impl ErrorFormatter for RichFormatter { (_, _) => {} } - 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 suggestions = error.get(ContextKind::Suggested); if let Some(ContextValue::StyledStrs(suggestions)) = suggestions { for suggestion in suggestions { diff --git a/src/error/mod.rs b/src/error/mod.rs index 928261ac70f..913d48181af 100644 --- a/src/error/mod.rs +++ b/src/error/mod.rs @@ -652,6 +652,18 @@ impl Error { #[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 { @@ -670,10 +682,10 @@ impl Error { ); } } - if suggested_trailing_arg { + if !suggestions.is_empty() { err = err.insert_context_unchecked( - ContextKind::SuggestedTrailingArg, - ContextValue::Bool(true), + ContextKind::Suggested, + ContextValue::StyledStrs(suggestions), ); } } From 63eec40652b4fc0d9a29c21dc16b0084a4af5ac9 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 13 Oct 2022 13:15:11 -0500 Subject: [PATCH 6/7] refactor(error): Clarify distinct suggestion cases --- src/error/mod.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/error/mod.rs b/src/error/mod.rs index 913d48181af..4ec2d0cc3f0 100644 --- a/src/error/mod.rs +++ b/src/error/mod.rs @@ -670,17 +670,24 @@ impl Error { 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))) => { err = err.insert_context_unchecked( ContextKind::SuggestedSubcommand, ContextValue::String(sub), ); + err = err.insert_context_unchecked( + ContextKind::SuggestedArg, + ContextValue::String(format!("--{}", flag)), + ); + } + Some((flag, None)) => { + err = err.insert_context_unchecked( + ContextKind::SuggestedArg, + ContextValue::String(format!("--{}", flag)), + ); } + None => {} } if !suggestions.is_empty() { err = err.insert_context_unchecked( From b9d298086cd50b70fb2f6de11b5a20916734890b Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 13 Oct 2022 13:17:44 -0500 Subject: [PATCH 7/7] refactor(error): Move subcommand suggestion to general suggestions --- src/error/format.rs | 28 ++++++++-------------------- src/error/mod.rs | 16 ++++++++-------- 2 files changed, 16 insertions(+), 28 deletions(-) diff --git a/src/error/format.rs b/src/error/format.rs index 5e78e08fdaf..2e0f8e9f824 100644 --- a/src/error/format.rs +++ b/src/error/format.rs @@ -64,30 +64,18 @@ impl ErrorFormatter for RichFormatter { } } - if let Some(valid) = error.get(ContextKind::SuggestedValue) { + if let Some(valid) = error.get(ContextKind::SuggestedSubcommand) { styled.none("\n\n"); did_you_mean(&mut styled, valid); } - - 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("'?"); - } - (Some(valid), None) | (None, Some(valid)) => { - 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 { diff --git a/src/error/mod.rs b/src/error/mod.rs index 4ec2d0cc3f0..af65529fce8 100644 --- a/src/error/mod.rs +++ b/src/error/mod.rs @@ -672,14 +672,14 @@ impl Error { } match did_you_mean { Some((flag, Some(sub))) => { - err = err.insert_context_unchecked( - ContextKind::SuggestedSubcommand, - ContextValue::String(sub), - ); - err = err.insert_context_unchecked( - ContextKind::SuggestedArg, - ContextValue::String(format!("--{}", flag)), - ); + 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(