TwP / inifile

Native Ruby package for reading and writing INI files
http://codeforpeople.rubyforge.org/inifile
95 stars 47 forks source link

Merge feature #6

Closed hilli closed 13 years ago

hilli commented 13 years ago

Hello

I needed a merge feature, so I could load one (system wide) inifile and let users have their own too (to be able to override settings) - But still just have one inifile object in my code. So I made a merge feature.

Hope you like it.

Best regards /Hilli

TwP commented 13 years ago

Just a short update - I will take a look at this over the weekend. However, it should get pulled in for the next release since it sounds like a perfectly reasonable feature to add. Just want to give you heads up on the process and let you know that your pull request has not fallen into a black hole.

Blessings, TwP

hilli commented 13 years ago

Mjello

On Mon, Mar 21, 2011 at 08:14:10AM -0700, TwP wrote:

Just a short update - I will take a look at this over the weekend. However, it should get pulled in for the next release since it sounds like a perfectly reasonable feature to add. Just want to give you heads up on the process and let you know that your pull request has not fallen into a black hole.

Sounds great - Thanks for the status update.

/Hilli

TwP commented 13 years ago

Looking through the code, the merge method modifies the other_inifile object during the merge process. I understand why this is being done (to pick up new sections from the other inifile). However, the merge method should not change the other_inifile object.

If you grab the section keys from each inifile, you can use the intersection and difference operators on the key arrays to find common sections and new sections:

my_keys = @ini.keys
other_keys = other_inifile.instance_variable_get(:@ini).keys

(my_keys & other_keys).each do |key|
  @ini[key].merge!(other_inifile[key])
end
(other_keys - my_keys).each do |key|
  @ini[key] = other_inifile[key]
end

The "&" operator on a ruby array performs set intersection and returns the keys common to both arrays.

Also, a quick check on the type of object being passed in to the merge method would be advisable. Just in case someone passes in a hash instead of an Inifile instance.

Those are my two suggestions for the pull request. Thanks for contributing to the project, and I'm glad your finding use for inifile!

Blessings, TwP