Opened 19 years ago
Closed 18 years ago
#3523 closed (fixed)
Extend "for" tag to allow unpacking of lists
| Reported by: | Chris Beaven | Owned by: | Russell Keith-Magee |
|---|---|---|---|
| Component: | Template system | Version: | dev |
| Severity: | Keywords: | for loop | |
| Cc: | ferringb@… | Triage Stage: | Ready for checkin |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
As discussed here: http://groups.google.com/group/django-developers/browse_thread/thread/5c0de4476c12029f
Basically, rather than having to do this:
{% for post in posts %}
{{ post.0 }} - {{ post.1 }}
{% endfor %}
We should be able to do this:
{% for name, description in posts %}
{{ name }} - {{ description }}
{% endfor %}
Attachments (6)
Change History (19)
by , 19 years ago
| Attachment: | for_tag.patch added |
|---|
comment:1 by , 19 years ago
| Has patch: | set |
|---|---|
| Needs documentation: | set |
| Needs tests: | set |
| Patch needs improvement: | set |
the patch is more a proof of concept thing...
with the patch:
In [1]: from django import template
In [2]: t=template.Template('''{% for a,b in items %} {{ a }} -> {{ b }} {% endfor %}''')
In [3]: t.render( template.Context( { 'items' : [ ('aa',2), (3,4)] } ) )
Out[3]: ' aa -> 2 3 -> 4 '
In [4]: t=template.Template('''{% for a, b in items %} {{ a }} -> {{ b }} {% endfor %}''')
In [5]: t.render( template.Context( { 'items' : [ ('aa',2), (3,4)] } ) )
Out[5]: ' aa -> 2 3 -> 4 '
In [6]: t=template.Template('''{% for a , b in items %} {{ a }} -> {{ b }} {% endfor %}''')
In [7]: t.render( template.Context( { 'items' : [ ('aa',2), (3,4)] } ) )
Out[7]: ' aa -> 2 3 -> 4 '
what remains to be done:
- cleanup (error handling in particular)
- tests
- documentation
by , 19 years ago
| Attachment: | for_tag.2.patch added |
|---|
sorry, a minor problem with the previous version
comment:2 by , 19 years ago
| Needs documentation: | unset |
|---|---|
| Needs tests: | unset |
| Patch needs improvement: | unset |
| Triage Stage: | Unreviewed → Design decision needed |
comment:3 by , 19 years ago
| Patch needs improvement: | set |
|---|---|
| Summary: | Extend "for" tag to allow unpacking of lists → A context is pushed in the loop which isn't popped |
for_tag.3.patch (line 125)
context.update(dict(zip(self.loopvars, item)))
will push a new context dict into the current context, so it will need to be popped later.
It could be changed to:
context.dicts[0].update(zip(self.loopvars, item))
which puts the variables into the current context dict.
BTW I think this is a bug in the Context class:
Context.update(x) is defined as self.dicts = [x] + self.dicts
I think it should be defined as self.dicts[0].update(x)
That would allow this line in your patch to be:
context.update(zip(self.loopvars, item))
--
Arnaud
comment:4 by , 19 years ago
| Summary: | A context is pushed in the loop which isn't popped → Extend "for" tag to allow unpacking of lists |
|---|
Sorry
comment:5 by , 19 years ago
It looks like you're correct. According to the docstring of Context.update it "Pushes an entire dictionary's keys and values onto the context." but currently it doesn't. I'll open another ticket regarding this.
comment:6 by , 19 years ago
| Patch needs improvement: | unset |
|---|
This patch doesn't need improvement, it just relies on bug #3529 being fixed.
comment:7 by , 18 years ago
| Keywords: | for loop added |
|---|
If patch 2 in #3529 is chosen, this patch should be updated
from:
context.update(dict(zip(self.loopvars, item)))
to:
context.update(dict(zip(self.loopvars, item)), push=False)
comment:8 by , 18 years ago
| Needs documentation: | set |
|---|---|
| Needs tests: | set |
comment:9 by , 18 years ago
| Needs documentation: | unset |
|---|---|
| Needs tests: | unset |
Gosh, I can't even remember my own patch already had tests and docs :P
comment:10 by , 18 years ago
| Cc: | added |
|---|
comment:11 by , 18 years ago
The bug fix was variable values from the previous loop could sneak on to the context if the current set of items was shorter than the loopvars list - see test for-tag-unpack09.
The format checking has been tightened so that keyword arguments must be comma separated -- either {% for a,b in c %} or {% for a, b in c %} will work.
This patch now has no reliance on bug #3529 since it actually uses the push behaviour now. It was however brought up in discussion that this tag could be more efficient if context.update also accepted 2-length tuples, but that's outside of the scope of this ticket.
by , 18 years ago
| Attachment: | for_tag.5.patch added |
|---|
Oops, broke previous version just before uploading
by , 18 years ago
| Attachment: | unpacking-for.diff added |
|---|
Slightly improved for loop unpacking implementation
comment:12 by , 18 years ago
| Owner: | changed from to |
|---|---|
| Triage Stage: | Design decision needed → Ready for checkin |
I've made a few additions to the patch. For the most part, it's documentation cleanup; but I have added a few more test cases for whitespace around the commas in the unpack list, and made the parser a little more forgiving with those whitespace cases (using RE's, rather than just str.replace).
Other than that, I'm happy for this to go in. I'll ping this on the list again, but if there's no objections, I'm happy for it to go into trunk.
comment:13 by , 18 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
first crude version of the patch