diff --git a/Gemfile b/Gemfile index 37ffe1d..99df65c 100644 --- a/Gemfile +++ b/Gemfile @@ -18,6 +18,6 @@ group :test do gem 'rubocop', '~> 0.53.0' platform :mri do - gem 'liquid-c', github: 'Shopify/liquid-c', ref: '9168659de45d6d576fce30c735f857e597fa26f6' + gem 'liquid-c', github: 'Shopify/liquid-c', ref: 'reversable-range' end end diff --git a/lib/liquid.rb b/lib/liquid.rb index 770d2f9..bab5e6a 100644 --- a/lib/liquid.rb +++ b/lib/liquid.rb @@ -74,6 +74,7 @@ require 'liquid/condition' require 'liquid/utils' require 'liquid/tokenizer' require 'liquid/parse_context' +require 'liquid/reversable_range' # Load all the tags of the standard library # diff --git a/lib/liquid/range_lookup.rb b/lib/liquid/range_lookup.rb index 93bb420..54db868 100644 --- a/lib/liquid/range_lookup.rb +++ b/lib/liquid/range_lookup.rb @@ -6,7 +6,7 @@ module Liquid if start_obj.respond_to?(:evaluate) || end_obj.respond_to?(:evaluate) new(start_obj, end_obj) else - start_obj.to_i..end_obj.to_i + ReversableRange.new(start_obj.to_i, end_obj.to_i) end end @@ -18,7 +18,7 @@ module Liquid def evaluate(context) start_int = to_integer(context.evaluate(@start_obj)) end_int = to_integer(context.evaluate(@end_obj)) - start_int..end_int + ReversableRange.new(start_int, end_int) end private diff --git a/lib/liquid/reversable_range.rb b/lib/liquid/reversable_range.rb new file mode 100644 index 0000000..48f4d1a --- /dev/null +++ b/lib/liquid/reversable_range.rb @@ -0,0 +1,77 @@ +module Liquid + class ReversableRange + include Enumerable + + def initialize(min, max) + @min = min + @max = max + @reversed = false + end + + def each + if reversed + index = max + while index >= min + yield index + index -= 1 + end + else + index = min + while index <= max + yield index + index += 1 + end + end + end + + def reverse! + @reversed = !reversed + self + end + + def empty? + max < min + end + + def size + difference = max - min + if difference > 0 + difference + 1 + else + 0 + end + end + + def load_slice(from, to = nil) + to ||= max + slice_max = [max, to].min + slice_min = [min, from].max + range = ReversableRange.new(slice_min, slice_max) + range.reverse! if reversed + range + end + + def ==(other) + other.is_a?(self.class) && + other.min == min && + other.max == max && + other.reversed == reversed + end + + def to_s + if reversed + "#{max}..#{min}" + else + "#{min}..#{max}" + end + end + + def to_liquid + self + end + + protected + + attr_reader :min, :max, :reversed + end +end diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb index f18fb71..2d88917 100644 --- a/lib/liquid/tags/for.rb +++ b/lib/liquid/tags/for.rb @@ -133,7 +133,6 @@ module Liquid end collection = context.evaluate(@collection_name) - collection = collection.to_a if collection.is_a?(Range) limit_value = context.evaluate(@limit) to = if limit_value.nil? @@ -145,14 +144,14 @@ module Liquid segment = Utils.slice_collection(collection, from, to) segment.reverse! if @reversed - offsets[@name] = from + segment.length + offsets[@name] = from + segment.size segment end def render_segment(context, segment) for_stack = context.registers[:for_stack] ||= [] - length = segment.length + length = segment.size result = '' diff --git a/lib/liquid/utils.rb b/lib/liquid/utils.rb index 516ac0c..33f9a0a 100644 --- a/lib/liquid/utils.rb +++ b/lib/liquid/utils.rb @@ -1,6 +1,8 @@ module Liquid module Utils def self.slice_collection(collection, from, to) + return collection.load_slice(from, to) if collection.is_a?(ReversableRange) + if (from != 0 || !to.nil?) && collection.respond_to?(:load_slice) collection.load_slice(from, to) else diff --git a/test/integration/tags/for_tag_test.rb b/test/integration/tags/for_tag_test.rb index 9980e25..4059db3 100644 --- a/test/integration/tags/for_tag_test.rb +++ b/test/integration/tags/for_tag_test.rb @@ -36,6 +36,10 @@ HERE assert_template_result('321', '{%for item in array reversed %}{{item}}{%endfor%}', assigns) end + def test_for_range_reversed + assert_template_result('321', '{%for item in (1..3) reversed %}{{item}}{%endfor%}', {}) + end + def test_for_with_range assert_template_result(' 1 2 3 ', '{%for item in (1..3) %} {{item}} {%endfor%}') diff --git a/test/unit/context_unit_test.rb b/test/unit/context_unit_test.rb index fab19b8..0603675 100644 --- a/test/unit/context_unit_test.rb +++ b/test/unit/context_unit_test.rb @@ -349,9 +349,9 @@ class ContextUnitTest < Minitest::Test def test_ranges @context.merge("test" => '5') - assert_equal (1..5), @context['(1..5)'] - assert_equal (1..5), @context['(1..test)'] - assert_equal (5..5), @context['(test..test)'] + assert_equal ReversableRange.new(1, 5), @context['(1..5)'] + assert_equal ReversableRange.new(1, 5), @context['(1..test)'] + assert_equal ReversableRange.new(5, 5), @context['(test..test)'] end def test_cents_through_drop_nestedly diff --git a/test/unit/reversable_range_unit_test.rb b/test/unit/reversable_range_unit_test.rb new file mode 100644 index 0000000..95cf271 --- /dev/null +++ b/test/unit/reversable_range_unit_test.rb @@ -0,0 +1,116 @@ +require 'test_helper' + +class ReversableRangeTest < Minitest::Test + include Liquid + + def test_each_iterates_through_items_in_the_range + actual_items = [] + ReversableRange.new(1, 10).each { |item| actual_items << item } + + expected_items = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10] + assert_equal expected_items, actual_items + end + + def test_implements_enumerable + actual_items = ReversableRange.new(1, 10).select(&:even?) + + expected_items = [2, 4, 6, 8, 10] + assert_equal expected_items, actual_items + end + + def test_is_not_empty_max_greater_than_min + range = ReversableRange.new(9, 10) + + refute_predicate range, :empty? + end + + def test_is_not_empty_max_equal_to_min + range = ReversableRange.new(10, 10) + + refute_predicate range, :empty? + end + + def test_is_empty_if_not_reversed_and_max_less_than_min + range = ReversableRange.new(10, 9) + + assert_predicate range, :empty? + end + + def test_ranges_with_the_same_max_and_min_have_one_element + actual_items = ReversableRange.new(1337, 1337).to_a + expected_items = [1337] + assert_equal expected_items, actual_items + end + + def test_load_slice_returns_a_sub_range + actual_items = ReversableRange.new(1, 10).load_slice(5, 8).to_a + + expected_items = [5, 6, 7, 8] + assert_equal expected_items, actual_items + end + + def test_load_slice_returns_a_reversed_sub_range_if_reversed + range = ReversableRange.new(1, 10) + range.reverse! + actual_items = range.load_slice(5, 8).to_a + + expected_items = [8, 7, 6, 5] + assert_equal expected_items, actual_items + end + + def test_is_equal_to_other_if_also_a_reversable_range_and_has_same_properties + one = ReversableRange.new(1, 10) + one.reverse! + + two = ReversableRange.new(1, 10) + two.reverse! + + assert_equal one, two + end + + def test_is_not_equal_to_a_non_reversable_range + range = ReversableRange.new(1, 10) + range.reverse! + + refute_equal range, :something_else + end + + def test_is_not_equal_if_ranges_have_different_mins + one = ReversableRange.new(1, 10) + two = ReversableRange.new(2, 10) + + refute_equal one, two + end + + def test_is_not_equal_if_ranges_have_different_maxes + one = ReversableRange.new(1, 10) + two = ReversableRange.new(1, 11) + + refute_equal one, two + end + + def test_is_not_equal_if_only_one_is_reversed + one = ReversableRange.new(1, 10) + + two = ReversableRange.new(1, 10) + two.reverse! + + refute_equal one, two + end + + def test_to_s_mirrors_rubys_range_syntax + range = ReversableRange.new(1, 10) + assert_equal '1..10', range.to_s + end + + def test_to_s_reverses_when_reversed + range = ReversableRange.new(1, 10) + range.reverse! + assert_equal '10..1', range.to_s + end + + def test_size + range = ReversableRange.new(1, 10) + assert_equal 10, range.size + end +end