Opened 11 years ago

Closed 5 years ago

#20995 closed New feature (fixed)

{% include %} uses get_template where it could select_template

Reported by: Keryn Knight <django@…> Owned by: KESHAV KUMAR
Component: Template system Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

It'd be nice if the Include template tag was sensible enough to allow fallbacks by selecting the most appropriate template, as things like render/render_to_response/render_to_string do. It's tripped me up on more than one occasion, and it seems a trivial feature to support, from my limited testing.

>>> from django.template import Template, Context
>>> tmpl = Template('{% include var %}')
>>> ctx = Context({'var':'admin/base.html'})
>>> ctx
[{'var': 'admin/base.html'}]
>>> tmpl.render(ctx)
... some HTML output ...
>>> ctx.update({'var':['admin/base.html', 'admin/fail.html']})
{'var': ['admin/base.html', 'admin/fail.html']}
>>> tmpl.render(ctx)
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/path/django/template/base.py", line 140, in render
    return self._render(context)
  File "/path/django/template/base.py", line 134, in _render
    return self.nodelist.render(context)
  File "/path/django/template/base.py", line 823, in render
    bit = self.render_node(node, context)
  File "/path/django/template/debug.py", line 74, in render_node
    return node.render(context)
  File "/path/django/template/loader_tags.py", line 165, in render
    template = get_template(template_name)
  File "/path/django/template/loader.py", line 145, in get_template
    template, origin = find_template(template_name)
  File "/path/django/template/loader.py", line 138, in find_template
    raise TemplateDoesNotExist(name)
TemplateDoesNotExist: ['admin/base.html', 'admin/fail.html']

The 'fix' is to change this line from get_template to select_template, though this might now be slightly complicated by the recent changes in 5cdacbda034af928f5033c9afc7b50ee0b13f75c to allow for rendering of Template instances.

Changing to select_template on 1.4 yields the results I'd expect:

>>> from django.template import Template, Context 
>>> tmpl = Template('{% include var %}')
>>> ctx = Context({'var':['admin/base.html', 'admin/fail.html']})
>>> tmpl.render(ctx)
... some HTML output ...

Both the above shell sessions assume django.contrib.admin is in INSTALLED_APPS.

Change History (10)

comment:1 by amberdoctor, 11 years ago

Owner: changed from nobody to amberdoctor
Status: newassigned

comment:2 by Keryn Knight <django@…>, 11 years ago

I now have a working patch + test for this, but because it introduces another slightly different way of selecting a template based on the given arguments, I'm holding off on opening a PR, to see if there's merit to refactoring template selection into one place; see #21065 for the details.

comment:3 by Daniele Procida, 11 years ago

Triage Stage: UnreviewedAccepted

comment:4 by Daniele Procida, 11 years ago

I agree that this should follow upon a resolution of #21065. Three different ways of selecting templates seems like two too many already.

comment:5 by Tim Graham, 11 years ago

Easy pickings: unset

comment:6 by KESHAV KUMAR, 5 years ago

Has patch: set
Needs documentation: set

Tried to solve this. PR

comment:7 by Adam Johnson, 5 years ago

Needs documentation: unset
Patch needs improvement: set

comment:8 by Adam Johnson, 5 years ago

Owner: changed from amberdoctor to KESHAV KUMAR

comment:9 by KESHAV KUMAR, 5 years ago

Patch needs improvement: unset

comment:10 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: fixed
Status: assignedclosed

In f37d548:

Fixed #20995 -- Added support for iterables of template names to {% include %} template tag.

Thanks Adam Johnson for the review.

Note: See TracTickets for help on using tickets.
Back to Top