From a8ece12fe2ac7838407954453e0d31af6186a5db Mon Sep 17 00:00:00 2001 From: Ryan Bates Date: Tue, 19 Aug 2008 19:09:04 -0500 Subject: [PATCH] Return nil instead of a space when passing an empty collection or nil to 'render :partial' [#791 state:resolved] Signed-off-by: Joshua Peek --- actionpack/lib/action_controller/base.rb | 9 ++++++--- actionpack/lib/action_view/base.rb | 2 +- actionpack/lib/action_view/partials.rb | 2 +- actionpack/test/controller/render_test.rb | 2 +- actionpack/test/template/render_test.rb | 8 ++++++++ 5 files changed, 17 insertions(+), 6 deletions(-) diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 0fdbcbd26fba6..09414e77022a9 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -939,8 +939,7 @@ def render(options = nil, extra_options = {}, &block) #:doc: render_for_text(generator.to_s, options[:status]) elsif options[:nothing] - # Safari doesn't pass the headers of the return if the response is zero length - render_for_text(" ", options[:status]) + render_for_text(nil, options[:status]) else render_for_file(default_template_name, options[:status], true) @@ -1154,7 +1153,11 @@ def render_for_text(text = nil, status = nil, append_response = false) #:nodoc: response.body ||= '' response.body << text.to_s else - response.body = text.is_a?(Proc) ? text : text.to_s + response.body = case text + when Proc then text + when nil then " " # Safari doesn't pass the headers of the return if the response is zero length + else text.to_s + end end end diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 62a01b33dd1e1..46bacbcbc1b8c 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -263,7 +263,7 @@ def render(options = {}, local_assigns = {}, &block) #:nodoc: end elsif options[:file] render_file(options[:file], nil, options[:locals]) - elsif options[:partial] && options[:collection] + elsif options[:partial] && options.has_key?(:collection) render_partial_collection(options[:partial], options[:collection], options[:spacer_template], options[:locals], options[:as]) elsif options[:partial] render_partial(options[:partial], options[:object], options[:locals]) diff --git a/actionpack/lib/action_view/partials.rb b/actionpack/lib/action_view/partials.rb index b661a62677991..074ba5a2b5192 100644 --- a/actionpack/lib/action_view/partials.rb +++ b/actionpack/lib/action_view/partials.rb @@ -161,7 +161,7 @@ def render_partial(partial_path, object_assigns = nil, local_assigns = {}) #:nod end def render_partial_collection(partial_path, collection, partial_spacer_template = nil, local_assigns = {}, as = nil) #:nodoc: - return " " if collection.empty? + return nil if collection.blank? local_assigns = local_assigns ? local_assigns.clone : {} spacer = partial_spacer_template ? render(:partial => partial_spacer_template) : '' diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 1b9b12acc6dfa..3008f5ca03b13 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -329,7 +329,7 @@ def test_render_custom_code_rjs def test_render_text_with_nil get :render_text_with_nil assert_response 200 - assert_equal '', @response.body + assert_equal ' ', @response.body end def test_render_text_with_false diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index 31cfdce531c05..25bbc263ddc67 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -87,6 +87,14 @@ def test_render_partial_collection_without_as @view.render(:partial => "test/local_inspector", :collection => [ Customer.new("mary") ]) end + def test_render_partial_with_empty_collection_should_return_nil + assert_nil @view.render(:partial => "test/customer", :collection => []) + end + + def test_render_partial_with_nil_collection_should_return_nil + assert_nil @view.render(:partial => "test/customer", :collection => nil) + end + # TODO: The reason for this test is unclear, improve documentation def test_render_partial_and_fallback_to_layout assert_equal "Before (Josh)\n\nAfter", @view.render(:partial => "test/layout_for_partial", :locals => { :name => "Josh" })