{% cycle %} no longer works within inclusion tags and included templates

[awmcclain]

Cycle only works within the template that contains the for loop -- if you include a subtemplate or inclusion tag, cycle doesn't work.

This is a regression from 1.1.1.

def test(request):
    return render_to_response('test.html', {'bar':range(4)})

----- test.html
{% for i in bar %}
{% cycle 'alpha' 'bravo' %} Tag:{% foo %} Include:{% include 'foo.html' %}
{% endfor %}
--------

@register.inclusion_tag('foo.html', takes_context=True)
def foo(context):
    return context

-------- foo.html
{% cycle 'alpha' 'bravo' %}

------------
output:
alpha Tag:alpha Include:alpha
bravo Tag:alpha Include:alpha 
alpha Tag:alpha Include:alpha 
bravo Tag:alpha Include:alpha 

[Alex Gaynor]

Honestly, the previous behavior sounds like a bug.

[Russell Keith-Magee]

Putting on the 1.2 milestone; Alex's comment may be right, but given that this is potentially a regression, it needs to be resolved before 1.2 gets to release candidate.

[awmcclain]

Replying to Alex:

Honestly, the previous behavior sounds like a bug.

I disagree. Subtemplates within a {% for %} tag still have access to the forloop template variables (forloop.counter, for example), so it doesn't make sense that they wouldn't be able to use {% cycle %}. They're still within the for loop.

[Russell Keith-Magee]

@awmcclain: After discussion with Jannis and Jacob on IRC, we've come to the conclusion that Alex is correct in saying that previous behavior is a bug.

When you use {% cycle %} in the way you describe, you're not setting a context variable. The state of the cycle tag isn't held in context, it's part of the state of the cycle tag itself. By cycling, you're updating the state of a specific node in the template tree. The only time that state reaches context is if you name the cycle (using {% cycle ... as foo %}), and that only gives you the ability to read, not modify the cycle contents.

The use case you demonstrate depends on two bugs in the v1.1 template engine:

1) The include tag didn't establish a clean environment for each template load (and each include should be a clean template load)
2) The cycle tag in v1.1 wasn't threadsafe, so multiple template loaders could access the same cycle states.

These bugs have both been corrected as part of the cleanup required to allow for cached template loaders.

Another way of looking at the point (1): Although the end user documentation isn't clear on this point, the implementation (both current and historical) makes it clear that {% include %} should be interpreted as "Render this entire template and insert the HTML", not as "insert the node hierarchy parsed from this subtemplate". Since each included template should be an independent rendering, it doesn't make sense that cycle states should leak between renderings.

One way to restore the "old" behaviour (depending on exactly how you're using cycle) relies on using the part of cycle that *is* pushed into context. If you define the cycle tag in test.html as:

{% for i in bar %}
{% cycle 'alpha' 'bravo' as testvalue %} Tag:{% foo %} Include:{% include 'foo.html' %}
{% endfor %}

and change your foo.html to read:

{{ testvalue }}

you will get the output you expected to see.

I'll formally accept this on the basis that extra documentation is required. In particular:

• Extra notes for the backwards compatibility guide (since it isn't immediately obvious why this is a bug, not a regression). We already have a section on tags that might be affected by the rendering changes; this is an excellent example of how those changes could affect people in practice.
• A note to the {% include %} clarifying the intention of that tag.

[Russell Keith-Magee]

(In [13063]) Fixed #13444 -- Improved the documentation around the backwards compatibility quirks of the cycle and include tags. Thanks to awmcclain for the report.

[Jacob]

Milestone 1.2 deleted