diff --git a/lib/liquid/block_body.rb b/lib/liquid/block_body.rb index a6150900b..f1dd2a1e2 100644 --- a/lib/liquid/block_body.rb +++ b/lib/liquid/block_body.rb @@ -59,7 +59,7 @@ def freeze end new_tag = tag.parse(tag_name, markup, tokenizer, parse_context) - next if new_tag.is_a?(Comment) + next if parse_context.omit_blank_nodes && blank_node?(new_tag) @blank &&= new_tag.blank? @nodelist << new_tag @@ -157,7 +157,7 @@ def self.rescue_render_node(context, output, line_number, exc, blank_tag) end new_tag = tag.parse(tag_name, markup, tokenizer, parse_context) - next if new_tag.is_a?(Comment) + next if parse_context.omit_blank_nodes && blank_node?(new_tag) @blank &&= new_tag.blank? @nodelist << new_tag @@ -275,5 +275,18 @@ def raise_missing_tag_terminator(token, parse_context) def raise_missing_variable_terminator(token, parse_context) BlockBody.raise_missing_variable_terminator(token, parse_context) end + + def blank_node?(node) + case node + when Comment + return true + when BlockBody + return true if node.nodelist.empty? + when Tag + return node.nodelist.all? { |n| blank_node?(n) } + end + + false + end end end diff --git a/lib/liquid/parse_context.rb b/lib/liquid/parse_context.rb index 7bd5418d5..6e29765e2 100644 --- a/lib/liquid/parse_context.rb +++ b/lib/liquid/parse_context.rb @@ -3,7 +3,7 @@ module Liquid class ParseContext attr_accessor :locale, :line_number, :trim_whitespace, :depth - attr_reader :partial, :warnings, :error_mode, :environment + attr_reader :partial, :warnings, :error_mode, :environment, :omit_blank_nodes def initialize(options = Const::EMPTY_HASH) @environment = options.fetch(:environment, Environment.default) @@ -12,6 +12,10 @@ def initialize(options = Const::EMPTY_HASH) @locale = @template_options[:locale] ||= I18n.new @warnings = [] + # remove blank nodes such as + # comment tags, empty if tags, etc from the AST + @omit_blank_nodes = options.fetch(:omit_blank_nodes, false) + self.depth = 0 self.partial = false end diff --git a/performance/tests/tribble/404.liquid b/performance/tests/tribble/404.liquid index acf184c74..4a2cd27e6 100644 --- a/performance/tests/tribble/404.liquid +++ b/performance/tests/tribble/404.liquid @@ -1,4 +1,3 @@ -
diff --git a/test/unit/block_unit_test.rb b/test/unit/block_unit_test.rb index bab11c400..d94e686ff 100644 --- a/test/unit/block_unit_test.rb +++ b/test/unit/block_unit_test.rb @@ -49,12 +49,107 @@ def test_variable_many_embedded_fragments def test_with_block template = Liquid::Template.parse(" {% comment %} {% endcomment %} ") - assert_equal([String, String], block_types(template.root.nodelist)) - assert_equal(2, template.root.nodelist.size) + assert_equal([String, Comment, String], block_types(template.root.nodelist)) + assert_equal(3, template.root.nodelist.size) + end + + def test_remove_empty_for_blocks_with_optimization_option + source = <<~LIQUID.chomp + {% for i in (1..1000000) %} + {% endfor %} + LIQUID + + assert_root_nodelist_size(source, 0, omit_blank_nodes: true) + + source = <<~LIQUID.chomp + {% for i in (1..1000000) %} + {% else %} + {% endfor %} + LIQUID + + assert_root_nodelist_size(source, 0, omit_blank_nodes: true) + + source = <<~LIQUID.chomp + {% for i in list %} + i + {% endfor %} + LIQUID + + assert_root_nodelist_size(source, 1, omit_blank_nodes: true) + + source = <<~LIQUID.chomp + {% for i in list %} + {% else %} + 1 + {% endfor %} + LIQUID + + assert_root_nodelist_size(source, 1, omit_blank_nodes: true) + end + + def test_remove_comment_nodes_with_optimization_option + source = <<~LIQUID.chomp + {% comment %} + {% if true %} + {% endif %} + {% endcomment %} + LIQUID + + assert_root_nodelist_size(source, 0, omit_blank_nodes: true) + + source = <<~LIQUID.chomp + {% liquid + comment + if true + endif + endcomment + %} + LIQUID + + assert_root_nodelist_size(source, 0, omit_blank_nodes: true) + end + + def test_remove_if_nodes_with_optimization_option + source = <<~LIQUID.chomp + {% if true %} + {% endif %} + LIQUID + + assert_root_nodelist_size(source, 0, omit_blank_nodes: true) + + source = <<~LIQUID.chomp + {% unless true %} + {% endunless %} + LIQUID + + assert_root_nodelist_size(source, 0, omit_blank_nodes: true) + + source = <<~LIQUID.chomp + {% if false %} + {% else %} + {% endif %} + LIQUID + + assert_root_nodelist_size(source, 0, omit_blank_nodes: true) + + source = <<~LIQUID.chomp + {% if false %} + {% else %} + Hello! + {% endif %} + LIQUID + + assert_root_nodelist_size(source, 1, omit_blank_nodes: true) end private + def assert_root_nodelist_size(source, expected_size, parse_options = {}) + template = Liquid::Template.parse(source, parse_options) + + assert_equal(expected_size, template.root.nodelist.size) + end + def block_types(nodelist) nodelist.collect(&:class) end diff --git a/test/unit/tags/comment_tag_unit_test.rb b/test/unit/tags/comment_tag_unit_test.rb index ad2a0d187..56618b720 100644 --- a/test/unit/tags/comment_tag_unit_test.rb +++ b/test/unit/tags/comment_tag_unit_test.rb @@ -199,26 +199,4 @@ def test_dont_override_liquid_tag_whitespace_control World! LIQUID end - - def test_comment_tag_node_is_not_in_nodelist - template = Liquid::Template.parse(<<~LIQUID.chomp) - {% comment %} - {% if true %} - {% endif %} - {% endcomment %} - LIQUID - - assert_equal(0, template.root.nodelist.size) - - template = Liquid::Template.parse(<<~LIQUID.chomp) - {% liquid - comment - if true - endif - endcomment - %} - LIQUID - - assert_equal(0, template.root.nodelist.size) - end end