From fece1c8fb1d1eecb470806bd462c1f783ffb8434 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20BEAU?= Date: Mon, 27 Jan 2020 17:43:02 +0100 Subject: [PATCH] Fix: #171 looping was modifing the original attribute --- lib/locomotive/steam/liquid/tags/concerns/attributes.rb | 8 ++++---- lib/locomotive/steam/liquid/tags/consume.rb | 4 ++-- lib/locomotive/steam/liquid/tags/with_scope.rb | 8 ++++---- spec/unit/liquid/tags/path_to_spec.rb | 7 +++++++ spec/unit/liquid/tags/with_scope_spec.rb | 9 +++++++++ 5 files changed, 26 insertions(+), 10 deletions(-) diff --git a/lib/locomotive/steam/liquid/tags/concerns/attributes.rb b/lib/locomotive/steam/liquid/tags/concerns/attributes.rb index 4ab25ec6..8c2f77c4 100644 --- a/lib/locomotive/steam/liquid/tags/concerns/attributes.rb +++ b/lib/locomotive/steam/liquid/tags/concerns/attributes.rb @@ -18,7 +18,7 @@ module Attributes private def parse_attributes(markup, default = {}) - @attributes = default || {} + @raw_attributes = default || {} attribute_markup = "" if markup =~ /^ *([a-zA-Z0-9_.]*:.*)$/ attribute_markup = $1 @@ -26,9 +26,9 @@ def parse_attributes(markup, default = {}) attribute_markup = $1 end unless attribute_markup.blank? - @attributes.merge!(AttributeParser.parse(attribute_markup)) + @raw_attributes.merge!(AttributeParser.parse(attribute_markup)) end - @attributes + @raw_attributes end def context_evaluate_array(vals) @@ -41,7 +41,7 @@ def context_evaluate(vals) def evaluate_attributes(context, lax: false) @attributes = HashWithIndifferentAccess.new.tap do |hash| - attributes.each do |key, value| + raw_attributes.each do |key, value| hash[evaluate_value(context, key, lax: lax)] = evaluate_value(context, value, lax: lax) end end diff --git a/lib/locomotive/steam/liquid/tags/consume.rb b/lib/locomotive/steam/liquid/tags/consume.rb index 8d9477f5..cc671039 100644 --- a/lib/locomotive/steam/liquid/tags/consume.rb +++ b/lib/locomotive/steam/liquid/tags/consume.rb @@ -25,9 +25,9 @@ def initialize(tag_name, markup, options) super if markup =~ Syntax - @variable_name, @url, attributes = $1.to_s, ::Liquid::Expression.parse($2), $3 + @variable_name, @url, _attributes = $1.to_s, ::Liquid::Expression.parse($2), $3 - parse_attributes(attributes) + parse_attributes(_attributes) else raise ::Liquid::SyntaxError.new("Syntax Error in 'consume' - Valid syntax: consume from \"\" [username: value, password: value]") end diff --git a/lib/locomotive/steam/liquid/tags/with_scope.rb b/lib/locomotive/steam/liquid/tags/with_scope.rb index 5628f800..ed507695 100644 --- a/lib/locomotive/steam/liquid/tags/with_scope.rb +++ b/lib/locomotive/steam/liquid/tags/with_scope.rb @@ -48,21 +48,21 @@ def initialize(tag_name, markup, options) # simple hash? parse_attributes(markup) { |value| parse_attribute(value) } - if attributes.empty? && markup =~ SingleVariable + if raw_attributes.empty? && markup =~ SingleVariable # alright, maybe we'vot got a single variable built # with the Action liquid tag instead? @attributes_var_name = Regexp.last_match(1) end - if attributes.empty? && attributes_var_name.blank? + if raw_attributes.empty? && attributes_var_name.blank? raise ::Liquid::SyntaxError.new("Syntax Error in 'with_scope' - Valid syntax: with_scope : , ..., : ") end end def render(context) context.stack do - @attributes = context[attributes_var_name] || {} if attributes_var_name.present? - @attributes.transform_keys! { |key| key.to_s == '_permalink' ? '_slug' : key.to_s } + @raw_attributes = context[attributes_var_name] || {} if attributes_var_name.present? + @raw_attributes.transform_keys! { |key| key.to_s == '_permalink' ? '_slug' : key.to_s } context['with_scope'] = evaluate_attributes(context) # for now, no content type is assigned to this with_scope diff --git a/spec/unit/liquid/tags/path_to_spec.rb b/spec/unit/liquid/tags/path_to_spec.rb index ee9c9ab6..75afdc2b 100644 --- a/spec/unit/liquid/tags/path_to_spec.rb +++ b/spec/unit/liquid/tags/path_to_spec.rb @@ -106,6 +106,13 @@ it { is_expected.to eq '/fr/a-notre-sujet' } + context 'loop on several locale from variable' do + let(:assigns) { { 'about_us' => drop, 'langs' => ['en', 'fr'] } } + let(:source) { "{% for lang in langs %}{% path_to about_us, locale: lang %}|{% endfor %}" } + + it { is_expected.to eq '/about-us|/fr/a-notre-sujet|' } + end + end end diff --git a/spec/unit/liquid/tags/with_scope_spec.rb b/spec/unit/liquid/tags/with_scope_spec.rb index b96cac0f..e8070147 100644 --- a/spec/unit/liquid/tags/with_scope_spec.rb +++ b/spec/unit/liquid/tags/with_scope_spec.rb @@ -190,6 +190,15 @@ end + describe 'In a loop context, each scope should be evaluated correctly' do + let(:assigns) { {'list' => ['A', 'B', 'C']} } + + let(:source) { "{% for key in list %}{% with_scope foo: key %}{% assign conditions = with_scope %}{% endwith_scope %}{{ conditions }}{% endfor %}" } + + it { expect(output).to eq '{"foo"=>"A"}{"foo"=>"B"}{"foo"=>"C"}' } + + end + end end