Fixed CaptureHelper#content_for to work with the optional content parameter instead of just the block #9434 [sandofsky/wildchild]

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

3 files changed, 30 insertions, 4 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index ee19d49c70..e32ba42b24 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Fixed CaptureHelper#content_for to work with the optional content parameter instead of just the block #9434 [sandofsky/wildchild].
+
 * Added Mime::Type.register_alias for dealing with different formats using the same mime type [DHH]. Example:
 
     class PostsController < ApplicationController
diff --git a/actionpack/lib/action_view/helpers/capture_helper.rb b/actionpack/lib/action_view/helpers/capture_helper.rb
index 9195641747..8a74ed04b2 100644
--- a/actionpack/lib/action_view/helpers/capture_helper.rb
+++ b/actionpack/lib/action_view/helpers/capture_helper.rb
@@ -93,7 +93,7 @@ module ActionView
       # That will place <script> tags for Prototype, Scriptaculous, and application.js (if it exists)
       # on the page; this technique is useful if you'll only be using these scripts in a few views.
       #
-      # Also, note that content_for concatenates the blocks it is given for a particular
+      # Note that content_for concatenates the blocks it is given for a particular
       # identifier in order. For example:
       #
       #   <% content_for :navigation do %>
@@ -109,7 +109,11 @@ module ActionView
       # Then, in another template or layout, this code would render both links in order:
       #
       #   <ul><%= yield :navigation %></ul>
-      #      
+      #
+      # Lastly, simple content can be passed as a parameter:
+      #
+      #   <% content_for :script, javascript_include_tag(:defaults) %>
+      #
       # WARNING: content_for is ignored in caches. So you shouldn't use it
       # for elements that will be fragment cached.
       #
@@ -118,7 +122,9 @@ module ActionView
       # would be avaiable as <tt><%= @content_for_footer %></tt>. The preferred usage is now
       # <tt><%= yield :footer %></tt>.
       def content_for(name, content = nil, &block)
-        eval "@content_for_#{name} = (@content_for_#{name} || '') + capture(&block)"
+        existing_content_for = instance_variable_get("@content_for_#{name}").to_s
+        new_content_for      = existing_content_for + (block_given? ? capture(&block) : content)
+        instance_variable_set("@content_for_#{name}", new_content_for)
       end
 
       private
diff --git a/actionpack/test/controller/capture_test.rb b/actionpack/test/controller/capture_test.rb
index 42ec2568da..7ec5f32a37 100644
--- a/actionpack/test/controller/capture_test.rb
+++ b/actionpack/test/controller/capture_test.rb
@@ -8,6 +8,14 @@ class CaptureController < ActionController::Base
     render :layout => "talk_from_action"
   end
 
+  def content_for_with_parameter
+    render :layout => "talk_from_action"
+  end
+  
+  def content_for_concatenated
+    render :layout => "talk_from_action"
+  end
+
   def erb_content_for
     render :layout => "talk_from_action"
   end
@@ -49,8 +57,18 @@ class CaptureTest < Test::Unit::TestCase
     assert_equal expected_content_for_output, @response.body
   end
 
+  def test_should_concatentate_content_for
+    get :content_for_concatenated
+    assert_equal expected_content_for_output, @response.body
+  end
+
   def test_erb_content_for
-    get :content_for
+    get :erb_content_for
+    assert_equal expected_content_for_output, @response.body
+  end
+
+  def test_should_set_content_for_with_parameter
+    get :content_for_with_parameter
     assert_equal expected_content_for_output, @response.body
   end