From 9de83050d3a4b260d4aeb5d09ec4eb64f913ba64 Mon Sep 17 00:00:00 2001 From: Carlhuda Date: Mon, 15 Mar 2010 11:58:05 -0700 Subject: Add deprecation notices for <% %>. * The approach is to compile <% %> into a method call that checks whether the value returned from a block is a String. If it is, it concats to the buffer and prints a deprecation warning. * <%= %> uses exactly the same logic to compile the template, which first checks to see whether it's compiling a block. * This should have no impact on other uses of block in templates. For instance, in <% [1,2,3].each do |i| %><%= i %><% end %>, the call to each returns an Array, not a String, so the result is not concatenated * In two cases (#capture and #cache), a String can be returned that should *never* be concatenated. We have temporarily created a String subclass called NonConcattingString which behaves (and is serialized) identically to String, but is not concatenated by the code that handles deprecated <% %> block helpers. Once we remove support for <% %> block helpers, we can remove NonConcattingString. --- .../lib/action_controller/caching/fragments.rb | 16 ++++--- actionpack/lib/action_view/base.rb | 3 ++ actionpack/lib/action_view/helpers/cache_helper.rb | 2 +- .../lib/action_view/helpers/capture_helper.rb | 28 ++++++------ .../helpers/deprecated_block_helpers.rb | 52 ---------------------- .../lib/action_view/helpers/prototype_helper.rb | 4 ++ actionpack/lib/action_view/render/rendering.rb | 3 +- .../lib/action_view/template/handlers/erb.rb | 19 +++++++- actionpack/test/controller/caching_test.rb | 15 +------ .../test/fixtures/layouts/block_with_layout.erb | 4 +- .../fixtures/test/deprecated_nested_layout.erb | 3 ++ actionpack/test/fixtures/test/nested_layout.erb | 2 +- .../test/using_layout_around_block.html.erb | 2 +- actionpack/test/template/erb/tag_helper_test.rb | 24 +++++++--- actionpack/test/template/render_test.rb | 8 ++++ 15 files changed, 88 insertions(+), 97 deletions(-) delete mode 100644 actionpack/lib/action_view/helpers/deprecated_block_helpers.rb create mode 100644 actionpack/test/fixtures/test/deprecated_nested_layout.erb (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/caching/fragments.rb b/actionpack/lib/action_controller/caching/fragments.rb index 89787727bd..a07fe2b255 100644 --- a/actionpack/lib/action_controller/caching/fragments.rb +++ b/actionpack/lib/action_controller/caching/fragments.rb @@ -34,17 +34,23 @@ module ActionController #:nodoc: ActiveSupport::Cache.expand_cache_key(key.is_a?(Hash) ? url_for(key).split("://").last : key, :views) end - def fragment_for(buffer, name = {}, options = nil, &block) #:nodoc: + def fragment_for(name = {}, options = nil, &block) #:nodoc: if perform_caching if fragment_exist?(name, options) - buffer.safe_concat(read_fragment(name, options)) + read_fragment(name, options) else + # VIEW TODO: Make #capture usable outside of ERB + # This dance is needed because Builder can't use capture + buffer = view_context.output_buffer pos = buffer.length - block.call - write_fragment(name, buffer[pos..-1], options) + yield + fragment = buffer[pos..-1] + write_fragment(name, fragment, options) + ActionView::NonConcattingString.new(fragment) end else - block.call + ret = yield + ActiveSupport::SafeBuffer.new(ret) if ret.is_a?(String) end end diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index feaf45c333..3920d8593f 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -3,6 +3,9 @@ require 'active_support/core_ext/module/delegation' require 'active_support/core_ext/class/attribute' module ActionView #:nodoc: + class NonConcattingString < ActiveSupport::SafeBuffer + end + class ActionViewError < StandardError #:nodoc: end diff --git a/actionpack/lib/action_view/helpers/cache_helper.rb b/actionpack/lib/action_view/helpers/cache_helper.rb index d5cc14b29a..3729d7daa8 100644 --- a/actionpack/lib/action_view/helpers/cache_helper.rb +++ b/actionpack/lib/action_view/helpers/cache_helper.rb @@ -32,7 +32,7 @@ module ActionView # Topics listed alphabetically # <% end %> def cache(name = {}, options = nil, &block) - controller.fragment_for(output_buffer, name, options, &block) + controller.fragment_for(name, options, &block) end end end diff --git a/actionpack/lib/action_view/helpers/capture_helper.rb b/actionpack/lib/action_view/helpers/capture_helper.rb index 03c7ba5a87..b64dc1533f 100644 --- a/actionpack/lib/action_view/helpers/capture_helper.rb +++ b/actionpack/lib/action_view/helpers/capture_helper.rb @@ -2,22 +2,22 @@ module ActionView module Helpers # CaptureHelper exposes methods to let you extract generated markup which # can be used in other parts of a template or layout file. - # It provides a method to capture blocks into variables through capture and + # It provides a method to capture blocks into variables through capture and # a way to capture a block of markup for use in a layout through content_for. module CaptureHelper - # The capture method allows you to extract part of a template into a - # variable. You can then use this variable anywhere in your templates or layout. - # + # The capture method allows you to extract part of a template into a + # variable. You can then use this variable anywhere in your templates or layout. + # # ==== Examples # The capture method can be used in ERb templates... - # + # # <% @greeting = capture do %> # Welcome to my shiny new web page! The date and time is # <%= Time.now %> # <% end %> # # ...and Builder (RXML) templates. - # + # # @timestamp = capture do # "The current timestamp is #{Time.now}." # end @@ -33,15 +33,17 @@ module ActionView def capture(*args) value = nil buffer = with_output_buffer { value = yield *args } - buffer.presence || value + if string = buffer.presence || value and string.is_a?(String) + NonConcattingString.new(string) + end end # Calling content_for stores a block of markup in an identifier for later use. # You can make subsequent calls to the stored content in other templates or the layout # by passing the identifier as an argument to yield. - # + # # ==== Examples - # + # # <% content_for :not_authorized do %> # alert('You are not authorized to do that!') # <% end %> @@ -92,7 +94,7 @@ module ActionView # <% end %> # # <%# Add some other content, or use a different template: %> - # + # # <% content_for :navigation do %> #
  • <%= link_to 'Login', :action => 'login' %>
  • # <% end %> @@ -109,13 +111,13 @@ module ActionView # for elements that will be fragment cached. def content_for(name, content = nil, &block) content = capture(&block) if block_given? - return @_content_for[name] << content if content - @_content_for[name] + @_content_for[name] << content if content + @_content_for[name] unless content end # content_for? simply checks whether any content has been captured yet using content_for # Useful to render parts of your layout differently based on what is in your views. - # + # # ==== Examples # # Perhaps you will use different css in you layout if no content_for :right_column diff --git a/actionpack/lib/action_view/helpers/deprecated_block_helpers.rb b/actionpack/lib/action_view/helpers/deprecated_block_helpers.rb deleted file mode 100644 index 3d0657e873..0000000000 --- a/actionpack/lib/action_view/helpers/deprecated_block_helpers.rb +++ /dev/null @@ -1,52 +0,0 @@ -module ActionView - module Helpers - module DeprecatedBlockHelpers - extend ActiveSupport::Concern - - include ActionView::Helpers::TagHelper - include ActionView::Helpers::TextHelper - include ActionView::Helpers::JavaScriptHelper - include ActionView::Helpers::FormHelper - - def content_tag(*, &block) - block_called_from_erb?(block) ? safe_concat(super) : super - end - - def javascript_tag(*, &block) - block_called_from_erb?(block) ? safe_concat(super) : super - end - - def form_for(*, &block) - block_called_from_erb?(block) ? safe_concat(super) : super - end - - def form_tag(*, &block) - block_called_from_erb?(block) ? safe_concat(super) : super - end - - def fields_for(*, &block) - block_called_from_erb?(block) ? safe_concat(super) : super - end - - def field_set_tag(*, &block) - block_called_from_erb?(block) ? safe_concat(super) : super - end - - BLOCK_CALLED_FROM_ERB = 'defined? __in_erb_template' - - if RUBY_VERSION < '1.9.0' - # Check whether we're called from an erb template. - # We'd return a string in any other case, but erb <%= ... %> - # can't take an <% end %> later on, so we have to use <% ... %> - # and implicitly concat. - def block_called_from_erb?(block) - block && eval(BLOCK_CALLED_FROM_ERB, block) - end - else - def block_called_from_erb?(block) - block && eval(BLOCK_CALLED_FROM_ERB, block.binding) - end - end - end - end -end \ No newline at end of file diff --git a/actionpack/lib/action_view/helpers/prototype_helper.rb b/actionpack/lib/action_view/helpers/prototype_helper.rb index e58fdf81fb..e46ca53275 100644 --- a/actionpack/lib/action_view/helpers/prototype_helper.rb +++ b/actionpack/lib/action_view/helpers/prototype_helper.rb @@ -689,6 +689,10 @@ module ActionView @generator << root if root end + def is_a?(klass) + klass == JavaScriptProxy + end + private def method_missing(method, *arguments, &block) if method.to_s =~ /(.*)=$/ diff --git a/actionpack/lib/action_view/render/rendering.rb b/actionpack/lib/action_view/render/rendering.rb index 310efe40e2..d8dfd5077b 100644 --- a/actionpack/lib/action_view/render/rendering.rb +++ b/actionpack/lib/action_view/render/rendering.rb @@ -16,8 +16,7 @@ module ActionView case options when Hash if block_given? - content = _render_partial(options.merge(:partial => options[:layout]), &block) - safe_concat(content) + _render_partial(options.merge(:partial => options[:layout]), &block) elsif options.key?(:partial) _render_partial(options) else diff --git a/actionpack/lib/action_view/template/handlers/erb.rb b/actionpack/lib/action_view/template/handlers/erb.rb index ac5902cc0e..705c2bf82e 100644 --- a/actionpack/lib/action_view/template/handlers/erb.rb +++ b/actionpack/lib/action_view/template/handlers/erb.rb @@ -8,6 +8,13 @@ module ActionView super(value.to_s) end alias :append= :<< + + def append_if_string=(value) + if value.is_a?(String) && !value.is_a?(NonConcattingString) + ActiveSupport::Deprecation.warn("<% %> style block helpers are deprecated. Please use <%= %>", caller) + self << value + end + end end module Template::Handlers @@ -21,14 +28,24 @@ module ActionView src << "@output_buffer.safe_concat('" << escape_text(text) << "');" end + BLOCK_EXPR = /(do|\{)(\s*\|[^|]*\|)?\s*\Z/ + def add_expr_literal(src, code) - if code =~ /(do|\{)(\s*\|[^|]*\|)?\s*\Z/ + if code =~ BLOCK_EXPR src << '@output_buffer.append= ' << code else src << '@output_buffer.append= (' << code << ');' end end + def add_stmt(src, code) + if code =~ BLOCK_EXPR + src << '@output_buffer.append_if_string= ' << code + else + super + end + end + def add_expr_escaped(src, code) src << '@output_buffer.append= ' << escaped_expr(code) << ';' end diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index a792752ef4..432433f976 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -617,7 +617,7 @@ class FragmentCachingTest < ActionController::TestCase fragment_computed = false buffer = 'generated till now -> '.html_safe - @controller.fragment_for(buffer, 'expensive') { fragment_computed = true } + buffer << @controller.fragment_for('expensive') { fragment_computed = true } assert fragment_computed assert_equal 'generated till now -> ', buffer @@ -628,7 +628,7 @@ class FragmentCachingTest < ActionController::TestCase fragment_computed = false buffer = 'generated till now -> '.html_safe - @controller.fragment_for(buffer, 'expensive') { fragment_computed = true } + buffer << @controller.fragment_for('expensive') { fragment_computed = true } assert !fragment_computed assert_equal 'generated till now -> fragment content', buffer @@ -742,15 +742,4 @@ CACHED assert_equal "

    Builder

    \n", @store.read('views/test.host/functional_caching/formatted_fragment_cached') end - - def test_js_formatted_fragment_caching - get :formatted_fragment_cached, :format => "js" - assert_response :success - expected_body = %(title = "Hey";\n$("element_1").visualEffect("highlight");\n) + - %($("element_2").visualEffect("highlight");\nfooter = "Bye";) - assert_equal expected_body, @response.body - - assert_equal ['$("element_1").visualEffect("highlight");', '$("element_2").visualEffect("highlight");'], - @store.read('views/test.host/functional_caching/formatted_fragment_cached') - end end diff --git a/actionpack/test/fixtures/layouts/block_with_layout.erb b/actionpack/test/fixtures/layouts/block_with_layout.erb index f25b41271d..73ac833e52 100644 --- a/actionpack/test/fixtures/layouts/block_with_layout.erb +++ b/actionpack/test/fixtures/layouts/block_with_layout.erb @@ -1,3 +1,3 @@ -<% render(:layout => "layout_for_partial", :locals => { :name => "Anthony" }) do %>Inside from first block in layout<% "Return value should be discarded" %><% end %> +<%= render(:layout => "layout_for_partial", :locals => { :name => "Anthony" }) do %>Inside from first block in layout<% "Return value should be discarded" %><% end %> <%= yield %> -<% render(:layout => "layout_for_partial", :locals => { :name => "Ramm" }) do %>Inside from second block in layout<% end %> +<%= render(:layout => "layout_for_partial", :locals => { :name => "Ramm" }) do %>Inside from second block in layout<% end %> diff --git a/actionpack/test/fixtures/test/deprecated_nested_layout.erb b/actionpack/test/fixtures/test/deprecated_nested_layout.erb new file mode 100644 index 0000000000..7b6dcbb6c7 --- /dev/null +++ b/actionpack/test/fixtures/test/deprecated_nested_layout.erb @@ -0,0 +1,3 @@ +<% content_for :title, "title" -%> +<% content_for :column do -%>column<% end -%> +<% render :layout => 'layouts/column' do -%>content<% end -%> \ No newline at end of file diff --git a/actionpack/test/fixtures/test/nested_layout.erb b/actionpack/test/fixtures/test/nested_layout.erb index 7b6dcbb6c7..6078f74b4c 100644 --- a/actionpack/test/fixtures/test/nested_layout.erb +++ b/actionpack/test/fixtures/test/nested_layout.erb @@ -1,3 +1,3 @@ <% content_for :title, "title" -%> <% content_for :column do -%>column<% end -%> -<% render :layout => 'layouts/column' do -%>content<% end -%> \ No newline at end of file +<%= render :layout => 'layouts/column' do -%>content<% end -%> \ No newline at end of file diff --git a/actionpack/test/fixtures/test/using_layout_around_block.html.erb b/actionpack/test/fixtures/test/using_layout_around_block.html.erb index a93fa02c9c..3d6661df9a 100644 --- a/actionpack/test/fixtures/test/using_layout_around_block.html.erb +++ b/actionpack/test/fixtures/test/using_layout_around_block.html.erb @@ -1 +1 @@ -<% render(:layout => "layout_for_partial", :locals => { :name => "David" }) do %>Inside from block<% end %> \ No newline at end of file +<%= render(:layout => "layout_for_partial", :locals => { :name => "David" }) do %>Inside from block<% end %> \ No newline at end of file diff --git a/actionpack/test/template/erb/tag_helper_test.rb b/actionpack/test/template/erb/tag_helper_test.rb index cc96a43901..b5b7ae87de 100644 --- a/actionpack/test/template/erb/tag_helper_test.rb +++ b/actionpack/test/template/erb/tag_helper_test.rb @@ -20,7 +20,7 @@ module ERBTest end class DeprecatedViewContext < ViewContext - include ActionView::Helpers::DeprecatedBlockHelpers + # include ActionView::Helpers::DeprecatedBlockHelpers end module SharedTagHelpers @@ -31,28 +31,36 @@ module ERBTest ActionView::Template::Handlers::Erubis.new(template).evaluate(context.new) end + def maybe_deprecated + if @deprecated + assert_deprecated { yield } + else + yield + end + end + test "percent equals works for content_tag and does not require parenthesis on method call" do - assert_equal "
    Hello world
    ", render_content("content_tag :div", "Hello world") + maybe_deprecated { assert_equal "
    Hello world
    ", render_content("content_tag :div", "Hello world") } end test "percent equals works for javascript_tag" do expected_output = "" - assert_equal expected_output, render_content("javascript_tag", "alert('Hello')") + maybe_deprecated { assert_equal expected_output, render_content("javascript_tag", "alert('Hello')") } end test "percent equals works for javascript_tag with options" do expected_output = "" - assert_equal expected_output, render_content("javascript_tag(:id => 'the_js_tag')", "alert('Hello')") + maybe_deprecated { assert_equal expected_output, render_content("javascript_tag(:id => 'the_js_tag')", "alert('Hello')") } end test "percent equals works with form tags" do expected_output = "
    hello
    " - assert_equal expected_output, render_content("form_tag('foo')", "<%= 'hello' %>") + maybe_deprecated { assert_equal expected_output, render_content("form_tag('foo')", "<%= 'hello' %>") } end test "percent equals works with fieldset tags" do expected_output = "
    foohello
    " - assert_equal expected_output, render_content("field_set_tag('foo')", "<%= 'hello' %>") + maybe_deprecated { assert_equal expected_output, render_content("field_set_tag('foo')", "<%= 'hello' %>") } end end @@ -77,6 +85,10 @@ module ERBTest "<% __in_erb_template=true %><% #{str} do %>#{rest}<% end %>" end + def setup + @deprecated = true + end + include SharedTagHelpers end end \ No newline at end of file diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index cea8ab1bce..e66202e8fe 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -228,6 +228,14 @@ module RenderTestCases @view.render(:file => "test/hello_world.erb", :layout => "layouts/yield") end + # TODO: Move to deprecated_tests.rb + def test_render_with_nested_layout_deprecated + assert_deprecated do + assert_equal %(title\n\n\n
    column
    \n
    content
    \n), + @view.render(:file => "test/deprecated_nested_layout.erb", :layout => "layouts/yield") + end + end + def test_render_with_nested_layout assert_equal %(title\n\n\n
    column
    \n
    content
    \n), @view.render(:file => "test/nested_layout.erb", :layout => "layouts/yield") -- cgit v1.2.3 From 5913dd478150710fc0b72a0568f68e13c958d6bc Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 15 Mar 2010 15:54:37 -0700 Subject: Reinstate old default_url_options method signature --- actionpack/lib/action_dispatch/routing/url_for.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index ec78f53fa6..9a64979074 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -100,7 +100,7 @@ module ActionDispatch end def url_options - default_url_options + default_url_options({}) end # Generate a url based on the options provided, default_url_options and the -- cgit v1.2.3 From c5a877f1421744096ce6ffbfce3fc1034bda880e Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Tue, 16 Mar 2010 00:22:22 +0100 Subject: adds test coverage for with_output_buffer Signed-off-by: Jeremy Kemper --- actionpack/test/template/capture_helper_test.rb | 48 +++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 3 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/template/capture_helper_test.rb b/actionpack/test/template/capture_helper_test.rb index f887c9ab5b..e65f8b15ed 100644 --- a/actionpack/test/template/capture_helper_test.rb +++ b/actionpack/test/template/capture_helper_test.rb @@ -3,6 +3,7 @@ require 'abstract_unit' class CaptureHelperTest < ActionView::TestCase def setup super + @av = ActionView::Base.new @_content_for = Hash.new {|h,k| h[k] = "" } end @@ -13,9 +14,50 @@ class CaptureHelperTest < ActionView::TestCase assert ! content_for?(:something_else) end + def test_with_output_buffer_swaps_the_output_buffer_given_no_argument + assert_nil @av.output_buffer + buffer = @av.with_output_buffer do + @av.output_buffer << '.' + end + assert_equal '.', buffer + assert_nil @av.output_buffer + end + + def test_with_output_buffer_swaps_the_output_buffer_with_an_argument + assert_nil @av.output_buffer + buffer = ActionView::OutputBuffer.new('.') + @av.with_output_buffer(buffer) do + @av.output_buffer << '.' + end + assert_equal '..', buffer + assert_nil @av.output_buffer + end + + def test_with_output_buffer_restores_the_output_buffer + buffer = ActionView::OutputBuffer.new + @av.output_buffer = buffer + @av.with_output_buffer do + @av.output_buffer << '.' + end + assert buffer.equal?(@av.output_buffer) + end + + unless RUBY_VERSION < '1.9' + def test_with_output_buffer_sets_proper_encoding + @av.output_buffer = ActionView::OutputBuffer.new + + # Ensure we set the output buffer to an encoding different than the default one. + alt_encoding = @av.output_buffer.encoding == Encoding::US_ASCII ? Encoding::UTF_8 : Encoding::US_ASCII + @av.output_buffer.force_encoding(alt_encoding) + + @av.with_output_buffer do + assert alt_encoding, @av.output_buffer.encoding + end + end + end + def test_with_output_buffer_does_not_assume_there_is_an_output_buffer - av = ActionView::Base.new - assert_nil av.output_buffer - assert_equal "", av.with_output_buffer {} + assert_nil @av.output_buffer + assert_equal "", @av.with_output_buffer {} end end -- cgit v1.2.3 From 1f5e2f2bad3c33ec52312a1700bacf06a71875a5 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 15 Mar 2010 16:49:03 -0700 Subject: Revert "Reinstate old default_url_options method signature" This reverts commit 5913dd478150710fc0b72a0568f68e13c958d6bc. --- actionpack/lib/action_dispatch/routing/url_for.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index 9a64979074..ec78f53fa6 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -100,7 +100,7 @@ module ActionDispatch end def url_options - default_url_options({}) + default_url_options end # Generate a url based on the options provided, default_url_options and the -- cgit v1.2.3 From 8dd731bc502a07f4fb76eb2706a1f3bca479ef63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Tue, 16 Mar 2010 02:08:34 +0100 Subject: Move more normalization up to the lookup context, so it does not have to repeat in every resolver. --- actionpack/lib/action_view/lookup_context.rb | 36 ++++++++++++++++++++----- actionpack/lib/action_view/template/resolver.rb | 24 +---------------- actionpack/test/lib/fixture_template.rb | 2 +- actionpack/test/template/lookup_context_test.rb | 16 +++++------ 4 files changed, 40 insertions(+), 38 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/lookup_context.rb b/actionpack/lib/action_view/lookup_context.rb index 8eb17bf8f1..598007c7a4 100644 --- a/actionpack/lib/action_view/lookup_context.rb +++ b/actionpack/lib/action_view/lookup_context.rb @@ -15,12 +15,10 @@ module ActionView def self.register_detail(name, options = {}, &block) self.registered_details << name - Setters.send :define_method, :"_#{name}_defaults", &block Setters.module_eval <<-METHOD, __FILE__, __LINE__ + 1 def #{name}=(value) value = Array(value.presence || _#{name}_defaults) - #{"value << nil unless value.include?(nil)" unless options[:allow_nil] == false} unless value == @details[:#{name}] @details_key, @details = nil, @details.merge(:#{name} => value) @@ -69,16 +67,16 @@ module ActionView end def find(name, prefix = nil, partial = false) - @view_paths.find(name, prefix, partial, details, details_key) + @view_paths.find(*args_for_lookup(name, prefix, partial)) end alias :find_template :find def find_all(name, prefix = nil, partial = false) - @view_paths.find_all(name, prefix, partial, details, details_key) + @view_paths.find_all(*args_for_lookup(name, prefix, partial)) end def exists?(name, prefix = nil, partial = false) - @view_paths.exists?(name, prefix, partial, details, details_key) + @view_paths.exists?(*args_for_lookup(name, prefix, partial)) end alias :template_exists? :exists? @@ -94,6 +92,32 @@ module ActionView ensure added_resolvers.times { view_paths.pop } end + + protected + + def args_for_lookup(name, prefix, partial) #:nodoc: + name, prefix = normalize_name(name, prefix) + details_key = self.details_key + details = self.details.merge(:handlers => default_handlers) + [name, prefix, partial || false, details, details_key] + end + + # Support legacy foo.erb names even though we now ignore .erb + # as well as incorrectly putting part of the path in the template + # name instead of the prefix. + def normalize_name(name, prefix) #:nodoc: + name = name.to_s.gsub(handlers_regexp, '') + parts = name.split('/') + return parts.pop, [prefix, *parts].compact.join("/") + end + + def default_handlers #:nodoc: + @detault_handlers ||= Template::Handlers.extensions + end + + def handlers_regexp #:nodoc: + @handlers_regexp ||= /\.(?:#{default_handlers.join('|')})$/ + end end module Details @@ -113,7 +137,7 @@ module ActionView end # Overload formats= to reject [:"*/*"] values. - def formats=(value, freeze=true) + def formats=(value) value = nil if value == [:"*/*"] super(value) end diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb index a43597e728..ee13707327 100644 --- a/actionpack/lib/action_view/template/resolver.rb +++ b/actionpack/lib/action_view/template/resolver.rb @@ -14,15 +14,8 @@ module ActionView @cached.clear end - def find(*args) - find_all(*args).first - end - # Normalizes the arguments and passes it on to find_template. def find_all(name, prefix=nil, partial=false, details={}, key=nil) - name, prefix = normalize_name(name, prefix) - details = details.merge(:handlers => default_handlers) - cached(key, prefix, name, partial) do find_templates(name, prefix, partial, details) end @@ -34,10 +27,6 @@ module ActionView @caching ||= !defined?(Rails.application) || Rails.application.config.cache_classes end - def default_handlers - Template::Handlers.extensions + [nil] - end - # This is what child classes implement. No defaults are needed # because Resolver guarantees that the arguments are present and # normalized. @@ -45,17 +34,6 @@ module ActionView raise NotImplementedError end - # Support legacy foo.erb names even though we now ignore .erb - # as well as incorrectly putting part of the path in the template - # name instead of the prefix. - def normalize_name(name, prefix) - handlers = Template::Handlers.extensions.join('|') - name = name.to_s.gsub(/\.(?:#{handlers})$/, '') - - parts = name.split('/') - return parts.pop, [prefix, *parts].compact.join("/") - end - def cached(key, prefix, name, partial) return yield unless key && caching? scope = @cached[key][prefix][name] @@ -93,7 +71,7 @@ module ActionView query = File.join(@path, path) exts.each do |ext| - query << '{' << ext.map {|e| e && ".#{e}" }.join(',') << '}' + query << '{' << ext.map {|e| e && ".#{e}" }.join(',') << ',}' end Dir[query].reject { |p| File.directory?(p) }.map do |p| diff --git a/actionpack/test/lib/fixture_template.rb b/actionpack/test/lib/fixture_template.rb index 02248d84ad..2e876d9249 100644 --- a/actionpack/test/lib/fixture_template.rb +++ b/actionpack/test/lib/fixture_template.rb @@ -12,7 +12,7 @@ module ActionView #:nodoc: def query(partial, path, exts) query = Regexp.escape(path) exts.each do |ext| - query << '(' << ext.map {|e| e && Regexp.escape(".#{e}") }.join('|') << ')' + query << '(' << ext.map {|e| e && Regexp.escape(".#{e}") }.join('|') << '|)' end templates = [] diff --git a/actionpack/test/template/lookup_context_test.rb b/actionpack/test/template/lookup_context_test.rb index 697ebc694a..37526957bc 100644 --- a/actionpack/test/template/lookup_context_test.rb +++ b/actionpack/test/template/lookup_context_test.rb @@ -22,14 +22,14 @@ class LookupContextTest < ActiveSupport::TestCase end test "normalizes details on initialization" do - formats = Mime::SET + [nil] - locale = [I18n.locale, nil] + formats = Mime::SET + locale = [I18n.locale] assert_equal Hash[:formats => formats, :locale => locale], @lookup_context.details end test "allows me to set details" do @lookup_context.details = { :formats => [:html], :locale => :pt } - assert_equal Hash[:formats => [:html, nil], :locale => [:pt, nil]], @lookup_context.details + assert_equal Hash[:formats => [:html], :locale => [:pt]], @lookup_context.details end test "does not allow details to be modified in place" do @@ -39,17 +39,17 @@ class LookupContextTest < ActiveSupport::TestCase test "allows me to update an specific detail" do @lookup_context.update_details(:locale => :pt) assert_equal :pt, I18n.locale - formats = Mime::SET + [nil] - locale = [I18n.locale, nil] + formats = Mime::SET + locale = [I18n.locale] assert_equal Hash[:formats => formats, :locale => locale], @lookup_context.details end test "allows me to change some details to execute an specific block of code" do - formats = Mime::SET + [nil] + formats = Mime::SET @lookup_context.update_details(:locale => :pt) do - assert_equal Hash[:formats => formats, :locale => [:pt, nil]], @lookup_context.details + assert_equal Hash[:formats => formats, :locale => [:pt]], @lookup_context.details end - assert_equal Hash[:formats => formats, :locale => [:en, nil]], @lookup_context.details + assert_equal Hash[:formats => formats, :locale => [:en]], @lookup_context.details end test "provides getters and setters for formats" do -- cgit v1.2.3 From 2a50eabf4576580dff9b43c3d830cd78e8fbb353 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 15 Mar 2010 19:26:58 -0700 Subject: Integration test url options should account for :protocol not just https? --- actionpack/lib/action_dispatch/testing/integration.rb | 10 ++-------- actionpack/test/dispatch/url_generation_test.rb | 7 ++++++- 2 files changed, 8 insertions(+), 9 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 0aff4250c1..31067e56b4 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -177,14 +177,8 @@ module ActionDispatch reset! end - def url_options - opts = super.reverse_merge( - :host => host, - :protocol => https? ? "https" : "http" - ) - - opts.merge!(:port => 443) if !opts.key?(:port) && https? - opts + def default_url_options + { :host => host, :protocol => https? ? "https" : "http" } end # Resets the instance. This can be used to reset the state information diff --git a/actionpack/test/dispatch/url_generation_test.rb b/actionpack/test/dispatch/url_generation_test.rb index 18b5b7ee00..b863e37a33 100644 --- a/actionpack/test/dispatch/url_generation_test.rb +++ b/actionpack/test/dispatch/url_generation_test.rb @@ -34,5 +34,10 @@ module TestUrlGeneration get "/foo", {}, 'SCRIPT_NAME' => "/new" assert_equal "/new/foo", response.body end + + test "handling http protocol with https set" do + https! + assert_equal "http://www.example.com/bar/foo", foo_url(:protocol => "http") + end end -end \ No newline at end of file +end -- cgit v1.2.3 From b9c48f519fbeee8fda3c989204619648f794fb02 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 15 Mar 2010 22:44:49 -0700 Subject: Swear I ran this test --- actionpack/test/dispatch/url_generation_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/test/dispatch/url_generation_test.rb b/actionpack/test/dispatch/url_generation_test.rb index b863e37a33..326b336a1a 100644 --- a/actionpack/test/dispatch/url_generation_test.rb +++ b/actionpack/test/dispatch/url_generation_test.rb @@ -37,7 +37,7 @@ module TestUrlGeneration test "handling http protocol with https set" do https! - assert_equal "http://www.example.com/bar/foo", foo_url(:protocol => "http") + assert_equal "http://www.example.com/foo", foo_url(:protocol => "http") end end end -- cgit v1.2.3 From 9e1e95f70af3c566190f47ee7a52fd49f785ec12 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 15 Mar 2010 23:05:12 -0700 Subject: link_to_remote -> link_to :remote => true --- actionpack/lib/action_view/helpers/capture_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/capture_helper.rb b/actionpack/lib/action_view/helpers/capture_helper.rb index b64dc1533f..42a67756e4 100644 --- a/actionpack/lib/action_view/helpers/capture_helper.rb +++ b/actionpack/lib/action_view/helpers/capture_helper.rb @@ -77,7 +77,7 @@ module ActionView # # Then, in another view, you could to do something like this: # - # <%= link_to_remote 'Logout', :action => 'logout' %> + # <%= link_to 'Logout', :action => 'logout', :remote => true %> # # <% content_for :script do %> # <%= javascript_include_tag :defaults %> -- cgit v1.2.3 From b3b6ff48dff49ebbdab0a53f576bc0572116767f Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 15 Mar 2010 23:26:48 -0700 Subject: Fix link_to with block --- actionpack/lib/action_view/helpers/url_helper.rb | 2 +- actionpack/test/template/url_helper_test.rb | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index 14d59034f1..94f1cecade 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -206,7 +206,7 @@ module ActionView if block_given? options = args.first || {} html_options = args.second - safe_concat(link_to(capture(&block), options, html_options)) + link_to(capture(&block), options, html_options) else name = args[0] options = args[1] || {} diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb index 165cb655da..87b2e59255 100644 --- a/actionpack/test/template/url_helper_test.rb +++ b/actionpack/test/template/url_helper_test.rb @@ -238,10 +238,7 @@ class UrlHelperTest < ActionView::TestCase end def test_link_tag_using_block_in_erb - __in_erb_template = '' - - link_to("http://example.com") { concat("Example site") } - + output_buffer = link_to("http://example.com") { concat("Example site") } assert_equal 'Example site', output_buffer end -- cgit v1.2.3 From 748c78ffc8b11a608745290e7d68bdf63720cfab Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 15 Mar 2010 23:48:32 -0700 Subject: RJS may cache an array --- actionpack/lib/action_controller/caching/fragments.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/caching/fragments.rb b/actionpack/lib/action_controller/caching/fragments.rb index a07fe2b255..19bf3ddd3b 100644 --- a/actionpack/lib/action_controller/caching/fragments.rb +++ b/actionpack/lib/action_controller/caching/fragments.rb @@ -46,7 +46,7 @@ module ActionController #:nodoc: yield fragment = buffer[pos..-1] write_fragment(name, fragment, options) - ActionView::NonConcattingString.new(fragment) + fragment.is_a?(String) ? ActionView::NonConcattingString.new(fragment) : fragment end else ret = yield -- cgit v1.2.3 From b65b989725320dc9e424bab7536f062d0dc94b0f Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 15 Mar 2010 23:48:46 -0700 Subject: Break a window :/ --- actionpack/test/controller/caching_test.rb | 2 +- actionpack/test/fixtures/functional_caching/_partial.erb | 4 ++-- .../fixtures/functional_caching/formatted_fragment_cached.html.erb | 4 ++-- actionpack/test/fixtures/functional_caching/fragment_cached.html.erb | 2 +- .../test/fixtures/functional_caching/inline_fragment_cached.html.erb | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 432433f976..2f9cc44308 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -726,7 +726,7 @@ CACHED def test_html_formatted_fragment_caching get :formatted_fragment_cached, :format => "html" assert_response :success - expected_body = "\n

    ERB

    \n" + expected_body = "\n

    ERB

    \n\n" assert_equal expected_body, @response.body diff --git a/actionpack/test/fixtures/functional_caching/_partial.erb b/actionpack/test/fixtures/functional_caching/_partial.erb index d0e4f72b95..ec0da7cf50 100644 --- a/actionpack/test/fixtures/functional_caching/_partial.erb +++ b/actionpack/test/fixtures/functional_caching/_partial.erb @@ -1,3 +1,3 @@ <% cache do %> -Fragment caching in a partial -<% end %> \ No newline at end of file +Old fragment caching in a partial +<% end %> diff --git a/actionpack/test/fixtures/functional_caching/formatted_fragment_cached.html.erb b/actionpack/test/fixtures/functional_caching/formatted_fragment_cached.html.erb index d7f43ad95e..9b88fa1f5a 100644 --- a/actionpack/test/fixtures/functional_caching/formatted_fragment_cached.html.erb +++ b/actionpack/test/fixtures/functional_caching/formatted_fragment_cached.html.erb @@ -1,3 +1,3 @@ -<% cache do %>

    ERB

    <% end %> - \ No newline at end of file +<%= cache do %>

    ERB

    <% end %> + diff --git a/actionpack/test/fixtures/functional_caching/fragment_cached.html.erb b/actionpack/test/fixtures/functional_caching/fragment_cached.html.erb index 268a298a42..c479adb897 100644 --- a/actionpack/test/fixtures/functional_caching/fragment_cached.html.erb +++ b/actionpack/test/fixtures/functional_caching/fragment_cached.html.erb @@ -1,2 +1,2 @@ Hello -<% cache do %>This bit's fragment cached<% end %> +<%= cache do %>This bit's fragment cached<% end %> diff --git a/actionpack/test/fixtures/functional_caching/inline_fragment_cached.html.erb b/actionpack/test/fixtures/functional_caching/inline_fragment_cached.html.erb index 87309b8ccb..41647f1404 100644 --- a/actionpack/test/fixtures/functional_caching/inline_fragment_cached.html.erb +++ b/actionpack/test/fixtures/functional_caching/inline_fragment_cached.html.erb @@ -1,2 +1,2 @@ <%= render :inline => 'Some inline content' %> -<% cache do %>Some cached content<% end %> +<%= cache do %>Some cached content<% end %> -- cgit v1.2.3 From e13c179499f227071cbe829221877e1c0d03c0b1 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Mon, 8 Mar 2010 19:55:57 -0200 Subject: Change array entries to safe doesn't worth then the array is joined as a string losing the safe property of his entries [#4134 status:resolved] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- actionpack/lib/action_view/helpers/translation_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/translation_helper.rb b/actionpack/lib/action_view/helpers/translation_helper.rb index 8a89ee58a0..f996762d6f 100644 --- a/actionpack/lib/action_view/helpers/translation_helper.rb +++ b/actionpack/lib/action_view/helpers/translation_helper.rb @@ -13,7 +13,7 @@ module ActionView def translate(key, options = {}) options[:raise] = true translation = I18n.translate(scope_key_by_partial(key), options) - translation.is_a?(Array) ? translation.map { |entry| entry.html_safe } : translation.html_safe + translation.respond_to?(:html_safe) ? translation.html_safe : translation rescue I18n::MissingTranslationData => e keys = I18n.normalize_keys(e.locale, e.key, e.options[:scope]) content_tag('span', keys.join(', '), :class => 'translation_missing') -- cgit v1.2.3 From c61ed70b00c93bdf42c7538a334f07e58c60bc4e Mon Sep 17 00:00:00 2001 From: Carlhuda Date: Tue, 16 Mar 2010 11:43:04 -0700 Subject: Some more tweaks on <% %>. * The cache helper is now semantically "mark this region for caching" * As a result, <% x = cache do %> no longer works --- actionpack/lib/action_controller/caching/fragments.rb | 3 +-- actionpack/lib/action_view/helpers/cache_helper.rb | 3 ++- actionpack/lib/action_view/helpers/text_helper.rb | 2 +- actionpack/test/controller/caching_test.rb | 8 ++++---- 4 files changed, 8 insertions(+), 8 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/caching/fragments.rb b/actionpack/lib/action_controller/caching/fragments.rb index 19bf3ddd3b..8a10bdfe23 100644 --- a/actionpack/lib/action_controller/caching/fragments.rb +++ b/actionpack/lib/action_controller/caching/fragments.rb @@ -44,9 +44,8 @@ module ActionController #:nodoc: buffer = view_context.output_buffer pos = buffer.length yield - fragment = buffer[pos..-1] + fragment = buffer.slice!(pos..-1) write_fragment(name, fragment, options) - fragment.is_a?(String) ? ActionView::NonConcattingString.new(fragment) : fragment end else ret = yield diff --git a/actionpack/lib/action_view/helpers/cache_helper.rb b/actionpack/lib/action_view/helpers/cache_helper.rb index 3729d7daa8..f5c2127d3f 100644 --- a/actionpack/lib/action_view/helpers/cache_helper.rb +++ b/actionpack/lib/action_view/helpers/cache_helper.rb @@ -32,7 +32,8 @@ module ActionView # Topics listed alphabetically # <% end %> def cache(name = {}, options = nil, &block) - controller.fragment_for(name, options, &block) + safe_concat controller.fragment_for(name, options, &block) + nil end end end diff --git a/actionpack/lib/action_view/helpers/text_helper.rb b/actionpack/lib/action_view/helpers/text_helper.rb index b19a9754f4..b5bf813e07 100644 --- a/actionpack/lib/action_view/helpers/text_helper.rb +++ b/actionpack/lib/action_view/helpers/text_helper.rb @@ -29,7 +29,7 @@ module ActionView end def safe_concat(string) - output_buffer.safe_concat(string) + output_buffer.respond_to?(:safe_concat) ? output_buffer.safe_concat(string) : concat(string) end # Truncates a given +text+ after a given :length if +text+ is longer than :length diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 2f9cc44308..f6264c0954 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -704,8 +704,8 @@ CACHED def test_fragment_caching_in_partials get :html_fragment_cached_with_partial assert_response :success - assert_match /Fragment caching in a partial/, @response.body - assert_match "Fragment caching in a partial", @store.read('views/test.host/functional_caching/html_fragment_cached_with_partial') + assert_match /Old fragment caching in a partial/, @response.body + assert_match "Old fragment caching in a partial", @store.read('views/test.host/functional_caching/html_fragment_cached_with_partial') end def test_render_inline_before_fragment_caching @@ -719,8 +719,8 @@ CACHED def test_fragment_caching_in_rjs_partials xhr :get, :js_fragment_cached_with_partial assert_response :success - assert_match /Fragment caching in a partial/, @response.body - assert_match "Fragment caching in a partial", @store.read('views/test.host/functional_caching/js_fragment_cached_with_partial') + assert_match /Old fragment caching in a partial/, @response.body + assert_match "Old fragment caching in a partial", @store.read('views/test.host/functional_caching/js_fragment_cached_with_partial') end def test_html_formatted_fragment_caching -- cgit v1.2.3 From 986cac73e3c56b3dfa22fd1464f6913e38d32cc3 Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Tue, 16 Mar 2010 20:33:38 +0100 Subject: adds tests for #capture Signed-off-by: Jeremy Kemper --- actionpack/test/template/capture_helper_test.rb | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) (limited to 'actionpack') diff --git a/actionpack/test/template/capture_helper_test.rb b/actionpack/test/template/capture_helper_test.rb index e65f8b15ed..c1e83fc04d 100644 --- a/actionpack/test/template/capture_helper_test.rb +++ b/actionpack/test/template/capture_helper_test.rb @@ -7,6 +7,29 @@ class CaptureHelperTest < ActionView::TestCase @_content_for = Hash.new {|h,k| h[k] = "" } end + def test_capture_captures_the_temporary_output_buffer_in_its_block + assert_nil @av.output_buffer + string = @av.capture do + @av.output_buffer << 'foo' + @av.output_buffer << 'bar' + end + assert_nil @av.output_buffer + assert_equal 'foobar', string + assert_kind_of ActionView::NonConcattingString, string + end + + def test_capture_captures_the_value_returned_by_the_block_in_the_temporary_buffer_is_blank + string = @av.capture('foo', 'bar') do |a, b| + a + b + end + assert_equal 'foobar', string + assert_kind_of ActionView::NonConcattingString, string + end + + def test_capture_returns_nil_if_the_returned_value_is_not_a_string + assert_nil @av.capture { 1 } + end + def test_content_for assert ! content_for?(:title) content_for :title, 'title' -- cgit v1.2.3 From 9659d18c9b76f6383854af0cd3a75abb6b1784ea Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Tue, 16 Mar 2010 21:05:01 +0100 Subject: adds tests for #flush_output_buffer [#4196 state:committed] Signed-off-by: Jeremy Kemper --- actionpack/test/template/capture_helper_test.rb | 39 +++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/template/capture_helper_test.rb b/actionpack/test/template/capture_helper_test.rb index c1e83fc04d..2216e6b578 100644 --- a/actionpack/test/template/capture_helper_test.rb +++ b/actionpack/test/template/capture_helper_test.rb @@ -18,7 +18,7 @@ class CaptureHelperTest < ActionView::TestCase assert_kind_of ActionView::NonConcattingString, string end - def test_capture_captures_the_value_returned_by_the_block_in_the_temporary_buffer_is_blank + def test_capture_captures_the_value_returned_by_the_block_if_the_temporary_buffer_is_blank string = @av.capture('foo', 'bar') do |a, b| a + b end @@ -70,7 +70,7 @@ class CaptureHelperTest < ActionView::TestCase @av.output_buffer = ActionView::OutputBuffer.new # Ensure we set the output buffer to an encoding different than the default one. - alt_encoding = @av.output_buffer.encoding == Encoding::US_ASCII ? Encoding::UTF_8 : Encoding::US_ASCII + alt_encoding = alt_encoding(@av.output_buffer) @av.output_buffer.force_encoding(alt_encoding) @av.with_output_buffer do @@ -83,4 +83,39 @@ class CaptureHelperTest < ActionView::TestCase assert_nil @av.output_buffer assert_equal "", @av.with_output_buffer {} end + + def test_flush_output_buffer_concats_output_buffer_to_response + view = view_with_controller + assert_equal [], view.response.body_parts + + view.output_buffer << 'OMG' + view.flush_output_buffer + assert_equal ['OMG'], view.response.body_parts + assert_equal '', view.output_buffer + + view.output_buffer << 'foobar' + view.flush_output_buffer + assert_equal ['OMG', 'foobar'], view.response.body_parts + assert_equal '', view.output_buffer + end + + unless RUBY_VERSION < '1.9' + def test_flush_output_buffer_preserves_the_encoding_of_the_output_buffer + view = view_with_controller + alt_encoding = alt_encoding(view.output_buffer) + view.output_buffer.force_encoding(alt_encoding) + flush_output_buffer + assert_equal alt_encoding, view.output_buffer.encoding + end + end + + def alt_encoding(output_buffer) + output_buffer.encoding == Encoding::US_ASCII ? Encoding::UTF_8 : Encoding::US_ASCII + end + + def view_with_controller + returning(ActionView::Base.for_controller(TestController.new)) do |view| + view.output_buffer = ActionView::OutputBuffer.new + end + end end -- cgit v1.2.3 From 12bf636461e3aab661119ceb3a104cfb70a11666 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Tue, 16 Mar 2010 17:36:09 -0300 Subject: translation method of TranslationHelper module returns always SafeBuffer [#4194 status:resolved] Signed-off-by: Jeremy Kemper --- actionpack/lib/action_view/helpers/translation_helper.rb | 2 +- actionpack/test/fixtures/test/array_translation.erb | 1 + actionpack/test/template/translation_helper_test.rb | 9 ++++++++- 3 files changed, 10 insertions(+), 2 deletions(-) create mode 100644 actionpack/test/fixtures/test/array_translation.erb (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/translation_helper.rb b/actionpack/lib/action_view/helpers/translation_helper.rb index f996762d6f..26ba4e2ca4 100644 --- a/actionpack/lib/action_view/helpers/translation_helper.rb +++ b/actionpack/lib/action_view/helpers/translation_helper.rb @@ -13,7 +13,7 @@ module ActionView def translate(key, options = {}) options[:raise] = true translation = I18n.translate(scope_key_by_partial(key), options) - translation.respond_to?(:html_safe) ? translation.html_safe : translation + (translation.respond_to?(:join) ? translation.join : translation).html_safe rescue I18n::MissingTranslationData => e keys = I18n.normalize_keys(e.locale, e.key, e.options[:scope]) content_tag('span', keys.join(', '), :class => 'translation_missing') diff --git a/actionpack/test/fixtures/test/array_translation.erb b/actionpack/test/fixtures/test/array_translation.erb new file mode 100644 index 0000000000..12c0763313 --- /dev/null +++ b/actionpack/test/fixtures/test/array_translation.erb @@ -0,0 +1 @@ +<%= t(['foo', 'bar']) %> \ No newline at end of file diff --git a/actionpack/test/template/translation_helper_test.rb b/actionpack/test/template/translation_helper_test.rb index 699fb2f5bc..5a32d71409 100644 --- a/actionpack/test/template/translation_helper_test.rb +++ b/actionpack/test/template/translation_helper_test.rb @@ -20,7 +20,14 @@ class TranslationHelperTest < ActiveSupport::TestCase def test_translation_of_an_array I18n.expects(:translate).with(["foo", "bar"], :raise => true).returns(["foo", "bar"]) - assert_equal ["foo", "bar"], translate(["foo", "bar"]) + assert_equal "foobar", translate(["foo", "bar"]) + end + + def test_translation_of_an_array_with_html + expected = 'foobar' + I18n.expects(:translate).with(["foo", "bar"], :raise => true).returns(['foo', 'bar']) + @view = ActionView::Base.new(ActionController::Base.view_paths, {}) + assert_equal expected, @view.render(:file => "test/array_translation") end def test_delegates_localize_to_i18n -- cgit v1.2.3 From 56fb60ebfe9a20ced1366f3e35b2f9bd0bac8e45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Tue, 16 Mar 2010 23:35:45 +0100 Subject: Fix rendering of HTML partials inside JS templates [#4197 status:resolved] --- actionpack/lib/action_view/render/layouts.rb | 17 ++++++++++++++--- actionpack/lib/action_view/template.rb | 2 ++ actionpack/test/controller/new_base/render_rjs_test.rb | 7 +++++++ 3 files changed, 23 insertions(+), 3 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/render/layouts.rb b/actionpack/lib/action_view/render/layouts.rb index 0cb688ca77..b4720aa681 100644 --- a/actionpack/lib/action_view/render/layouts.rb +++ b/actionpack/lib/action_view/render/layouts.rb @@ -43,10 +43,16 @@ module ActionView # This is the method which actually finds the layout using details in the lookup # context object. If no layout is found, it checkes if at least a layout with # the given name exists across all details before raising the error. - def find_layout(layout) #:nodoc: + # + # If self.formats contains several formats, just the first one is considered in + # the layout lookup. + def find_layout(layout) begin - layout =~ /^\// ? - with_fallbacks { find_template(layout) } : find_template(layout) + if formats.size == 1 + _find_layout(layout) + else + update_details(:formats => self.formats[0,1]){ _find_layout(layout) } + end rescue ActionView::MissingTemplate => e update_details(:formats => nil) do raise unless template_exists?(layout) @@ -54,6 +60,11 @@ module ActionView end end + def _find_layout(layout) #:nodoc: + layout =~ /^\// ? + with_fallbacks { find_template(layout) } : find_template(layout) + end + # Contains the logic that actually renders the layout. def _render_layout(layout, locals, &block) #:nodoc: layout.render(self, locals){ |*name| _layout_for(*name, &block) } diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index b4fdb49d3b..15fafac53d 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -31,7 +31,9 @@ module ActionView format = details[:format] format ||= handler.default_format.to_sym if handler.respond_to?(:default_format) format ||= :html + @formats = [format.to_sym] + @formats << :html if @formats.first == :js end def render(view, locals, &block) diff --git a/actionpack/test/controller/new_base/render_rjs_test.rb b/actionpack/test/controller/new_base/render_rjs_test.rb index f4516ade63..7c8f1cb869 100644 --- a/actionpack/test/controller/new_base/render_rjs_test.rb +++ b/actionpack/test/controller/new_base/render_rjs_test.rb @@ -5,8 +5,10 @@ module RenderRjs self.view_paths = [ActionView::FixtureResolver.new( "render_rjs/basic/index.js.rjs" => "page[:customer].replace_html render(:partial => 'customer')", "render_rjs/basic/index_html.js.rjs" => "page[:customer].replace_html :partial => 'customer'", + "render_rjs/basic/index_no_js.js.rjs" => "page[:developer].replace_html render(:partial => 'developer')", "render_rjs/basic/_customer.js.erb" => "JS Partial", "render_rjs/basic/_customer.html.erb" => "HTML Partial", + "render_rjs/basic/_developer.html.erb" => "HTML Partial", "render_rjs/basic/index_locale.js.rjs" => "page[:customer].replace_html :partial => 'customer'", "render_rjs/basic/_customer.da.html.erb" => "Danish HTML Partial", "render_rjs/basic/_customer.da.js.erb" => "Danish JS Partial" @@ -37,6 +39,11 @@ module RenderRjs assert_response("$(\"customer\").update(\"JS Partial\");") end + test "rendering a partial in an RJS template should pick the HTML one if no JS is available" do + get :index_no_js, "format" => "js" + assert_response("$(\"developer\").update(\"HTML Partial\");") + end + test "replacing an element with a partial in an RJS template should pick the HTML template over the JS one" do get :index_html, "format" => "js" assert_response("$(\"customer\").update(\"HTML Partial\");") -- cgit v1.2.3 From 3abf5ad7f8b23d955225ba96e82fd5565dd2571d Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Tue, 16 Mar 2010 12:38:37 -0700 Subject: Make RouteSet#finalize! a NOOP if it's been called already. Call finalize! the first time call() and url_for() are called if the RouteSet has not been finalized yet. --- actionpack/lib/action_dispatch/routing/route_set.rb | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 722be432c7..048764263e 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -219,6 +219,8 @@ module ActionDispatch end def finalize! + return if @finalized + @finalized = true @set.add_route(NotFound) install_helpers @set.freeze @@ -227,6 +229,7 @@ module ActionDispatch def clear! # Clear the controller cache so we may discover new ones @controller_constraints = nil + @finalized = false routes.clear named_routes.clear @set = ::Rack::Mount::RouteSet.new(:parameters_key => PARAMETERS_KEY) @@ -406,6 +409,7 @@ module ActionDispatch RESERVED_OPTIONS = [:anchor, :params, :only_path, :host, :protocol, :port, :trailing_slash] def url_for(options) + finalize! options = default_url_options.merge(options || {}) handle_positional_args(options) @@ -437,6 +441,7 @@ module ActionDispatch end def call(env) + finalize! @set.call(env) end -- cgit v1.2.3 From 23b6def0eb76ac0719e420fce91ba862f880a37b Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Tue, 16 Mar 2010 14:55:42 -0700 Subject: Do not always include the named URL helpers into AC::Base and AV::Base. --- actionpack/lib/action_dispatch/routing/route_set.rb | 1 - .../lib/action_dispatch/testing/assertions/routing.rb | 18 ++++++++++++++++-- actionpack/lib/action_view/base.rb | 4 ++++ actionpack/lib/action_view/helpers/url_helper.rb | 7 +++++++ actionpack/lib/action_view/test_case.rb | 3 ++- 5 files changed, 29 insertions(+), 4 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 048764263e..5537bbbbe8 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -222,7 +222,6 @@ module ActionDispatch return if @finalized @finalized = true @set.add_route(NotFound) - install_helpers @set.freeze end diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb index 1d7e8090e4..eb28cd5107 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb @@ -145,11 +145,25 @@ module ActionDispatch old_routes, @router = @router, ActionDispatch::Routing::RouteSet.new old_controller, @controller = @controller, @controller.clone if @controller _router = @router - @controller.singleton_class.send(:send, :include, @router.url_helpers) if @controller + + # Unfortunately, there is currently an abstraction leak between AC::Base + # and AV::Base which requires having the URL helpers in both AC and AV. + # To do this safely at runtime for tests, we need to bump up the helper serial + # to that the old AV subclass isn't cached. + # + # TODO: Make this unnecessary + if @controller + @controller.singleton_class.send(:include, @router.url_helpers) + @controller.class._helper_serial += 1 + @controller.view_context.singleton_class.send(:include, @router.url_helpers) + end yield @router ensure @router = old_routes - @controller = old_controller if @controller + if @controller + @controller = old_controller + @controller.class._helper_serial += 1 if @controller + end end # ROUTES TODO: These assertions should really work in an integration context diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 3920d8593f..daabe6d196 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -235,6 +235,10 @@ module ActionView #:nodoc: include controller._helpers self.helpers = controller._helpers end + + if controller.respond_to?(:_router) + include controller._router.url_helpers + end end else klass = self diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index 94f1cecade..f877378ebe 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -9,6 +9,9 @@ module ActionView # This allows you to use the same format for links in views # and controllers. module UrlHelper + extend ActiveSupport::Concern + + include ActionDispatch::Routing::UrlFor include JavaScriptHelper # Need to map default url options to controller one. @@ -16,6 +19,10 @@ module ActionView controller.send(:default_url_options, *args) end + def url_options + controller.url_options + end + # Returns the URL for the set of +options+ provided. This takes the # same options as +url_for+ in Action Controller (see the # documentation for ActionController::Base#url_for). Note that by default diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index 1578ac9479..7e609eb640 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -124,7 +124,8 @@ module ActionView def _view view = ActionView::Base.new(ActionController::Base.view_paths, _assigns, @controller) - view.class.send :include, _helpers + view.singleton_class.send :include, _helpers + view.singleton_class.send :include, @controller._router.url_helpers view.output_buffer = self.output_buffer view end -- cgit v1.2.3 From 0c1ac36ccb7d72f3d17d950d030442a7e83d0708 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Tue, 16 Mar 2010 20:12:04 -0300 Subject: scope_key_by_partial fix for Ruby 1.9 when there's virtual_path [#4202 state:resolved] Signed-off-by: Jeremy Kemper --- actionpack/lib/action_view/helpers/translation_helper.rb | 5 +++-- actionpack/test/fixtures/test/scoped_array_translation.erb | 1 + actionpack/test/template/translation_helper_test.rb | 6 ++++++ 3 files changed, 10 insertions(+), 2 deletions(-) create mode 100644 actionpack/test/fixtures/test/scoped_array_translation.erb (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/translation_helper.rb b/actionpack/lib/action_view/helpers/translation_helper.rb index 26ba4e2ca4..457944dbb6 100644 --- a/actionpack/lib/action_view/helpers/translation_helper.rb +++ b/actionpack/lib/action_view/helpers/translation_helper.rb @@ -29,9 +29,10 @@ module ActionView private def scope_key_by_partial(key) - if (key.respond_to?(:join) ? key.join : key.to_s).first == "." + strkey = key.respond_to?(:join) ? key.join : key.to_s + if strkey.first == "." if @_virtual_path - @_virtual_path.gsub(%r{/_?}, ".") + key.to_s + @_virtual_path.gsub(%r{/_?}, ".") + strkey else raise "Cannot use t(#{key.inspect}) shortcut because path is not available" end diff --git a/actionpack/test/fixtures/test/scoped_array_translation.erb b/actionpack/test/fixtures/test/scoped_array_translation.erb new file mode 100644 index 0000000000..0a0c79f717 --- /dev/null +++ b/actionpack/test/fixtures/test/scoped_array_translation.erb @@ -0,0 +1 @@ +<%= t(['.foo', '.bar']) %> \ No newline at end of file diff --git a/actionpack/test/template/translation_helper_test.rb b/actionpack/test/template/translation_helper_test.rb index 5a32d71409..6782bf06d4 100644 --- a/actionpack/test/template/translation_helper_test.rb +++ b/actionpack/test/template/translation_helper_test.rb @@ -41,4 +41,10 @@ class TranslationHelperTest < ActiveSupport::TestCase @view = ActionView::Base.new(ActionController::Base.view_paths, {}) assert_equal "helper", @view.render(:file => "test/translation") end + + def test_scoping_by_partial_of_an_array + I18n.expects(:translate).with("test.scoped_array_translation.foo.bar", :raise => true).returns(["foo", "bar"]) + @view = ActionView::Base.new(ActionController::Base.view_paths, {}) + assert_equal "foobar", @view.render(:file => "test/scoped_array_translation") + end end -- cgit v1.2.3 From 7c49b1adbbe1ffd42c8cd6fc0439d53895c861cf Mon Sep 17 00:00:00 2001 From: wycats Date: Tue, 16 Mar 2010 17:28:44 -0700 Subject: Make sure options[:anchor] is correct in shorthand cases --- actionpack/lib/action_dispatch/routing/mapper.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index f9b27a5a03..668abb5fdf 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -51,7 +51,9 @@ module ActionDispatch options.merge!(:to => to).delete(path) if path when using_match_shorthand?(args, options) path = args.first - options = { :to => path.gsub("/", "#"), :as => path.gsub("/", "_") } + options = { :to => path.gsub("/", "#"), + :as => path.gsub("/", "_") + }.merge(options || {}) else path = args.first end @@ -301,7 +303,6 @@ module ActionDispatch options = args.extract_options! options = (@scope[:options] || {}).merge(options) - options[:anchor] = true unless options.key?(:anchor) if @scope[:name_prefix] && !options[:as].blank? options[:as] = "#{@scope[:name_prefix]}_#{options[:as]}" @@ -563,6 +564,8 @@ module ActionDispatch def match(*args) options = args.extract_options! + options[:anchor] = true unless options.key?(:anchor) + if args.length > 1 args.each { |path| match(path, options) } return self -- cgit v1.2.3 From 55aac2c6969e4f5209ba786120f1d7b57c80b9a0 Mon Sep 17 00:00:00 2001 From: wycats Date: Tue, 16 Mar 2010 17:32:42 -0700 Subject: Fix missing require --- actionpack/lib/action_view/helpers/url_helper.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index f877378ebe..79232e297f 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -1,6 +1,7 @@ require 'action_view/helpers/javascript_helper' require 'active_support/core_ext/array/access' require 'active_support/core_ext/hash/keys' +require 'action_dispatch' module ActionView module Helpers #:nodoc: -- cgit v1.2.3 From d69e5616e821afc40efa5936c5ab6e087eb4e0c6 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 16 Mar 2010 22:06:16 -0500 Subject: link_to_function is here to stay --- .../lib/action_view/helpers/javascript_helper.rb | 87 ++++++++++++++++++++++ .../lib/action_view/helpers/prototype_helper.rb | 33 -------- actionpack/test/template/javascript_helper_test.rb | 31 +++++++- 3 files changed, 117 insertions(+), 34 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/javascript_helper.rb b/actionpack/lib/action_view/helpers/javascript_helper.rb index 8dab3094dd..0aa539031d 100644 --- a/actionpack/lib/action_view/helpers/javascript_helper.rb +++ b/actionpack/lib/action_view/helpers/javascript_helper.rb @@ -89,6 +89,93 @@ module ActionView def javascript_cdata_section(content) #:nodoc: "\n//#{cdata_section("\n#{content}\n//")}\n".html_safe end + + # Returns a button with the given +name+ text that'll trigger a JavaScript +function+ using the + # onclick handler. + # + # The first argument +name+ is used as the button's value or display text. + # + # The next arguments are optional and may include the javascript function definition and a hash of html_options. + # + # The +function+ argument can be omitted in favor of an +update_page+ + # block, which evaluates to a string when the template is rendered + # (instead of making an Ajax request first). + # + # The +html_options+ will accept a hash of html attributes for the link tag. Some examples are :class => "nav_button", :id => "articles_nav_button" + # + # Note: if you choose to specify the javascript function in a block, but would like to pass html_options, set the +function+ parameter to nil + # + # Examples: + # button_to_function "Greeting", "alert('Hello world!')" + # button_to_function "Delete", "if (confirm('Really?')) do_delete()" + # button_to_function "Details" do |page| + # page[:details].visual_effect :toggle_slide + # end + # button_to_function "Details", :class => "details_button" do |page| + # page[:details].visual_effect :toggle_slide + # end + def button_to_function(name, *args, &block) + html_options = args.extract_options!.symbolize_keys + + function = block_given? ? update_page(&block) : args[0] || '' + onclick = "#{"#{html_options[:onclick]}; " if html_options[:onclick]}#{function};" + + tag(:input, html_options.merge(:type => 'button', :value => name, :onclick => onclick)) + end + + # Returns a link of the given +name+ that will trigger a JavaScript +function+ using the + # onclick handler and return false after the fact. + # + # The first argument +name+ is used as the link text. + # + # The next arguments are optional and may include the javascript function definition and a hash of html_options. + # + # The +function+ argument can be omitted in favor of an +update_page+ + # block, which evaluates to a string when the template is rendered + # (instead of making an Ajax request first). + # + # The +html_options+ will accept a hash of html attributes for the link tag. Some examples are :class => "nav_button", :id => "articles_nav_button" + # + # Note: if you choose to specify the javascript function in a block, but would like to pass html_options, set the +function+ parameter to nil + # + # + # Examples: + # link_to_function "Greeting", "alert('Hello world!')" + # Produces: + # Greeting + # + # link_to_function(image_tag("delete"), "if (confirm('Really?')) do_delete()") + # Produces: + # + # Delete + # + # + # link_to_function("Show me more", nil, :id => "more_link") do |page| + # page[:details].visual_effect :toggle_blind + # page[:more_link].replace_html "Show me less" + # end + # Produces: + # Show me more + # + def link_to_function(name, *args, &block) + html_options = args.extract_options!.symbolize_keys + + function = block_given? ? update_page(&block) : args[0] || '' + onclick = "#{"#{html_options[:onclick]}; " if html_options[:onclick]}#{function}; return false;" + href = html_options[:href] || '#' + + content_tag(:a, name, html_options.merge(:href => href, :onclick => onclick)) + end end end end diff --git a/actionpack/lib/action_view/helpers/prototype_helper.rb b/actionpack/lib/action_view/helpers/prototype_helper.rb index e46ca53275..ad3bc8c79c 100644 --- a/actionpack/lib/action_view/helpers/prototype_helper.rb +++ b/actionpack/lib/action_view/helpers/prototype_helper.rb @@ -102,39 +102,6 @@ module ActionView :form, :with, :update, :script, :type ]).merge(CALLBACKS) end - # Returns a button with the given +name+ text that'll trigger a JavaScript +function+ using the - # onclick handler. - # - # The first argument +name+ is used as the button's value or display text. - # - # The next arguments are optional and may include the javascript function definition and a hash of html_options. - # - # The +function+ argument can be omitted in favor of an +update_page+ - # block, which evaluates to a string when the template is rendered - # (instead of making an Ajax request first). - # - # The +html_options+ will accept a hash of html attributes for the link tag. Some examples are :class => "nav_button", :id => "articles_nav_button" - # - # Note: if you choose to specify the javascript function in a block, but would like to pass html_options, set the +function+ parameter to nil - # - # Examples: - # button_to_function "Greeting", "alert('Hello world!')" - # button_to_function "Delete", "if (confirm('Really?')) do_delete()" - # button_to_function "Details" do |page| - # page[:details].visual_effect :toggle_slide - # end - # button_to_function "Details", :class => "details_button" do |page| - # page[:details].visual_effect :toggle_slide - # end - def button_to_function(name, *args, &block) - html_options = args.extract_options!.symbolize_keys - - function = block_given? ? update_page(&block) : args[0] || '' - onclick = "#{"#{html_options[:onclick]}; " if html_options[:onclick]}#{function};" - - tag(:input, html_options.merge(:type => 'button', :value => name, :onclick => onclick)) - end - # Returns the JavaScript needed for a remote function. # Takes the same arguments as link_to_remote. # diff --git a/actionpack/test/template/javascript_helper_test.rb b/actionpack/test/template/javascript_helper_test.rb index f49b763881..c5c2a6b952 100644 --- a/actionpack/test/template/javascript_helper_test.rb +++ b/actionpack/test/template/javascript_helper_test.rb @@ -17,7 +17,7 @@ class JavaScriptHelperTest < ActionView::TestCase ActiveSupport.escape_html_entities_in_json = true @template = self end - + def teardown ActiveSupport.escape_html_entities_in_json = false end @@ -60,6 +60,35 @@ class JavaScriptHelperTest < ActionView::TestCase button_to_function("Greeting") end + def test_link_to_function + assert_dom_equal %(Greeting), + link_to_function("Greeting", "alert('Hello world!')") + end + + def test_link_to_function_with_existing_onclick + assert_dom_equal %(Greeting), + link_to_function("Greeting", "alert('Hello world!')", :onclick => "confirm('Sanity!')") + end + + def test_link_to_function_with_rjs_block + html = link_to_function( "Greet me!" ) do |page| + page.replace_html 'header', "

    Greetings

    " + end + assert_dom_equal %(Greet me!), html + end + + def test_link_to_function_with_rjs_block_and_options + html = link_to_function( "Greet me!", :class => "updater" ) do |page| + page.replace_html 'header', "

    Greetings

    " + end + assert_dom_equal %(Greet me!), html + end + + def test_link_to_function_with_href + assert_dom_equal %(Greeting), + link_to_function("Greeting", "alert('Hello world!')", :href => 'http://example.com/') + end + def test_javascript_tag self.output_buffer = 'foo' -- cgit v1.2.3 From c7388124153e1b1f85965998e5d1c20eed670da8 Mon Sep 17 00:00:00 2001 From: wycats Date: Tue, 16 Mar 2010 21:37:43 -0700 Subject: Another missing require --- actionpack/lib/action_dispatch/routing.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing.rb b/actionpack/lib/action_dispatch/routing.rb index 5bc3205c51..c6e942555f 100644 --- a/actionpack/lib/action_dispatch/routing.rb +++ b/actionpack/lib/action_dispatch/routing.rb @@ -1,5 +1,6 @@ require 'active_support/core_ext/object/to_param' require 'active_support/core_ext/regexp' +require 'action_controller/polymorphic_routes' module ActionDispatch # == Routing -- cgit v1.2.3 From cd9ffd11e13ef6e62eba2cbd5c3760ff04132820 Mon Sep 17 00:00:00 2001 From: wycats Date: Tue, 16 Mar 2010 23:24:00 -0700 Subject: Eliminate warnings for AM on 1.8 --- actionpack/lib/abstract_controller/layouts.rb | 4 ++++ actionpack/lib/action_controller/metal.rb | 6 +++++- actionpack/lib/action_controller/metal/rack_delegation.rb | 6 ++---- actionpack/lib/action_dispatch/http/filter_parameters.rb | 1 - actionpack/lib/action_view/helpers/capture_helper.rb | 2 +- 5 files changed, 12 insertions(+), 7 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/abstract_controller/layouts.rb b/actionpack/lib/abstract_controller/layouts.rb index 95a6101109..528a430b04 100644 --- a/actionpack/lib/abstract_controller/layouts.rb +++ b/actionpack/lib/abstract_controller/layouts.rb @@ -1,3 +1,5 @@ +require "active_support/core_ext/module/remove_method" + module AbstractController # 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: @@ -237,6 +239,8 @@ module AbstractController # name, return that string. Otherwise, use the superclass' # layout (which might also be implied) def _write_layout_method + remove_possible_method(:_layout) + case defined?(@_layout) ? @_layout : nil when String self.class_eval %{def _layout; #{@_layout.inspect} end} diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index eebd2c943a..a148c19a31 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -34,7 +34,7 @@ module ActionController # and response object available. You might wish to control the # environment and response manually for performance reasons. - attr_internal :status, :headers, :content_type, :response, :request + attr_internal :headers, :response, :request delegate :session, :to => "@_request" def initialize(*) @@ -62,6 +62,10 @@ module ActionController headers["Location"] = url end + def status + @_status + end + def status=(status) @_status = Rack::Utils.status_code(status) end diff --git a/actionpack/lib/action_controller/metal/rack_delegation.rb b/actionpack/lib/action_controller/metal/rack_delegation.rb index 37106733cb..060117756e 100644 --- a/actionpack/lib/action_controller/metal/rack_delegation.rb +++ b/actionpack/lib/action_controller/metal/rack_delegation.rb @@ -5,10 +5,8 @@ module ActionController module RackDelegation extend ActiveSupport::Concern - included do - delegate :headers, :status=, :location=, :content_type=, - :status, :location, :content_type, :to => "@_response" - end + delegate :headers, :status=, :location=, :content_type=, + :status, :location, :content_type, :to => "@_response" def dispatch(action, request) @_response = ActionDispatch::Response.new diff --git a/actionpack/lib/action_dispatch/http/filter_parameters.rb b/actionpack/lib/action_dispatch/http/filter_parameters.rb index 451b79b190..e42b4d09b0 100644 --- a/actionpack/lib/action_dispatch/http/filter_parameters.rb +++ b/actionpack/lib/action_dispatch/http/filter_parameters.rb @@ -25,7 +25,6 @@ module ActionDispatch module FilterParameters extend ActiveSupport::Concern - mattr_reader :compiled_parameter_filter_for @@compiled_parameter_filter_for = {} # Return a hash of parameters with all sensitive data replaced. diff --git a/actionpack/lib/action_view/helpers/capture_helper.rb b/actionpack/lib/action_view/helpers/capture_helper.rb index 42a67756e4..f0be814700 100644 --- a/actionpack/lib/action_view/helpers/capture_helper.rb +++ b/actionpack/lib/action_view/helpers/capture_helper.rb @@ -32,7 +32,7 @@ module ActionView # def capture(*args) value = nil - buffer = with_output_buffer { value = yield *args } + buffer = with_output_buffer { value = yield(*args) } if string = buffer.presence || value and string.is_a?(String) NonConcattingString.new(string) end -- cgit v1.2.3 From a5587efc1903fd27d4b179753aa6e139445ad18c Mon Sep 17 00:00:00 2001 From: wycats Date: Wed, 17 Mar 2010 00:15:55 -0700 Subject: Remove some 1.9 warnings (resulting in some fixed bugs). Remaining AM warnings are in dependencies. --- actionpack/lib/action_controller/metal/redirecting.rb | 2 +- actionpack/lib/action_view/base.rb | 1 + actionpack/lib/action_view/helpers/form_helper.rb | 6 +++--- actionpack/lib/action_view/helpers/javascript_helper.rb | 1 - actionpack/lib/action_view/helpers/prototype_helper.rb | 2 -- actionpack/lib/action_view/helpers/text_helper.rb | 2 +- actionpack/lib/action_view/template.rb | 1 - 7 files changed, 6 insertions(+), 9 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index 25e4e18493..b5f1d23ef0 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -76,7 +76,7 @@ module ActionController # The scheme name consist of a letter followed by any combination of # letters, digits, and the plus ("+"), period ("."), or hyphen ("-") # characters; and is terminated by a colon (":"). - when %r{^\w[\w\d+.-]*:.*} + when %r{^\w[\w+.-]*:.*} options when String request.protocol + request.host_with_port + options diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index daabe6d196..326b79f9bf 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -187,6 +187,7 @@ module ActionView #:nodoc: @@debug_rjs = false class_attribute :helpers + remove_method :helpers attr_reader :helpers class << self diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index 48df26efaa..01a585af95 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -1014,7 +1014,7 @@ module ActionView class FormBuilder #:nodoc: # The methods which wrap a form helper call. class_inheritable_accessor :field_helpers - self.field_helpers = (FormHelper.instance_methods - ['form_for']) + self.field_helpers = (FormHelper.instance_method_names - ['form_for']) attr_accessor :object_name, :object, :options @@ -1040,7 +1040,7 @@ module ActionView end (field_helpers - %w(label check_box radio_button fields_for hidden_field)).each do |selector| - src = <<-end_src + src, file, line = <<-end_src, __FILE__, __LINE__ + 1 def #{selector}(method, options = {}) # def text_field(method, options = {}) @template.send( # @template.send( #{selector.inspect}, # "text_field", @@ -1049,7 +1049,7 @@ module ActionView objectify_options(options)) # objectify_options(options)) end # end end_src - class_eval src, __FILE__, __LINE__ + class_eval src, file, line end def fields_for(record_or_name_or_array, *args, &block) diff --git a/actionpack/lib/action_view/helpers/javascript_helper.rb b/actionpack/lib/action_view/helpers/javascript_helper.rb index 0aa539031d..5635f88c11 100644 --- a/actionpack/lib/action_view/helpers/javascript_helper.rb +++ b/actionpack/lib/action_view/helpers/javascript_helper.rb @@ -1,5 +1,4 @@ require 'action_view/helpers/tag_helper' -require 'action_view/helpers/prototype_helper' module ActionView module Helpers diff --git a/actionpack/lib/action_view/helpers/prototype_helper.rb b/actionpack/lib/action_view/helpers/prototype_helper.rb index ad3bc8c79c..2e5fe5744e 100644 --- a/actionpack/lib/action_view/helpers/prototype_helper.rb +++ b/actionpack/lib/action_view/helpers/prototype_helper.rb @@ -853,5 +853,3 @@ module ActionView end end end - -require 'action_view/helpers/javascript_helper' diff --git a/actionpack/lib/action_view/helpers/text_helper.rb b/actionpack/lib/action_view/helpers/text_helper.rb index b5bf813e07..13fad0ed7a 100644 --- a/actionpack/lib/action_view/helpers/text_helper.rb +++ b/actionpack/lib/action_view/helpers/text_helper.rb @@ -576,7 +576,7 @@ module ActionView # each email is yielded and the result is used as the link text. def auto_link_email_addresses(text, html_options = {}) body = text.dup - text.gsub(/([\w\.!#\$%\-+.]+@[A-Za-z0-9\-]+(\.[A-Za-z0-9\-]+)+)/) do + text.gsub(/([\w\.!#\$%\-+]+@[A-Za-z0-9\-]+(\.[A-Za-z0-9\-]+)+)/) do text = $1 if body.match(/]*>(.*)(#{Regexp.escape(text)})(.*)<\/a>/) diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 15fafac53d..6315e502dd 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -2,7 +2,6 @@ # This is so that templates compiled in this file are UTF-8 require 'set' -require "action_view/template/resolver" module ActionView class Template -- cgit v1.2.3 From 13a783672aad338b9c7ade5319ec7967768905d7 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Wed, 17 Mar 2010 16:04:41 -0500 Subject: Install url helpers on module instance so they can be accessed globally --- actionpack/lib/action_dispatch/routing/route_set.rb | 18 +++++++++++++----- actionpack/test/dispatch/routing_test.rb | 1 + 2 files changed, 14 insertions(+), 5 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 5537bbbbe8..8936d7659a 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -65,7 +65,7 @@ module ActionDispatch # named routes. class NamedRouteCollection #:nodoc: include Enumerable - attr_reader :routes, :helpers + attr_reader :routes, :helpers, :module def initialize clear! @@ -241,21 +241,29 @@ module ActionDispatch def url_helpers @url_helpers ||= begin - router = self + routes = self - Module.new do + helpers = Module.new do extend ActiveSupport::Concern include UrlFor + @routes = routes + class << self + delegate :url_for, :to => '@routes' + end + extend routes.named_routes.module + # ROUTES TODO: install_helpers isn't great... can we make a module with the stuff that # we can include? # Yes plz - JP included do - router.install_helpers(self) + routes.install_helpers(self) end - define_method(:_router) { router } + define_method(:_router) { routes } end + + helpers end end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index f5fcf9b0df..e4d83fa0a4 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -193,6 +193,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_equal '/login', url_for(:controller => 'sessions', :action => 'new', :only_path => true) assert_equal 'http://rubyonrails.org/login', Routes.url_for(:controller => 'sessions', :action => 'create') + assert_equal 'http://rubyonrails.org/login', Routes.url_helpers.login_url end end -- cgit v1.2.3 From 947f86c699b33bd44703b3554db58e4cfca37c86 Mon Sep 17 00:00:00 2001 From: Carlhuda Date: Wed, 17 Mar 2010 14:19:42 -0700 Subject: Modify assert_template to use instrumentation --- actionpack/lib/action_controller/test_case.rb | 82 ++++++++++++++++++++++ .../action_dispatch/testing/assertions/response.rb | 52 -------------- actionpack/lib/action_view/template.rb | 8 ++- actionpack/lib/action_view/test_case.rb | 1 + actionpack/test/abstract_unit.rb | 41 ----------- 5 files changed, 89 insertions(+), 95 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index cdb5db32aa..107f70d3a3 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -2,6 +2,87 @@ require 'rack/session/abstract/id' require 'action_view/test_case' module ActionController + module TemplateAssertions + extend ActiveSupport::Concern + + included do + setup :setup_subscriptions + teardown :teardown_subscriptions + end + + def setup_subscriptions + @partials = Hash.new(0) + @templates = Hash.new(0) + ActiveSupport::Notifications.subscribe("action_view.slow_render_template") do |name, start, finish, id, payload| + path = payload[:virtual_path] + next unless path + partial = path =~ /^.*\/_[^\/]*$/ + if partial + @partials[path] += 1 + @partials[path.split("/").last] += 1 + @templates[path] += 1 + else + @templates[path] += 1 + end + end + end + + def teardown_subscriptions + ActiveSupport::Notifications.unsubscribe("action_view.slow_render_template") + end + + # Asserts that the request was rendered with the appropriate template file or partials + # + # ==== Examples + # + # # assert that the "new" view template was rendered + # assert_template "new" + # + # # assert that the "_customer" partial was rendered twice + # assert_template :partial => '_customer', :count => 2 + # + # # assert that no partials were rendered + # assert_template :partial => false + # + def assert_template(options = {}, message = nil) + validate_request! + + case options + when NilClass, String + rendered = @templates + msg = build_message(message, + "expecting but rendering with ", + options, rendered.keys.join(', ')) + assert_block(msg) do + if options.nil? + @templates.blank? + else + rendered.any? { |t,num| t.match(options) } + end + end + when Hash + if expected_partial = options[:partial] + if expected_count = options[:count] + actual_count = @partials[expected_partial] + # actual_count = found.nil? ? 0 : found[1] + msg = build_message(message, + "expecting ? to be rendered ? time(s) but rendered ? time(s)", + expected_partial, expected_count, actual_count) + assert(actual_count == expected_count.to_i, msg) + else + msg = build_message(message, + "expecting partial but action rendered ", + options[:partial], @partials.keys) + assert(@partials.include?(expected_partial), msg) + end + else + assert @partials.empty?, + "Expected no partials to be rendered" + end + end + end + end + class TestRequest < ActionDispatch::TestRequest #:nodoc: def initialize(env = {}) super @@ -181,6 +262,7 @@ module ActionController # assert_redirected_to page_url(:title => 'foo') class TestCase < ActiveSupport::TestCase include ActionDispatch::TestProcess + include ActionController::TemplateAssertions # Executes a request simulating GET HTTP method and set/volley the response def get(action, parameters = nil, session = nil, flash = nil) diff --git a/actionpack/lib/action_dispatch/testing/assertions/response.rb b/actionpack/lib/action_dispatch/testing/assertions/response.rb index 937c9f48d2..ec5e9efe44 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/response.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/response.rb @@ -73,58 +73,6 @@ module ActionDispatch end end - # Asserts that the request was rendered with the appropriate template file or partials - # - # ==== Examples - # - # # assert that the "new" view template was rendered - # assert_template "new" - # - # # assert that the "_customer" partial was rendered twice - # assert_template :partial => '_customer', :count => 2 - # - # # assert that no partials were rendered - # assert_template :partial => false - # - def assert_template(options = {}, message = nil) - validate_request! - - case options - when NilClass, String - rendered = (@controller.template.rendered[:template] || []).map { |t| t.identifier } - msg = build_message(message, - "expecting but rendering with ", - options, rendered.join(', ')) - assert_block(msg) do - if options.nil? - @controller.template.rendered[:template].blank? - else - rendered.any? { |t| t.match(options) } - end - end - when Hash - if expected_partial = options[:partial] - partials = @controller.template.rendered[:partials] - if expected_count = options[:count] - found = partials.detect { |p, _| p.identifier.match(expected_partial) } - actual_count = found.nil? ? 0 : found.second - msg = build_message(message, - "expecting ? to be rendered ? time(s) but rendered ? time(s)", - expected_partial, expected_count, actual_count) - assert(actual_count == expected_count.to_i, msg) - else - msg = build_message(message, - "expecting partial but action rendered ", - options[:partial], partials.keys) - assert(partials.keys.any? { |p| p.identifier.match(expected_partial) }, msg) - end - else - assert @controller.template.rendered[:partials].empty?, - "Expected no partials to be rendered" - end - end - end - private # Proxy to to_param if the object will respond to it. def parameterize(value) diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 6315e502dd..ad37eb4c4a 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -36,8 +36,12 @@ module ActionView end def render(view, locals, &block) - method_name = compile(locals, view) - view.send(method_name, locals, &block) + # TODO: Revisit this name + # This is only slow if it's being listened to. Do not instrument this in production. + ActiveSupport::Notifications.instrument("action_view.slow_render_template", :virtual_path => @virtual_path) do + method_name = compile(locals, view) + view.send(method_name, locals, &block) + end rescue Exception => e if e.is_a?(Template::Error) e.sub_template_of(self) diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index 7e609eb640..8750a501b7 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -42,6 +42,7 @@ module ActionView end include ActionDispatch::Assertions, ActionDispatch::TestProcess + include ActionController::TemplateAssertions include ActionView::Context include ActionController::PolymorphicRoutes diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 67aa412d3d..a3486aa039 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -239,47 +239,6 @@ module ActionController setup do @router = SharedTestRoutes end - - def assert_template(options = {}, message = nil) - validate_request! - - hax = @controller.view_context.instance_variable_get(:@_rendered) - - case options - when NilClass, String - rendered = (hax[:template] || []).map { |t| t.identifier } - msg = build_message(message, - "expecting but rendering with ", - options, rendered.join(', ')) - assert_block(msg) do - if options.nil? - hax[:template].blank? - else - rendered.any? { |t| t.match(options) } - end - end - when Hash - if expected_partial = options[:partial] - partials = hax[:partials] - if expected_count = options[:count] - found = partials.detect { |p, _| p.identifier.match(expected_partial) } - actual_count = found.nil? ? 0 : found[1] - msg = build_message(message, - "expecting ? to be rendered ? time(s) but rendered ? time(s)", - expected_partial, expected_count, actual_count) - assert(actual_count == expected_count.to_i, msg) - else - msg = build_message(message, - "expecting partial but action rendered ", - options[:partial], partials.keys) - assert(partials.keys.any? { |p| p.identifier.match(expected_partial) }, msg) - end - else - assert hax[:partials].empty?, - "Expected no partials to be rendered" - end - end - end end end -- cgit v1.2.3 From 6416a35f4b3290a93145d40045147fc01d36e756 Mon Sep 17 00:00:00 2001 From: Carlhuda Date: Wed, 17 Mar 2010 14:23:00 -0700 Subject: Remove unneeded AV::Base and AV::Template monkey-patches --- actionpack/lib/abstract_controller/rendering.rb | 1 + actionpack/lib/action_view/test_case.rb | 21 --------------------- .../test/controller/action_pack_assertions_test.rb | 5 ++--- 3 files changed, 3 insertions(+), 24 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb index 16664098e5..94c9ec7478 100644 --- a/actionpack/lib/abstract_controller/rendering.rb +++ b/actionpack/lib/abstract_controller/rendering.rb @@ -1,4 +1,5 @@ require "abstract_controller/base" +require "action_view/base" module AbstractController class DoubleRenderError < Error diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index 8750a501b7..1c69c23bfc 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -2,27 +2,6 @@ require 'action_controller/test_case' require 'action_view' module ActionView - class Base - alias_method :initialize_without_template_tracking, :initialize - def initialize(*args) - @_rendered = { :template => nil, :partials => Hash.new(0) } - initialize_without_template_tracking(*args) - end - - attr_internal :rendered - end - - class Template - alias_method :render_without_tracking, :render - def render(view, locals, &blk) - rendered = view.rendered - rendered[:partials][self] += 1 if partial? - rendered[:template] ||= [] - rendered[:template] << self - render_without_tracking(view, locals, &blk) - end - end - class TestCase < ActiveSupport::TestCase class TestController < ActionController::Base attr_accessor :request, :response, :params diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index 26e0d6d844..b6810b0c27 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -365,11 +365,10 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase # check if we were rendered by a file-based template? def test_rendered_action process :nothing - assert_nil @controller.template.rendered[:template] + assert_template nil process :hello_world - assert @controller.template.rendered[:template] - assert 'hello_world', @controller.template.rendered[:template].to_s + assert_template 'hello_world' end # check the redirection location -- cgit v1.2.3 From 21dcbb17de86b92df1a67e233fdc457be4fdf2d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 17 Mar 2010 23:09:28 +0100 Subject: Ensure json is loaded before using responders. --- actionpack/lib/abstract_controller/rendering.rb | 1 - actionpack/lib/action_controller/metal/responder.rb | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb index 94c9ec7478..16664098e5 100644 --- a/actionpack/lib/abstract_controller/rendering.rb +++ b/actionpack/lib/abstract_controller/rendering.rb @@ -1,5 +1,4 @@ require "abstract_controller/base" -require "action_view/base" module AbstractController class DoubleRenderError < Error diff --git a/actionpack/lib/action_controller/metal/responder.rb b/actionpack/lib/action_controller/metal/responder.rb index 6178a59029..0b2cee6868 100644 --- a/actionpack/lib/action_controller/metal/responder.rb +++ b/actionpack/lib/action_controller/metal/responder.rb @@ -1,3 +1,5 @@ +require 'active_support/json' + module ActionController #:nodoc: # Responder is responsible for exposing a resource to different mime requests, # usually depending on the HTTP verb. The responder is triggered when -- cgit v1.2.3 From a6dc227167a8a720bd18495268305b15aa08d8f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 17 Mar 2010 23:44:03 +0100 Subject: Mark bang instrumentations as something that you shuold not be listening to. --- actionpack/lib/action_controller/test_case.rb | 4 ++-- actionpack/lib/action_view/template.rb | 11 +++-------- 2 files changed, 5 insertions(+), 10 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 107f70d3a3..330b950d7c 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -13,7 +13,7 @@ module ActionController def setup_subscriptions @partials = Hash.new(0) @templates = Hash.new(0) - ActiveSupport::Notifications.subscribe("action_view.slow_render_template") do |name, start, finish, id, payload| + ActiveSupport::Notifications.subscribe("action_view.render_template!") do |name, start, finish, id, payload| path = payload[:virtual_path] next unless path partial = path =~ /^.*\/_[^\/]*$/ @@ -28,7 +28,7 @@ module ActionController end def teardown_subscriptions - ActiveSupport::Notifications.unsubscribe("action_view.slow_render_template") + ActiveSupport::Notifications.unsubscribe("action_view.render_template!") end # Asserts that the request was rendered with the appropriate template file or partials diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index ad37eb4c4a..9145d007a8 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -23,7 +23,6 @@ module ActionView @identifier = identifier @handler = handler - @partial = details[:partial] @virtual_path = details[:virtual_path] @method_names = {} @@ -36,9 +35,9 @@ module ActionView end def render(view, locals, &block) - # TODO: Revisit this name - # This is only slow if it's being listened to. Do not instrument this in production. - ActiveSupport::Notifications.instrument("action_view.slow_render_template", :virtual_path => @virtual_path) do + # Notice that we use a bang in this instrumentation because you don't want to + # consume this in production. This is only slow if it's being listened to. + ActiveSupport::Notifications.instrument("action_view.render_template!", :virtual_path => @virtual_path) do method_name = compile(locals, view) view.send(method_name, locals, &block) end @@ -63,10 +62,6 @@ module ActionView @counter_name ||= "#{variable_name}_counter".to_sym end - def partial? - @partial - end - def inspect if defined?(Rails.root) identifier.sub("#{Rails.root}/", '') -- cgit v1.2.3 From d9375f3f302a5d1856ad57946c7263d4e6a45a2a Mon Sep 17 00:00:00 2001 From: Carlhuda Date: Wed, 17 Mar 2010 16:28:05 -0700 Subject: Modify assert_template to use notifications. Also, remove ActionController::Base#template since it is no longer needed. --- .../lib/action_controller/metal/compatibility.rb | 9 ----- actionpack/lib/action_controller/test_case.rb | 20 +++++++++++ actionpack/lib/action_view/base.rb | 9 +++-- .../lib/action_view/helpers/prototype_helper.rb | 2 +- actionpack/lib/action_view/render/rendering.rb | 3 +- actionpack/lib/action_view/test_case.rb | 4 ++- .../test/controller/action_pack_assertions_test.rb | 8 +++-- actionpack/test/controller/filters_test.rb | 4 +-- actionpack/test/controller/layout_test.rb | 16 ++++----- actionpack/test/controller/render_test.rb | 6 ++-- actionpack/test/template/body_parts_test.rb | 3 +- actionpack/test/template/erb/form_for_test.rb | 11 ++++++ actionpack/test/template/erb/helper.rb | 30 ++++++++++++++++ actionpack/test/template/erb/tag_helper_test.rb | 40 ++-------------------- actionpack/test/template/output_buffer_test.rb | 18 +++++----- 15 files changed, 104 insertions(+), 79 deletions(-) create mode 100644 actionpack/test/template/erb/form_for_test.rb create mode 100644 actionpack/test/template/erb/helper.rb (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/compatibility.rb b/actionpack/lib/action_controller/metal/compatibility.rb index ab8d87b2c4..e6cea483bb 100644 --- a/actionpack/lib/action_controller/metal/compatibility.rb +++ b/actionpack/lib/action_controller/metal/compatibility.rb @@ -40,15 +40,6 @@ module ActionController def initialize_template_class(*) end def assign_shortcuts(*) end - def template - @template ||= view_context - end - - def process_action(*) - template - super - end - def _normalize_options(options) if options[:action] && options[:action].to_s.include?(?/) ActiveSupport::Deprecation.warn "Giving a path to render :action is deprecated. " << diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 330b950d7c..8ab0c32a9e 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -13,6 +13,13 @@ module ActionController def setup_subscriptions @partials = Hash.new(0) @templates = Hash.new(0) + @layouts = Hash.new(0) + + ActiveSupport::Notifications.subscribe("action_view.render_template") do |name, start, finish, id, payload| + path = payload[:layout] + @layouts[path] += 1 + end + ActiveSupport::Notifications.subscribe("action_view.render_template!") do |name, start, finish, id, payload| path = payload[:virtual_path] next unless path @@ -69,6 +76,19 @@ module ActionController "expecting ? to be rendered ? time(s) but rendered ? time(s)", expected_partial, expected_count, actual_count) assert(actual_count == expected_count.to_i, msg) + elsif options.key?(:layout) + msg = build_message(message, + "expecting layout but action rendered ", + expected_layout, @layouts.keys) + + case layout = options[:layout] + when String + assert(@layouts.include?(expected_layout), msg) + when Regexp + assert(@layouts.any? {|l| l =~ layout }, msg) + when nil + assert(@layouts.empty?, msg) + end else msg = build_message(message, "expecting partial but action rendered ", diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 326b79f9bf..4d9f53cf95 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -196,7 +196,7 @@ module ActionView #:nodoc: end attr_accessor :base_path, :assigns, :template_extension, :lookup_context - attr_internal :captures, :request, :layout, :controller, :template, :config + attr_internal :captures, :request, :controller, :template, :config delegate :find_template, :template_exists?, :formats, :formats=, :locale, :locale=, :view_paths, :view_paths=, :with_fallbacks, :update_details, :to => :lookup_context @@ -206,6 +206,11 @@ module ActionView #:nodoc: delegate :logger, :to => :controller, :allow_nil => true + # TODO: HACK FOR RJS + def view_context + self + end + def self.xss_safe? #:nodoc: true end @@ -254,7 +259,7 @@ module ActionView #:nodoc: @helpers = self.class.helpers || Module.new @_controller = controller - @_config = ActiveSupport::InheritableOptions.new(controller.config) if controller + @_config = ActiveSupport::InheritableOptions.new(controller.config) if controller && controller.respond_to?(:config) @_content_for = Hash.new { |h,k| h[k] = ActiveSupport::SafeBuffer.new } @_virtual_path = nil diff --git a/actionpack/lib/action_view/helpers/prototype_helper.rb b/actionpack/lib/action_view/helpers/prototype_helper.rb index 2e5fe5744e..aba2565c67 100644 --- a/actionpack/lib/action_view/helpers/prototype_helper.rb +++ b/actionpack/lib/action_view/helpers/prototype_helper.rb @@ -582,7 +582,7 @@ module ActionView # page.hide 'spinner' # end def update_page(&block) - JavaScriptGenerator.new(@template, &block).to_s.html_safe + JavaScriptGenerator.new(view_context, &block).to_s.html_safe end # Works like update_page but wraps the generated JavaScript in a ").html_safe? + assert number_to_human("asdf".html_safe).html_safe? + + assert number_to_human_size(1).html_safe? + assert number_to_human_size(1000000).html_safe? + assert !number_to_human_size("").html_safe? + assert number_to_human_size("asdf".html_safe).html_safe? + + assert number_with_precision(1, :strip_unsignificant_zeros => false).html_safe? + assert number_with_precision(1, :strip_unsignificant_zeros => true).html_safe? + assert !number_with_precision("").html_safe? + assert number_with_precision("asdf".html_safe).html_safe? + + assert number_to_currency(1).html_safe? + assert !number_to_currency("").html_safe? + assert number_to_currency("asdf".html_safe).html_safe? + + assert number_to_percentage(1).html_safe? + assert !number_to_percentage("").html_safe? + assert number_to_percentage("asdf".html_safe).html_safe? + + assert number_to_phone(1).html_safe? + assert !number_to_phone("").html_safe? + assert number_to_phone("asdf".html_safe).html_safe? + + assert number_with_delimiter(1).html_safe? + assert !number_with_delimiter("").html_safe? + assert number_with_delimiter("asdf".html_safe).html_safe? + end + + def test_number_helpers_should_raise_error_if_invalid_when_specified + assert_raise InvalidNumberError do + number_to_human("x", :raise => true) + end + begin + number_to_human("x", :raise => true) + rescue InvalidNumberError => e + assert_equal "x", e.number + end + + assert_raise InvalidNumberError do + number_to_human_size("x", :raise => true) + end + begin + number_to_human_size("x", :raise => true) + rescue InvalidNumberError => e + assert_equal "x", e.number + end + + assert_raise InvalidNumberError do + number_with_precision("x", :raise => true) + end + begin + number_with_precision("x", :raise => true) + rescue InvalidNumberError => e + assert_equal "x", e.number + end + + assert_raise InvalidNumberError do + number_to_currency("x", :raise => true) + end + begin + number_with_precision("x", :raise => true) + rescue InvalidNumberError => e + assert_equal "x", e.number + end + + assert_raise InvalidNumberError do + number_to_percentage("x", :raise => true) + end + begin + number_to_percentage("x", :raise => true) + rescue InvalidNumberError => e + assert_equal "x", e.number + end + + assert_raise InvalidNumberError do + number_with_delimiter("x", :raise => true) + end + begin + number_with_delimiter("x", :raise => true) + rescue InvalidNumberError => e + assert_equal "x", e.number + end + + assert_raise InvalidNumberError do + number_to_phone("x", :raise => true) + end + begin + number_to_phone("x", :raise => true) + rescue InvalidNumberError => e + assert_equal "x", e.number + end + + end + end -- cgit v1.2.3 From 13bb4a6e68f3aa5b6e8d0a649f8a105819014974 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 22 Mar 2010 23:57:06 +0100 Subject: Current url helpers become actions in controller. Added a failing test case for it. --- actionpack/test/controller/base_test.rb | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/controller/base_test.rb b/actionpack/test/controller/base_test.rb index f047e7da30..8b935097da 100644 --- a/actionpack/test/controller/base_test.rb +++ b/actionpack/test/controller/base_test.rb @@ -124,11 +124,11 @@ class ControllerInstanceTests < Test::Unit::TestCase def test_action_methods @empty_controllers.each do |c| - assert_equal Set.new, c.class.__send__(:action_methods), "#{c.controller_path} should be empty!" + assert_equal Set.new, c.class.action_methods, "#{c.controller_path} should be empty!" end @non_empty_controllers.each do |c| - assert_equal Set.new(%w(public_action)), c.class.__send__(:action_methods), "#{c.controller_path} should not be empty!" + assert_equal Set.new(%w(public_action)), c.class.action_methods, "#{c.controller_path} should not be empty!" end end @@ -191,7 +191,7 @@ class UrlOptionsTest < ActionController::TestCase def test_url_options_override with_routing do |set| - set.draw do |map| + set.draw do match 'from_view', :to => 'url_options#from_view', :as => :from_view match ':controller/:action' end @@ -202,7 +202,18 @@ class UrlOptionsTest < ActionController::TestCase assert_equal 'http://www.override.com/from_view?locale=en', @controller.send(:from_view_url) assert_equal 'http://www.override.com/default_url_options/new?locale=en', @controller.url_for(:controller => 'default_url_options') end - end + end + + def test_url_helpers_does_not_become_actions + with_routing do |set| + set.draw do + match "account/overview" + end + + @controller.class.send(:include, set.url_helpers) + assert !@controller.class.action_methods.include?("account_overview_path") + end + end end class DefaultUrlOptionsTest < ActionController::TestCase @@ -216,7 +227,7 @@ class DefaultUrlOptionsTest < ActionController::TestCase def test_default_url_options_override with_routing do |set| - set.draw do |map| + set.draw do match 'from_view', :to => 'default_url_options#from_view', :as => :from_view match ':controller/:action' end @@ -231,7 +242,7 @@ class DefaultUrlOptionsTest < ActionController::TestCase def test_default_url_options_are_used_in_non_positional_parameters with_routing do |set| - set.draw do |map| + set.draw do scope("/:locale") do resources :descriptions end -- cgit v1.2.3 From c53f77f3be76b37e9692db0e3fd0e8c16d954e1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Tue, 23 Mar 2010 00:26:12 +0100 Subject: Rename unsignificant to insignificant. --- .../lib/action_view/helpers/number_helper.rb | 26 +++++++++++----------- actionpack/lib/action_view/locale/en.yml | 10 ++++----- .../test/template/number_helper_i18n_test.rb | 8 +++---- actionpack/test/template/number_helper_test.rb | 16 ++++++------- 4 files changed, 30 insertions(+), 30 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/number_helper.rb b/actionpack/lib/action_view/helpers/number_helper.rb index ca2322499c..719b64b940 100644 --- a/actionpack/lib/action_view/helpers/number_helper.rb +++ b/actionpack/lib/action_view/helpers/number_helper.rb @@ -132,7 +132,7 @@ module ActionView # * :significant - If +true+, precision will be the # of significant_digits. If +false+, the # of fractional digits (defaults to +false+) # * :separator - Sets the separator between the fractional and integer digits (defaults to "."). # * :delimiter - Sets the thousands delimiter (defaults to ""). - # * :strip_unsignificant_zeros - If +true+ removes unsignificant zeros after the decimal separator (defaults to +false+) + # * :strip_insignificant_zeros - If +true+ removes insignificant zeros after the decimal separator (defaults to +false+) # # ==== Examples # number_to_percentage(100) # => 100.000% @@ -221,7 +221,7 @@ module ActionView # * :significant - If +true+, precision will be the # of significant_digits. If +false+, the # of fractional digits (defaults to +false+) # * :separator - Sets the separator between the fractional and integer digits (defaults to "."). # * :delimiter - Sets the thousands delimiter (defaults to ""). - # * :strip_unsignificant_zeros - If +true+ removes unsignificant zeros after the decimal separator (defaults to +false+) + # * :strip_insignificant_zeros - If +true+ removes insignificant zeros after the decimal separator (defaults to +false+) # # ==== Examples # number_with_precision(111.2345) # => 111.235 @@ -231,7 +231,7 @@ module ActionView # number_with_precision(111.2345, :significant => true) # => 111 # number_with_precision(111.2345, :precision => 1, :significant => true) # => 100 # number_with_precision(13, :precision => 5, :significant => true) # => 13.000 - # number_with_precision(13, :precision => 5, :significant => true, strip_unsignificant_zeros => true) + # number_with_precision(13, :precision => 5, :significant => true, strip_insignificant_zeros => true) # # => 13 # number_with_precision(389.32314, :precision => 4, :significant => true) # => 389.3 # number_with_precision(1111.2345, :precision => 2, :separator => ',', :delimiter => '.') @@ -269,7 +269,7 @@ module ActionView options = options.reverse_merge(defaults) # Allow the user to unset default values: Eg.: :significant => false precision = options.delete :precision significant = options.delete :significant - strip_unsignificant_zeros = options.delete :strip_unsignificant_zeros + strip_insignificant_zeros = options.delete :strip_insignificant_zeros if significant and precision > 0 digits = (Math.log10(number) + 1).floor @@ -280,7 +280,7 @@ module ActionView rounded_number = BigDecimal.new((number * (10 ** precision)).to_s).round.to_f / 10 ** precision end formatted_number = number_with_delimiter("%01.#{precision}f" % rounded_number, options) - if strip_unsignificant_zeros + if strip_insignificant_zeros escaped_separator = Regexp.escape(options[:separator]) formatted_number.sub(/(#{escaped_separator})(\d*[1-9])?0+\z/, '\1\2').sub(/#{escaped_separator}\z/, '').html_safe else @@ -303,7 +303,7 @@ module ActionView # * :significant - If +true+, precision will be the # of significant_digits. If +false+, the # of fractional digits (defaults to +true+) # * :separator - Sets the separator between the fractional and integer digits (defaults to "."). # * :delimiter - Sets the thousands delimiter (defaults to ""). - # * :strip_unsignificant_zeros - If +true+ removes unsignificant zeros after the decimal separator (defaults to +true+) + # * :strip_insignificant_zeros - If +true+ removes insignificant zeros after the decimal separator (defaults to +true+) # ==== Examples # number_to_human_size(123) # => 123 Bytes # number_to_human_size(1234) # => 1.21 KB @@ -316,7 +316,7 @@ module ActionView # number_to_human_size(1234567, :precision => 2, :separator => ',') # => 1,2 MB # # Unsignificant zeros after the fractional separator are stripped out by default (set - # :strip_unsignificant_zeros to +false+ to change that): + # :strip_insignificant_zeros to +false+ to change that): # number_to_human_size(1234567890123, :precision => 5) # => "1.1229 TB" # number_to_human_size(524288000, :precision=>5) # => "500 MB" # @@ -349,8 +349,8 @@ module ActionView end options = options.reverse_merge(defaults) - #for backwards compatibility with those that didn't add strip_unsignificant_zeros to their locale files - options[:strip_unsignificant_zeros] = true if not options.key?(:strip_unsignificant_zeros) + #for backwards compatibility with those that didn't add strip_insignificant_zeros to their locale files + options[:strip_insignificant_zeros] = true if not options.key?(:strip_insignificant_zeros) storage_units_format = I18n.translate(:'number.human.storage_units.format', :locale => options[:locale], :raise => true) @@ -389,7 +389,7 @@ module ActionView # * :significant - If +true+, precision will be the # of significant_digits. If +false+, the # of fractional digits (defaults to +true+) # * :separator - Sets the separator between the fractional and integer digits (defaults to "."). # * :delimiter - Sets the thousands delimiter (defaults to ""). - # * :strip_unsignificant_zeros - If +true+ removes unsignificant zeros after the decimal separator (defaults to +true+) + # * :strip_insignificant_zeros - If +true+ removes insignificant zeros after the decimal separator (defaults to +true+) # * :units - A Hash of unit quantifier names. Or a string containing an i18n scope where to find this hash. It might have the following keys: # * *integers*: :unit, :ten, :hundred, :thousand, :million, :billion, :trillion, :quadrillion # * *fractionals*: :deci, :centi, :mili, :micro, :nano, :pico, :femto @@ -416,7 +416,7 @@ module ActionView # :significant => false) # => "1,2 Million" # # Unsignificant zeros after the decimal separator are stripped out by default (set - # :strip_unsignificant_zeros to +false+ to change that): + # :strip_insignificant_zeros to +false+ to change that): # number_to_human(12345012345, :significant_digits => 6) # => "12.345 Billion" # number_to_human(500000000, :precision=>5) # => "500 Million" # @@ -465,8 +465,8 @@ module ActionView defaults = defaults.merge(human) options = options.reverse_merge(defaults) - #for backwards compatibility with those that didn't add strip_unsignificant_zeros to their locale files - options[:strip_unsignificant_zeros] = true if not options.key?(:strip_unsignificant_zeros) + #for backwards compatibility with those that didn't add strip_insignificant_zeros to their locale files + options[:strip_insignificant_zeros] = true if not options.key?(:strip_insignificant_zeros) units = options.delete :units unit_exponents = case units diff --git a/actionpack/lib/action_view/locale/en.yml b/actionpack/lib/action_view/locale/en.yml index cfb4f7c390..a3e2230f6f 100644 --- a/actionpack/lib/action_view/locale/en.yml +++ b/actionpack/lib/action_view/locale/en.yml @@ -13,7 +13,7 @@ # of the number of decimal digits (1234 with precision 2 becomes 1200, 1.23543 becomes 1.2) significant: false # If set, the zeros after the decimal separator will always be stripped (eg.: 1.200 will be 1.2) - strip_unsignificant_zeros: false + strip_insignificant_zeros: false # Used in number_to_currency() currency: @@ -26,7 +26,7 @@ delimiter: "," precision: 2 significant: false - strip_unsignificant_zeros: false + strip_insignificant_zeros: false # Used in number_to_percentage() percentage: @@ -36,7 +36,7 @@ delimiter: "" # precision: # significant: false - # strip_unsignificant_zeros: false + # strip_insignificant_zeros: false # Used in number_to_precision() precision: @@ -46,7 +46,7 @@ delimiter: "" # precision: # significant: false - # strip_unsignificant_zeros: false + # strip_insignificant_zeros: false # Used in number_to_human_size() and number_to_human() human: @@ -56,7 +56,7 @@ delimiter: "" precision: 3 significant: true - strip_unsignificant_zeros: true + strip_insignificant_zeros: true # Used in number_to_human_size() storage_units: # Storage units output formatting. diff --git a/actionpack/test/template/number_helper_i18n_test.rb b/actionpack/test/template/number_helper_i18n_test.rb index 07a0e2792c..f730a0d7f5 100644 --- a/actionpack/test/template/number_helper_i18n_test.rb +++ b/actionpack/test/template/number_helper_i18n_test.rb @@ -6,13 +6,13 @@ class NumberHelperTest < ActionView::TestCase def setup I18n.backend.store_translations 'ts', :number => { - :format => { :precision => 3, :delimiter => ',', :separator => '.', :significant => false, :strip_unsignificant_zeros => false }, + :format => { :precision => 3, :delimiter => ',', :separator => '.', :significant => false, :strip_insignificant_zeros => false }, :currency => { :format => { :unit => '&$', :format => '%u - %n', :precision => 2 } }, :human => { :format => { :precision => 2, :significant => true, - :strip_unsignificant_zeros => true + :strip_insignificant_zeros => true }, :storage_units => { :format => "%n %u", @@ -35,7 +35,7 @@ class NumberHelperTest < ActionView::TestCase } } }, - :percentage => { :format => {:delimiter => '', :precision => 2, :strip_unsignificant_zeros => true} }, + :percentage => { :format => {:delimiter => '', :precision => 2, :strip_insignificant_zeros => true} }, :precision => { :format => {:delimiter => '', :significant => true} } }, :custom_units_for_number_to_human => {:mili => "mm", :centi => "cm", :deci => "dm", :unit => "m", :ten => "dam", :hundred => "hm", :thousand => "km"} @@ -60,7 +60,7 @@ class NumberHelperTest < ActionView::TestCase end def test_number_to_percentage - # to see if strip_unsignificant_zeros is true + # to see if strip_insignificant_zeros is true assert_equal("1%", number_to_percentage(1, :locale => 'ts')) # precision is 2, significant should be inherited assert_equal("1.24%", number_to_percentage(1.2434, :locale => 'ts')) diff --git a/actionpack/test/template/number_helper_test.rb b/actionpack/test/template/number_helper_test.rb index 6adbe9c098..50c57a5588 100644 --- a/actionpack/test/template/number_helper_test.rb +++ b/actionpack/test/template/number_helper_test.rb @@ -58,7 +58,7 @@ class NumberHelperTest < ActionView::TestCase assert_equal("302.06%", number_to_percentage(302.0574, {:precision => 2})) assert_equal("100.000%", number_to_percentage("100")) assert_equal("1000.000%", number_to_percentage("1000")) - assert_equal("123.4%", number_to_percentage(123.400, :precision => 3, :strip_unsignificant_zeros => true)) + assert_equal("123.4%", number_to_percentage(123.400, :precision => 3, :strip_insignificant_zeros => true)) assert_equal("1.000,000%", number_to_percentage(1000, :delimiter => '.', :separator => ',')) end @@ -124,9 +124,9 @@ class NumberHelperTest < ActionView::TestCase assert_equal "5.392900", number_with_precision(5.3929, :precision => 7, :significant => true ) end - def test_number_with_precision_with_strip_unsignificant_zeros - assert_equal "9775.43", number_with_precision(9775.43, :precision => 4, :strip_unsignificant_zeros => true ) - assert_equal "9775.2", number_with_precision(9775.2, :precision => 6, :significant => true, :strip_unsignificant_zeros => true ) + def test_number_with_precision_with_strip_insignificant_zeros + assert_equal "9775.43", number_with_precision(9775.43, :precision => 4, :strip_insignificant_zeros => true ) + assert_equal "9775.2", number_with_precision(9775.2, :precision => 6, :significant => true, :strip_insignificant_zeros => true ) end def test_number_with_precision_with_significant_true_and_zero_precision @@ -181,7 +181,7 @@ class NumberHelperTest < ActionView::TestCase assert_equal '500 MB', number_to_human_size(524288000, :precision=>3) assert_equal '40 KB', number_to_human_size(41010, :precision => 1) assert_equal '40 KB', number_to_human_size(41100, :precision => 2) - assert_equal '1.0 KB', number_to_human_size(kilobytes(1.0123), :precision => 2, :strip_unsignificant_zeros => false) + assert_equal '1.0 KB', number_to_human_size(kilobytes(1.0123), :precision => 2, :strip_insignificant_zeros => false) assert_equal '1.012 KB', number_to_human_size(kilobytes(1.0123), :precision => 3, :significant => false) assert_equal '1 KB', number_to_human_size(kilobytes(1.0123), :precision => 0, :significant => true) #ignores significant it precision is 0 end @@ -212,7 +212,7 @@ class NumberHelperTest < ActionView::TestCase assert_equal '490 Thousand', number_to_human(489939, :precision => 2) assert_equal '489.9 Thousand', number_to_human(489939, :precision => 4) assert_equal '489 Thousand', number_to_human(489000, :precision => 4) - assert_equal '489.0 Thousand', number_to_human(489000, :precision => 4, :strip_unsignificant_zeros => false) + assert_equal '489.0 Thousand', number_to_human(489000, :precision => 4, :strip_insignificant_zeros => false) assert_equal '1.2346 Million', number_to_human(1234567, :precision => 4, :significant => false) assert_equal '1,2 Million', number_to_human(1234567, :precision => 1, :significant => false, :separator => ',') assert_equal '1 Million', number_to_human(1234567, :precision => 0, :significant => true, :separator => ',') #significant forced to false @@ -289,8 +289,8 @@ class NumberHelperTest < ActionView::TestCase assert !number_to_human_size("").html_safe? assert number_to_human_size("asdf".html_safe).html_safe? - assert number_with_precision(1, :strip_unsignificant_zeros => false).html_safe? - assert number_with_precision(1, :strip_unsignificant_zeros => true).html_safe? + assert number_with_precision(1, :strip_insignificant_zeros => false).html_safe? + assert number_with_precision(1, :strip_insignificant_zeros => true).html_safe? assert !number_with_precision("").html_safe? assert number_with_precision("asdf".html_safe).html_safe? -- cgit v1.2.3 From 15c31c7639b4329eba341bbe894abc9b79edc5c3 Mon Sep 17 00:00:00 2001 From: wycats Date: Mon, 22 Mar 2010 17:04:56 -0700 Subject: open_session can just return the a dup of the current context. At this point, its entire purpose in the open_session {} case was to delegate back to the IntegrationTest anyway. --- .../lib/action_dispatch/testing/integration.rb | 27 +++------------------- actionpack/test/controller/integration_test.rb | 8 +++---- 2 files changed, 6 insertions(+), 29 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 4ed62a6ac4..621d63c5e2 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -297,7 +297,7 @@ module ActionDispatch # Reset the current session. This is useful for testing multiple sessions # in a single test case. def reset! - @integration_session = open_session + @integration_session = Integration::Session.new(app) end %w(get post put head delete cookies assigns @@ -323,30 +323,9 @@ module ActionDispatch # can use this method to open multiple sessions that ought to be tested # simultaneously. def open_session(app = nil) - session = Integration::Session.new(app || self.app) - - # delegate the fixture accessors back to the test instance - extras = Module.new { attr_accessor :delegate, :test_result } - if self.class.respond_to?(:fixture_table_names) - self.class.fixture_table_names.each do |table_name| - name = table_name.tr(".", "_") - next unless respond_to?(name) - extras.__send__(:define_method, name) { |*args| - delegate.send(name, *args) - } - end + dup.tap do |session| + yield session if block_given? end - - # delegate add_assertion to the test case - extras.__send__(:define_method, :add_assertion) { - test_result.add_assertion - } - session.extend(extras) - session.delegate = self - session.test_result = @_result - - yield session if block_given? - session end # Copy the instance variables from the current session instance into the diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index 2180466ca7..c9782856bd 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -173,14 +173,12 @@ class IntegrationTestTest < Test::Unit::TestCase end def test_opens_new_session - @test.class.expects(:fixture_table_names).times(2).returns(['foo']) - session1 = @test.open_session { |sess| } session2 = @test.open_session # implicit session - assert_kind_of ::ActionController::Integration::Session, session1 - assert_kind_of ::ActionController::Integration::Session, session2 - assert_not_equal session1, session2 + assert session1.respond_to?(:assert_template), "open_session makes assert_template available" + assert session2.respond_to?(:assert_template), "open_session makes assert_template available" + assert !session1.equal?(session2) end # RSpec mixes Matchers (which has a #method_missing) into -- cgit v1.2.3 From cc0e402aa870fa710d6b6189dc090244b4462308 Mon Sep 17 00:00:00 2001 From: wycats Date: Tue, 23 Mar 2010 14:08:31 -0700 Subject: Protect routes again so they don't end up as actions. We need a better solution than this. --- actionpack/lib/action_dispatch/routing/route_set.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 52f3fd1610..bb689beed9 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -179,6 +179,7 @@ module ActionDispatch url_for(options) end + protected :#{selector} END_EVAL helpers << selector end -- cgit v1.2.3 From 4c7c4061558bb8781da0d54159e3cebcb0a8c07a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Fri, 26 Mar 2010 01:12:24 +0100 Subject: Remove reference to unexistent methods and fix typo. --- actionpack/lib/action_view/lookup_context.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/lookup_context.rb b/actionpack/lib/action_view/lookup_context.rb index cf28772f12..10f1911e31 100644 --- a/actionpack/lib/action_view/lookup_context.rb +++ b/actionpack/lib/action_view/lookup_context.rb @@ -112,7 +112,7 @@ module ActionView end def default_handlers #:nodoc: - @detault_handlers ||= Template::Handlers.extensions + @default_handlers ||= Template::Handlers.extensions end def handlers_regexp #:nodoc: -- cgit v1.2.3 From b2c2b0ce459a215d389f3ab8bb9e33718460cf51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Fri, 26 Mar 2010 15:41:05 +0100 Subject: Rails router automatically calculated for you the controller and named routes in the following scenarios: match "home/about" #=> maps to home#about with named route home_about_path match "about" #=> does not work because it cannot guess the controller match "about" => "home#about" #=> maps to home#about with named route home_about_path match "home/about", :as => "about" #=> maps to home#about with named route about_path --- actionpack/lib/action_dispatch/routing/mapper.rb | 8 +++----- actionpack/test/dispatch/routing_test.rb | 14 +++++++++++--- 2 files changed, 14 insertions(+), 8 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index ddee742021..278cf383ee 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -55,10 +55,8 @@ module ActionDispatch path = normalize_path(path) if using_match_shorthand?(path, options) - options = { - :to => path[1..-1].sub(%r{/([^/]*)$}, '#\1'), - :as => path[1..-1].gsub("/", "_") - }.merge!(options) + options[:to] ||= path[1..-1].sub(%r{/([^/]*)$}, '#\1') + options[:as] ||= path[1..-1].gsub("/", "_") end [ path, options ] @@ -71,7 +69,7 @@ module ActionDispatch # match "account/overview" def using_match_shorthand?(path, options) - path && options.except(:via, :anchor).empty? && !path.include?(':') + path && options.except(:via, :anchor, :to, :as).empty? && path =~ %r{^/[\w\/]+$} end def normalize_path(path) diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index e0500af29d..87a46feec7 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -18,10 +18,9 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest default_url_options :host => "rubyonrails.org" controller :sessions do - get 'login' => :new, :as => :login + get 'login' => :new post 'login' => :create - - delete 'logout' => :destroy, :as => :logout + delete 'logout' => :destroy end resource :session do @@ -35,6 +34,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest match 'account/overview' match '/account/nested/overview' + match 'sign_in' => "sessions#new" match 'account/modulo/:name', :to => redirect("/%{name}s") match 'account/proc/:name', :to => redirect {|params| "/#{params[:name].pluralize}" } @@ -673,6 +673,14 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end + def test_convention_with_explicit_end + with_test_routes do + get '/sign_in' + assert_equal 'sessions#new', @response.body + assert_equal '/sign_in', sign_in_path + end + end + def test_redirect_with_complete_url with_test_routes do get '/account/google' -- cgit v1.2.3 From 395d6648ce7549f71dd0a76dc061e87f608aaaab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Fri, 26 Mar 2010 18:47:55 +0100 Subject: Move application configuration to the application configuration object, remove railtie_name and engine_name and allow to set the configuration object. --- actionpack/lib/action_controller/base.rb | 1 + actionpack/lib/action_controller/railtie.rb | 13 +++++++------ actionpack/lib/action_controller/railties/url_helpers.rb | 2 +- actionpack/lib/action_dispatch/railtie.rb | 3 +-- actionpack/lib/action_view/base.rb | 4 ++-- actionpack/lib/action_view/railtie.rb | 12 ++++++++++-- 6 files changed, 22 insertions(+), 13 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index ad2b68af21..5797282b41 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -19,6 +19,7 @@ module ActionController include SessionManagement include ActionController::Caching include ActionController::MimeResponds + include ActionController::PolymorphicRoutes # Rails 2.x compatibility include ActionController::Compatibility diff --git a/actionpack/lib/action_controller/railtie.rb b/actionpack/lib/action_controller/railtie.rb index 2626a31fc2..0ec89928af 100644 --- a/actionpack/lib/action_controller/railtie.rb +++ b/actionpack/lib/action_controller/railtie.rb @@ -1,16 +1,17 @@ require "rails" require "action_controller" +require "action_dispatch/railtie" require "action_view/railtie" require "active_support/core_ext/class/subclasses" require "active_support/deprecation/proxy_wrappers" require "active_support/deprecation" +require "action_controller/railties/log_subscriber" +require "action_controller/railties/url_helpers" + module ActionController class Railtie < Rails::Railtie - railtie_name :action_controller - - require "action_controller/railties/log_subscriber" - require "action_controller/railties/url_helpers" + config.action_controller = ActiveSupport::OrderedOptions.new ad = config.action_dispatch config.action_controller.singleton_class.send(:define_method, :session) do @@ -37,7 +38,7 @@ module ActionController ad.session_store = val end - log_subscriber ActionController::Railties::LogSubscriber.new + log_subscriber :action_controller, ActionController::Railties::LogSubscriber.new initializer "action_controller.logger" do ActionController.base_hook { self.logger ||= Rails.logger } @@ -69,7 +70,7 @@ module ActionController initializer "action_controller.url_helpers" do |app| ActionController.base_hook do - extend ::ActionController::Railtie::UrlHelpers.with(app.routes) + extend ::ActionController::Railties::UrlHelpers.with(app.routes) end message = "ActionController::Routing::Routes is deprecated. " \ diff --git a/actionpack/lib/action_controller/railties/url_helpers.rb b/actionpack/lib/action_controller/railties/url_helpers.rb index ad2a8d4ef3..5f95e1c621 100644 --- a/actionpack/lib/action_controller/railties/url_helpers.rb +++ b/actionpack/lib/action_controller/railties/url_helpers.rb @@ -1,5 +1,5 @@ module ActionController - class Railtie + module Railties module UrlHelpers def self.with(router) Module.new do diff --git a/actionpack/lib/action_dispatch/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb index e486bd4079..430c90e515 100644 --- a/actionpack/lib/action_dispatch/railtie.rb +++ b/actionpack/lib/action_dispatch/railtie.rb @@ -3,8 +3,7 @@ require "rails" module ActionDispatch class Railtie < Rails::Railtie - railtie_name :action_dispatch - + config.action_dispatch = ActiveSupport::OrderedOptions.new config.action_dispatch.x_sendfile_header = "X-Sendfile" config.action_dispatch.ip_spoofing_check = true diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 9eec8ecce8..919b1e3470 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -159,8 +159,6 @@ module ActionView #:nodoc: include Helpers, Rendering, Partials, Layouts, ::ERB::Util, Context extend ActiveSupport::Memoizable - ActionView.run_base_hooks(self) - # Specify whether RJS responses should be wrapped in a try/catch block # that alert()s the caught exception (and then re-raises it). cattr_accessor :debug_rjs @@ -175,6 +173,8 @@ module ActionView #:nodoc: delegate :logger, :to => 'ActionController::Base', :allow_nil => true end + ActionView.run_base_hooks(self) + attr_accessor :base_path, :assigns, :template_extension, :lookup_context attr_internal :captures, :request, :controller, :template, :config diff --git a/actionpack/lib/action_view/railtie.rb b/actionpack/lib/action_view/railtie.rb index 2e5d115630..9cf007cd2b 100644 --- a/actionpack/lib/action_view/railtie.rb +++ b/actionpack/lib/action_view/railtie.rb @@ -3,10 +3,10 @@ require "rails" module ActionView class Railtie < Rails::Railtie - railtie_name :action_view + config.action_view = ActiveSupport::OrderedOptions.new require "action_view/railties/log_subscriber" - log_subscriber ActionView::Railties::LogSubscriber.new + log_subscriber :action_view, ActionView::Railties::LogSubscriber.new initializer "action_view.cache_asset_timestamps" do |app| unless app.config.cache_classes @@ -15,5 +15,13 @@ module ActionView end end end + + initializer "action_view.set_configs" do |app| + ActionView.base_hook do + app.config.action_view.each do |k,v| + send "#{k}=", v + end + end + end end end \ No newline at end of file -- cgit v1.2.3