More cleanup of ActionView and reduction in need for blocks in some c…

…ases: * only one of partial_name or :as will be available as a local * `object` is removed * Simplify _layout_for in most cases. * Remove <% render :partial do |args| %> * <% render :partial do %> still works fine
rails · Aug 15, 2009 · 9f5cd01 · 9f5cd01 · metaskills · Aug 23, 2009
1 parent 27adcd1
commit 9f5cd01
Show file tree

Hide file tree

Showing 9 changed files with 14 additions and 42 deletions.
diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb
@@ -223,16 +223,14 @@ def render_collection
       def collection_with_template(template)
         options = @options
 
-        segments, locals, as = [], @locals, options[:as] || :object
+        segments, locals, as = [], @locals, options[:as] || template.variable_name
 
-        variable_name = template.variable_name
         counter_name  = template.counter_name
         locals[counter_name] = -1
 
         collection.each do |object|
           locals[counter_name] += 1
-          locals[variable_name] = object
-          locals[as] = object if as
+          locals[as] = object
 
           segments << template.render(@view, locals)
         end
@@ -242,14 +240,13 @@ def collection_with_template(template)
       def collection_without_template
         options = @options
 
-        segments, locals, as = [], @locals, options[:as] || :object
+        segments, locals, as = [], @locals, options[:as]
         index, template = -1, nil
 
         collection.each do |object|
           template = find_template(partial_path(object))
           locals[template.counter_name] = (index += 1)
           locals[template.variable_name] = object
-          locals[as] = object if as
 
           segments << template.render(@view, locals)
         end
@@ -265,8 +262,8 @@ def render_template(template, object = @object)
         @locals[:object] = @locals[template.variable_name] = object
         @locals[@options[:as]] = object if @options[:as]
 
-        content = template.render(@view, @locals) do |*names|
-          @view._layout_for(names, &@block)
+        content = template.render(@view, @locals) do |*name|
+          @view._layout_for(*name, &@block)
         end
         return content if @block || !@options[:layout]
         find_template(@options[:layout]).render(@view, @locals) { content }

diff --git a/actionpack/lib/action_view/render/rendering.rb b/actionpack/lib/action_view/render/rendering.rb
@@ -70,21 +70,11 @@ def render(options = {}, locals = {}, &block) #:nodoc:
     # 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(names, &block)
+    def _layout_for(name = nil)
+      return @_content_for[name || :layout] if !block_given? || name
+
       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
+        return yield
       end
     end
 
@@ -93,7 +83,7 @@ def _render_inline(inline, layout, options)
       template = Template.new(options[:inline], "inline #{options[:inline].inspect}", handler, {})
       locals = options[:locals] || {}
       content = template.render(self, locals)
-      content = layout.render(self, locals) { content } if layout
+      content = layout.render(self, locals) {|*name| _layout_for(*name) } if layout
       content
     end
 
@@ -134,7 +124,7 @@ def _render_template(template, layout = nil, options = {}, partial = nil)
       if layout
         @_layout = layout.identifier
         logger.info("Rendering template within #{layout.identifier}") if logger
-        content = layout.render(self, locals)
+        content = layout.render(self, locals) {|*name| _layout_for(*name) }
       end
       content
     end

diff --git a/actionpack/lib/action_view/template/handler.rb b/actionpack/lib/action_view/template/handler.rb
@@ -29,10 +29,6 @@ def self.call(template)
       raise "Need to implement #{self.class.name}#call(template)"
     end
 
-    def initialize(view = nil)
-      @view = view
-    end
-
     def render(template, local_assigns)
       raise "Need to implement #{self.class.name}#render(template, local_assigns)"
     end

diff --git a/actionpack/lib/action_view/template/template.rb b/actionpack/lib/action_view/template/template.rb
@@ -28,7 +28,6 @@ def initialize(source, identifier, handler, details)
 
     def render(view, locals, &block)
       method_name = compile(locals, view)
-      block ||= proc {|*names| view._layout_for(names) }
       view.send(method_name, locals, &block)
     rescue Exception => e
       if e.is_a?(TemplateError)

diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb
@@ -486,10 +486,6 @@ def render_using_layout_around_block
     render :action => "using_layout_around_block"
   end
 
-  def render_using_layout_around_block_with_args
-    render :action => "using_layout_around_block_with_args"
-  end
-
   def render_using_layout_around_block_in_main_layout_and_within_content_for_layout
     render :action => "using_layout_around_block", :layout => "layouts/block_with_layout"
   end
@@ -1161,11 +1157,6 @@ def test_using_layout_around_block_in_main_layout_and_within_content_for_layout
     assert_equal "Before (Anthony)\nInside from first block in layout\nAfter\nBefore (David)\nInside from block\nAfter\nBefore (Ramm)\nInside from second block in layout\nAfter\n", @response.body
   end
 
-  def test_using_layout_around_block_with_args
-    get :render_using_layout_around_block_with_args
-    assert_equal "Before\narg1arg2\nAfter", @response.body
-  end
-
   def test_partial_only
     get :partial_only
     assert_equal "only partial", @response.body

diff --git a/actionpack/test/fixtures/test/_customer_with_var.erb b/actionpack/test/fixtures/test/_customer_with_var.erb
@@ -1 +1 @@
-<%= customer.name %> <%= customer.name %> <%= customer_with_var.name %>
+<%= customer.name %> <%= customer.name %> <%= customer.name %>
diff --git a/actionpack/test/fixtures/test/_hash_object.erb b/actionpack/test/fixtures/test/_hash_object.erb
@@ -1,2 +1,2 @@
 <%= hash_object[:first_name] %>
-<%= object[:first_name].reverse %>
+<%= hash_object[:first_name].reverse %>
diff --git a/actionpack/test/fixtures/test/using_layout_around_block_with_args.html.erb b/actionpack/test/fixtures/test/using_layout_around_block_with_args.html.erb
diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb
@@ -142,7 +142,7 @@ def test_render_partial_collection_as
   end
 
   def test_render_partial_collection_without_as
-    assert_equal "local_inspector,local_inspector_counter,object",
+    assert_equal "local_inspector,local_inspector_counter",
       @view.render(:partial => "test/local_inspector", :collection => [ Customer.new("mary") ])
   end