Problem
It appears that in cases where there are nested for
loops in Twig, the inner loop does not increment properly.
So for something like:
{% for foo in foos %}
{% for bar in foo.bars %}
{{ bar }}
{% endfor %}
{% endfor %}
The variable bar
will not increment properly.
Workarounds
Use the index to reference the inner values
{% for i,foo in foos %}
{% for j,bar in foo.bars %}
{{ foo.bars[j] }}
{% endfor %}
{% endfor %}
or
{% for i in foos|keys %}
{% for j in foos[i].bars|keys %}
{{ foos[i].bars[j] }}
{% endfor %}
{% endfor %}
Original summary by joelpittet
Note: Make sure that twig caching is off to see this issue
Problem:
{# Columns #}
{% if colgroups %}
{% for colgroup in colgroups %}
{% if colgroup.cols %}
<colgroup{{ colgroup.attributes }}>
{% for col in colgroup.cols %}
<col{{ col.attributes }}/> {# PROBLEM HERE #}
{% endfor %}
</colgroup>
{% else %}
<colgroup{{ colgroup.attributes }}/>
{% endif %}
{% endfor %}
{% endif %}
The first pass through the nested for .. in
loop the attributes will print but in the subsequent loops they are marked as printed() and will not print out.
Temporary Fixes Found:
{% for i,col in colgroup.cols %}
<col{{ colgroup.cols[i].attributes }}/>
{% endfor %}
or
{% for col in colgroup.cols %}
{% set col_attributes = col.attributes %}
<col{{ col_attributes }}/>
{% endfor %}
or less ideal but for testing:
Comment out if (!$value->printed())
from Attribute.php
Back Reference:
http://drupal.org/node/1778968#comment-6820122
Examples Tables:
colgroup2 shows the issue the best
https://github.com/joelpittet/twig-tables
Comment | File | Size | Author |
---|---|---|---|
#28 | interdiff.txt | 1002 bytes | joelpittet |
#28 | 1867090-28-twigreference-nested-for-loop.patch | 859 bytes | joelpittet |
#26 | 1867090-26-twigreference-nested-for-loop.patch | 834 bytes | joelpittet |
#26 | interdiff.txt | 925 bytes | joelpittet |
#25 | 1867090-25-twigreference-nested-for-loop.patch | 711 bytes | joelpittet |
Comments
Comment #1
c4rl CreditAttribution: c4rl commentedI also noticed this issue when working on theme_table.
Will update summary to list workarounds.
Comment #1.0
c4rl CreditAttribution: c4rl commentedcleanup spacing
Comment #2
joelpittetSteveoliver saw this too. Do we know anybody at SensioLabs maybe they could have a peek?
Comment #3
jwilson3This should really be reported to Twig's issue queue.
Comment #4
Fabianx CreditAttribution: Fabianx commentedAre we able to reproduce this with pure twig? It might be TwigReference object related.
Comment #5
bzitzow CreditAttribution: bzitzow commentedComment #6
bzitzow CreditAttribution: bzitzow commentedTested nested twig loop in drupal implementation (and outside drupal implementation) and found it is working as expected. Maybe it is resolved? Maybe the issue could be more explicitly illustrated?
Using the following data, I discovered the nested loop is incrementing correctly.
Data:
Twig:
Result:
Comment #7
bzitzow CreditAttribution: bzitzow commentedComment #8
c4rl CreditAttribution: c4rl commentedI noticed this in another theme function, I believe as part of #1898454: system.module - Convert PHPTemplate templates to Twig (though against core and not the sandbox). I'll need some time to dig into this, but I think it is still happening.
Comment #9
bzitzow CreditAttribution: bzitzow commentedI am still interested in looking into it if you can duplicate it.
Comment #10
c4rl CreditAttribution: c4rl commentedThis is still happening both in core and in the front-end branch. Some variables seem to work, some do not, even outside of a nested context. Will attempt to dig into this more later.
Comment #11
joelpittetMoving this to core, ran into it again in.
#1898466: update.module - Convert theme_ functions to Twig
Still exists in:
#1939008: Convert theme_table() to Twig
Comment #12
joelpittetSeems not to be an issue with twig but something in drupal is messing up the context...
I took down a stock version of Twig and the context issue doesn't seem to be there...
If you want to try it out (grabbed twig with composer): http://twig.sensiolabs.org/doc/intro.html#installation
PHP:
Twig:
Comment #13
joelpittetSo as a second test, I've used the update_report twig file and dumped the array and twig file from #12 into a preprocessor and this is what I got.
Original twig file from #12:
File from update_report.html.twig (with the project_title hack back to project.title)
Comment #14
joelpittetI'm going to assume this is related for now but the loop indexing is all wonky.
Stock Twig:
Drupal Twig:
Comment #15
joelpittetHere is an example that doesn't pass variables in, just straight twig same results as #14. (I just took a preprocess and gutted the contents)
Comment #16
joelpittetI did a bit more research to try and track down where this is coming from and have some hunches to what's up here.
TwigReference seems to be main culprit. And ArrayObject's exchangeArray may have something to do with the index number issue, though I could not get the nested variables to show up correctly. One thought is to replace ArrayObject with
implements \ArrayAccess, \IteratorAggregate, \Countable
and do all that stuff ourselves. Thoughts being it may not be doing things the way you want due to notes I found here/vendor/symfony/validator/Symfony/Component/Validator/Tests/Constraints/CollectionValidatorCustomArrayObjectTest.php
Comment #17
Fabianx CreditAttribution: Fabianx commentedHere is a patch:
Please test.
This solution is:
a) it keeps being fast (isset() is very very fast)
b) hide, show is not supported in loops, but that should be acceptable trade-off for now.
So this patch should have zero overhead to what is in core currently.
Comment #18
joelpittetWorked great with the simple test! I am going to test it later tonight on theme_table and theme_update conversions to see how it fairs for those two suckers, then add it in here.
Comment #19
joelpittetHere is a patch file from #17
Thanks @Fabianx.
Comment #20
joelpittetComment #22
joelpittetI must not be awake yet, sorry. Here's the patch.
Comment #23
joelpittetWith some testing I found that the key is rarely 'loop' in the wild, so this patch didn't hold water. I am thinking we turn off the reference caching for now. Patch attached.
Comment #24
joelpittet#22 but with '_seq' instead of loop from @Fabianx on IRC.
Comment #25
joelpittetGoing too fast... argh.
Comment #26
joelpittetOk, so I tested this with update_report #1898466-40: update.module - Convert theme_ functions to Twig as well as the simple test in #15
This fixes the indexing issues for the looped items as well as the items in each iteration using the first TwigReference/Array in the loop.
Attached is just document updates to hopefully make it more clear what this is doing.
Thank you very, very much @Fabianx!
Comment #27
joelpittettagging.
Comment #28
joelpittetDoc tweak again.
Comment #29
Fabianx CreditAttribution: Fabianx commentedLooks good to me.
I can verify that _seq key is used only internally by Twig to store information about the loop it is in. It is also unset() as soon as loop context is left.
Please lets get this in as this unblocks some conversions.
=> RTBC
Comment #30
webchickComment needs some clean-up...
Reduced to the following and fixed an it's/its:
...aaaaand committed and pushed to 8.x. Thanks! :)
Comment #31
joelpittetThank you, Thank you @webchick and @Fabianx!
Comment #32.0
(not verified) CreditAttribution: commentedRevised summary