From 1129a24caff9f1804c2bff6569c0cbd8598dfa86 Mon Sep 17 00:00:00 2001 From: Ryan Bates Date: Thu, 21 Aug 2008 21:02:10 -0500 Subject: Cleanup around partial rendering Signed-off-by: Joshua Peek --- actionpack/lib/action_controller/base.rb | 20 +----- actionpack/lib/action_controller/rescue.rb | 2 +- .../templates/rescues/diagnostics.erb | 4 +- .../templates/rescues/template_error.erb | 4 +- actionpack/lib/action_view/base.rb | 43 +++-------- actionpack/lib/action_view/partials.rb | 83 +++++++++++++--------- 6 files changed, 66 insertions(+), 90 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 082f09ff16..5887e1e3b4 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -780,9 +780,6 @@ module ActionController #:nodoc: # render :file => "/path/to/some/template.erb", :layout => true, :status => 404 # render :file => "c:/path/to/some/template.erb", :layout => true, :status => 404 # - # # Renders a template relative to the template root and chooses the proper file extension - # render :file => "some/template", :use_full_path => true - # # === Rendering text # # Rendering of text is usually used for tests or for rendering prepared content, such as a cache. By default, text @@ -913,21 +910,10 @@ module ActionController #:nodoc: response.content_type ||= Mime::JSON render_for_text(json, options[:status]) - elsif partial = options[:partial] - partial = default_template_name if partial == true + elsif options[:partial] + options[:partial] = default_template_name if options[:partial] == true add_variables_to_assigns - - if collection = options[:collection] - render_for_text( - @template.send!(:render_partial_collection, partial, collection, - options[:spacer_template], options[:locals], options[:as]), options[:status] - ) - else - render_for_text( - @template.send!(:render_partial, partial, - options[:object], options[:locals]), options[:status] - ) - end + render_for_text(@template.render(options), options[:status]) elsif options[:update] add_variables_to_assigns diff --git a/actionpack/lib/action_controller/rescue.rb b/actionpack/lib/action_controller/rescue.rb index 482ac7d7a4..a1a9d68a35 100644 --- a/actionpack/lib/action_controller/rescue.rb +++ b/actionpack/lib/action_controller/rescue.rb @@ -182,7 +182,7 @@ module ActionController #:nodoc: @template.instance_variable_set("@rescues_path", File.dirname(rescues_path("stub"))) @template.send!(:assign_variables_from_controller) - @template.instance_variable_set("@contents", @template.render(:file => template_path_for_local_rescue(exception), :use_full_path => false)) + @template.instance_variable_set("@contents", @template.render(:file => template_path_for_local_rescue(exception))) response.content_type = Mime::HTML render_for_file(rescues_path("layout"), response_code_for_rescue(exception)) diff --git a/actionpack/lib/action_controller/templates/rescues/diagnostics.erb b/actionpack/lib/action_controller/templates/rescues/diagnostics.erb index 385c6c1b09..b5483eccae 100644 --- a/actionpack/lib/action_controller/templates/rescues/diagnostics.erb +++ b/actionpack/lib/action_controller/templates/rescues/diagnostics.erb @@ -6,6 +6,6 @@ 
<%=h @exception.clean_message %>
-<%= render(:file => @rescues_path + "/_trace.erb", :use_full_path => false) %> +<%= render(:file => @rescues_path + "/_trace.erb") %> -<%= render(:file => @rescues_path + "/_request_and_response.erb", :use_full_path => false) %> +<%= render(:file => @rescues_path + "/_request_and_response.erb") %> diff --git a/actionpack/lib/action_controller/templates/rescues/template_error.erb b/actionpack/lib/action_controller/templates/rescues/template_error.erb index 4aecc68d18..76fa3df89d 100644 --- a/actionpack/lib/action_controller/templates/rescues/template_error.erb +++ b/actionpack/lib/action_controller/templates/rescues/template_error.erb @@ -15,7 +15,7 @@ <% @real_exception = @exception @exception = @exception.original_exception || @exception %> -<%= render(:file => @rescues_path + "/_trace.erb", :use_full_path => false) %> +<%= render(:file => @rescues_path + "/_trace.erb") %> <% @exception = @real_exception %> -<%= render(:file => @rescues_path + "/_request_and_response.erb", :use_full_path => false) %> +<%= render(:file => @rescues_path + "/_request_and_response.erb") %> diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 46bacbcbc1..cdbb71bdaa 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -239,7 +239,7 @@ module ActionView #:nodoc: local_assigns ||= {} if options.is_a?(String) - render_file(options, nil, local_assigns) + render(:file => options, :locals => local_assigns) elsif options == :update update_page(&block) elsif options.is_a?(Hash) @@ -262,13 +262,15 @@ module ActionView #:nodoc: end end elsif options[:file] - render_file(options[:file], nil, options[:locals]) - elsif options[:partial] && options.has_key?(:collection) - render_partial_collection(options[:partial], options[:collection], options[:spacer_template], options[:locals], options[:as]) + if options[:use_full_path] + ActiveSupport::Deprecation.warn("use_full_path option has been deprecated and has no affect.", caller) + end + + pick_template(options[:file]).render_template(self, options[:locals]) elsif options[:partial] - render_partial(options[:partial], options[:object], options[:locals]) + render_partial(options) elsif options[:inline] - render_inline(options[:inline], options[:locals], options[:type]) + InlineTemplate.new(options[:inline], options[:type]).render(self, options[:locals]) end end end @@ -345,35 +347,6 @@ module ActionView #:nodoc: memoize :pick_template private - # Renders the template present at template_path. The hash in local_assigns - # is made available as local variables. - def render_file(template_path, use_full_path = nil, local_assigns = {}) #:nodoc: - unless use_full_path == nil - ActiveSupport::Deprecation.warn("use_full_path option has been deprecated and has no affect.", caller) - end - - if defined?(ActionMailer) && defined?(ActionMailer::Base) && controller.is_a?(ActionMailer::Base) && - template_path.is_a?(String) && !template_path.include?("/") - raise ActionViewError, <<-END_ERROR - Due to changes in ActionMailer, you need to provide the mailer_name along with the template name. - - render "user_mailer/signup" - render :file => "user_mailer/signup" - - If you are rendering a subtemplate, you must now use controller-like partial syntax: - - render :partial => 'signup' # no mailer_name necessary - END_ERROR - end - - template = pick_template(template_path) - template.render_template(self, local_assigns) - end - - def render_inline(text, local_assigns = {}, type = nil) - InlineTemplate.new(text, type).render(self, local_assigns) - end - # Evaluate the local assigns and pushes them to the view. def evaluate_assigns unless @assigns_added diff --git a/actionpack/lib/action_view/partials.rb b/actionpack/lib/action_view/partials.rb index 074ba5a2b5..7f19532bc1 100644 --- a/actionpack/lib/action_view/partials.rb +++ b/actionpack/lib/action_view/partials.rb @@ -1,14 +1,15 @@ 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 - # the template for display. + # 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,24 +19,24 @@ 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 - # "advertiser/_ad.erb" and pass the local variable +ad+ to the template for display. + # 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 # - # The example of partial use describes a familiar pattern where a template needs to iterate over an array and render a sub - # template for each of the elements. This pattern has been implemented as a single method that accepts an array and renders - # a partial by the same name as the elements contained within. So the three-lined example in "Using partials" can be rewritten - # with a single line: + # The example of partial use describes a familiar pattern where a template needs to iterate over an array and + # render a sub template for each of the elements. This pattern has been implemented as a single method that + # accepts an array and renders a partial by the same name as the elements contained within. So the three-lined + # example in "Using partials" can be rewritten with a single line: # # <%= 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 - # example above, the template would be fed +ad_counter+. + # 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 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. + # 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 # @@ -47,8 +48,9 @@ module ActionView # # == Rendering partials with layouts # - # Partials can have their own layouts applied to them. These layouts are different than the ones that are specified globally - # for the entire action, but they work in a similar fashion. Imagine a list with two types of users: + # Partials can have their own layouts applied to them. These layouts are different than the ones that are + # specified globally for the entire action, but they work in a similar fashion. Imagine a list with two types + # of users: # # <%# app/views/users/index.html.erb &> # Here's the administrator: @@ -139,36 +141,51 @@ module ActionView extend ActiveSupport::Memoizable private - def render_partial(partial_path, object_assigns = nil, local_assigns = {}) #:nodoc: - local_assigns ||= {} + def render_partial(options = {}) #:nodoc: + local_assigns = options[:locals] || {} - case partial_path + case partial_path = options[:partial] when String, Symbol, NilClass - pick_template(find_partial_path(partial_path)).render_partial(self, object_assigns, local_assigns) + if options.has_key?(:collection) + render_partial_collection(options) + else + pick_template(find_partial_path(partial_path)).render_partial(self, options[:object], local_assigns) + end when ActionView::Helpers::FormBuilder builder_partial_path = partial_path.class.to_s.demodulize.underscore.sub(/_builder$/, '') - render_partial(builder_partial_path, object_assigns, (local_assigns || {}).merge(builder_partial_path.to_sym => partial_path)) + render_partial( + :partial => builder_partial_path, + :object => options[:object], + :locals => local_assigns.merge(builder_partial_path.to_sym => partial_path) + ) when Array, ActiveRecord::Associations::AssociationCollection, ActiveRecord::NamedScope::Scope if partial_path.any? - collection = partial_path - render_partial_collection(nil, collection, nil, local_assigns) + render_partial(:collection => partial_path, :locals => local_assigns) else "" end else - render_partial(ActionController::RecordIdentifier.partial_path(partial_path, controller.class.controller_path), partial_path, local_assigns) + object = partial_path + render_partial( + :partial => ActionController::RecordIdentifier.partial_path(object, controller.class.controller_path), + :object => object, + :locals => local_assigns + ) end end - def render_partial_collection(partial_path, collection, partial_spacer_template = nil, local_assigns = {}, as = nil) #:nodoc: - return nil if collection.blank? + def render_partial_collection(options = {}) #:nodoc: + return nil if options[:collection].blank? - local_assigns = local_assigns ? local_assigns.clone : {} - spacer = partial_spacer_template ? render(:partial => partial_spacer_template) : '' + partial = options[:partial] + spacer = options[:spacer_template] ? render(:partial => options[:spacer_template]) : '' + local_assigns = options[:locals] ? options[:locals].clone : {} + as = options[:as] index = 0 - collection.map do |object| - _partial_path ||= partial_path || ActionController::RecordIdentifier.partial_path(object, controller.class.controller_path) + options[:collection].map do |object| + _partial_path ||= partial || + ActionController::RecordIdentifier.partial_path(object, controller.class.controller_path) path = find_partial_path(_partial_path) template = pick_template(path) local_assigns[template.counter_name] = index @@ -178,7 +195,7 @@ module ActionView end.join(spacer) end - def find_partial_path(partial_path) + def find_partial_path(partial_path) #:nodoc: if partial_path.include?('/') File.join(File.dirname(partial_path), "_#{File.basename(partial_path)}") elsif respond_to?(:controller) -- cgit v1.2.3 From ba516b40f5867a68588c2fa5b6710dbfb97a12c6 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Thu, 21 Aug 2008 21:24:08 -0500 Subject: Tidy up pick partial template logic --- actionpack/lib/action_view/partials.rb | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/partials.rb b/actionpack/lib/action_view/partials.rb index 7f19532bc1..a91417293a 100644 --- a/actionpack/lib/action_view/partials.rb +++ b/actionpack/lib/action_view/partials.rb @@ -149,21 +149,14 @@ module ActionView if options.has_key?(:collection) render_partial_collection(options) else - pick_template(find_partial_path(partial_path)).render_partial(self, options[:object], local_assigns) + _pick_partial_template(partial_path).render_partial(self, options[:object], local_assigns) end when ActionView::Helpers::FormBuilder builder_partial_path = partial_path.class.to_s.demodulize.underscore.sub(/_builder$/, '') - render_partial( - :partial => builder_partial_path, - :object => options[:object], - :locals => local_assigns.merge(builder_partial_path.to_sym => partial_path) - ) + local_assigns.merge!(builder_partial_path.to_sym => partial_path) + render_partial(:partial => builder_partial_path, :object => options[:object], :locals => local_assigns) when Array, ActiveRecord::Associations::AssociationCollection, ActiveRecord::NamedScope::Scope - if partial_path.any? - render_partial(:collection => partial_path, :locals => local_assigns) - else - "" - end + partial_path.any? ? render_partial(:collection => partial_path, :locals => local_assigns) : "" else object = partial_path render_partial( @@ -186,8 +179,7 @@ module ActionView options[:collection].map do |object| _partial_path ||= partial || ActionController::RecordIdentifier.partial_path(object, controller.class.controller_path) - path = find_partial_path(_partial_path) - template = pick_template(path) + template = _pick_partial_template(_partial_path) local_assigns[template.counter_name] = index result = template.render_partial(self, object, local_assigns, as) index += 1 @@ -195,15 +187,17 @@ module ActionView end.join(spacer) end - def find_partial_path(partial_path) #:nodoc: + def _pick_partial_template(partial_path) #:nodoc: if partial_path.include?('/') - File.join(File.dirname(partial_path), "_#{File.basename(partial_path)}") + path = File.join(File.dirname(partial_path), "_#{File.basename(partial_path)}") elsif respond_to?(:controller) - "#{controller.class.controller_path}/_#{partial_path}" + path = "#{controller.class.controller_path}/_#{partial_path}" else - "_#{partial_path}" + path = "_#{partial_path}" end + + pick_template(path) end - memoize :find_partial_path + memoize :_pick_partial_template end end -- cgit v1.2.3 From 0096f5586987b720ca24c09103c9371f64ed26e5 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Thu, 21 Aug 2008 21:34:03 -0500 Subject: Removed template_public? because it will always be true since the default template is never a partial --- actionpack/lib/action_controller/base.rb | 6 +----- actionpack/lib/action_view/base.rb | 5 ----- 2 files changed, 1 insertion(+), 10 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 5887e1e3b4..91023cd774 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -1189,7 +1189,7 @@ module ActionController #:nodoc: elsif respond_to? :method_missing method_missing action_name default_render unless performed? - elsif template_exists? && template_public? + elsif template_exists? default_render else raise UnknownAction, "No action responded to #{action_name}. Actions: #{action_methods.sort.to_sentence}", caller @@ -1264,10 +1264,6 @@ module ActionController #:nodoc: @template.file_exists?(template_name) end - def template_public?(template_name = default_template_name) - @template.file_public?(template_name) - end - def template_exempt_from_layout?(template_name = default_template_name) template_name = @template.pick_template(template_name).to_s if @template @@exempt_from_layout.any? { |ext| template_name =~ ext } diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index cdbb71bdaa..a85e698c1f 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -275,11 +275,6 @@ module ActionView #:nodoc: end end - # Returns true is the file may be rendered implicitly. - def file_public?(template_path)#:nodoc: - template_path.split('/').last[0,1] != '_' - end - # The format to be used when choosing between multiple templates with # the same name but differing formats. See +Request#template_format+ # for more details. -- cgit v1.2.3