Django: Ticket #1400: [magic-removal] [patch] proposed update to template systemhttps://code.djangoproject.com/ticket/1400
<p>
Django's template tags do not always resolve arguments consistently
and as more people write custom template tags there is a risk that
there will be a proliferation of arbitrary argument syntaxes, each
with its own quirks.
</p>
<p>
Currently, arguments are sometimes resolved as context variables
('if' tag and 'ifequal' in a slightly different way), sometimes as
pure tag-specific syntax ('as', 'in'), sometimes as Python
primitives (strings and numbers in 'simple_tag' tags but not strings
containing whitespace), sometimes variables take filters, sometimes
not... For people writing custom template tags it would be nice if
there was one (simple) way of doing things.
</p>
<p>
Some related tickets: <a class="closed ticket" href="https://code.djangoproject.com/ticket/365" title="#365: defect: [patch] Change template.resolve_variable() to resolve hard-coded strings (closed: fixed)">#365</a>, <a class="closed ticket" href="https://code.djangoproject.com/ticket/566" title="#566: defect: No mention about using quotes to ignore variable resolution in templates. (closed: invalid)">#566</a>, <a class="closed ticket" href="https://code.djangoproject.com/ticket/959" title="#959: defect: [patch]Template improvements (closed: invalid)">#959</a>, <a class="closed ticket" href="https://code.djangoproject.com/ticket/1338" title="#1338: task: Undefined context variables should not resolve to the value of ... (closed: fixed)">#1338</a>
</p>
<p>
Robert's 'simple_tag' decorator introduced the nice idea of mapping
template tags directly to Python callables. The existing
implementation is a bit convoluted in order to allow the first
argument to be either an alternative name or the decorated function.
This seems a bit "magic":
</p>
<pre class="wiki">register.tag(tag_function)
register.tag('another_name', tag_function)
</pre><p>
How come the first argument changed its meaning?
</p>
<pre class="wiki">register.inclusion_tag('mytemplate')(tag_function)
</pre><p>
Why does inclusion_tag have a double-call syntax (if not used as a
decorator)?
</p>
<p>
Proposed solution
</p>
<p>
This patch to the magic-removal branch supersedes <a class="closed ticket" href="https://code.djangoproject.com/ticket/1105" title="#1105: defect: [patch] simple_tag decorator enhancement (closed: duplicate)">#1105</a> and provides a
more regular mapping from 'simple_tag' to python callables.
Argument resolution handles quoted strings with whitespace, boolean
literals, and optional named "keyword" arguments. Also fixes <a class="closed ticket" href="https://code.djangoproject.com/ticket/1338" title="#1338: task: Undefined context variables should not resolve to the value of ... (closed: fixed)">#1338</a>.
</p>
<p>
This works as expected:
</p>
<pre class="wiki">@register.simple_tag
def chant(message):
return '&lt;p&gt;%s&lt;/p&gt;' % message
</pre><pre class="wiki">{% chant "Django beats Chuck Norris." %}
</pre><p>
outputs:
</p>
<pre class="wiki">&lt;p&gt;Django beats Chuck Norris.&lt;/p&gt;
</pre><p>
Optional arguments are useful for letting template authors customise output:
</p>
<pre class="wiki">@register.simple_tag(takes_context=True, takes_block=True)
def chant(context, block, color='red', bold=False, class_='chant'):
attributes = {
'style':'color:%s; font-weight:%s;' % (color, bold and 'bold' or 'normal'),
'class':class_,
}
return '&lt;p%s&gt;%s&lt;/p&gt;' % (
''.join(' %s="%s"' % (name, value) for name, value in attributes.items()),
block.render(context),
)
</pre><pre class="wiki">{% chant bold=True class="chuck-hidden chant" %}Django beats Chuck Norris.{% endchant %}
</pre><p>
outputs:
</p>
<pre class="wiki">&lt;p style="color:red; font-weight:bold;" class="chuck-hidden chant"&gt;Django beats Chuck Norris.&lt;/p&gt;
</pre><p>
For a real-world use case, I've got a tag that
dynamically renders an arbitrary block using a truetype font with the
Python Imaging Library. Optional keyword arguments control color,
font, and size. See <a class="ext-link" href="http://www.kieranholland.com/code/django/typeset-tag/"><span class="icon">​</span>http://www.kieranholland.com/code/django/typeset-tag/</a>
</p>
<p>
Patch summary:
</p>
<ol><li>Adds regular expression parser for template tag arguments that permits single or double quoted strings with whitespace and named 'keyword' arguments. If a keyword argument name is a Python keyword an underscore is appended.
</li></ol><ol start="2"><li>Resolves literal True and False arguments to Python booleans.
</li></ol><ol start="3"><li>TEMPLATE_STRING_IF_INVALID is not returned from resolve_variable - only at render time.
</li></ol><ol start="4"><li>simple_tag decorator allows takes_context, and takes_block arguments (per <a class="closed ticket" href="https://code.djangoproject.com/ticket/1105" title="#1105: defect: [patch] simple_tag decorator enhancement (closed: duplicate)">#1105</a>) and can either return a string to be inserted in the template or None.
</li></ol><ol start="5"><li>The tag registration class has been simplified with the benefit that the inclusion_tag and simple_tag syntax is more consistent: the tag function always comes first and optional alternate names - the uncommon case - are provided by the 'name' keyword argument.
</li></ol><p>
Summary of backwards-incompatible changes:
</p>
<ol><li>Alternative names for tag registration come after the tag function.
</li></ol><ol start="2"><li>resolve_variable raises VariableDoesNotExist exception for any unresolved variable so the caller is responsible for handling this.
</li></ol><p>
I have fixed all backwards-incompatible code that I could find in the
magic-removal branch, but there may be a few bits and pieces
remaining. All existing unit tests pass(*) and I've added a few for
the tag argument tokenizer and made some minor doc fixes.
</p>
<ul><li>Except one I think should fail: In 'resolve_variable' if a variable is resolved to a callable and a TypeError is thrown because the callable expected arguments why should the exception be masked? Doesn't it indicate a templating error or am I missing something?
</li></ul><p>
Thoughts?
</p>
<p>
Kieran
</p>
en-usDjangohttps://www.djangoproject.com/s/img/site/hdr_logo.gifhttps://code.djangoproject.com/ticket/1400
Trac 1.2django@…Sun, 26 Feb 2006 00:30:25 GMTattachment sethttps://code.djangoproject.com/ticket/1400
https://code.djangoproject.com/ticket/1400
<ul>
<li><strong>attachment</strong>
set to <em>templates_r2388.diff</em>
</li>
</ul>
<p>
template patch
</p>
TicketJacobMon, 27 Feb 2006 18:46:02 GMThttps://code.djangoproject.com/ticket/1400#comment:1
https://code.djangoproject.com/ticket/1400#comment:1
<p>
I very much don't like the keyword-arguments part of this; template tags should be as simple as possible and accessible to non-programmers. I'm leaving this open since the rest of the patch seems like a pretty good idea, but this needs more thought.
</p>
Ticketdjango@…Mon, 27 Feb 2006 19:58:31 GMThttps://code.djangoproject.com/ticket/1400#comment:2
https://code.djangoproject.com/ticket/1400#comment:2
<p>
Keyword arguments are optional and make certain things simpler for template authors. For example, in the case of my 'typeset' tag, linked above, it is way easier to give just the named arguments that you want to alter rather than to have to give the whole set of positional arguments to change just one default (how to remember the order of arguments?). I am all for keeping things simple and I do not want to see complex logic embedded in templates either, but I do not think that keyword arguments have this effect. There is a conceptual parallel for template authors with giving named attributes to HTML tags ( imagine if they were positional ;). The intention is to simplify customisation by template authors of features well-encapsulated in Python and add a degree of self-documentation to tag calls (which is one of the really nice things about keyword arguments in Python) rather than have every custom tag do its own arbitrary syntax.
</p>
Ticketdjango@…Mon, 27 Feb 2006 21:27:18 GMThttps://code.djangoproject.com/ticket/1400#comment:3
https://code.djangoproject.com/ticket/1400#comment:3
<p>
@jacob if a html programmer can't understand keywords, he isn't a html programmer at all :)
keywords make certains things much easier then lists, where the template author has to remember the order instead of just using names which he most like can remember better, especially on complex tags
i'm +1 on this
</p>
TicketAntti KaiholaWed, 01 Mar 2006 21:22:56 GMThttps://code.djangoproject.com/ticket/1400#comment:4
https://code.djangoproject.com/ticket/1400#comment:4
<p>
What about performance? I remember reading that the superior speed of "manual" tag argument parsing as it's done now was the main reason for not developing a more advanced parser.
</p>
<p>
Anyway, this looks very good to me, and since template tags are good for so many situations, it's definitely a plus if they're simple to create.
</p>
Ticketdjango@…Thu, 02 Mar 2006 02:03:11 GMThttps://code.djangoproject.com/ticket/1400#comment:5
https://code.djangoproject.com/ticket/1400#comment:5
<p>
Simplistic testing suggests that rendering a template consisting of an 'upper' block tag (converts content to upper case) containing some plain text is about 20% slower with the simple_tag version. Given that custom tags are usually doing only a small part of the work in rendering a template this is unlikely to be significant. In the context of handling an entire request it is even less likely to be significant. The time saved in development _is_ significant in my experience. There is also no compulsion to use the feature. The built-in tags are written the standard way and that would remain an option for those who think it will improve performance.
</p>
TicketAntti KaiholaTue, 07 Mar 2006 10:06:40 GMThttps://code.djangoproject.com/ticket/1400#comment:6
https://code.djangoproject.com/ticket/1400#comment:6
<p>
While waiting for this patch to be applied (or waiting in vain if it won't), I'd love to try it as a separate module instead of patching Django. Do you happen to already have made a cleanly separated library?
</p>
Ticketdjango@…Tue, 07 Mar 2006 11:18:15 GMThttps://code.djangoproject.com/ticket/1400#comment:7
https://code.djangoproject.com/ticket/1400#comment:7
<p>
The easiest way of testing it (for you and me:) is to apply the patch to a spare copy of Django magic-removal branch. The way I do it is to keep a local branch with svk, but just a second checkout would do. I use a shell script to toggle PYTHONPATH between branches. Let me know if you have any problems - it'd be good to hear how you go.
</p>
Ticketdjango@…Sun, 23 Apr 2006 22:17:20 GMTcc sethttps://code.djangoproject.com/ticket/1400#comment:8
https://code.djangoproject.com/ticket/1400#comment:8
<ul>
<li><strong>cc</strong>
<em>django@…</em> added
</li>
</ul>
<p>
django@…, do you have an uptodate patch. It doesn't apply to MR anymore.
</p>
Ticketdjango@…Sun, 23 Apr 2006 22:44:24 GMTattachment sethttps://code.djangoproject.com/ticket/1400
https://code.djangoproject.com/ticket/1400
<ul>
<li><strong>attachment</strong>
set to <em>templates_r2738.diff</em>
</li>
</ul>
<p>
Updated patch for template extension requirement
</p>
Ticketdjango@…Sun, 23 Apr 2006 22:45:36 GMThttps://code.djangoproject.com/ticket/1400#comment:9
https://code.djangoproject.com/ticket/1400#comment:9
<p>
Patch updated for new template extension requirement.
</p>
TicketRussell Keith-MageeTue, 04 Jul 2006 01:34:37 GMTowner changedhttps://code.djangoproject.com/ticket/1400#comment:10
https://code.djangoproject.com/ticket/1400#comment:10
<ul>
<li><strong>owner</strong>
changed from <em>Adrian Holovaty</em> to <em>Russell Keith-Magee</em>
</li>
</ul>
TicketRussell Keith-MageeTue, 04 Jul 2006 03:21:44 GMThttps://code.djangoproject.com/ticket/1400#comment:11
https://code.djangoproject.com/ticket/1400#comment:11
<p>
(In <a class="changeset" href="https://code.djangoproject.com/changeset/3268/">[3268]</a>) Fixes <a class="closed ticket" href="https://code.djangoproject.com/ticket/1338" title="#1338: task: Undefined context variables should not resolve to the value of ... (closed: fixed)">#1338</a>, Refs <a class="closed ticket" href="https://code.djangoproject.com/ticket/1400" title="#1400: enhancement: [magic-removal] [patch] proposed update to template system (closed: wontfix)">#1400</a>, <a class="closed ticket" href="https://code.djangoproject.com/ticket/2237" title="#2237: defect: Setting TEMPLATE_STRING_IF_INVALID causes admin page headline to vanish (closed: duplicate)">#2237</a> -- Modified variable resolution to allow template 'if' statements to work if TEMPLATE_STRING_IF_INVALID is set. Modified unit tests to force the use of this variable, so that returning <em> isn't confused with an actual failure.
</em></p>
TicketRussell Keith-MageeTue, 04 Jul 2006 03:44:56 GMThttps://code.djangoproject.com/ticket/1400#comment:12
https://code.djangoproject.com/ticket/1400#comment:12
<p>
(In <a class="changeset" href="https://code.djangoproject.com/changeset/3269/">[3269]</a>) Refs <a class="closed ticket" href="https://code.djangoproject.com/ticket/1400" title="#1400: enhancement: [magic-removal] [patch] proposed update to template system (closed: wontfix)">#1400</a> -- Variable resolver now converts literal strings 'False' and 'True' into booleans when used as template arguments. This is point 2 from ticket <a class="closed ticket" href="https://code.djangoproject.com/ticket/1400" title="#1400: enhancement: [magic-removal] [patch] proposed update to template system (closed: wontfix)">#1400</a>. Thanks Kieren Holland.
</p>
TicketRussell Keith-MageeTue, 04 Jul 2006 03:58:35 GMTstatus changed; resolution sethttps://code.djangoproject.com/ticket/1400#comment:13
https://code.djangoproject.com/ticket/1400#comment:13
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>wontfix</em>
</li>
</ul>
<p>
Firstly, Kieran - apologies for the misspelled name on the last checkin.
</p>
<p>
Second - I'm going to mark this ticket wontfix - but that's just because it's a bit of a monster. I've cherry picked the easy and obvious bits (2 and 3), but as for the others:
</p>
<ol><li>I'm with Jacob, and not convinced of the efficacy of adding keyword arguments to the template language.
</li></ol><ol start="4"><li>This is a duplicate of <a class="closed ticket" href="https://code.djangoproject.com/ticket/1105" title="#1105: defect: [patch] simple_tag decorator enhancement (closed: duplicate)">#1105</a> - discussion should continue there (where Robert made some points I am inclined to agree with)
</li></ol><ol start="5"><li>Seems like a good idea, but the subset of the patch that relates to this change is no longer obvious.
</li></ol><p>
If you want to pursue 1 and 5, open a new ticket that covers just that feature; pursue 4 in ticket <a class="closed ticket" href="https://code.djangoproject.com/ticket/1105" title="#1105: defect: [patch] simple_tag decorator enhancement (closed: duplicate)">#1105</a>. Tickets should be atomic whenever possible. This makes tickets easier to understand, and more likely that they will be dealt with in a timely fashion.
</p>
TicketLuke PlantMon, 10 Jul 2006 09:09:56 GMThttps://code.djangoproject.com/ticket/1400#comment:14
https://code.djangoproject.com/ticket/1400#comment:14
<p>
Kieran, Russell,
</p>
<p>
I only just noticed this (because of the django-updates not being sent out).
</p>
<p>
I'm -1 on the <a class="changeset" href="https://code.djangoproject.com/changeset/3269/">[3269]</a> changeset. This is a backwards incompatible change (though not noted in the list of backwards incompatible changes), and it could introduce bugs into some of my existing code (and other people's I'm sure). Every time I have something like:
</p>
<pre class="wiki"> {% if foo %}
&lt;p&gt;{{ foo }}&lt;/p&gt;
{% endif %}
</pre><p>
where foo is a string, I now have a bug in the case where foo happened to contain the string 'False'. I know it is unlikely, but now the code I have to write for correct behaviour is massively complicated.
</p>
<p>
When would you actually have a variable that contains the string 'False' instead of boolean False? The new behaviour is very unpythonic, and while I know that this is a template language, all the values being fed into the template system will be Python values, so I can't see any need for this.
</p>
TicketSimon WillisonMon, 10 Jul 2006 13:29:08 GMThttps://code.djangoproject.com/ticket/1400#comment:15
https://code.djangoproject.com/ticket/1400#comment:15
<p>
I agree with lukeplant. The 'True' =&gt; True thing is just asking for trouble.
</p>
TicketRussell Keith-MageeWed, 30 Aug 2006 00:54:08 GMThttps://code.djangoproject.com/ticket/1400#comment:16
https://code.djangoproject.com/ticket/1400#comment:16
<p>
(In <a class="changeset" href="https://code.djangoproject.com/changeset/3680/">[3680]</a>) Refs <a class="closed ticket" href="https://code.djangoproject.com/ticket/1400" title="#1400: enhancement: [magic-removal] [patch] proposed update to template system (closed: wontfix)">#1400</a> - Reverted <a class="changeset" href="https://code.djangoproject.com/changeset/3269/">r3269</a>. Template variable evalution should follow Python norms.
</p>
Ticket