From 43d27e9105b385f64ec195f60d10ab3d64281bd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 22 Sep 2011 15:37:38 +0200 Subject: [PATCH] Deprecate passing the template handler in the template name. For example, calling hello.erb is now deprecated. Since Rails 3.0 passing the handler had no effect whatsover. This commit simply deprecates such cases so we can clean up the code in later releases. --- .../middleware/show_exceptions.rb | 4 +- actionpack/lib/action_view/lookup_context.rb | 7 +++- .../action_view/renderer/abstract_renderer.rb | 2 +- .../test/abstract/abstract_controller_test.rb | 6 +-- actionpack/test/controller/caching_test.rb | 2 +- actionpack/test/controller/layout_test.rb | 4 +- .../controller/new_base/render_file_test.rb | 10 ++--- actionpack/test/controller/render_test.rb | 28 ++++++------- .../_layout_with_partial_and_yield.html.erb | 2 +- .../test/template/compiled_templates_test.rb | 16 +++---- .../test/template/log_subscriber_test.rb | 2 +- actionpack/test/template/render_test.rb | 42 ++++++++++--------- .../test/template/streaming_render_test.rb | 16 +++---- 13 files changed, 72 insertions(+), 69 deletions(-) diff --git a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb index a765c23dae..2fa68c64c5 100644 --- a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb @@ -86,8 +86,8 @@ module ActionDispatch :framework_trace => framework_trace(exception), :full_trace => full_trace(exception) ) - file = "rescues/#{@@rescue_templates[exception.class.name]}.erb" - body = template.render(:file => file, :layout => 'rescues/layout.erb') + file = "rescues/#{@@rescue_templates[exception.class.name]}" + body = template.render(:template => file, :layout => 'rescues/layout') render(status_code(exception), body) end diff --git a/actionpack/lib/action_view/lookup_context.rb b/actionpack/lib/action_view/lookup_context.rb index ef1ef44317..fa4bf70f77 100644 --- a/actionpack/lib/action_view/lookup_context.rb +++ b/actionpack/lib/action_view/lookup_context.rb @@ -147,7 +147,12 @@ module ActionView # as well as incorrectly putting part of the path in the template # name instead of the prefix. def normalize_name(name, prefixes) #:nodoc: - name = name.to_s.gsub(handlers_regexp, '') + name = name.to_s.sub(handlers_regexp) do |match| + ActiveSupport::Deprecation.warn "Passing a template handler in the template name is deprecated. " \ + "You can simply remove the handler name or pass render :handlers => [:#{match[1..-1]}] instead.", caller + "" + end + parts = name.split('/') name = parts.pop diff --git a/actionpack/lib/action_view/renderer/abstract_renderer.rb b/actionpack/lib/action_view/renderer/abstract_renderer.rb index 24bae64ca5..1656cf7ec7 100644 --- a/actionpack/lib/action_view/renderer/abstract_renderer.rb +++ b/actionpack/lib/action_view/renderer/abstract_renderer.rb @@ -16,7 +16,7 @@ module ActionView 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 + "Please pass render with :formats => [:#{$1}] instead.", caller details[:formats] ||= [$1.to_sym] end end diff --git a/actionpack/test/abstract/abstract_controller_test.rb b/actionpack/test/abstract/abstract_controller_test.rb index 5823e64637..bf068aedcd 100644 --- a/actionpack/test/abstract/abstract_controller_test.rb +++ b/actionpack/test/abstract/abstract_controller_test.rb @@ -50,7 +50,7 @@ module AbstractController end def index_to_string - self.response_body = render_to_string "index.erb" + self.response_body = render_to_string "index" end def action_with_ivars @@ -63,11 +63,11 @@ module AbstractController end def rendering_to_body - self.response_body = render_to_body :template => "naked_render.erb" + self.response_body = render_to_body :template => "naked_render" end def rendering_to_string - self.response_body = render_to_string :template => "naked_render.erb" + self.response_body = render_to_string :template => "naked_render" end end diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index da3314fe6d..f3b180283f 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -197,7 +197,7 @@ class ActionCachingTestController < CachingController caches_action :layout_false, :layout => false caches_action :record_not_found, :four_oh_four, :simple_runtime_error - layout 'talk_from_action.erb' + layout 'talk_from_action' def index @cache_this = MockTime.now.to_f.to_s diff --git a/actionpack/test/controller/layout_test.rb b/actionpack/test/controller/layout_test.rb index cafe2b9320..25299eb8b8 100644 --- a/actionpack/test/controller/layout_test.rb +++ b/actionpack/test/controller/layout_test.rb @@ -79,7 +79,7 @@ class DefaultLayoutController < LayoutTest end class AbsolutePathLayoutController < LayoutTest - layout File.expand_path(File.expand_path(__FILE__) + '/../../fixtures/layout_tests/layouts/layout_test.erb') + layout File.expand_path(File.expand_path(__FILE__) + '/../../fixtures/layout_tests/layouts/layout_test') end class HasOwnLayoutController < LayoutTest @@ -184,7 +184,7 @@ class RenderWithTemplateOptionController < LayoutTest end class SetsNonExistentLayoutFile < LayoutTest - layout "nofile.erb" + layout "nofile" end class LayoutExceptionRaised < ActionController::TestCase diff --git a/actionpack/test/controller/new_base/render_file_test.rb b/actionpack/test/controller/new_base/render_file_test.rb index 8b2fdf8f96..a961cbf849 100644 --- a/actionpack/test/controller/new_base/render_file_test.rb +++ b/actionpack/test/controller/new_base/render_file_test.rb @@ -10,7 +10,7 @@ module RenderFile def with_instance_variables @secret = 'in the sauce' - render :file => File.join(File.dirname(__FILE__), '../../fixtures/test/render_file_with_ivar.erb') + render :file => File.join(File.dirname(__FILE__), '../../fixtures/test/render_file_with_ivar') end def without_file_key @@ -19,7 +19,7 @@ module RenderFile def without_file_key_with_instance_variable @secret = 'in the sauce' - render File.join(File.dirname(__FILE__), '../../fixtures/test/render_file_with_ivar.erb') + render File.join(File.dirname(__FILE__), '../../fixtures/test/render_file_with_ivar') end def relative_path @@ -34,16 +34,16 @@ module RenderFile def pathname @secret = 'in the sauce' - render :file => Pathname.new(File.dirname(__FILE__)).join(*%w[.. .. fixtures test dot.directory render_file_with_ivar.erb]) + render :file => Pathname.new(File.dirname(__FILE__)).join(*%w[.. .. fixtures test dot.directory render_file_with_ivar]) end def with_locals - path = File.join(File.dirname(__FILE__), '../../fixtures/test/render_file_with_locals.erb') + path = File.join(File.dirname(__FILE__), '../../fixtures/test/render_file_with_locals') render :file => path, :locals => {:secret => 'in the sauce'} end def without_file_key_with_locals - path = FIXTURES.join('test/render_file_with_locals.erb').to_s + path = FIXTURES.join('test/render_file_with_locals').to_s render path, :locals => {:secret => 'in the sauce'} end end diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 719e37799d..aea603b014 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -163,14 +163,14 @@ class TestController < ActionController::Base # :ported: def render_file_with_instance_variables @secret = 'in the sauce' - path = File.join(File.dirname(__FILE__), '../fixtures/test/render_file_with_ivar.erb') + path = File.join(File.dirname(__FILE__), '../fixtures/test/render_file_with_ivar') render :file => path end # :ported: def render_file_as_string_with_instance_variables @secret = 'in the sauce' - path = File.expand_path(File.join(File.dirname(__FILE__), '../fixtures/test/render_file_with_ivar.erb')) + path = File.expand_path(File.join(File.dirname(__FILE__), '../fixtures/test/render_file_with_ivar')) render path end @@ -187,21 +187,21 @@ class TestController < ActionController::Base def render_file_using_pathname @secret = 'in the sauce' - render :file => Pathname.new(File.dirname(__FILE__)).join('..', 'fixtures', 'test', 'dot.directory', 'render_file_with_ivar.erb') + render :file => Pathname.new(File.dirname(__FILE__)).join('..', 'fixtures', 'test', 'dot.directory', 'render_file_with_ivar') end def render_file_from_template @secret = 'in the sauce' - @path = File.expand_path(File.join(File.dirname(__FILE__), '../fixtures/test/render_file_with_ivar.erb')) + @path = File.expand_path(File.join(File.dirname(__FILE__), '../fixtures/test/render_file_with_ivar')) end def render_file_with_locals - path = File.join(File.dirname(__FILE__), '../fixtures/test/render_file_with_locals.erb') + path = File.join(File.dirname(__FILE__), '../fixtures/test/render_file_with_locals') render :file => path, :locals => {:secret => 'in the sauce'} end def render_file_as_string_with_locals - path = File.expand_path(File.join(File.dirname(__FILE__), '../fixtures/test/render_file_with_locals.erb')) + path = File.expand_path(File.join(File.dirname(__FILE__), '../fixtures/test/render_file_with_locals')) render path, :locals => {:secret => 'in the sauce'} end @@ -453,17 +453,13 @@ class TestController < ActionController::Base render :action => "potential_conflicts" end - # :deprecated: - # Tests being able to pick a .builder template over a .erb - # For instance, being able to have hello.xml.builder and hello.xml.erb - # and select one via "hello.builder" or "hello.erb" def hello_world_from_rxml_using_action - render :action => "hello_world_from_rxml.builder" + render :action => "hello_world_from_rxml", :handlers => [:builder] end # :deprecated: def hello_world_from_rxml_using_template - render :template => "test/hello_world_from_rxml.builder" + render :template => "test/hello_world_from_rxml", :handlers => [:builder] end def action_talk_to_layout @@ -525,8 +521,8 @@ class TestController < ActionController::Base render :action => "using_layout_around_block", :layout => "layouts/block_with_layout" end - def partial_dot_html - render :partial => 'partial.html.erb' + def partial_formats_html + render :partial => 'partial', :formats => [:html] end def partial @@ -1235,8 +1231,8 @@ class RenderTest < ActionController::TestCase assert_equal 'partial html', @response.body end - def test_should_render_html_partial_with_dot - get :partial_dot_html + def test_should_render_html_partial_with_formats + get :partial_formats_html assert_equal 'partial html', @response.body end diff --git a/actionpack/test/fixtures/test/_layout_with_partial_and_yield.html.erb b/actionpack/test/fixtures/test/_layout_with_partial_and_yield.html.erb index 5db0822f07..820e7db789 100644 --- a/actionpack/test/fixtures/test/_layout_with_partial_and_yield.html.erb +++ b/actionpack/test/fixtures/test/_layout_with_partial_and_yield.html.erb @@ -1,4 +1,4 @@ Before -<%= render :partial => "test/partial.html.erb" %> +<%= render :partial => "test/partial" %> <%= yield %> After diff --git a/actionpack/test/template/compiled_templates_test.rb b/actionpack/test/template/compiled_templates_test.rb index 8be0f452fb..8fc78283d8 100644 --- a/actionpack/test/template/compiled_templates_test.rb +++ b/actionpack/test/template/compiled_templates_test.rb @@ -10,24 +10,24 @@ class CompiledTemplatesTest < Test::Unit::TestCase end def test_template_gets_recompiled_when_using_different_keys_in_local_assigns - assert_equal "one", render(:file => "test/render_file_with_locals_and_default.erb") - assert_equal "two", render(:file => "test/render_file_with_locals_and_default.erb", :locals => { :secret => "two" }) + assert_equal "one", render(:file => "test/render_file_with_locals_and_default") + assert_equal "two", render(:file => "test/render_file_with_locals_and_default", :locals => { :secret => "two" }) end def test_template_changes_are_not_reflected_with_cached_templates - assert_equal "Hello world!", render(:file => "test/hello_world.erb") + assert_equal "Hello world!", render(:file => "test/hello_world") modify_template "test/hello_world.erb", "Goodbye world!" do - assert_equal "Hello world!", render(:file => "test/hello_world.erb") + assert_equal "Hello world!", render(:file => "test/hello_world") end - assert_equal "Hello world!", render(:file => "test/hello_world.erb") + assert_equal "Hello world!", render(:file => "test/hello_world") end def test_template_changes_are_reflected_with_uncached_templates - assert_equal "Hello world!", render_without_cache(:file => "test/hello_world.erb") + assert_equal "Hello world!", render_without_cache(:file => "test/hello_world") modify_template "test/hello_world.erb", "Goodbye world!" do - assert_equal "Goodbye world!", render_without_cache(:file => "test/hello_world.erb") + assert_equal "Goodbye world!", render_without_cache(:file => "test/hello_world") end - assert_equal "Hello world!", render_without_cache(:file => "test/hello_world.erb") + assert_equal "Hello world!", render_without_cache(:file => "test/hello_world") end private diff --git a/actionpack/test/template/log_subscriber_test.rb b/actionpack/test/template/log_subscriber_test.rb index 50e1cccd3b..752b0f23a8 100644 --- a/actionpack/test/template/log_subscriber_test.rb +++ b/actionpack/test/template/log_subscriber_test.rb @@ -27,7 +27,7 @@ class AVLogSubscriberTest < ActiveSupport::TestCase end def test_render_file_template - @view.render(:file => "test/hello_world.erb") + @view.render(:file => "test/hello_world") wait assert_equal 1, @logger.logged(:info).size diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index f3dce0b7f2..120f91b0e7 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -21,11 +21,11 @@ module RenderTestCases end def test_render_file - assert_equal "Hello world!", @view.render(:file => "test/hello_world.erb") + assert_equal "Hello world!", @view.render(:file => "test/hello_world") end def test_render_file_not_using_full_path - assert_equal "Hello world!", @view.render(:file => "test/hello_world.erb") + assert_equal "Hello world!", @view.render(:file => "test/hello_world") end def test_render_file_without_specific_extension @@ -62,17 +62,17 @@ module RenderTestCases end def test_render_file_with_full_path - template_path = File.join(File.dirname(__FILE__), '../fixtures/test/hello_world.erb') + template_path = File.join(File.dirname(__FILE__), '../fixtures/test/hello_world') assert_equal "Hello world!", @view.render(:file => template_path) end def test_render_file_with_instance_variables - assert_equal "The secret is in the sauce\n", @view.render(:file => "test/render_file_with_ivar.erb") + assert_equal "The secret is in the sauce\n", @view.render(:file => "test/render_file_with_ivar") end def test_render_file_with_locals locals = { :secret => 'in the sauce' } - assert_equal "The secret is in the sauce\n", @view.render(:file => "test/render_file_with_locals.erb", :locals => locals) + assert_equal "The secret is in the sauce\n", @view.render(:file => "test/render_file_with_locals", :locals => locals) end def test_render_file_not_using_full_path_with_dot_in_path @@ -92,12 +92,12 @@ module RenderTestCases end def test_render_partial_at_top_level - # file fixtures/_top_level_partial_only.erb (not fixtures/test) + # file fixtures/_top_level_partial_only (not fixtures/test) assert_equal 'top level partial', @view.render(:partial => '/top_level_partial_only') end def test_render_partial_with_format_at_top_level - # file fixtures/_top_level_partial.html.erb (not fixtures/test, with format extension) + # file fixtures/_top_level_partial.html (not fixtures/test, with format extension) assert_equal 'top level partial html', @view.render(:partial => '/top_level_partial') end @@ -256,7 +256,7 @@ module RenderTestCases end def test_render_layout_with_block_and_other_partial_inside - render = @view.render(:layout => "test/layout_with_partial_and_yield.html.erb") { "Yield!" } + render = @view.render(:layout => "test/layout_with_partial_and_yield") { "Yield!" } assert_equal "Before\npartial html\nYield!\nAfter\n", render end @@ -293,24 +293,26 @@ module RenderTestCases end def test_render_ignores_templates_with_malformed_template_handlers - %w(malformed malformed.erb malformed.html.erb malformed.en.html.erb).each do |name| - assert_raise(ActionView::MissingTemplate) { @view.render(:file => "test/malformed/#{name}") } + ActiveSupport::Deprecation.silence do + %w(malformed malformed.erb malformed.html.erb malformed.en.html.erb).each do |name| + assert_raise(ActionView::MissingTemplate) { @view.render(:file => "test/malformed/#{name}") } + end end end def test_render_with_layout assert_equal %(\nHello world!\n), - @view.render(:file => "test/hello_world.erb", :layout => "layouts/yield") + @view.render(:file => "test/hello_world", :layout => "layouts/yield") end def test_render_with_layout_which_has_render_inline assert_equal %(welcome\nHello world!\n), - @view.render(:file => "test/hello_world.erb", :layout => "layouts/yield_with_render_inline_inside") + @view.render(:file => "test/hello_world", :layout => "layouts/yield_with_render_inline_inside") end def test_render_with_layout_which_renders_another_partial assert_equal %(partial html\nHello world!\n), - @view.render(:file => "test/hello_world.erb", :layout => "layouts/yield_with_render_partial_inside") + @view.render(:file => "test/hello_world", :layout => "layouts/yield_with_render_partial_inside") end def test_render_layout_with_block_and_yield @@ -355,17 +357,17 @@ module RenderTestCases def test_render_with_nested_layout assert_equal %(title\n\n
column
\n
content
\n), - @view.render(:file => "test/nested_layout.erb", :layout => "layouts/yield") + @view.render(:file => "test/nested_layout", :layout => "layouts/yield") end def test_render_with_file_in_layout assert_equal %(\ntitle\n\n), - @view.render(:file => "test/layout_render_file.erb") + @view.render(:file => "test/layout_render_file") end def test_render_layout_with_object assert_equal %(David), - @view.render(:file => "test/layout_render_object.erb") + @view.render(:file => "test/layout_render_object") end end @@ -403,7 +405,7 @@ class LazyViewRenderTest < ActiveSupport::TestCase if '1.9'.respond_to?(:force_encoding) def test_render_utf8_template_with_magic_comment with_external_encoding Encoding::ASCII_8BIT do - result = @view.render(:file => "test/utf8_magic.html.erb", :layouts => "layouts/yield") + result = @view.render(:file => "test/utf8_magic.html", :layouts => "layouts/yield") assert_equal Encoding::UTF_8, result.encoding assert_equal "\nРусский \nтекст\n\nUTF-8\nUTF-8\nUTF-8\n", result end @@ -411,7 +413,7 @@ class LazyViewRenderTest < ActiveSupport::TestCase def test_render_utf8_template_with_default_external_encoding with_external_encoding Encoding::UTF_8 do - result = @view.render(:file => "test/utf8.html.erb", :layouts => "layouts/yield") + result = @view.render(:file => "test/utf8.html", :layouts => "layouts/yield") assert_equal Encoding::UTF_8, result.encoding assert_equal "Русский текст\n\nUTF-8\nUTF-8\nUTF-8\n", result end @@ -420,7 +422,7 @@ class LazyViewRenderTest < ActiveSupport::TestCase def test_render_utf8_template_with_incompatible_external_encoding with_external_encoding Encoding::SHIFT_JIS do begin - @view.render(:file => "test/utf8.html.erb", :layouts => "layouts/yield") + @view.render(:file => "test/utf8.html", :layouts => "layouts/yield") flunk 'Should have raised incompatible encoding error' rescue ActionView::Template::Error => error assert_match 'Your template was not saved as valid Shift_JIS', error.original_exception.message @@ -431,7 +433,7 @@ class LazyViewRenderTest < ActiveSupport::TestCase def test_render_utf8_template_with_partial_with_incompatible_encoding with_external_encoding Encoding::SHIFT_JIS do begin - @view.render(:file => "test/utf8_magic_with_bare_partial.html.erb", :layouts => "layouts/yield") + @view.render(:file => "test/utf8_magic_with_bare_partial.html", :layouts => "layouts/yield") flunk 'Should have raised incompatible encoding error' rescue ActionView::Template::Error => error assert_match 'Your template was not saved as valid Shift_JIS', error.original_exception.message diff --git a/actionpack/test/template/streaming_render_test.rb b/actionpack/test/template/streaming_render_test.rb index 023ce723ed..4d01352b43 100644 --- a/actionpack/test/template/streaming_render_test.rb +++ b/actionpack/test/template/streaming_render_test.rb @@ -28,7 +28,7 @@ class FiberedTest < ActiveSupport::TestCase def test_streaming_works content = [] - body = render_body(:template => "test/hello_world.erb", :layout => "layouts/yield") + body = render_body(:template => "test/hello_world", :layout => "layouts/yield") body.each do |piece| content << piece @@ -42,12 +42,12 @@ class FiberedTest < ActiveSupport::TestCase end def test_render_file - assert_equal "Hello world!", buffered_render(:file => "test/hello_world.erb") + assert_equal "Hello world!", buffered_render(:file => "test/hello_world") end def test_render_file_with_locals locals = { :secret => 'in the sauce' } - assert_equal "The secret is in the sauce\n", buffered_render(:file => "test/render_file_with_locals.erb", :locals => locals) + assert_equal "The secret is in the sauce\n", buffered_render(:file => "test/render_file_with_locals", :locals => locals) end def test_render_partial @@ -64,27 +64,27 @@ class FiberedTest < ActiveSupport::TestCase def test_render_with_layout assert_equal %(\nHello world!\n), - buffered_render(:template => "test/hello_world.erb", :layout => "layouts/yield") + buffered_render(:template => "test/hello_world", :layout => "layouts/yield") end def test_render_with_layout_which_has_render_inline assert_equal %(welcome\nHello world!\n), - buffered_render(:template => "test/hello_world.erb", :layout => "layouts/yield_with_render_inline_inside") + buffered_render(:template => "test/hello_world", :layout => "layouts/yield_with_render_inline_inside") end def test_render_with_layout_which_renders_another_partial assert_equal %(partial html\nHello world!\n), - buffered_render(:template => "test/hello_world.erb", :layout => "layouts/yield_with_render_partial_inside") + buffered_render(:template => "test/hello_world", :layout => "layouts/yield_with_render_partial_inside") end def test_render_with_nested_layout assert_equal %(title\n\n
column
\n
content
\n), - buffered_render(:template => "test/nested_layout.erb", :layout => "layouts/yield") + buffered_render(:template => "test/nested_layout", :layout => "layouts/yield") end def test_render_with_file_in_layout assert_equal %(\ntitle\n\n), - buffered_render(:template => "test/layout_render_file.erb") + buffered_render(:template => "test/layout_render_file") end def test_render_with_handler_without_streaming_support