From 700c58355c04595795b03ff29e727eac7200e928 Mon Sep 17 00:00:00 2001
From: Carlos Antonio da Silva <carlosantoniodasilva@gmail.com>
Date: Thu, 26 Apr 2012 22:21:47 -0300
Subject: [PATCH] Improve and cleanup a bit partial renderer

* Remove template assignment: there is no need for this assignment,
  given we are rendering a collection with possibly different templates,
  and a second call to render (with the same instance) would behave
  differently if the template is set.
* Remove segments array in favor of Array#map
* Use local vars whenever possible
* Cache local template keys, remove defaults from find_template
---
 .../action_view/renderer/abstract_renderer.rb |  4 +-
 .../action_view/renderer/partial_renderer.rb  | 71 ++++++++++---------
 2 files changed, 37 insertions(+), 38 deletions(-)

diff --git a/actionpack/lib/action_view/renderer/abstract_renderer.rb b/actionpack/lib/action_view/renderer/abstract_renderer.rb
index 52473cd222..72616b7463 100644
--- a/actionpack/lib/action_view/renderer/abstract_renderer.rb
+++ b/actionpack/lib/action_view/renderer/abstract_renderer.rb
@@ -14,12 +14,10 @@ module ActionView
     protected
 
     def extract_details(options)
-      details = {}
-      @lookup_context.registered_details.each do |key|
+      @lookup_context.registered_details.each_with_object({}) do |key, details|
         next unless value = options[key]
         details[key] = Array(value)
       end
-      details
     end
 
     def instrument(name, options={})
diff --git a/actionpack/lib/action_view/renderer/partial_renderer.rb b/actionpack/lib/action_view/renderer/partial_renderer.rb
index 87609fd5ff..9100545718 100644
--- a/actionpack/lib/action_view/renderer/partial_renderer.rb
+++ b/actionpack/lib/action_view/renderer/partial_renderer.rb
@@ -283,7 +283,7 @@ module ActionView
       return nil if @collection.blank?
 
       if @options.key?(:spacer_template)
-        spacer = find_template(@options[:spacer_template]).render(@view, @locals)
+        spacer = find_template(@options[:spacer_template], @locals.keys).render(@view, @locals)
       end
 
       result = @template ? collection_with_template : collection_without_template
@@ -291,11 +291,11 @@ module ActionView
     end
 
     def render_partial
-      locals, view, block = @locals, @view, @block
+      view, locals, block = @view, @locals, @block
       object, as = @object, @variable
 
       if !block && (layout = @options[:layout])
-        layout = find_template(layout, @locals.keys + [@variable])
+        layout = find_template(layout, @template_keys)
       end
 
       object ||= locals[as]
@@ -337,6 +337,7 @@ module ActionView
 
       if @path
         @variable, @variable_counter = retrieve_variable(@path)
+        @template_keys = retrieve_template_keys
       else
         paths.map! { |path| retrieve_variable(path).unshift(path) }
       end
@@ -358,62 +359,55 @@ module ActionView
     end
 
     def collection_from_object
-      if @object.respond_to?(:to_ary)
-        @object.to_ary
-      end
+      @object.to_ary if @object.respond_to?(:to_ary)
     end
 
     def find_partial
       if path = @path
-        locals = @locals.keys
-        locals << @variable
-        locals << @variable_counter if @collection
-        find_template(path, locals)
+        find_template(path, @template_keys)
       end
     end
 
-    def find_template(path=@path, locals=@locals.keys)
+    def find_template(path, locals)
       prefixes = path.include?(?/) ? [] : @lookup_context.prefixes
       @lookup_context.find_template(path, prefixes, true, locals, @details)
     end
 
     def collection_with_template
-      segments, locals, template = [], @locals, @template
+      view, locals, template = @view, @locals, @template
       as, counter = @variable, @variable_counter
 
       if layout = @options[:layout]
-        layout = find_template(layout, @locals.keys + [@variable, @variable_counter])
+        layout = find_template(layout, @template_keys)
       end
 
-      locals[counter] = -1
+      index = -1
+      @collection.map do |object|
+        locals[as]      = object
+        locals[counter] = (index += 1)
 
-      @collection.each do |object|
-        locals[counter] += 1
-        locals[as] = object
-
-        content = template.render(@view, locals)
-        content = layout.render(@view, locals) { content } if layout
-        segments << content
+        content = template.render(view, locals)
+        content = layout.render(view, locals) { content } if layout
+        content
       end
-
-      segments
     end
 
     def collection_without_template
-      segments, locals, collection_data = [], @locals, @collection_data
-      index, template, cache = -1, nil, {}
-      keys = @locals.keys
+      view, locals, collection_data = @view, @locals, @collection_data
+      cache = {}
+      keys  = @locals.keys
 
-      @collection.each_with_index do |object, i|
-        path, *data = collection_data[i]
-        template = (cache[path] ||= find_template(path, keys + data))
-        locals[data[0]] = object
-        locals[data[1]] = (index += 1)
-        segments << template.render(@view, locals)
+      index = -1
+      @collection.map do |object|
+        index += 1
+        path, as, counter = collection_data[index]
+
+        locals[as]      = object
+        locals[counter] = index
+
+        template = (cache[path] ||= find_template(path, keys + [as, counter]))
+        template.render(view, locals)
       end
-
-      @template = template
-      segments
     end
 
     def partial_path(object = @object)
@@ -453,6 +447,13 @@ module ActionView
       end
     end
 
+    def retrieve_template_keys
+      keys = @locals.keys
+      keys << @variable
+      keys << @variable_counter if @collection
+      keys
+    end
+
     def retrieve_variable(path)
       variable = @options.fetch(:as) { path[%r'_?(\w+)(\.\w+)*$', 1] }.try(:to_sym)
       variable_counter = :"#{variable}_counter" if @collection