Enabling context access in simple_tag
Julien Phalip
I'd like to bring up an itch that I've been desperate to scratch since
my very early days with Django: the inability to access the context
from a simple_tag.
There are especially two use cases which I encounter on a near weekly-
basis:
(a) Generate a simple string or some very concise html code based on
the context.
(b) Update the context itself.
Both use cases can currently be achieved either by creating a Node
class and a parser (tedious and boilerplatey) or by using an
inclusion_tag (overkill having to create a template). You may also use
some workarounds such as in ReviewBoard's Djblets [1], but that means
you'd have to introduce a dependency in your projects or copy/paste
those workarounds in every project. Really, I've always thought that
life as a Django developer would be so much easier if only simple_tag
could access the context...
To give a bit of history: when I first got interested in this problem,
I created #7462, and then was advised to close it in favour of an
older one #1105. Finally, nearly two years later, #1105 got closed as
a duplicate of #14262.
#14262 particularly addresses use case (b), that is, create a special
template tag that would make updating the context easy while
presumably returning an empty string. In #1105, however, I had written
a patch which would have killed two birds with one stone by addressing
both (a) and (b). Plus that patch would have allowed you to update the
context AND to return some result if you wanted to (instead of just
returning an empty string). That patch enabled context access in
simple_tag, following the same declarative syntax as for
inclusion_tag:
@register.simple_tag(takes_context=True)
def mytag(context, foo, bar):
...
It never made sense to me why inclusion_tag could access the context,
and not simple_tag. One common argument I've heard is that it is
because simple_tag should remain "simple". However, I don't see how
enabling context access would necessarily make simple_tag "complex".
From my own developer's perspective, the "simple" in "simple_tag"
mostly means that the *template syntax* should be straightforward as
opposed to a complex syntax like {% mytag arg1 with arg2 as var1 and
var2 %} for which you should definitely prepare yourself to go down
the dark alley of Nodes and parsers (or give it a chance with the
rather neat django-template-sugar [2]).
I think the main reason why #1105 has been closed is that it has
drifted a bit too far off. I started by adding content access in my
initial patch, and then I got carried away following people's
suggestions by adding access to the inner block as well. This led to
the 'takes_context_plus_sink' problem that Malcolm rightly pointed out
in a recent tweet [3]. Even if I'd love to have easy access to the
inner block, I now admit it might be a bit much to make that work with
simple_tag (probably should be a separate new ticket?).
This topic has received some support from Simon [4] [5] and Jacob [6]
when I initially brought it up on this list 2 years ago. I believe
Malcolm and Chris Beaven, amongst others, have also expressed some
interest by participating in the relevant tickets. I hope that people
still want this and that there's a chance to make it ship in 1.3
(since 1.3's focus is on scratching all those annoying little itches).
In summary, what do you think about enabling context access in
simple_tag (using the takes_context syntax)? If the response is
positive, I'm very keen to lead the way and promptly write a patch for
it. A side question is: would this make #14262 redundant?
Thanks!
Julien :)
[1] http://www.chipx86.com/blog/2008/02/29/django-development-with-djblets-custom-tag-helpers/
[2] https://github.com/alex/django-templatetag-sugar
[3] http://twitter.com/malcolmt/status/13145334247063552 (click on the
'In reply to...' links to get the whole discussion)
[4] http://groups.google.com/group/django-developers/browse_thread/thread/fba22c3e3c910bb9/b39a0a79ed991ca8
[5] http://groups.google.com/group/django-developers/browse_thread/thread/62d0cefde54a50a3/e58e2202ef125976
[6] http://groups.google.com/group/django-developers/browse_thread/thread/afb7c3cd93e7a659/637973df3b839b45
Relevant tickets:
http://code.djangoproject.com/ticket/1105
http://code.djangoproject.com/ticket/2619
http://code.djangoproject.com/ticket/7462
http://code.djangoproject.com/ticket/14262
Also discussed in:
http://groups.google.com/group/django-developers/browse_thread/thread/8737db04db7e60af/e3276998ab34f022
http://groups.google.com/group/django-developers/browse_thread/thread/d83241466444a02/bcb78463537c82f3
Russell Keith-Magee
> Hello,
>
> I'd like to bring up an itch that I've been desperate to scratch since
> my very early days with Django: the inability to access the context
> from a simple_tag.
>
> There are especially two use cases which I encounter on a near weekly-
> basis:
>
> (a) Generate a simple string or some very concise html code based on
> the context.
> (b) Update the context itself.
>
> Both use cases can currently be achieved either by creating a Node
> class and a parser (tedious and boilerplatey) or by using an
> inclusion_tag (overkill having to create a template). You may also use
> some workarounds such as in ReviewBoard's Djblets [1], but that means
> you'd have to introduce a dependency in your projects or copy/paste
> those workarounds in every project. Really, I've always thought that
> life as a Django developer would be so much easier if only simple_tag
> could access the context...
Thanks for the summary, Julien.
Since I'm one of the Grinches that closed ticket #1105 :-), let me try
to explain the motivation behind that action.
I'm in complete agreement that template tags are too hard to write
from scratch, and I'm interested in discussing any proposal that
addresses this.
However, my concern with modifications like the one proposed by #1105
is that simple_tag becomes more complex, yet still fails to hit a
range of 'simple' use cases -- or, at least, fails to hit them in a
way that is broadly useful.
I can't deny that adding 'takes_context' would open up opportunities
to simple_tag that aren't currently available. But there are also some
fairly major practical limitations that are born out of the
fundamental framework that simple_tag provides.
As an example of what I'm talking about -- #14262 is a manifestation
of a use case that is undeniably simple: "get_function() as var". This
pattern is used in several places in Django's own codebase.
However, adding "takes_context" to simple_tag won't allow you to
completely hit the #14262 use case without dropping the ability to
specify the context variable that get_function() is stored. Although
this may seem like a little detail, it's a fairly important part of
the API if you want your tag to be useful to a broad audience --
without this capability, your template tag must always insert the same
context variable. This limits the ways that you can use your tag
without inducing collisions -- for example, you can't call the tag
twice in the same template.
This gets even more acute when you consider takes_nodelist. From a
quick audit, the only builtin tags that could be replaced with a
'simple_tag with takes_nodelist' are comment, spaceless, and the
old-style ifequal family of tags, which is hardly a compelling list of
use cases. In all other tags, there is a need to do some sort of
parsing of the input tokens to provide 'as' placeholders and the like.
So - the reason #1105 was closed was because simple_tag should be
simple. Adding complexity to simple_tag in an attempt to do an end run
around the difficulties of writing custom tags just leaves us with a
hideously complex simple tag, and misses the real underlying problem
-- that writing custom tag parsers is difficult, and shouldn't be.
Now, the one piece of the puzzle that undermines my argument here is
inclusion_tag, which provides a 'takes_context' argument, providing
precedent. I'm also hamstrung by the fact that I don't have a good
counterproposal.
To that end, I'm willing to be practical and concede that adding
takes_context would at the very least be consistent, even if it still
fails to hit a large range of test cases.
However, this is orthogonal to #14262, because of the inability to
define a tag that is assigned to an arbitrary context variable that I
mentioned earlier. There is also the broader (and completely nebulous)
proposal regarding making tag parsers easier to write. That's a *much*
bigger can of worms.
However, as with all things, I'm also interested in hearing community opinion.
Yours,
Russ Magee %-)
Brian O'Connor
As an example of what I'm talking about -- #14262 is a manifestation
of a use case that is undeniably simple: "get_function() as var". This
pattern is used in several places in Django's own codebase.
To that end, I'm willing to be practical and concede that adding takes_context would at the very least be consistent, even if it still fails to hit a large range of test cases.
There is also the broader (and completely nebulous)
proposal regarding making tag parsers easier to write. That's a *much*
bigger can of worms.
Tai Lee
a long time now. To be honest, I've hit the need to have access to the
existing context in a simple_tag far more frequently than the need to
return a single value as a named variable in the context, and I'm
definitely +1 on adding a `takes_context` argument to `simple_tag` (or
something else that achieves the same thing).
However, regarding the `register.assignment_tag` proposal, we still
have the same problem -- some people will want access to the context
in there, too.
One suggestion from #1105 was to split out this functionality into
individual decorators, @takes_context, @takes_block. I'm not sure how
easy this would be technically to implement, but I think it would
solve the `takes_context_plus` sink problem Malcolm describes as we
potentially add more special case tag types (simple, inclusion,
assignment, etc.)
However, I guess this would be more involved and may involve some
backwards incompatibility and a deprecation path. I don't think any
full blown ideal solution should prevent us form committing just the
`takes_context` argument to `simple_tag` immediately (bringing it
inline with `inclusion_tag`, which has had this ability forever), a
request that has been around for a long time that would solve both use
cases mentioned here.
Even though it doesn't have the "as X" syntactic sugar of an
`assignment_tag`, it is more versatile -- you can update more than
just one variable, and if you need to name the variable it will go
into you could always just use the first argument to your simple tag
to define it e.g. `{% getfunction_as "varname" %}`
Cheers.
Tai.
Russell Keith-Magee
>> As an example of what I'm talking about -- #14262 is a manifestation
>> of a use case that is undeniably simple: "get_function() as var". This
>> pattern is used in several places in Django's own codebase.
>
> <snip>
>>
>> To that end, I'm willing to be practical and concede that
>> adding takes_context would at the very least be consistent, even if it
>> still fails to hit a large range of test cases.
>
> Forgive me if this proposal has already been turned down, but what about the
> following:
What you've described is essentially what the current proposal *is* -
i.e., add takes_context to context_tag (#1105), but also add a
`get_function() as var` shortcut (#14262). The exact color of #14262
hasn't been picked, but assignment_tag is as good as any I've heard.
Yours,
Russ Magee %-)
Julien Phalip
-snip-
> One suggestion from #1105 was to split out this functionality into
> individual decorators, @takes_context, @takes_block. I'm not sure how
> easy this would be technically to implement, but I think it would
> solve the `takes_context_plus` sink problem Malcolm describes as we
> potentially add more special case tag types (simple, inclusion,
> assignment, etc.)
decorators for exactly this purpose:
@basictag: https://github.com/djblets/djblets/blob/master/djblets/util/decorators.py#L96
@blocktag: https://github.com/djblets/djblets/blob/master/djblets/util/decorators.py#L161
This now seems to me like the perfect compromise. It would generally
allow for more versatility and to keep simple_tag (and the future
assignment_tag) free of takes_xxx cruft.
Any chance these two decorators could be considered for inclusion in
Django core?
Cheers,
Julien
Christian Hammond
> On Dec 13, 10:16 am, Tai Lee <real.hu...@mrmachine.net> wrote:
>
> -snip-
>
> > One suggestion from #1105 was to split out this functionality into
> > individual decorators, @takes_context, @takes_block. I'm not sure how
> > easy this would be technically to implement, but I think it would
> > solve the `takes_context_plus` sink problem Malcolm describes as we
> > potentially add more special case tag types (simple, inclusion,
> > assignment, etc.)
>
> The djblets (created by the guys at reviewboard.org) contain two nifty
> decorators for exactly this purpose:
> @blocktag:https://github.com/djblets/djblets/blob/master/djblets/util/decorator...
> This now seems to me like the perfect compromise. It would generally
> allow for more versatility and to keep simple_tag (and the future
> assignment_tag) free of takes_xxx cruft.
>
> Any chance these two decorators could be considered for inclusion in
> Django core?
our development and maintenance burdens. Whether they'd be sufficient
as-is, I don't know (though you're welcome to have the code), but I'd
also love to see equivalent functionality in Django.
If I can help in some way to get these in shape (assuming you'd want
to go that direction), just let me know.
Christian
Julien Phalip
provides the feature originally requested without disrupting any
existing code or API. The current code works and has been successfully
tested in production environments for a long time.
I'm keen to push this through, but I'm also concerned that the future
freeze deadline is upon us. I don't want to create too much noise in
the issue tracker, so could a core dev advise on the way to go from
here? Considering the djblet approach is judged reasonable, shall we
create two different tickets (one for each tag)?
Many thanks!
Julien
Russell Keith-Magee
I'm only one voice, but I'm *really* not a fan of decorators changing
function signatures like that. I don't deny it works; it just grates
me the wrong way.
I've already indicated my preferred approach -- consistency with inclusion_tag.
Yours
Russ Magee %-)
Julien Phalip
wrote:
original itch. I've created a ticket for it and will work on a patch
over the weekend: http://code.djangoproject.com/ticket/14908#preview
Thank you all!
Regards,
Julien