diff --git a/lib/liquid/tags/cycle.rb b/lib/liquid/tags/cycle.rb index b7d3069c8..17ed46525 100644 --- a/lib/liquid/tags/cycle.rb +++ b/lib/liquid/tags/cycle.rb @@ -17,7 +17,6 @@ module Liquid class Cycle < Tag SimpleSyntax = /\A#{QuotedFragment}+/o NamedSyntax = /\A(#{QuotedFragment})\s*\:\s*(.*)/om - UNNAMED_CYCLE_PATTERN = /\w+:0x\h{8}/ attr_reader :variables @@ -86,8 +85,8 @@ def strict2_parse(markup) p.consume(:end_of_string) unless @is_named - @name = @variables.to_s - @is_named = !@name.match?(UNNAMED_CYCLE_PATTERN) + @name = cycle_key + @is_named = @variables.none? { |variable| variable.is_a?(VariableLookup) } end end @@ -103,8 +102,8 @@ def lax_parse(markup) @is_named = true when SimpleSyntax @variables = variables_from_string(markup) - @name = @variables.to_s - @is_named = !@name.match?(UNNAMED_CYCLE_PATTERN) + @name = cycle_key + @is_named = @variables.none? { |variable| variable.is_a?(VariableLookup) } else raise SyntaxError, options[:locale].t("errors.syntax.cycle") end @@ -120,13 +119,14 @@ def variables_from_string(markup) end.compact end - # For backwards compatibility, whenever a lookup is used in an unnamed cycle, - # we make it so that the @variables.to_s produces different strings for cycles - # called with the same arguments (since @variables.to_s is used as the cycle counter key) - # This makes it so {% cycle a, b %} and {% cycle a, b %} have independent counters even if a and b share value. - # This is not true for literal values, {% cycle "a", "b" %} and {% cycle "a", "b" %} share the same counter. - # I was really scratching my head about this one, but migrating away from this would be more headache - # than it's worth. So we're keeping this quirk for now. + # Build an unnamed cycle key from lookup paths instead of object identity so + # repeated cycle tags using the same variables share their counter. + def cycle_key + @variables.map do |variable| + variable.is_a?(VariableLookup) ? [variable.name, variable.lookups] : variable + end.to_s + end + def maybe_dup_lookup(var) var.is_a?(VariableLookup) ? var.dup : var end diff --git a/test/integration/tags/cycle_tag_test.rb b/test/integration/tags/cycle_tag_test.rb index b0cdf606f..8a4435b93 100644 --- a/test/integration/tags/cycle_tag_test.rb +++ b/test/integration/tags/cycle_tag_test.rb @@ -50,7 +50,7 @@ def test_cycle_named_groups_vlookup assert_template_result("112233", template) end - def test_unnamed_cycle_have_independent_counters_when_used_with_lookups + def test_unnamed_cycle_share_counters_when_used_with_lookups template = <<~LIQUID {%- assign a = "1" -%} {%- for i in (1..3) -%} @@ -59,7 +59,12 @@ def test_unnamed_cycle_have_independent_counters_when_used_with_lookups {%- endfor -%} LIQUID - assert_template_result("112211", template) + assert_template_result("121212", template) + end + + def test_unnamed_cycle_advances_when_first_value_is_a_lookup + assert_template_result("b", "{% cycle a, 'b' %}{% cycle a, 'b' %}") + assert_template_result("ab", "{% assign a = 'a' %}{% cycle a, 'b' %}{% cycle a, 'b' %}") end def test_unnamed_cycle_dependent_counter_when_used_with_literal_values