rust-sailfish / sailfish

Simple, small, and extremely fast template engine for Rust
https://rust-sailfish.github.io/sailfish/
MIT License
829 stars 56 forks source link

Feature: Implementing `Render` for `TemplateOnce` #51

Open d4h0 opened 3 years ago

d4h0 commented 3 years ago

Hi,

Would it be possible to automatically implement Render for everything that implements TemplateOnce?

So this:

<%- Footer {}.render_once().unwrap() %>

...would become this:

<%- Footer {} %>

...which would improve ergonomics a bit, which would be nice.

Basically, if .render_once() returns an error, the template would return that error.

Is my assumption correct, that this wouldn't be difficult to implement?

Kogia-sima commented 3 years ago

@d4h0 Thank you for your suggestion.

At first, I'd like to recommend using include! macro, which allows embedding another template. Also it prevents users from escaping rendering results of child templates unexpectedly.

<% include!("footer.stpl"); %>

With similar reason, I prefer another solution based on template inheritance feature, which is not supported yet.

If you still want to return error instead of panics, simply use ? operator:

<%- Footer {}.render_once()? %>
d4h0 commented 3 years ago

@Kogia-sima: Thanks for your feedback.

The problem with include! is, that no arguments can be passed in. I also would much more prefer pure Rust as syntax.

Ideally, the following would be possible:

<%- Footer {
    headline: "the headline",
    links: some_links, // Vec<&str>,
    some_custom_code: <% Copyright &copy; <%= copyrights %> %>
} %>

This would introduce nested template blocks, which probably wouldn't be hard to parse. However, I don't know, how hard it would be to implement proper syntax highlighting. If it turned out to be a problem, a second template tag could be introduced, to include template content into Rust code (but I would much more prefer to use the regular template tag).

I think, this would be a pretty nice syntax for including templates, and would make include! obsolete (reducing unnecessary syntax is an advantage, in my opinion).

Also it prevents users from escaping rendering results of child templates unexpectedly.

That's another disadvantage of include!. This eliminates the option to control if a template should be HTML-escaped, or not (or at least users would have to hard-code that at the level of the template attribute macro). There are most likely many use-cases where escaping is desired. And if a user inadvertently HTML-escapes a template, that would visible immediately. So that wouldn't be a big disadvantage.

This leads me to another advantage of my proposed approach for including templates.

With my approach, it would be easy to apply functions and filters to the output of a template.

For example:

<%- MyMarkdownTemplate { arg1: "foo", arg2: "bar" } | markdown %>

I'm not 100% sure, if it's possible to define custom filters (that information is missing from the docs). If it's not possible, the following could be used:

<%- markdown(MyMarkdownTemplate {}) %>

And from the docs:

The path format is platform-specific. You must use \ character as a separator on Windows.

With the above, this wouldn't be a problem anymore (however, you probably still have to use the proper character at the template attribute macro level).

Also, if someone moves the template file somewhere else, or renames it, only one location would need to change (if the name of the template struct changes, Rust Analyzer has support for refactoring that, as far as I know). With include!, every location where include! is used, needs to change.

I think, my approach would be much more powerful, than the current include! macro.

Is there a reason, why Render can't be implemented for TemplateOnce easily?

Regarding, template inheritance: This has many of the same limitations as include!. However, inheritance would still be needed with the above (to overwrite parent blocks from within children).

<%- Footer {}.render_once()? %>

Thanks, for the hint. That's already a small improvement.