From b47970294e53ed9142ed16a560a8f68b1ae1b5d0 Mon Sep 17 00:00:00 2001 From: schneems Date: Sun, 21 Aug 2016 14:08:24 -0500 Subject: [PATCH] Favor `public_folder: true` over `public_*` Adding all those `public_*` methods is a bit heavy handed, we can change the API to instead use `public_folder: true`. Change was pretty easy since it was already implemented that way. --- .../action_view/helpers/asset_tag_helper.rb | 35 +----- .../action_view/helpers/asset_url_helper.rb | 112 ------------------ .../test/application/asset_debugging_test.rb | 69 +++++++---- 3 files changed, 47 insertions(+), 169 deletions(-) diff --git a/actionview/lib/action_view/helpers/asset_tag_helper.rb b/actionview/lib/action_view/helpers/asset_tag_helper.rb index 2e31bfbc9a..4ff2b8c872 100644 --- a/actionview/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionview/lib/action_view/helpers/asset_tag_helper.rb @@ -193,13 +193,6 @@ def favicon_link_tag(source="favicon.ico", options={}) }.merge!(options.symbolize_keys)) end - # Returns a link tag for a favicon asset in the public - # folder. This uses +favicon_link_tag+ and skips any asset - # lookups by assuming any assets are in the `public` folder. - def public_favicon_link_tag(source="favicon.ico", options={}) - favicon_link_tag(source, { public_folder: true }.merge!(options)) - end - # Returns an HTML image tag for the +source+. The +source+ can be a full # path or a file. # @@ -244,13 +237,6 @@ def image_tag(source, options={}) tag("img", options) end - # Returns an HTML image tag for the source asset in the public - # folder. This uses +image_tag+ and skips any asset - # lookups by assuming any assets are in the `public` folder. - def public_image_tag(source, options={}) - image_tag(source, { public_folder: true }.merge!(options)) - end - # Returns a string suitable for an HTML image tag alt attribute. # The +src+ argument is meant to be an image file path. # The method removes the basename of the file path and the digest, @@ -314,20 +300,12 @@ def video_tag(*sources) options = sources.extract_options!.symbolize_keys public_poster_folder = options.delete(:public_poster_folder) sources << options - multiple_sources_tag("video", sources) do |options| + multiple_sources_tag_builder("video", sources) do |options| options[:poster] = path_to_image(options[:poster], public_folder: public_poster_folder) if options[:poster] options[:width], options[:height] = extract_dimensions(options.delete(:size)) if options[:size] end end - # Returns an HTML video tag for the source asset in the public - # folder. This uses +video_tag+ and skips any asset - # lookups by assuming any assets are in the `public` folder. - def public_video_tag(*sources) - options = sources.extract_options! - video_tag(*sources, { public_folder: true , public_poster_folder: true }.merge!(options)) - end - # Returns an HTML audio tag for the +source+. # The +source+ can be full path or file that exists in # your public audios directory. @@ -341,18 +319,11 @@ def public_video_tag(*sources) # audio_tag("sound.wav", "sound.mid") # # => def audio_tag(*sources) - multiple_sources_tag("audio", sources) - end - - # Returns an HTML audio tag for the source asset in the public - # folder. This uses +audio_tag+ and skips any asset - def public_audio_tag(*sources) - options = sources.extract_options! - audio_tag(*sources, { public_folder: true }.merge!(options)) + multiple_sources_tag_builder("audio", sources) end private - def multiple_sources_tag(type, sources) + def multiple_sources_tag_builder(type, sources) options = sources.extract_options!.symbolize_keys public_folder = options.delete(:public_folder) sources.flatten! diff --git a/actionview/lib/action_view/helpers/asset_url_helper.rb b/actionview/lib/action_view/helpers/asset_url_helper.rb index a21536dd9d..50b13c0271 100644 --- a/actionview/lib/action_view/helpers/asset_url_helper.rb +++ b/actionview/lib/action_view/helpers/asset_url_helper.rb @@ -167,14 +167,6 @@ def asset_path(source, options = {}) end alias_method :path_to_asset, :asset_path # aliased to avoid conflicts with an asset_path named route - # Computes the path to an asset in the public folder. - # This uses +asset_path+ and skips any asset lookups by assuming the asset is in the - # `public` folder. - def public_asset_path(source, options = {}) - path_to_asset(source, { public_folder: true }.merge!(options)) - end - alias_method :path_to_public_asset, :public_asset_path # aliased to avoid conflicts with an public_asset_path named route - # Computes the full URL to an asset in the public directory. This # will use +asset_path+ internally, so most of their behaviors # will be the same. If :host options is set, it overwrites global @@ -190,14 +182,6 @@ def asset_url(source, options = {}) end alias_method :url_to_asset, :asset_url # aliased to avoid conflicts with an asset_url named route - # Computes the full URL to an asset in the public folder. - # This uses +asset_url+ and skips any asset lookups by assuming the asset is in the - # `public` folder. - def public_asset_url(source, options = {}) - url_to_asset(source, { public_folder: true }.merge!(options)) - end - alias_method :path_to_public_asset, :public_asset_path # aliased to avoid conflicts with a public_asset_path named route - ASSET_EXTENSIONS = { javascript: ".js", stylesheet: ".css" @@ -284,14 +268,6 @@ def javascript_path(source, options = {}) end alias_method :path_to_javascript, :javascript_path # aliased to avoid conflicts with a javascript_path named route - # Computes the path to a javascript asset in the public folder. - # This uses +javascript_path+ and skips any asset lookups by assuming the asset is in the - # `public` folder. - def public_javascript_path(source, options = {}) - path_to_javascript(source, { public_folder: true }.merge!(options)) - end - alias_method :path_to_public_javascript, :public_javascript_path # aliased to avoid conflicts with a public_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. # Since +javascript_url+ is based on +asset_url+ method you can set :host options. If :host @@ -304,14 +280,6 @@ def javascript_url(source, options = {}) end alias_method :url_to_javascript, :javascript_url # aliased to avoid conflicts with a javascript_url named route - # Computes the full URL to a javascript asset in the public folder. - # This uses +javascript_url+ and skips any asset lookups by assuming the asset is in the - # `public` folder. - def public_javascript_url(source, options = {}) - url_to_javascript(source, { public_folder: true }.merge!(options)) - end - alias_method :path_to_public_javascript, :public_javascript_path # aliased to avoid conflicts with a public_javascript_path named route - # Computes the path to a stylesheet asset in the public stylesheets directory. # If the +source+ filename has no extension, .css will be appended (except for explicit URIs). # Full paths from the document root will be passed through. @@ -327,14 +295,6 @@ def stylesheet_path(source, options = {}) end alias_method :path_to_stylesheet, :stylesheet_path # aliased to avoid conflicts with a stylesheet_path named route - # Computes the path to a stylesheet asset in the public folder. - # This uses +stylesheet_path+ and skips any asset lookups by assuming the asset is in the - # `public` folder. - def public_stylesheet_path(source, options = {}) - path_to_stylesheet(source, { public_folder: true }.merge!(options)) - end - alias_method :path_to_public_stylesheet, :public_stylesheet_path # aliased to avoid conflicts with a public_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. # Since +stylesheet_url+ is based on +asset_url+ method you can set :host options. If :host @@ -347,14 +307,6 @@ def stylesheet_url(source, options = {}) end alias_method :url_to_stylesheet, :stylesheet_url # aliased to avoid conflicts with a stylesheet_url named route - # Computes the full URL to a stylesheet asset in the public folder. - # This uses +stylesheet_url+ and skips any asset lookups by assuming the asset is in the - # `public` folder. - def public_stylesheet_url(source, options = {}) - url_to_stylesheet(source, { public_folder: true }.merge!(options)) - end - alias_method :path_to_public_stylesheet, :public_stylesheet_path # aliased to avoid conflicts with a public_stylesheet_path named route - # Computes the path to an image asset. # Full paths from the document root will be passed through. # Used internally by +image_tag+ to build the image path: @@ -373,14 +325,6 @@ def image_path(source, options = {}) end alias_method :path_to_image, :image_path # aliased to avoid conflicts with an image_path named route - # Computes the path to a image asset in the public folder. - # This uses +image_path+ and skips any asset lookups by assuming the asset is in the - # `public` folder. - def public_image_path(source, options = {}) - path_to_image(source, { public_folder: true }.merge!(options)) - end - alias_method :path_to_public_image, :public_image_path # aliased to avoid conflicts with a public_image_path named route - # Computes the full URL to an image asset. # This will use +image_path+ internally, so most of their behaviors will be the same. # Since +image_url+ is based on +asset_url+ method you can set :host options. If :host @@ -393,14 +337,6 @@ def image_url(source, options = {}) end alias_method :url_to_image, :image_url # aliased to avoid conflicts with an image_url named route - # Computes the full URL to a image asset in the public folder. - # This uses +image_url+ and skips any asset lookups by assuming the asset is in the - # `public` folder. - def public_image_url(source, options = {}) - url_to_image(source, { public_folder: true }.merge!(options)) - end - alias_method :path_to_public_image, :public_image_path # aliased to avoid conflicts with a public_image_path 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. @@ -415,14 +351,6 @@ def video_path(source, options = {}) end alias_method :path_to_video, :video_path # aliased to avoid conflicts with a video_path named route - # Computes the path to a video asset in the public folder. - # This uses +video_path+ and skips any asset lookups by assuming the asset is in the - # `public` folder. - def public_video_path(source, options = {}) - path_to_video(source, { public_folder: true }.merge!(options)) - end - alias_method :path_to_public_video, :public_video_path # aliased to avoid conflicts with a public_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. # Since +video_url+ is based on +asset_url+ method you can set :host options. If :host @@ -435,14 +363,6 @@ def video_url(source, options = {}) end alias_method :url_to_video, :video_url # aliased to avoid conflicts with an video_url named route - # Computes the full URL to a video asset in the public folder. - # This uses +video_url+ and skips any asset lookups by assuming the asset is in the - # `public` folder. - def public_video_url(source, options = {}) - url_to_video(source, { public_folder: true }.merge!(options)) - end - alias_method :path_to_public_video, :public_video_path # aliased to avoid conflicts with a public_video_path 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. @@ -457,14 +377,6 @@ def audio_path(source, options = {}) end alias_method :path_to_audio, :audio_path # aliased to avoid conflicts with an audio_path named route - # Computes the path to a audio asset in the public folder. - # This uses +audio_path+ and skips any asset lookups by assuming the asset is in the - # `public` folder. - def public_audio_path(source, options = {}) - path_to_audio(source, { public_folder: true }.merge!(options)) - end - alias_method :path_to_public_audio, :public_audio_path # aliased to avoid conflicts with a public_audio_path named route - # Computes the full URL to an audio asset in the public audios directory. # This will use +audio_path+ internally, so most of their behaviors will be the same. # Since +audio_url+ is based on +asset_url+ method you can set :host options. If :host @@ -477,14 +389,6 @@ def audio_url(source, options = {}) end alias_method :url_to_audio, :audio_url # aliased to avoid conflicts with an audio_url named route - # Computes the full URL to a audio asset in the public folder. - # This uses +audio_url+ and skips any asset lookups by assuming the asset is in the - # `public` folder. - def public_audio_url(source, options = {}) - url_to_audio(source, { public_folder: true }.merge!(options)) - end - alias_method :path_to_public_audio, :public_audio_path # aliased to avoid conflicts with a public_audio_path named route - # Computes the path to a font asset. # Full paths from the document root will be passed through. # @@ -498,14 +402,6 @@ def font_path(source, options = {}) end alias_method :path_to_font, :font_path # aliased to avoid conflicts with an font_path named route - # Computes the path to a font asset in the public folder. - # This uses +font_path+ and skips any asset lookups by assuming the asset is in the - # `public` folder. - def public_font_path(source, options = {}) - path_to_font(source, { public_folder: true }.merge!(options)) - end - alias_method :path_to_public_font, :public_font_path # aliased to avoid conflicts with a public_font_path named route - # Computes the full URL to a font asset. # This will use +font_path+ internally, so most of their behaviors will be the same. # Since +font_url+ is based on +asset_url+ method you can set :host options. If :host @@ -517,14 +413,6 @@ def font_url(source, options = {}) url_to_asset(source, { type: :font }.merge!(options)) end alias_method :url_to_font, :font_url # aliased to avoid conflicts with an font_url named route - - # Computes the full URL to a font asset in the public folder. - # This uses +font_url+ and skips any asset lookups by assuming the asset is in the - # `public` folder. - def public_font_url(source, options = {}) - url_to_font(source, { public_folder: true }.merge!(options)) - end - alias_method :path_to_public_font, :public_font_path # aliased to avoid conflicts with a public_font_url named route end end end diff --git a/railties/test/application/asset_debugging_test.rb b/railties/test/application/asset_debugging_test.rb index 1d27cecd3c..e6a660cf25 100644 --- a/railties/test/application/asset_debugging_test.rb +++ b/railties/test/application/asset_debugging_test.rb @@ -72,23 +72,23 @@ class ::PostsController < ActionController::Base ; end test "public path and tag methods are not over-written by the asset pipeline" do contents = "doesnotexist" cases = { - public_asset_path: %r{/#{ contents }}, - public_image_path: %r{/images/#{ contents }}, - public_video_path: %r{/videos/#{ contents }}, - public_audio_path: %r{/audios/#{ contents }}, - public_font_path: %r{/fonts/#{ contents }}, - public_javascript_path: %r{/javascripts/#{ contents }}, - public_stylesheet_path: %r{/stylesheets/#{ contents }}, - public_image_tag: %r{}, - public_stylesheet_link_tag: %r{}, - public_javascript_include_tag: %r{