Change ActionView template defaults. Look for templates using the request format first, such as show.html.erb or show.xml.builder, before looking for the old defaults like show.erb or show.builder [Rick]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@6499 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

actionpack/CHANGELOG

@@ -1,5 +1,7 @@
 *SVN*
 
+* Change ActionView template defaults.  Look for templates using the request format first, such as "show.html.erb" or "show.xml.builder", before looking for the old defaults like "show.erb" or "show.builder" [Rick]
+
 * Highlight helper highlights one or many terms in a single pass.  [Jeremy Kemper]
 
 * Dropped the use of ; as a separator of non-crud actions on resources and went back to the vanilla slash. It was a neat idea, but lots of the non-crud actions turned out not to be RPC (as the ; was primarily intended to discourage), but legitimate sub-resources, like /parties/recent, which didn't deserve the uglification of /parties;recent. Further more, the semicolon caused issues with caching and HTTP authentication in Safari. Just Not Worth It [DHH]

actionpack/lib/action_controller/base.rb

@@ -1225,7 +1225,7 @@ module ActionController #:nodoc:
 
       def assert_existence_of_template_file(template_name)
         unless template_exists?(template_name) || ignore_missing_templates
-          full_template_path = @template.send(:full_template_path, template_name, 'erb')
+          full_template_path = @template.send(:full_template_path, template_name, "#{@template.send(:template_format)}.erb")
           template_type = (template_name =~ /layouts/i) ? 'layout' : 'template'
           raise(MissingTemplate, "Missing #{template_type} #{full_template_path}")
         end

actionpack/lib/action_view/base.rb

@@ -252,6 +252,7 @@ module ActionView #:nodoc:
         else
           template_extension = pick_template_extension(template_path).to_s
           template_file_name = full_template_path(template_path, template_extension)
+          template_extension.gsub!(/^\w+\./, '') # strip off any formats
         end
       else
         template_file_name = template_path
@@ -330,10 +331,11 @@ module ActionView #:nodoc:
     end
 
     def pick_template_extension(template_path)#:nodoc:
+      formatted_template_path = "#{template_path}.#{template_format}"
       if @@cache_template_extensions
-        @@cached_template_extension[template_path] ||= find_template_extension_for(template_path)
+        @@cached_template_extension[formatted_template_path] ||= find_template_extension_for(template_path, formatted_template_path)
       else
-        find_template_extension_for(template_path)
+        find_template_extension_for(template_path, formatted_template_path)
       end
     end
 
@@ -359,15 +361,24 @@ module ActionView #:nodoc:
       template_exists?(template_path, :rjs)
     end
 
+    def formatted_template_exists?(formatted_template_exists)
+      [:erb, :builder, :rjs].each do |ext|
+        return ext if template_exists?(formatted_template_exists, ext)
+      end
+      nil
+    end
+
     def file_exists?(template_path)#:nodoc:
       template_file_name, template_file_extension = path_and_extension(template_path)
       if template_file_extension
         template_exists?(template_file_name, template_file_extension)
       else
-        cached_template_extension(template_path) ||
-           %w(erb rhtml builder rxml javascript delegate).any? do |template_type|
-             send("#{template_type}_template_exists?", template_path)
-           end
+        formatted_template_path = "#{template_path}.#{template_format}"
+        cached_template_extension(formatted_template_path) ||
+          formatted_template_exists?(formatted_template_path) ||
+          %w(erb rhtml builder rxml javascript delegate).any? do |template_type|
+            send("#{template_type}_template_exists?", template_path)
+          end
       end
     end
 
@@ -376,6 +387,17 @@ module ActionView #:nodoc:
       template_path.split('/').last[0,1] != '_'
     end
 
+    def template_format
+      if @template_format != false
+        # check controller.respond_to?(:request) in case its an ActionMailer::Base, or some other sneaky class.
+        @template_format = controller.respond_to?(:request) ? false : :html
+        if controller && controller.respond_to?(:request) && controller.request && controller.request.format
+          @template_format = controller.request.format == Mime::ALL ? :html : controller.request.format.to_sym
+        end
+      end
+      @template_format
+    end
+
     private
       def full_template_path(template_path, extension)
         file_name = "#{template_path}.#{extension}"
@@ -389,13 +411,16 @@ module ActionView #:nodoc:
         @@method_names.has_key?(file_path) || FileTest.exists?(file_path)
       end
 
+      # Splits the path and extension from the given template_path and returns as an array.
       def path_and_extension(template_path)
         template_path_without_extension = template_path.sub(/\.(\w+)$/, '')
         [ template_path_without_extension, $1 ]
       end
 
-      def cached_template_extension(template_path)
-        @@cache_template_extensions && @@cached_template_extension[template_path]
+      # Caches the extension for the given formatted template path.  The extension may have the format
+      # too, such as 'html.erb'.
+      def cached_template_extension(formatted_template_path)
+        @@cache_template_extensions && @@cached_template_extension[formatted_template_path]
       end
       
       # Returns the view path that contains the given relative template path.
@@ -404,11 +429,13 @@ module ActionView #:nodoc:
       end
 
       # Determines the template's file extension, such as rhtml, rxml, or rjs.
-      def find_template_extension_for(template_path)
+      def find_template_extension_for(template_path, formatted_template_path = nil)
+        formatted_template_path ||= "#{template_path}.#{template_format}"
         if match = delegate_template_exists?(template_path)
           match.first.to_sym
-        elsif extension = erb_template_exists?(template_path):        extension
-        elsif extension = builder_template_exists?(template_path):    extension
+        elsif extension = formatted_template_exists?(formatted_template_path): "#{template_format}.#{extension}"
+        elsif extension = erb_template_exists?(template_path):                 extension
+        elsif extension = builder_template_exists?(template_path):             extension
         elsif javascript_template_exists?(template_path): :rjs
         else
           raise ActionViewError, "No erb, builder, rhtml, rxml, rjs or delegate template found for #{template_path} in #{@view_paths.inspect}"

actionpack/test/controller/render_test.rb

@@ -122,6 +122,12 @@ class TestController < ActionController::Base
     ActionView::Base.local_assigns_support_string_keys = false
   end
 
+  def formatted_html_erb
+  end
+
+  def formatted_xml_erb
+  end
+
   def render_to_string_test
     @foo = render_to_string :inline => "this is a test"
   end
@@ -341,6 +347,20 @@ class RenderTest < Test::Unit::TestCase
     assert_equal etag_for("<wrapper>\n<html>\n  <p>Hello </p>\n<p>This is grand!</p>\n</html>\n</wrapper>\n"), @response.headers['ETag']
   end
 
+  def test_should_render_formatted_template
+    get :formatted_html_erb
+    assert_equal 'formatted html erb', @response.body
+  end
+  
+  def test_should_render_formatted_xml_erb_template
+    get :formatted_xml_erb, :format => :xml
+    assert_equal '<test>passed formatted xml erb</test>', @response.body
+  end
+  
+  def test_should_render_formatted_html_erb_template
+    get :formatted_xml_erb
+    assert_equal '<test>passed formatted html erb</test>', @response.body
+  end
 
   protected
     def assert_deprecated_render(&block)

actionpack/test/fixtures/test/formatted_html_erb.html.erb

@@ -0,0 +1 @@
+formatted html erb

actionpack/test/fixtures/test/formatted_xml_erb.builder

@@ -0,0 +1 @@
+xml.test 'failed'

actionpack/test/fixtures/test/formatted_xml_erb.html.erb

@@ -0,0 +1 @@
+<test>passed formatted html erb</test>

actionpack/test/fixtures/test/formatted_xml_erb.xml.erb

@@ -0,0 +1 @@
+<test>passed formatted xml erb</test>

actionpack/test/template/base_test.rb

@@ -14,14 +14,22 @@ class ActionViewTemplateTest < Test::Unit::TestCase
       assert_equal :foo, @template.send(:find_template_extension_for, 'foo')
     end
     
+    def test_should_find_formatted_erb_extension
+      @template.expects(:delegate_template_exists?).with('foo').returns(nil)
+      @template.expects(:formatted_template_exists?).with('foo.html').returns("erb")
+      assert_equal "html.erb", @template.send(:find_template_extension_for, 'foo')
+    end
+    
     def test_should_find_erb_extension
       @template.expects(:delegate_template_exists?).with('foo').returns(nil)
+      @template.expects(:formatted_template_exists?).with('foo.html').returns(nil)
       @template.expects(:erb_template_exists?).with('foo').returns(:erb)
       assert_equal :erb, @template.send(:find_template_extension_for, 'foo')
     end
     
     def test_should_find_builder_extension
       @template.expects(:delegate_template_exists?).with('foo').returns(nil)
+      @template.expects(:formatted_template_exists?).with('foo.html').returns(nil)
       @template.expects(:erb_template_exists?).with('foo').returns(nil)
       @template.expects(:builder_template_exists?).with('foo').returns(:builder)
       assert_equal :builder, @template.send(:find_template_extension_for, 'foo')
@@ -29,6 +37,7 @@ class ActionViewTemplateTest < Test::Unit::TestCase
     
     def test_should_find_javascript_extension
       @template.expects(:delegate_template_exists?).with('foo').returns(nil)
+      @template.expects(:formatted_template_exists?).with('foo.html').returns(nil)
       @template.expects(:erb_template_exists?).with('foo').returns(nil)
       @template.expects(:builder_template_exists?).with('foo').returns(nil)
       @template.expects(:javascript_template_exists?).with('foo').returns(true)