Cleaning up and documenting AbstractController::Layouts

5 files changed, 255 insertions, 43 deletions

diff --git a/actionpack/lib/action_controller/abstract/layouts.rb b/actionpack/lib/action_controller/abstract/layouts.rb
index 273063f74b..9ff8e9beb1 100644
--- a/actionpack/lib/action_controller/abstract/layouts.rb
+++ b/actionpack/lib/action_controller/abstract/layouts.rb
@@ -5,16 +5,26 @@ module AbstractController
     include Renderer
 
     included do
-      extlib_inheritable_accessor :_layout_conditions
-      self._layout_conditions = {}
+      extlib_inheritable_accessor(:_layout_conditions) { Hash.new }
     end
 
     module ClassMethods
+      # Specify the layout to use for this class.
+      #
+      # If the specified layout is a:
+      # String:: the String is the template name
+      # Symbol:: call the method specified by the symbol, which will return
+      #   the template name
+      # false::  There is no layout
+      # true::   raise an ArgumentError
+      #
+      # ==== Parameters
+      # layout<String, Symbol, false)>:: The layout to use.
+      #
+      # ==== Options (conditions)
+      # :only<#to_s, Array[#to_s]>:: A list of actions to apply this layout to.
+      # :except<#to_s, Array[#to_s]>:: Apply this layout to all actions but this one
       def layout(layout, conditions = {})
-        unless [String, Symbol, FalseClass, NilClass].include?(layout.class)
-          raise ArgumentError, "Layouts must be specified as a String, Symbol, false, or nil"
-        end
-
         conditions.each {|k, v| conditions[k] = Array(v).map {|a| a.to_s} }
         self._layout_conditions = conditions
 
@@ -22,6 +32,11 @@ module AbstractController
         _write_layout_method
       end
 
+      # If no layout is supplied, look for a template named the return
+      # value of this method.
+      #
+      # ==== Returns
+      # String:: A template name
       def _implied_layout_name
         name.underscore
       end
@@ -29,23 +44,31 @@ module AbstractController
       # Takes the specified layout and creates a _layout method to be called
       # by _default_layout
       #
-      # If the specified layout is a:
-      # String:: return the string
-      # Symbol:: call the method specified by the symbol
-      # false::  return nil
-      # none::   If a layout is found in the view paths with the controller's
-      #          name, return that string. Otherwise, use the superclass'
-      #          layout (which might also be implied)
+      # If there is no explicit layout specified:
+      # If a layout is found in the view paths with the controller's
+      # name, return that string. Otherwise, use the superclass'
+      # layout (which might also be implied)
       def _write_layout_method
         case @_layout
         when String
           self.class_eval %{def _layout(details) #{@_layout.inspect} end}
         when Symbol
-          self.class_eval %{def _layout(details) #{@_layout} end}
+          self.class_eval <<-ruby_eval, __FILE__, __LINE__ + 1
+            def _layout(details)
+              #{@_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_eval
         when false
           self.class_eval %{def _layout(details) end}
-        else
-          self.class_eval %{
+        when true
+          raise ArgumentError, "Layouts must be specified as a String, Symbol, false, or nil"
+        when nil
+          self.class_eval <<-ruby_eval, __FILE__, __LINE__ + 1
             def _layout(details)
               if view_paths.find_by_parts?("#{_implied_layout_name}", details, "layouts")
                 "#{_implied_layout_name}"
@@ -53,33 +76,54 @@ module AbstractController
                 super
               end
             end
-          }
+          ruby_eval
         end
       end
     end
 
   private
-    # This will be overwritten
-    def _layout(details)
-    end
+    # This will be overwritten by _write_layout_method
+    def _layout(details) end
 
-    # :api: plugin
-    # ====
-    # Override this to mutate the inbound layout name
+    # Determine the layout for a given name and details.
+    #
+    # ==== Parameters
+    # name<String>:: The name of the template
+    # details<Hash{Symbol => Object}>:: A list of details to restrict
+    #   the lookup to. By default, layout lookup is limited to the
+    #   formats specified for the current request.
     def _layout_for_name(name, details = {:formats => formats})
-      unless [String, FalseClass, NilClass].include?(name.class)
-        raise ArgumentError, "String, false, or nil expected; you passed #{name.inspect}"
-      end
-
-      name && view_paths.find_by_parts(name, details, _layout_prefix(name))
+      name && _find_by_parts(name, details)
     end
 
-    # TODO: Decide if this is the best hook point for the feature
-    def _layout_prefix(name)
-      "layouts"
+    # Take in the name and details and find a Template.
+    #
+    # ==== Parameters
+    # name<String>:: The name of the template to retrieve
+    # details<Hash>:: A list of details to restrict the search by. This
+    #   might include details like the format or locale of the template.
+    #
+    # ==== Returns
+    # Template:: A template object matching the name and details
+    def _find_by_parts(name, details)
+      # TODO: Make prefix actually part of details in ViewPath#find_by_parts
+      prefix = details.key?(:prefix) ? details.delete(:prefix) : "layouts"
+      view_paths.find_by_parts(name, details, prefix)
     end
 
-    def _default_layout(require_layout = false, details = {:formats => formats})
+    # Returns the default layout for this controller and a given set of details. 
+    # Optionally raises an exception if the layout could not be found.
+    #
+    # ==== Parameters
+    # details<Hash>:: A list of details to restrict the search by. This
+    #   might include details like the format or locale of the template.
+    # require_layout<Boolean>:: If this is true, raise an ArgumentError
+    #   with details about the fact that the exception could not be
+    #   found (defaults to false)
+    #
+    # ==== Returns
+    # Template:: The template object for the default layout (or nil)
+    def _default_layout(details, require_layout = false)
       if require_layout && _action_has_layout? && !_layout(details)
         raise ArgumentError,
           "There was no default layout for #{self.class} in #{view_paths.inspect}"
@@ -93,6 +137,12 @@ module AbstractController
       end
     end
 
+    # Determines whether the current action has a layout by checking the
+    # action name against the :only and :except conditions set on the
+    # layout.
+    #
+    # ==== Returns
+    # Boolean:: True if the action has a layout, false otherwise.
     def _action_has_layout?
       conditions = _layout_conditions
       if only = conditions[:only]
diff --git a/actionpack/lib/action_controller/new_base/compatibility.rb b/actionpack/lib/action_controller/new_base/compatibility.rb
index f278c2da14..29ba43a879 100644
--- a/actionpack/lib/action_controller/new_base/compatibility.rb
+++ b/actionpack/lib/action_controller/new_base/compatibility.rb
@@ -114,8 +114,9 @@ module ActionController
       super || (respond_to?(:method_missing) && "_handle_method_missing")
     end
 
-    def _layout_prefix(name)
-      super unless name =~ /\blayouts/
+    def _find_by_parts(name, details)
+      details[:prefix] = nil if name =~ /\blayouts/
+      super
     end
 
     def performed?
diff --git a/actionpack/lib/action_controller/new_base/layouts.rb b/actionpack/lib/action_controller/new_base/layouts.rb
index 0ff71587d6..ace4b148c9 100644
--- a/actionpack/lib/action_controller/new_base/layouts.rb
+++ b/actionpack/lib/action_controller/new_base/layouts.rb
@@ -1,4 +1,163 @@
 module ActionController
+  # Layouts reverse the common pattern of including shared headers and footers in many templates to isolate changes in
+  # repeated setups. The inclusion pattern has pages that look like this:
+  #
+  #   <%= render "shared/header" %>
+  #   Hello World
+  #   <%= render "shared/footer" %>
+  #
+  # This approach is a decent way of keeping common structures isolated from the changing content, but it's verbose
+  # and if you ever want to change the structure of these two includes, you'll have to change all the templates.
+  #
+  # With layouts, you can flip it around and have the common structure know where to insert changing content. This means
+  # that the header and footer are only mentioned in one place, like this:
+  #
+  #   // The header part of this layout
+  #   <%= yield %>
+  #   // The footer part of this layout
+  #
+  # And then you have content pages that look like this:
+  #
+  #    hello world
+  #
+  # At rendering time, the content page is computed and then inserted in the layout, like this:
+  #
+  #   // The header part of this layout
+  #   hello world
+  #   // The footer part of this layout
+  #
+  # NOTE: The old notation for rendering the view from a layout was to expose the magic <tt>@content_for_layout</tt> instance
+  # variable. The preferred notation now is to use <tt>yield</tt>, as documented above.
+  #
+  # == Accessing shared variables
+  #
+  # Layouts have access to variables specified in the content pages and vice versa. This allows you to have layouts with
+  # references that won't materialize before rendering time:
+  #
+  #   <h1><%= @page_title %></h1>
+  #   <%= yield %>
+  #
+  # ...and content pages that fulfill these references _at_ rendering time:
+  #
+  #    <% @page_title = "Welcome" %>
+  #    Off-world colonies offers you a chance to start a new life
+  #
+  # The result after rendering is:
+  #
+  #   <h1>Welcome</h1>
+  #   Off-world colonies offers you a chance to start a new life
+  #
+  # == Layout assignment
+  #
+  # You can either specify a layout declaratively (using the #layout class method) or give
+  # it the same name as your controller, and place it in <tt>app/views/layouts</tt>.
+  # If a subclass does not have a layout specified, it inherits its layout using normal Ruby inheritance.
+  #
+  # For instance, if you have PostsController and a template named <tt>app/views/layouts/posts.html.erb</tt>,
+  # that template will be used for all actions in PostsController and controllers inheriting
+  # from PostsController.
+  #
+  # If you use a module, for instance Weblog::PostsController, you will need a template named
+  # <tt>app/views/layouts/weblog/posts.html.erb</tt>.
+  #
+  # Since all your controllers inherit from ApplicationController, they will use
+  # <tt>app/views/layouts/application.html.erb</tt> if no other layout is specified
+  # or provided.
+  #
+  # == Inheritance Examples
+  #
+  #   class BankController < ActionController::Base
+  #     layout "bank_standard"
+  #
+  #   class InformationController < BankController
+  #
+  #   class TellerController < BankController
+  #     # teller.html.erb exists
+  #
+  #   class TillController < TellerController
+  #
+  #   class VaultController < BankController
+  #     layout :access_level_layout
+  #
+  #   class EmployeeController < BankController
+  #     layout nil
+  #
+  # The InformationController uses "bank_standard" inherited from the BankController, the VaultController overwrites
+  # and picks the layout dynamically, and the EmployeeController doesn't want to use a layout at all.
+  #
+  # The TellerController uses +teller.html.erb+, and TillController inherits that layout and
+  # uses it as well.
+  #
+  # == Types of layouts
+  #
+  # Layouts are basically just regular templates, but the name of this template needs not be specified statically. Sometimes
+  # you want to alternate layouts depending on runtime information, such as whether someone is logged in or not. This can
+  # be done either by specifying a method reference as a symbol or using an inline method (as a proc).
+  #
+  # The method reference is the preferred approach to variable layouts and is used like this:
+  #
+  #   class WeblogController < ActionController::Base
+  #     layout :writers_and_readers
+  #
+  #     def index
+  #       # fetching posts
+  #     end
+  #
+  #     private
+  #       def writers_and_readers
+  #         logged_in? ? "writer_layout" : "reader_layout"
+  #       end
+  #
+  # Now when a new request for the index action is processed, the layout will vary depending on whether the person accessing
+  # is logged in or not.
+  #
+  # If you want to use an inline method, such as a proc, do something like this:
+  #
+  #   class WeblogController < ActionController::Base
+  #     layout proc{ |controller| controller.logged_in? ? "writer_layout" : "reader_layout" }
+  #
+  # Of course, the most common way of specifying a layout is still just as a plain template name:
+  #
+  #   class WeblogController < ActionController::Base
+  #     layout "weblog_standard"
+  #
+  # If no directory is specified for the template name, the template will by default be looked for in <tt>app/views/layouts/</tt>.
+  # Otherwise, it will be looked up relative to the template root.
+  #
+  # == 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
+  # a given action or set of actions without a layout, or restricting a layout to only a single action or a set of actions. The
+  # <tt>:only</tt> and <tt>:except</tt> options can be passed to the layout call. For example:
+  #
+  #   class WeblogController < ActionController::Base
+  #     layout "weblog_standard", :except => :rss
+  #
+  #     # ...
+  #
+  #   end
+  #
+  # This will assign "weblog_standard" as the WeblogController's layout  except for the +rss+ action, which will not wrap a layout
+  # around the rendered view.
+  #
+  # Both the <tt>:only</tt> and <tt>:except</tt> condition can accept an arbitrary number of method references, so
+  # #<tt>:except => [ :rss, :text_only ]</tt> is valid, as is <tt>:except => :rss</tt>.
+  #
+  # == Using a different layout in the action render call
+  #
+  # If most of your actions use the same layout, it makes perfect sense to define a controller-wide layout as described above.
+  # Sometimes you'll have exceptions where one action wants to use a different layout than the rest of the controller.
+  # You can do this by passing a <tt>:layout</tt> option to the <tt>render</tt> call. For example:
+  #
+  #   class WeblogController < ActionController::Base
+  #     layout "weblog_standard"
+  #
+  #     def help
+  #       render :action => "help", :layout => "help"
+  #     end
+  #   end
+  #
+  # This will render the help action with the "help" layout instead of the controller-wide "weblog_standard" layout.
   module Layouts
     extend ActiveSupport::Concern
 
@@ -6,6 +165,7 @@ module ActionController
     include AbstractController::Layouts
 
     module ClassMethods
+      # If no layout is provided, look for a layout with this name.
       def _implied_layout_name
         controller_path
       end
@@ -14,16 +174,17 @@ module ActionController
     private
       def _determine_template(options)
         super
-        if (!options.key?(:text) && !options.key?(:inline) && !options.key?(:partial)) || options.key?(:layout)
-          options[:_layout] = _layout_for_option(options.key?(:layout) ? options[:layout] : :none, options[:_template].details)
-        end
+
+        return if (options.key?(:text) || options.key?(:inline) || options.key?(:partial)) && !options.key?(:layout)
+        layout = options.key?(:layout) ? options[:layout] : :none
+        options[:_layout] = _layout_for_option(layout, options[:_template].details)
       end
 
       def _layout_for_option(name, details)
         case name
         when String     then _layout_for_name(name, details)
-        when true       then _default_layout(true, details)
-        when :none      then _default_layout(false, details)
+        when true       then _default_layout(details, true)
+        when :none      then _default_layout(details, false)
         when false, nil then nil
         else
           raise ArgumentError,
diff --git a/actionpack/test/abstract_controller/abstract_controller_test.rb b/actionpack/test/abstract_controller/abstract_controller_test.rb
index c7eaaeb4ba..05b55216c8 100644
--- a/actionpack/test/abstract_controller/abstract_controller_test.rb
+++ b/actionpack/test/abstract_controller/abstract_controller_test.rb
@@ -154,7 +154,7 @@ module AbstractController
       end
 
       def render_to_body(options = {})
-        options[:_layout] = options[:layout] || _default_layout
+        options[:_layout] = options[:layout] || _default_layout({})
         super
       end  
     end
diff --git a/actionpack/test/abstract_controller/layouts_test.rb b/actionpack/test/abstract_controller/layouts_test.rb
index 4b66f063f3..64c435abb7 100644
--- a/actionpack/test/abstract_controller/layouts_test.rb
+++ b/actionpack/test/abstract_controller/layouts_test.rb
@@ -25,7 +25,7 @@ module AbstractControllerTests
       def controller_path() self.class.controller_path end
       
       def render_to_body(options)
-        options[:_layout] = _default_layout
+        options[:_layout] = _default_layout({})
         super
       end
     end
@@ -221,11 +221,11 @@ module AbstractControllerTests
       
       test "raises an exception when specifying layout true" do
         assert_raises ArgumentError do
-          Object.class_eval <<-RUBY_EVAL, __FILE__, __LINE__ + 1
+          Object.class_eval do
             class ::BadOmgFailLolLayout < AbstractControllerTests::Layouts::Base
               layout true
             end
-          RUBY_EVAL
+          end
         end
       end
     end