Merge pull request #218 from Shopify/dont_render_blank_blocks

Dont render blank blocks

lib/liquid/block.rb

@@ -1,17 +1,20 @@
 module Liquid
-
   class Block < Tag
     IsTag             = /^#{TagStart}/o
     IsVariable        = /^#{VariableStart}/o
     FullToken         = /^#{TagStart}\s*(\w+)\s*(.*)?#{TagEnd}$/o
     ContentOfVariable = /^#{VariableStart}(.*)#{VariableEnd}$/o
 
+    def blank?
+      @blank || false
+    end
+
     def parse(tokens)
+      @blank = true
       @nodelist ||= []
       @nodelist.clear
 
       while token = tokens.shift
-
         case token
         when IsTag
           if token =~ FullToken
@@ -25,7 +28,9 @@ module Liquid
 
             # 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 &&= 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 +41,12 @@ module Liquid
           end
         when IsVariable
           @nodelist << create_variable(token)
+          @blank = false
         when ''
           # pass
         else
           @nodelist << token
+          @blank &&= (token =~ /\A\s*\z/)
         end
       end
 
@@ -112,7 +119,9 @@ module Liquid
             context.resource_limits[:reached] = true
             raise MemoryError.new("Memory limits exceeded")
           end
-          output << token_output
+          unless token.is_a?(Block) && token.blank?
+            output << token_output
+          end
         rescue MemoryError => e
           raise e
         rescue ::StandardError => e

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 blank?
+      @blank || true
+    end
   end # Tag
-
 end # Liquid

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 blank?
+      true
+    end
   end
 
   Template.register_tag('capture', Capture)

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)

lib/liquid/tags/comment.rb

@@ -3,6 +3,10 @@ module Liquid
     def render(context)
       ''
     end
+
+    def blank?
+      true
+    end
   end
 
   Template.register_tag('comment', Comment)

lib/liquid/tags/cycle.rb

@@ -1,5 +1,4 @@
 module Liquid
-
   # Cycle is usually used within a loop to alternate between values, like colors or DOM classes.
   #
   #   {% for item in items %}
@@ -44,15 +43,17 @@ module Liquid
       end
     end
 
-    private
+    def blank?
+      false
+    end
 
+    private
     def variables_from_string(markup)
       markup.split(',').collect do |var|
         var =~ /\s*(#{QuotedFragment})\s*/o
         $1 ? $1 : nil
       end.compact
     end
-
   end
 
   Template.register_tag('cycle', Cycle)

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)

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]]

lib/liquid/tags/increment.rb

@@ -1,12 +1,11 @@
 module Liquid
-
   # increment is used in a place where one needs to insert a counter
   #     into a template, and needs the counter to survive across
   #     multiple instantiations of the template.
   #     (To achieve the survival, the application must keep the context)
   #
   #     if the variable does not exist, it is created with value 0.
-
+  #
   #   Hello: {% increment variable %}
   #
   # gives you:
@@ -18,7 +17,6 @@ module Liquid
   class Increment < Tag
     def initialize(tag_name, markup, tokens)
       @variable = markup.strip
-
       super
     end
 
@@ -28,7 +26,9 @@ module Liquid
       value.to_s
     end
 
-    private
+    def blank?
+      false
+    end
   end
 
   Template.register_tag('increment', Increment)

test/liquid/blank_test.rb

@@ -0,0 +1,98 @@
+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
+
+  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_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
+
+  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 is not "*(N+1),
+      wrap(%q{{% if true %} {% comment %} this is blank {% endcomment %} {% endif %}
+      {% if true %} but this is 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_increment_is_not_blank
+    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
+
+  def test_variables_are_not_blank
+    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
+
+  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