Get rid of update_details in favor of passing details to find_template.

actionmailer/test/mailers/base_mailer.rb

@@ -113,6 +113,6 @@ def different_layout(layout_name='')
   end
 
   def email_with_translations
-    mail :body => render("email_with_translations.html")
+    mail :body => render("email_with_translations", :formats => [:html])
   end
 end

actionpack/lib/action_view/lookup_context.rb

@@ -20,18 +20,16 @@ class LookupContext #:nodoc:
 
     def self.register_detail(name, options = {}, &block)
       self.registered_details << name
-
       initialize = registered_details.map { |n| "self.#{n} = details[:#{n}]" }
-      update     = registered_details.map { |n| "self.#{n} = details[:#{n}] if details.key?(:#{n})" }
 
-      Accessors.send :define_method, :"_#{name}_defaults", &block
+      Accessors.send :define_method, :"default_#{name}", &block
       Accessors.module_eval <<-METHOD, __FILE__, __LINE__ + 1
         def #{name}
           @details[:#{name}]
         end
 
         def #{name}=(value)
-          value = Array.wrap(value.presence || _#{name}_defaults)
+          value = Array.wrap(value.presence || default_#{name})
           _set_detail(:#{name}, value) if value != @details[:#{name}]
         end
 
@@ -39,11 +37,6 @@ def #{name}=(value)
         def initialize_details(details)
           #{initialize.join("\n")}
         end
-
-        remove_possible_method :update_details
-        def update_details(details)
-          #{update.join("\n")}
-        end
       METHOD
     end
 
@@ -88,24 +81,6 @@ def disable_cache
         @cache = old_value
       end
 
-      # Update the details keys by merging the given hash into the current
-      # details hash. If a block is given, the details are modified just during
-      # the execution of the block and reverted to the previous value after.
-      def update_details(new_details)
-        if block_given?
-          old_details, old_key = @details.dup, @details_key
-          super
-
-          begin
-            yield
-          ensure
-            @details, @details_key = old_details, old_key
-          end
-        else
-          super
-        end
-      end
-
     protected
 
       def _set_detail(key, value)
@@ -220,7 +195,7 @@ def freeze_formats(formats, unless_frozen=false) #:nodoc:
     # add :html as fallback to :js.
     def formats=(values)
       if values
-        values.concat(_formats_defaults) if values.delete "*/*"
+        values.concat(default_formats) if values.delete "*/*"
         values << :html if values == [:js]
       end
       super(values)
@@ -246,7 +221,7 @@ def locale=(value)
         config.locale = value
       end
 
-      super(@skip_default_locale ? I18n.locale : _locale_defaults)
+      super(@skip_default_locale ? I18n.locale : default_locale)
     end
 
     # A method which only uses the first format in the formats array for layout lookup.

actionpack/lib/action_view/renderer/abstract_renderer.rb

@@ -11,15 +11,13 @@ def render
       raise NotImplementedError
     end
 
-    # Checks if the given path contains a format and if so, change
-    # the lookup context to take this new format into account.
-    def wrap_formats(value)
-      return yield unless value.is_a?(String)
+    protected
 
-      if value.sub!(formats_regexp, "")
-        update_details(:formats => [$1.to_sym]){ yield }
-      else
-        yield
+    def extract_format(value, details)
+      if value.is_a?(String) && value.sub!(formats_regexp, "")
+        ActiveSupport::Deprecation.warn "Passing the format in the template name is deprecated. " \
+          "Please pass render with :formats => #{$1} instead.", caller
+        details[:formats] ||= [$1.to_sym]
       end
     end
 
@@ -27,8 +25,6 @@ def formats_regexp
       @@formats_regexp ||= /\.(#{Mime::SET.symbols.join('|')})$/
     end
 
-    protected
-
     def instrument(name, options={})
       ActiveSupport::Notifications.instrument("render_#{name}.action_view", options){ yield }
     end

actionpack/lib/action_view/renderer/partial_renderer.rb

@@ -216,18 +216,15 @@ def initialize(*)
 
     def render(context, options, block)
       setup(context, options, block)
+      identifier = (@template = find_partial) ? @template.identifier : @path
 
-      wrap_formats(@path) do
-        identifier = ((@template = find_partial) ? @template.identifier : @path)
-
-        if @collection
-          instrument(:collection, :identifier => identifier || "collection", :count => @collection.size) do
-            render_collection
-          end
-        else
-          instrument(:partial, :identifier => identifier) do
-            render_partial
-          end
+      if @collection
+        instrument(:collection, :identifier => identifier || "collection", :count => @collection.size) do
+          render_collection
+        end
+      else
+        instrument(:partial, :identifier => identifier) do
+          render_partial
         end
       end
     end
@@ -271,6 +268,7 @@ def setup(context, options, block)
       @options = options
       @locals  = options[:locals] || {}
       @block   = block
+      @details = options.slice(:formats, :locale, :handlers)
 
       if String === partial
         @object     = options[:object]
@@ -299,6 +297,7 @@ def setup(context, options, block)
                                 "and is followed by any combinations of letters, numbers, or underscores.")
       end
 
+      extract_format(@path, @details)
       self
     end
 
@@ -326,7 +325,7 @@ def find_partial
 
     def find_template(path=@path, locals=@locals.keys)
       prefixes = path.include?(?/) ? [] : @lookup_context.prefixes
-      @lookup_context.find_template(path, prefixes, true, locals)
+      @lookup_context.find_template(path, prefixes, true, locals, @details)
     end
 
     def collection_with_template

actionpack/lib/action_view/renderer/template_renderer.rb

@@ -4,30 +4,28 @@
 module ActionView
   class TemplateRenderer < AbstractRenderer #:nodoc:
     def render(context, options)
-      @view = context
-
-      wrap_formats(options[:template] || options[:file]) do
-        template = determine_template(options)
-        freeze_formats(template.formats, true)
-        render_template(template, options[:layout], options[:locals])
-      end
+      @view    = context
+      @details = options.slice(:formats, :locale, :handlers)
+      extract_format(options[:file] || options[:template], @details)
+      template = determine_template(options)
+      freeze_formats(template.formats, true)
+      render_template(template, options[:layout], options[:locals])
     end
 
     # Determine the template to be rendered using the given options.
     def determine_template(options) #:nodoc:
-      keys    = options[:locals].try(:keys) || []
-      details = options.slice(:formats, :locale, :handlers)
+      keys = options[:locals].try(:keys) || []
 
       if options.key?(:text)
         Template::Text.new(options[:text], formats.try(:first))
       elsif options.key?(:file)
-        with_fallbacks { find_template(options[:file], nil, false, keys, details) }
+        with_fallbacks { find_template(options[:file], nil, false, keys, @details) }
       elsif options.key?(:inline)
         handler = Template.handler_for_extension(options[:type] || "erb")
         Template.new(options[:inline], "inline template", handler, :locals => keys)
       elsif options.key?(:template)
         options[:template].respond_to?(:render) ?
-          options[:template] : find_template(options[:template], options[:prefixes], false, keys)
+          options[:template] : find_template(options[:template], options[:prefixes], false, keys, @details)
       end
     end
 
@@ -63,12 +61,11 @@ def find_layout(layout, keys)
       begin
         with_layout_format do
           layout =~ /^\// ?
-            with_fallbacks { find_template(layout, nil, false, keys) } : find_template(layout, nil, false, keys)
+            with_fallbacks { find_template(layout, nil, false, keys, @details) } : find_template(layout, nil, false, keys, @details)
         end
       rescue ActionView::MissingTemplate
-        update_details(:formats => nil) do
-          raise unless template_exists?(layout)
-        end
+        all_details = @details.merge(:formats => @lookup_context.default_formats)
+        raise unless template_exists?(layout, nil, false, keys, all_details)
       end
     end
   end

actionpack/test/controller/new_base/render_partial_test.rb

@@ -7,12 +7,12 @@ class BasicController < ActionController::Base
     self.view_paths = [ActionView::FixtureResolver.new(
       "render_partial/basic/_basic.html.erb"     => "BasicPartial!",
       "render_partial/basic/basic.html.erb"      => "<%= @test_unchanged = 'goodbye' %><%= render :partial => 'basic' %><%= @test_unchanged %>",
-      "render_partial/basic/with_json.html.erb"  => "<%= render 'with_json.json' %>",
-      "render_partial/basic/_with_json.json.erb" => "<%= render 'final' %>",
+      "render_partial/basic/with_json.html.erb"  => "<%= render :partial => 'with_json', :formats => [:json] %>",
+      "render_partial/basic/_with_json.json.erb" => "<%= render :partial => 'final', :formats => [:json] %>",
       "render_partial/basic/_final.json.erb"     => "{ final: json }",
-      "render_partial/basic/overriden.html.erb"    => "<%= @test_unchanged = 'goodbye' %><%= render :partial => 'overriden' %><%= @test_unchanged %>",
-      "render_partial/basic/_overriden.html.erb"    => "ParentPartial!",
-      "render_partial/child/_overriden.html.erb"    => "OverridenPartial!"
+      "render_partial/basic/overriden.html.erb"  => "<%= @test_unchanged = 'goodbye' %><%= render :partial => 'overriden' %><%= @test_unchanged %>",
+      "render_partial/basic/_overriden.html.erb" => "ParentPartial!",
+      "render_partial/child/_overriden.html.erb" => "OverridenPartial!"
     )]
 
     def html_with_json_inside_json

actionpack/test/controller/new_base/render_template_test.rb

@@ -10,8 +10,8 @@ class WithoutLayoutController < ActionController::Base
       "xml_template.xml.builder"   => "xml.html do\n  xml.p 'Hello'\nend",
       "with_raw.html.erb"          => "Hello <%=raw '<strong>this is raw</strong>' %>",
       "with_implicit_raw.html.erb" => "Hello <%== '<strong>this is also raw</strong>' %>",
-      "test/with_json.html.erb"    => "<%= render :template => 'test/with_json.json' %>",
-      "test/with_json.json.erb"    => "<%= render :template => 'test/final' %>",
+      "test/with_json.html.erb"    => "<%= render :template => 'test/with_json', :formats => [:json] %>",
+      "test/with_json.json.erb"    => "<%= render :template => 'test/final', :formats => [:json]  %>",
       "test/final.json.erb"        => "{ final: json }",
       "test/with_error.html.erb"   => "<%= idontexist %>"
     )]
@@ -117,7 +117,7 @@ class TestWithoutLayout < Rack::TestCase
       assert_response "{ final: json }"
     end
 
-    test "rendering a template with error properly exceprts the code" do
+    test "rendering a template with error properly excerts the code" do
       get :with_error
       assert_status 500
       assert_match "undefined local variable or method `idontexist'", response.body

actionpack/test/controller/render_test.rb

@@ -797,7 +797,9 @@ def test_render_file_with_instance_variables
   end
 
   def test_render_file
-    get :hello_world_file
+    assert_deprecated do
+      get :hello_world_file
+    end
     assert_equal "Hello world!", @response.body
   end
 

actionpack/test/template/lookup_context_test.rb

@@ -31,16 +31,6 @@ def teardown
     assert @lookup_context.formats.frozen?
   end
 
-  test "allows me to change some details to execute an specific block of code" do
-    formats = Mime::SET
-    @lookup_context.update_details(:locale => :pt) do
-      assert_equal formats, @lookup_context.formats
-      assert_equal :pt, @lookup_context.locale
-    end
-    assert_equal formats, @lookup_context.formats
-    assert_equal :en, @lookup_context.locale
-  end
-
   test "provides getters and setters for formats" do
     @lookup_context.formats = [:html]
     assert_equal [:html], @lookup_context.formats

actionpack/test/template/render_test.rb

@@ -38,6 +38,11 @@ def test_render_file_with_format
     assert_equal "<error>No Comment</error>", @view.render(:file => "comments/empty", :formats => [:xml])
   end
 
+  def test_render_template_with_format
+    assert_equal "<h1>No Comment</h1>", @view.render(:template => "comments/empty", :formats => [:html])
+    assert_equal "<error>No Comment</error>", @view.render(:template => "comments/empty", :formats => [:xml])
+  end
+
   def test_render_file_with_localization
     old_locale, @view.locale = @view.locale, :da
     assert_equal "Hey verden", @view.render(:file => "test/hello_world")