Opened 11 years ago

Closed 10 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: master
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: UI/UX:

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)

for_tag.patch (3.0 KB) - added by Honza Král <Honza.Kral@…> 11 years ago.
first crude version of the patch
for_tag.2.patch (3.0 KB) - added by Honza Král <Honza.Kral@…> 11 years ago.
sorry, a minor problem with the previous version
for_tag.3.patch (7.1 KB) - added by Chris Beaven 11 years ago.
new and improved :)
for_tag.4.patch (7.9 KB) - added by Chris Beaven 10 years ago.
small bug fix and tighter format checking
for_tag.5.patch (7.9 KB) - added by Chris Beaven 10 years ago.
Oops, broke previous version just before uploading
unpacking-for.diff (9.0 KB) - added by Russell Keith-Magee 10 years ago.
Slightly improved for loop unpacking implementation

Download all attachments as: .zip

Change History (19)

Changed 11 years ago by Honza Král <Honza.Kral@…>

Attachment: for_tag.patch added

first crude version of the patch

comment:1 Changed 11 years ago by Honza Král <Honza.Kral@…>

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

Changed 11 years ago by Honza Král <Honza.Kral@…>

Attachment: for_tag.2.patch added

sorry, a minor problem with the previous version

Changed 11 years ago by Chris Beaven

Attachment: for_tag.3.patch added

new and improved :)

comment:2 Changed 11 years ago by Chris Beaven

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedDesign decision needed

comment:3 Changed 11 years ago by arnodel@…

Patch needs improvement: set
Summary: Extend "for" tag to allow unpacking of listsA 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 Changed 11 years ago by arnodel@…

Summary: A context is pushed in the loop which isn't poppedExtend "for" tag to allow unpacking of lists

Sorry

comment:5 Changed 11 years ago by Chris Beaven

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 Changed 11 years ago by Chris Beaven

Patch needs improvement: unset

This patch doesn't need improvement, it just relies on bug #3529 being fixed.

comment:7 Changed 10 years ago by Chris Beaven

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 Changed 10 years ago by Chris Beaven

Needs documentation: set
Needs tests: set

comment:9 Changed 10 years ago by Chris Beaven

Needs documentation: unset
Needs tests: unset

Gosh, I can't even remember my own patch already had tests and docs :P

comment:10 Changed 10 years ago by (removed)

Cc: ferringb@… added

Changed 10 years ago by Chris Beaven

Attachment: for_tag.4.patch added

small bug fix and tighter format checking

comment:11 Changed 10 years ago by Chris Beaven

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.

Changed 10 years ago by Chris Beaven

Attachment: for_tag.5.patch added

Oops, broke previous version just before uploading

Changed 10 years ago by Russell Keith-Magee

Attachment: unpacking-for.diff added

Slightly improved for loop unpacking implementation

comment:12 Changed 10 years ago by Russell Keith-Magee

Owner: changed from Adrian Holovaty to Russell Keith-Magee
Triage Stage: Design decision neededReady 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 Changed 10 years ago by Russell Keith-Magee

Resolution: fixed
Status: newclosed

(In [5443]) Fixed #3523 -- Added list unpacking to for loops in templates. Thanks to SmileyChris and Honza Kral for their work.

Note: See TracTickets for help on using tickets.
Back to Top