#6398 closed (fixed)
Add optional {% default %} clause to {% for %} template tag
| Reported by: | Jannis Leidel | Owned by: | Jannis Leidel |
|---|---|---|---|
| Component: | Template system | Version: | dev |
| Severity: | Keywords: | easy-pickings | |
| Cc: | Triage Stage: | Accepted | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
As requested by Simon I propose an {% else %} tag for the {% for %} template tag which could be used like this:
from django.template import *
>>> t1 = Template("""
{% load mytags %}
{% for athlete in athlete_list %}
{{ athlete }}
{% else %}
No athlete in list!
{% endfor %}
""")
>>> c1 = Context(
{'athlete_list':
['me', 'myself', 'I']
})
>>> t1.render(c1)
u'me myself I '
>>> c2 = Context({})
>>> t1.render(c2)
u'No athlete in list!'
For a standalone version look at http://www.djangosnippets.org/snippets/546/
Attachments (6)
Change History (33)
by , 18 years ago
| Attachment: | add_else_to_for.diff added |
|---|
comment:1 by , 18 years ago
| Triage Stage: | Unreviewed → Ready for checkin |
|---|
comment:2 by , 18 years ago
The same be accomplished by this:
{% if athlete_list %}
{% for athlete in athlete_list %}
...
{% endfor %}
{% else %}
Nothing in list
{% endif %}
comment:3 by , 18 years ago
| Triage Stage: | Ready for checkin → Design decision needed |
|---|
comment:4 by , 18 years ago
Such behaviour can be confusing as it is not similar to python
for object in objects:
if object:
print 'Found'
break
else:
print 'Not found'
pattern.
comment:5 by , 18 years ago
| Triage Stage: | Design decision needed → Accepted |
|---|
The idea gets a +1 from me.
comment:6 by , 18 years ago
I wonder how useful this new syntax is, as you can't use it for the following (pretty common) pattern:
{% if athlete_list %}
<ul>
{% for athlete in athlete_list %}
<li>{{ athlete.name }}</li>
{% endfor %}
</ul>
{% else %}
<p>Nothing in list</p>
{% endif %}
(Because there is no place where you can put the <ul> tags.)
Plus it doesn't comply with the Python syntax (as Anonymous@20:51 says).
comment:7 by , 18 years ago
Thanks Adrian! Matthias, I think it's still worthwhile and useful if you use:
<ul>
{% for athlete in athlete_list %}
<li>{{ athlete.name }}</li>
{% else %}
<li class="emptylist">Nothing in list</li>
{% endfor %}
</ul>
I don't see a reason why compliance with the Python syntax is required for a template tag.
comment:8 by , 18 years ago
There's a good discussion about this on my blog, where Fredrik points out that this would be semantically different from how Python handles else: blocks attached to for statements. As such, the extra block would need a different name - {% default %} was suggested.
by , 18 years ago
| Attachment: | add_default_to_for.diff added |
|---|
Second implementation, including documentation
comment:10 by , 18 years ago
| Summary: | Add optional {% else %} clause to {% for %} template tag → Add optional {% default %} clause to {% for %} template tag |
|---|
Ok, this sounds like a reasonable compromise. I changed it to having an optional {% default %} clause.
comment:11 by , 18 years ago
The working example is:
>>> from django.template import *
>>> t = Template("""
{% for athlete in athlete_list %}
{{ athlete }}
{% default %}
No athlete in list!
{% endfor %}
""")
>>> c1 = Context(
{'athlete_list':
['me', 'myself', 'I']
})
>>> t.render(c1)
u'me myself I '
>>> c2 = Context({})
>>> t.render(c2)
u'No athlete in list!'
comment:12 by , 18 years ago
| Needs tests: | set |
|---|---|
| Patch needs improvement: | set |
Still a reference to else in the patch (in a docstring), and needs tests.
I'd suggest dropping the "As you can see, " in the docs and also to mention that the default node is displayed if the given array is empty or not found (emphasis for the bit that I'm suggesting adding).
Fix those and it's good to go.
by , 18 years ago
| Attachment: | add_default_to_for.2.diff added |
|---|
Third implementation, including documentation and tests, fixing some issues, thanks SmileyChris!
comment:13 by , 18 years ago
| Needs tests: | unset |
|---|
comment:15 by , 18 years ago
If you're confident that you've fixed the issue, sure. It doesn't change it's status so that is alright.
Final thoughts:
- Perhaps make
nodelist_defaultoptional in case someone is usingForNodefor thier own devices?def __init__(self, loopvars, sequence, is_reversed, nodelist_loop, nodelist_default=None):(and then move the emptyNodeListgeneration into their too)
- You should have a test for the actual working case (i.e. non-empty list)
- You need an empty line after "New in Django development version"
comment:16 by , 18 years ago
Ok, sounds all reasonable, except one thing: Do you mean with "the empty NodeList generation" the following?
class ForNode(Node):
...
def render(self, context):
nodelist = NodeList() # <-- this line?
...
comment:17 by , 18 years ago
Yea, that logic obviously needs to move if nodelist_default could be None in __init__
by , 18 years ago
| Attachment: | add_default_to_for.3.diff added |
|---|
Fourth implementation, including documentation and tests. Do you think this is sufficient? Maby I still don't see your point :/
comment:19 by , 18 years ago
Close but not quite, you need something like this:
lass ForNode(Node): def __init__(self, loopvars, sequence, is_reversed, nodelist_loop, nodelist_default=None): self.loopvars, self.sequence = loopvars, sequence self.is_reversed = is_reversed self.nodelist_loop = nodelist_loop if nodelist_default is None: self.nodelist_default = NodeList() else: self.nodelist_default = nodelist_default
comment:20 by , 18 years ago
| Owner: | changed from to |
|---|
comment:21 by , 18 years ago
| Keywords: | easy-pickings added |
|---|
comment:22 by , 18 years ago
| Patch needs improvement: | unset |
|---|
by , 18 years ago
| Attachment: | add_default_to_for.4.diff added |
|---|
fifth implementation with optional nodelist_default, including documentation and tests.
comment:23 by , 17 years ago
| milestone: | → post-1.0 |
|---|---|
| Status: | new → assigned |
by , 17 years ago
| Attachment: | ticket6398-r9032.diff added |
|---|
Updated patch to Django r9032 (http://github.com/jezdez/django/tree/ticket6398)
comment:25 by , 17 years ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
(In [9530]) Fixed #6398: added an optional {% empty %} clause to the {% for %} template tag. The contents of this clause are rendered if the list iterated over turns out to be empty. Thanks, Jannis Leidel.
Astute readers will notice that the patch originally called this default; after consideration I decided that empty is a very slightly better color for this particular bikeshed.
comment:26 by , 17 years ago
| Resolution: | fixed |
|---|---|
| Status: | closed → reopened |
As far as I can see, the conversion from 'default' to 'empty' in the patch is not complete. At least, the tests seems broken, as they use "{% default %}" and not "{% empty %}".
Check out the following in #9530:
- django/trunk/django/template/defaulttags.py lines 682 and 684 (and optionally 'nodelist_default' at 685, 688-689), and
- django/trunk/tests/regressiontests/templates/tests.py lines 487-489.
comment:27 by , 17 years ago
| Resolution: | → fixed |
|---|---|
| Status: | reopened → closed |
First implementation, including documentation