Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change extends tag to accept expressions #41

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

williamthome
Copy link
Contributor

@williamthome williamthome commented Mar 27, 2023

This PR adds a missing ability to resolve expressions in extends tag, e.g.

{% extends m.foo.template %}

Motivation

The motivation is that today only strings are accepted by the extends tag.

Implementation notes

I tested this code in Zotonic, and it works.
I could not add tests because the template_compiler_runtime:modal_call/4 does not resolve model calls.

@williamthome williamthome force-pushed the feat/improve-extends branch from 3af14f1 to d1aeff1 Compare March 27, 2023 18:30
@mworrell mworrell self-assigned this Mar 28, 2023
@mworrell
Copy link
Member

Nice to make it more dynamic.

I was also thinking of a "with" in the extends (or override) so that we can assign extra variables.

Why did you only add a model call and not an expression?

@williamthome
Copy link
Contributor Author

Agree, it could use an expression, but I could not find a way to do so in the compile_tokens time.
The compile_tokens function calls compile_blocks, as you can see in my approach here.
What I see that can solve this is to use template_compiler_expr:compile, e.g.:

compile_tokens({ok, {extends, ExtendsBlock, Elements}}, CState, _Options) ->
    Blocks = find_blocks(Elements),
    {Ws, BlockAsts} = compile_blocks(Blocks, CState),
    {Ws1, ExtendsBlockAsts} = template_compiler_expr:compile(ExtendsBlock, CState, Ws), % <- here
    % TODO: eval ExtendsBlockAsts and expects it to be a binary

How about?
I will take a look and will try to do some implementation.

@mworrell
Copy link
Member

Yes, and then evaluate the expression AST runtime, expecting a (binary) string.
Which is no overhead for the case where the AST is just a string anyway.

@williamthome williamthome changed the title Add model call to extends tag Change extends tag to accept expressions Mar 28, 2023
@williamthome
Copy link
Contributor Author

@mworrell, can you take a look at my changes?
It works, but not how I expected, maybe you have a better idea of how to solve this.
What is not working as expected is passing variables to the extended template.
Maybe this makes no sense, but what I tried is to call a model that resolves the template name and pass some variables too, for example:

{% extends m.foo.template["base.tpl"]
	title="Zotonic"
	foo=`foo`
%}

{% block title %}{{ title }}{% endblock %}

{% block content %}
	{% print foo %}
	{% debug %}
{% endblock %}

The template is rendered, but this is what is rendered:
image
The title and foo are undefined, and the debug shows that the variables are not injected in the Context.
However, the first time the variables are in the Context but do not persist. Below is a ?DEBUG in z_template_compiler_runtime:set_context_vars:

(zotonic@orp)1> =NOTICE REPORT==== 28-Mar-2023::13:52:59.103888 ===
DEBUG: {vars,#{auth_expires => 12460,auth_options => #{},
               csp_nonce => <<"uakEb9kC2UxBjZupmdHG">>,foo => foo,id => 326,
               q =>
                   [{<<"zotonic_dispatch">>,home},
                    {<<"zotonic_dispatch_path">>,[]},
                    {<<"zotonic_site">>,foo}],
               session_id => <<"084e7de9-464d-6e07-c914-8acc874756df">>,
               template => "home.tpl",title => <<"Zotonic">>,
               zotonic_dispatch_file => <<"dispatch">>,
               zotonic_dispatch_module => foo}}
=NOTICE REPORT==== 28-Mar-2023::13:52:59.104653 ===
DEBUG: {vars,#{auth_expires => 12460,auth_options => #{},code => en,
               csp_nonce => <<"uakEb9kC2UxBjZupmdHG">>,id => 326,
               q =>
                   [{<<"zotonic_dispatch">>,home},
                    {<<"zotonic_dispatch_path">>,[]},
                    {<<"zotonic_site">>,foo}],
               session_id => <<"084e7de9-464d-6e07-c914-8acc874756df">>,
               template => "home.tpl",z_language => en,
               zotonic_dispatch_file => <<"dispatch">>,
               zotonic_dispatch_module => foo}}
...

The first log shows title and foo in the Context variables, but the next 5x logs do not.
Suggestions welcome.

@mworrell
Copy link
Member

@williamthome I will have a look 👍 (Am on a couple of deadlines right now, so might take a couple of days...)

@williamthome
Copy link
Contributor Author

Sure, no problem. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants