From 5ad52152117ecda1166359c499bcd03ae6be3365 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Fri, 9 Dec 2011 07:20:55 +0100 Subject: Deprecate implicit layout lookup in favor of inheriting the _layout config. --- actionpack/CHANGELOG.md | 13 +++ actionpack/lib/abstract_controller/layouts.rb | 118 +++++++++++++++++++------- actionpack/test/abstract/layouts_test.rb | 4 +- 3 files changed, 101 insertions(+), 34 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 473115dc6d..e00b4a64f1 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,5 +1,18 @@ ## Rails 3.2.0 (unreleased) ## +* Deprecated implied layout lookup in controllers whose parent had a explicit layout set: + + class ApplicationController + layout "application" + end + + class PostsController < ApplicationController + end + + In the example above, Posts controller will no longer automatically look up for a posts layout. + + If you need this functionality you could either remove `layout "application"` from ApplicationController or explicitly set it to nil in PostsController. *José Valim* + * Rails will now use your default layout (such as "layouts/application") when you specify a layout with `:only` and `:except` condition, and those conditions fail. *Prem Sichanugrist* For example, consider this snippet: diff --git a/actionpack/lib/abstract_controller/layouts.rb b/actionpack/lib/abstract_controller/layouts.rb index 8356d822da..8b6136d6ba 100644 --- a/actionpack/lib/abstract_controller/layouts.rb +++ b/actionpack/lib/abstract_controller/layouts.rb @@ -66,27 +66,40 @@ module AbstractController # == Inheritance Examples # # class BankController < ActionController::Base - # layout "bank_standard" + # # bank.html.erb exists + # + # class ExchangeController < BankController + # # exchange.html.erb exists + # + # class CurrencyController < BankController # # class InformationController < BankController + # layout "information" # - # class TellerController < BankController + # class TellerController < InformationController # # teller.html.erb exists # - # class TillController < TellerController + # class EmployeeController < InformationController + # # employee.html.erb exists + # layout nil # # class VaultController < BankController # layout :access_level_layout # - # class EmployeeController < BankController - # layout nil + # class TillController < BankController + # layout false + # + # In these examples, we have three implicit lookup scenrios: + # * The BankController uses the "bank" layout. + # * The ExchangeController uses the "exchange" layout. + # * The CurrencyController inherits the layout from BankController. # - # In these examples: - # * The InformationController uses the "bank_standard" layout, inherited from BankController. - # * The TellerController follows convention and uses +app/views/layouts/teller.html.erb+. - # * The TillController inherits the layout from TellerController and uses +teller.html.erb+ as well. + # However, when a layout is explicitly set, the explicitly set layout wins: + # * The InformationController uses the "information" layout, explicitly set. + # * The TellerController also uses the "information" layout, because the parent explicitly set it. + # * The EmployeeController uses the "employee" layout, because it set the layout to nil, resetting the parent configuration. # * The VaultController chooses a layout dynamically by calling the access_level_layout method. - # * The EmployeeController does not use a layout at all. + # * The TillController does not use a layout at all. # # == Types of layouts # @@ -126,6 +139,22 @@ module AbstractController # If no directory is specified for the template name, the template will by default be looked for in app/views/layouts/. # Otherwise, it will be looked up relative to the template root. # + # Setting the layout to nil forces it to be looked up in the filesystem and fallbacks to the parent behavior if none exists. + # Setting it to nil is useful to re-enable template lookup overriding a previous configuration set in the parent: + # + # class ApplicationController < ActionController::Base + # layout "application" + # end + # + # class PostsController < ApplicationController + # # Will use "application" layout + # end + # + # class CommentsController < ApplicationController + # # Will search for "comments" layout and fallback "application" layout + # layout nil + # end + # # == Conditional layouts # # If you have a layout that by default is applied to all the actions of a controller, you still have the option of rendering @@ -252,28 +281,53 @@ module AbstractController RUBY end - layout_definition = case defined?(@_layout) ? @_layout : nil - when String - @_layout.inspect - when Symbol - <<-RUBY - #{@_layout}.tap do |layout| - unless layout.is_a?(String) || !layout - raise ArgumentError, "Your layout method :#{@_layout} returned \#{layout}. It " \ - "should have returned a String, false, or nil" + if defined?(@_layout) + layout_definition = case @_layout + when String + @_layout.inspect + when Symbol + <<-RUBY + #{@_layout}.tap do |layout| + unless layout.is_a?(String) || !layout + raise ArgumentError, "Your layout method :#{@_layout} returned \#{layout}. It " \ + "should have returned a String, false, or nil" + end end - end - RUBY - when Proc - define_method :_layout_from_proc, &@_layout - "_layout_from_proc(self)" - when false - nil - when true - raise ArgumentError, "Layouts must be specified as a String, Symbol, false, or nil" - when nil - name_clause - end + RUBY + when Proc + define_method :_layout_from_proc, &@_layout + "_layout_from_proc(self)" + when false + nil + when true + raise ArgumentError, "Layouts must be specified as a String, Symbol, false, or nil" + when nil + name_clause + end + else + # Add a deprecation if the parent layout was explicitly set and the child + # still does a dynamic lookup. In next Rails release, we should @_layout + # to be inheritable so we can skip the child lookup if the parent explicitly + # set the layout. + parent = self.superclass.instance_variable_get(:@_layout) + @_layout = nil + inspect = parent.is_a?(Proc) ? parent.inspect : parent + + layout_definition = if parent.nil? + name_clause + elsif name + <<-RUBY + if template = lookup_context.find_all("#{_implied_layout_name}", #{prefixes.inspect}).first + ActiveSupport::Deprecation.warn 'Layout found at "#{_implied_layout_name}" for #{name} but parent controller ' \ + 'set layout to #{inspect.inspect}. Please explicitly set your layout to "#{_implied_layout_name}" ' \ + 'or set it to nil to force a dynamic lookup.' + template + else + super + end + RUBY + end + end self.class_eval <<-RUBY, __FILE__, __LINE__ + 1 def _layout @@ -283,8 +337,8 @@ module AbstractController #{name_clause} end end + private :_layout RUBY - self.class_eval { private :_layout } end end diff --git a/actionpack/test/abstract/layouts_test.rb b/actionpack/test/abstract/layouts_test.rb index a5382a730d..de6f42d826 100644 --- a/actionpack/test/abstract/layouts_test.rb +++ b/actionpack/test/abstract/layouts_test.rb @@ -258,7 +258,7 @@ module AbstractControllerTests test "when a child controller has an implied layout, use that layout and not the parent controller layout" do controller = WithStringImpliedChild.new - controller.process(:index) + assert_deprecated { controller.process(:index) } assert_equal "With Implied Hello string!", controller.response_body end @@ -271,7 +271,7 @@ module AbstractControllerTests test "when a grandchild has no layout specified, the child has an implied layout, and the " \ "parent has specified a layout, use the child controller layout" do controller = WithChildOfImplied.new - controller.process(:index) + assert_deprecated { controller.process(:index) } assert_equal "With Implied Hello string!", controller.response_body end -- cgit v1.2.3