[patch] simple_tag decorator enhancement

[django@…]

Attached is a patch for a tag function decorator that I find useful. It's a simple extension of Robert's simple_tag. The tag function is passed the context as the first argument and also any arguments to the tag, resolved in the same way as for the simple_tag decorator. The decorated function can manipulate the context and either return a string, which will be inserted into the template, or None.

For example, this tag puts the project settings into the context:

@register.simple_tag_with_context
def get_settings(context):

    """
    Put the project settings into the context.

    Usage::

        {% get_settings %}
    """

    from django.conf import settings
    context['settings'] = settings

And this one is the equivalent of the existing simple_tag example in NewAdminChanges:

@register.simple_tag_with_context
def monkey_tag(context, verb):
    return "I %s no evil" % verb

[django@…]

Patch adds simple_tag_with_context tag function decorator

[rjwittams]

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.

[django@…]

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;).

[django@…]

simple_tag enhancement

[Chris Beaven]

Seems like a useful enhancement.

[racter]

patch doesn't apply anymore

[Julien Phalip]

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):

[Julien Phalip]

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?

[Johannes Dollinger]

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.

[Julien Phalip]

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

[Justin]

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

[Alex Gaynor]

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.

[Julien Phalip]

Patch + tests + doc (created from revision 7774)

[Julien Phalip]

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

[Johannes Dollinger]

takes_nodelist instead of takes_block

[Johannes Dollinger]

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

[Julien Phalip]

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

[Julien Phalip]

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).

[timolux]

What about also allowing for variable number of args?

[Julien Phalip]

Updated patch to r8984

[Julien Phalip]

Updated patch to r9050

[Julien Phalip]

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.

[Skaffen]

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 :)

[Ilya Semenov]

That is just so unfair that register.inclusion_tag does accept takes_context=True and register.simple_tag doesn't. That leads to simple yet inefficient tags that render from a template just to be able to access the context. (The alternative is to create monstrous full-blown Node classes which is an overkill in most situations).

[twig]

i agree with semenov.

the reason i've found this ticket is because i was looking for a simpler way of accessing the context.

[valentin golev]

So, will it be added to trunk?

[Ilya Semenov]

Considering my last attempt to add a bit of more usability to Django API (#11824), that's not going to happen. You can use this third-party implementation of simple_tag which has takes_context parameter: ​http://svn.navi.cx/misc/trunk/djblets/djblets/util/decorators.py

[Carl Meyer]

Per design conversation with Malcolm, Russell, and Jannis at sprint, this is in essence accepted, but we're opening a new ticket (#14262) for better clarity.