Fix rescue_from subclasses default #651
Conversation
| @@ -214,7 +214,13 @@ def rescue_from(*args, &block) | |||
| options = args.last.is_a?(Hash) ? args.pop : {} | |||
| handler ||= proc { options[:with] } if options.key?(:with) | |||
|
|
|||
| handler_type = !!options[:rescue_subclasses] ? :rescue_handlers : :base_only_rescue_handlers | |||
| handler_type = | |||
There was a problem hiding this comment.
@dblock The !! trick works when you want nil, false but since we want nil, true I ended up with a case statement. Do you know of a prettier more idiomatic way of doing this?
There was a problem hiding this comment.
You could just swap it around (nil != false):
handler_type = options[:rescue_subclasses] == false ? :base_only_rescue_handlers : :rescue_handlers
There was a problem hiding this comment.
@adrianmacneil Yeah that would do the trick, thanks for the suggestion. In this case I'll stick with the case as it reads a bit clearer.
|
This really needs a CHANGELOG entry, please. |
|
@dblock Separated the handler that rescues from all so it's less obscure. Added CHANGELOG. |
|
Merged via f55e55f. I also added an integration test at API level. |
|
This is good work, thanks much. |
|
@dblock Thanks for CC'ing me, adding the integration test and the quick merge. |
This fixes
rescue_fromto properly setrescue_subclassestotrueby default, as per the documentation. This was brought up in #649Because it was defaulting to false, all other unit tests that involved rescuing (such as
rescue_from :alland rescuing from a given block) were passing, but when the default was properly set to true, some of them broke. In other words, it exposed a bug in a combination ofrescue_subclassesset totruein combination withrescue_from :alland rescuing using a given block as a handler. This has been fixed as well.Added a unit test to ensure that by default, subclasses are rescued from (i.e., code behaves as if
rescue_subclassesistrue).