Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

remove useless rescue #2647

Merged
merged 1 commit into from

3 participants

@dmathieu
Collaborator

params is a method, defined in every controller, which always returns a hash.

If it raises a NoMethodError, it means there's a bug somewhere else, which we want to know about.

actionpack/lib/sprockets/helpers/rails_helper.rb
@@ -70,12 +70,7 @@ module Sprockets
private
def debug_assets?
- begin
- params[:debug_assets] == '1' ||
- params[:debug_assets] == 'true'
- rescue NoMethodError
- false
- end || Rails.application.config.assets.debug
+ params[:debug_assets] == '1' || params[:debug_assets] == 'true' || Rails.application.config.assets.debug
@dasch
dasch added a note

Can you split this line up? Or perhaps use a more concise version, e.g.:

%w(1 true).include?(params[:debug_assets]) || Rails.application.config.assets.debug
@dmathieu Collaborator

Yup, that's something I can do. I don't like the %w format though (that's a very personal opinion) though.

@dasch
dasch added a note

['1', 'true'] is still an improvement. I'm on a crusade against long lines in Rails these days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@spastorino
Owner

I prefer the first version is more readable and faster you can split the lines anyway ...

params[:debug_assets] == '1' ||
params[:debug_assets] == 'true' ||
Rails.application.config.assets.debug
@dmathieu dmathieu remove useless rescue
params is a method, defined in every controller, which always returns a hash.
If it raises a NoMethodError, it means there's a bug somewhere else, which we want to know about.
13dd775
@dmathieu
Collaborator

I've changed it. Thank you for your opinion :)

@spastorino spastorino merged commit f162f84 into rails:master
@spastorino spastorino referenced this pull request from a commit
@spastorino spastorino Revert "Merge pull request #2647 from dmathieu/no_rescue"
This reverts commit 125b1b0.
69c7213
@spastorino spastorino referenced this pull request from a commit
@spastorino spastorino Revert "Merge pull request #2647 from dmathieu/no_rescue"
This reverts commit 125b1b0.
8bc6fc5
@tenderlove tenderlove referenced this pull request from a commit
@tenderlove tenderlove Merge branch '3-1-0' into 3-1-stable
* 3-1-0:
  bumping to 3.1.0
  Bump sprockets up
  Depend on sass-rails and coffee-rails 3.1.0
  Revert "Merge pull request #2647 from dmathieu/no_rescue"
  Merge pull request #2756 from guilleiguaran/manifest-location
  Merge pull request #2748 from guilleiguaran/assets-version-config
  adds the asset pipeline guide to the index
  incorporate feedback from vijaydev and dasch to rephrase this to sound more natural, and some grammar fixes.
7cdc9fd
@ttosch ttosch referenced this pull request from a commit
@spastorino spastorino Revert "Merge pull request #2647 from dmathieu/no_rescue"
This reverts commit 125b1b0.
2905886
@ttosch ttosch referenced this pull request from a commit
@spastorino spastorino Revert "Merge pull request #2647 from dmathieu/no_rescue"
This reverts commit 125b1b0.
7884cbc
@ttosch ttosch referenced this pull request from a commit
@tenderlove tenderlove Merge branch '3-1-0' into 3-1-stable
* 3-1-0:
  bumping to 3.1.0
  Bump sprockets up
  Depend on sass-rails and coffee-rails 3.1.0
  Revert "Merge pull request #2647 from dmathieu/no_rescue"
  Merge pull request #2756 from guilleiguaran/manifest-location
  Merge pull request #2748 from guilleiguaran/assets-version-config
  adds the asset pipeline guide to the index
  incorporate feedback from vijaydev and dasch to rephrase this to sound more natural, and some grammar fixes.
e568dce
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Aug 24, 2011
  1. @dmathieu

    remove useless rescue

    dmathieu authored
    params is a method, defined in every controller, which always returns a hash.
    If it raises a NoMethodError, it means there's a bug somewhere else, which we want to know about.
This page is out of date. Refresh to see the latest.
View
9 actionpack/lib/sprockets/helpers/rails_helper.rb
@@ -70,12 +70,9 @@ def asset_path(source, default_ext = nil, body = false, protocol = nil)
private
def debug_assets?
- begin
- params[:debug_assets] == '1' ||
- params[:debug_assets] == 'true'
- rescue NoMethodError
- false
- end || Rails.application.config.assets.debug
+ params[:debug_assets] == '1' ||
+ params[:debug_assets] == 'true' ||
+ Rails.application.config.assets.debug
end
# Override to specify an alternative prefix for asset path generation.
View
6 actionpack/test/abstract_unit.rb
@@ -142,7 +142,11 @@ def call(env)
end
class BasicController
- attr_accessor :request
+ attr_accessor :request, :params
+
+ def initialize
+ @params = {}
+ end
def config
@config ||= ActiveSupport::InheritableOptions.new(ActionController::Base.config).tap do |config|
Something went wrong with that request. Please try again.