Repository navigation
Fix variable ranges, for limit, if/for scoping, append coercion & string delimiters - #18
Merged
Merged
Conversation
…ng delimiters
Brings several behaviors in line with the Ruby/Shopify Liquid reference
implementation. All existing tests pass; adds test/test_bug_fixes.ml.
- Variable ranges: `(1..n)` and `(a..b)` now work, not just integer
literals. `LexRange` carries two `lex_value`s and a new deferred
`Range of value * value` is resolved at interpret time in `Values.unwrap`
(descending ranges yield an empty list, matching Ruby).
- `for` default limit: removed the implicit cap of 50. Ruby has no default
limit; only an explicit `limit:` caps iteration.
- if/for scoping: `if`/`else` no longer discard variables assigned inside a
branch, and a `for` body's assignments now persist after the loop (Ruby
writes assigns to the root scope). The loop variable and `forloop` are
still restored to their pre-loop state, and break/continue no longer leak
past the loop (verified with nested loops and shadowed loop variables).
- append/prepend: coerce non-string arguments using the same stringification
as `{{ }}` output, so `{{ 'n=' | append: 5 }}` => "n=5".
- Lexer: the block-token scanner is now quote-aware, so `{{`, `}}`, `{%`,
`%}` inside a string literal in a tag are no longer mistaken for
delimiters (e.g. `{% assign o = '{% raw %}' %}`). Raw text is unaffected.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Several fixes that bring
liquid_mlin line with the Ruby/Shopify Liquidreference implementation. Each is covered by new regression tests in
test/test_bug_fixes.ml; the full suite (incl. the real Shopify themefixtures) passes — 294 tests.
What's fixed
Variable ranges
(1..n)/(a..b)Previously only integer literals were accepted; a variable bound silently
produced an empty loop.
LexRangenow carries twolex_values, and a newdeferred
Range of value * valueis resolved at interpret time inValues.unwrap. Descending ranges yield an empty list (matches Ruby).{%- assign n = 4 -%}{% for i in (1..n) %}{{ i }}{% endfor %} => 1234forhad an implicit limit of 50Ruby's
forhas no default limit; only an explicitlimit:caps it. Thedefault is now effectively unbounded.
if/forno longer discard in-body assignmentsThis matched neither Ruby nor Shopify: a variable assigned inside an
if/forbody was deleted when the block ended. Ruby writesassignto theroot scope, so those values persist.
{% if true %}{% assign x = 5 %}{% endif %}{{ x }} => 5 {% for i in (1..3) %}{% assign last = i %}{% endfor %}{{ last }} => 3The loop variable and
forloopare still scoped to the loop (restored totheir pre-loop state afterwards), and
break/continueno longer leak pastthe loop. Verified with nested loops and shadowed loop variables
(
{%- assign i='before' -%}{% for i in (1..3) %}{% endfor %}{{ i }}=>before).append/prependcoerce non-string argumentsRuby coerces operands to strings. These now stringify the same way
{{ }}output does.
{{ 'n=' | append: 5 }} => n=5Lexer: delimiters inside string literals
The block-token scanner is now quote-aware:
{{,}},{%,%}inside astring literal in a tag are no longer mistaken for block delimiters. Raw text
is unaffected (quotes there are not treated specially), so well-formed
templates are unchanged — this only makes the lexer accept a superset.
{% assign o = '{% raw %}' %}{{ o }} => {% raw %}Compatibility note
liquid_ml renders
nil/undefined as the literal"nil"(Ruby renders"").That is pre-existing and unchanged here, but
append/prependnow surface it(
{{ missing | append: 'y' }}=>"nily"). I keptappendconsistent with{{ }}interpolation, so fixing the globalnil→""rendering later wouldmake
appendRuby-correct automatically. Happy to do that in a follow-up.Files
liquid_syntax/tokens.ml,liquid_syntax/syntax.ml,liquid_syntax/values.ml—Rangevalue + resolution, defaultforlimitliquid_parser/lexer.ml— range-bound atoms, quote-aware block scannerliquid_std/liquid_string.ml—append/prependcoercionliquid_interpreter/interpreter.ml— if/for scopingtest/test_bug_fixes.ml(+test/test_runner.ml) — 18 regression tests