diff --git a/cli/src/template_builder.rs b/cli/src/template_builder.rs index 84f93a7be..0e7f57684 100644 --- a/cli/src/template_builder.rs +++ b/cli/src/template_builder.rs @@ -192,6 +192,14 @@ impl<'a, I: 'a> IntoTemplateProperty<'a, I> for CoreTemplatePropertyKind<'a, I> match self { CoreTemplatePropertyKind::String(property) => Some(property), _ => { + // TODO: Runtime property evaluation error will be materialized + // as string here. Ideally, the error should propagate because + // the caller expects a value, not a template to render. Some + // ideas to work around the problem: + // 1. stringify property without using Template type (works only for + // non-template expressions) + // 2. add flag to propagate property error as io::Error::other() (e.g. + // Template::format(context, formatter, propagate_err)) let template = self.try_into_template()?; Some(Box::new(PlainTextFormattedProperty::new(template))) } @@ -841,8 +849,12 @@ fn build_global_function<'a, L: TemplateLanguage<'a>>( let width = expect_integer_expression(language, build_ctx, width_node)?; let content = expect_template_expression(language, build_ctx, content_node)?; let template = ReformatTemplate::new(content, move |context, formatter, recorded| { - let width = width.extract(context).try_into().unwrap_or(0); - text_util::write_wrapped(formatter, recorded, width) + match width.extract(context) { + Ok(width) => { + text_util::write_wrapped(formatter, recorded, width.try_into().unwrap_or(0)) + } + Err(err) => err.format(&(), formatter), + } }); language.wrap_template(Box::new(template)) } @@ -852,12 +864,6 @@ fn build_global_function<'a, L: TemplateLanguage<'a>>( let content = expect_template_expression(language, build_ctx, content_node)?; let template = ReformatTemplate::new(content, move |context, formatter, recorded| { text_util::write_indented(formatter, recorded, |formatter| { - // If Template::format() returned a custom error type, we would need to - // handle template evaluation error out of this closure: - // prefix.format(context, &mut prefix_recorder)?; - // write_indented(formatter, recorded, |formatter| { - // prefix_recorder.replay(formatter) - // }) prefix.format(context, formatter) }) }); diff --git a/cli/src/templater.rs b/cli/src/templater.rs index 623692e6d..b52aaec5d 100644 --- a/cli/src/templater.rs +++ b/cli/src/templater.rs @@ -13,8 +13,8 @@ // limitations under the License. use std::cell::RefCell; -use std::io; use std::rc::Rc; +use std::{error, io, iter}; use jj_lib::backend::{Signature, Timestamp}; @@ -157,7 +157,10 @@ where L: TemplateProperty>, { fn format(&self, context: &C, formatter: &mut dyn Formatter) -> io::Result<()> { - let labels = self.labels.extract(context); + let labels = match self.labels.extract(context) { + Ok(labels) => labels, + Err(err) => return err.format(&(), formatter), + }; for label in &labels { formatter.push_label(label)?; } @@ -256,16 +259,39 @@ where } } +/// Wrapper around an error occurred during template evaluation. +#[derive(Debug)] +pub struct TemplatePropertyError(pub Box); + +// Implements conversion from any error type to support `expr?` in function +// binding. This type doesn't implement `std::error::Error` instead. +// https://github.com/dtolnay/anyhow/issues/25#issuecomment-544140480 +impl From for TemplatePropertyError +where + E: error::Error + Send + Sync + 'static, +{ + fn from(err: E) -> Self { + TemplatePropertyError(err.into()) + } +} + +/// Prints the evaluation error as inline template output. +impl Template<()> for TemplatePropertyError { + fn format(&self, _: &(), formatter: &mut dyn Formatter) -> io::Result<()> { + format_error_inline(formatter, &*self.0) + } +} + pub trait TemplateProperty { type Output; - fn extract(&self, context: &C) -> Self::Output; + fn extract(&self, context: &C) -> Result; } impl + ?Sized> TemplateProperty for Box

{ type Output =

>::Output; - fn extract(&self, context: &C) -> Self::Output { + fn extract(&self, context: &C) -> Result {

>::extract(self, context) } } @@ -273,8 +299,10 @@ impl + ?Sized> TemplateProperty for Box

{ impl> TemplateProperty for Option

{ type Output = Option; - fn extract(&self, context: &C) -> Self::Output { - self.as_ref().map(|property| property.extract(context)) + fn extract(&self, context: &C) -> Result { + self.as_ref() + .map(|property| property.extract(context)) + .transpose() } } @@ -285,8 +313,8 @@ macro_rules! tuple_impls { impl,)+> TemplateProperty for ($($T,)+) { type Output = ($($T::Output,)+); - fn extract(&self, context: &C) -> Self::Output { - ($(self.$n.extract(context),)+) + fn extract(&self, context: &C) -> Result { + Ok(($(self.$n.extract(context)?,)+)) } } )+ @@ -312,8 +340,8 @@ impl> Template for Literal { impl TemplateProperty for Literal { type Output = O; - fn extract(&self, _context: &C) -> O { - self.0.clone() + fn extract(&self, _context: &C) -> Result { + Ok(self.0.clone()) } } @@ -323,8 +351,8 @@ pub struct TemplatePropertyFn(pub F); impl O> TemplateProperty for TemplatePropertyFn { type Output = O; - fn extract(&self, context: &C) -> Self::Output { - (self.0)(context) + fn extract(&self, context: &C) -> Result { + Ok((self.0)(context)) } } @@ -349,8 +377,10 @@ where P::Output: Template<()>, { fn format(&self, context: &C, formatter: &mut dyn Formatter) -> io::Result<()> { - let template = self.property.extract(context); - template.format(&(), formatter) + match self.property.extract(context) { + Ok(template) => template.format(&(), formatter), + Err(err) => err.format(&(), formatter), + } } } @@ -377,13 +407,13 @@ impl PlainTextFormattedProperty { impl> TemplateProperty for PlainTextFormattedProperty { type Output = String; - fn extract(&self, context: &C) -> Self::Output { + fn extract(&self, context: &C) -> Result { let mut output = vec![]; self.template .format(context, &mut PlainTextFormatter::new(&mut output)) .expect("write() to PlainTextFormatter should never fail"); // TODO: Use from_utf8_lossy() if we added template that embeds file content - String::from_utf8(output).expect("template output should be utf-8 bytes") + Ok(String::from_utf8(output).expect("template output should be utf-8 bytes")) } } @@ -421,7 +451,10 @@ where F: Fn(&C, &mut dyn Formatter, O) -> io::Result<()>, { fn format(&self, context: &C, formatter: &mut dyn Formatter) -> io::Result<()> { - let contents = self.property.extract(context); + let contents = match self.property.extract(context) { + Ok(contents) => contents, + Err(err) => return err.format(&(), formatter), + }; format_joined_with( context, formatter, @@ -489,7 +522,11 @@ where U: Template, { fn format(&self, context: &C, formatter: &mut dyn Formatter) -> io::Result<()> { - if self.condition.extract(context) { + let condition = match self.condition.extract(context) { + Ok(condition) => condition, + Err(err) => return err.format(&(), formatter), + }; + if condition { self.true_template.format(context, formatter)?; } else if let Some(false_template) = &self.false_template { false_template.format(context, formatter)?; @@ -522,8 +559,8 @@ where { type Output = O; - fn extract(&self, context: &C) -> Self::Output { - (self.function)(self.property.extract(context)) + fn extract(&self, context: &C) -> Result { + Ok((self.function)(self.property.extract(context)?)) } } @@ -565,12 +602,13 @@ impl Default for PropertyPlaceholder { impl TemplateProperty for PropertyPlaceholder { type Output = O; - fn extract(&self, _: &C) -> Self::Output { - self.value + fn extract(&self, _: &C) -> Result { + Ok(self + .value .borrow() .as_ref() .expect("placeholder value must be set before evaluating template") - .clone() + .clone()) } } @@ -616,3 +654,14 @@ where } Ok(()) } + +fn format_error_inline(formatter: &mut dyn Formatter, err: &dyn error::Error) -> io::Result<()> { + formatter.with_label("error", |formatter| { + write!(formatter, "")?; + Ok(()) + }) +}