ActiveSupport OrderedHash Doesn't Support Passing Block to merge

rubyonrails-core
1

Hi all,

On versions of Ruby prior to 1.9 (where Hashes' keys have no set order), ActiveSupport::OrderedHash implements its own merge and merge! methods. These implementations differ to the standard library however in that they do not support passing a block to merge (c.f. Class: Hash (Ruby 3.1.2) for the standard method signature).

I have posted a patch including tests at https://rails.lighthouseapp.com/projects/8994/tickets/4838-patch-add-block-support-to-activesupport-orderedhash-merge to correct this missing functionality. The implementation also closely matches that of Rubinius' (c.f. http://github.com/evanphx/rubinius/blob/master/kernel/common/hash.rb#L429-447 )

Any feedback would be greatly appreciated.

2

Excellent, applied

    http://github.com/rails/rails/compare/a087bc8...9183eae

3

Just realized the patch is not compatible with 1.9, since in 1.9 only existing keys are merged (that's the key? key test in Rubinius). Will revise it myself.

4

Rather, yield is called only for existing keys. This revisions seems to be correct:

http://github.com/rails/rails/commit/36143d26cb841210b5f22aff4ed9c093a0554a1a

5

I've done a tiny improvement to this implementation. Please Xavier review it ;). http://github.com/spastorino/rails/commit/63b43575765c7d4042c23ac3ddb1213c1968b5a3

6

Good, it is up http://github.com/rails/rails/commit/6d19a4a664914e908e75cfe90a0507cc9f53d1cd

Thanks!