Opened 7 years ago

Last modified 22 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 7 years ago.
First stab at resetcycle tag
5908_resetcycle.2.diff (7.0 KB) - added by akaihola 6 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@… 22 months ago.
Updated patch to current trunk
5908-resetcycle.5.diff (7.6 KB) - added by b.schube@… 22 months ago.
small correction
t5908.diff (7.9 KB) - added by apollo13 22 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 7 years ago by Uninen

  • Cc ville@… added

Changed 7 years ago by Uninen

First stab at resetcycle tag

comment:6 Changed 7 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 7 years ago by anonymous

  • Cc girzel@… added

comment:8 Changed 7 years ago by imbaczek@…

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

comment:9 Changed 7 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 6 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 6 years ago by akaihola

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

comment:12 Changed 6 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 5 years ago by hejsan

  • Cc hr.bjarni+django@… added

comment:14 Changed 5 years ago by vbmendes

  • Cc vbmendes@… added

comment:15 Changed 5 years ago by JosefAssad

  • Cc joe@… added

comment:16 Changed 5 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 4 years ago by gabrielhurley

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

comment:19 Changed 4 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 2 years ago by wim@…

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

comment:22 Changed 2 years ago by isaacsutherland@…

  • Cc isaacsutherland@… added

Changed 22 months ago by b.schube@…

Updated patch to current trunk

Changed 22 months ago by b.schube@…

small correction

comment:23 Changed 22 months ago by apollo13

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

Changed 22 months ago by apollo13

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