From ea68fe59c670dd5580f3aa34fdfa0eb89eb717d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 8 Mar 2010 14:46:57 +0100 Subject: More refactoring on the views side of rendering. --- actionpack/lib/abstract_controller/layouts.rb | 9 +- actionpack/lib/action_view.rb | 1 + actionpack/lib/action_view/base.rb | 2 +- actionpack/lib/action_view/render/layouts.rb | 64 ++++++++++++++ actionpack/lib/action_view/render/partials.rb | 33 ++----- actionpack/lib/action_view/render/rendering.rb | 95 ++++++-------------- actionpack/test/abstract/layouts_test.rb | 116 ++++++++++++++++++------- actionpack/test/abstract/render_test.rb | 19 ---- 8 files changed, 185 insertions(+), 154 deletions(-) create mode 100644 actionpack/lib/action_view/render/layouts.rb diff --git a/actionpack/lib/abstract_controller/layouts.rb b/actionpack/lib/abstract_controller/layouts.rb index 6ac3806149..2f9616124a 100644 --- a/actionpack/lib/abstract_controller/layouts.rb +++ b/actionpack/lib/abstract_controller/layouts.rb @@ -281,14 +281,9 @@ module AbstractController super if _include_layout?(options) - layout = options.key?(:layout) ? options[:layout] : :default + layout = options.key?(:layout) ? options.delete(:layout) : :default value = _layout_for_option(layout) - - # TODO Revisit this. Maybe we should pass a :layout_prefix? - options[:layout] = ((!value || value =~ /\blayouts/) ? value : "layouts/#{value}") - - # TODO Revisit this. :layout with :partial from controllers are not the same as in views - options[:layout] = view_context._find_layout(options[:layout]) if options.key?(:partial) + options[:layout] = (value =~ /\blayouts/ ? value : "layouts/#{value}") if value end end diff --git a/actionpack/lib/action_view.rb b/actionpack/lib/action_view.rb index f5035fe45a..f111950540 100644 --- a/actionpack/lib/action_view.rb +++ b/actionpack/lib/action_view.rb @@ -37,6 +37,7 @@ module ActionView autoload :Helpers autoload_under "render" do + autoload :Layouts autoload :Partials autoload :Rendering end diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 5acd2a8b82..656d73e32d 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -173,7 +173,7 @@ module ActionView #:nodoc: module Subclasses end - include Helpers, Rendering, Partials, ::ERB::Util + include Helpers, Rendering, Partials, Layouts, ::ERB::Util extend ActiveSupport::Memoizable diff --git a/actionpack/lib/action_view/render/layouts.rb b/actionpack/lib/action_view/render/layouts.rb new file mode 100644 index 0000000000..e1dbd3c120 --- /dev/null +++ b/actionpack/lib/action_view/render/layouts.rb @@ -0,0 +1,64 @@ +require 'active_support/core_ext/object/try' + +module ActionView + module Layouts + + # You can think of a layout as a method that is called with a block. _layout_for + # returns the contents that are yielded to the layout. If the user calls yield + # :some_name, the block, by default, returns content_for(:some_name). If the user + # calls yield, the default block returns content_for(:layout). + # + # The user can override this default by passing a block to the layout. + # + # ==== Example + # + # # The template + # <% render :layout => "my_layout" do %>Content<% end %> + # + # # The layout + # <% yield %> + # + # In this case, instead of the default block, which would return content_for(:layout), + # this method returns the block that was passed in to render layout, and the response + # would be Content. + # + # Finally, the block can take block arguments, which can be passed in by yield. + # + # ==== Example + # + # # The template + # <% render :layout => "my_layout" do |customer| %>Hello <%= customer.name %><% end %> + # + # # The layout + # <% yield Struct.new(:name).new("David") %> + # + # In this case, the layout would receive the block passed into render :layout, + # and the Struct specified in the layout would be passed into the block. The result + # would be Hello David. + def _layout_for(name = nil, &block) #:nodoc: + if !block || name + @_content_for[name || :layout] + else + capture(&block) + end + end + + # This is the method which actually finds the layout using details in the lookup + # context object. If no layout is found, it checkes if at least a layout with + # the given name exists across all details before raising the error. + def _find_layout(layout) #:nodoc: + begin + find(layout) + rescue ActionView::MissingTemplate => e + update_details(:formats => nil) do + raise unless template_lookup.exists?(layout) + end + end + end + + # Contains the logic that actually renders the layout. + def _render_layout(layout, locals, &block) #:nodoc: + layout.render(self, locals){ |*name| _layout_for(*name, &block) } + end + end +end diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb index eecc88a1e0..950c9d2cd8 100644 --- a/actionpack/lib/action_view/render/partials.rb +++ b/actionpack/lib/action_view/render/partials.rb @@ -174,9 +174,6 @@ module ActionView class PartialRenderer PARTIAL_NAMES = Hash.new {|h,k| h[k] = {} } - TEMPLATES = Hash.new {|h,k| h[k] = {} } - - attr_reader :template def initialize(view_context, options, block) @view = view_context @@ -225,6 +222,7 @@ module ActionView if !@block && (layout = @options[:layout]) content = @view._render_layout(find_template(layout), @locals){ content } end + content end end @@ -241,9 +239,9 @@ module ActionView end def collection_with_template(template = @template) - segments, locals, as = [], @locals, @options[:as] || template.variable_name + segments, locals, as, template = [], @locals, @options[:as] || @template.variable_name, @template - counter_name = template.counter_name + counter_name = template.counter_name locals[counter_name] = -1 @collection.each do |object| @@ -253,7 +251,6 @@ module ActionView segments << template.render(@view, locals) end - @template = template segments end @@ -274,7 +271,7 @@ module ActionView end def render_partial(object = @object) - locals, view = @locals, @view + locals, view, template = @locals, @view, @template object ||= locals[template.variable_name] locals[@options[:as] || template.variable_name] = object @@ -285,6 +282,7 @@ module ActionView end private + def collection if @object.respond_to?(:to_ary) @object @@ -295,11 +293,7 @@ module ActionView def find_template(path=@path) return path unless path.is_a?(String) - - if controller = @view.controller - prefix = controller.controller_path unless path.include?(?/) - end - + prefix = @view.controller_path unless path.include?(?/) @view.find(path, prefix, true) end @@ -315,21 +309,8 @@ module ActionView end end - def render_partial(options) - _evaluate_assigns_and_ivars - - details = options[:_details] - - # TODO This should happen automatically as well - self.formats = details[:formats] if details[:formats] - renderer = PartialRenderer.new(self, options, nil) - text = renderer.render - options[:_template] = renderer.template - text - end - def _render_partial(options, &block) #:nodoc: - if defined? @renderer + if defined?(@renderer) @renderer.setup(options, block) else @renderer = PartialRenderer.new(self, options, block) diff --git a/actionpack/lib/action_view/render/rendering.rb b/actionpack/lib/action_view/render/rendering.rb index c6d95e88e2..61fea6f49e 100644 --- a/actionpack/lib/action_view/render/rendering.rb +++ b/actionpack/lib/action_view/render/rendering.rb @@ -15,17 +15,12 @@ module ActionView def render(options = {}, locals = {}, &block) #:nodoc: case options when Hash - layout = options[:layout] - options[:locals] ||= {} - if block_given? - return safe_concat(_render_partial(options.merge(:partial => layout), &block)) - elsif options.key?(:partial) - return _render_partial(options) + content = _render_partial(options.merge(:partial => options[:layout]), &block) + safe_concat(content) + else + _render(options) end - - template = _determine_template(options) - _render_template(template, layout, :locals => options[:locals]) if template when :update update_page(&block) else @@ -33,50 +28,24 @@ module ActionView end end - # You can think of a layout as a method that is called with a block. _layout_for - # returns the contents that are yielded to the layout. If the user calls yield - # :some_name, the block, by default, returns content_for(:some_name). If the user - # calls yield, the default block returns content_for(:layout). - # - # The user can override this default by passing a block to the layout. - # - # ==== Example - # - # # The template - # <% render :layout => "my_layout" do %>Content<% end %> - # - # # The layout - # <% yield %> - # - # In this case, instead of the default block, which would return content_for(:layout), - # this method returns the block that was passed in to render layout, and the response - # would be Content. - # - # Finally, the block can take block arguments, which can be passed in by yield. - # - # ==== Example - # - # # The template - # <% render :layout => "my_layout" do |customer| %>Hello <%= customer.name %><% end %> - # - # # The layout - # <% yield Struct.new(:name).new("David") %> - # - # In this case, the layout would receive the block passed into render :layout, - # and the Struct specified in the layout would be passed into the block. The result - # would be Hello David. - def _layout_for(name = nil, &block) - return @_content_for[name || :layout] if !block_given? || name - capture(&block) - end - # This is the API to render a ViewContext's template from a controller. - # - # Internal Options: - # _template:: The Template object to render - # _layout:: The layout, if any, to wrap the Template in - def render_template(options) + # TODO Review this name since it does not render only templates, but also + # partials, files and so forth. + def render_template(options, &block) _evaluate_assigns_and_ivars + + # TODO Layout for partials should be handled here, because inside the + # partial renderer it looks for the layout as a partial. + if options.key?(:partial) && options[:layout] + options[:layout] = _find_layout(options[:layout]) + end + + _render(options, &block) + end + + # This method holds the common render logic for both controllers and + # views rendering stacks. + def _render(options) #:nodoc: if options.key?(:partial) _render_partial(options) else @@ -86,14 +55,13 @@ module ActionView end end - def _determine_template(options) + # Determine the template to be rendered using the given options. + def _determine_template(options) #:nodoc: if options.key?(:inline) handler = Template.handler_class_for_extension(options[:type] || "erb") Template.new(options[:inline], "inline template", handler, {}) elsif options.key?(:text) Template::Text.new(options[:text], self.formats.try(:first)) - elsif options.key?(:_template) - options[:_template] elsif options.key?(:file) find(options[:file], options[:_prefix]) elsif options.key?(:template) @@ -101,24 +69,16 @@ module ActionView end end - def _find_layout(layout) - begin - find(layout) - rescue ActionView::MissingTemplate => e - update_details(:formats => nil) do - raise unless template_lookup.exists?(layout) - end - end - end - - def _render_template(template, layout = nil, options = {}) + # Renders the given template. An string representing the layout can be + # supplied as well. + def _render_template(template, layout = nil, options = {}) #:nodoc: locals = options[:locals] || {} layout = _find_layout(layout) if layout ActiveSupport::Notifications.instrument("action_view.render_template", :identifier => template.identifier, :layout => layout.try(:identifier)) do - content = template.render(self, locals) {|*name| _layout_for(*name) } + content = template.render(self, locals) { |*name| _layout_for(*name) } @_content_for[:layout] = content if layout @@ -130,8 +90,5 @@ module ActionView end end - def _render_layout(layout, locals, &block) - layout.render(self, locals){ |*name| _layout_for(*name, &block) } - end end end diff --git a/actionpack/test/abstract/layouts_test.rb b/actionpack/test/abstract/layouts_test.rb index fcc91d03f1..65a50807fd 100644 --- a/actionpack/test/abstract/layouts_test.rb +++ b/actionpack/test/abstract/layouts_test.rb @@ -8,41 +8,52 @@ module AbstractControllerTests include AbstractController::Rendering include AbstractController::Layouts + def _prefix + "template" + end + self.view_paths = [ActionView::FixtureResolver.new( - "layouts/hello.erb" => "With String <%= yield %>", - "layouts/hello_override.erb" => "With Override <%= yield %>", "abstract_controller_tests/layouts/with_string_implied_child.erb" => - "With Implied <%= yield %>", - "layouts/overwrite.erb" => "Overwrite <%= yield %>", - "layouts/with_false_layout.erb" => "False Layout <%= yield %>" + "With Implied <%= yield %>", + "layouts/hello.erb" => "With String <%= yield %>", + "layouts/hello_override.erb" => "With Override <%= yield %>", + "layouts/overwrite.erb" => "Overwrite <%= yield %>", + "layouts/with_false_layout.erb" => "False Layout <%= yield %>" )] end class Blank < Base - self.view_paths = [] - + self.view_paths = ActionView::FixtureResolver.new("template/index.erb" => "Hello blank!") + def index - render :_template => ActionView::Template::Text.new("Hello blank!") + render end end class WithString < Base layout "hello" - + + append_view_path ActionView::FixtureResolver.new( + "template/index.erb" => "Hello string!", + "template/overwrite_default.erb" => "Hello string!", + "template/overwrite_false.erb" => "Hello string!", + "template/overwrite_string.erb" => "Hello string!" + ) + def index - render :_template => ActionView::Template::Text.new("Hello string!") + render end def overwrite_default - render :_template => ActionView::Template::Text.new("Hello string!"), :layout => :default + render :layout => :default end def overwrite_false - render :_template => ActionView::Template::Text.new("Hello string!"), :layout => false + render :layout => false end def overwrite_string - render :_template => ActionView::Template::Text.new("Hello string!"), :layout => "overwrite" + render :layout => "overwrite" end def overwrite_skip @@ -70,18 +81,28 @@ module AbstractControllerTests class WithProc < Base layout proc { |c| "overwrite" } + append_view_path ActionView::FixtureResolver.new( + "template/index.erb" => "Hello proc!" + ) + def index - render :_template => ActionView::Template::Text.new("Hello proc!") + render end end class WithSymbol < Base layout :hello - + + append_view_path ActionView::FixtureResolver.new( + "template/index.erb" => "Hello symbol!" + ) + def index - render :_template => ActionView::Template::Text.new("Hello symbol!") + render end - private + + private + def hello "overwrite" end @@ -89,11 +110,17 @@ module AbstractControllerTests class WithSymbolReturningString < Base layout :no_hello - + + append_view_path ActionView::FixtureResolver.new( + "template/index.erb" => "Hello missing symbol!" + ) + def index - render :_template => ActionView::Template::Text.new("Hello missing symbol!") + render end - private + + private + def no_hello nil end @@ -101,19 +128,28 @@ module AbstractControllerTests class WithSymbolReturningNil < Base layout :nilz - + + append_view_path ActionView::FixtureResolver.new( + "template/index.erb" => "Hello nilz!" + ) + def index - render :_template => ActionView::Template::Text.new("Hello nilz!") + render end - def nilz() end + def nilz + end end class WithSymbolReturningObj < Base layout :objekt - + + append_view_path ActionView::FixtureResolver.new( + "template/index.erb" => "Hello object!" + ) + def index - render :_template => ActionView::Template::Text.new("Hello nilz!") + render end def objekt @@ -123,33 +159,49 @@ module AbstractControllerTests class WithSymbolAndNoMethod < Base layout :no_method - + + append_view_path ActionView::FixtureResolver.new( + "template/index.erb" => "Hello boom!" + ) + def index - render :_template => ActionView::Template::Text.new("Hello boom!") + render end end class WithMissingLayout < Base layout "missing" - + + append_view_path ActionView::FixtureResolver.new( + "template/index.erb" => "Hello missing!" + ) + def index - render :_template => ActionView::Template::Text.new("Hello missing!") + render end end class WithFalseLayout < Base layout false - + + append_view_path ActionView::FixtureResolver.new( + "template/index.erb" => "Hello false!" + ) + def index - render :_template => ActionView::Template::Text.new("Hello false!") + render end end class WithNilLayout < Base layout nil + + append_view_path ActionView::FixtureResolver.new( + "template/index.erb" => "Hello nil!" + ) def index - render :_template => ActionView::Template::Text.new("Hello nil!") + render end end diff --git a/actionpack/test/abstract/render_test.rb b/actionpack/test/abstract/render_test.rb index db924633ca..e54d056666 100644 --- a/actionpack/test/abstract/render_test.rb +++ b/actionpack/test/abstract/render_test.rb @@ -15,7 +15,6 @@ module AbstractController "renderer/default.erb" => "With Default", "renderer/string.erb" => "With String", "renderer/symbol.erb" => "With Symbol", - "renderer/template_name.erb" => "With Template Name", "string/with_path.erb" => "With String With Path", "some/file.erb" => "With File", "with_format.html.erb" => "With html format", @@ -56,14 +55,6 @@ module AbstractController render :symbol end - def template_name - render :_template_name => :template_name - end - - def object - render :_template => ActionView::Template::Text.new("With Object") - end - def with_html_format render :template => "with_format", :format => :html end @@ -127,16 +118,6 @@ module AbstractController assert_equal "With String With Path", @controller.response_body end - def test_render_template_name - @controller.process(:template_name) - assert_equal "With Template Name", @controller.response_body - end - - def test_render_object - @controller.process(:object) - assert_equal "With Object", @controller.response_body - end - def test_render_with_html_format @controller.process(:with_html_format) assert_equal "With html format", @controller.response_body -- cgit v1.2.3