From c30f166d09021ea6defd4a87506d7215afbee23e Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Fri, 29 Mar 2024 12:39:34 +0900 Subject: [PATCH 1/6] revset, templater: split parse error constructor that sets source error object I'm going to add RevsetParseError constructor for InvalidFunctionArguments, with/without a source error, and I don't want to duplicate code for all combinations. The templater change is just for consistency. I couldn't find a good naming convention for the builder-like API, so it's called .with_source(mut self, _). Another option was .source_set(source). Apparently, it's not uncommon to name consuming constructor as with_(). --- cli/src/template_parser.rs | 33 +++++++++------------------------ lib/src/revset.rs | 26 +++++++------------------- 2 files changed, 16 insertions(+), 43 deletions(-) diff --git a/cli/src/template_parser.rs b/cli/src/template_parser.rs index 1b8f80ef21..78bf23265b 100644 --- a/cli/src/template_parser.rs +++ b/cli/src/template_parser.rs @@ -119,21 +119,9 @@ impl TemplateParseError { } } - pub fn with_span_and_source( - kind: TemplateParseErrorKind, - span: pest::Span<'_>, - source: impl Into>, - ) -> Self { - let message = kind.to_string(); - let pest_error = Box::new(pest::error::Error::new_from_span( - pest::error::ErrorVariant::CustomError { message }, - span, - )); - TemplateParseError { - kind, - pest_error, - source: Some(source.into()), - } + pub fn with_source(mut self, source: impl Into>) -> Self { + self.source = Some(source.into()); + self } // TODO: migrate all callers to table-based lookup_method() @@ -179,19 +167,19 @@ impl TemplateParseError { source: impl Into>, span: pest::Span<'_>, ) -> Self { - TemplateParseError::with_span_and_source( + TemplateParseError::with_span( TemplateParseErrorKind::UnexpectedExpression(message.into()), span, - source, ) + .with_source(source) } pub fn within_alias_expansion(self, id: TemplateAliasId<'_>, span: pest::Span<'_>) -> Self { - TemplateParseError::with_span_and_source( + TemplateParseError::with_span( TemplateParseErrorKind::BadAliasExpansion(id.to_string()), span, - self, ) + .with_source(self) } /// If this is a `NoSuchKeyword` error, expands the candidates list with the @@ -435,11 +423,8 @@ fn parse_term_node(pair: Pair) -> TemplateParseResult { } Rule::integer_literal => { let value = expr.as_str().parse().map_err(|err| { - TemplateParseError::with_span_and_source( - TemplateParseErrorKind::ParseIntError, - span, - err, - ) + TemplateParseError::with_span(TemplateParseErrorKind::ParseIntError, span) + .with_source(err) })?; ExpressionNode::new(ExpressionKind::Integer(value), span) } diff --git a/lib/src/revset.rs b/lib/src/revset.rs index 792412f95e..841c1ca913 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -204,21 +204,9 @@ impl RevsetParseError { } } - fn with_span_and_source( - kind: RevsetParseErrorKind, - span: pest::Span<'_>, - source: impl Into>, - ) -> Self { - let message = kind.to_string(); - let pest_error = Box::new(pest::error::Error::new_from_span( - pest::error::ErrorVariant::CustomError { message }, - span, - )); - RevsetParseError { - kind, - pest_error, - source: Some(source.into()), - } + fn with_source(mut self, source: impl Into>) -> Self { + self.source = Some(source.into()); + self } pub fn kind(&self) -> &RevsetParseErrorKind { @@ -803,11 +791,11 @@ impl ParseState<'_> { allow_string_pattern: self.allow_string_pattern, }; f(expanding_state).map_err(|e| { - RevsetParseError::with_span_and_source( + RevsetParseError::with_span( RevsetParseErrorKind::BadAliasExpansion(id.to_string()), span, - e, ) + .with_source(e) }) } } @@ -1269,14 +1257,14 @@ static BUILTIN_FUNCTION_MAP: Lazy> = Lazy: let needle = parse_function_argument_to_string(name, arg, state)?; let path = RepoPathBuf::parse_fs_path(ctx.cwd, ctx.workspace_root, needle) .map_err(|e| { - RevsetParseError::with_span_and_source( + RevsetParseError::with_span( RevsetParseErrorKind::InvalidFunctionArguments { name: name.to_owned(), message: "Invalid file pattern".to_owned(), }, span, - e, ) + .with_source(e) })?; Ok(path) }) From 4dbe2eb7a671444c7149d84c25c01ec04462a3ac Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Fri, 29 Mar 2024 20:35:36 +0900 Subject: [PATCH 2/6] templater: consolidate "unexpected expression" error constructors I also renamed "UnexpectedExpression" to just "Expression" because "unexpected" doesn't apply to "immutable" revset parse/evaluation errors. --- cli/examples/custom-commit-templater/main.rs | 2 +- .../custom-operation-templater/main.rs | 2 +- cli/src/commit_templater.rs | 9 +++-- cli/src/template_builder.rs | 9 +++-- cli/src/template_parser.rs | 34 +++++-------------- 5 files changed, 20 insertions(+), 36 deletions(-) diff --git a/cli/examples/custom-commit-templater/main.rs b/cli/examples/custom-commit-templater/main.rs index ce20d7a64f..6bbfe97865 100644 --- a/cli/examples/custom-commit-templater/main.rs +++ b/cli/examples/custom-commit-templater/main.rs @@ -108,7 +108,7 @@ impl CommitTemplateLanguageExtension for HexCounter { let chars: Vec<_> = string.chars().collect(); match chars[..] { [ch] => Ok(ch), - _ => Err(TemplateParseError::unexpected_expression( + _ => Err(TemplateParseError::expression( "Expected singular character argument", span, )), diff --git a/cli/examples/custom-operation-templater/main.rs b/cli/examples/custom-operation-templater/main.rs index 562f1efb4f..2c4a29b0bf 100644 --- a/cli/examples/custom-operation-templater/main.rs +++ b/cli/examples/custom-operation-templater/main.rs @@ -68,7 +68,7 @@ impl OperationTemplateLanguageExtension for HexCounter { let chars: Vec<_> = string.chars().collect(); match chars[..] { [ch] => Ok(ch), - _ => Err(TemplateParseError::unexpected_expression( + _ => Err(TemplateParseError::expression( "Expected singular character argument", span, )), diff --git a/cli/src/commit_templater.rs b/cli/src/commit_templater.rs index 628ed8beb3..ca65f19b1b 100644 --- a/cli/src/commit_templater.rs +++ b/cli/src/commit_templater.rs @@ -636,10 +636,13 @@ fn evaluate_immutable_revset<'repo>( // It's usually smaller than the immutable set. The revset engine can also // optimize "::" query to use bitset-based implementation. let expression = revset_util::parse_immutable_expression(&language.revset_parse_context) - .map_err(|err| TemplateParseError::other("Failed to parse revset", err, span))?; + .map_err(|err| { + TemplateParseError::expression("Failed to parse revset", span).with_source(err) + })?; let symbol_resolver = revset_util::default_symbol_resolver(repo, language.id_prefix_context); - let revset = revset_util::evaluate(repo, &symbol_resolver, expression) - .map_err(|err| TemplateParseError::other("Failed to evaluate revset", err, span))?; + let revset = revset_util::evaluate(repo, &symbol_resolver, expression).map_err(|err| { + TemplateParseError::expression("Failed to evaluate revset", span).with_source(err) + })?; Ok(revset) } diff --git a/cli/src/template_builder.rs b/cli/src/template_builder.rs index a4043a9a41..1767cb1a96 100644 --- a/cli/src/template_builder.rs +++ b/cli/src/template_builder.rs @@ -682,9 +682,8 @@ fn builtin_timestamp_methods<'a, L: TemplateLanguage<'a> + ?Sized>( let [format_node] = template_parser::expect_exact_arguments(function)?; let format = template_parser::expect_string_literal_with(format_node, |format, span| { - time_util::FormattingItems::parse(format).ok_or_else(|| { - TemplateParseError::unexpected_expression("Invalid time format", span) - }) + time_util::FormattingItems::parse(format) + .ok_or_else(|| TemplateParseError::expression("Invalid time format", span)) })? .into_owned(); let out_property = self_property.and_then(move |timestamp| { @@ -847,7 +846,7 @@ where if let [name] = lambda.params.as_slice() { local_variables.insert(name, &item_fn); } else { - return Err(TemplateParseError::unexpected_expression( + return Err(TemplateParseError::expression( "Expected 1 lambda parameters", lambda.params_span, )); @@ -1028,7 +1027,7 @@ pub fn build_expression<'a, L: TemplateLanguage<'a> + ?Sized>( expression.labels.push(method.function.name.to_owned()); Ok(expression) } - ExpressionKind::Lambda(_) => Err(TemplateParseError::unexpected_expression( + ExpressionKind::Lambda(_) => Err(TemplateParseError::expression( "Lambda cannot be defined here", node.span, )), diff --git a/cli/src/template_parser.rs b/cli/src/template_parser.rs index 78bf23265b..5bf45f06c9 100644 --- a/cli/src/template_parser.rs +++ b/cli/src/template_parser.rs @@ -98,7 +98,7 @@ pub enum TemplateParseErrorKind { #[error("Redefinition of function parameter")] RedefinedFunctionParameter, #[error("{0}")] - UnexpectedExpression(String), + Expression(String), #[error(r#"Alias "{0}" cannot be expanded"#)] BadAliasExpansion(String), #[error(r#"Alias "{0}" expanded recursively"#)] @@ -151,27 +151,12 @@ impl TemplateParseError { pub fn expected_type(type_name: &str, span: pest::Span<'_>) -> Self { let message = format!(r#"Expected expression of type "{type_name}""#); - TemplateParseError::unexpected_expression(message, span) + TemplateParseError::expression(message, span) } - pub fn unexpected_expression(message: impl Into, span: pest::Span<'_>) -> Self { - TemplateParseError::with_span( - TemplateParseErrorKind::UnexpectedExpression(message.into()), - span, - ) - } - - /// Some other expression error with the underlying error `source`. - pub fn other( - message: impl Into, - source: impl Into>, - span: pest::Span<'_>, - ) -> Self { - TemplateParseError::with_span( - TemplateParseErrorKind::UnexpectedExpression(message.into()), - span, - ) - .with_source(source) + /// Some other expression error. + pub fn expression(message: impl Into, span: pest::Span<'_>) -> Self { + TemplateParseError::with_span(TemplateParseErrorKind::Expression(message.into()), span) } pub fn within_alias_expansion(self, id: TemplateAliasId<'_>, span: pest::Span<'_>) -> Self { @@ -328,10 +313,7 @@ fn parse_identifier_name(pair: Pair) -> TemplateParseResult<&str> { if let ExpressionKind::Identifier(name) = parse_identifier_or_literal(pair) { Ok(name) } else { - Err(TemplateParseError::unexpected_expression( - "Expected identifier", - span, - )) + Err(TemplateParseError::expression("Expected identifier", span)) } } @@ -853,7 +835,7 @@ pub fn expect_string_literal_with<'a, 'i, T>( | ExpressionKind::Concat(_) | ExpressionKind::FunctionCall(_) | ExpressionKind::MethodCall(_) - | ExpressionKind::Lambda(_) => Err(TemplateParseError::unexpected_expression( + | ExpressionKind::Lambda(_) => Err(TemplateParseError::expression( "Expected string literal", node.span, )), @@ -877,7 +859,7 @@ pub fn expect_lambda_with<'a, 'i, T>( | ExpressionKind::Binary(..) | ExpressionKind::Concat(_) | ExpressionKind::FunctionCall(_) - | ExpressionKind::MethodCall(_) => Err(TemplateParseError::unexpected_expression( + | ExpressionKind::MethodCall(_) => Err(TemplateParseError::expression( "Expected lambda expression", node.span, )), From bb886ac1ca2cfddc0f60b2c2ab812113f8806ea7 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Fri, 29 Mar 2024 23:00:07 +0900 Subject: [PATCH 3/6] templater: merge ParseIntError into generic Expression error It was only needed to attach the source error object, which is now handled by the outer error type. --- cli/src/template_parser.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/cli/src/template_parser.rs b/cli/src/template_parser.rs index 5bf45f06c9..8c42717a0f 100644 --- a/cli/src/template_parser.rs +++ b/cli/src/template_parser.rs @@ -75,8 +75,6 @@ pub struct TemplateParseError { pub enum TemplateParseErrorKind { #[error("Syntax error")] SyntaxError, - #[error("Invalid integer literal")] - ParseIntError, #[error(r#"Keyword "{name}" doesn't exist"#)] NoSuchKeyword { name: String, @@ -405,8 +403,7 @@ fn parse_term_node(pair: Pair) -> TemplateParseResult { } Rule::integer_literal => { let value = expr.as_str().parse().map_err(|err| { - TemplateParseError::with_span(TemplateParseErrorKind::ParseIntError, span) - .with_source(err) + TemplateParseError::expression("Invalid integer literal", span).with_source(err) })?; ExpressionNode::new(ExpressionKind::Integer(value), span) } @@ -1229,7 +1226,7 @@ mod tests { ); assert_matches!( parse_into_kind(&format!("{}", (i64::MAX as u64) + 1)), - Err(TemplateParseErrorKind::ParseIntError) + Err(TemplateParseErrorKind::Expression(_)) ); } From 813403d866a1733fb336c36154306b2331a0bab1 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Fri, 29 Mar 2024 12:50:00 +0900 Subject: [PATCH 4/6] revset: add constructor for InvalidFunctionArguments error Inlined some of the make_error() closures instead. I'll make string pattern handler preserve the source error object. --- lib/src/revset.rs | 83 ++++++++++++++++++++++------------------------- 1 file changed, 39 insertions(+), 44 deletions(-) diff --git a/lib/src/revset.rs b/lib/src/revset.rs index 841c1ca913..cc30dfa1f6 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -209,6 +209,20 @@ impl RevsetParseError { self } + fn invalid_arguments( + name: impl Into, + message: impl Into, + span: pest::Span<'_>, + ) -> Self { + Self::with_span( + RevsetParseErrorKind::InvalidFunctionArguments { + name: name.into(), + message: message.into(), + }, + span, + ) + } + pub fn kind(&self) -> &RevsetParseErrorKind { &self.kind } @@ -1257,24 +1271,16 @@ static BUILTIN_FUNCTION_MAP: Lazy> = Lazy: let needle = parse_function_argument_to_string(name, arg, state)?; let path = RepoPathBuf::parse_fs_path(ctx.cwd, ctx.workspace_root, needle) .map_err(|e| { - RevsetParseError::with_span( - RevsetParseErrorKind::InvalidFunctionArguments { - name: name.to_owned(), - message: "Invalid file pattern".to_owned(), - }, - span, - ) - .with_source(e) + RevsetParseError::invalid_arguments(name, "Invalid file pattern", span) + .with_source(e) })?; Ok(path) }) .try_collect()?; if paths.is_empty() { - Err(RevsetParseError::with_span( - RevsetParseErrorKind::InvalidFunctionArguments { - name: name.to_owned(), - message: "Expected at least 1 argument".to_string(), - }, + Err(RevsetParseError::invalid_arguments( + name, + "Expected at least 1 argument", arguments_span, )) } else { @@ -1349,22 +1355,13 @@ fn expect_named_arguments_vec<'i>( ) -> Result<(Vec>, Vec>), RevsetParseError> { assert!(argument_names.len() <= max_arg_count); let arguments_span = arguments_pair.as_span(); - let make_error = |message, span| { - RevsetParseError::with_span( - RevsetParseErrorKind::InvalidFunctionArguments { - name: function_name.to_owned(), - message, - }, - span, - ) - }; let make_count_error = || { let message = if min_arg_count == max_arg_count { format!("Expected {min_arg_count} arguments") } else { format!("Expected {min_arg_count} to {max_arg_count} arguments") }; - make_error(message, arguments_span) + RevsetParseError::invalid_arguments(function_name, message, arguments_span) }; let mut pos_iter = Some(0..max_arg_count); @@ -1376,8 +1373,9 @@ fn expect_named_arguments_vec<'i>( let pos = pos_iter .as_mut() .ok_or_else(|| { - make_error( - "Positional argument follows keyword argument".to_owned(), + RevsetParseError::invalid_arguments( + function_name, + "Positional argument follows keyword argument", span, ) })? @@ -1397,13 +1395,15 @@ fn expect_named_arguments_vec<'i>( .iter() .position(|&n| n == name.as_str()) .ok_or_else(|| { - make_error( + RevsetParseError::invalid_arguments( + function_name, format!(r#"Unexpected keyword argument "{}""#, name.as_str()), span, ) })?; if extracted_pairs[pos].is_some() { - return Err(make_error( + return Err(RevsetParseError::invalid_arguments( + function_name, format!(r#"Got multiple values for keyword "{}""#, name.as_str()), span, )); @@ -1437,16 +1437,6 @@ fn parse_function_argument_to_string_pattern( state: ParseState, ) -> Result { let span = pair.as_span(); - let make_error = |message| { - RevsetParseError::with_span( - RevsetParseErrorKind::InvalidFunctionArguments { - name: name.to_string(), - message, - }, - span, - ) - }; - let make_type_error = || make_error("Expected function argument of string pattern".to_owned()); let expression = { let mut inner_state = state; inner_state.allow_string_pattern = true; @@ -1459,9 +1449,16 @@ fn parse_function_argument_to_string_pattern( } RevsetExpression::StringPattern { kind, value } => { // TODO: error span can be narrowed to the lhs node - StringPattern::from_str_kind(value, kind).map_err(|err| make_error(err.to_string()))? + StringPattern::from_str_kind(value, kind) + .map_err(|err| RevsetParseError::invalid_arguments(name, err.to_string(), span))? + } + _ => { + return Err(RevsetParseError::invalid_arguments( + name, + "Expected function argument of string pattern", + span, + )); } - _ => return Err(make_type_error()), }; Ok(pattern) } @@ -1474,11 +1471,9 @@ fn parse_function_argument_as_literal( ) -> Result { let span = pair.as_span(); let make_error = || { - RevsetParseError::with_span( - RevsetParseErrorKind::InvalidFunctionArguments { - name: name.to_string(), - message: format!("Expected function argument of type {type_name}"), - }, + RevsetParseError::invalid_arguments( + name, + format!("Expected function argument of type {type_name}"), span, ) }; From 65eba30eae10a31fdcfa9d5b6ce242720502ef48 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Fri, 29 Mar 2024 13:07:31 +0900 Subject: [PATCH 5/6] revset: don't stringify StringPatternParseError This helps to add hint at the CLI layer. --- cli/tests/test_revset_output.rs | 8 +++++--- lib/src/revset.rs | 8 +++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/cli/tests/test_revset_output.rs b/cli/tests/test_revset_output.rs index 03e4f3e3a1..86a558cd07 100644 --- a/cli/tests/test_revset_output.rs +++ b/cli/tests/test_revset_output.rs @@ -167,13 +167,15 @@ fn test_bad_function_call() { let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "branches(bad:pattern)"]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse revset: Function "branches": Invalid string pattern kind "bad:", try prefixing with one of `exact:`, `glob:` or `substring:` - Caused by: --> 1:10 + Error: Failed to parse revset: Function "branches": Invalid string pattern + Caused by: + 1: --> 1:10 | 1 | branches(bad:pattern) | ^---------^ | - = Function "branches": Invalid string pattern kind "bad:", try prefixing with one of `exact:`, `glob:` or `substring:` + = Function "branches": Invalid string pattern + 2: Invalid string pattern kind "bad:", try prefixing with one of `exact:`, `glob:` or `substring:` "###); let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "root()::whatever()"]); diff --git a/lib/src/revset.rs b/lib/src/revset.rs index cc30dfa1f6..de61b438cc 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -1449,8 +1449,10 @@ fn parse_function_argument_to_string_pattern( } RevsetExpression::StringPattern { kind, value } => { // TODO: error span can be narrowed to the lhs node - StringPattern::from_str_kind(value, kind) - .map_err(|err| RevsetParseError::invalid_arguments(name, err.to_string(), span))? + StringPattern::from_str_kind(value, kind).map_err(|err| { + RevsetParseError::invalid_arguments(name, "Invalid string pattern", span) + .with_source(err) + })? } _ => { return Err(RevsetParseError::invalid_arguments( @@ -2926,7 +2928,7 @@ mod tests { parse(r#"branches(bad:"foo")"#), Err(RevsetParseErrorKind::InvalidFunctionArguments { name: "branches".to_owned(), - message: r#"Invalid string pattern kind "bad:", try prefixing with one of `exact:`, `glob:` or `substring:`"#.to_owned() + message: "Invalid string pattern".to_owned() }) ); assert_eq!( From 426b18d8d591c25fda0feb94db633916cc616941 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Wed, 6 Mar 2024 17:49:15 +0900 Subject: [PATCH 6/6] cli: render string pattern suggestion as a hint Templater doesn't have the one yet, but I think it belongs to the same category. For clap::Error, we could use clap's own mechanism to render suggestions as "tip: ...", but I feel "Hint: ..." looks better because our error/hint message is capitalized. --- cli/src/command_error.rs | 29 ++++++++++++++++++++++++++++- cli/tests/test_branch_command.rs | 3 ++- cli/tests/test_revset_output.rs | 3 ++- lib/src/str_util.rs | 2 +- 4 files changed, 33 insertions(+), 4 deletions(-) diff --git a/cli/src/command_error.rs b/cli/src/command_error.rs index 0083e2d06a..d0aa801ab5 100644 --- a/cli/src/command_error.rs +++ b/cli/src/command_error.rs @@ -30,6 +30,7 @@ use jj_lib::revset::{ RevsetEvaluationError, RevsetParseError, RevsetParseErrorKind, RevsetResolutionError, }; use jj_lib::signing::SignInitError; +use jj_lib::str_util::StringPatternParseError; use jj_lib::working_copy::{ResetError, SnapshotError, WorkingCopyStateError}; use jj_lib::workspace::WorkspaceInitError; use thiserror::Error; @@ -423,6 +424,9 @@ impl From for CommandError { name: _, candidates, } => format_similarity_hint(candidates), + RevsetParseErrorKind::InvalidFunctionArguments { .. } => { + find_source_parse_error_hint(bottom_err) + } _ => None, }; let mut cmd_err = @@ -470,6 +474,8 @@ impl From for CommandError { | TemplateParseErrorKind::NoSuchMethod { candidates, .. } => { format_similarity_hint(candidates) } + TemplateParseErrorKind::InvalidArguments { .. } + | TemplateParseErrorKind::Expression(_) => find_source_parse_error_hint(bottom_err), _ => None, }; let mut cmd_err = @@ -489,7 +495,10 @@ impl From for CommandError { impl From for CommandError { fn from(err: clap::Error) -> Self { - cli_error(err) + let hint = find_source_parse_error_hint(&err); + let mut cmd_err = cli_error(err); + cmd_err.extend_hints(hint); + cmd_err } } @@ -511,6 +520,24 @@ impl From for CommandError { } } +fn find_source_parse_error_hint(err: &dyn error::Error) -> Option { + let source = err.source()?; + if let Some(source) = source.downcast_ref() { + string_pattern_parse_error_hint(source) + } else { + None + } +} + +fn string_pattern_parse_error_hint(err: &StringPatternParseError) -> Option { + match err { + StringPatternParseError::InvalidKind(_) => { + Some("Try prefixing with one of `exact:`, `glob:` or `substring:`".into()) + } + StringPatternParseError::GlobPattern(_) => None, + } +} + const BROKEN_PIPE_EXIT_CODE: u8 = 3; pub(crate) fn handle_command_result(ui: &mut Ui, result: Result<(), CommandError>) -> ExitCode { diff --git a/cli/tests/test_branch_command.rs b/cli/tests/test_branch_command.rs index b31f390378..decf83f0d8 100644 --- a/cli/tests/test_branch_command.rs +++ b/cli/tests/test_branch_command.rs @@ -396,9 +396,10 @@ fn test_branch_delete_glob() { // Unknown pattern kind let stderr = test_env.jj_cmd_cli_error(&repo_path, &["branch", "forget", "whatever:branch"]); insta::assert_snapshot!(stderr, @r###" - error: invalid value 'whatever:branch' for '[NAMES]...': Invalid string pattern kind "whatever:", try prefixing with one of `exact:`, `glob:` or `substring:` + error: invalid value 'whatever:branch' for '[NAMES]...': Invalid string pattern kind "whatever:" For more information, try '--help'. + Hint: Try prefixing with one of `exact:`, `glob:` or `substring:` "###); } diff --git a/cli/tests/test_revset_output.rs b/cli/tests/test_revset_output.rs index 86a558cd07..2d3bdd5afb 100644 --- a/cli/tests/test_revset_output.rs +++ b/cli/tests/test_revset_output.rs @@ -175,7 +175,8 @@ fn test_bad_function_call() { | ^---------^ | = Function "branches": Invalid string pattern - 2: Invalid string pattern kind "bad:", try prefixing with one of `exact:`, `glob:` or `substring:` + 2: Invalid string pattern kind "bad:" + Hint: Try prefixing with one of `exact:`, `glob:` or `substring:` "###); let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "root()::whatever()"]); diff --git a/lib/src/str_util.rs b/lib/src/str_util.rs index 1df3fdf2fc..2cef8bae9b 100644 --- a/lib/src/str_util.rs +++ b/lib/src/str_util.rs @@ -25,7 +25,7 @@ use thiserror::Error; #[derive(Debug, Error)] pub enum StringPatternParseError { /// Unknown pattern kind is specified. - #[error(r#"Invalid string pattern kind "{0}:", try prefixing with one of `exact:`, `glob:` or `substring:`"#)] + #[error(r#"Invalid string pattern kind "{0}:""#)] InvalidKind(String), /// Failed to parse glob pattern. #[error(transparent)]