Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fixed #3544, #12064, #16147 -- {% include %} behavior #1528

Closed
wants to merge 2 commits into from

4 participants

@funkybob

Merged BaseIncludeNode, ConstantIncludeNode and Include node.

This avoids raising TemplateDoesNotExist at parsing time, allows recursion
when passing a literal template name, and should make TEMPLATE_DEBUG behavior
consistant.

@timgraham
Owner

Are you planning to add some tests?

@funkybob

I'm investigating the existing tests to see which of these cases they don't already cover.

@prestontimmons

Good job simplifying my pull request. I had changed more stuff than was necessary.

I forked your branch and added the appropriate tests:

prestontimmons@e804ef0

I'm not sure if it's a problem, but I wrote the test using a standard test case, rather than the inline style you see with other tags. I just find those unwieldy to work with.

I tested your branch out on my side and things look like their working.

@funkybob

Hey, thanks for that... I was struggling to get my own tests to fail on master. And it looks like yours don't fail on master, either...?

@funkybob funkybob Fixed #3544, #12064, #16147 -- {% include %} behavior
Merged BaseIncludeNode, ConstantIncludeNode and Include node.

This avoids raising TemplateDoesNotExist at parsing time, allows recursion
when passing a literal template name, and should make TEMPLATE_DEBUG behavior
consistant.

Thanks loic84 for help with the tests.
3d41484
django/template/loader_tags.py
((47 lines not shown))
# Does this quack like a Template?
if not callable(getattr(template, 'render', None)):
# If not, we'll try get_template
template = get_template(template)
- return self.render_template(template, context)
+ values = dict([
+ (name, var.resolve(context))
+ for name, var in six.iteritems(self.extra_context)
+ ])
@charettes Collaborator

Unneeded list comprehension.

Was copied from the original, but sure... dict comprehension coming up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham
Owner

merged in e2f0622

@timgraham timgraham closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Aug 30, 2013
  1. @funkybob

    Fixed #3544, #12064, #16147 -- {% include %} behavior

    funkybob authored
    Merged BaseIncludeNode, ConstantIncludeNode and Include node.
    
    This avoids raising TemplateDoesNotExist at parsing time, allows recursion
    when passing a literal template name, and should make TEMPLATE_DEBUG behavior
    consistant.
    
    Thanks loic84 for help with the tests.
  2. @funkybob
This page is out of date. Refresh to see the latest.
View
51 django/template/loader_tags.py
@@ -121,55 +121,34 @@ def render(self, context):
# the same.
return compiled_parent._render(context)
-class BaseIncludeNode(Node):
- def __init__(self, *args, **kwargs):
+class IncludeNode(Node):
+ def __init__(self, template, *args, **kwargs):
+ self.template = template
self.extra_context = kwargs.pop('extra_context', {})
self.isolated_context = kwargs.pop('isolated_context', False)
- super(BaseIncludeNode, self).__init__(*args, **kwargs)
-
- def render_template(self, template, context):
- values = dict([(name, var.resolve(context)) for name, var
- in six.iteritems(self.extra_context)])
- if self.isolated_context:
- return template.render(context.new(values))
- with context.push(**values):
- return template.render(context)
-
-
-class ConstantIncludeNode(BaseIncludeNode):
- def __init__(self, template_path, *args, **kwargs):
- super(ConstantIncludeNode, self).__init__(*args, **kwargs)
- try:
- t = get_template(template_path)
- self.template = t
- except:
- if settings.TEMPLATE_DEBUG:
- raise
- self.template = None
-
- def render(self, context):
- if not self.template:
- return ''
- return self.render_template(self.template, context)
-
-class IncludeNode(BaseIncludeNode):
- def __init__(self, template_name, *args, **kwargs):
super(IncludeNode, self).__init__(*args, **kwargs)
- self.template_name = template_name
def render(self, context):
try:
- template = self.template_name.resolve(context)
+ template = self.template.resolve(context)
# Does this quack like a Template?
if not callable(getattr(template, 'render', None)):
# If not, we'll try get_template
template = get_template(template)
- return self.render_template(template, context)
+ values = {
+ name: var.resolve(context)
+ for name, var in six.iteritems(self.extra_context)
+ }
+ if self.isolated_context:
+ return template.render(context.new(values))
+ with context.push(**values):
+ return template.render(context)
except:
if settings.TEMPLATE_DEBUG:
raise
return ''
+
@register.tag('block')
def do_block(parser, token):
"""
@@ -258,9 +237,5 @@ def do_include(parser, token):
options[option] = value
isolated_context = options.get('only', False)
namemap = options.get('with', {})
- path = bits[1]
- if path[0] in ('"', "'") and path[-1] == path[0]:
- return ConstantIncludeNode(path[1:-1], extra_context=namemap,
- isolated_context=isolated_context)
return IncludeNode(parser.compile_filter(bits[1]), extra_context=namemap,
isolated_context=isolated_context)
View
7 tests/template_tests/templates/recursive_include.html
@@ -0,0 +1,7 @@
+Recursion!
+{% for comment in comments %}
+ {{ comment.comment }}
+ {% if comment.children %}
+ {% include "recursive_include.html" with comments=comment.children %}
+ {% endif %}
+{% endfor %}
View
34 tests/template_tests/tests.py
@@ -349,6 +349,40 @@ def test_include_template_argument(self):
output = outer_tmpl.render(ctx)
self.assertEqual(output, 'This worked!')
+ @override_settings(TEMPLATE_DEBUG=True)
+ def test_include_immediate_missing(self):
+ """
+ Regression test for #16417 -- {% include %} tag raises TemplateDoesNotExist at compile time if TEMPLATE_DEBUG is True
+
+ Test that an {% include %} tag with a literal string referencing a
+ template that does not exist does not raise an exception at parse
+ time.
+ """
+ ctx = Context()
+ tmpl = Template('{% include "this_does_not_exist.html" %}')
+ self.assertIsInstance(tmpl, Template)
+
+ @override_settings(TEMPLATE_DEBUG=True)
+ def test_include_recursive(self):
+ comments = [
+ {
+ 'comment': 'A1',
+ 'children': [
+ {'comment': 'B1', 'children': []},
+ {'comment': 'B2', 'children': []},
+ {'comment': 'B3', 'children': [
+ {'comment': 'C1', 'children': []}
+ ]},
+ ]
+ }
+ ]
+
+ t = loader.get_template('recursive_include.html')
+ self.assertEqual(
+ "Recursion! A1 Recursion! B1 B2 B3 Recursion! C1",
+ t.render(Context({'comments': comments})).replace(' ', '').replace('\n', ' ').strip(),
+ )
+
class TemplateRegressionTests(TestCase):
Something went wrong with that request. Please try again.