Allow macro definition in renderable template

[sdedovic]

This change refactors the MacroDefinition rendering to no-op so that a template may contain a macro definition and still render. The previous behaviour was to error.

This means I may define a macro in a template and use it with the self namespace, as the test shows. It's useful for me to be able to extract some common behaviour in my templates out without having multiple files.

Related issues:

• Macro fails to be defined at top of template file #821
• Macro definition doesn't work in simple template #644

[sdedovic]

Just now seeing the discussion here: #370 (comment)

My opinion is that forcing separation of the macro can make things less ergonomic. Additionally, it was be trivial to enforce macros only at the top level.

In my case, I have a template for rending a recipe and a macro that renders the list of ingredients. I want to call the macro multiple times, because my recipe has a few different sections (e.g. dough, topping, filling). This template uses some shared CSS and only makes sense in my recipe template. I don't want to create a set of files, one for templates, and one for macros, just to achieve this.

[sdedovic]

I have updated the grammer, parser, and docs to force macros at the top-level only. This should remove any concern of a macro in a for, if, etc.

[sdedovic]

Result of running cargo test -- --format=terse

running 423 tests
........................................................................................ 88/423
........................................................................................ 176/423
........................................................................................ 264/423
........................................................................................ 352/423
.......................................................................
test result: ok. 423 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.07s

     Running tests/render_fails.rs (target/debug/deps/render_fails-1e0a12e06effa9e9)

running 11 tests
...........
test result: ok. 11 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.02s

   Doc-tests tera

running 4 tests
....
test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 1.06s```

[Keats]

That looks ok. I'm not sure what behaviour I want for Tera v2 concerning that.

[sdedovic]

That looks ok. I'm not sure what behaviour I want for Tera v2 concerning that.

I'll leave my reply here and also in the #637 thread:

I believe macro definition should only be done:

• the top-level of a file (not in an if/else, for, or block)
• in any file (self, as in this PR),
• in any location (after usage, i.e. forward reference).

See the next section to understand why.

---

With jinja2 templates you can do the following:

{% if some_val %}
  {% macro foo() %}42{% endmacro %}
{%  endif %}
{{ foo() }}

The macro may or may not be defined during rendering based on the truthiness of some_val. This is because Jinja takes the template AST and converts it into a Python module (using string concatenation) and the "rendering" is more or less evaling the Python at runtime.

This can't work easily with Rust, for obvious reasons. (Maybe if you turn the Tera template AST into rust code, compile it, and load it dynamically...) anyway,

This becomes a point of divergence for Tera macros. The current code will load all the macro definitions into one table as a first pass during rendering. Then they are looked up from this table when traversing the AST, and thus we cannot have dynamic, conditional macro definitions. This can be fixed by defining macros during AST traversal with Context in scope, or by extending the multi-pass approach to optimize the AST and generate macro definitions prior to rendering.

In any case, I personally believe this is likely an anti-pattern. Rarely is there a valid use case to define globals dynamically, and even less so when these act like pure functions, or "template snippets" as I see it.

[sdedovic]

Hey @Keats I think the CI is failing due to some kind of configuration issues. Haven't looked into it. Is there a way to re-trigger a build or should I look to fix the problem?

[Keats]

I think it was just a spurious issue, i've restarted the jobs

[sdedovic]

Rebased on top of #825 to fix failing builds