Merge pull request #4381 from epage/consistent

fix(error): Be more consistent in error quoting

examples/tutorial_builder/04_01_enum.md

@@ -39,7 +39,7 @@ Tortoise
 
 $ 04_01_enum medium
 ? failed
-error: "medium" isn't a valid value for '<MODE>'
+error: 'medium' isn't a valid value for '<MODE>'
   [possible values: fast, slow]
 
 For more information try '--help'

examples/tutorial_builder/04_01_possible.md

@@ -19,7 +19,7 @@ Tortoise
 
 $ 04_01_possible medium
 ? failed
-error: "medium" isn't a valid value for '<MODE>'
+error: 'medium' isn't a valid value for '<MODE>'
   [possible values: fast, slow]
 
 For more information try '--help'

examples/tutorial_builder/04_02_parse.md

@@ -16,13 +16,13 @@ PORT = 22
 
 $ 04_02_parse foobar
 ? failed
-error: Invalid value "foobar" for '<PORT>': invalid digit found in string
+error: Invalid value 'foobar' for '<PORT>': invalid digit found in string
 
 For more information try '--help'
 
 $ 04_02_parse_derive 0
 ? failed
-error: Invalid value "0" for '<PORT>': 0 is not in 1..=65535
+error: Invalid value '0' for '<PORT>': 0 is not in 1..=65535
 
 For more information try '--help'
 

examples/tutorial_builder/04_02_validate.md

@@ -16,13 +16,13 @@ PORT = 22
 
 $ 04_02_validate foobar
 ? failed
-error: Invalid value "foobar" for '<PORT>': `foobar` isn't a port number
+error: Invalid value 'foobar' for '<PORT>': `foobar` isn't a port number
 
 For more information try '--help'
 
 $ 04_02_validate 0
 ? failed
-error: Invalid value "0" for '<PORT>': Port not in range 1-65535
+error: Invalid value '0' for '<PORT>': Port not in range 1-65535
 
 For more information try '--help'
 

examples/tutorial_derive/04_01_enum.md

@@ -39,7 +39,7 @@ Tortoise
 
 $ 04_01_enum_derive medium
 ? failed
-error: "medium" isn't a valid value for '<MODE>'
+error: 'medium' isn't a valid value for '<MODE>'
   [possible values: fast, slow]
 
 For more information try '--help'

examples/tutorial_derive/04_02_parse.md

@@ -16,13 +16,13 @@ PORT = 22
 
 $ 04_02_parse_derive foobar
 ? failed
-error: Invalid value "foobar" for '<PORT>': invalid digit found in string
+error: Invalid value 'foobar' for '<PORT>': invalid digit found in string
 
 For more information try '--help'
 
 $ 04_02_parse_derive 0
 ? failed
-error: Invalid value "0" for '<PORT>': 0 is not in 1..=65535
+error: Invalid value '0' for '<PORT>': 0 is not in 1..=65535
 
 For more information try '--help'
 

examples/tutorial_derive/04_02_validate.md

@@ -16,13 +16,13 @@ PORT = 22
 
 $ 04_02_validate_derive foobar
 ? failed
-error: Invalid value "foobar" for '<PORT>': `foobar` isn't a port number
+error: Invalid value 'foobar' for '<PORT>': `foobar` isn't a port number
 
 For more information try '--help'
 
 $ 04_02_validate_derive 0
 ? failed
-error: Invalid value "0" for '<PORT>': Port not in range 1-65535
+error: Invalid value '0' for '<PORT>': Port not in range 1-65535
 
 For more information try '--help'
 

examples/typed-derive.md

@@ -24,7 +24,7 @@ Args { optimization: Some(1), include: None, bind: None, sleep: None, defines: [
 
 $ typed-derive -O plaid
 ? failed
-error: Invalid value "plaid" for '-O <OPTIMIZATION>': invalid digit found in string
+error: Invalid value 'plaid' for '-O <OPTIMIZATION>': invalid digit found in string
 
 For more information try '--help'
 
@@ -44,7 +44,7 @@ Args { optimization: None, include: None, bind: Some(192.0.0.1), sleep: None, de
 
 $ typed-derive --bind localhost
 ? failed
-error: Invalid value "localhost" for '--bind <BIND>': invalid IP address syntax
+error: Invalid value 'localhost' for '--bind <BIND>': invalid IP address syntax
 
 For more information try '--help'
 
@@ -57,7 +57,7 @@ Args { optimization: None, include: None, bind: None, sleep: Some(Duration(10s))
 
 $ typed-derive --sleep forever
 ? failed
-error: Invalid value "forever" for '--sleep <SLEEP>': expected number at 0
+error: Invalid value 'forever' for '--sleep <SLEEP>': expected number at 0
 
 For more information try '--help'
 
@@ -70,13 +70,13 @@ Args { optimization: None, include: None, bind: None, sleep: None, defines: [("F
 
 $ typed-derive -D Foo
 ? failed
-error: Invalid value "Foo" for '-D <DEFINES>': invalid KEY=value: no `=` found in `Foo`
+error: Invalid value 'Foo' for '-D <DEFINES>': invalid KEY=value: no `=` found in `Foo`
 
 For more information try '--help'
 
 $ typed-derive -D Foo=Bar
 ? failed
-error: Invalid value "Foo=Bar" for '-D <DEFINES>': invalid digit found in string
+error: Invalid value 'Foo=Bar' for '-D <DEFINES>': invalid digit found in string
 
 For more information try '--help'
 
@@ -99,7 +99,7 @@ For more information try '--help'
 
 $ typed-derive --port 3000
 ? failed
-error: "3000" isn't a valid value for '--port <PORT>'
+error: '3000' isn't a valid value for '--port <PORT>'
   [possible values: 22, 80]
 
 For more information try '--help'
@@ -123,7 +123,7 @@ For more information try '--help'
 
 $ typed-derive --log-level critical
 ? failed
-error: "critical" isn't a valid value for '--log-level <LOG_LEVEL>'
+error: 'critical' isn't a valid value for '--log-level <LOG_LEVEL>'
   [possible values: info, debug, info, warn, error]
 
 For more information try '--help'

src/error/format.rs

@@ -147,8 +147,9 @@ fn write_dynamic_context(error: &crate::error::Error, styled: &mut StyledStr) ->
                     styled.warning(invalid_arg);
                     styled.none("' requires a value but none was supplied");
                 } else {
-                    styled.none(quote(invalid_value));
-                    styled.none(" isn't a valid value for '");
+                    styled.none("'");
+                    styled.none(invalid_value);
+                    styled.none("' isn't a valid value for '");
                     styled.warning(invalid_arg);
                     styled.none("'");
                 }
@@ -170,13 +171,9 @@ fn write_dynamic_context(error: &crate::error::Error, styled: &mut StyledStr) ->
                     }
                 }
 
-                let suggestion = error.get(ContextKind::SuggestedValue);
-                if let Some(ContextValue::String(suggestion)) = suggestion {
+                if let Some(valid) = error.get(ContextKind::SuggestedValue) {
                     styled.none("\n\n");
-                    styled.none(TAB);
-                    styled.none("Did you mean ");
-                    styled.good(quote(suggestion));
-                    styled.none("?");
+                    did_you_mean(styled, valid);
                 }
                 true
             } else {
@@ -190,13 +187,9 @@ fn write_dynamic_context(error: &crate::error::Error, styled: &mut StyledStr) ->
                 styled.warning(invalid_sub);
                 styled.none("' wasn't recognized");
 
-                let valid_sub = error.get(ContextKind::SuggestedSubcommand);
-                if let Some(ContextValue::String(valid_sub)) = valid_sub {
+                if let Some(valid) = error.get(ContextKind::SuggestedSubcommand) {
                     styled.none("\n\n");
-                    styled.none(TAB);
-                    styled.none("Did you mean ");
-                    styled.good(valid_sub);
-                    styled.none("?");
+                    did_you_mean(styled, valid);
                 }
 
                 let suggestion = error.get(ContextKind::SuggestedCommand);
@@ -287,9 +280,9 @@ fn write_dynamic_context(error: &crate::error::Error, styled: &mut StyledStr) ->
                 Some(ContextValue::String(invalid_value)),
             ) = (invalid_arg, invalid_value)
             {
-                styled.none("Invalid value ");
-                styled.warning(quote(invalid_value));
-                styled.none(" for '");
+                styled.none("Invalid value '");
+                styled.warning(invalid_value);
+                styled.none("' for '");
                 styled.warning(invalid_arg);
                 if let Some(source) = error.inner.source.as_deref() {
                     styled.none("': ");
@@ -341,19 +334,15 @@ fn write_dynamic_context(error: &crate::error::Error, styled: &mut StyledStr) ->
                     ) => {
                         styled.none("\n\n");
                         styled.none(TAB);
-                        styled.none("Did you mean ");
-                        styled.none("to put '");
+                        styled.none("Did you mean to put '");
                         styled.good(valid_arg);
                         styled.none("' after the subcommand '");
                         styled.good(valid_sub);
                         styled.none("'?");
                     }
-                    (None, Some(ContextValue::String(valid_arg))) => {
+                    (None, Some(valid)) => {
                         styled.none("\n\n");
-                        styled.none(TAB);
-                        styled.none("Did you mean '");
-                        styled.good(valid_arg);
-                        styled.none("'?");
+                        did_you_mean(styled, valid);
                     }
                     (_, _) => {}
                 }
@@ -447,15 +436,32 @@ fn try_help(styled: &mut StyledStr, help: Option<&str>) {
     }
 }
 
-fn quote(s: impl AsRef<str>) -> String {
-    let s = s.as_ref();
-    format!("{:?}", s)
+#[cfg(feature = "error-context")]
+fn did_you_mean(styled: &mut StyledStr, valid: &ContextValue) {
+    if let ContextValue::String(valid) = valid {
+        styled.none(TAB);
+        styled.none("Did you mean '");
+        styled.good(valid);
+        styled.none("'?");
+    } else if let ContextValue::Strings(valid) = valid {
+        styled.none(TAB);
+        styled.none("Did you mean ");
+        for (i, valid) in valid.iter().enumerate() {
+            if i != 0 {
+                styled.none(", ");
+            }
+            styled.none("'");
+            styled.good(valid);
+            styled.none("'");
+        }
+        styled.none("?");
+    }
 }
 
 fn escape(s: impl AsRef<str>) -> String {
     let s = s.as_ref();
     if s.contains(char::is_whitespace) {
-        quote(s)
+        format!("{:?}", s)
     } else {
         s.to_owned()
     }

src/error/mod.rs

@@ -422,7 +422,7 @@ impl<F: ErrorFormatter> Error<F> {
     pub(crate) fn invalid_subcommand(
         cmd: &Command,
         subcmd: String,
-        did_you_mean: String,
+        did_you_mean: Vec<String>,
         name: String,
         usage: Option<StyledStr>,
     ) -> Self {
@@ -435,7 +435,7 @@ impl<F: ErrorFormatter> Error<F> {
                 (ContextKind::InvalidSubcommand, ContextValue::String(subcmd)),
                 (
                     ContextKind::SuggestedSubcommand,
-                    ContextValue::String(did_you_mean),
+                    ContextValue::Strings(did_you_mean),
                 ),
                 (
                     ContextKind::SuggestedCommand,

src/parser/parser.rs

@@ -510,14 +510,10 @@ impl<'cmd> Parser<'cmd> {
         );
         // If the argument looks like a subcommand.
         if !candidates.is_empty() {
-            let candidates: Vec<_> = candidates
-                .iter()
-                .map(|candidate| format!("'{}'", candidate))
-                .collect();
             return ClapError::invalid_subcommand(
                 self.cmd,
                 arg_os.display().to_string(),
-                candidates.join(" or "),
+                candidates,
                 self.cmd
                     .get_bin_name()
                     .unwrap_or_else(|| self.cmd.get_name())

tests/builder/default_missing_vals.rs

@@ -261,7 +261,7 @@ fn delimited_missing_value() {
 #[cfg(debug_assertions)]
 #[test]
 #[cfg(feature = "error-context")]
-#[should_panic = "Argument `arg`'s default_missing_value=\"value\" failed validation: error: \"value\" isn't a valid value for '[arg]'"]
+#[should_panic = "Argument `arg`'s default_missing_value=\"value\" failed validation: error: 'value' isn't a valid value for '[arg]'"]
 fn default_missing_values_are_possible_values() {
     use clap::{Arg, Command};
 
@@ -277,7 +277,7 @@ fn default_missing_values_are_possible_values() {
 #[cfg(debug_assertions)]
 #[test]
 #[cfg(feature = "error-context")]
-#[should_panic = "Argument `arg`'s default_missing_value=\"value\" failed validation: error: Invalid value \"value\" for '[arg]"]
+#[should_panic = "Argument `arg`'s default_missing_value=\"value\" failed validation: error: Invalid value 'value' for '[arg]"]
 fn default_missing_values_are_valid() {
     use clap::{Arg, Command};
 

tests/builder/default_vals.rs

@@ -780,7 +780,7 @@ fn required_args_with_default_values() {
 #[cfg(debug_assertions)]
 #[test]
 #[cfg(feature = "error-context")]
-#[should_panic = "Argument `arg`'s default_value=\"value\" failed validation: error: \"value\" isn't a valid value for '[arg]'"]
+#[should_panic = "Argument `arg`'s default_value=\"value\" failed validation: error: 'value' isn't a valid value for '[arg]'"]
 fn default_values_are_possible_values() {
     use clap::{Arg, Command};
 
@@ -796,7 +796,7 @@ fn default_values_are_possible_values() {
 #[cfg(debug_assertions)]
 #[test]
 #[cfg(feature = "error-context")]
-#[should_panic = "Argument `arg`'s default_value=\"one\" failed validation: error: Invalid value \"one\" for '[arg]"]
+#[should_panic = "Argument `arg`'s default_value=\"one\" failed validation: error: Invalid value 'one' for '[arg]"]
 fn invalid_default_values() {
     use clap::{Arg, Command};
 
@@ -826,7 +826,7 @@ fn valid_delimited_default_values() {
 #[cfg(debug_assertions)]
 #[test]
 #[cfg(feature = "error-context")]
-#[should_panic = "Argument `arg`'s default_value=\"one\" failed validation: error: Invalid value \"one\" for '[arg]"]
+#[should_panic = "Argument `arg`'s default_value=\"one\" failed validation: error: Invalid value 'one' for '[arg]"]
 fn invalid_delimited_default_values() {
     use clap::{Arg, Command};