Skip to content

Commit

Permalink
templater: update comment why Template isn't TemplateProperty<Output …
Browse files Browse the repository at this point in the history
…= ..>

They are similar in a way that both of them can represent dynamic/static
evaluation, but their behaviors are different in error handling.
  • Loading branch information
yuja committed Mar 25, 2024
1 parent 577e030 commit df7be43
Showing 1 changed file with 10 additions and 5 deletions.
15 changes: 10 additions & 5 deletions cli/src/template_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,16 @@ pub enum CoreTemplatePropertyKind<'a> {
Timestamp(Box<dyn TemplateProperty<Output = Timestamp> + 'a>),
TimestampRange(Box<dyn TemplateProperty<Output = TimestampRange> + 'a>),

// TODO: This argument no longer makes sense. Maybe we can migrate these to
// TemplateProperty<..>:
// Similar to `TemplateProperty<I, Output = Box<dyn Template<()> + 'a>`, but doesn't
// capture `I` to produce `Template<()>`. The context `I` would have to be cloned
// to convert `Template<I>` to `Template<()>`.
// Both TemplateProperty and Template can represent a value to be evaluated
// dynamically, which suggests that `Box<dyn Template + 'a>` could be
// composed as `Box<dyn TemplateProperty<Output = Box<dyn Template ..`.
// However, there's a subtle difference: TemplateProperty is strict on
// error, whereas Template is usually lax and prints an error inline. If
// `concat(x, y)` were a property returning Template, and if `y` failed to
// evaluate, the whole expression would fail. In this example, a partial
// evaluation output is more useful. That's one reason why Template isn't
// wrapped in a TemplateProperty. Another reason is that the outermost
// caller expects a Template, not a TemplateProperty of Template output.
Template(Box<dyn Template + 'a>),
ListTemplate(Box<dyn ListTemplate + 'a>),
}
Expand Down

1 comment on commit df7be43

@ilyagr
Copy link
Contributor

@ilyagr ilyagr commented on df7be43 Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks!

Please sign in to comment.