From 3ef7eead27271b4541f81bddd15dc8a8b8d20b10 Mon Sep 17 00:00:00 2001 From: Mike Angell Date: Wed, 28 Aug 2019 04:25:26 +1000 Subject: [PATCH] Stack scope by variable and not by level --- lib/liquid/context.rb | 39 ++++++++++++++++++--------- lib/liquid/tags/assign.rb | 2 +- lib/liquid/tags/capture.rb | 2 +- lib/liquid/tags/for.rb | 3 +++ test/integration/tags/for_tag_test.rb | 4 +-- test/unit/context_unit_test.rb | 29 -------------------- 6 files changed, 33 insertions(+), 46 deletions(-) diff --git a/lib/liquid/context.rb b/lib/liquid/context.rb index cc25b42..9d58bc2 100644 --- a/lib/liquid/context.rb +++ b/lib/liquid/context.rb @@ -88,7 +88,7 @@ module Liquid # Merge a hash of variables in the current local scope def merge(new_scopes) - @scope.merge!(new_scopes) + new_scopes.each { |k, v| self[k] = v } end # Pushes a new local scope on the stack, pops it at the end of the block @@ -100,25 +100,19 @@ module Liquid # # context['var] #=> nil def stack(*variable_names) - previous_values = {} - variable_names.each do |variable_name| - previous_values[variable_name] = @scope[variable_name] - end - @stack_level += 1 raise StackLevelError, "Nesting too deep".freeze if @stack_level > Block::MAX_DEPTH begin yield ensure - @scope.merge!(previous_values) @stack_level -= 1 end end # Only allow String, Numeric, Hash, Array, Proc, Boolean or Liquid::Drop def []=(key, value) - @scope[key] = value + (@scope[key] ||= [nil]) << value end # Look up variable, either resolve directly after considering the name. We can directly handle @@ -133,6 +127,19 @@ module Liquid evaluate(Expression.parse(expression)) end + def unset(key) + if @scope[key].size <= 1 + @scope.delete(key) + else + @scope[key].pop + end + end + + def set_root(key, val) + @scope[key] ||= [] + @scope[key][0] = val + end + def key?(key) self[key] != nil end @@ -143,8 +150,10 @@ module Liquid # Fetches an object starting at the local scope and then moving up the hierachy def find_variable(key, raise_on_not_found: true) + trigger = false value = @scope[key] - scope = @scope if value != nil + scope = @scope unless value.nil? + trigger = true unless value.nil? if scope.nil? index = @environments.find_index do |e| @@ -157,7 +166,7 @@ module Liquid scope = @environments[index || -1] end - variable ||= lookup_and_evaluate(scope, key, raise_on_not_found: raise_on_not_found) + variable ||= lookup_and_evaluate(scope, key, trigger, raise_on_not_found: raise_on_not_found) variable = variable.to_liquid variable.context = self if variable.respond_to?(:context=) @@ -165,12 +174,16 @@ module Liquid variable end - def lookup_and_evaluate(obj, key, raise_on_not_found: true) + def lookup_and_evaluate(obj, key, trigger = false, raise_on_not_found: true) if @strict_variables && raise_on_not_found && obj.respond_to?(:key?) && !obj.key?(key) raise Liquid::UndefinedVariable, "undefined variable #{key}" end - value = obj[key] + value = if trigger == true + obj[key][-1] + else + obj[key] + end if value.is_a?(Proc) && obj.respond_to?(:[]=) obj[key] = (value.arity == 0) ? value.call : value.call(self) @@ -192,7 +205,7 @@ module Liquid @scope.each_key do |k| @environments.each do |env| if env.key?(k) - @scope[k] = lookup_and_evaluate(env, k) + @scope[k] = [lookup_and_evaluate(env, k)] break end end diff --git a/lib/liquid/tags/assign.rb b/lib/liquid/tags/assign.rb index ca630f6..33f1e13 100644 --- a/lib/liquid/tags/assign.rb +++ b/lib/liquid/tags/assign.rb @@ -24,7 +24,7 @@ module Liquid def render(context) val = @from.render(context) - context[@to] = val + context.set_root(@to, val) context.resource_limits.assign_score += assign_score_of(val) ''.freeze end diff --git a/lib/liquid/tags/capture.rb b/lib/liquid/tags/capture.rb index 92f25d8..4c47fa7 100644 --- a/lib/liquid/tags/capture.rb +++ b/lib/liquid/tags/capture.rb @@ -24,7 +24,7 @@ module Liquid def render(context) output = super - context[@to] = output + context.set_root(@to, output) context.resource_limits.assign_score += output.bytesize ''.freeze end diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb index ef68239..1637724 100644 --- a/lib/liquid/tags/for.rb +++ b/lib/liquid/tags/for.rb @@ -168,6 +168,7 @@ module Liquid context[@variable_name] = item result << @for_block.render(context) loop_vars.send(:increment!) + context.unset(@variable_name) # Handle any interrupts if they exist. if context.interrupt? @@ -176,6 +177,8 @@ module Liquid next if interrupt.is_a? ContinueInterrupt end end + + context.unset('forloop'.freeze) ensure for_stack.pop end diff --git a/test/integration/tags/for_tag_test.rb b/test/integration/tags/for_tag_test.rb index a74ab99..dbe8d24 100644 --- a/test/integration/tags/for_tag_test.rb +++ b/test/integration/tags/for_tag_test.rb @@ -369,7 +369,7 @@ HERE end def test_overwriting_internal_variable - template = <<-END_TEMPLATE + template = <<-HEREDOC {% assign forloop = 'first' %} {% for item in items %} @@ -379,7 +379,7 @@ HERE {% endfor %} {{ forloop }} - END_TEMPLATE + HEREDOC result = Liquid::Template.parse(template).render('items' => '1') assert_equal 'Liquid::ForloopDrop Liquid::ForloopDrop second', result.split.map(&:strip).join(' ') diff --git a/test/unit/context_unit_test.rb b/test/unit/context_unit_test.rb index 2acadf9..d353d70 100644 --- a/test/unit/context_unit_test.rb +++ b/test/unit/context_unit_test.rb @@ -163,35 +163,6 @@ class ContextUnitTest < Minitest::Test assert_equal 'test', @context['test'] end - def test_add_item_in_inner_scope - @context.stack('test') do - @context['test'] = 'test' - assert_equal 'test', @context['test'] - end - - assert_nil @context['test'] - end - - def test_nested_scopes - @context['test'] = 1 - - @context.stack('test') do - assert_equal 1, @context['test'] - @context['test'] = 2 - assert_equal 2, @context['test'] - - @context.stack('test') do - assert_equal 2, @context['test'] - @context['test'] = 3 - assert_equal 3, @context['test'] - end - - assert_equal 2, @context['test'] - end - - assert_equal 1, @context['test'] - end - def test_hierachical_data @context['hash'] = { "name" => 'tobi' } assert_equal 'tobi', @context['hash.name']