Re: Enabling context access in simple_tag
On Dec 15, 10:52 pm, Russell Keith-Mageewrote: > On Wed, Dec 15, 2010 at 6:41 PM, Julien Phalip wrote: > > On Dec 14, 7:34 pm, Christian Hammond wrote: > >> On Dec 14, 12:02 am, Julien Phalip wrote: > > >> > On Dec 13, 10:16 am, Tai Lee 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: > >> > @basictag:https://github.com/djblets/djblets/blob/master/djblets/util/decorator... > >> > @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? > > >> For what it's worth, these two decorators have seriously cut down on > >> 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 > > > I think the djblet tags approach is the most reasonable one. It > > 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)? > > 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 %-) Ok, it looks like we've got a consensus which will scratch the 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 -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: Enabling context access in simple_tag
On Dec 14, 7:34 pm, Christian Hammondwrote: > On Dec 14, 12:02 am, Julien Phalip wrote: > > > > On Dec 13, 10:16 am, Tai Lee 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: > > @basictag:https://github.com/djblets/djblets/blob/master/djblets/util/decorator... > > @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? > > For what it's worth, these two decorators have seriously cut down on > 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 I think the djblet tags approach is the most reasonable one. It 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 -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: Enabling context access in simple_tag
On Dec 14, 12:02 am, Julien Phalipwrote: > On Dec 13, 10:16 am, Tai Lee 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: > @basictag:https://github.com/djblets/djblets/blob/master/djblets/util/decorator... > @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? For what it's worth, these two decorators have seriously cut down on 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 -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: Enabling context access in simple_tag
On Dec 13, 10:16 am, Tai Leewrote: -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: @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 -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: Enabling context access in simple_tag
On Sun, Dec 12, 2010 at 11:07 PM, Brian O'Connorwrote: >> 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. > > 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 %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: Enabling context access in simple_tag
I've been using the patch from #1105 in my local branch of Django for 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. -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: Enabling context access in simple_tag
> > 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. Forgive me if this proposal has already been turned down, but what about the following: We give takes_context to the simple_tag shortcut, for consistency. Then a new shortcut, assignment_tag is added such as @register.assignment_tag def get_foobars(): // do work return results Then, within the template, the user simply calls `get_foobars as coconuts`, `results` gets assigned to `coconuts` within the context. There is also the broader (and completely nebulous) > proposal regarding making tag parsers easier to write. That's a *much* > bigger can of worms. Personally, I don't think writing a template tag is _too hard_. I think it's overkill for the simple tasks, namely assigning content to a variable. If we had a shortcut to do that, anything beyond that could be done as a regular custom tag. (sorry if this was formatted horribly, I was battling with gmail here). -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: Enabling context access in simple_tag
On Sat, Dec 11, 2010 at 5:29 PM, Julien Phalipwrote: > 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 %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Enabling context access in simple_tag
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... 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 -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.