Deprecation: privatize deprecated render_partial and render_partial_collection methods.

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

4 files changed, 47 insertions, 50 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index b2921a49a8..15394b2b98 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,6 +1,6 @@
 *SVN*
 
-* Deprecation: remove deprecated request, redirect, and dependency methods. Remove deprecated instance variables. Remove uses_component_template_root for toplevel components directory.  [Jeremy Kemper]
+* Deprecation: remove deprecated request, redirect, and dependency methods. Remove deprecated instance variables. Remove uses_component_template_root for toplevel components directory. Privatize deprecated render_partial and render_partial_collection view methods.  [Jeremy Kemper]
 
 * Consistent public/protected/private visibility for chained methods.  #7813 [Dan Manges]
 
diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb
index 0ee7bf9529..9ce3c31de8 100755
--- a/actionpack/lib/action_controller/base.rb
+++ b/actionpack/lib/action_controller/base.rb
@@ -650,9 +650,6 @@ module ActionController #:nodoc:
       # Note that the partial filename must also be a valid Ruby variable name,
       # so e.g. 2005 and register-user are invalid.
       #
-      # _Deprecation_ _notice_: This used to have the signatures
-      # <tt>render_partial(partial_path = default_template_name, object = nil, local_assigns = {})</tt> and
-      # <tt>render_partial_collection(partial_name, collection, partial_spacer_template = nil, local_assigns = {})</tt>.
       #
       # == Automatic etagging
       #
@@ -907,12 +904,12 @@ module ActionController #:nodoc:
 
       def render_partial(partial_path = default_template_name, object = nil, local_assigns = nil, status = nil) #:nodoc:
         add_variables_to_assigns
-        render_text(@template.render_partial(partial_path, object, local_assigns), status)
+        render_text(@template.send(:render_partial, partial_path, object, local_assigns), status)
       end
 
       def render_partial_collection(partial_name, collection, partial_spacer_template = nil, local_assigns = nil, status = nil) #:nodoc:
         add_variables_to_assigns
-        render_text(@template.render_partial_collection(partial_name, collection, partial_spacer_template, local_assigns), status)
+        render_text(@template.send(:render_partial_collection, partial_name, collection, partial_spacer_template, local_assigns), status)
       end
 
       def render_with_layout(template_name = default_template_name, status = nil, layout = nil) #:nodoc:
diff --git a/actionpack/lib/action_view/partials.rb b/actionpack/lib/action_view/partials.rb
index c6049a4683..3d57574aaf 100644
--- a/actionpack/lib/action_view/partials.rb
+++ b/actionpack/lib/action_view/partials.rb
@@ -1,13 +1,13 @@
 module ActionView
-  # There's also a convenience method for rendering sub templates within the current controller that depends on a single object 
-  # (we call this kind of sub templates for partials). It relies on the fact that partials should follow the naming convention of being 
-  # prefixed with an underscore -- as to separate them from regular templates that could be rendered on their own. 
+  # There's also a convenience method for rendering sub templates within the current controller that depends on a single object
+  # (we call this kind of sub templates for partials). It relies on the fact that partials should follow the naming convention of being
+  # prefixed with an underscore -- as to separate them from regular templates that could be rendered on their own.
   #
   # In a template for Advertiser#account:
   #
   #  <%= render :partial => "account" %>
   #
-  # This would render "advertiser/_account.erb" and pass the instance variable @account in as a local variable +account+ to 
+  # This would render "advertiser/_account.erb" and pass the instance variable @account in as a local variable +account+ to
   # the template for display.
   #
   # In another template for Advertiser#buy, we could have:
@@ -18,7 +18,7 @@ module ActionView
   #     <%= render :partial => "ad", :locals => { :ad => ad } %>
   #   <% end %>
   #
-  # This would first render "advertiser/_account.erb" with @buyer passed in as the local variable +account+, then render 
+  # This would first render "advertiser/_account.erb" with @buyer passed in as the local variable +account+, then render
   # "advertiser/_ad.erb" and pass the local variable +ad+ to the template for display.
   #
   # == Rendering a collection of partials
@@ -31,12 +31,12 @@ module ActionView
   #   <%= render :partial => "ad", :collection => @advertisements %>
   #
   # This will render "advertiser/_ad.erb" and pass the local variable +ad+ to the template for display. An iteration counter
-  # will automatically be made available to the template with a name of the form +partial_name_counter+. In the case of the 
+  # will automatically be made available to the template with a name of the form +partial_name_counter+. In the case of the
   # example above, the template would be fed +ad_counter+.
   #
   # NOTE: Due to backwards compatibility concerns, the collection can't be one of hashes. Normally you'd also just keep domain objects,
   # like Active Records, in there.
-  # 
+  #
   # == Rendering shared partials
   #
   # Two controllers can share a set of partials and render them like this:
@@ -45,47 +45,47 @@ module ActionView
   #
   # This will render the partial "advertisement/_ad.erb" regardless of which controller this is being called from.
   module Partials
-    # Deprecated, use render :partial
-    def render_partial(partial_path, local_assigns = nil, deprecated_local_assigns = nil) #:nodoc:
-      path, partial_name = partial_pieces(partial_path)
-      object = extracting_object(partial_name, local_assigns, deprecated_local_assigns)
-      local_assigns = extract_local_assigns(local_assigns, deprecated_local_assigns)
-      local_assigns = local_assigns ? local_assigns.clone : {}
-      add_counter_to_local_assigns!(partial_name, local_assigns)
-      add_object_to_local_assigns!(partial_name, local_assigns, object)
+    private
+      # Deprecated, use render :partial
+      def render_partial(partial_path, local_assigns = nil, deprecated_local_assigns = nil) #:nodoc:
+        path, partial_name = partial_pieces(partial_path)
+        object = extracting_object(partial_name, local_assigns, deprecated_local_assigns)
+        local_assigns = extract_local_assigns(local_assigns, deprecated_local_assigns)
+        local_assigns = local_assigns ? local_assigns.clone : {}
+        add_counter_to_local_assigns!(partial_name, local_assigns)
+        add_object_to_local_assigns!(partial_name, local_assigns, object)
 
-      if logger
-        ActionController::Base.benchmark("Rendered #{path}/_#{partial_name}", Logger::DEBUG, false) do
+        if logger
+          ActionController::Base.benchmark("Rendered #{path}/_#{partial_name}", Logger::DEBUG, false) do
+            render("#{path}/_#{partial_name}", local_assigns)
+          end
+        else
           render("#{path}/_#{partial_name}", local_assigns)
         end
-      else
-        render("#{path}/_#{partial_name}", local_assigns)
       end
-    end
 
-    # Deprecated, use render :partial, :collection
-    def render_partial_collection(partial_name, collection, partial_spacer_template = nil, local_assigns = nil) #:nodoc:
-      collection_of_partials = Array.new
-      counter_name = partial_counter_name(partial_name)
-      local_assigns = local_assigns ? local_assigns.clone : {}
-      collection.each_with_index do |element, counter|
-        local_assigns[counter_name] = counter
-        collection_of_partials.push(render_partial(partial_name, element, local_assigns))
-      end
+      # Deprecated, use render :partial, :collection
+      def render_partial_collection(partial_name, collection, partial_spacer_template = nil, local_assigns = nil) #:nodoc:
+        collection_of_partials = Array.new
+        counter_name = partial_counter_name(partial_name)
+        local_assigns = local_assigns ? local_assigns.clone : {}
+        collection.each_with_index do |element, counter|
+          local_assigns[counter_name] = counter
+          collection_of_partials.push(render_partial(partial_name, element, local_assigns))
+        end
 
-      return " " if collection_of_partials.empty?
+        return " " if collection_of_partials.empty?
 
-      if partial_spacer_template
-        spacer_path, spacer_name = partial_pieces(partial_spacer_template)
-        collection_of_partials.join(render("#{spacer_path}/_#{spacer_name}"))
-      else
-        collection_of_partials.join
+        if partial_spacer_template
+          spacer_path, spacer_name = partial_pieces(partial_spacer_template)
+          collection_of_partials.join(render("#{spacer_path}/_#{spacer_name}"))
+        else
+          collection_of_partials.join
+        end
       end
-    end
-    
-    alias_method :render_collection_of_partials, :render_partial_collection
-    
-    private
+
+      alias_method :render_collection_of_partials, :render_partial_collection
+
       def partial_pieces(partial_path)
         if partial_path.include?('/')
           return File.dirname(partial_path), File.basename(partial_path)
@@ -97,7 +97,7 @@ module ActionView
       def partial_counter_name(partial_name)
         "#{partial_name.split('/').last}_counter".intern
       end
-      
+
       def extracting_object(partial_name, local_assigns, deprecated_local_assigns)
         if local_assigns.is_a?(Hash) || local_assigns.nil?
           controller.instance_variable_get("@#{partial_name}")
@@ -106,11 +106,11 @@ module ActionView
           local_assigns
         end
       end
-      
+
       def extract_local_assigns(local_assigns, deprecated_local_assigns)
         local_assigns.is_a?(Hash) ? local_assigns : deprecated_local_assigns
       end
-      
+
       def add_counter_to_local_assigns!(partial_name, local_assigns)
         counter_name = partial_counter_name(partial_name)
         local_assigns[counter_name] = 1 unless local_assigns.has_key?(counter_name)
diff --git a/actionpack/test/fixtures/test/list.erb b/actionpack/test/fixtures/test/list.erb
index cd0ab45d26..0a4bda58ee 100644
--- a/actionpack/test/fixtures/test/list.erb
+++ b/actionpack/test/fixtures/test/list.erb
@@ -1 +1 @@
-<%= @test_unchanged = 'goodbye' %><%= render_collection_of_partials "customer", @customers %><%= @test_unchanged %>
+<%= @test_unchanged = 'goodbye' %><%= render :partial => 'customer', :collection => @customers %><%= @test_unchanged %>