From 2cd70bdf14161abcf38516e596e8f6b49c52d25f Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Tue, 26 Mar 2024 00:54:39 +0900 Subject: [PATCH] revset, templater: render parse error as usual error chain Because the CLI error handler now prints error sources in multi-line format, it doesn't make much sense to render Revset/TemplateParseError differently. This patch also fixes the source() of the SyntaxError kind. It should be self.pest_error.source() (= None), not self.pest_error. --- cli/src/cli_util.rs | 5 +- cli/src/command_error.rs | 11 ++-- cli/src/commit_templater.rs | 6 +- cli/src/revset_util.rs | 6 -- cli/src/template_builder.rs | 2 +- cli/src/template_parser.rs | 31 ++++++---- cli/tests/test_commit_template.rs | 20 +++++-- cli/tests/test_log_command.rs | 2 +- cli/tests/test_revset_output.rs | 96 ++++++++++++++++++++----------- cli/tests/test_templater.rs | 66 +++++++++++++-------- lib/src/revset.rs | 18 +++--- 11 files changed, 160 insertions(+), 103 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 5d7a869f4..c11b86fd2 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -909,10 +909,7 @@ impl WorkspaceCommandHelper { .unwrap_or_else(|_| self.settings.default_revset()); if !revset_string.is_empty() { let disambiguation_revset = self.parse_revset(&revset_string).map_err(|err| { - config_error_with_message( - "Invalid `revsets.short-prefixes`", - revset_util::format_parse_error(&err), - ) + config_error_with_message("Invalid `revsets.short-prefixes`", err) })?; context = context.disambiguate_within(revset::optimize(disambiguation_revset)); } diff --git a/cli/src/command_error.rs b/cli/src/command_error.rs index f4a9052c0..eb4c2a836 100644 --- a/cli/src/command_error.rs +++ b/cli/src/command_error.rs @@ -39,7 +39,7 @@ use crate::formatter::{FormatRecorder, Formatter}; use crate::merge_tools::{ ConflictResolveError, DiffEditError, DiffGenerateError, MergeToolConfigError, }; -use crate::revset_util::{self, UserRevsetEvaluationError}; +use crate::revset_util::UserRevsetEvaluationError; use crate::template_parser::{TemplateParseError, TemplateParseErrorKind}; use crate::ui::Ui; @@ -421,7 +421,7 @@ impl From for CommandError { } => format_similarity_hint(candidates), _ => None, }; - let mut cmd_err = user_error(revset_util::format_parse_error(&err)); + let mut cmd_err = user_error_with_message("Failed to parse revset", err); cmd_err.extend_hints(hint); cmd_err } @@ -457,10 +457,9 @@ impl From for CommandError { impl From for CommandError { fn from(err: TemplateParseError) -> Self { - let err_chain = iter::successors(Some(&err), |e| e.origin()); - let message = err_chain.clone().join("\n"); // Only for the bottom error, which is usually the root cause - let hint = match err_chain.last().unwrap().kind() { + let bottom_err = iter::successors(Some(&err), |e| e.origin()).last().unwrap(); + let hint = match bottom_err.kind() { TemplateParseErrorKind::NoSuchKeyword { candidates, .. } | TemplateParseErrorKind::NoSuchFunction { candidates, .. } | TemplateParseErrorKind::NoSuchMethod { candidates, .. } => { @@ -468,7 +467,7 @@ impl From for CommandError { } _ => None, }; - let mut cmd_err = user_error(format!("Failed to parse template: {message}")); + let mut cmd_err = user_error_with_message("Failed to parse template", err); cmd_err.extend_hints(hint); cmd_err } diff --git a/cli/src/commit_templater.rs b/cli/src/commit_templater.rs index fb1581b3c..9d1632e4a 100644 --- a/cli/src/commit_templater.rs +++ b/cli/src/commit_templater.rs @@ -637,12 +637,10 @@ 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::unexpected_expression(revset_util::format_parse_error(&err), span) - })?; + .map_err(|err| TemplateParseError::other("Failed to parse revset", err, span))?; 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::unexpected_expression(err.to_string(), span))?; + .map_err(|err| TemplateParseError::other("Failed to evaluate revset", err, span))?; Ok(revset) } diff --git a/cli/src/revset_util.rs b/cli/src/revset_util.rs index 49ffbba09..c74efebe8 100644 --- a/cli/src/revset_util.rs +++ b/cli/src/revset_util.rs @@ -14,7 +14,6 @@ //! Utility for parsing and evaluating user-provided revset expressions. -use std::iter; use std::rc::Rc; use itertools::Itertools as _; @@ -128,8 +127,3 @@ pub fn parse_immutable_expression( let heads = revset::parse(immutable_heads_str, context)?; Ok(heads.union(&RevsetExpression::root()).ancestors()) } - -pub fn format_parse_error(err: &RevsetParseError) -> String { - let message = iter::successors(Some(err), |e| e.origin()).join("\n"); - format!("Failed to parse revset: {message}") -} diff --git a/cli/src/template_builder.rs b/cli/src/template_builder.rs index db9788e10..3f3078ffd 100644 --- a/cli/src/template_builder.rs +++ b/cli/src/template_builder.rs @@ -1328,7 +1328,7 @@ mod tests { 1 | 10000000000000000000 | ^------------------^ | - = Invalid integer literal: number too large to fit in target type + = Invalid integer literal "###); insta::assert_snapshot!(env.parse_err(r#"42.foo()"#), @r###" --> 1:4 diff --git a/cli/src/template_parser.rs b/cli/src/template_parser.rs index 89999b475..aa56336ba 100644 --- a/cli/src/template_parser.rs +++ b/cli/src/template_parser.rs @@ -14,7 +14,7 @@ use std::collections::HashMap; use std::num::ParseIntError; -use std::{error, fmt, iter, mem}; +use std::{error, fmt, mem}; use itertools::Itertools as _; use once_cell::sync::Lazy; @@ -107,7 +107,7 @@ pub enum TemplateParseErrorKind { impl TemplateParseError { pub fn with_span(kind: TemplateParseErrorKind, span: pest::Span<'_>) -> Self { - let message = iter::successors(Some(&kind as &dyn error::Error), |e| e.source()).join(": "); + let message = kind.to_string(); let pest_error = Box::new(pest::error::Error::new_from_span( pest::error::ErrorVariant::CustomError { message }, span, @@ -124,7 +124,7 @@ impl TemplateParseError { span: pest::Span<'_>, source: impl Into>, ) -> Self { - let message = iter::successors(Some(&kind as &dyn error::Error), |e| e.source()).join(": "); + let message = kind.to_string(); let pest_error = Box::new(pest::error::Error::new_from_span( pest::error::ErrorVariant::CustomError { message }, span, @@ -173,6 +173,19 @@ impl TemplateParseError { ) } + /// 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_and_source( + TemplateParseErrorKind::UnexpectedExpression(message.into()), + span, + source, + ) + } + pub fn within_alias_expansion(self, id: TemplateAliasId<'_>, span: pest::Span<'_>) -> Self { TemplateParseError::with_span_and_source( TemplateParseErrorKind::BadAliasExpansion(id.to_string()), @@ -248,13 +261,11 @@ impl fmt::Display for TemplateParseError { impl error::Error for TemplateParseError { fn source(&self) -> Option<&(dyn error::Error + 'static)> { if let Some(e) = self.source.as_deref() { - return Some(e); - } - match &self.kind { - // SyntaxError is a wrapper for pest::error::Error. - TemplateParseErrorKind::SyntaxError => Some(&self.pest_error as &dyn error::Error), - // Otherwise the kind represents this error. - e => e.source(), + Some(e) + } else { + // SyntaxError originates from self.pest_error, but + // pest::error::Error doesn't have a source anyway. + self.kind.source() } } } diff --git a/cli/tests/test_commit_template.rs b/cli/tests/test_commit_template.rs index 5dfa90dcc..9c64e17e1 100644 --- a/cli/tests/test_commit_template.rs +++ b/cli/tests/test_commit_template.rs @@ -54,7 +54,8 @@ fn test_log_parents() { // Commit object isn't printable let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-T", "parents"]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse template: --> 1:1 + Error: Failed to parse template + Caused by: --> 1:1 | 1 | parents | ^-----^ @@ -66,7 +67,8 @@ fn test_log_parents() { let template = r#"parents.map(|c| c.commit_id(""))"#; let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-T", template]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse template: --> 1:29 + Error: Failed to parse template + Caused by: --> 1:29 | 1 | parents.map(|c| c.commit_id("")) | ^^ @@ -597,12 +599,15 @@ fn test_log_immutable() { test_env.add_config("revset-aliases.'immutable_heads()' = 'unknown_fn()'"); let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r::", "-T", template]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse template: --> 5:10 + Error: Failed to parse template + Caused by: + 1: --> 5:10 | 5 | if(immutable, "[immutable]"), | ^-------^ | - = Failed to parse revset: --> 1:1 + = Failed to parse revset + 2: --> 1:1 | 1 | unknown_fn() | ^--------^ @@ -613,11 +618,14 @@ fn test_log_immutable() { test_env.add_config("revset-aliases.'immutable_heads()' = 'unknown_symbol'"); let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r::", "-T", template]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse template: --> 5:10 + Error: Failed to parse template + Caused by: + 1: --> 5:10 | 5 | if(immutable, "[immutable]"), | ^-------^ | - = Revision "unknown_symbol" doesn't exist + = Failed to evaluate revset + 2: Revision "unknown_symbol" doesn't exist "###); } diff --git a/cli/tests/test_log_command.rs b/cli/tests/test_log_command.rs index af5d6c9a1..0782d6ba3 100644 --- a/cli/tests/test_log_command.rs +++ b/cli/tests/test_log_command.rs @@ -446,7 +446,7 @@ fn test_log_bad_short_prefixes() { insta::assert_snapshot!(stderr, @r###" Config error: Invalid `revsets.short-prefixes` - Caused by: Failed to parse revset: --> 1:1 + Caused by: --> 1:1 | 1 | !nval!d | ^--- diff --git a/cli/tests/test_revset_output.rs b/cli/tests/test_revset_output.rs index 6aba204a7..4d17b4ec6 100644 --- a/cli/tests/test_revset_output.rs +++ b/cli/tests/test_revset_output.rs @@ -22,7 +22,8 @@ fn test_syntax_error() { let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", ":x"]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse revset: --> 1:1 + Error: Failed to parse revset + Caused by: --> 1:1 | 1 | :x | ^ @@ -33,7 +34,8 @@ fn test_syntax_error() { let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "x &"]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse revset: --> 1:4 + Error: Failed to parse revset + Caused by: --> 1:4 | 1 | x & | ^--- @@ -43,7 +45,8 @@ fn test_syntax_error() { let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "x - y"]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse revset: --> 1:3 + Error: Failed to parse revset + Caused by: --> 1:3 | 1 | x - y | ^ @@ -54,7 +57,8 @@ fn test_syntax_error() { let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "HEAD^"]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse revset: --> 1:5 + Error: Failed to parse revset + Caused by: --> 1:5 | 1 | HEAD^ | ^ @@ -72,7 +76,8 @@ fn test_bad_function_call() { let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "all(or::nothing)"]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse revset: --> 1:5 + Error: Failed to parse revset + Caused by: --> 1:5 | 1 | all(or::nothing) | ^---------^ @@ -82,7 +87,8 @@ fn test_bad_function_call() { let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "parents()"]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse revset: --> 1:9 + Error: Failed to parse revset + Caused by: --> 1:9 | 1 | parents() | ^ @@ -92,7 +98,8 @@ fn test_bad_function_call() { let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "parents(foo, bar)"]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse revset: --> 1:9 + Error: Failed to parse revset + Caused by: --> 1:9 | 1 | parents(foo, bar) | ^------^ @@ -102,7 +109,8 @@ fn test_bad_function_call() { let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "heads(foo, bar)"]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse revset: --> 1:7 + Error: Failed to parse revset + Caused by: --> 1:7 | 1 | heads(foo, bar) | ^------^ @@ -112,7 +120,8 @@ fn test_bad_function_call() { let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "latest(a, not_an_integer)"]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse revset: --> 1:11 + Error: Failed to parse revset + Caused by: --> 1:11 | 1 | latest(a, not_an_integer) | ^------------^ @@ -122,7 +131,8 @@ fn test_bad_function_call() { let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "file()"]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse revset: --> 1:6 + Error: Failed to parse revset + Caused by: --> 1:6 | 1 | file() | ^ @@ -132,7 +142,8 @@ fn test_bad_function_call() { let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "file(a, not:a-string)"]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse revset: --> 1:9 + Error: Failed to parse revset + Caused by: --> 1:9 | 1 | file(a, not:a-string) | ^----------^ @@ -142,17 +153,22 @@ fn test_bad_function_call() { let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", r#"file(a, "../out")"#]); insta::assert_snapshot!(stderr.replace('\\', "/"), @r###" - Error: Failed to parse revset: --> 1:9 + Error: Failed to parse revset + Caused by: + 1: --> 1:9 | 1 | file(a, "../out") | ^------^ | - = Invalid file pattern: Path "../out" is not in the repo ".": Invalid component ".." in repo-relative path "../out" + = Invalid file pattern + 2: Path "../out" is not in the repo "." + 3: Invalid component ".." in repo-relative path "../out" "###); let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "branches(bad:pattern)"]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse revset: --> 1:10 + Error: Failed to parse revset + Caused by: --> 1:10 | 1 | branches(bad:pattern) | ^---------^ @@ -162,7 +178,8 @@ fn test_bad_function_call() { let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "root()::whatever()"]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse revset: --> 1:9 + Error: Failed to parse revset + Caused by: --> 1:9 | 1 | root()::whatever() | ^------^ @@ -175,7 +192,8 @@ fn test_bad_function_call() { &["log", "-r", "remote_branches(a, b, remote=c)"], ); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse revset: --> 1:23 + Error: Failed to parse revset + Caused by: --> 1:23 | 1 | remote_branches(a, b, remote=c) | ^------^ @@ -186,7 +204,8 @@ fn test_bad_function_call() { let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "remote_branches(remote=a, b)"]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse revset: --> 1:27 + Error: Failed to parse revset + Caused by: --> 1:27 | 1 | remote_branches(remote=a, b) | ^ @@ -196,7 +215,8 @@ fn test_bad_function_call() { let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "remote_branches(=foo)"]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse revset: --> 1:17 + Error: Failed to parse revset + Caused by: --> 1:17 | 1 | remote_branches(=foo) | ^--- @@ -206,7 +226,8 @@ fn test_bad_function_call() { let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "remote_branches(remote=)"]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse revset: --> 1:24 + Error: Failed to parse revset + Caused by: --> 1:24 | 1 | remote_branches(remote=) | ^--- @@ -234,7 +255,8 @@ fn test_function_name_hint() { // The suggestion "branches" shouldn't be duplicated insta::assert_snapshot!(evaluate_err("branch()"), @r###" - Error: Failed to parse revset: --> 1:1 + Error: Failed to parse revset + Caused by: --> 1:1 | 1 | branch() | ^----^ @@ -245,7 +267,8 @@ fn test_function_name_hint() { // Both builtin function and function alias should be suggested insta::assert_snapshot!(evaluate_err("author_()"), @r###" - Error: Failed to parse revset: --> 1:1 + Error: Failed to parse revset + Caused by: --> 1:1 | 1 | author_() | ^-----^ @@ -255,13 +278,15 @@ fn test_function_name_hint() { "###); insta::assert_snapshot!(evaluate_err("my_branches"), @r###" - Error: Failed to parse revset: --> 1:1 + Error: Failed to parse revset + Caused by: + 1: --> 1:1 | 1 | my_branches | ^---------^ | = Alias "my_branches" cannot be expanded - --> 1:1 + 2: --> 1:1 | 1 | branch() | ^----^ @@ -302,13 +327,15 @@ fn test_alias() { let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "root() & syntax-error"]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse revset: --> 1:10 + Error: Failed to parse revset + Caused by: + 1: --> 1:10 | 1 | root() & syntax-error | ^----------^ | = Alias "syntax-error" cannot be expanded - --> 1:11 + 2: --> 1:11 | 1 | whatever & | ^--- @@ -318,7 +345,8 @@ fn test_alias() { let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "identity()"]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse revset: --> 1:10 + Error: Failed to parse revset + Caused by: --> 1:10 | 1 | identity() | ^ @@ -328,13 +356,15 @@ fn test_alias() { let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "my_author(none())"]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse revset: --> 1:1 + Error: Failed to parse revset + Caused by: + 1: --> 1:1 | 1 | my_author(none()) | ^---------------^ | = Alias "my_author()" cannot be expanded - --> 1:8 + 2: --> 1:8 | 1 | author(x) | ^ @@ -344,25 +374,27 @@ fn test_alias() { let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "root() & recurse"]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse revset: --> 1:10 + Error: Failed to parse revset + Caused by: + 1: --> 1:10 | 1 | root() & recurse | ^-----^ | = Alias "recurse" cannot be expanded - --> 1:1 + 2: --> 1:1 | 1 | recurse1 | ^------^ | = Alias "recurse1" cannot be expanded - --> 1:1 + 3: --> 1:1 | 1 | recurse2() | ^--------^ | = Alias "recurse2()" cannot be expanded - --> 1:1 + 4: --> 1:1 | 1 | recurse | ^-----^ diff --git a/cli/tests/test_templater.rs b/cli/tests/test_templater.rs index da8c4d982..9248177fb 100644 --- a/cli/tests/test_templater.rs +++ b/cli/tests/test_templater.rs @@ -24,7 +24,8 @@ fn test_templater_parse_error() { let render_err = |template| test_env.jj_cmd_failure(&repo_path, &["log", "-T", template]); insta::assert_snapshot!(render_err(r#"description ()"#), @r###" - Error: Failed to parse template: --> 1:13 + Error: Failed to parse template + Caused by: --> 1:13 | 1 | description () | ^--- @@ -43,7 +44,8 @@ fn test_templater_parse_error() { "###, ); insta::assert_snapshot!(render_err(r#"conflicts"#), @r###" - Error: Failed to parse template: --> 1:1 + Error: Failed to parse template + Caused by: --> 1:1 | 1 | conflicts | ^-------^ @@ -52,7 +54,8 @@ fn test_templater_parse_error() { Hint: Did you mean "conflict", "conflicting"? "###); insta::assert_snapshot!(render_err(r#"commit_id.shorter()"#), @r###" - Error: Failed to parse template: --> 1:11 + Error: Failed to parse template + Caused by: --> 1:11 | 1 | commit_id.shorter() | ^-----^ @@ -61,7 +64,8 @@ fn test_templater_parse_error() { Hint: Did you mean "short", "shortest"? "###); insta::assert_snapshot!(render_err(r#"oncat()"#), @r###" - Error: Failed to parse template: --> 1:1 + Error: Failed to parse template + Caused by: --> 1:1 | 1 | oncat() | ^---^ @@ -70,7 +74,8 @@ fn test_templater_parse_error() { Hint: Did you mean "concat", "socat"? "###); insta::assert_snapshot!(render_err(r#""".lines().map(|s| se)"#), @r###" - Error: Failed to parse template: --> 1:20 + Error: Failed to parse template + Caused by: --> 1:20 | 1 | "".lines().map(|s| se) | ^^ @@ -79,13 +84,15 @@ fn test_templater_parse_error() { Hint: Did you mean "s", "self"? "###); insta::assert_snapshot!(render_err(r#"format_id(commit_id)"#), @r###" - Error: Failed to parse template: --> 1:1 + Error: Failed to parse template + Caused by: + 1: --> 1:1 | 1 | format_id(commit_id) | ^------------------^ | = Alias "format_id()" cannot be expanded - --> 1:4 + 2: --> 1:4 | 1 | id.sort() | ^--^ @@ -97,7 +104,8 @@ fn test_templater_parse_error() { // -Tbuiltin shows the predefined builtin_* aliases. This isn't 100% // guaranteed, but is nice. insta::assert_snapshot!(render_err(r#"builtin"#), @r###" - Error: Failed to parse template: --> 1:1 + Error: Failed to parse template + Caused by: --> 1:1 | 1 | builtin | ^-----^ @@ -147,13 +155,15 @@ fn test_templater_alias() { insta::assert_snapshot!(render("identity(my_commit_id)"), @"000000000000"); insta::assert_snapshot!(render_err("commit_id ++ syntax_error"), @r###" - Error: Failed to parse template: --> 1:14 + Error: Failed to parse template + Caused by: + 1: --> 1:14 | 1 | commit_id ++ syntax_error | ^----------^ | = Alias "syntax_error" cannot be expanded - --> 1:5 + 2: --> 1:5 | 1 | foo. | ^--- @@ -162,13 +172,15 @@ fn test_templater_alias() { "###); insta::assert_snapshot!(render_err("commit_id ++ name_error"), @r###" - Error: Failed to parse template: --> 1:14 + Error: Failed to parse template + Caused by: + 1: --> 1:14 | 1 | commit_id ++ name_error | ^--------^ | = Alias "name_error" cannot be expanded - --> 1:1 + 2: --> 1:1 | 1 | unknown_id | ^--------^ @@ -177,19 +189,21 @@ fn test_templater_alias() { "###); insta::assert_snapshot!(render_err(r#"identity(identity(commit_id.short("")))"#), @r###" - Error: Failed to parse template: --> 1:1 + Error: Failed to parse template + Caused by: + 1: --> 1:1 | 1 | identity(identity(commit_id.short(""))) | ^-------------------------------------^ | = Alias "identity()" cannot be expanded - --> 1:10 + 2: --> 1:10 | 1 | identity(identity(commit_id.short(""))) | ^---------------------------^ | = Alias "identity()" cannot be expanded - --> 1:35 + 3: --> 1:35 | 1 | identity(identity(commit_id.short(""))) | ^^ @@ -198,25 +212,27 @@ fn test_templater_alias() { "###); insta::assert_snapshot!(render_err("commit_id ++ recurse"), @r###" - Error: Failed to parse template: --> 1:14 + Error: Failed to parse template + Caused by: + 1: --> 1:14 | 1 | commit_id ++ recurse | ^-----^ | = Alias "recurse" cannot be expanded - --> 1:1 + 2: --> 1:1 | 1 | recurse1 | ^------^ | = Alias "recurse1" cannot be expanded - --> 1:1 + 3: --> 1:1 | 1 | recurse2() | ^--------^ | = Alias "recurse2()" cannot be expanded - --> 1:1 + 4: --> 1:1 | 1 | recurse | ^-----^ @@ -225,7 +241,8 @@ fn test_templater_alias() { "###); insta::assert_snapshot!(render_err("identity()"), @r###" - Error: Failed to parse template: --> 1:10 + Error: Failed to parse template + Caused by: --> 1:10 | 1 | identity() | ^ @@ -233,7 +250,8 @@ fn test_templater_alias() { = Function "identity": Expected 1 arguments "###); insta::assert_snapshot!(render_err("identity(commit_id, commit_id)"), @r###" - Error: Failed to parse template: --> 1:10 + Error: Failed to parse template + Caused by: --> 1:10 | 1 | identity(commit_id, commit_id) | ^------------------^ @@ -242,13 +260,15 @@ fn test_templater_alias() { "###); insta::assert_snapshot!(render_err(r#"coalesce(label("x", "not boolean"), "")"#), @r###" - Error: Failed to parse template: --> 1:1 + Error: Failed to parse template + Caused by: + 1: --> 1:1 | 1 | coalesce(label("x", "not boolean"), "") | ^-------------------------------------^ | = Alias "coalesce()" cannot be expanded - --> 1:10 + 2: --> 1:10 | 1 | coalesce(label("x", "not boolean"), "") | ^-----------------------^ diff --git a/lib/src/revset.rs b/lib/src/revset.rs index 23ce56df5..de8092f7a 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -21,7 +21,7 @@ use std::path::Path; use std::rc::Rc; use std::str::FromStr; use std::sync::Arc; -use std::{error, fmt, iter}; +use std::{error, fmt}; use itertools::Itertools; use once_cell::sync::Lazy; @@ -193,7 +193,7 @@ pub enum RevsetParseErrorKind { impl RevsetParseError { fn with_span(kind: RevsetParseErrorKind, span: pest::Span<'_>) -> Self { - let message = iter::successors(Some(&kind as &dyn error::Error), |e| e.source()).join(": "); + let message = kind.to_string(); let pest_error = Box::new(pest::error::Error::new_from_span( pest::error::ErrorVariant::CustomError { message }, span, @@ -210,7 +210,7 @@ impl RevsetParseError { span: pest::Span<'_>, source: impl Into>, ) -> Self { - let message = iter::successors(Some(&kind as &dyn error::Error), |e| e.source()).join(": "); + let message = kind.to_string(); let pest_error = Box::new(pest::error::Error::new_from_span( pest::error::ErrorVariant::CustomError { message }, span, @@ -251,13 +251,11 @@ impl fmt::Display for RevsetParseError { impl error::Error for RevsetParseError { fn source(&self) -> Option<&(dyn error::Error + 'static)> { if let Some(e) = self.source.as_deref() { - return Some(e); - } - match &self.kind { - // SyntaxError is a wrapper for pest::error::Error. - RevsetParseErrorKind::SyntaxError => Some(&self.pest_error as &dyn error::Error), - // Otherwise the kind represents this error. - e => e.source(), + Some(e) + } else { + // SyntaxError originates from self.pest_error, but + // pest::error::Error doesn't have a source anyway. + self.kind.source() } } }