diff options
29 files changed, 389 insertions, 111 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index df72794c31..8922a4ac32 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,5 +1,11 @@ ## Rails 4.0.0 (unreleased) ## +* Adds `image_url`, `javascript_url`, `stylesheet_url`, `audio_url`, `video_url`, and `font_url` + to assets tag helper. These URL helpers will return the full path to your assets. This is useful + when you are going to reference this asset from external host. *Prem Sichanugrist* + +* Default responder will now always use your overridden block in `respond_with` to render your response. *Prem Sichanugrist* + * Allow `value_method` and `text_method` arguments from `collection_select` and `options_from_collection_for_select` to receive an object that responds to `:call`, such as a `proc`, to evaluate the option in the current element context. This works diff --git a/actionpack/lib/abstract_controller/callbacks.rb b/actionpack/lib/abstract_controller/callbacks.rb index d61475c844..44c9ea34ba 100644 --- a/actionpack/lib/abstract_controller/callbacks.rb +++ b/actionpack/lib/abstract_controller/callbacks.rb @@ -8,7 +8,7 @@ module AbstractController include ActiveSupport::Callbacks included do - define_callbacks :process_action, :terminator => "response_body" + define_callbacks :process_action, :terminator => "response_body", :skip_after_callbacks_if_terminated => true end # Override AbstractController::Base's process_action to run the @@ -165,7 +165,6 @@ module AbstractController # for details on the allowed parameters. def #{filter}_filter(*names, &blk) # def before_filter(*names, &blk) _insert_callbacks(names, blk) do |name, options| # _insert_callbacks(names, blk) do |name, options| - options[:if] = (Array(options[:if]) << "!halted") if #{filter == :after} # options[:if] = (Array(options[:if]) << "!halted") if false set_callback(:process_action, :#{filter}, name, options) # set_callback(:process_action, :before, name, options) end # end end # end @@ -174,7 +173,6 @@ module AbstractController # for details on the allowed parameters. def prepend_#{filter}_filter(*names, &blk) # def prepend_before_filter(*names, &blk) _insert_callbacks(names, blk) do |name, options| # _insert_callbacks(names, blk) do |name, options| - options[:if] = (Array(options[:if]) << "!halted") if #{filter == :after} # options[:if] = (Array(options[:if]) << "!halted") if false set_callback(:process_action, :#{filter}, name, options.merge(:prepend => true)) # set_callback(:process_action, :before, name, options.merge(:prepend => true)) end # end end # end diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index ca383be76b..80ecc16d53 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -191,8 +191,9 @@ module ActionController #:nodoc: def respond_to(*mimes, &block) raise ArgumentError, "respond_to takes either types or a block, never both" if mimes.any? && block_given? - if response = retrieve_response_from_mimes(mimes, &block) - response.call(nil) + if collector = retrieve_collector_from_mimes(mimes, &block) + response = collector.response + response ? response.call : default_render({}) end end @@ -232,10 +233,18 @@ module ActionController #:nodoc: raise "In order to use respond_with, first you need to declare the formats your " << "controller responds to in the class level" if self.class.mimes_for_respond_to.empty? - if response = retrieve_response_from_mimes(&block) + if collector = retrieve_collector_from_mimes(&block) options = resources.size == 1 ? {} : resources.extract_options! - options.merge!(:default_response => response) - (options.delete(:responder) || self.class.responder).call(self, resources, options) + + if defined_response = collector.response + if action = options.delete(:action) + render :action => action + else + defined_response.call + end + else + (options.delete(:responder) || self.class.responder).call(self, resources, options) + end end end @@ -263,15 +272,16 @@ module ActionController #:nodoc: # Collects mimes and return the response for the negotiated format. Returns # nil if :not_acceptable was sent to the client. # - def retrieve_response_from_mimes(mimes=nil, &block) #:nodoc: + def retrieve_collector_from_mimes(mimes=nil, &block) #:nodoc: mimes ||= collect_mimes_from_class_level - collector = Collector.new(mimes) { |options| default_render(options || {}) } + collector = Collector.new(mimes) block.call(collector) if block_given? + format = collector.negotiate_format(request) - if format = request.negotiate_mime(collector.order) + if format self.content_type ||= format.to_s lookup_context.freeze_formats([format.to_sym]) - collector.response_for(format) + collector else head :not_acceptable nil @@ -280,10 +290,10 @@ module ActionController #:nodoc: class Collector #:nodoc: include AbstractController::Collector - attr_accessor :order + attr_accessor :order, :format - def initialize(mimes, &block) - @order, @responses, @default_response = [], {}, block + def initialize(mimes) + @order, @responses = [], {} mimes.each { |mime| send(mime) } end @@ -302,8 +312,12 @@ module ActionController #:nodoc: @responses[mime_type] ||= block end - def response_for(mime) - @responses[mime] || @responses[Mime::ALL] || @default_response + def response + @responses[format] || @responses[Mime::ALL] + end + + def negotiate_format(request) + @format = request.negotiate_mime(order) end end end diff --git a/actionpack/lib/action_controller/metal/responder.rb b/actionpack/lib/action_controller/metal/responder.rb index 9500a349cb..4ad64bff20 100644 --- a/actionpack/lib/action_controller/metal/responder.rb +++ b/actionpack/lib/action_controller/metal/responder.rb @@ -129,7 +129,6 @@ module ActionController #:nodoc: @resources = resources @options = options @action = options.delete(:action) - @default_response = options.delete(:default_response) end delegate :head, :render, :redirect_to, :to => :controller @@ -226,7 +225,7 @@ module ActionController #:nodoc: # controller. # def default_render - @default_response.call(options) + controller.default_render(options) end # Display is just a shortcut to render a resource with the current format. diff --git a/actionpack/lib/action_dispatch/middleware/stack.rb b/actionpack/lib/action_dispatch/middleware/stack.rb index a4308f528c..28e8fbdab8 100644 --- a/actionpack/lib/action_dispatch/middleware/stack.rb +++ b/actionpack/lib/action_dispatch/middleware/stack.rb @@ -93,8 +93,9 @@ module ActionDispatch end def swap(target, *args, &block) - insert_before(target, *args, &block) - delete(target) + index = assert_index(target, :before) + insert(index, *args, &block) + middlewares.delete_at(index + 1) end def delete(target) diff --git a/actionpack/lib/action_view/helpers/asset_tag_helper.rb b/actionpack/lib/action_view/helpers/asset_tag_helper.rb index 40a950c644..7029ca8a8d 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helper.rb @@ -278,6 +278,13 @@ module ActionView end alias_method :path_to_image, :image_path # aliased to avoid conflicts with an image_path named route + # Computes the full URL to an image asset in the public images directory. + # This will use +image_path+ internally, so most of their behaviors will be the same. + def image_url(source) + URI.join(current_host, path_to_image(source)).to_s + end + alias_method :url_to_image, :image_url # aliased to avoid conflicts with an image_url named route + # Computes the path to a video asset in the public videos directory. # Full paths from the document root will be passed through. # Used internally by +video_tag+ to build the video path. @@ -293,6 +300,13 @@ module ActionView end alias_method :path_to_video, :video_path # aliased to avoid conflicts with a video_path named route + # Computes the full URL to a video asset in the public videos directory. + # This will use +video_path+ internally, so most of their behaviors will be the same. + def video_url(source) + URI.join(current_host, path_to_video(source)).to_s + end + alias_method :url_to_video, :video_url # aliased to avoid conflicts with an video_url named route + # Computes the path to an audio asset in the public audios directory. # Full paths from the document root will be passed through. # Used internally by +audio_tag+ to build the audio path. @@ -308,6 +322,13 @@ module ActionView end alias_method :path_to_audio, :audio_path # aliased to avoid conflicts with an audio_path named route + # Computes the full URL to a audio asset in the public audios directory. + # This will use +audio_path+ internally, so most of their behaviors will be the same. + def audio_url(source) + URI.join(current_host, path_to_audio(source)).to_s + end + alias_method :url_to_audio, :audio_url # aliased to avoid conflicts with an audio_url named route + # Computes the path to a font asset in the public fonts directory. # Full paths from the document root will be passed through. # @@ -322,6 +343,13 @@ module ActionView end alias_method :path_to_font, :font_path # aliased to avoid conflicts with an font_path named route + # Computes the full URL to a font asset in the public fonts directory. + # This will use +font_path+ internally, so most of their behaviors will be the same. + def font_url(source) + URI.join(current_host, path_to_font(source)).to_s + end + alias_method :url_to_font, :font_url # aliased to avoid conflicts with an font_url named route + # Returns an html image tag for the +source+. The +source+ can be a full # path or a file that exists in your public images directory. # @@ -463,6 +491,10 @@ module ActionView tag(type, options) end end + + def current_host + url_for(:only_path => false) + end end end end diff --git a/actionpack/lib/action_view/helpers/asset_tag_helpers/javascript_tag_helpers.rb b/actionpack/lib/action_view/helpers/asset_tag_helpers/javascript_tag_helpers.rb index d9f1f88ade..c67f81dcf4 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helpers/javascript_tag_helpers.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helpers/javascript_tag_helpers.rb @@ -87,6 +87,13 @@ module ActionView end alias_method :path_to_javascript, :javascript_path # aliased to avoid conflicts with a javascript_path named route + # Computes the full URL to a javascript asset in the public javascripts directory. + # This will use +javascript_path+ internally, so most of their behaviors will be the same. + def javascript_url(source) + URI.join(current_host, path_to_javascript(source)).to_s + end + alias_method :url_to_javascript, :javascript_url # aliased to avoid conflicts with a javascript_url named route + # Returns an HTML script tag for each of the +sources+ provided. # # Sources may be paths to JavaScript files. Relative paths are assumed to be relative diff --git a/actionpack/lib/action_view/helpers/asset_tag_helpers/stylesheet_tag_helpers.rb b/actionpack/lib/action_view/helpers/asset_tag_helpers/stylesheet_tag_helpers.rb index 41958c6559..2584b67548 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helpers/stylesheet_tag_helpers.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helpers/stylesheet_tag_helpers.rb @@ -65,6 +65,13 @@ module ActionView end alias_method :path_to_stylesheet, :stylesheet_path # aliased to avoid conflicts with a stylesheet_path named route + # Computes the full URL to a stylesheet asset in the public stylesheets directory. + # This will use +stylesheet_path+ internally, so most of their behaviors will be the same. + def stylesheet_url(source) + URI.join(current_host, path_to_stylesheet(source)).to_s + end + alias_method :url_to_stylesheet, :stylesheet_url # aliased to avoid conflicts with a stylesheet_url named route + # Returns a stylesheet link tag for the sources specified as arguments. If # you don't specify an extension, <tt>.css</tt> will be appended automatically. # You can modify the link attributes by passing a hash as the last argument. diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index 60498822ff..69a8f4f213 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -593,6 +593,19 @@ class RenderJsonRespondWithController < RespondWithController format.json { render :json => RenderJsonTestException.new('boom') } end end + + def create + resource = ValidatedCustomer.new(params[:name], 1) + respond_with(resource) do |format| + format.json do + if resource.errors.empty? + render :json => { :valid => true } + else + render :json => { :valid => false } + end + end + end + end end class EmptyRespondWithController < ActionController::Base @@ -964,6 +977,18 @@ class RespondWithControllerTest < ActionController::TestCase assert_match(/"error":"RenderJsonTestException"/, @response.body) end + def test_api_response_with_valid_resource_respect_override_block + @controller = RenderJsonRespondWithController.new + post :create, :name => "sikachu", :format => :json + assert_equal '{"valid":true}', @response.body + end + + def test_api_response_with_invalid_resource_respect_override_block + @controller = RenderJsonRespondWithController.new + post :create, :name => "david", :format => :json + assert_equal '{"valid":false}', @response.body + end + def test_no_double_render_is_raised @request.accept = "text/html" assert_raise ActionView::MissingTemplate do diff --git a/actionpack/test/dispatch/middleware_stack_test.rb b/actionpack/test/dispatch/middleware_stack_test.rb index 831f3db3e2..4191ed1ff4 100644 --- a/actionpack/test/dispatch/middleware_stack_test.rb +++ b/actionpack/test/dispatch/middleware_stack_test.rb @@ -81,6 +81,12 @@ class MiddlewareStackTest < ActiveSupport::TestCase assert_equal BazMiddleware, @stack[0].klass end + test "swaps one middleware out for same middleware class" do + assert_equal FooMiddleware, @stack[0].klass + @stack.swap(FooMiddleware, FooMiddleware, Proc.new { |env| [500, {}, ['error!']] }) + assert_equal FooMiddleware, @stack[0].klass + end + test "raise an error on invalid index" do assert_raise RuntimeError do @stack.insert("HiyaMiddleware", BazMiddleware) diff --git a/actionpack/test/dispatch/static_test.rb b/actionpack/test/dispatch/static_test.rb index 9f3cbd19ef..b7a53353a9 100644 --- a/actionpack/test/dispatch/static_test.rb +++ b/actionpack/test/dispatch/static_test.rb @@ -1,3 +1,4 @@ +# encoding: utf-8 require 'abstract_unit' module StaticTests @@ -30,6 +31,10 @@ module StaticTests assert_html "/foo/index.html", get("/foo") end + def test_served_static_file_with_non_english_filename + assert_html "means hello in Japanese\n", get("/foo/#{Rack::Utils.escape("こんにちは.html")}") + end + private def assert_html(body, response) @@ -53,4 +58,4 @@ class StaticTest < ActiveSupport::TestCase end include StaticTests -end
\ No newline at end of file +end diff --git a/actionpack/test/fixtures/public/foo/こんにちは.html b/actionpack/test/fixtures/public/foo/こんにちは.html new file mode 100644 index 0000000000..1df9166522 --- /dev/null +++ b/actionpack/test/fixtures/public/foo/こんにちは.html @@ -0,0 +1 @@ +means hello in Japanese diff --git a/actionpack/test/lib/controller/fake_models.rb b/actionpack/test/lib/controller/fake_models.rb index f2362714d7..bbb4cc5ef3 100644 --- a/actionpack/test/lib/controller/fake_models.rb +++ b/actionpack/test/lib/controller/fake_models.rb @@ -34,6 +34,16 @@ end class GoodCustomer < Customer end +class ValidatedCustomer < Customer + def errors + if name =~ /Sikachu/i + [] + else + [{:name => "is invalid"}] + end + end +end + module Quiz class Question < Struct.new(:name, :id) extend ActiveModel::Naming diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb index 90c2ca7a3d..2d48cd18b4 100644 --- a/actionpack/test/template/asset_tag_helper_test.rb +++ b/actionpack/test/template/asset_tag_helper_test.rb @@ -88,6 +88,18 @@ class AssetTagHelperTest < ActionView::TestCase %(path_to_javascript("/super/xmlhr.js")) => %(/super/xmlhr.js) } + JavascriptUrlToTag = { + %(javascript_url("xmlhr")) => %(http://www.example.com/javascripts/xmlhr.js), + %(javascript_url("super/xmlhr")) => %(http://www.example.com/javascripts/super/xmlhr.js), + %(javascript_url("/super/xmlhr.js")) => %(http://www.example.com/super/xmlhr.js) + } + + UrlToJavascriptToTag = { + %(url_to_javascript("xmlhr")) => %(http://www.example.com/javascripts/xmlhr.js), + %(url_to_javascript("super/xmlhr")) => %(http://www.example.com/javascripts/super/xmlhr.js), + %(url_to_javascript("/super/xmlhr.js")) => %(http://www.example.com/super/xmlhr.js) + } + JavascriptIncludeToTag = { %(javascript_include_tag("bank")) => %(<script src="/javascripts/bank.js" type="text/javascript"></script>), %(javascript_include_tag("bank.js")) => %(<script src="/javascripts/bank.js" type="text/javascript"></script>), @@ -119,6 +131,20 @@ class AssetTagHelperTest < ActionView::TestCase %(path_to_stylesheet('/dir/file.rcss')) => %(/dir/file.rcss) } + StyleUrlToTag = { + %(stylesheet_url("bank")) => %(http://www.example.com/stylesheets/bank.css), + %(stylesheet_url("bank.css")) => %(http://www.example.com/stylesheets/bank.css), + %(stylesheet_url('subdir/subdir')) => %(http://www.example.com/stylesheets/subdir/subdir.css), + %(stylesheet_url('/subdir/subdir.css')) => %(http://www.example.com/subdir/subdir.css) + } + + UrlToStyleToTag = { + %(url_to_stylesheet("style")) => %(http://www.example.com/stylesheets/style.css), + %(url_to_stylesheet("style.css")) => %(http://www.example.com/stylesheets/style.css), + %(url_to_stylesheet('dir/file')) => %(http://www.example.com/stylesheets/dir/file.css), + %(url_to_stylesheet('/dir/file.rcss')) => %(http://www.example.com/dir/file.rcss) + } + StyleLinkToTag = { %(stylesheet_link_tag("bank")) => %(<link href="/stylesheets/bank.css" media="screen" rel="stylesheet" type="text/css" />), %(stylesheet_link_tag("bank.css")) => %(<link href="/stylesheets/bank.css" media="screen" rel="stylesheet" type="text/css" />), @@ -149,6 +175,20 @@ class AssetTagHelperTest < ActionView::TestCase %(path_to_image("/dir/xml.png")) => %(/dir/xml.png) } + ImageUrlToTag = { + %(image_url("xml")) => %(http://www.example.com/images/xml), + %(image_url("xml.png")) => %(http://www.example.com/images/xml.png), + %(image_url("dir/xml.png")) => %(http://www.example.com/images/dir/xml.png), + %(image_url("/dir/xml.png")) => %(http://www.example.com/dir/xml.png) + } + + UrlToImageToTag = { + %(url_to_image("xml")) => %(http://www.example.com/images/xml), + %(url_to_image("xml.png")) => %(http://www.example.com/images/xml.png), + %(url_to_image("dir/xml.png")) => %(http://www.example.com/images/dir/xml.png), + %(url_to_image("/dir/xml.png")) => %(http://www.example.com/dir/xml.png) + } + ImageLinkToTag = { %(image_tag("xml.png")) => %(<img alt="Xml" src="/images/xml.png" />), %(image_tag("rss.gif", :alt => "rss syndication")) => %(<img alt="rss syndication" src="/images/rss.gif" />), @@ -189,6 +229,20 @@ class AssetTagHelperTest < ActionView::TestCase %(path_to_video("/dir/xml.ogg")) => %(/dir/xml.ogg) } + VideoUrlToTag = { + %(video_url("xml")) => %(http://www.example.com/videos/xml), + %(video_url("xml.ogg")) => %(http://www.example.com/videos/xml.ogg), + %(video_url("dir/xml.ogg")) => %(http://www.example.com/videos/dir/xml.ogg), + %(video_url("/dir/xml.ogg")) => %(http://www.example.com/dir/xml.ogg) + } + + UrlToVideoToTag = { + %(url_to_video("xml")) => %(http://www.example.com/videos/xml), + %(url_to_video("xml.ogg")) => %(http://www.example.com/videos/xml.ogg), + %(url_to_video("dir/xml.ogg")) => %(http://www.example.com/videos/dir/xml.ogg), + %(url_to_video("/dir/xml.ogg")) => %(http://www.example.com/dir/xml.ogg) + } + VideoLinkToTag = { %(video_tag("xml.ogg")) => %(<video src="/videos/xml.ogg" />), %(video_tag("rss.m4v", :autoplay => true, :controls => true)) => %(<video autoplay="autoplay" controls="controls" src="/videos/rss.m4v" />), @@ -206,7 +260,7 @@ class AssetTagHelperTest < ActionView::TestCase %(video_tag(["multiple.ogg", "multiple.avi"], :size => "160x120", :controls => true)) => %(<video controls="controls" height="120" width="160"><source src="/videos/multiple.ogg" /><source src="/videos/multiple.avi" /></video>) } - AudioPathToTag = { + AudioPathToTag = { %(audio_path("xml")) => %(/audios/xml), %(audio_path("xml.wav")) => %(/audios/xml.wav), %(audio_path("dir/xml.wav")) => %(/audios/dir/xml.wav), @@ -220,6 +274,20 @@ class AssetTagHelperTest < ActionView::TestCase %(path_to_audio("/dir/xml.wav")) => %(/dir/xml.wav) } + AudioUrlToTag = { + %(audio_url("xml")) => %(http://www.example.com/audios/xml), + %(audio_url("xml.wav")) => %(http://www.example.com/audios/xml.wav), + %(audio_url("dir/xml.wav")) => %(http://www.example.com/audios/dir/xml.wav), + %(audio_url("/dir/xml.wav")) => %(http://www.example.com/dir/xml.wav) + } + + UrlToAudioToTag = { + %(url_to_audio("xml")) => %(http://www.example.com/audios/xml), + %(url_to_audio("xml.wav")) => %(http://www.example.com/audios/xml.wav), + %(url_to_audio("dir/xml.wav")) => %(http://www.example.com/audios/dir/xml.wav), + %(url_to_audio("/dir/xml.wav")) => %(http://www.example.com/dir/xml.wav) + } + AudioLinkToTag = { %(audio_tag("xml.wav")) => %(<audio src="/audios/xml.wav" />), %(audio_tag("rss.wav", :autoplay => true, :controls => true)) => %(<audio autoplay="autoplay" controls="controls" src="/audios/rss.wav" />), @@ -242,6 +310,14 @@ class AssetTagHelperTest < ActionView::TestCase PathToJavascriptToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } end + def test_javascript_url + JavascriptUrlToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } + end + + def test_url_to_javascript_alias_for_javascript_url + UrlToJavascriptToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } + end + def test_javascript_include_tag_with_blank_asset_id ENV["RAILS_ASSET_ID"] = "" JavascriptIncludeToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } @@ -349,6 +425,15 @@ class AssetTagHelperTest < ActionView::TestCase PathToStyleToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } end + def test_stylesheet_url + ENV["RAILS_ASSET_ID"] = "" + StyleUrlToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } + end + + def test_url_to_stylesheet_alias_for_stylesheet_url + UrlToStyleToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } + end + def test_stylesheet_link_tag ENV["RAILS_ASSET_ID"] = "" StyleLinkToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } @@ -433,6 +518,14 @@ class AssetTagHelperTest < ActionView::TestCase PathToImageToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } end + def test_image_url + ImageUrlToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } + end + + def test_url_to_image_alias_for_image_url + UrlToImageToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } + end + def test_image_alt [nil, '/', '/foo/bar/', 'foo/bar/'].each do |prefix| assert_equal 'Rails', image_alt("#{prefix}rails.png") @@ -478,6 +571,14 @@ class AssetTagHelperTest < ActionView::TestCase PathToVideoToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } end + def test_video_url + VideoUrlToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } + end + + def test_url_to_video_alias_for_video_url + UrlToVideoToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } + end + def test_video_tag VideoLinkToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } end @@ -490,6 +591,14 @@ class AssetTagHelperTest < ActionView::TestCase PathToAudioToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } end + def test_audio_url + AudioUrlToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } + end + + def test_url_to_audio_alias_for_audio_url + UrlToAudioToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } + end + def test_audio_tag AudioLinkToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } end @@ -573,7 +682,6 @@ class AssetTagHelperTest < ActionView::TestCase end end - @controller.request.stubs(:ssl?).returns(false) assert_equal "http://assets15.example.com/images/xml.png", image_path("xml.png") @@ -1141,6 +1249,22 @@ class AssetTagHelperNonVhostTest < ActionView::TestCase assert_dom_equal(%(<img alt="Mouse2" onmouseover="this.src='gopher://assets.example.com/collaboration/hieraki/images/mouse_over2.png'" onmouseout="this.src='gopher://assets.example.com/collaboration/hieraki/images/mouse2.png'" src="gopher://assets.example.com/collaboration/hieraki/images/mouse2.png" />), image_tag("mouse2.png", :mouseover => image_path("mouse_over2.png"))) end + def test_should_compute_proper_url_with_asset_host + @controller.config.asset_host = "assets.example.com" + assert_dom_equal(%(<link href="http://www.example.com/collaboration/hieraki" rel="alternate" title="RSS" type="application/rss+xml" />), auto_discovery_link_tag) + assert_dom_equal(%(gopher://assets.example.com/collaboration/hieraki/javascripts/xmlhr.js), javascript_url("xmlhr")) + assert_dom_equal(%(gopher://assets.example.com/collaboration/hieraki/stylesheets/style.css), stylesheet_url("style")) + assert_dom_equal(%(gopher://assets.example.com/collaboration/hieraki/images/xml.png), image_url("xml.png")) + end + + def test_should_compute_proper_url_with_asset_host_and_default_protocol + @controller.config.asset_host = "assets.example.com" + @controller.config.default_asset_host_protocol = :request + assert_dom_equal(%(gopher://assets.example.com/collaboration/hieraki/javascripts/xmlhr.js), javascript_url("xmlhr")) + assert_dom_equal(%(gopher://assets.example.com/collaboration/hieraki/stylesheets/style.css), stylesheet_url("style")) + assert_dom_equal(%(gopher://assets.example.com/collaboration/hieraki/images/xml.png), image_url("xml.png")) + end + def test_should_ignore_asset_host_on_complete_url @controller.config.asset_host = "http://assets.example.com" assert_dom_equal(%(<link href="http://bar.example.com/stylesheets/style.css" media="screen" rel="stylesheet" type="text/css" />), stylesheet_link_tag("http://bar.example.com/stylesheets/style.css")) @@ -1154,16 +1278,19 @@ class AssetTagHelperNonVhostTest < ActionView::TestCase def test_should_wildcard_asset_host_between_zero_and_four @controller.config.asset_host = 'http://a%d.example.com' assert_match(%r(http://a[0123].example.com/collaboration/hieraki/images/xml.png), image_path('xml.png')) + assert_match(%r(http://a[0123].example.com/collaboration/hieraki/images/xml.png), image_url('xml.png')) end def test_asset_host_without_protocol_should_be_protocol_relative @controller.config.asset_host = 'a.example.com' assert_equal 'gopher://a.example.com/collaboration/hieraki/images/xml.png', image_path('xml.png') + assert_equal 'gopher://a.example.com/collaboration/hieraki/images/xml.png', image_url('xml.png') end def test_asset_host_without_protocol_should_be_protocol_relative_even_if_path_present @controller.config.asset_host = 'a.example.com/files/go/here' assert_equal 'gopher://a.example.com/files/go/here/collaboration/hieraki/images/xml.png', image_path('xml.png') + assert_equal 'gopher://a.example.com/files/go/here/collaboration/hieraki/images/xml.png', image_url('xml.png') end def test_assert_css_and_js_of_the_same_name_return_correct_extension diff --git a/activemodel/lib/active_model/callbacks.rb b/activemodel/lib/active_model/callbacks.rb index 25d26ede52..ebb4b51aa3 100644 --- a/activemodel/lib/active_model/callbacks.rb +++ b/activemodel/lib/active_model/callbacks.rb @@ -88,6 +88,7 @@ module ActiveModel options = callbacks.extract_options! options = { :terminator => "result == false", + :skip_after_callbacks_if_terminated => true, :scope => [:kind, :name], :only => [:before, :around, :after] }.merge(options) @@ -124,7 +125,7 @@ module ActiveModel def self.after_#{callback}(*args, &block) options = args.extract_options! options[:prepend] = true - options[:if] = Array(options[:if]) << "!halted && value != false" + options[:if] = Array(options[:if]) << "value != false" set_callback(:#{callback}, :after, *(args << options), &block) end CALLBACK diff --git a/activemodel/lib/active_model/validations/callbacks.rb b/activemodel/lib/active_model/validations/callbacks.rb index c80ace7b82..c39c85e1af 100644 --- a/activemodel/lib/active_model/validations/callbacks.rb +++ b/activemodel/lib/active_model/validations/callbacks.rb @@ -23,7 +23,7 @@ module ActiveModel included do include ActiveSupport::Callbacks - define_callbacks :validation, :terminator => "result == false", :scope => [:kind, :name] + define_callbacks :validation, :terminator => "result == false", :skip_after_callbacks_if_terminated => true, :scope => [:kind, :name] end module ClassMethods @@ -40,7 +40,6 @@ module ActiveModel options = args.extract_options! options[:prepend] = true options[:if] = Array(options[:if]) - options[:if] << "!halted" options[:if].unshift("self.validation_context == :#{options[:on]}") if options[:on] set_callback(:validation, :after, *(args << options), &block) end diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 4b061cae4a..c929ae0ae5 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,5 +1,9 @@ ## Rails 4.0.0 (unreleased) ## +* AS::Callbacks: `:per_key` option is no longer supported + +* `AS::Callbacks#define_callbacks`: add `:skip_after_callbacks_if_terminated` option. + * Add html_escape_once to ERB::Util, and delegate escape_once tag helper to it. *Carlos Antonio da Silva* * Remove ActiveSupport::TestCase#pending method, use `skip` instead. *Carlos Antonio da Silva* diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index bc6bd55a45..1834027e7b 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -66,8 +66,6 @@ module ActiveSupport # # Calls the before and around callbacks in the order they were set, yields # the block (if given one), and then runs the after callbacks in reverse order. - # Optionally accepts a key, which will be used to compile an optimized callback - # method for each key. See +ClassMethods.define_callbacks+ for more information. # # If the callback chain was halted, returns +false+. Otherwise returns the result # of the block, or +true+ if no block is given. @@ -77,7 +75,8 @@ module ActiveSupport # end # def run_callbacks(kind, key = nil, &block) - self.class.__run_callbacks(key, kind, self, &block) + #TODO: deprecate key argument + self.class.__run_callbacks(kind, self, &block) end private @@ -95,12 +94,18 @@ module ActiveSupport def initialize(chain, filter, kind, options, klass) @chain, @kind, @klass = chain, kind, klass + deprecate_per_key_option(options) normalize_options!(options) @raw_filter, @options = filter, options @filter = _compile_filter(filter) - @compiled_options = _compile_options(options) - @callback_id = next_id + recompile_options! + end + + def deprecate_per_key_option(options) + if options[:per_key] + raise NotImplementedError, ":per_key option is no longer supported. Use generic :if and :unless options instead." + end end def clone(chain, klass) @@ -116,11 +121,6 @@ module ActiveSupport def normalize_options!(options) options[:if] = Array(options[:if]) options[:unless] = Array(options[:unless]) - - options[:per_key] ||= {} - - options[:if] += Array(options[:per_key][:if]) - options[:unless] += Array(options[:per_key][:unless]) end def name @@ -140,17 +140,15 @@ module ActiveSupport filter_options[:unless].concat(Array(new_options[:if])) if new_options.key?(:if) end - def recompile!(_options, _per_key) + def recompile!(_options) + deprecate_per_key_option(_options) _update_filter(self.options, _options) - _update_filter(self.options, _per_key) - @callback_id = next_id - @filter = _compile_filter(@raw_filter) - @compiled_options = _compile_options(@options) + recompile_options! end # Wraps code with filter - def apply(code, key=nil, object=nil) + def apply(code) case @kind when :before <<-RUBY_EVAL @@ -169,7 +167,7 @@ module ActiveSupport when :after <<-RUBY_EVAL #{code} - if #{@compiled_options} + if #{!chain.config[:skip_after_callbacks_if_terminated] || "!halted"} && #{@compiled_options} #{@filter} end RUBY_EVAL @@ -220,7 +218,7 @@ module ActiveSupport # Options support the same options as filters themselves (and support # symbols, string, procs, and objects), so compile a conditional # expression based on the options - def _compile_options(options) + def recompile_options! conditions = ["true"] unless options[:if].empty? @@ -231,7 +229,7 @@ module ActiveSupport conditions << Array(_compile_filter(options[:unless])).map {|f| "!#{f}"} end - conditions.flatten.join(" && ") + @compiled_options = conditions.flatten.join(" && ") end # Filters support: @@ -314,14 +312,14 @@ module ActiveSupport }.merge(config) end - def compile(key=nil, object=nil) + def compile method = [] method << "value = nil" method << "halted = false" callbacks = yielding reverse_each do |callback| - callbacks = callback.apply(callbacks, key, object) + callbacks = callback.apply(callbacks) end method << callbacks @@ -352,14 +350,14 @@ module ActiveSupport module ClassMethods - # This method calls the callback method for the given key. - # If this called first time it creates a new callback method for the key, - # calculating which callbacks can be omitted because of per_key conditions. + # This method runs callback chain for the given kind. + # If this called first time it creates a new callback method for the kind. + # This generated method plays caching role. # - def __run_callbacks(key, kind, object, &blk) #:nodoc: + def __run_callbacks(kind, object, &blk) #:nodoc: name = __callback_runner_name(kind) unless object.respond_to?(name) - str = object.send("_#{kind}_callbacks").compile(key, object) + str = object.send("_#{kind}_callbacks").compile class_eval <<-RUBY_EVAL, __FILE__, __LINE__ + 1 def #{name}() #{str} end protected :#{name} @@ -427,29 +425,6 @@ module ActiveSupport # will be called only when it returns a false value. # * <tt>:prepend</tt> - If true, the callback will be prepended to the existing # chain rather than appended. - # * <tt>:per_key</tt> - A hash with <tt>:if</tt> and <tt>:unless</tt> options; - # see "Per-key conditions" below. - # - # ===== Per-key conditions - # - # When creating or skipping callbacks, you can specify conditions that - # are always the same for a given key. For instance, in Action Pack, - # we convert :only and :except conditions into per-key conditions. - # - # before_filter :authenticate, :except => "index" - # - # becomes - # - # set_callback :process_action, :before, :authenticate, :per_key => {:unless => proc {|c| c.action_name == "index"}} - # - # Per-key conditions are evaluated only once per use of a given key. - # In the case of the above example, you would do: - # - # run_callbacks(:process_action, action_name) { ... dispatch stuff ... } - # - # In that case, each action_name would get its own compiled callback - # method that took into consideration the per_key conditions. This - # is a speed improvement for ActionPack. # def set_callback(name, *filter_list, &block) mapped = nil @@ -484,7 +459,7 @@ module ActiveSupport if filter && options.any? new_filter = filter.clone(chain, self) chain.insert(chain.index(filter), new_filter) - new_filter.recompile!(options, options[:per_key] || {}) + new_filter.recompile!(options) end chain.delete(filter) @@ -528,6 +503,11 @@ module ActiveSupport # other callbacks are not executed. Defaults to "false", meaning no value # halts the chain. # + # * <tt>:skip_after_callbacks_if_terminated</tt> - Determines if after callbacks should be terminated + # by the <tt>:terminator</tt> option. By default after callbacks executed no matter + # if callback chain was terminated or not. + # Option makes sence only when <tt>:terminator</tt> option is specified. + # # * <tt>:rescuable</tt> - By default, after filters are not executed if # the given block or a before filter raises an error. By setting this option # to <tt>true</tt> exception raised by given block is stored and after diff --git a/activesupport/lib/active_support/core_ext/time/calculations.rb b/activesupport/lib/active_support/core_ext/time/calculations.rb index 5e433f5dd9..4f300329f5 100644 --- a/activesupport/lib/active_support/core_ext/time/calculations.rb +++ b/activesupport/lib/active_support/core_ext/time/calculations.rb @@ -273,9 +273,9 @@ class Time beginning_of_day..end_of_day end - # Returns a Range representing the whole week of the current time. - def all_week - beginning_of_week..end_of_week + # Returns a Range representing the whole week of the current time. Week starts on start_day (default is :monday, i.e. end of Sunday). + def all_week(start_day = :monday) + beginning_of_week(start_day)..end_of_week(start_day) end # Returns a Range representing the whole month of the current time. diff --git a/activesupport/test/callback_inheritance_test.rb b/activesupport/test/callback_inheritance_test.rb index b5ad34c204..e5ac9511df 100644 --- a/activesupport/test/callback_inheritance_test.rb +++ b/activesupport/test/callback_inheritance_test.rb @@ -9,8 +9,8 @@ class GrandParent end define_callbacks :dispatch - set_callback :dispatch, :before, :before1, :before2, :per_key => {:if => proc {|c| c.action_name == "index" || c.action_name == "update" }} - set_callback :dispatch, :after, :after1, :after2, :per_key => {:if => proc {|c| c.action_name == "update" || c.action_name == "delete" }} + set_callback :dispatch, :before, :before1, :before2, :if => proc {|c| c.action_name == "index" || c.action_name == "update" } + set_callback :dispatch, :after, :after1, :after2, :if => proc {|c| c.action_name == "update" || c.action_name == "delete" } def before1 @log << "before1" @@ -37,12 +37,12 @@ class GrandParent end class Parent < GrandParent - skip_callback :dispatch, :before, :before2, :per_key => {:unless => proc {|c| c.action_name == "update" }} - skip_callback :dispatch, :after, :after2, :per_key => {:unless => proc {|c| c.action_name == "delete" }} + skip_callback :dispatch, :before, :before2, :unless => proc {|c| c.action_name == "update" } + skip_callback :dispatch, :after, :after2, :unless => proc {|c| c.action_name == "delete" } end class Child < GrandParent - skip_callback :dispatch, :before, :before2, :per_key => {:unless => proc {|c| c.action_name == "update" }}, :if => :state_open? + skip_callback :dispatch, :before, :before2, :unless => proc {|c| c.action_name == "update" }, :if => :state_open? def state_open? @state == :open @@ -112,15 +112,15 @@ class BasicCallbacksTest < ActiveSupport::TestCase @unknown = GrandParent.new("unknown").dispatch end - def test_basic_per_key1 + def test_basic_conditional_callback1 assert_equal %w(before1 before2 index), @index.log end - def test_basic_per_key2 + def test_basic_conditional_callback2 assert_equal %w(before1 before2 update after2 after1), @update.log end - def test_basic_per_key3 + def test_basic_conditional_callback3 assert_equal %w(delete after2 after1), @delete.log end end diff --git a/activesupport/test/callbacks_test.rb b/activesupport/test/callbacks_test.rb index 032787f0f4..3c995e0793 100644 --- a/activesupport/test/callbacks_test.rb +++ b/activesupport/test/callbacks_test.rb @@ -95,7 +95,7 @@ module CallbacksTest define_callbacks :dispatch - set_callback :dispatch, :before, :log, :per_key => {:unless => proc {|c| c.action_name == :index || c.action_name == :show }} + set_callback :dispatch, :before, :log, :unless => proc {|c| c.action_name == :index || c.action_name == :show } set_callback :dispatch, :after, :log2 attr_reader :action_name, :logger @@ -120,7 +120,7 @@ module CallbacksTest end class Child < ParentController - skip_callback :dispatch, :before, :log, :per_key => {:if => proc {|c| c.action_name == :update} } + skip_callback :dispatch, :before, :log, :if => proc {|c| c.action_name == :update} skip_callback :dispatch, :after, :log2 end @@ -131,10 +131,10 @@ module CallbacksTest super end - before_save Proc.new {|r| r.history << [:before_save, :starts_true, :if] }, :per_key => {:if => :starts_true} - before_save Proc.new {|r| r.history << [:before_save, :starts_false, :if] }, :per_key => {:if => :starts_false} - before_save Proc.new {|r| r.history << [:before_save, :starts_true, :unless] }, :per_key => {:unless => :starts_true} - before_save Proc.new {|r| r.history << [:before_save, :starts_false, :unless] }, :per_key => {:unless => :starts_false} + before_save Proc.new {|r| r.history << [:before_save, :starts_true, :if] }, :if => :starts_true + before_save Proc.new {|r| r.history << [:before_save, :starts_false, :if] }, :if => :starts_false + before_save Proc.new {|r| r.history << [:before_save, :starts_true, :unless] }, :unless => :starts_true + before_save Proc.new {|r| r.history << [:before_save, :starts_false, :unless] }, :unless => :starts_false def starts_true if @@starts_true @@ -329,7 +329,7 @@ module CallbacksTest define_callbacks :save attr_reader :stuff - set_callback :save, :before, :action, :per_key => {:if => :yes} + set_callback :save, :before, :action, :if => :yes def yes() true end @@ -700,5 +700,21 @@ module CallbacksTest assert_equal [1, 2, 3], model.recorder end end + + class PerKeyOptionDeprecationTest < ActiveSupport::TestCase + + def test_per_key_option_deprecaton + assert_raise NotImplementedError do + Phone.class_eval do + set_callback :save, :before, :before_save1, :per_key => {:if => "true"} + end + end + assert_raise NotImplementedError do + Phone.class_eval do + skip_callback :save, :before, :before_save1, :per_key => {:if => "true"} + end + end + end + end end diff --git a/activesupport/test/core_ext/integer_ext_test.rb b/activesupport/test/core_ext/integer_ext_test.rb index bfbb2260c6..7597f9c6f5 100644 --- a/activesupport/test/core_ext/integer_ext_test.rb +++ b/activesupport/test/core_ext/integer_ext_test.rb @@ -21,6 +21,5 @@ class IntegerExtTest < ActiveSupport::TestCase # Its results are tested comprehensively in the inflector test cases. assert_equal '1st', 1.ordinalize assert_equal '8th', 8.ordinalize - 1000000000000000000000000000000000000000000000000000000000000000000000.ordinalize end end diff --git a/activesupport/test/core_ext/time_ext_test.rb b/activesupport/test/core_ext/time_ext_test.rb index d45ab70f53..eda8066579 100644 --- a/activesupport/test/core_ext/time_ext_test.rb +++ b/activesupport/test/core_ext/time_ext_test.rb @@ -807,6 +807,7 @@ class TimeExtCalculationsTest < ActiveSupport::TestCase def test_all_week assert_equal Time.local(2011,6,6,0,0,0)..Time.local(2011,6,12,23,59,59,999999.999), Time.local(2011,6,7,10,10,10).all_week + assert_equal Time.local(2011,6,5,0,0,0)..Time.local(2011,6,11,23,59,59,999999.999), Time.local(2011,6,7,10,10,10).all_week(:sunday) end def test_all_month diff --git a/railties/guides/rails_guides/generator.rb b/railties/guides/rails_guides/generator.rb index 49ad8f7769..d6a98f9ac4 100644 --- a/railties/guides/rails_guides/generator.rb +++ b/railties/guides/rails_guides/generator.rb @@ -167,7 +167,7 @@ module RailsGuides def select_only(guides) prefixes = ENV['ONLY'].split(",").map(&:strip) guides.select do |guide| - prefixes.any? {|p| guide.start_with?(p)} + prefixes.any? { |p| guide.start_with?(p) || guide.start_with?("kindle") } end end diff --git a/railties/guides/source/action_view_overview.textile b/railties/guides/source/action_view_overview.textile index ac5c9fa4d8..3b0fbfb464 100644 --- a/railties/guides/source/action_view_overview.textile +++ b/railties/guides/source/action_view_overview.textile @@ -585,6 +585,14 @@ Computes the path to an image asset in the +public/images+ directory. Full paths image_path("edit.png") # => /images/edit.png </ruby> +h5. image_url + +Computes the url to an image asset in the +public/images+ directory. This will call +image_path+ internally and merge with your current host or your asset host. + +<ruby> +image_url("edit.png") # => http://www.example.com/images/edit.png +</ruby> + h5. image_tag Returns an html image tag for the source. The source can be a full path or a file that exists in your +public/images+ directory. @@ -629,6 +637,14 @@ Computes the path to a JavaScript asset in the +public/javascripts+ directory. I javascript_path "common" # => /javascripts/common.js </ruby> +h5. javascript_url + +Computes the url to a JavaScript asset in the +public/javascripts+ directory. This will call +javascript_path+ internally and merge with your current host or your asset host. + +<ruby> +javascript_url "common" # => http://www.example.com/javascripts/common.js +</ruby> + h5. stylesheet_link_tag Returns a stylesheet link tag for the sources specified as arguments. If you don't specify an extension, +.css+ will be appended automatically. @@ -659,6 +675,14 @@ Computes the path to a stylesheet asset in the +public/stylesheets+ directory. I stylesheet_path "application" # => /stylesheets/application.css </ruby> +h5. stylesheet_url + +Computes the url to a stylesheet asset in the +public/stylesheets+ directory. This will call +stylesheet_path+ internally and merge with your current host or your asset host. + +<ruby> +stylesheet_url "application" # => http://www.example.com/stylesheets/application.css +</ruby> + h4. AtomFeedHelper h5. atom_feed diff --git a/railties/guides/source/configuring.textile b/railties/guides/source/configuring.textile index 7e715ff79f..95f93101ab 100644 --- a/railties/guides/source/configuring.textile +++ b/railties/guides/source/configuring.textile @@ -273,6 +273,8 @@ h4. Configuring Active Record * +config.active_record.auto_explain_threshold_in_seconds+ configures the threshold for automatic EXPLAINs (+nil+ disables this feature). Queries exceeding the threshold get their query plan logged. Default is 0.5 in development mode. +* +config.active_record.dependent_restrict_raises+ will control the behavior when an object with a <tt>:dependent => :restrict</tt> association is deleted. Setting this to false will prevent +DeleteRestrictionError+ from being raised and instead will add an error on the model object. Defaults to false in the development mode. + The MySQL adapter adds one additional configuration option: * +ActiveRecord::ConnectionAdapters::MysqlAdapter.emulate_booleans+ controls whether Active Record will consider all +tinyint(1)+ columns in a MySQL database to be booleans and is true by default. diff --git a/railties/guides/source/ruby_on_rails_guides_guidelines.textile b/railties/guides/source/ruby_on_rails_guides_guidelines.textile index 29aefd25f8..f3c8fa654d 100644 --- a/railties/guides/source/ruby_on_rails_guides_guidelines.textile +++ b/railties/guides/source/ruby_on_rails_guides_guidelines.textile @@ -40,7 +40,9 @@ The guides and the API should be coherent where appropriate. Please have a look Those guidelines apply also to guides. -h3. HTML Generation +h3. HTML Guides + +h4. Generation To generate all the guides, just +cd+ into the +railties+ directory and execute: @@ -68,7 +70,7 @@ If you want to generate guides in languages other than English, you can keep the bundle exec rake generate_guides GUIDES_LANGUAGE=es </plain> -h3. HTML Validation +h4. Validation Please validate the generated HTML with: @@ -77,3 +79,13 @@ bundle exec rake validate_guides </plain> Particularly, titles get an ID generated from their content and this often leads to duplicates. Please set +WARNINGS=1+ when generating guides to detect them. The warning messages suggest a way to fix them. + +h3. Kindle Guides + +h4. Generation + +To generate guides for the Kindle, you need to provide +KINDLE=1+ as an environment variable: + +<plain> +KINDLE=1 bundle exec rake generate_guides +</plain> diff --git a/railties/lib/rails/source_annotation_extractor.rb b/railties/lib/rails/source_annotation_extractor.rb index 684beb32a3..9bfc2b16ab 100644 --- a/railties/lib/rails/source_annotation_extractor.rb +++ b/railties/lib/rails/source_annotation_extractor.rb @@ -53,7 +53,7 @@ class SourceAnnotationExtractor # Returns a hash that maps filenames under +dir+ (recursively) to arrays # with their annotations. Only files with annotations are included, and only - # those with extension +.builder+, +.rb+, +.erb+, +.haml+ and +.slim+ + # those with extension +.builder+, +.rb+, +.erb+, +.haml+, +.slim+ and +.coffee+ # are taken into account. def find_in(dir) results = {} @@ -63,7 +63,7 @@ class SourceAnnotationExtractor if File.directory?(item) results.update(find_in(item)) - elsif item =~ /\.(builder|rb)$/ + elsif item =~ /\.(builder|rb|coffee)$/ results.update(extract_annotations_from(item, /#\s*(#{tag}):?\s*(.*)$/)) elsif item =~ /\.erb$/ results.update(extract_annotations_from(item, /<%\s*#\s*(#{tag}):?\s*(.*?)\s*%>/)) diff --git a/railties/test/application/rake/notes_test.rb b/railties/test/application/rake/notes_test.rb index e121d6f1ab..4ab20afc47 100644 --- a/railties/test/application/rake/notes_test.rb +++ b/railties/test/application/rake/notes_test.rb @@ -7,7 +7,7 @@ module ApplicationTests build_app require "rails/all" end - + def teardown teardown_app end @@ -17,6 +17,7 @@ module ApplicationTests app_file "app/views/home/index.html.erb", "<% # TODO: note in erb %>" app_file "app/views/home/index.html.haml", "-# TODO: note in haml" app_file "app/views/home/index.html.slim", "/ TODO: note in slim" + app_file "app/assets/javascripts/application.js.coffee", "# TODO: note in coffee" app_file "app/controllers/application_controller.rb", 1000.times.map { "" }.join("\n") << "# TODO: note in ruby" boot_rails @@ -25,25 +26,26 @@ module ApplicationTests require 'rake/testtask' Rails.application.load_tasks - + Dir.chdir(app_path) do output = `bundle exec rake notes` lines = output.scan(/\[([0-9\s]+)\]/).flatten - + assert_match /note in erb/, output assert_match /note in haml/, output assert_match /note in slim/, output assert_match /note in ruby/, output + assert_match /note in coffee/, output - assert_equal 4, lines.size - assert_equal 4, lines[0].size - assert_equal 4, lines[1].size - assert_equal 4, lines[2].size - assert_equal 4, lines[3].size + assert_equal 5, lines.size + + lines.each do |line_number| + assert_equal 4, line_number.size + end end - + end - + private def boot_rails super |