From 3e7ca92173f3f86f007f1068f0f38f37293ca77e Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 6 Jan 2023 16:43:40 -0600 Subject: [PATCH 1/2] chore: Upgrade trycmd --- Cargo.lock | 12 ++++++------ Cargo.toml | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1bf6fd3b..11e929dd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -817,9 +817,9 @@ checksum = "62ac7f900db32bf3fd12e0117dd3dc4da74bc52ebaac97f39668446d89694803" [[package]] name = "snapbox" -version = "0.4.3" +version = "0.4.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "efbd7b250c7243273b5aec4ca366fced84ad716d110bb7baae4814678952ebde" +checksum = "34eced5a65e76d5a00047986a83c65f80dc666faa27b5138f331659e2ca6bcf5" dependencies = [ "concolor", "libc", @@ -916,9 +916,9 @@ dependencies = [ [[package]] name = "toml_edit" -version = "0.16.2" +version = "0.17.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dd30deba9a1cd7153c22aecf93e86df639e7b81c622b0af8d9255e989991a7b7" +checksum = "a34cc558345efd7e88b9eda9626df2138b80bb46a7606f695e751c892bc7dac6" dependencies = [ "indexmap", "itertools", @@ -944,9 +944,9 @@ dependencies = [ [[package]] name = "trycmd" -version = "0.14.6" +version = "0.14.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cd0b37ad14571d245340fb1d56cec65507bd73571005adc63dc968aa38fa9d49" +checksum = "8a050cd97463d2f8df73e65b122dadb7f04ea31f0bd53a1306ea915cbb156849" dependencies = [ "escargot", "glob", diff --git a/Cargo.toml b/Cargo.toml index 72ef85b3..750c26ee 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -107,7 +107,7 @@ once_cell = { version = "1.12.0", optional = true } trybuild = "1.0.73" rustversion = "1" # Cutting out `filesystem` feature -trycmd = { version = "0.14.6", default-features = false, features = ["color-auto", "diff", "examples"] } +trycmd = { version = "0.14.9", default-features = false, features = ["color-auto", "diff", "examples"] } humantime = "2" snapbox = "0.4" shlex = "1.1.0" From 762b06fba48533169c36a1b60c364657c06800de Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 6 Jan 2023 17:01:34 -0600 Subject: [PATCH 2/2] fix(error): Try to polish/clarify messages In text communication you need to balance - Scannability, putting the most important information upfront - Brevity so people don't get lost in the message - Softness to help ease people through a frustrating experience I feel we weren't doing great on the first two points, so tried to iterate on the messages to improve them. I hope we aren't suffering too much on the third point as a side effect. --- examples/derive_ref/interop_tests.md | 12 +++---- examples/escaped-positional-derive.md | 2 +- examples/escaped-positional.md | 2 +- examples/tutorial_builder/03_01_flag_bool.md | 2 +- examples/tutorial_builder/04_01_enum.md | 2 +- examples/tutorial_builder/04_01_possible.md | 2 +- examples/tutorial_derive/03_01_flag_bool.md | 2 +- examples/tutorial_derive/04_01_enum.md | 2 +- examples/typed-derive.md | 8 ++--- src/error/format.rs | 38 ++++++++++---------- src/error/kind.rs | 12 +++---- tests/builder/conflicts.rs | 4 +-- tests/builder/default_missing_vals.rs | 2 +- tests/builder/default_vals.rs | 4 +-- tests/builder/error.rs | 10 +++--- tests/builder/flags.rs | 6 ++-- tests/builder/help.rs | 2 +- tests/builder/opts.rs | 4 +-- tests/builder/possible_values.rs | 18 +++++----- tests/builder/subcommands.rs | 16 ++++----- tests/ui/error_stderr.toml | 2 +- 21 files changed, 74 insertions(+), 78 deletions(-) diff --git a/examples/derive_ref/interop_tests.md b/examples/derive_ref/interop_tests.md index ceff9c83..bfa06f10 100644 --- a/examples/derive_ref/interop_tests.md +++ b/examples/derive_ref/interop_tests.md @@ -35,7 +35,7 @@ Value of derived: DerivedArgs { ```console $ interop_augment_args --unknown ? failed -error: found argument '--unknown' which wasn't expected, or isn't valid in this context +error: unexpected argument '--unknown' Usage: interop_augment_args[EXE] [OPTIONS] @@ -70,7 +70,7 @@ Derived subcommands: Derived { ```console $ interop_augment_subcommands derived --unknown ? failed -error: found argument '--unknown' which wasn't expected, or isn't valid in this context +error: unexpected argument '--unknown' Usage: interop_augment_subcommands[EXE] derived [OPTIONS] @@ -81,7 +81,7 @@ For more information, try '--help'. ```console $ interop_augment_subcommands unknown ? failed -error: the subcommand 'unknown' wasn't recognized +error: unrecognized subcommand 'unknown' Usage: interop_augment_subcommands[EXE] [COMMAND] @@ -140,7 +140,7 @@ Cli { ```console $ interop_hand_subcommand add --unknown ? failed -error: found argument '--unknown' which wasn't expected, or isn't valid in this context +error: unexpected argument '--unknown' note: to pass '--unknown' as a value, use '-- --unknown' @@ -185,7 +185,7 @@ Cli { ```console $ interop_hand_subcommand unknown ? failed -error: the subcommand 'unknown' wasn't recognized +error: unrecognized subcommand 'unknown' Usage: interop_hand_subcommand[EXE] [OPTIONS] @@ -239,7 +239,7 @@ Cli { ```console $ interop_flatten_hand_args --unknown ? failed -error: found argument '--unknown' which wasn't expected, or isn't valid in this context +error: unexpected argument '--unknown' Usage: interop_flatten_hand_args[EXE] [OPTIONS] diff --git a/examples/escaped-positional-derive.md b/examples/escaped-positional-derive.md index adc0ca32..d075f99a 100644 --- a/examples/escaped-positional-derive.md +++ b/examples/escaped-positional-derive.md @@ -33,7 +33,7 @@ Notice that we can't pass positional arguments before `--`: ```console $ escaped-positional-derive foo bar ? failed -error: found argument 'foo' which wasn't expected, or isn't valid in this context +error: unexpected argument 'foo' Usage: escaped-positional-derive[EXE] [OPTIONS] [-- ...] diff --git a/examples/escaped-positional.md b/examples/escaped-positional.md index 31ddd3fc..fec34c79 100644 --- a/examples/escaped-positional.md +++ b/examples/escaped-positional.md @@ -33,7 +33,7 @@ Notice that we can't pass positional arguments before `--`: ```console $ escaped-positional foo bar ? failed -error: found argument 'foo' which wasn't expected, or isn't valid in this context +error: unexpected argument 'foo' Usage: escaped-positional[EXE] [OPTIONS] [-- ...] diff --git a/examples/tutorial_builder/03_01_flag_bool.md b/examples/tutorial_builder/03_01_flag_bool.md index 11aeaf3f..feec8e0a 100644 --- a/examples/tutorial_builder/03_01_flag_bool.md +++ b/examples/tutorial_builder/03_01_flag_bool.md @@ -17,7 +17,7 @@ verbose: true $ 03_01_flag_bool --verbose --verbose ? failed -error: the argument '--verbose' was provided more than once, but cannot be used multiple times +error: the argument '--verbose' cannot be used multiple times Usage: 03_01_flag_bool[EXE] [OPTIONS] diff --git a/examples/tutorial_builder/04_01_enum.md b/examples/tutorial_builder/04_01_enum.md index 04ab07d5..ec4c0aea 100644 --- a/examples/tutorial_builder/04_01_enum.md +++ b/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 '' +error: invalid value 'medium' for '' [possible values: fast, slow] For more information, try '--help'. diff --git a/examples/tutorial_builder/04_01_possible.md b/examples/tutorial_builder/04_01_possible.md index 5be0c822..fa2c8353 100644 --- a/examples/tutorial_builder/04_01_possible.md +++ b/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 '' +error: invalid value 'medium' for '' [possible values: fast, slow] For more information, try '--help'. diff --git a/examples/tutorial_derive/03_01_flag_bool.md b/examples/tutorial_derive/03_01_flag_bool.md index 86eb07d6..97622600 100644 --- a/examples/tutorial_derive/03_01_flag_bool.md +++ b/examples/tutorial_derive/03_01_flag_bool.md @@ -17,7 +17,7 @@ verbose: true $ 03_01_flag_bool_derive --verbose --verbose ? failed -error: the argument '--verbose' was provided more than once, but cannot be used multiple times +error: the argument '--verbose' cannot be used multiple times Usage: 03_01_flag_bool_derive[EXE] [OPTIONS] diff --git a/examples/tutorial_derive/04_01_enum.md b/examples/tutorial_derive/04_01_enum.md index ea580d62..89db08c9 100644 --- a/examples/tutorial_derive/04_01_enum.md +++ b/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 '' +error: invalid value 'medium' for '' [possible values: fast, slow] For more information, try '--help'. diff --git a/examples/typed-derive.md b/examples/typed-derive.md index 6d4819e4..bf7a56a5 100644 --- a/examples/typed-derive.md +++ b/examples/typed-derive.md @@ -92,14 +92,14 @@ Args { optimization: None, include: None, bind: None, sleep: None, defines: [], $ typed-derive --port ? failed -error: the argument '--port ' requires a value but none was supplied +error: a value is required for '--port ' but none was supplied [possible values: 22, 80] For more information, try '--help'. $ typed-derive --port 3000 ? failed -error: '3000' isn't a valid value for '--port ' +error: invalid value '3000' for '--port ' [possible values: 22, 80] For more information, try '--help'. @@ -116,14 +116,14 @@ Args { optimization: None, include: None, bind: None, sleep: None, defines: [], $ typed-derive --log-level ? failed -error: the argument '--log-level ' requires a value but none was supplied +error: a value is required for '--log-level ' but none was supplied [possible values: info, debug, info, warn, error] For more information, try '--help'. $ typed-derive --log-level critical ? failed -error: 'critical' isn't a valid value for '--log-level ' +error: invalid value 'critical' for '--log-level ' [possible values: info, debug, info, warn, error] For more information, try '--help'. diff --git a/src/error/format.rs b/src/error/format.rs index 9efc2eb0..17af8664 100644 --- a/src/error/format.rs +++ b/src/error/format.rs @@ -131,7 +131,7 @@ fn write_dynamic_context(error: &crate::error::Error, styled: &mut StyledStr) -> if ContextValue::String(invalid_arg.clone()) == *prior_arg { styled.none("the argument '"); styled.warning(invalid_arg); - styled.none("' was provided more than once, but cannot be used multiple times"); + styled.none("' cannot be used multiple times"); } else { styled.none("the argument '"); styled.warning(invalid_arg); @@ -181,13 +181,13 @@ fn write_dynamic_context(error: &crate::error::Error, styled: &mut StyledStr) -> ) = (invalid_arg, invalid_value) { if invalid_value.is_empty() { - styled.none("the argument '"); + styled.none("a value is required for '"); styled.warning(invalid_arg); - styled.none("' requires a value but none was supplied"); + styled.none("' but none was supplied"); } else { - styled.none("'"); + styled.none("invalid value '"); styled.none(invalid_value); - styled.none("' isn't a valid value for '"); + styled.none("' for '"); styled.warning(invalid_arg); styled.none("'"); } @@ -216,9 +216,9 @@ fn write_dynamic_context(error: &crate::error::Error, styled: &mut StyledStr) -> ErrorKind::InvalidSubcommand => { let invalid_sub = error.get(ContextKind::InvalidSubcommand); if let Some(ContextValue::String(invalid_sub)) = invalid_sub { - styled.none("the subcommand '"); + styled.none("unrecognized subcommand '"); styled.warning(invalid_sub); - styled.none("' wasn't recognized"); + styled.none("'"); true } else { false @@ -276,11 +276,11 @@ fn write_dynamic_context(error: &crate::error::Error, styled: &mut StyledStr) -> Some(ContextValue::String(invalid_value)), ) = (invalid_arg, invalid_value) { - styled.none("the value '"); + styled.none("unexpected value '"); styled.warning(invalid_value); - styled.none("' was provided to '"); + styled.none("' for '"); styled.warning(invalid_arg); - styled.none("' but it wasn't expecting any more values"); + styled.none("'; no more were expected"); true } else { false @@ -297,11 +297,10 @@ fn write_dynamic_context(error: &crate::error::Error, styled: &mut StyledStr) -> ) = (invalid_arg, actual_num_values, min_values) { let were_provided = singular_or_plural(*actual_num_values as usize); - styled.none("the argument '"); - styled.warning(invalid_arg); - styled.none("' requires at least "); styled.warning(min_values.to_string()); - styled.none(" values but only "); + styled.none(" more values required by '"); + styled.warning(invalid_arg); + styled.none("'; only "); styled.warning(actual_num_values.to_string()); styled.none(were_provided); true @@ -343,11 +342,10 @@ fn write_dynamic_context(error: &crate::error::Error, styled: &mut StyledStr) -> ) = (invalid_arg, actual_num_values, num_values) { let were_provided = singular_or_plural(*actual_num_values as usize); - styled.none("the argument '"); - styled.warning(invalid_arg); - styled.none("' requires "); styled.warning(num_values.to_string()); - styled.none(" values, but "); + styled.none(" values required for '"); + styled.warning(invalid_arg); + styled.none("' but "); styled.warning(actual_num_values.to_string()); styled.none(were_provided); true @@ -358,9 +356,9 @@ fn write_dynamic_context(error: &crate::error::Error, styled: &mut StyledStr) -> ErrorKind::UnknownArgument => { let invalid_arg = error.get(ContextKind::InvalidArg); if let Some(ContextValue::String(invalid_arg)) = invalid_arg { - styled.none("found argument '"); + styled.none("unexpected argument '"); styled.warning(invalid_arg.to_string()); - styled.none("' which wasn't expected, or isn't valid in this context"); + styled.none("'"); true } else { false diff --git a/src/error/kind.rs b/src/error/kind.rs index 4c254b44..c643b22b 100644 --- a/src/error/kind.rs +++ b/src/error/kind.rs @@ -317,15 +317,13 @@ impl ErrorKind { pub fn as_str(self) -> Option<&'static str> { match self { Self::InvalidValue => Some("one of the values isn't valid for an argument"), - Self::UnknownArgument => { - Some("found an argument which wasn't expected or isn't valid in this context") - } - Self::InvalidSubcommand => Some("a subcommand wasn't recognized"), + Self::UnknownArgument => Some("unexpected argument"), + Self::InvalidSubcommand => Some("unrecognized subcommand"), Self::NoEquals => Some("equal is needed when assigning values to one of the arguments"), Self::ValueValidation => Some("invalid value for one of the arguments"), - Self::TooManyValues => Some("an argument received an unexpected value"), - Self::TooFewValues => Some("an argument requires more values"), - Self::WrongNumberOfValues => Some("an argument received too many or too few values"), + Self::TooManyValues => Some("unexpected value for an argument"), + Self::TooFewValues => Some("more values required for an argument"), + Self::WrongNumberOfValues => Some("too many or too few values for an argument"), Self::ArgumentConflict => { Some("an argument cannot be used with one or more of the other specified arguments") } diff --git a/tests/builder/conflicts.rs b/tests/builder/conflicts.rs index 284d807c..59c69d61 100644 --- a/tests/builder/conflicts.rs +++ b/tests/builder/conflicts.rs @@ -359,7 +359,7 @@ For more information, try '--help'. #[cfg(feature = "error-context")] fn conflict_output_repeat() { static ERR: &str = "\ -error: the argument '-F' was provided more than once, but cannot be used multiple times +error: the argument '-F' cannot be used multiple times Usage: clap-test [OPTIONS] [positional] [positional2] [positional3]... [COMMAND] @@ -734,7 +734,7 @@ fn args_negate_subcommands_two_levels() { #[cfg(feature = "error-context")] fn subcommand_conflict_error_message() { static CONFLICT_ERR: &str = "\ -error: found argument 'sub1' which wasn't expected, or isn't valid in this context +error: unexpected argument 'sub1' Usage: test [OPTIONS] test diff --git a/tests/builder/default_missing_vals.rs b/tests/builder/default_missing_vals.rs index 99deabd5..da34c28b 100644 --- a/tests/builder/default_missing_vals.rs +++ b/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: invalid value 'value' for '[arg]'"] fn default_missing_values_are_possible_values() { use clap::{Arg, Command}; diff --git a/tests/builder/default_vals.rs b/tests/builder/default_vals.rs index f08906d2..e8ddf7c1 100644 --- a/tests/builder/default_vals.rs +++ b/tests/builder/default_vals.rs @@ -52,7 +52,7 @@ fn opt_without_value_fail() { assert_eq!(err.kind(), ErrorKind::InvalidValue); assert!(err .to_string() - .contains("the argument '-o ' requires a value but none was supplied")); + .contains("a value is required for '-o ' but none was supplied")); } #[test] @@ -796,7 +796,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: invalid value 'value' for '[arg]'"] fn default_values_are_possible_values() { use clap::{Arg, Command}; diff --git a/tests/builder/error.rs b/tests/builder/error.rs index 7c2033fa..1ac45584 100644 --- a/tests/builder/error.rs +++ b/tests/builder/error.rs @@ -106,7 +106,7 @@ fn kind_formats_validation_error() { let err = res.unwrap_err(); let expected_kind = ErrorKind::UnknownArgument; static MESSAGE: &str = "\ -error: found an argument which wasn't expected or isn't valid in this context +error: unexpected argument "; assert_error(err, expected_kind, MESSAGE, true); } @@ -120,7 +120,7 @@ fn rich_formats_validation_error() { let err = res.unwrap_err(); let expected_kind = ErrorKind::UnknownArgument; static MESSAGE: &str = "\ -error: found argument 'unused' which wasn't expected, or isn't valid in this context +error: unexpected argument 'unused' Usage: test @@ -139,7 +139,7 @@ fn suggest_trailing() { let err = res.unwrap_err(); let expected_kind = ErrorKind::UnknownArgument; static MESSAGE: &str = "\ -error: found argument '--foo' which wasn't expected, or isn't valid in this context +error: unexpected argument '--foo' note: to pass '--foo' as a value, use '-- --foo' @@ -160,7 +160,7 @@ fn trailing_already_in_use() { let err = res.unwrap_err(); let expected_kind = ErrorKind::UnknownArgument; static MESSAGE: &str = "\ -error: found argument '--foo' which wasn't expected, or isn't valid in this context +error: unexpected argument '--foo' Usage: rg [PATTERN] @@ -179,7 +179,7 @@ fn cant_use_trailing() { let err = res.unwrap_err(); let expected_kind = ErrorKind::UnknownArgument; static MESSAGE: &str = "\ -error: found argument '--foo' which wasn't expected, or isn't valid in this context +error: unexpected argument '--foo' Usage: test diff --git a/tests/builder/flags.rs b/tests/builder/flags.rs index 8eee4ea3..c59b5245 100644 --- a/tests/builder/flags.rs +++ b/tests/builder/flags.rs @@ -140,7 +140,7 @@ fn multiple_flags_in_single() { #[cfg(feature = "error-context")] fn unexpected_value_error() { const USE_FLAG_AS_ARGUMENT: &str = "\ -error: the value 'foo' was provided to '--a-flag' but it wasn't expecting any more values +error: unexpected value 'foo' for '--a-flag'; no more were expected Usage: mycat --a-flag [filename] @@ -158,7 +158,7 @@ For more information, try '--help'. #[cfg(feature = "error-context")] 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 +error: unexpected argument '--another-flag' note: to pass '--another-flag' as a value, use '-- --another-flag' @@ -202,7 +202,7 @@ For more information, try '--help'. #[cfg(feature = "error-context")] fn issue_2308_multiple_dashes() { static MULTIPLE_DASHES: &str = "\ -error: found argument '-----' which wasn't expected, or isn't valid in this context +error: unexpected argument '-----' note: to pass '-----' as a value, use '-- -----' diff --git a/tests/builder/help.rs b/tests/builder/help.rs index b6776bb2..984a86c2 100644 --- a/tests/builder/help.rs +++ b/tests/builder/help.rs @@ -79,7 +79,7 @@ fn help_multi_subcommand_error() { .try_get_matches_from(["ctest", "help", "subcmd", "multi", "foo"]) .unwrap_err(); - static EXPECTED: &str = "error: the subcommand 'foo' wasn't recognized + static EXPECTED: &str = "error: unrecognized subcommand 'foo' Usage: ctest subcmd multi [OPTIONS] diff --git a/tests/builder/opts.rs b/tests/builder/opts.rs index 3a116576..fbb870bf 100644 --- a/tests/builder/opts.rs +++ b/tests/builder/opts.rs @@ -446,7 +446,7 @@ fn leading_hyphen_with_only_pos_follows() { #[cfg(feature = "error-context")] fn did_you_mean() { static DYM: &str = "\ -error: found argument '--optio' which wasn't expected, or isn't valid in this context +error: unexpected argument '--optio' note: argument '--option' exists @@ -544,7 +544,7 @@ fn issue_1105_empty_value_short_explicit_no_space() { #[cfg(feature = "error-context")] 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 +error: unexpected argument '--files-without-matches' note: argument '--files-without-match' exists diff --git a/tests/builder/possible_values.rs b/tests/builder/possible_values.rs index d7a8c481..7e0d50ed 100644 --- a/tests/builder/possible_values.rs +++ b/tests/builder/possible_values.rs @@ -178,7 +178,7 @@ fn possible_values_of_option_multiple_fail() { fn possible_values_output() { #[cfg(feature = "suggestions")] static PV_ERROR: &str = "\ -error: 'slo' isn't a valid value for '-O