More Action View refactoring. Knock :erb default down a notch. Closes #10455.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@8374 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

actionpack/CHANGELOG

@@ -4,7 +4,7 @@
 
 * render :xml and :json preserve custom content types.  #10388 [jmettraux, Chu Yeow]
 
-* Refactor Action View template handlers.  #10437 [Josh Peek]
+* Refactor Action View template handlers.  #10437, #10455 [Josh Peek]
 
 * Fix DoubleRenderError message and leave out mention of returning false from filters.  Closes #10380 [Frederick Cheung]
 

actionpack/lib/action_controller/base.rb

@@ -862,7 +862,7 @@ def render(options = nil, &block) #:doc:
 
           elsif inline = options[:inline]
             add_variables_to_assigns
-            render_for_text(@template.render_template(options[:type] || :erb, inline, nil, options[:locals] || {}), options[:status])
+            render_for_text(@template.render_template(options[:type], inline, nil, options[:locals] || {}), options[:status])
 
           elsif action_name = options[:action]
             template = default_template_name(action_name.to_s)

actionpack/lib/action_view/base.rb

@@ -243,17 +243,17 @@ def self.register_template_handler(extension, klass)
       @@template_handlers[extension.to_sym] = klass
     end
 
+    def self.template_handler_extensions
+      @@template_handler_extensions ||= @@template_handlers.keys.map(&:to_s).sort
+    end
+
     def self.register_default_template_handler(extension, klass)
       register_template_handler(extension, klass)
       @@default_template_handlers = klass
     end
 
     def self.handler_for_extension(extension)
-      @@template_handlers[extension.to_sym] || @@default_template_handlers
-    end
-
-    def self.template_handler_extensions
-      @@template_handler_extensions ||= @@template_handlers.keys.map(&:to_s).sort
+      (extension && @@template_handlers[extension.to_sym]) || @@default_template_handlers
     end
 
     register_default_template_handler :erb, TemplateHandlers::ERB
@@ -332,7 +332,7 @@ def render(options = {}, old_local_assigns = {}, &block) #:nodoc:
       elsif options == :update
         update_page(&block)
       elsif options.is_a?(Hash)
-        options = options.reverse_merge(:type => :erb, :locals => {}, :use_full_path => true)
+        options = options.reverse_merge(:locals => {}, :use_full_path => true)
 
         if options[:layout]
           path, partial_name = partial_pieces(options.delete(:layout))
@@ -362,38 +362,13 @@ def render_template(template_extension, template, file_path = nil, local_assigns
       handler = self.class.handler_for_extension(template_extension)
 
       if template_handler_is_compilable?(handler)
-        compile_and_render_template(template_extension, template, file_path, local_assigns)
+        compile_and_render_template(handler, template, file_path, local_assigns)
       else
         template ||= read_template_file(file_path, template_extension) # Make sure that a lazyily-read template is loaded.
         delegate_render(handler, template, local_assigns)
       end
     end
 
-    # Render the provided template with the given local assigns. If the template has not been rendered with the provided
-    # local assigns yet, or if the template has been updated on disk, then the template will be compiled to a method.
-    #
-    # Either, but not both, of template and file_path may be nil. If file_path is given, the template
-    # will only be read if it has to be compiled.
-    #
-    def compile_and_render_template(extension, template = nil, file_path = nil, local_assigns = {}) #:nodoc:
-      # convert string keys to symbols if requested
-      local_assigns = local_assigns.symbolize_keys if @@local_assigns_support_string_keys
-
-      # compile the given template, if necessary
-      if compile_template?(template, file_path, local_assigns)
-        template ||= read_template_file(file_path, extension)
-        compile_template(extension, template, file_path, local_assigns)
-      end
-
-      # Get the method name for this template and run it
-      method_name = @@method_names[file_path || template]
-      evaluate_assigns
-
-      send(method_name, local_assigns) do |*name|
-        instance_variable_get "@content_for_#{name.first || 'layout'}"
-      end
-    end
-
     # Gets the full template path with base path for the given template_path and extension.
     #
     #   full_template_path('users/show', 'html.erb')
@@ -581,9 +556,7 @@ def template_changed_since?(file_name, compile_time)
       end
 
       # Method to create the source code for a given template.
-      def create_template_source(extension, template, render_symbol, locals)
-        # TODO: Have handler passed to this method because was already looked up in render_template
-        handler = self.class.handler_for_extension(extension)
+      def create_template_source(handler, template, render_symbol, locals)
         body = delegate_compile(handler, template)
 
         @@template_args[render_symbol] ||= {}
@@ -598,13 +571,13 @@ def create_template_source(extension, template, render_symbol, locals)
         "def #{render_symbol}(local_assigns)\n#{locals_code}#{body}\nend"
       end
 
-      def assign_method_name(extension, template, file_name)
+      def assign_method_name(handler, template, file_name)
         method_key = file_name || template
-        @@method_names[method_key] ||= compiled_method_name(extension, template, file_name)
+        @@method_names[method_key] ||= compiled_method_name(handler, template, file_name)
       end
 
-      def compiled_method_name(extension, template, file_name)
-        ['_run', extension, compiled_method_name_file_path_segment(file_name)].compact.join('_').to_sym
+      def compiled_method_name(handler, template, file_name)
+        ['_run', handler.to_s.demodulize.underscore, compiled_method_name_file_path_segment(file_name)].compact.join('_').to_sym
       end
 
       def compiled_method_name_file_path_segment(file_name)
@@ -619,25 +592,14 @@ def compiled_method_name_file_path_segment(file_name)
       end
 
       # Compile and evaluate the template's code
-      def compile_template(extension, template, file_name, local_assigns)
-        render_symbol = assign_method_name(extension, template, file_name)
-        render_source = create_template_source(extension, template, render_symbol, local_assigns.keys)
-
-        # TODO: Push line_offset logic into appropriate TemplateHandler class
-        line_offset = @@template_args[render_symbol].size
-        if extension
-          case extension.to_sym
-          when :builder, :rxml, :rjs
-            line_offset += 2
-          end
-        end
-        
+      def compile_template(handler, template, file_name, local_assigns)
+        render_symbol = assign_method_name(handler, template, file_name)
+        render_source = create_template_source(handler, template, render_symbol, local_assigns.keys)
+        line_offset   = @@template_args[render_symbol].size + handler.line_offset
+
         begin
-          unless file_name.blank?
-            CompiledTemplates.module_eval(render_source, file_name, -line_offset)
-          else
-            CompiledTemplates.module_eval(render_source, 'compiled-template', -line_offset)
-          end
+          file_name = 'compiled-template' if file_name.blank?
+          CompiledTemplates.module_eval(render_source, file_name, -line_offset)
         rescue Exception => e  # errors from template code
           if logger
             logger.debug "ERROR: compiling #{render_symbol} RAISED #{e}"
@@ -651,5 +613,30 @@ def compile_template(extension, template, file_name, local_assigns)
         @@compile_time[render_symbol] = Time.now
         # logger.debug "Compiled template #{file_name || template}\n  ==> #{render_symbol}" if logger
       end
+
+      # Render the provided template with the given local assigns. If the template has not been rendered with the provided
+      # local assigns yet, or if the template has been updated on disk, then the template will be compiled to a method.
+      #
+      # Either, but not both, of template and file_path may be nil. If file_path is given, the template
+      # will only be read if it has to be compiled.
+      #
+      def compile_and_render_template(handler, template = nil, file_path = nil, local_assigns = {}) #:nodoc:
+        # convert string keys to symbols if requested
+        local_assigns = local_assigns.symbolize_keys if @@local_assigns_support_string_keys
+
+        # compile the given template, if necessary
+        if compile_template?(template, file_path, local_assigns)
+          template ||= read_template_file(file_path, nil)
+          compile_template(handler, template, file_path, local_assigns)
+        end
+
+        # Get the method name for this template and run it
+        method_name = @@method_names[file_path || template]
+        evaluate_assigns
+
+        send(method_name, local_assigns) do |*name|
+          instance_variable_get "@content_for_#{name.first || 'layout'}"
+        end
+      end
   end
 end

actionpack/lib/action_view/template_handler.rb

 module ActionView
   class TemplateHandler
+    def self.line_offset
+      0
+    end
+
     def initialize(view)
       @view = view
     end

actionpack/lib/action_view/template_handlers/builder.rb

@@ -3,6 +3,10 @@
 module ActionView
   module TemplateHandlers
     class Builder < TemplateHandler
+      def self.line_offset
+        2
+      end
+
       def compile(template)
         content_type_handler = (@view.send!(:controller).respond_to?(:response) ? "controller.response" : "controller")
         "#{content_type_handler}.content_type ||= Mime::XML\n" +

actionpack/lib/action_view/template_handlers/rjs.rb

 module ActionView
   module TemplateHandlers
     class RJS < TemplateHandler
+      def self.line_offset
+        2
+      end
+
       def compile(template)
         "controller.response.content_type ||= Mime::JS\n" +
         "update_page do |page|\n#{template}\nend"

actionpack/test/template/compiled_templates_test.rb

@@ -95,11 +95,13 @@ def test_compile_time
     assert v.send(:compile_template?, nil, @b, {})
     assert v.send(:compile_template?, nil, @s, {}) unless windows
 
+    @handler = ActionView::Base.handler_for_extension(:rhtml)
+
     # All templates are rendered at t+2
     Time.expects(:now).times(windows ? 2 : 3).returns(t + 2.seconds)
-    v.compile_and_render_template(:rhtml, '', @a)
-    v.compile_and_render_template(:rhtml, '', @b)
-    v.compile_and_render_template(:rhtml, '', @s) unless windows
+    v.send(:compile_and_render_template, @handler, '', @a)
+    v.send(:compile_and_render_template, @handler, '', @b)
+    v.send(:compile_and_render_template, @handler, '', @s) unless windows
     a_n = v.method_names[@a]
     b_n = v.method_names[@b]
     s_n = v.method_names[@s] unless windows
@@ -117,9 +119,9 @@ def test_compile_time
     assert !v.send(:compile_template?, nil, @a, {})
     assert !v.send(:compile_template?, nil, @b, {})
     assert !v.send(:compile_template?, nil, @s, {}) unless windows
-    v.compile_and_render_template(:rhtml, '', @a)
-    v.compile_and_render_template(:rhtml, '', @b)
-    v.compile_and_render_template(:rhtml, '', @s)  unless windows
+    v.send(:compile_and_render_template, @handler, '', @a)
+    v.send(:compile_and_render_template, @handler, '', @b)
+    v.send(:compile_and_render_template, @handler, '', @s)  unless windows
     # none of the files have changed since last compile
     assert v.compile_time[a_n] < t + 3.seconds
     assert v.compile_time[b_n] < t + 3.seconds
@@ -142,9 +144,9 @@ def test_compile_time
 
     # Only the symlink template gets rendered at t+3
     Time.stubs(:now).returns(t + 3.seconds) unless windows
-    v.compile_and_render_template(:rhtml, '', @a)
-    v.compile_and_render_template(:rhtml, '', @b)
-    v.compile_and_render_template(:rhtml, '', @s) unless windows
+    v.send(:compile_and_render_template, @handler, '', @a)
+    v.send(:compile_and_render_template, @handler, '', @b)
+    v.send(:compile_and_render_template, @handler, '', @s) unless windows
     # the symlink has changed since last compile
     assert v.compile_time[a_n] < t + 3.seconds
     assert v.compile_time[b_n] < t + 3.seconds
@@ -167,9 +169,9 @@ def test_compile_time
     assert v.send(:compile_template?, nil, @s, {}) unless windows
 
     Time.expects(:now).times(windows ? 1 : 2).returns(t + 5.seconds)
-    v.compile_and_render_template(:rhtml, '', @a)
-    v.compile_and_render_template(:rhtml, '', @b)
-    v.compile_and_render_template(:rhtml, '', @s) unless windows
+    v.send(:compile_and_render_template, @handler, '', @a)
+    v.send(:compile_and_render_template, @handler, '', @b)
+    v.send(:compile_and_render_template, @handler, '', @s) unless windows
     # the file at the end of the symlink has changed since last compile
     # both the symlink and the file at the end of it should be recompiled
     assert v.compile_time[a_n] < t + 5.seconds