cycle in nested loops

114 views
Skip to first unread message

Wim Feijen

unread,
Jan 21, 2013, 12:01:07 PM1/21/13
to django-d...@googlegroups.com
Hi,

Quoting ticket 5908:

"Under the following example, I would expect the cycle tag's internal counter to be reset after leaving the inner loop. That way it'd be 'nesting safe' :-) 

{% for group in grouped %}
  {% for item in group.list %}
   ... {% cycle row1,row2,row3 %} ...
  {% endfor %}
{% endfor %}
"
Actually, the current behaviour is not to reset between loops. Is that intentional? 

A patch to ticket 5908 adds a reset_cycle tag, which resets the cycle-loop. I am wondering if it is the right way to solve this problem, or whether it would be better to modify cycle's current behaviour to reset the loop after each endfor; or maybe if we want another kind of cycle which allows for setting both options?

What do you think? 

Aymeric Augustin

unread,
Jan 31, 2013, 6:01:35 PM1/31/13
to django-d...@googlegroups.com
Hi Wim,

Le 21 janv. 2013 à 18:01, Wim Feijen <w...@go2people.nl> a écrit :
> Actually, the current behaviour is not to reset between loops. Is that intentional?

Yes, this behavior is intentional. See Malcolm's comment (#4) on the ticket.

> A patch to ticket 5908 adds a reset_cycle tag, which resets the cycle-loop. I am wondering if it is the right way to solve this problem

That's what Malcolm said, and I tend to agree with him. I'm not particularly excited by adding a new tag but I don't have a better idea. Something like {% cycle row_colors reset %} would conflict with the regular {% cycle %} syntax.

> or whether it would be better to modify cycle's current behaviour to reset the loop after each endfor; or maybe if we want another kind of cycle which allows for setting both options?

I don't think we could justify the backwards incompatibility for the current {% cycle %} tag. Besides, introducing coupling between {% cycle %} and {% for %} / {% endfor %} sounds like a bad idea.

--
Aymeric.



Shai Berger

unread,
Feb 2, 2013, 3:10:51 PM2/2/13
to django-d...@googlegroups.com
Hi,

On Friday 01 February 2013, Aymeric Augustin wrote:
> Hi Wim,
>
> Le 21 janv. 2013 à 18:01, Wim Feijen <w...@go2people.nl> a écrit :
> > Actually, the current behaviour is not to reset between loops. Is that
> > intentional?
>
> Yes, this behavior is intentional. See Malcolm's comment (#4) on the
> ticket.
>
> > A patch to ticket 5908 adds a reset_cycle tag, which resets the
> > cycle-loop. I am wondering if it is the right way to solve this problem
>
> That's what Malcolm said, and I tend to agree with him. I'm not
> particularly excited by adding a new tag but I don't have a better idea.
> Something like {% cycle row_colors reset %} would conflict with the
> regular {% cycle %} syntax.
>

There are two other ways I see.

1) There is a way to include a 'reset' keyword in the cycle tag -- it only
requires the use of an assignment; the same way allows use of the 'silent'
keyword.

{% cycle 'v1' 'v2' v3 as dummy reset %}

This is a little awkward -- the 'silent' keyword indeed doesn't make sense
without an assignment, but for 'reset' forcing the assignment is only done for
backwards-compatibility's sake. Also, as Aymeric said,

> Besides, introducing coupling between {% cycle %} and
> {% for %} / {% endfor %} sounds like a bad idea.

2) I think it may be better to solve this by giving the user more control over
the cycling -- letting them set the index variable, so, use something like

{% cycle 'v1' 'v2' v3 index=forloop.counter0 %}

Where this still deserves being called "cycle" and not "select" because of the
cyclic interpretation of the index. This solves the problem in the ticket, and
allows other possibilities as well.

The syntax is new, so it doesn't collide with old syntax in the cycle tag.
Named arguments like this are not used often in template tags, but they do
exist in the {% include %} tag (admittedly, there is a reason for it there
that is not present here). Other alternatives which I see as acceptable are to
use one of the keywords used in other tags -- 'on' (used in autoescape) or
'by' (used in regroup) seem appropriate:

{% cycle 'v1' 'v2' v3 by forloop.counter0 %}

This is strictly not backwards-compatible, as it will break templates where a
context-variable named 'by' was used as an argument to the cycle tag, but I
suspect that may be acceptable.

Anyway, my 2 cents,

Shai.
Reply all
Reply to author
Forward
0 new messages