From: Fabien Potencier <fabien.potencier@gmail.com>
Date: Mon, 7 Jun 2010 09:04:23 +0000 (+0200)
Subject: fixed the for tag for large arrays (closes #64 - based on an idea from gwilym - aeb56... 
X-Git-Url: http://git.silmor.de/gitweb/?a=commitdiff_plain;h=d47902c7b9d99f0ab17a970eb1eb71656d4c2672;p=web%2Fkonrad%2Ftwig.git

fixed the for tag for large arrays (closes #64 - based on an idea from gwilym - aeb56f9f966bd23b77d35c87999be0606dbb3f47)
---

diff --git a/CHANGELOG b/CHANGELOG
index 033521c..540dc84 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -5,6 +5,7 @@ Backward incompatibilities:
  * removed the sandboxed attribute of the include tag (use the new sandbox tag instead)
  * refactored the Node system (if you have custom nodes, you will have to update them to use the new API)
 
+ * fixed the for tag for large arrays (some loop variables are now only available for arrays and objects that implement the Countable interface)
  * removed the Twig_Resource::resolveMissingFilter() method
  * fixed the filter tag which did not apply filtering to included files
  * added a bunch of unit tests
diff --git a/doc/02-Twig-for-Template-Designers.markdown b/doc/02-Twig-for-Template-Designers.markdown
index 5031133..f56506b 100644
--- a/doc/02-Twig-for-Template-Designers.markdown
+++ b/doc/02-Twig-for-Template-Designers.markdown
@@ -467,6 +467,13 @@ for a small speed boost):
 | `loop.parent`         | The parent context
 
 >**NOTE**
+>The `loop.length`, `loop.revindex`, `loop.revindex0`, and `loop.last`
+>variables are only available for PHP arrays, or objects that implement the
+>`Countable` interface (as of Twig 0.9.7).
+
+-
+
+>**NOTE**
 >Unlike in PHP it's not possible to `break` or `continue` in a loop.
 
 If no iteration took place because the sequence was empty, you can render a
diff --git a/lib/Twig/Extension/Core.php b/lib/Twig/Extension/Core.php
index 9564ee2..64a6189 100644
--- a/lib/Twig/Extension/Core.php
+++ b/lib/Twig/Extension/Core.php
@@ -293,7 +293,7 @@ function twig_iterator_to_array($seq, $useKeys = true)
     if (is_array($seq)) {
         return $seq;
     } elseif (is_object($seq) && $seq instanceof Traversable) {
-        return $seq instanceof Countable ? $seq : iterator_to_array($seq, $useKeys);
+        return $seq;
     } else {
         return array();
     }
diff --git a/lib/Twig/Node/For.php b/lib/Twig/Node/For.php
index ace076f..e557653 100644
--- a/lib/Twig/Node/For.php
+++ b/lib/Twig/Node/For.php
@@ -39,23 +39,28 @@ class Twig_Node_For extends Twig_Node
         $compiler
             ->write("\$context['_seq'] = twig_iterator_to_array(")
             ->subcompile($this->seq)
-            ->raw(", ".(null !== $this->key_target ? 'true' : 'false').");\n")
+            ->raw(");\n")
         ;
 
         if ($this['with_loop']) {
             $compiler
-                ->write("\$length = count(\$context['_seq']);\n")
+                ->write("\$countable = is_array(\$context['_seq']) || (is_object(\$context['_seq']) && \$context['_seq'] instanceof Countable);\n")
+                ->write("\$length = \$countable ? count(\$context['_seq']) : null;\n")
 
                 ->write("\$context['loop'] = array(\n")
-                ->write("  'parent'    => \$context['_parent'],\n")
-                ->write("  'length'    => \$length,\n")
-                ->write("  'index0'    => 0,\n")
-                ->write("  'index'     => 1,\n")
-                ->write("  'revindex0' => \$length - 1,\n")
-                ->write("  'revindex'  => \$length,\n")
-                ->write("  'first'     => true,\n")
-                ->write("  'last'      => 1 === \$length,\n")
+                ->write("  'parent' => \$context['_parent'],\n")
+                ->write("  'index0' => 0,\n")
+                ->write("  'index'  => 1,\n")
+                ->write("  'first'  => true,\n")
                 ->write(");\n")
+                ->write("if (\$countable) {\n")
+                ->indent()
+                ->write("\$context['loop']['revindex0'] = \$length - 1;\n")
+                ->write("\$context['loop']['revindex'] = \$length;\n")
+                ->write("\$context['loop']['length'] = \$length;\n")
+                ->write("\$context['loop']['last'] = 1 === \$length;\n")
+                ->outdent()
+                ->write("}\n")
             ;
         }
 
@@ -78,10 +83,14 @@ class Twig_Node_For extends Twig_Node
             $compiler
                 ->write("++\$context['loop']['index0'];\n")
                 ->write("++\$context['loop']['index'];\n")
+                ->write("\$context['loop']['first'] = false;\n")
+                ->write("if (\$countable) {\n")
+                ->indent()
                 ->write("--\$context['loop']['revindex0'];\n")
                 ->write("--\$context['loop']['revindex'];\n")
-                ->write("\$context['loop']['first'] = false;\n")
                 ->write("\$context['loop']['last'] = 0 === \$context['loop']['revindex0'];\n")
+                ->outdent()
+                ->write("}\n")
             ;
         }
 
diff --git a/test/Twig/Tests/Node/ForTest.php b/test/Twig/Tests/Node/ForTest.php
index 9517972..f55cd74 100644
--- a/test/Twig/Tests/Node/ForTest.php
+++ b/test/Twig/Tests/Node/ForTest.php
@@ -62,7 +62,7 @@ class Twig_Tests_Node_ForTest extends Twig_Tests_Node_TestCase
 
         $tests[] = array($node, <<<EOF
 \$context['_parent'] = (array) \$context;
-\$context['_seq'] = twig_iterator_to_array(\$this->getContext(\$context, 'items'), true);
+\$context['_seq'] = twig_iterator_to_array(\$this->getContext(\$context, 'items'));
 foreach (\$context['_seq'] as \$context['key'] => \$context['item']) {
     echo \$this->getContext(\$context, 'foo');
 }
@@ -82,26 +82,31 @@ EOF
 
         $tests[] = array($node, <<<EOF
 \$context['_parent'] = (array) \$context;
-\$context['_seq'] = twig_iterator_to_array(\$this->getContext(\$context, 'values'), true);
-\$length = count(\$context['_seq']);
+\$context['_seq'] = twig_iterator_to_array(\$this->getContext(\$context, 'values'));
+\$countable = is_array(\$context['_seq']) || (is_object(\$context['_seq']) && \$context['_seq'] instanceof Countable);
+\$length = \$countable ? count(\$context['_seq']) : null;
 \$context['loop'] = array(
-  'parent'    => \$context['_parent'],
-  'length'    => \$length,
-  'index0'    => 0,
-  'index'     => 1,
-  'revindex0' => \$length - 1,
-  'revindex'  => \$length,
-  'first'     => true,
-  'last'      => 1 === \$length,
+  'parent' => \$context['_parent'],
+  'index0' => 0,
+  'index'  => 1,
+  'first'  => true,
 );
+if (\$countable) {
+    \$context['loop']['revindex0'] = \$length - 1;
+    \$context['loop']['revindex'] = \$length;
+    \$context['loop']['length'] = \$length;
+    \$context['loop']['last'] = 1 === \$length;
+}
 foreach (\$context['_seq'] as \$context['k'] => \$context['v']) {
     echo \$this->getContext(\$context, 'foo');
     ++\$context['loop']['index0'];
     ++\$context['loop']['index'];
-    --\$context['loop']['revindex0'];
-    --\$context['loop']['revindex'];
     \$context['loop']['first'] = false;
-    \$context['loop']['last'] = 0 === \$context['loop']['revindex0'];
+    if (\$countable) {
+        --\$context['loop']['revindex0'];
+        --\$context['loop']['revindex'];
+        \$context['loop']['last'] = 0 === \$context['loop']['revindex0'];
+    }
 }
 \$_parent = \$context['_parent'];
 unset(\$context['_seq'], \$context['_iterated'], \$context['k'], \$context['v'], \$context['_parent'], \$context['loop']);
@@ -120,27 +125,32 @@ EOF
         $tests[] = array($node, <<<EOF
 \$context['_parent'] = (array) \$context;
 \$context['_iterated'] = false;
-\$context['_seq'] = twig_iterator_to_array(\$this->getContext(\$context, 'values'), true);
-\$length = count(\$context['_seq']);
+\$context['_seq'] = twig_iterator_to_array(\$this->getContext(\$context, 'values'));
+\$countable = is_array(\$context['_seq']) || (is_object(\$context['_seq']) && \$context['_seq'] instanceof Countable);
+\$length = \$countable ? count(\$context['_seq']) : null;
 \$context['loop'] = array(
-  'parent'    => \$context['_parent'],
-  'length'    => \$length,
-  'index0'    => 0,
-  'index'     => 1,
-  'revindex0' => \$length - 1,
-  'revindex'  => \$length,
-  'first'     => true,
-  'last'      => 1 === \$length,
+  'parent' => \$context['_parent'],
+  'index0' => 0,
+  'index'  => 1,
+  'first'  => true,
 );
+if (\$countable) {
+    \$context['loop']['revindex0'] = \$length - 1;
+    \$context['loop']['revindex'] = \$length;
+    \$context['loop']['length'] = \$length;
+    \$context['loop']['last'] = 1 === \$length;
+}
 foreach (\$context['_seq'] as \$context['k'] => \$context['v']) {
     \$context['_iterated'] = true;
     echo \$this->getContext(\$context, 'foo');
     ++\$context['loop']['index0'];
     ++\$context['loop']['index'];
-    --\$context['loop']['revindex0'];
-    --\$context['loop']['revindex'];
     \$context['loop']['first'] = false;
-    \$context['loop']['last'] = 0 === \$context['loop']['revindex0'];
+    if (\$countable) {
+        --\$context['loop']['revindex0'];
+        --\$context['loop']['revindex'];
+        \$context['loop']['last'] = 0 === \$context['loop']['revindex0'];
+    }
 }
 if (!\$context['_iterated']) {
     echo \$this->getContext(\$context, 'foo');
diff --git a/test/fixtures/tags/for/objects.test b/test/fixtures/tags/for/objects.test
index d444a21..5034437 100644
--- a/test/fixtures/tags/for/objects.test
+++ b/test/fixtures/tags/for/objects.test
@@ -4,8 +4,7 @@
 {% for item in items %}
   * {{ item }}
   * {{ loop.index }}/{{ loop.index0 }}
-  * {{ loop.revindex }}/{{ loop.revindex0 }}
-  * {{ loop.first }}/{{ loop.last }}/{{ loop.length }}
+  * {{ loop.first }}
 
 {% endfor %}
 
@@ -30,13 +29,11 @@ return array('items' => new ItemsIterator())
 --EXPECT--
   * bar
   * 1/0
-  * 2/1
-  * 1//2
+  * 1
 
   * foo
   * 2/1
-  * 1/0
-  * /1/2
+  * 
 
 
   * foo/bar
diff --git a/test/fixtures/tags/for/objects_countable.test b/test/fixtures/tags/for/objects_countable.test
index 6ff17d4..4a1ff61 100644
--- a/test/fixtures/tags/for/objects_countable.test
+++ b/test/fixtures/tags/for/objects_countable.test
@@ -3,6 +3,10 @@
 --TEMPLATE--
 {% for item in items %}
   * {{ item }}
+  * {{ loop.index }}/{{ loop.index0 }}
+  * {{ loop.revindex }}/{{ loop.revindex0 }}
+  * {{ loop.first }}/{{ loop.last }}/{{ loop.length }}
+
 {% endfor %}
 
 {% for key, value in items %}
@@ -26,7 +30,15 @@ class ItemsIteratorCountable implements Iterator, Countable
 return array('items' => new ItemsIteratorCountable())
 --EXPECT--
   * bar
+  * 1/0
+  * 2/1
+  * 1//2
+
   * foo
+  * 2/1
+  * 1/0
+  * /1/2
+
 
   * foo/bar
   * bar/foo