From d6e13faa4312eb372473878dfd4162b389a2d871 Mon Sep 17 00:00:00 2001 From: Florian Weingarten Date: Wed, 26 Jun 2013 20:53:13 -0400 Subject: [PATCH 1/8] Don't render blank blocks --- lib/liquid/block.rb | 20 ++++++++++++++++---- lib/liquid/tag.rb | 6 +++--- lib/liquid/tags/capture.rb | 4 ++++ lib/liquid/tags/comment.rb | 4 ++++ 4 files changed, 27 insertions(+), 7 deletions(-) diff --git a/lib/liquid/block.rb b/lib/liquid/block.rb index a0a07e4..022a5e5 100644 --- a/lib/liquid/block.rb +++ b/lib/liquid/block.rb @@ -1,17 +1,22 @@ module Liquid - class Block < Tag + attr_reader :blank + IsTag = /^#{TagStart}/o IsVariable = /^#{VariableStart}/o FullToken = /^#{TagStart}\s*(\w+)\s*(.*)?#{TagEnd}$/o ContentOfVariable = /^#{VariableStart}(.*)#{VariableEnd}$/o + def self.blank? + false + end + def parse(tokens) + @blank = true @nodelist ||= [] @nodelist.clear while token = tokens.shift - case token when IsTag if token =~ FullToken @@ -20,12 +25,15 @@ module Liquid # proceed if block_delimiter == $1 end_tag + @blank = true if self.class.blank? return end # fetch the tag from registered blocks if tag = Template.tags[$1] - @nodelist << tag.new($1, $2, tokens) + new_tag = tag.new($1, $2, tokens) + @blank = false if new_tag.is_a?(Block) && !new_tag.blank + @nodelist << new_tag else # this tag is not registered with the system # pass it to the current block for special handling or error reporting @@ -36,10 +44,12 @@ module Liquid end when IsVariable @nodelist << create_variable(token) + @blank = false when '' # pass else @nodelist << token + @blank = false unless token =~ /\A\s*\z/ end end @@ -112,7 +122,9 @@ module Liquid context.resource_limits[:reached] = true raise MemoryError.new("Memory limits exceeded") end - output << token_output + unless token.respond_to?(:blank) && token.blank + output << token_output + end rescue MemoryError => e raise e rescue ::StandardError => e diff --git a/lib/liquid/tag.rb b/lib/liquid/tag.rb index b7f2aa4..c672344 100644 --- a/lib/liquid/tag.rb +++ b/lib/liquid/tag.rb @@ -1,7 +1,5 @@ module Liquid - class Tag - attr_accessor :nodelist def initialize(tag_name, markup, tokens) @@ -21,6 +19,8 @@ module Liquid '' end + def self.blank? + true + end end # Tag - end # Tag diff --git a/lib/liquid/tags/capture.rb b/lib/liquid/tags/capture.rb index 495a6f7..76c464f 100644 --- a/lib/liquid/tags/capture.rb +++ b/lib/liquid/tags/capture.rb @@ -30,6 +30,10 @@ module Liquid context.resource_limits[:assign_score_current] += (output.respond_to?(:length) ? output.length : 1) '' end + + def self.blank? + true + end end Template.register_tag('capture', Capture) diff --git a/lib/liquid/tags/comment.rb b/lib/liquid/tags/comment.rb index 37fb4c8..99e08c1 100644 --- a/lib/liquid/tags/comment.rb +++ b/lib/liquid/tags/comment.rb @@ -3,6 +3,10 @@ module Liquid def render(context) '' end + + def self.blank? + true + end end Template.register_tag('comment', Comment) From 10c151e3aaf00ae6231d89f9504bf2f685cecccf Mon Sep 17 00:00:00 2001 From: Florian Weingarten Date: Wed, 26 Jun 2013 22:30:33 -0400 Subject: [PATCH 2/8] Some tests for whitespace collapsing --- test/liquid/blank_test.rb | 67 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 test/liquid/blank_test.rb diff --git a/test/liquid/blank_test.rb b/test/liquid/blank_test.rb new file mode 100644 index 0000000..33feb8f --- /dev/null +++ b/test/liquid/blank_test.rb @@ -0,0 +1,67 @@ +require 'test_helper' + +class BlankTest < Test::Unit::TestCase + include Liquid + N = 10 + + def wrap_in_for(body) + "{% for i in (1..#{N}) %}#{body}{% endfor %}" + end + + def wrap_in_if(body) + "{% if true %}#{body}{% endif %}" + end + + def wrap(body) + wrap_in_for(body) + wrap_in_if(body) + end + + def test_loops_are_blank + assert_template_result("", wrap_in_for(" ")) + end + + def test_if_else_are_blank + assert_template_result("", "{% if true %} {% elsif false %} {% else %} {% endif %}") + end + + def test_unless_is_blank + assert_template_result("", wrap("{% unless true %} {% endunless %}")) + end + + def test_comments_are_blank + assert_template_result("", wrap(" {% comment %} whatever {% endcomment %} ")) + end + + def test_captures_are_blank + assert_template_result("", wrap(" {% capture foo %} whatever {% endcapture %} ")) + end + + def test_nested_blocks_are_blank_but_only_if_all_children_are + assert_template_result("", wrap(wrap(" "))) + assert_template_result("\n but this not "*(N+1), + wrap(%q{{% if true %} {% comment %} this is blank {% endcomment %} {% endif %} + {% if true %} but this not {% endif %}})) + end + + def test_assigns_are_blank + assert_template_result("", wrap(' {% assign foo = "bar" %} ')) + end + + def test_whitespace_is_blank + assert_template_result("", wrap(" ")) + assert_template_result("", wrap("\t")) + end + + def test_whitespace_is_not_blank_if_other_stuff_is_present + body = " x " + assert_template_result(body*(N+1), wrap(body)) + end + + def test_raw_is_not_blank + assert_template_result(" "*(N+1), wrap("{% raw %} {% endraw %}")) + end + + def test_variables_are_not_blank + assert_template_result(" "*(N+1), wrap(' {{ "" }} ')) + end +end From f01d0dbea603eed74f6dc86b0d1fc46a429c21a7 Mon Sep 17 00:00:00 2001 From: Florian Weingarten Date: Thu, 27 Jun 2013 13:39:04 +0200 Subject: [PATCH 3/8] More tests for whitespace collapsing --- test/liquid/blank_test.rb | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/test/liquid/blank_test.rb b/test/liquid/blank_test.rb index 33feb8f..e6e5224 100644 --- a/test/liquid/blank_test.rb +++ b/test/liquid/blank_test.rb @@ -28,6 +28,10 @@ class BlankTest < Test::Unit::TestCase assert_template_result("", wrap("{% unless true %} {% endunless %}")) end + def test_mark_as_blank_only_during_parsing + assert_template_result(" "*(N+1), wrap(" {% if false %} this never happens, but still, this block is not blank {% endif %}")) + end + def test_comments_are_blank assert_template_result("", wrap(" {% comment %} whatever {% endcomment %} ")) end @@ -38,9 +42,9 @@ class BlankTest < Test::Unit::TestCase def test_nested_blocks_are_blank_but_only_if_all_children_are assert_template_result("", wrap(wrap(" "))) - assert_template_result("\n but this not "*(N+1), + assert_template_result("\n but this is not "*(N+1), wrap(%q{{% if true %} {% comment %} this is blank {% endcomment %} {% endif %} - {% if true %} but this not {% endif %}})) + {% if true %} but this is not {% endif %}})) end def test_assigns_are_blank @@ -58,10 +62,11 @@ class BlankTest < Test::Unit::TestCase end def test_raw_is_not_blank - assert_template_result(" "*(N+1), wrap("{% raw %} {% endraw %}")) + assert_template_result(" "*(N+1), wrap(" {% raw %} {% endraw %}")) end def test_variables_are_not_blank assert_template_result(" "*(N+1), wrap(' {{ "" }} ')) + assert_template_result(" "*(N+1), wrap("{% assign foo = ' ' %}{{ foo }}")) end end From c16697746b23abbd9304c613aa70c8b29431c405 Mon Sep 17 00:00:00 2001 From: Florian Weingarten Date: Thu, 27 Jun 2013 13:56:33 +0200 Subject: [PATCH 4/8] Clean up whitespace collapsing a bit --- lib/liquid/block.rb | 7 +++---- lib/liquid/tag.rb | 4 ++-- lib/liquid/tags/capture.rb | 2 +- lib/liquid/tags/comment.rb | 2 +- lib/liquid/tags/increment.rb | 4 +++- test/liquid/blank_test.rb | 4 ++++ 6 files changed, 14 insertions(+), 9 deletions(-) diff --git a/lib/liquid/block.rb b/lib/liquid/block.rb index 022a5e5..261ae0f 100644 --- a/lib/liquid/block.rb +++ b/lib/liquid/block.rb @@ -7,8 +7,8 @@ module Liquid FullToken = /^#{TagStart}\s*(\w+)\s*(.*)?#{TagEnd}$/o ContentOfVariable = /^#{VariableStart}(.*)#{VariableEnd}$/o - def self.blank? - false + def blank? + @blank || false end def parse(tokens) @@ -25,14 +25,13 @@ module Liquid # proceed if block_delimiter == $1 end_tag - @blank = true if self.class.blank? return end # fetch the tag from registered blocks if tag = Template.tags[$1] new_tag = tag.new($1, $2, tokens) - @blank = false if new_tag.is_a?(Block) && !new_tag.blank + @blank = false unless new_tag.blank? @nodelist << new_tag else # this tag is not registered with the system diff --git a/lib/liquid/tag.rb b/lib/liquid/tag.rb index c672344..a4aac71 100644 --- a/lib/liquid/tag.rb +++ b/lib/liquid/tag.rb @@ -19,8 +19,8 @@ module Liquid '' end - def self.blank? - true + def blank? + @blank || true end end # Tag end # Tag diff --git a/lib/liquid/tags/capture.rb b/lib/liquid/tags/capture.rb index 76c464f..4f5b34c 100644 --- a/lib/liquid/tags/capture.rb +++ b/lib/liquid/tags/capture.rb @@ -31,7 +31,7 @@ module Liquid '' end - def self.blank? + def blank? true end end diff --git a/lib/liquid/tags/comment.rb b/lib/liquid/tags/comment.rb index 99e08c1..1eaf71f 100644 --- a/lib/liquid/tags/comment.rb +++ b/lib/liquid/tags/comment.rb @@ -4,7 +4,7 @@ module Liquid '' end - def self.blank? + def blank? true end end diff --git a/lib/liquid/tags/increment.rb b/lib/liquid/tags/increment.rb index e6a30ca..e5c31fe 100644 --- a/lib/liquid/tags/increment.rb +++ b/lib/liquid/tags/increment.rb @@ -28,7 +28,9 @@ module Liquid value.to_s end - private + def blank? + false + end end Template.register_tag('increment', Increment) diff --git a/test/liquid/blank_test.rb b/test/liquid/blank_test.rb index e6e5224..aa0f6a7 100644 --- a/test/liquid/blank_test.rb +++ b/test/liquid/blank_test.rb @@ -61,6 +61,10 @@ class BlankTest < Test::Unit::TestCase assert_template_result(body*(N+1), wrap(body)) end + def test_increment_is_not_blank + assert_template_result(" 0"*2*(N+1), wrap("{% assign foo = 0 %} {% increment foo %} {% decrement foo %}")) + end + def test_raw_is_not_blank assert_template_result(" "*(N+1), wrap(" {% raw %} {% endraw %}")) end From b4fbcea114437150c1ce5169db6d5f2523e86bed Mon Sep 17 00:00:00 2001 From: Florian Weingarten Date: Thu, 27 Jun 2013 14:00:38 +0200 Subject: [PATCH 5/8] Cycle tags are never blank --- lib/liquid/tags/cycle.rb | 6 +++++- test/liquid/blank_test.rb | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/liquid/tags/cycle.rb b/lib/liquid/tags/cycle.rb index 9b3f342..347426b 100644 --- a/lib/liquid/tags/cycle.rb +++ b/lib/liquid/tags/cycle.rb @@ -43,7 +43,11 @@ module Liquid result end end - + + def blank? + false + end + private def variables_from_string(markup) diff --git a/test/liquid/blank_test.rb b/test/liquid/blank_test.rb index aa0f6a7..bcc7999 100644 --- a/test/liquid/blank_test.rb +++ b/test/liquid/blank_test.rb @@ -65,6 +65,10 @@ class BlankTest < Test::Unit::TestCase assert_template_result(" 0"*2*(N+1), wrap("{% assign foo = 0 %} {% increment foo %} {% decrement foo %}")) end + def test_cycle_is_not_blank + assert_template_result("12"*((N+1)/2)+"1", wrap("{% cycle '1', '2' %}")) + end + def test_raw_is_not_blank assert_template_result(" "*(N+1), wrap(" {% raw %} {% endraw %}")) end From 668ee5e1c428852d19ff54b67fbd8739af90eff0 Mon Sep 17 00:00:00 2001 From: Florian Weingarten Date: Thu, 27 Jun 2013 14:03:38 +0200 Subject: [PATCH 6/8] Clean up whitespace collapsing --- lib/liquid/block.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/liquid/block.rb b/lib/liquid/block.rb index 261ae0f..b36bddc 100644 --- a/lib/liquid/block.rb +++ b/lib/liquid/block.rb @@ -1,7 +1,5 @@ module Liquid class Block < Tag - attr_reader :blank - IsTag = /^#{TagStart}/o IsVariable = /^#{VariableStart}/o FullToken = /^#{TagStart}\s*(\w+)\s*(.*)?#{TagEnd}$/o @@ -31,7 +29,7 @@ module Liquid # fetch the tag from registered blocks if tag = Template.tags[$1] new_tag = tag.new($1, $2, tokens) - @blank = false unless new_tag.blank? + @blank &&= new_tag.blank? @nodelist << new_tag else # this tag is not registered with the system @@ -48,7 +46,7 @@ module Liquid # pass else @nodelist << token - @blank = false unless token =~ /\A\s*\z/ + @blank &&= (token =~ /\A\s*\z/) end end @@ -121,7 +119,7 @@ module Liquid context.resource_limits[:reached] = true raise MemoryError.new("Memory limits exceeded") end - unless token.respond_to?(:blank) && token.blank + unless token.is_a?(Block) && token.blank? output << token_output end rescue MemoryError => e From b53601100f8ef56946c765bfbf3bc69d3cb0986d Mon Sep 17 00:00:00 2001 From: Florian Weingarten Date: Tue, 2 Jul 2013 13:57:27 -0400 Subject: [PATCH 7/8] Make sure include tags are never blank --- lib/liquid/tags/include.rb | 4 ++++ test/liquid/blank_test.rb | 13 +++++++++++++ 2 files changed, 17 insertions(+) diff --git a/lib/liquid/tags/include.rb b/lib/liquid/tags/include.rb index e19c373..42eff17 100644 --- a/lib/liquid/tags/include.rb +++ b/lib/liquid/tags/include.rb @@ -38,6 +38,10 @@ module Liquid def parse(tokens) end + def blank? + false + end + def render(context) partial = load_cached_partial(context) variable = context[@variable_name || @template_name[1..-2]] diff --git a/test/liquid/blank_test.rb b/test/liquid/blank_test.rb index bcc7999..0817d91 100644 --- a/test/liquid/blank_test.rb +++ b/test/liquid/blank_test.rb @@ -1,5 +1,11 @@ require 'test_helper' +class BlankTestFileSystem + def read_template_file(template_path, context) + template_path + end +end + class BlankTest < Test::Unit::TestCase include Liquid N = 10 @@ -77,4 +83,11 @@ class BlankTest < Test::Unit::TestCase assert_template_result(" "*(N+1), wrap(' {{ "" }} ')) assert_template_result(" "*(N+1), wrap("{% assign foo = ' ' %}{{ foo }}")) end + + def test_include_is_blank + Liquid::Template.file_system = BlankTestFileSystem.new + assert_equal "foobar"*(N+1), Template.parse(wrap("{% include 'foobar' %}")).render() + assert_equal " foobar "*(N+1), Template.parse(wrap("{% include ' foobar ' %}")).render() + assert_equal " ", Template.parse(" {% include ' ' %} ").render() + end end From 0f38fe359659e228da94cb93cb3c40e0c0b9bf62 Mon Sep 17 00:00:00 2001 From: Florian Weingarten Date: Tue, 2 Jul 2013 18:08:20 -0400 Subject: [PATCH 8/8] Add blank test for case tags --- lib/liquid/tags/case.rb | 3 --- lib/liquid/tags/if.rb | 6 ------ test/liquid/blank_test.rb | 5 +++++ 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/lib/liquid/tags/case.rb b/lib/liquid/tags/case.rb index 9150b15..a9b2316 100644 --- a/lib/liquid/tags/case.rb +++ b/lib/liquid/tags/case.rb @@ -62,7 +62,6 @@ module Liquid end def record_else_condition(markup) - if not markup.strip.empty? raise SyntaxError.new("Syntax Error in tag 'case' - Valid else condition: {% else %} (no parameters) ") end @@ -71,8 +70,6 @@ module Liquid block.attach(@nodelist) @blocks << block end - - end Template.register_tag('case', Case) diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb index 7f23dde..c7b55be 100644 --- a/lib/liquid/tags/if.rb +++ b/lib/liquid/tags/if.rb @@ -1,5 +1,4 @@ module Liquid - # If is the conditional block # # {% if user.admin %} @@ -10,7 +9,6 @@ module Liquid # # There are {% if count < 5 %} less {% else %} more {% endif %} items than you need. # - # class If < Block SyntaxHelp = "Syntax Error in tag 'if' - Valid syntax: if [expression]" Syntax = /(#{QuotedFragment})\s*([=!<>a-z_]+)?\s*(#{QuotedFragment})?/o @@ -18,9 +16,7 @@ module Liquid def initialize(tag_name, markup, tokens) @blocks = [] - push_block('if', markup) - super end @@ -71,8 +67,6 @@ module Liquid @blocks.push(block) @nodelist = block.attach(Array.new) end - - end Template.register_tag('if', If) diff --git a/test/liquid/blank_test.rb b/test/liquid/blank_test.rb index 0817d91..d948f6a 100644 --- a/test/liquid/blank_test.rb +++ b/test/liquid/blank_test.rb @@ -90,4 +90,9 @@ class BlankTest < Test::Unit::TestCase assert_equal " foobar "*(N+1), Template.parse(wrap("{% include ' foobar ' %}")).render() assert_equal " ", Template.parse(" {% include ' ' %} ").render() end + + def test_case_is_blank + assert_template_result("", wrap(" {% assign foo = 'bar' %} {% case foo %} {% when 'bar' %} {% when 'whatever' %} {% else %} {% endcase %} ")) + assert_template_result("", wrap(" {% assign foo = 'else' %} {% case foo %} {% when 'bar' %} {% when 'whatever' %} {% else %} {% endcase %} ")) + end end