This project is archived and is in readonly mode.
Nested with_options merge hash values
Consider this:
map.with_options :conditions => { :subdomain => /^www$/ } do |m|
m.with_options :conditions => { :method => :get } do |home|
home.main_home '/'
- etc.
end
- etc.
end
I expected that hitting http://foo.mydomain.com would not match home.main_home, but it does. This is because the with_options method overwrites the value for :conditions, losing the condition for subdomain.
Attached is a patch including tests that will merge hash values within options.
In the above example this would effectively result in:
map.main_home "/", :conditions => { :subdomain => /^www$/, :method => :get }
-
with_options.diff
2.98 KB
-
Lawrence Pit- Tag changed from activesupport to activesupport, core_ext, patch, tests
Again, hopefully nicely formatted this time:
map.with_options :conditions => { :subdomain => /^www$/ } do |m| m.with_options :conditions => { :method => :get } do |home| home.root '/' end endshould equal:
map.root '/', :conditions => { :subdomain => /^www$/, :method => :get } -
Pratik- Assigned user set to Pratik
Hey Lawrence,
Looks nice. I wonder if we should rather add Hash#deep_merge method and simply use it here. Thoughts ?
-
Lawrence Pit
Attached version with deep_merge.
The code of deep_merge is a bit ugly though... You need to test for :to_hash for it to work with with_options, because OrderedHash is an Array, not a Hash. Nicer would of course be if you only needed to test for is_a?(Hash).
Secondly, calling is_a?(Hash) doesn't work within deep_merge. hash/conversions.rb shows the same issue, it comments:
- something weird with classes not matching here. maybe singleton methods breaking is_a?
hence the use of: val.class.to_s == 'Hash'.
-
with_options_deep_merge.diff
6.1 KB
-
-
Lawrence Pit
just thinking, maybe we should add this method to OrderedHash? :
def is_a?(o) o == Hash endLast patch implements this. Simplifies deep_merge.
-
Repository- State changed from new to resolved
(from [40dbebba28bfa1c55737da7354542c3bdca4e1a1]) Allow deep merging of hash values for nested with_options. [#490 state:resolved]
Signed-off-by: Pratik Naik
-
Pratik
- State changed from resolved to new
Intentionally committed version without OrderedHash#is_a? as OrderedHash doesn't really behave like a hash ( lots of missing features like merge/update/etc. )
-
Pratik
- State changed from new to resolved