[patch] simple_tag decorator enhancement

Patch adds simple_tag_with_context tag function decorator

I would prefer this to be similar to the inclusion_tag , ie optionally take an argument of takes_context=1. This is slightly tricky due to how decorators work ( making the no args and args versions work the same). register_tag does this correctly.

Going further, I'd like to split this stuff out a bit more. gen_compile_func should make use of a function attribute, eg arg_info. This would be a list of stuff that could be taken into account when creating the compile function. Decorators should then be provided, eg

@register.simple_tag @takes_context @takes_block def my_tag(context, block, arg1, arg2):

pass

This way the concerns are separated a bit.

I've reimplemented simple_tag in line with Robert's first suggestion.

Now simple_tag passes in context and/or block arguments (always in that order) if requested. If a block is requested then the tag will automatically consume the template from {% tagname %} until {% endtagname %} passing the resulting nodelist in as the block argument.

The comment tag can now be implemented:

@register.simple_tag(takes_block=True)
def comment(block):
    pass

And the upper tag example from the template documentation becomes:

@register.simple_tag(takes_context=True, takes_block=True)
def upper(context, block):
    return block.render(context).upper()

Robert, I'm not sure if I like the idea of manipulating simple_tag with accessory decorators. As far as I can tell what you are proposing is just a synatactic hack to pass arguments to the simple_tag decorator; the accessory decorators would have no obvious meaning except in the context of simple_tag. I think I prefer the syntax above because there really is only one decorator being applied to the function (but if the original debate about Python decorator syntax is anything to go by then the "right" way will have to be determined by a benevolent dictator;).

simple_tag enhancement

Seems like a useful enhancement.

patch doesn't apply anymore

I have made a patch (super_simple_tag.diff) that improves simple_tag quite a bit. It can now take the context and also the inner block.

It is backward compatible, so you still can do:

@register.simple_tag
def my_tag(bla):

Now you can also do:

@register.simple_tag(takes_context=True)
def my_tag(context, bla):

If you're making a block tage, e.g:

{% mytag "blablabla" %}
....
{% endmytag %}

then you can also do:

@register.simple_tag(takes_block=True)
def my_tag(block_nodelist, bla):

or even:

@register.simple_tag(takes_block=True, takes_context=True)
def my_tag(block_nodelist, context, bla):

Now, the patch seems to work pretty well. I just came across an oddity with block tags. When you do block_nodelist.render(context) it does not render the children templates content properly. You can reproduce that with something like this:

@register.simple_tag(takes_block=True, takes_context=True)
def customspaceless(context, block_nodelist):
    from django.utils.html import strip_spaces_between_tags
    return strip_spaces_between_tags(block_nodelist.render(context).strip())

In parent.html:

{% customspaceless %}
header
{% block content %}
{% endblock %}
footer
{% endcustomspaceless %}

In child.html:

{% extends "parent.html" %}

{% block content %}
middle
{% endblock %}

That will render:

headerfooter

(the child's 'middle' is left out)

Can you spot what's the problem here?

My first assumption is that the nodelist is generated at the right time. Currently I generate it in the generic_tag_compiler function:

def generic_tag_compiler(params, defaults, name, node_class, parser, token, takes_context=False, takes_block=False):
    ...
    if takes_block:
        nodelist = parser.parse(('end%s' % name,)) 
        parser.delete_first_token() 
        node_class = curry(node_class, block_nodelist=nodelist)

Maybe that should be done elsewhere. Any idea?

Replying to julien:

Block substitution depends on Node.get_nodes_by_type(), which will look for a nodelist attribute. So probably you just have to rename block_nodelist to nodelist to fix this.

Thanks emulbreh, well spotted! I have updated the patch, which is now fully fonctional. Will post documentation and test shortly.

line 913 of init.py looks off:

if takes_context != None or takes_context != None:

I think maybe the second takes_context should be takes_block

Quick style note, when comparing for none you should generally do if var is None or if var is not None , instead of using the equality or inequality operator.

Patch + tests + doc (created from revision 7774)

Thanks Alex and justinf, I've applied your suggestions in the new patch.

takes_nodelist instead of takes_block

I'd prefer a takes_nodelist argument since you will get a NodeList that has nothing to do with {% block %}.

Replaced "takes_block" by "takes_nodelist", and "block_nodelist" by "nodelist". Amended the doc and tests as well.

Thanks emulbreh for the suggestion. I too prefer takes_nodelist. I made the change in the latest patch. I kept my previous patch as a base as it takes care of the doc and tests. It is also necessary not to set the "nodelist" attribute if it is None (see SimpleTag?.init).

What about also allowing for variable number of args?

Updated patch to r8984

Updated patch to r9050

Marking DDN as per Malcolm's email on the dev-list: http://groups.google.com/group/django-developers/msg/7692178f04969f5b

Specifically responding to Malcolm's comment:

... we need to consider whether adding something like "takes_nodelist" is really a good idea for something called *simple* tag decorator.

I think that it is worth having takes_nodelist, as it does not necessarily "complexify" simple_tag. My interpretation of "simple tag", in this context, is two-fold: "simple to create a template tag" and "tag with simple syntax". The proposed patch preserves both of those.

Conversely, a complex tag would be something with a complex syntax (e.g. {% if ... and not ... %}, or {% get_comment_count for entry as comment_count %}), and therefore would justify getting your hands dirty with variable resolving and tokens.

At this stage, there is no way to "simply" create a "simple" block tag.

I was wondering: if support for the "takes_context" addition to "simple_tag" is uncontentious could a fresh request be spawned for it so that the need for a decision about the extra "takes_nodelist" stuff doesn't further hold up or prevent that support for "takes_context" being added to simple_tag. If spawning a fresh ticket is not a sensible/desirable thing to do (esp. if I've misunderstood and takes_context support actually is contentious too), fair enough, but thought I'd ask :)