Merge pull request #321 from Shopify/move-tag-parse-out-of-initialize

Refactor to create tags with a parse class method instead of new.

History.md

@@ -3,6 +3,7 @@
 ## 3.0.0 / not yet released / branch "master"
 
 * ...
+* Tag#parse is called after initialize, which now takes options instead of tokens as the 3rd argument. See #321 [Dylan Thacker-Smith, dylanahsmith]
 * Raise `Liquid::ArgumentError` instead of `::ArgumentError` when filter has wrong number of arguments #309 [Bogdan Gusiev, bogdan]
 * Add a to_s default for liquid drops, see #306 [Adam Doeler, releod]
 * Add strip, lstrip, and rstrip to standard filters [Florian Weingarten, fw42]

lib/liquid/block.rb

@@ -31,7 +31,7 @@ module Liquid
 
             # fetch the tag from registered blocks
             if tag = Template.tags[$1]
-              new_tag = tag.new_with_options($1, $2, tokens, @options || {})
+              new_tag = tag.parse($1, $2, tokens, @options)
               @blank &&= new_tag.blank?
               @nodelist << new_tag
               @children << new_tag

lib/liquid/document.rb

@@ -1,9 +1,8 @@
 module Liquid
   class Document < Block
+    def self.parse(tokens, options={})
       # we don't need markup to open this block
-    def initialize(tokens, options = {})
+      super(nil, nil, tokens, options)
-      @options = options
-      parse(tokens)
     end
 
     # There isn't a real delimiter

lib/liquid/tag.rb

@@ -3,20 +3,20 @@ module Liquid
     attr_accessor :options
     attr_reader :nodelist, :warnings
 
-    def self.new_with_options(tag_name, markup, tokens, options)
+    class << self
-      # Forgive me Matz for I have sinned. I know this code is weird
+      def parse(tag_name, markup, tokens, options)
-      # but it was necessary to maintain API compatibility.
+        tag = new(tag_name, markup, options)
-      new_tag = self.allocate
+        tag.parse(tokens)
-      new_tag.options = options
+        tag
-      new_tag.send(:initialize, tag_name, markup, tokens)
-      new_tag
       end
 
-    def initialize(tag_name, markup, tokens)
+      private :new
+    end
+
+    def initialize(tag_name, markup, options)
       @tag_name   = tag_name
       @markup     = markup
-      @options    ||= {} # needs || because might be set before initialize
+      @options    = options
-      parse(tokens)
     end
 
     def parse(tokens)

lib/liquid/tags/assign.rb

@@ -11,15 +11,14 @@ module Liquid
   class Assign < Tag
     Syntax = /(#{VariableSignature}+)\s*=\s*(.*)\s*/o
 
-    def initialize(tag_name, markup, tokens)
+    def initialize(tag_name, markup, options)
+      super
       if markup =~ Syntax
         @to = $1
         @from = Variable.new($2)
       else
         raise SyntaxError.new options[:locale].t("errors.syntax.assign")
       end
-
-      super
     end
 
     def render(context)

lib/liquid/tags/capture.rb

@@ -14,14 +14,13 @@ module Liquid
   class Capture < Block
     Syntax = /(\w+)/
 
-    def initialize(tag_name, markup, tokens)
+    def initialize(tag_name, markup, options)
+      super
       if markup =~ Syntax
         @to = $1
       else
         raise SyntaxError.new(options[:locale].t("errors.syntax.capture"))
       end
-
-      super
     end
 
     def render(context)

lib/liquid/tags/case.rb

@@ -3,7 +3,8 @@ module Liquid
     Syntax     = /(#{QuotedFragment})/o
     WhenSyntax = /(#{QuotedFragment})(?:(?:\s+or\s+|\s*\,\s*)(#{QuotedFragment}.*))?/o
 
-    def initialize(tag_name, markup, tokens)
+    def initialize(tag_name, markup, options)
+      super
       @blocks = []
 
       if markup =~ Syntax
@@ -11,8 +12,6 @@ module Liquid
       else
         raise SyntaxError.new(options[:locale].t("errors.syntax.case"))
       end
-
-      super
     end
 
     def nodelist

lib/liquid/tags/cycle.rb

@@ -15,7 +15,8 @@ module Liquid
     SimpleSyntax = /\A#{QuotedFragment}+/o
     NamedSyntax  = /\A(#{QuotedFragment})\s*\:\s*(.*)/o
 
-    def initialize(tag_name, markup, tokens)
+    def initialize(tag_name, markup, options)
+      super
       case markup
       when NamedSyntax
         @variables = variables_from_string($2)
@@ -26,7 +27,6 @@ module Liquid
       else
         raise SyntaxError.new(options[:locale].t("errors.syntax.cycle"))
       end
-      super
     end
 
     def render(context)

lib/liquid/tags/decrement.rb

@@ -19,10 +19,9 @@ module Liquid
   #    Hello: -3
   #
   class Decrement < Tag
-    def initialize(tag_name, markup, tokens)
+    def initialize(tag_name, markup, options)
-      @variable = markup.strip
-
       super
+      @variable = markup.strip
     end
 
     def render(context)

lib/liquid/tags/for.rb

@@ -46,10 +46,10 @@ module Liquid
   class For < Block
     Syntax = /\A(#{VariableSegment}+)\s+in\s+(#{QuotedFragment}+)\s*(reversed)?/o
 
-    def initialize(tag_name, markup, tokens)
+    def initialize(tag_name, markup, options)
+      super
       parse_with_selected_parser(markup)
       @nodelist = @for_block = []
-      super
     end
 
     def nodelist

lib/liquid/tags/if.rb

@@ -14,10 +14,10 @@ module Liquid
     ExpressionsAndOperators = /(?:\b(?:\s?and\s?|\s?or\s?)\b|(?:\s*(?!\b(?:\s?and\s?|\s?or\s?)\b)(?:#{QuotedFragment}|\S+)\s*)+)/o
     BOOLEAN_OPERATORS = %w(and or)
 
-    def initialize(tag_name, markup, tokens)
+    def initialize(tag_name, markup, options)
+      super
       @blocks = []
       push_block('if', markup)
-      super
     end
 
     def nodelist

lib/liquid/tags/include.rb

@@ -17,7 +17,9 @@ module Liquid
   class Include < Tag
     Syntax = /(#{QuotedFragment}+)(\s+(?:with|for)\s+(#{QuotedFragment}+))?/o
 
-    def initialize(tag_name, markup, tokens)
+    def initialize(tag_name, markup, options)
+      super
+
       if markup =~ Syntax
 
         @template_name = $1
@@ -31,8 +33,6 @@ module Liquid
       else
         raise SyntaxError.new(options[:locale].t("errors.syntax.include"))
       end
-
-      super
     end
 
     def parse(tokens)

lib/liquid/tags/increment.rb

@@ -15,9 +15,9 @@ module Liquid
   #    Hello: 2
   #
   class Increment < Tag
-    def initialize(tag_name, markup, tokens)
+    def initialize(tag_name, markup, options)
-      @variable = markup.strip
       super
+      @variable = markup.strip
     end
 
     def render(context)

lib/liquid/tags/table_row.rb

@@ -2,7 +2,8 @@ module Liquid
   class TableRow < Block
     Syntax = /(\w+)\s+in\s+(#{QuotedFragment}+)/o
 
-    def initialize(tag_name, markup, tokens)
+    def initialize(tag_name, markup, options)
+      super
       if markup =~ Syntax
         @variable_name = $1
         @collection_name = $2
@@ -13,8 +14,6 @@ module Liquid
       else
         raise SyntaxError.new(options[:locale].t("errors.syntax.table_row"))
       end
-
-      super
     end
 
     def render(context)

lib/liquid/template.rb

@@ -72,7 +72,7 @@ module Liquid
     # Parse source code.
     # Returns self for easy chaining
     def parse(source, options = {})
-      @root = Document.new(tokenize(source), DEFAULT_OPTIONS.merge(options))
+      @root = Document.parse(tokenize(source), DEFAULT_OPTIONS.merge(options))
       @warnings = nil
       self
     end

performance/shopify/comment_form.rb

@@ -1,15 +1,15 @@
 class CommentForm < Liquid::Block
   Syntax = /(#{Liquid::VariableSignature}+)/
 
-  def initialize(tag_name, markup, tokens)
+  def initialize(tag_name, markup, options)
+    super
+
     if markup =~ Syntax
       @variable_name = $1
       @attributes = {}
     else
       raise SyntaxError.new("Syntax Error in 'comment_form' - Valid syntax: comment_form [article]")
     end
-
-    super
   end
 
   def render(context)

performance/shopify/paginate.rb

@@ -1,7 +1,9 @@
 class Paginate < Liquid::Block
   Syntax     = /(#{Liquid::QuotedFragment})\s*(by\s*(\d+))?/
 
-  def initialize(tag_name, markup, tokens)
+  def initialize(tag_name, markup, options)
+    super
+
     @nodelist = []
 
     if markup =~ Syntax
@@ -19,8 +21,6 @@ class Paginate < Liquid::Block
     else
       raise SyntaxError.new("Syntax Error in tag 'paginate' - Valid syntax: paginate [collection] by number")
     end
-
-    super
   end
 
   def render(context)

test/liquid/tags/standard_tag_test.rb

@@ -4,7 +4,7 @@ class StandardTagTest < Test::Unit::TestCase
   include Liquid
 
   def test_tag
-    tag = Tag.new('tag', [], [])
+    tag = Tag.parse('tag', [], [], {})
     assert_equal 'liquid::tag', tag.name
     assert_equal '', tag.render(Context.new)
   end