Modify various partial methods to carry along the block that can be passed in with render

  * _render_single_template, which renders a template without layout
  * _render_partial_unknown_type, which renders a partial of unknown type
    (the entry method for most partial rendering; supports strings, objects, and collections)
  * _render_partial_object, which renders a partial for a single object.
  * extracted _render_partial_path so it can be used to render the spacer without going
    through the public render :partial

3 files changed, 54 insertions, 44 deletions

diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb
index 8e094240d3..3141eb20d5 100644
--- a/actionpack/lib/action_view/render/partials.rb
+++ b/actionpack/lib/action_view/render/partials.rb
@@ -188,27 +188,33 @@ module ActionView
       _render_partial_unknown_type(options)
     end
 
-    def _render_partial_unknown_type(options) #:nodoc:
+    def _render_partial_unknown_type(options, &block) #:nodoc:
       options[:locals] ||= {}
 
       path = partial = options[:partial]
 
       if partial.respond_to?(:to_ary)
-        return _render_partial_collection(partial, options)
+        return _render_partial_collection(partial, options, &block)
       elsif !partial.is_a?(String)
         options[:object] = object = partial
         path = _partial_path(object)
       end
 
-      parts = [path, {:formats => formats}]
-      parts.push path.include?(?/) ? nil : controller_path
-      parts.push true
-
-      template = find_by_parts(*parts)
-      _render_partial_object(template, options)
+      _render_partial_path(path, options, &block)
     end
 
     private
+      def _render_partial_path(path, options, &block)
+        return '' if path.nil?
+
+        parts = [path, {:formats => formats}]
+        parts.push path.include?(?/) ? nil : controller_path
+        parts.push true
+
+        template = find_by_parts(*parts)
+        _render_partial_object(template, options, &block)
+      end
+
       def _partial_path(object)
         self.class._partial_names[[controller.class, object.class]] ||= begin
           name = object.class.model_name
@@ -220,14 +226,9 @@ module ActionView
         end
       end
 
-      def _render_partial(layout, options, block = nil)
-        if block
-          begin
-            @_proc_for_layout = block
-            concat(_render_partial_unknown_type(options.merge(:partial => layout)))
-          ensure
-            @_proc_for_layout = nil
-          end
+      def _render_partial(layout, options, &block)
+        if block_given?
+          concat(_render_partial_unknown_type(options.merge(:partial => layout), &block))
         else
           if layout
             prefix = layout.include?(?/) ? nil : controller_path
@@ -237,11 +238,11 @@ module ActionView
         end
       end
 
-      def _render_partial_object(template, options)
+      def _render_partial_object(template, options, &block)
         object = options[:object]
 
         if options.key?(:collection)
-          _render_partial_collection(options.delete(:collection), options, template)
+          _render_partial_collection(options.delete(:collection), options, template, &block)
         else
           locals = (options[:locals] ||= {})
           object ||= locals[:object] || locals[template.variable_name]
@@ -250,7 +251,7 @@ module ActionView
 
           options[:_template] = template
 
-          _render_single_template(template, locals)
+          _render_single_template(template, locals, &block)
         end
       end
 
@@ -259,26 +260,23 @@ module ActionView
         locals[options[:as]] = object if options[:as]
       end
 
-      def _render_partial_collection(collection, options = {}, passed_template = nil) #:nodoc:
+      def _render_partial_collection(collection, options = {}, template = nil, &block) #:nodoc:
         return nil if collection.blank?
 
-        spacer = options[:spacer_template] ?
-          _render_partial_unknown_type(:partial => options[:spacer_template]) : ''
+        spacer = _render_partial_path(options[:spacer_template], {})
+
+        locals, index = options[:locals] || {}, 0
 
-        locals = (options[:locals] ||= {})
-        index, @_partial_path = 0, nil
         collection.map do |object|
-          options[:_template] = template = passed_template || begin
-            _partial_path = _partial_path(object)
-            template = _pick_partial_template(_partial_path)
-          end
+          tmp = template || _pick_partial_template(_partial_path(object))
+          options[:_template] = tmp
 
-          _set_locals(object, locals, template, options)
-          locals[template.counter_name] = index
+          _set_locals(object, locals, tmp, options)
+          locals[tmp.counter_name] = index
 
           index += 1
 
-          _render_single_template(template, locals)
+          _render_single_template(tmp, locals, &block)
         end.join(spacer)
       end
 
@@ -286,6 +284,5 @@ module ActionView
         prefix = controller_path unless partial_path.include?('/')
         find_by_parts(partial_path, {:formats => formats}, prefix, true)
       end
-      memoize :_pick_partial_template
   end
 end
diff --git a/actionpack/lib/action_view/render/rendering.rb b/actionpack/lib/action_view/render/rendering.rb
index 0a5953be88..a721ade4e1 100644
--- a/actionpack/lib/action_view/render/rendering.rb
+++ b/actionpack/lib/action_view/render/rendering.rb
@@ -18,7 +18,7 @@ module ActionView
         layout = options[:layout]
             
         if options.key?(:partial) || block_given?
-          return _render_partial(layout, options, block)
+          return _render_partial(layout, options, &block)
         end
     
         layout = find_by_parts(layout, {:formats => formats}) if layout
@@ -56,10 +56,10 @@ module ActionView
       end
     end
 
-    # You can think of a layout as a method that is called with a block. This method
-    # returns the block that the layout is called with. 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).
+    # 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.
     #
@@ -88,15 +88,28 @@ module ActionView
     # 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_proc(name)
-      @_default_layout ||= proc { |*names| @_content_for[names.first || :layout] }
-      !@_content_for.key?(name) && @_proc_for_layout || @_default_layout
+    def _layout_for(names, &block)
+      with_output_buffer do
+        # This is due to the potentially ambiguous use of yield when
+        # a block is passed in to a template *and* there is a content_for()
+        # of the same name. Suggested solution: require explicit use of content_for
+        # in these ambiguous cases.
+        #
+        # We would be able to continue supporting yield in all non-ambiguous
+        # cases. Question: should we deprecate yield in favor of content_for
+        # and reserve yield for cases where there is a yield into a real block?
+        if @_content_for.key?(names.first) || !block_given?
+          return @_content_for[names.first || :layout]
+        else
+          return yield(names)
+        end
+      end
     end
 
-    def _render_single_template(template, locals = {})
+    def _render_single_template(template, locals = {}, &block)
       with_template(template) do
         template.render(self, locals) do |*names|
-          capture(*names, &layout_proc(names.first))
+          _layout_for(names, &block)
         end
       end
     rescue Exception => e
diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb
index 71a4a88afe..e51744d095 100644
--- a/actionpack/lib/action_view/test_case.rb
+++ b/actionpack/lib/action_view/test_case.rb
@@ -10,13 +10,13 @@ module ActionView
 
     attr_internal :rendered
     alias_method :_render_template_without_template_tracking, :_render_single_template
-    def _render_single_template(template, local_assigns = {})
+    def _render_single_template(template, local_assigns, &block)
       if template.respond_to?(:identifier) && template.present?
         @_rendered[:partials][template] += 1 if template.partial?
         @_rendered[:template] ||= []
         @_rendered[:template] << template
       end
-      _render_template_without_template_tracking(template, local_assigns)
+      _render_template_without_template_tracking(template, local_assigns, &block)
     end
   end