More refactoring on the views side of rendering.

ea68fe59 · José Valim · 0a853809 · ea68fe59 · ea68fe59 · ea68fe59
8 changed file
--- a/actionpack/lib/abstract_controller/layouts.rb
+++ b/actionpack/lib/abstract_controller/layouts.rb
@@ -281,14 +281,9 @@ def _normalize_options(options)
      super
      if _include_layout?(options)
-        layout = options.key?(:layout) ? options[:layout] : :default
+        layout = options.key?(:layout) ? options.delete(:layout) : :default
        value = _layout_for_option(layout)
+        options[:layout] = (value =~ /\blayouts/ ? value : "layouts/#{value}") if value
-        # TODO Revisit this. Maybe we should pass a :layout_prefix?
-        options[:layout] = ((!value || value =~ /\blayouts/) ? value : "layouts/#{value}")
-        # TODO Revisit this. :layout with :partial from controllers are not the same as in views
-        options[:layout] = view_context._find_layout(options[:layout]) if options.key?(:partial)
      end
    end

--- a/actionpack/lib/action_view.rb
+++ b/actionpack/lib/action_view.rb
@@ -37,6 +37,7 @@ module ActionView
    autoload :Helpers
    autoload_under "render" do
+      autoload :Layouts
      autoload :Partials
      autoload :Rendering
    end

--- a/actionpack/lib/action_view/base.rb
+++ b/actionpack/lib/action_view/base.rb
@@ -173,7 +173,7 @@ class Base
    module Subclasses
    end
-    include Helpers, Rendering, Partials, ::ERB::Util
+    include Helpers, Rendering, Partials, Layouts, ::ERB::Util
    extend ActiveSupport::Memoizable

--- a/actionpack/lib/action_view/render/layouts.rb
+++ b/actionpack/lib/action_view/render/layouts.rb
+require 'active_support/core_ext/object/try'
+module ActionView
+  module Layouts
+    # You can think of a layout as a method that is called with a block. _layout_for
+    # returns the contents that are yielded to the layout. If the user calls yield
+    # :some_name, the block, by default, returns content_for(:some_name). If the user
+    # calls yield, the default block returns content_for(:layout).
+    #
+    # The user can override this default by passing a block to the layout.
+    #
+    # ==== Example
+    #
+    #   # The template
+    #   <% render :layout => "my_layout" do %>Content<% end %>
+    #
+    #   # The layout
+    #   <html><% yield %></html>
+    #
+    # In this case, instead of the default block, which would return content_for(:layout),
+    # this method returns the block that was passed in to render layout, and the response
+    # would be <html>Content</html>.
+    #
+    # Finally, the block can take block arguments, which can be passed in by yield.
+    #
+    # ==== Example
+    #
+    #   # The template
+    #   <% render :layout => "my_layout" do |customer| %>Hello <%= customer.name %><% end %>
+    #
+    #   # The layout
+    #   <html><% yield Struct.new(:name).new("David") %></html>
+    #
+    # In this case, the layout would receive the block passed into <tt>render :layout</tt>,
+    # and the Struct specified in the layout would be passed into the block. The result
+    # would be <html>Hello David</html>.
+    def _layout_for(name = nil, &block) #:nodoc:
+      if !block || name
+        @_content_for[name || :layout]
+      else
+        capture(&block)
+      end
+    end
+    # This is the method which actually finds the layout using details in the lookup
+    # context object. If no layout is found, it checkes if at least a layout with
+    # the given name exists across all details before raising the error.
+    def _find_layout(layout) #:nodoc:
+      begin
+        find(layout)
+      rescue ActionView::MissingTemplate => e
+        update_details(:formats => nil) do
+          raise unless template_lookup.exists?(layout)
+        end
+      end
+    end
+    # Contains the logic that actually renders the layout.
+    def _render_layout(layout, locals, &block) #:nodoc:
+      layout.render(self, locals){ |*name| _layout_for(*name, &block) }
+    end
+  end
+end
--- a/actionpack/lib/action_view/render/partials.rb
+++ b/actionpack/lib/action_view/render/partials.rb
@@ -174,9 +174,6 @@ module Partials
    class PartialRenderer
      PARTIAL_NAMES = Hash.new {|h,k| h[k] = {} }
-      TEMPLATES = Hash.new {|h,k| h[k] = {} }
-      attr_reader :template
      def initialize(view_context, options, block)
        @view           = view_context
@@ -225,6 +222,7 @@ def render
          if !@block && (layout = @options[:layout])
            content = @view._render_layout(find_template(layout), @locals){ content }
          end
          content
        end
      end
@@ -241,9 +239,9 @@ def render_collection
      end
      def collection_with_template(template = @template)
-        segments, locals, as = [], @locals, @options[:as] || template.variable_name
+        segments, locals, as, template = [], @locals, @options[:as] || @template.variable_name, @template
-        counter_name  = template.counter_name
+        counter_name = template.counter_name
        locals[counter_name] = -1
        @collection.each do |object|
@@ -253,7 +251,6 @@ def collection_with_template(template = @template)
          segments << template.render(@view, locals)
        end
-        @template = template
        segments
      end
@@ -274,7 +271,7 @@ def collection_without_template(collection_paths = @collection_paths)
      end
      def render_partial(object = @object)
-        locals, view = @locals, @view
+        locals, view, template = @locals, @view, @template
        object ||= locals[template.variable_name]
        locals[@options[:as] || template.variable_name] = object
@@ -285,6 +282,7 @@ def render_partial(object = @object)
      end
    private
      def collection
        if @object.respond_to?(:to_ary)
          @object
@@ -295,11 +293,7 @@ def collection
      def find_template(path=@path)
        return path unless path.is_a?(String)
+        prefix = @view.controller_path unless path.include?(?/)
-        if controller = @view.controller
-          prefix = controller.controller_path unless path.include?(?/)
-        end
        @view.find(path, prefix, true)
      end
@@ -315,21 +309,8 @@ def partial_path(object = @object)
      end
    end
-    def render_partial(options)
-      _evaluate_assigns_and_ivars
-      details = options[:_details]
-      # TODO This should happen automatically as well
-      self.formats = details[:formats] if details[:formats]
-      renderer = PartialRenderer.new(self, options, nil)
-      text = renderer.render
-      options[:_template] = renderer.template
-      text
-    end
    def _render_partial(options, &block) #:nodoc:
-      if defined? @renderer
+      if defined?(@renderer)
        @renderer.setup(options, block)
      else
        @renderer = PartialRenderer.new(self, options, block)

--- a/actionpack/lib/action_view/render/rendering.rb
+++ b/actionpack/lib/action_view/render/rendering.rb
@@ -15,17 +15,12 @@ module Rendering
    def render(options = {}, locals = {}, &block) #:nodoc:
      case options
      when Hash
-        layout = options[:layout]
-        options[:locals] ||= {}
        if block_given?
-          return safe_concat(_render_partial(options.merge(:partial => layout), &block))
+          content = _render_partial(options.merge(:partial => options[:layout]), &block)
-        elsif options.key?(:partial)
+          safe_concat(content)
-          return _render_partial(options)
+        else
+          _render(options)
        end
-        template = _determine_template(options)
-        _render_template(template, layout, :locals => options[:locals]) if template
      when :update
        update_page(&block)
      else
@@ -33,50 +28,24 @@ def render(options = {}, locals = {}, &block) #:nodoc:
      end
    end
-    # You can think of a layout as a method that is called with a block. _layout_for
-    # returns the contents that are yielded to the layout. If the user calls yield
-    # :some_name, the block, by default, returns content_for(:some_name). If the user
-    # calls yield, the default block returns content_for(:layout).
-    #
-    # The user can override this default by passing a block to the layout.
-    #
-    # ==== Example
-    #
-    #   # The template
-    #   <% render :layout => "my_layout" do %>Content<% end %>
-    #
-    #   # The layout
-    #   <html><% yield %></html>
-    #
-    # In this case, instead of the default block, which would return content_for(:layout),
-    # this method returns the block that was passed in to render layout, and the response
-    # would be <html>Content</html>.
-    #
-    # Finally, the block can take block arguments, which can be passed in by yield.
-    #
-    # ==== Example
-    #
-    #   # The template
-    #   <% render :layout => "my_layout" do |customer| %>Hello <%= customer.name %><% end %>
-    #
-    #   # The layout
-    #   <html><% yield Struct.new(:name).new("David") %></html>
-    #
-    # In this case, the layout would receive the block passed into <tt>render :layout</tt>,
-    # and the Struct specified in the layout would be passed into the block. The result
-    # would be <html>Hello David</html>.
-    def _layout_for(name = nil, &block)
-      return @_content_for[name || :layout] if !block_given? || name
-      capture(&block)
-    end
    # This is the API to render a ViewContext's template from a controller.
-    #
+    # TODO Review this name since it does not render only templates, but also
-    # Internal Options:
+    # partials, files and so forth.
-    # _template:: The Template object to render
+    def render_template(options, &block)
-    # _layout::   The layout, if any, to wrap the Template in
-    def render_template(options)
      _evaluate_assigns_and_ivars
+      # TODO Layout for partials should be handled here, because inside the
+      # partial renderer it looks for the layout as a partial.
+      if options.key?(:partial) && options[:layout]
+        options[:layout] = _find_layout(options[:layout])
+      end
+      _render(options, &block)
+    end
+    # This method holds the common render logic for both controllers and
+    # views rendering stacks.
+    def _render(options) #:nodoc:
      if options.key?(:partial)
        _render_partial(options)
      else
@@ -86,14 +55,13 @@ def render_template(options)
      end
    end
-    def _determine_template(options)
+    # Determine the template to be rendered using the given options.
+    def _determine_template(options) #:nodoc:
      if options.key?(:inline)
        handler = Template.handler_class_for_extension(options[:type] || "erb")
        Template.new(options[:inline], "inline template", handler, {})
      elsif options.key?(:text)
        Template::Text.new(options[:text], self.formats.try(:first))
-      elsif options.key?(:_template)
-        options[:_template]
      elsif options.key?(:file)
        find(options[:file], options[:_prefix])
      elsif options.key?(:template)
@@ -101,24 +69,16 @@ def _determine_template(options)
      end
    end
-    def _find_layout(layout)
+    # Renders the given template. An string representing the layout can be
-      begin
+    # supplied as well.
-        find(layout)
+    def _render_template(template, layout = nil, options = {}) #:nodoc:
-      rescue ActionView::MissingTemplate => e
-        update_details(:formats => nil) do
-          raise unless template_lookup.exists?(layout)
-        end
-      end
-    end
-    def _render_template(template, layout = nil, options = {})
      locals = options[:locals] || {}
      layout = _find_layout(layout) if layout
      ActiveSupport::Notifications.instrument("action_view.render_template",
        :identifier => template.identifier, :layout => layout.try(:identifier)) do
-        content = template.render(self, locals) {|*name| _layout_for(*name) }
+        content = template.render(self, locals) { |*name| _layout_for(*name) }
        @_content_for[:layout] = content
        if layout
@@ -130,8 +90,5 @@ def _render_template(template, layout = nil, options = {})
      end
    end
-    def _render_layout(layout, locals, &block)
-      layout.render(self, locals){ |*name| _layout_for(*name, &block) }
-    end
  end
 end
--- a/actionpack/test/abstract/layouts_test.rb
+++ b/actionpack/test/abstract/layouts_test.rb
@@ -8,41 +8,52 @@ class Base < AbstractController::Base
      include AbstractController::Rendering
      include AbstractController::Layouts
+      def _prefix
+        "template"
+      end
      self.view_paths = [ActionView::FixtureResolver.new(
-        "layouts/hello.erb"                     => "With String <%= yield %>",
-        "layouts/hello_override.erb"            => "With Override <%= yield %>",
        "abstract_controller_tests/layouts/with_string_implied_child.erb" =>
-                                                   "With Implied <%= yield %>",
+                                           "With Implied <%= yield %>",
-        "layouts/overwrite.erb"                  => "Overwrite <%= yield %>",
+        "layouts/hello.erb"             => "With String <%= yield %>",
-        "layouts/with_false_layout.erb"         => "False Layout <%= yield %>"
+        "layouts/hello_override.erb"    => "With Override <%= yield %>",
+        "layouts/overwrite.erb"         => "Overwrite <%= yield %>",
+        "layouts/with_false_layout.erb" => "False Layout <%= yield %>"
      )]
    end
    class Blank < Base
-      self.view_paths = []
+      self.view_paths = ActionView::FixtureResolver.new("template/index.erb" => "Hello blank!")
      def index
-        render :_template => ActionView::Template::Text.new("Hello blank!")
+        render 
      end
    end
    class WithString < Base
      layout "hello"
+      append_view_path ActionView::FixtureResolver.new(
+        "template/index.erb"             => "Hello string!",
+        "template/overwrite_default.erb" => "Hello string!",
+        "template/overwrite_false.erb"   => "Hello string!",
+        "template/overwrite_string.erb"  => "Hello string!"
+      )
      def index
-        render :_template => ActionView::Template::Text.new("Hello string!")
+        render
      end
      def overwrite_default
-        render :_template => ActionView::Template::Text.new("Hello string!"), :layout => :default
+        render :layout => :default
      end
      def overwrite_false
-        render :_template => ActionView::Template::Text.new("Hello string!"), :layout => false
+        render :layout => false
      end
      def overwrite_string
-        render :_template => ActionView::Template::Text.new("Hello string!"), :layout => "overwrite"
+        render :layout => "overwrite"
      end
      def overwrite_skip
@@ -70,18 +81,28 @@ class WithChildOfImplied < WithStringImpliedChild
    class WithProc < Base
      layout proc { |c| "overwrite" }
+      append_view_path ActionView::FixtureResolver.new(
+        "template/index.erb" => "Hello proc!"
+      )
      def index
-        render :_template => ActionView::Template::Text.new("Hello proc!")
+        render
      end
    end
    class WithSymbol < Base
      layout :hello
+      append_view_path ActionView::FixtureResolver.new(
+        "template/index.erb" => "Hello symbol!"
+      )
      def index
-        render :_template => ActionView::Template::Text.new("Hello symbol!")
+        render
      end
-    private  
+    private
      def hello
        "overwrite"
      end
@@ -89,11 +110,17 @@ def hello
    class WithSymbolReturningString < Base
      layout :no_hello
+      append_view_path ActionView::FixtureResolver.new(
+        "template/index.erb" => "Hello missing symbol!"
+      )
      def index
-        render :_template => ActionView::Template::Text.new("Hello missing symbol!")
+        render
      end
-    private  
+    private
      def no_hello
        nil
      end
@@ -101,19 +128,28 @@ def no_hello
    class WithSymbolReturningNil < Base
      layout :nilz
+      append_view_path ActionView::FixtureResolver.new(
+        "template/index.erb" => "Hello nilz!"
+      )
      def index
-        render :_template => ActionView::Template::Text.new("Hello nilz!")
+        render
      end
-      def nilz() end
+      def nilz
+      end
    end
    class WithSymbolReturningObj < Base
      layout :objekt
+      append_view_path ActionView::FixtureResolver.new(
+        "template/index.erb" => "Hello object!"
+      )
      def index
-        render :_template => ActionView::Template::Text.new("Hello nilz!")
+        render
      end
      def objekt
@@ -123,33 +159,49 @@ def objekt
    class WithSymbolAndNoMethod < Base
      layout :no_method
+      append_view_path ActionView::FixtureResolver.new(
+        "template/index.erb" => "Hello boom!"
+      )
      def index
-        render :_template => ActionView::Template::Text.new("Hello boom!")
+        render
      end
    end
    class WithMissingLayout < Base
      layout "missing"
+      append_view_path ActionView::FixtureResolver.new(
+        "template/index.erb" => "Hello missing!"
+      )
      def index
-        render :_template => ActionView::Template::Text.new("Hello missing!")
+        render
      end
    end
    class WithFalseLayout < Base
      layout false
+      append_view_path ActionView::FixtureResolver.new(
+        "template/index.erb" => "Hello false!"
+      )
      def index
-        render :_template => ActionView::Template::Text.new("Hello false!")
+        render
      end
    end
    class WithNilLayout < Base
      layout nil
+      append_view_path ActionView::FixtureResolver.new(
+        "template/index.erb" => "Hello nil!"
+      )
      def index
-        render :_template => ActionView::Template::Text.new("Hello nil!")
+        render
      end
    end

--- a/actionpack/test/abstract/render_test.rb
+++ b/actionpack/test/abstract/render_test.rb
@@ -15,7 +15,6 @@ def _prefix
        "renderer/default.erb" => "With Default",
        "renderer/string.erb" => "With String",
        "renderer/symbol.erb" => "With Symbol",
-        "renderer/template_name.erb" => "With Template Name",
        "string/with_path.erb" => "With String With Path",
        "some/file.erb" => "With File",
        "with_format.html.erb" => "With html format",
@@ -56,14 +55,6 @@ def symbol
        render :symbol
      end
-      def template_name
-        render :_template_name => :template_name
-      end
-      def object
-        render :_template => ActionView::Template::Text.new("With Object")
-      end
      def with_html_format
        render :template => "with_format", :format => :html
      end
@@ -127,16 +118,6 @@ def test_render_string_with_path
        assert_equal "With String With Path", @controller.response_body
      end
-      def test_render_template_name
-        @controller.process(:template_name)
-        assert_equal "With Template Name", @controller.response_body
-      end
-      def test_render_object
-        @controller.process(:object)
-        assert_equal "With Object", @controller.response_body
-      end
      def test_render_with_html_format
        @controller.process(:with_html_format)
        assert_equal "With html format", @controller.response_body