Code

Opened 7 years ago

Last modified 14 months ago

#5908 new Cleanup/optimization

Cycle tag should reset after it steps out of scope(?)

Reported by: Simon Litchfield <simon@…> Owned by: nobody
Component: Template system Version: master
Severity: Normal Keywords: cycle templatetag tag counter for loop reset
Cc: simon@…, ville@…, girzel@…, imbaczek@…, apacheco.uy@…, hr.bjarni+django@…, vbmendes@…, joe@…, chrischambers, jeffschroeder@…, isaacsutherland@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

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' :-) Thoughts, anyone?

{% for group in grouped %}
  {% for item in group.list %}
   ... {% cycle row1,row2,row3 %} ...
  {% endfor %}
{% endfor %}

Attachments (7)

cycle.py (3.5 KB) - added by Simon Litchfield <simon@…> 7 years ago.
New safe_cycle tag which respects for loop nesting
resetcycle.diff (2.4 KB) - added by Uninen 6 years ago.
First stab at resetcycle tag
5908_resetcycle.2.diff (7.0 KB) - added by akaihola 5 years ago.
Patch: add the {% resetcycle %} tag, with tests and documentation
5908-resetcycle.3.diff (7.3 KB) - added by jamesp 3 years ago.
Updated patch to current trunk; added TemplateSyntaxError tests
5908-resetcycle.4.diff (7.7 KB) - added by b.schube@… 14 months ago.
Updated patch to current trunk
5908-resetcycle.5.diff (7.6 KB) - added by b.schube@… 14 months ago.
small correction
t5908.diff (7.9 KB) - added by apollo13 14 months ago.

Download all attachments as: .zip

Change History (30)

comment:1 Changed 7 years ago by anonymous

  • Cc simon@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 7 years ago by anonymous

  • Triage Stage changed from Unreviewed to Design decision needed

Changed 7 years ago by Simon Litchfield <simon@…>

New safe_cycle tag which respects for loop nesting

comment:3 Changed 7 years ago by anonymous

  • Has patch set

comment:4 Changed 7 years ago by mtredinnick

  • Triage Stage changed from Design decision needed to Accepted

The current behaviour of the cycle tag is very useful, so we don't want to change that. The scope of a cycle tag is everywhere after it is defined for just this reason. Adding another cycle tag isn't a neat solution, either.

The solution here is to add a "resetcycle" tag. Given no arguments, it reset the most recent looped cycle tag. Given an argument, it resets the cycle tag with that name.

comment:5 Changed 6 years ago by Uninen

  • Cc ville@… added

Changed 6 years ago by Uninen

First stab at resetcycle tag

comment:6 Changed 6 years ago by Uninen

I made a patch based on Malcolms last comment. This works for me but I'm not quite sure if it is fully valid solution. If someone could take a look and comment this, it would be appreciated.

comment:7 Changed 6 years ago by anonymous

  • Cc girzel@… added

comment:8 Changed 6 years ago by imbaczek@…

I've just ran into this very problem, this needs a solution before 1.0 IMHO.

comment:9 Changed 6 years ago by anonymous

  • Cc imbaczek@… added

comment:10 Changed 6 years ago by anibal

  • Cc apacheco.uy@… added

Yes, I think this will be useful too. Any news?

comment:11 Changed 5 years ago by akaihola

Replying to Uninen:

I made a patch based on Malcolms last comment. This works for me but I'm not quite sure if it is fully valid solution. If someone could take a look and comment this, it would be appreciated.

I see a couple of problems (looking at the code, didn't actually test it):

  • reserves the {{ resetcycle }} context variable; any templates which happen to use it will be broken
  • can't reset multiple simultaneous cycles (will forget previous reset)
  • no tests
  • no documentation

I'll try to think about this a bit.

Changed 5 years ago by akaihola

Patch: add the {% resetcycle %} tag, with tests and documentation

comment:12 Changed 5 years ago by akaihola

The above patch adds a {% resetcycle %} tag which does not pollute the context. It should be able to reset the last unnamed and multiple named {% cycle %} tags.

The patch also lives in a GitHub branch.

comment:13 Changed 4 years ago by hejsan

  • Cc hr.bjarni+django@… added

comment:14 Changed 4 years ago by vbmendes

  • Cc vbmendes@… added

comment:15 Changed 4 years ago by JosefAssad

  • Cc joe@… added

comment:16 Changed 4 years ago by SmileyChris

  • Patch needs improvement set

This needs to be brought up to speed with the thread-safe changes to template tags (cycle uses the RenderContext now).

comment:17 Changed 4 years ago by chrischambers

  • Cc chrischambers added

comment:18 Changed 3 years ago by gabrielhurley

  • Severity set to Normal
  • Type set to Cleanup/optimization

comment:19 Changed 3 years ago by SEJeff

  • Cc jeffschroeder@… added
  • Easy pickings unset

Changed 3 years ago by jamesp

Updated patch to current trunk; added TemplateSyntaxError tests

comment:20 Changed 3 years ago by jamesp

  • UI/UX unset

Updated the patch to the current trunk but I am wondering if {% resetcycle %} shouldn't reset the most recent cycle if a name isn't provided.
This would be consistent with {% endblock %} and may lead to confusion if not implemented similarly.

comment:21 Changed 18 months ago by wim@…

James, what do you think of changing the name to endcycle, like endblock and endcomment ?

comment:22 Changed 17 months ago by isaacsutherland@…

  • Cc isaacsutherland@… added

Changed 14 months ago by b.schube@…

Updated patch to current trunk

Changed 14 months ago by b.schube@…

small correction

comment:23 Changed 14 months ago by apollo13

Changed the cycle tag to always reset the "last" cycle instead of the "last unnamed".

Changed 14 months ago by apollo13

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as new
The owner will be changed from nobody to anonymous. Next status will be 'assigned'
as The resolution will be set. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.