Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove blank nodes from AST with omit_blank_nodes option #1870

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion History.md
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
# Liquid Change Log

## 5.6.1 2025-01-07

Add `omit_blank_nodes` parse option to skip blank nodes in the AST (#1870) [Michael Go]

## 5.6.0 (unreleased)

### Fixes

* Fix Tokenizer to handle null source value (#1873) [Bahar Pourazar]


## 5.5.0 2024-03-21

Please reference the GitHub release for more information.
Expand Down
19 changes: 19 additions & 0 deletions lib/liquid/block_body.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ def freeze
return yield tag_name, markup
end
new_tag = tag.parse(tag_name, markup, tokenizer, parse_context)

next if parse_context.omit_blank_nodes && blank_node?(new_tag)

@blank &&= new_tag.blank?
@nodelist << new_tag
end
Expand Down Expand Up @@ -153,6 +156,9 @@ def self.rescue_render_node(context, output, line_number, exc, blank_tag)
return yield tag_name, markup
end
new_tag = tag.parse(tag_name, markup, tokenizer, parse_context)

next if parse_context.omit_blank_nodes && blank_node?(new_tag)

@blank &&= new_tag.blank?
@nodelist << new_tag
when token.start_with?(VARSTART)
Expand Down Expand Up @@ -269,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
true
when BlockBody
true if node.nodelist.empty?
when Tag
node.nodelist.all? { |n| blank_node?(n) }
else
false
end
end
end
end
6 changes: 5 additions & 1 deletion lib/liquid/parse_context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/liquid/version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
# frozen_string_literal: true

module Liquid
VERSION = "5.6.0"
VERSION = "5.6.1"
end
1 change: 0 additions & 1 deletion performance/tests/tribble/404.liquid
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
<div id="page" class="innerpage clearfix">

<div id="text-page">
<div class="entry">
Expand Down
95 changes: 95 additions & 0 deletions test/unit/block_unit_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,103 @@ def test_with_block
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
Expand Down
4 changes: 2 additions & 2 deletions test/unit/template_unit_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ class TemplateUnitTest < Minitest::Test

def test_sets_default_localization_in_document
t = Template.new
t.parse('{%comment%}{%endcomment%}')
t.parse('{%raw%}{%endraw%}')
assert_instance_of(I18n, t.root.nodelist[0].options[:locale])
end

def test_sets_default_localization_in_context_with_quick_initialization
t = Template.new
t.parse('{%comment%}{%endcomment%}', locale: I18n.new(fixture("en_locale.yml")))
t.parse('{%raw%}{%endraw%}', locale: I18n.new(fixture("en_locale.yml")))

locale = t.root.nodelist[0].options[:locale]
assert_instance_of(I18n, locale)
Expand Down
Loading