Skip to content

Commit d3d8d54

Browse files
sanmaisanmai
authored
Merge pull request #95 from sanmai/fix-93
Fixes #93
2 parents 8b08a46 + 34a240e commit d3d8d54

File tree

7 files changed

+208
-115
lines changed

7 files changed

+208
-115
lines changed

‎CHANGELOG

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
* master
22

3+
* 1.4.6 (2018-02-07)
4+
5+
* TagPaginate shall not pullute the global scope, but work in own scope.
6+
* TagPaginate errors if no collection present instead of vague warning.
7+
38
* 1.4.5 (2017-12-12)
49

510
* Capture tag shall save a variable in the global context.

‎src/Liquid/Tag/TagFor.php

Lines changed: 119 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -103,121 +103,126 @@ public function render(Context $context)
103103
$context->registers['for'] = array();
104104
}
105105

106-
switch ($this->type) {
107-
108-
case 'collection':
109-
110-
$collection = $context->get($this->collectionName);
111-
112-
if ($collection instanceof \Traversable) {
113-
$collection = iterator_to_array($collection);
114-
}
115-
116-
if (is_null($collection) || !is_array($collection) || count($collection) == 0) {
117-
return '';
118-
}
119-
120-
$range = array(0, count($collection));
121-
122-
if (isset($this->attributes['limit']) || isset($this->attributes['offset'])) {
123-
$offset = 0;
124-
125-
if (isset($this->attributes['offset'])) {
126-
$offset = ($this->attributes['offset'] == 'continue') ? $context->registers['for'][$this->name] : $context->get($this->attributes['offset']);
127-
}
128-
129-
$limit = (isset($this->attributes['limit'])) ? $context->get($this->attributes['limit']) : null;
130-
$rangeEnd = $limit ? $limit : count($collection) - $offset;
131-
$range = array($offset, $rangeEnd);
132-
133-
$context->registers['for'][$this->name] = $rangeEnd + $offset;
134-
}
135-
136-
$result = '';
137-
$segment = array_slice($collection, $range[0], $range[1]);
138-
if (!count($segment)) {
139-
return null;
140-
}
141-
142-
$context->push();
143-
$length = count($segment);
144-
145-
$index = 0;
146-
foreach ($segment as $key => $item) {
147-
$value = is_numeric($key) ? $item : array($key, $item);
148-
$context->set($this->variableName, $value);
149-
$context->set('forloop', array(
150-
'name' => $this->name,
151-
'length' => $length,
152-
'index' => $index + 1,
153-
'index0' => $index,
154-
'rindex' => $length - $index,
155-
'rindex0' => $length - $index - 1,
156-
'first' => (int)($index == 0),
157-
'last' => (int)($index == $length - 1)
158-
));
159-
160-
$result .= $this->renderAll($this->nodelist, $context);
161-
162-
$index++;
163-
164-
if (isset($context->registers['break'])) {
165-
unset($context->registers['break']);
166-
break;
167-
}
168-
if (isset($context->registers['continue'])) {
169-
unset($context->registers['continue']);
170-
}
171-
}
172-
173-
break;
174-
175-
case 'digit':
176-
177-
$start = $this->start;
178-
if (!is_integer($this->start)) {
179-
$start = $context->get($this->start);
180-
}
181-
182-
$end = $this->collectionName;
183-
if (!is_integer($this->collectionName)) {
184-
$end = $context->get($this->collectionName);
185-
}
186-
187-
$range = array($start, $end);
188-
189-
$context->push();
190-
$result = '';
191-
$index = 0;
192-
$length = $range[1] - $range[0];
193-
for ($i=$range[0]; $i<=$range[1]; $i++) {
194-
$context->set($this->variableName, $i);
195-
$context->set('forloop', array(
196-
'name' => $this->name,
197-
'length' => $length,
198-
'index' => $index + 1,
199-
'index0' => $index,
200-
'rindex' => $length - $index,
201-
'rindex0' => $length - $index - 1,
202-
'first' => (int)($index == 0),
203-
'last' => (int)($index == $length - 1)
204-
));
205-
206-
$result .= $this->renderAll($this->nodelist, $context);
207-
208-
$index++;
209-
210-
if (isset($context->registers['break'])) {
211-
unset($context->registers['break']);
212-
break;
213-
}
214-
if (isset($context->registers['continue'])) {
215-
unset($context->registers['continue']);
216-
}
217-
}
218-
219-
break;
106+
if ($this->type == 'digit') {
107+
return $this->renderDigit($context);
108+
}
109+
110+
// that's the default
111+
return $this->renderCollection($context);
112+
}
113+
114+
private function renderCollection(Context $context)
115+
{
116+
$collection = $context->get($this->collectionName);
117+
118+
if ($collection instanceof \Traversable) {
119+
$collection = iterator_to_array($collection);
120+
}
121+
122+
if (is_null($collection) || !is_array($collection) || count($collection) == 0) {
123+
return '';
124+
}
125+
126+
$range = array(0, count($collection));
127+
128+
if (isset($this->attributes['limit']) || isset($this->attributes['offset'])) {
129+
$offset = 0;
130+
131+
if (isset($this->attributes['offset'])) {
132+
$offset = ($this->attributes['offset'] == 'continue') ? $context->registers['for'][$this->name] : $context->get($this->attributes['offset']);
133+
}
134+
135+
$limit = (isset($this->attributes['limit'])) ? $context->get($this->attributes['limit']) : null;
136+
$rangeEnd = $limit ? $limit : count($collection) - $offset;
137+
$range = array($offset, $rangeEnd);
138+
139+
$context->registers['for'][$this->name] = $rangeEnd + $offset;
140+
}
141+
142+
$result = '';
143+
$segment = array_slice($collection, $range[0], $range[1]);
144+
if (!count($segment)) {
145+
return null;
146+
}
147+
148+
$context->push();
149+
$length = count($segment);
150+
151+
$index = 0;
152+
foreach ($segment as $key => $item) {
153+
$value = is_numeric($key) ? $item : array($key, $item);
154+
$context->set($this->variableName, $value);
155+
$context->set('forloop', array(
156+
'name' => $this->name,
157+
'length' => $length,
158+
'index' => $index + 1,
159+
'index0' => $index,
160+
'rindex' => $length - $index,
161+
'rindex0' => $length - $index - 1,
162+
'first' => (int)($index == 0),
163+
'last' => (int)($index == $length - 1)
164+
));
165+
166+
$result .= $this->renderAll($this->nodelist, $context);
167+
168+
$index++;
169+
170+
if (isset($context->registers['break'])) {
171+
unset($context->registers['break']);
172+
break;
173+
}
174+
if (isset($context->registers['continue'])) {
175+
unset($context->registers['continue']);
176+
}
177+
}
220178

179+
$context->pop();
180+
181+
return $result;
182+
}
183+
184+
private function renderDigit(Context $context)
185+
{
186+
$start = $this->start;
187+
if (!is_integer($this->start)) {
188+
$start = $context->get($this->start);
189+
}
190+
191+
$end = $this->collectionName;
192+
if (!is_integer($this->collectionName)) {
193+
$end = $context->get($this->collectionName);
194+
}
195+
196+
$range = array($start, $end);
197+
198+
$context->push();
199+
$result = '';
200+
$index = 0;
201+
$length = $range[1] - $range[0];
202+
for ($i = $range[0]; $i <= $range[1]; $i++) {
203+
$context->set($this->variableName, $i);
204+
$context->set('forloop', array(
205+
'name' => $this->name,
206+
'length' => $length,
207+
'index' => $index + 1,
208+
'index0' => $index,
209+
'rindex' => $length - $index,
210+
'rindex0' => $length - $index - 1,
211+
'first' => (int)($index == 0),
212+
'last' => (int)($index == $length - 1)
213+
));
214+
215+
$result .= $this->renderAll($this->nodelist, $context);
216+
217+
$index++;
218+
219+
if (isset($context->registers['break'])) {
220+
unset($context->registers['break']);
221+
break;
222+
}
223+
if (isset($context->registers['continue'])) {
224+
unset($context->registers['continue']);
225+
}
221226
}
222227

223228
$context->pop();

‎src/Liquid/Tag/TagPaginate.php

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Liquid\Context;
1818
use Liquid\FileSystem;
1919
use Liquid\Regexp;
20+
use Liquid\Exception\RenderException;
2021

2122
/**
2223
* The paginate tag works in conjunction with the for tag to split content into numerous pages.
@@ -108,13 +109,23 @@ public function render(Context $context)
108109
$this->currentPage = (is_numeric($context->get('page'))) ? $context->get('page') : 1;
109110
$this->currentOffset = ($this->currentPage - 1) * $this->numberItems;
110111
$this->collection = $context->get($this->collectionName);
112+
111113
if ($this->collection instanceof \Traversable) {
112114
$this->collection = iterator_to_array($this->collection);
113115
}
116+
117+
if (!is_array($this->collection)) {
118+
// TODO do not throw up if error mode allows, see #83
119+
throw new RenderException("Missing collection with name '{$this->collectionName}'");
120+
}
121+
114122
$this->collectionSize = count($this->collection);
115123
$this->totalPages = ceil($this->collectionSize / $this->numberItems);
116124
$paginatedCollection = array_slice($this->collection, $this->currentOffset, $this->numberItems);
117125

126+
// We must work in a new scope so we won't pollute a global scope
127+
$context->push();
128+
118129
// Sets the collection if it's a key of another collection (ie search.results, collection.products, blog.articles)
119130
$segments = explode('.', $this->collectionName);
120131
if (count($segments) == 2) {
@@ -143,7 +154,11 @@ public function render(Context $context)
143154

144155
$context->set('paginate', $paginate);
145156

146-
return parent::render($context);
157+
$result = parent::render($context);
158+
159+
$context->pop();
160+
161+
return $result;
147162
}
148163

149164
/**

‎tests/Liquid/Tag/TagPaginateTest.php

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,4 +43,28 @@ public function testSyntaxErrorCase()
4343
{
4444
$this->assertTemplateResult('', '{% paginate products %}{% endpaginate %}');
4545
}
46+
47+
/**
48+
* @expectedException \Liquid\Exception\RenderException
49+
* @expectedExceptionMessage Missing collection
50+
*/
51+
public function testNoCollection()
52+
{
53+
$this->assertTemplateResult('', '{% paginate products by 1 %}{% for product in products %}{{ product.id }}{% endfor %}{% endpaginate %}');
54+
}
55+
56+
public function testPaginationForRepeatedCalls()
57+
{
58+
$assigns = array(
59+
'articles' => array(array('title' => 1), array('title' => 2), array('title' => 3)),
60+
);
61+
62+
$text = '{% for article in articles %}{{ article.title }},{% endfor %}';
63+
$expected = '1,2,3,';
64+
$this->assertTemplateResult($expected, $text, $assigns);
65+
66+
$text = '{% paginate articles by 2 %}{% for article in articles %}{{ article.title }},{% endfor %}{% endpaginate %} '.$text;
67+
$expected = '1,2, 1,2,3,';
68+
$this->assertTemplateResult($expected, $text, $assigns);
69+
}
4670
}

‎tests/Liquid/fixtures/for.html

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,3 +78,27 @@
7878

7979
5
8080

81+
82+
Regular loop:
83+
84+
1
85+
86+
2
87+
88+
3
89+
90+
91+
Within a pagination:
92+
93+
1
94+
95+
2
96+
97+
98+
Should not be affected by a pagination:
99+
100+
1
101+
102+
2
103+
104+
3

‎tests/Liquid/fixtures/for.liquid

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,18 @@ Range loop:
2424
{% for i in (1..item.quantity) %}
2525
{{ i }}
2626
{% endfor %}
27+
28+
Regular loop:
29+
{% for article in articles %}
30+
{{ article.title }}
31+
{% endfor %}
32+
33+
Within a pagination:
34+
{% paginate articles by 2 %}{% for article in articles %}
35+
{{ article.title }}
36+
{% endfor %}{% endpaginate %}
37+
38+
Should not be affected by a pagination:
39+
{% for article in articles %}
40+
{{ article.title }}
41+
{% endfor %}

‎tests/Liquid/fixtures/for.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,9 @@
1414
'item' => array(
1515
'quantity' => 5,
1616
),
17+
'articles' => array(
18+
array('title' => 1),
19+
array('title' => 2),
20+
array('title' => 3),
21+
),
1722
);

0 commit comments

Comments
 (0)