Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix OrderedHash merging with block given. #1740

Merged
merged 2 commits into from

5 participants

@Antiarchitect

Code taken from Rails 3 branch. Now OrderedHash merging can accept a block.

@gmile

Any chances to see tests for this?

@Antiarchitect

Delivered!

@vijaydev
Collaborator

I think 2-3-stable is frozen except for security fixes.

@Antiarchitect

I've made this fix for the sake of justice and beauty. Yesterday I've spent about 3 hours to understand why my ordered hashes merging doesn't work with block given. So there is ugly hack in my app now.

@dmathieu
Collaborator

There won't be any 2.3 release unless it's for security.
So even if your patch were to be merged, you won't see it in action.

You should either monkey patch OrderedHash to fix the problem, or, even better, upgrade to 3.x

@Antiarchitect

Yes I know. App is working fine with my hack now. I hope it can save some other's time.

@josevalim josevalim merged commit 8d02083 into rails:2-3-stable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 16, 2011
  1. @Antiarchitect
Commits on Jun 17, 2011
  1. @Antiarchitect
This page is out of date. Refresh to see the latest.
View
10 activesupport/lib/active_support/ordered_hash.rb
@@ -130,14 +130,18 @@ def shift
end
def merge!(other_hash)
- other_hash.each {|k,v| self[k] = v }
+ if block_given?
+ other_hash.each { |k, v| self[k] = key?(k) ? yield(k, self[k], v) : v }
+ else
+ other_hash.each { |k, v| self[k] = v }
+ end
self
end
alias_method :update, :merge!
- def merge(other_hash)
- dup.merge!(other_hash)
+ def merge(other_hash, &block)
+ dup.merge!(other_hash, &block)
end
# When replacing with another hash, the initial order of our keys must come from the other hash -ordered or not.
View
26 activesupport/test/ordered_hash_test.rb
@@ -147,6 +147,32 @@ def test_merge
assert_equal @ordered_hash.keys, merged.keys
end
+ def test_merge_with_block
+ hash = ActiveSupport::OrderedHash.new
+ hash[:a] = 0
+ hash[:b] = 0
+ merged = hash.merge(:b => 2, :c => 7) do |key, old_value, new_value|
+ new_value + 1
+ end
+
+ assert_equal 0, merged[:a]
+ assert_equal 3, merged[:b]
+ assert_equal 7, merged[:c]
+ end
+
+ def test_merge_bang_with_block
+ hash = ActiveSupport::OrderedHash.new
+ hash[:a] = 0
+ hash[:b] = 0
+ hash.merge!(:a => 1, :c => 7) do |key, old_value, new_value|
+ new_value + 3
+ end
+
+ assert_equal 4, hash[:a]
+ assert_equal 0, hash[:b]
+ assert_equal 7, hash[:c]
+ end
+
def test_shift
pair = @ordered_hash.shift
assert_equal [@keys.first, @values.first], pair
Something went wrong with that request. Please try again.