Skip to content

Commit 610c36d

Browse files
osteeleclaude
osteele
and
claude
authored
Compare loop semantics with Ruby implementation (#117)
* docs: investigate loop modifier semantics (issue #6) This commit addresses issue #6 by investigating how the loop modifiers (reversed, limit, offset) interact when used together. Key findings: - Go implementation: modifiers are always applied in fixed order (reversed → offset → limit) regardless of syntax order - Ruby/Shopify Liquid: syntax order matters, and reversed only works when placed before named parameters (limit/offset) Changes: - Add LOOP_SEMANTICS_INVESTIGATION.md with detailed analysis - Add test_ruby_liquid.rb for Ruby reference implementation testing - Update iteration_tags_test.go with tests for combined modifiers - Document the behavior differences from Ruby implementation The Go implementation provides more consistent and predictable behavior by using a fixed application order, though this differs from the Ruby reference implementation. * docs: move loop semantics investigation to docs/loop-semantics.md * refactor: move test_ruby_liquid.rb to scripts directory - Move test_ruby_liquid.rb to scripts/ to keep it with other utility scripts - Make script executable - Update docs/loop-semantics.md to reference the new location --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4237459 commit 610c36d

3 files changed

Lines changed: 266 additions & 4 deletions

File tree

‎docs/loop-semantics.md‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
# Loop Modifier Semantics Investigation
2+
3+
**Issue:** https://github.com/osteele/liquid/issues/6
4+
5+
**Question:** Do the loop modifiers `reversed`, `limit`, and `offset` depend on the order they're specified in the template?
6+
7+
## Summary of Findings
8+
9+
**YES**, the order matters in Ruby/Shopify Liquid, but **NO**, it doesn't matter in the Go implementation.
10+
11+
### Critical Difference
12+
13+
- **Ruby/Shopify Liquid (v5.10.0):**
14+
- Syntax order DOES matter
15+
- `reversed` keyword only works when placed BEFORE named parameters (`limit:` and `offset:`)
16+
- When `reversed` comes after named parameters, it is ignored
17+
18+
- **Go osteele/liquid Implementation:**
19+
- Syntax order does NOT matter
20+
- Modifiers are always applied in fixed order: `reversed` → `offset` → `limit`
21+
- All modifiers work regardless of their position in the syntax
22+
23+
## Detailed Test Results
24+
25+
Test array: `[1, 2, 3, 4, 5]`
26+
27+
| Test Case | Template | Ruby Result | Go Result | Match? |
28+
|-----------|----------|-------------|-----------|--------|
29+
| reversed only | `reversed` | `54321` | `54321` | ✅ |
30+
| limit only | `limit:2` | `12` | `12` | ✅ |
31+
| offset only | `offset:2` | `345` | `345` | ✅ |
32+
| **reversed + limit** | `reversed limit:2` | `21` | `54` | ❌ |
33+
| **limit + reversed** | `limit:2 reversed` | `12` | `54` | ❌ |
34+
| limit + offset (order 1) | `limit:2 offset:1` | `23` | `23` | ✅ |
35+
| offset + limit (order 2) | `offset:1 limit:2` | `23` | `23` | ✅ |
36+
| **all three (order 1)** | `reversed limit:2 offset:1` | `32` | `43` | ❌ |
37+
| **all three (order 2)** | `reversed offset:1 limit:2` | `32` | `43` | ❌ |
38+
| **all three (order 3)** | `limit:2 offset:1 reversed` | `23` | `43` | ❌ |
39+
| **all three (order 4)** | `offset:1 limit:2 reversed` | `23` | `43` | ❌ |
40+
41+
## Analysis
42+
43+
### Ruby/Shopify Liquid Behavior
44+
45+
The Ruby implementation appears to:
46+
47+
1. Parse `reversed` as a boolean flag (only when it appears before named parameters)
48+
2. Parse `limit:N` and `offset:N` as named parameters
49+
3. Apply in the order: **offset → limit → reversed**
50+
51+
**Critical Finding:** The `reversed` keyword is ONLY recognized when it appears BEFORE the named parameters:
52+
- ✅ `{% for item in array reversed limit:2 %}` - reversed works
53+
- ❌ `{% for item in array limit:2 reversed %}` - reversed is IGNORED
54+
55+
This explains the different results:
56+
- `reversed limit:2` on [1,2,3,4,5]:
57+
1. offset=0, limit=2: extract [1,2]
58+
2. reversed=true: reverse to [2,1]
59+
3. Result: `21`
60+
61+
- `limit:2 reversed` on [1,2,3,4,5]:
62+
1. offset=0, limit=2: extract [1,2]
63+
2. reversed=false (keyword not recognized): no reversal
64+
3. Result: `12`
65+
66+
### Go osteele/liquid Behavior
67+
68+
The Go implementation (in `tags/iteration_tags.go:225-263`):
69+
70+
```go
71+
func applyLoopModifiers(loop expressions.Loop, ctx render.Context, iter iterable) (iterable, error) {
72+
if loop.Reversed {
73+
iter = reverseWrapper{iter}
74+
}
75+
76+
if loop.Offset != nil {
77+
// ... apply offset
78+
iter = offsetWrapper{iter, offset}
79+
}
80+
81+
if loop.Limit != nil {
82+
// ... apply limit
83+
iter = limitWrapper{iter, limit}
84+
}
85+
86+
return iter, nil
87+
}
88+
```
89+
90+
This code:
91+
1. Accepts `reversed` in any position (it's just a boolean field)
92+
2. Always applies in the order: **reversed → offset → limit**
93+
3. This gives consistent behavior regardless of syntax order
94+
95+
Example: `reversed limit:2` on [1,2,3,4,5]:
96+
1. reversed=true: reverse to [5,4,3,2,1]
97+
2. offset=0: still [5,4,3,2,1]
98+
3. limit=2: extract [5,4]
99+
4. Result: `54`
100+
101+
## Implications
102+
103+
### Compatibility Issue
104+
105+
The Go implementation is **NOT compatible** with Ruby/Shopify Liquid when:
106+
1. `reversed` is used with `limit` or `offset`
107+
2. The syntax order varies
108+
109+
### Which Behavior is "Correct"?
110+
111+
Both implementations have merit:
112+
113+
**Ruby's approach (syntax-order-dependent):**
114+
- Pros: More flexible - different orders produce different results
115+
- Cons:
116+
- Confusing that `reversed` only works in one position
117+
- Not intuitive for users
118+
- Violates principle of least surprise
119+
120+
**Go's approach (fixed application order):**
121+
- Pros:
122+
- Consistent regardless of syntax order
123+
- More predictable
124+
- Easier to understand and document
125+
- Cons:
126+
- Different from Ruby reference implementation
127+
- Only one semantic meaning possible
128+
129+
### Historical Note
130+
131+
PR #456 (https://github.com/Shopify/liquid/pull/456) claimed to fix `reversed limit:2` to produce the "correct" result by applying `reversed` before `limit`. However, based on testing Liquid v5.10.0, the current Ruby behavior actually applies:
132+
- offset first
133+
- limit second
134+
- reversed last
135+
136+
This suggests either:
137+
1. PR #456 was never merged, or
138+
2. It was merged but implemented differently than described, or
139+
3. There was a regression
140+
141+
## Recommendations
142+
143+
1. **Document the current Go behavior clearly** - users should know that modifier order in the template doesn't matter
144+
2. **Decide on compatibility goal:**
145+
- Option A: Match Ruby exactly (including the quirk that `reversed` only works when placed first)
146+
- Option B: Keep current behavior and document as a known difference
147+
- Option C: Propose a fix to Ruby/Shopify Liquid to adopt the more logical Go approach
148+
149+
3. **Add test cases** to prevent regression and document expected behavior
150+
151+
## Test Code
152+
153+
The investigation included:
154+
- `tags/iteration_tags_test.go` - Go implementation tests (added test cases for combined modifiers)
155+
- `scripts/test_ruby_liquid.rb` - Ruby reference implementation test script
156+
157+
The Ruby script can be run to verify the behavior of the Shopify Ruby Liquid implementation.

‎scripts/test_ruby_liquid.rb‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
#!/usr/bin/env ruby
2+
# Test script to verify Shopify Ruby Liquid implementation behavior
3+
# for loop modifiers: reversed, limit, and offset
4+
5+
require 'liquid'
6+
7+
array = [1, 2, 3, 4, 5]
8+
9+
tests = [
10+
{
11+
name: "reversed only",
12+
template: "{% for item in array reversed %}{{ item }}{% endfor %}",
13+
notes: "Should reverse the entire array"
14+
},
15+
{
16+
name: "limit only",
17+
template: "{% for item in array limit:2 %}{{ item }}{% endfor %}",
18+
notes: "Should take first 2 items"
19+
},
20+
{
21+
name: "offset only",
22+
template: "{% for item in array offset:2 %}{{ item }}{% endfor %}",
23+
notes: "Should skip first 2 items"
24+
},
25+
{
26+
name: "reversed then limit (syntax order)",
27+
template: "{% for item in array reversed limit:2 %}{{ item }}{% endfor %}",
28+
notes: "According to PR#456, should give [5,4] - reverse first, then take first 2"
29+
},
30+
{
31+
name: "limit then reversed (syntax order)",
32+
template: "{% for item in array limit:2 reversed %}{{ item }}{% endfor %}",
33+
notes: "Does syntax order matter? Or is application order fixed?"
34+
},
35+
{
36+
name: "limit then offset (syntax order)",
37+
template: "{% for item in array limit:2 offset:1 %}{{ item }}{% endfor %}",
38+
notes: "Should offset first, then limit: skip 1, take 2 = [2,3]"
39+
},
40+
{
41+
name: "offset then limit (syntax order)",
42+
template: "{% for item in array offset:1 limit:2 %}{{ item }}{% endfor %}",
43+
notes: "Should offset first, then limit: skip 1, take 2 = [2,3]"
44+
},
45+
{
46+
name: "reversed limit offset (syntax order 1)",
47+
template: "{% for item in array reversed limit:2 offset:1 %}{{ item }}{% endfor %}",
48+
notes: "How do all three combine?"
49+
},
50+
{
51+
name: "reversed offset limit (syntax order 2)",
52+
template: "{% for item in array reversed offset:1 limit:2 %}{{ item }}{% endfor %}",
53+
notes: "Does changing syntax order change result?"
54+
},
55+
{
56+
name: "limit offset reversed (syntax order 3)",
57+
template: "{% for item in array limit:2 offset:1 reversed %}{{ item }}{% endfor %}",
58+
notes: "Does reversed at end change things?"
59+
},
60+
{
61+
name: "offset limit reversed (syntax order 4)",
62+
template: "{% for item in array offset:1 limit:2 reversed %}{{ item }}{% endfor %}",
63+
notes: "Another permutation"
64+
},
65+
{
66+
name: "offset beyond length",
67+
template: "{% for item in array offset:10 %}{{ item }}{% endfor %}",
68+
notes: "Should produce empty result"
69+
},
70+
{
71+
name: "limit 0",
72+
template: "{% for item in array limit:0 %}{{ item }}{% endfor %}",
73+
notes: "Should produce empty result"
74+
},
75+
{
76+
name: "reversed with offset beyond length",
77+
template: "{% for item in array reversed offset:10 %}{{ item }}{% endfor %}",
78+
notes: "Should produce empty result"
79+
}
80+
]
81+
82+
puts "=" * 80
83+
puts "Testing Shopify Ruby Liquid Implementation"
84+
puts "Liquid version: #{Liquid::VERSION}"
85+
puts "=" * 80
86+
puts
87+
88+
tests.each_with_index do |test, i|
89+
template = Liquid::Template.parse(test[:template])
90+
result = template.render('array' => array)
91+
92+
puts "#{i+1}. #{test[:name]}"
93+
puts " Template: #{test[:template]}"
94+
puts " Result: '#{result}'"
95+
puts " Notes: #{test[:notes]}"
96+
puts
97+
end

‎tags/iteration_tags_test.go‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,18 @@ var iterationTests = []struct{ in, expected string }{
4141
{`{% for a in array limit: 0 %}{{ a }}.{% else %}ELSE{% endfor %}`, "ELSE"},
4242
{`{% for a in array offset: 3 %}{{ a }}.{% endfor %}`, ""},
4343
{`{% for a in array offset: 10 %}{{ a }}.{% endfor %}`, ""},
44-
// TODO investigate how these combine; does it depend on the order?
45-
// {`{% for a in array reversed offset:1 %}{{ a }}.{% endfor %}`, "second.first."},
46-
// {`{% for a in array limit:1 offset:1 %}{{ a }}.{% endfor %}`, "second."},
47-
// {`{% for a in array reversed limit:1 offset:1 %}{{ a }}.{% endfor %}`, "second."},
44+
// Combining multiple modifiers (issue #6)
45+
// Note: In this implementation, modifiers are always applied in the order: reversed -> offset -> limit
46+
// The order they appear in the template syntax does not matter.
47+
// This differs from Ruby Shopify Liquid where syntax order matters and reversed only works when placed first.
48+
{`{% for a in array reversed offset:1 %}{{ a }}.{% endfor %}`, "second.first."},
49+
{`{% for a in array offset:1 reversed %}{{ a }}.{% endfor %}`, "second.first."}, // same result - syntax order doesn't matter
50+
{`{% for a in array limit:1 offset:1 %}{{ a }}.{% endfor %}`, "second."},
51+
{`{% for a in array offset:1 limit:1 %}{{ a }}.{% endfor %}`, "second."}, // same result
52+
{`{% for a in array reversed limit:1 offset:1 %}{{ a }}.{% endfor %}`, "second."},
53+
{`{% for a in array reversed offset:1 limit:1 %}{{ a }}.{% endfor %}`, "second."}, // same result
54+
{`{% for a in array limit:1 offset:1 reversed %}{{ a }}.{% endfor %}`, "second."}, // same result
55+
{`{% for a in array offset:1 limit:1 reversed %}{{ a }}.{% endfor %}`, "second."}, // same result
4856

4957
// loop variables
5058
{`{% for a in array %}{{ forloop.first }}.{% endfor %}`, "true.false.false."},

0 commit comments

Comments
 (0)