Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Support included blocks override #84

Merged
merged 1 commit into from

6 participants

@paradoxxxzero

This commit fix inheritance with included blocks.
Consider these templates:

A.jinja2

I am A
{% include 'I.jinja2' %}

I.jinja2

Who are you ?
{% block whoami %}
    I am I
{% endblock %}

B.jinja2

{% extends 'A.jinja2' %}
{% block whoami %}
    I am B
{% endblock %}

As of now, rendering B.jinja2 would output:

I am A
Who are you ?
I am I

This patch allow B to override I block, the output will be instead:

I am A
Who are you ?
I am B
@mitsuhiko
Owner

This changes behavior and was never intended to be supported. However since the whole scoping is something I want to overhaul I will decide in neither way right now and try to find a solution that is compatible with the current behavior and would support blocks be defined in included templates.

@SimonSapin

How is the proposed patch not compatible with the current behavior?

@equake

I would love to be able to override blocks like this.
In my case, i have a base template that is extended by a lot of templates.
Sometimes those templates have to include "components" (for example: an slideshow).
This include need to add some <script> tag in the page header to work properly. It would be nice if it could override/extends the "scripts" block defined in the base template.
Is there any alternative way to accomplish this with jinja?

@mitsuhiko mitsuhiko merged commit 33aee12 into mitsuhiko:master
@mitsuhiko
Owner

I merged this now. I don't see how this breaks anything currently.

@lakshmivyas lakshmivyas referenced this pull request from a commit in hyde/j2includebug
@lakshmivyas lakshmivyas Jinja2 2.7 breaks include statements by allowing block overrides.
This provides a basic jinja2 static template that demonstrates the
breaking behavior introduced by mitsuhiko/jinja2#84.
877976b
@lakshmivyas

@mitsuhiko - This change breaks a few websites that use hyde. Here is a simple demo:
https://github.com/hyde/j2includebug.


Pushing parent blocks along with other context variables is surprising IMO. I'd like to propose:

{% include "xxx" with context and blocks %}

or something similar instead if its not too late.

Edit:

Here is (hopefully) a better statement of the bug:

Given a container C and an include I, this patch really includes container C with the include I injected at the top of the extension hierarchy of C. As long as I and C have completely different set of blocks this is okay. However, when there is an overlap between the blocks of I and C, the results will likely be surprising(bad).

@lakshmivyas

I think its safer to completely pull this change out and reintroduce it only if a block resolution order can be provided alongside.

@ripperdoc

To add to this, this update seem to break my site badly (unless I'm doing something else wrong of course...)

I have three template files, A, B and Base.

Base
{% block content %}{% endblock}

A
{% extends Base %}
{% block content %}
lorem ipsum {% include B %}
{% endblock}

B
{% block content %}
dolor sit
{% enblock %}

This throws me in maximum recursion, as (if I understand it correctly), in A.content we include B.content which was defined in A.content and so on? I had the same structure which was not an issue before, I presume before this update the block content in the included template was simply ignored?

I can include without context but that will break other things, e.g. not being able to pass my model objects down to the included templates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 5, 2012
  1. @paradoxxxzero
This page is out of date. Refresh to see the latest.
Showing with 25 additions and 2 deletions.
  1. +9 −2 jinja2/compiler.py
  2. +16 −0 jinja2/testsuite/imports.py
View
11 jinja2/compiler.py
@@ -950,9 +950,16 @@ def visit_Include(self, node, frame):
self.indent()
if node.with_context:
+ self.writeline('include_context = template.new_context('
+ 'context.parent, True, locals())')
+ self.writeline('for name, context_blocks in context.'
+ 'blocks.%s():' % dict_item_iter)
+ self.indent()
+ self.writeline('include_context.blocks.setdefault('
+ 'name, [])[0:0] = context_blocks')
+ self.outdent()
self.writeline('for event in template.root_render_func('
- 'template.new_context(context.parent, True, '
- 'locals())):')
+ 'include_context):')
else:
self.writeline('for event in template.module._body_stream:')
View
16 jinja2/testsuite/imports.py
@@ -121,6 +121,22 @@ def test_context_include_with_overrides(self):
)))
assert env.get_template("main").render() == "123"
+ def test_included_block_override(self):
+ env = Environment(loader=DictLoader(dict(
+ main="{% extends 'base' %}{% block b %}1337{% endblock %}",
+ base="{% include 'inc' %}",
+ inc="{% block b %}42{% endblock %}"
+ )))
+ assert env.get_template("main").render() == "1337"
+
+ def test_included_block_override_with_super(self):
+ env = Environment(loader=DictLoader(dict(
+ main="{% extends 'base' %}{% block b %}1337|{{ super() }}{% endblock %}",
+ base="{% include 'inc' %}",
+ inc="{% block b %}42{% endblock %}"
+ )))
+ assert env.get_template("main").render() == "1337|42"
+
def test_unoptimized_scopes(self):
t = test_env.from_string("""
{% macro outer(o) %}
Something went wrong with that request. Please try again.