Django

Code

Ticket #3523 (closed: fixed)

Opened 2 years ago

Last modified 1 year ago

Extend "for" tag to allow unpacking of lists

Reported by: SmileyChris Assigned to: russellm
Milestone: Component: Template system
Version: SVN Keywords: for loop
Cc: ferringb@gmail.com Triage Stage: Ready for checkin
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

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

for_tag.patch (3.0 kB) - added by Honza Král <Honza.Kral@gmail.com> on 02/18/07 16:25:21.
first crude version of the patch
for_tag.2.patch (3.0 kB) - added by Honza Král <Honza.Kral@gmail.com> on 02/18/07 16:31:08.
sorry, a minor problem with the previous version
for_tag.3.patch (7.1 kB) - added by SmileyChris on 02/18/07 23:54:42.
new and improved :)
for_tag.4.patch (7.9 kB) - added by SmileyChris on 06/05/07 18:50:25.
small bug fix and tighter format checking
for_tag.5.patch (7.9 kB) - added by SmileyChris on 06/05/07 23:25:44.
Oops, broke previous version just before uploading
unpacking-for.diff (9.0 kB) - added by russellm on 06/07/07 08:42:13.
Slightly improved for loop unpacking implementation

Change History

02/18/07 16:25:21 changed by Honza Král <Honza.Kral@gmail.com>

  • attachment for_tag.patch added.

first crude version of the patch

02/18/07 16:29:46 changed by Honza Král <Honza.Kral@gmail.com>

  • needs_better_patch set to 1.
  • has_patch set to 1.
  • needs_tests set to 1.
  • needs_docs set to 1.

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

02/18/07 16:31:08 changed by Honza Král <Honza.Kral@gmail.com>

  • attachment for_tag.2.patch added.

sorry, a minor problem with the previous version

02/18/07 23:54:42 changed by SmileyChris

  • attachment for_tag.3.patch added.

new and improved :)

02/18/07 23:56:49 changed by SmileyChris

  • needs_better_patch deleted.
  • stage changed from Unreviewed to Design decision needed.
  • needs_tests deleted.
  • needs_docs deleted.

02/19/07 01:25:04 changed by arnodel@gmail.com

  • needs_better_patch set to 1.
  • summary changed from Extend "for" tag to allow unpacking of lists to 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

02/19/07 01:26:17 changed by arnodel@gmail.com

  • summary changed from A context is pushed in the loop which isn't popped to Extend "for" tag to allow unpacking of lists.

Sorry

02/19/07 13:22:06 changed by SmileyChris

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.

02/19/07 13:28:49 changed by SmileyChris

  • needs_better_patch deleted.

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

05/30/07 17:22:16 changed by SmileyChris

  • keywords set to for loop.

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) 

05/31/07 16:23:37 changed by SmileyChris

  • needs_docs set to 1.
  • needs_tests set to 1.

05/31/07 16:25:01 changed by SmileyChris

  • needs_docs deleted.
  • needs_tests deleted.

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

06/02/07 22:48:39 changed by Brian Harring <ferringb@gmail.com>

  • cc set to ferringb@gmail.com.

06/05/07 18:50:25 changed by SmileyChris

  • attachment for_tag.4.patch added.

small bug fix and tighter format checking

06/05/07 18:59:51 changed by SmileyChris

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.

06/05/07 23:25:44 changed by SmileyChris

  • attachment for_tag.5.patch added.

Oops, broke previous version just before uploading

06/07/07 08:42:13 changed by russellm

  • attachment unpacking-for.diff added.

Slightly improved for loop unpacking implementation

06/07/07 08:45:40 changed by russellm

  • owner changed from adrian to russellm.
  • stage changed from Design decision needed to 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.

06/08/07 06:58:03 changed by russellm

  • status changed from new to closed.
  • resolution set to fixed.

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


Add/Change #3523 (Extend "for" tag to allow unpacking of lists)




Change Properties
Action