mbklein/confstruct — pull request #23 by jrochkind (2014-12-08 20:27:18+00:00)
Title: Base Confstruct on Hashie

This is a fairly major 'fork' of Confstruct, to base some functionality on `Mashie` from https://github.com/intridea/hashie gem.

The motivation behind this attempt was running into the weird issues reported for Ruby 2.1 in Issue #21. I reported the issues as a potential bug in ruby 2.1 at https://bugs.ruby-lang.org/issues/10564. Matz says it's not a bug -- but it sure is undesirable behavior in Confstruct. Since Hashie does the things we were trying to do but without using DelegateClass, it does not run into this problem.

It may seem odd to be sub-classing Hashie::Mash and then over-riding the `method_missing` where it does most of it's work -- but it works.

This fork _mostly_ passed all existing tests, with a few notable changes:

* Hashie::Mash normalizes all keys to strings, instead of symbols. So if you're calling `#keys` on a Confstruct::Configuration, or calling `#to_hash` on it and looking at returned hashes, you get strings instead of symbols. This could be a backwards-compat issue in some cases, depending on what calling code is doing. _Potentially_ it's possible to change this to backwards compat behavior, but it'd be fighting with Hashie::Mash, and I'd rather just call it a change in behavior in a next major version.

* I eliminated the special handling for using ActiveSupport hashes to get ordered-key behavior in ruby 1.8. Ruby 1.8 is dead.

* Previously, Confstruct normalized "keys with spaces" to `keys_with_spaces`. I did not replicate that behavior after i discovered via tests that I had broken it, it seems weird and undesirable to me, but if you think it's needed I could try to.

* Previously, Confstruct over-rode #methods to return existing keys in the methods list. No other ruby code I've seen anywhere does this -- making sure `method(:some_method)` and `respond_to? :some_method` works seems to be seen as good enough. So I didn't do this either, figuring if Mashie didn't do it, I'd just leave it to Mashie. I doubt anyone is using that feature, but I could try to add it back in if you think it important.

* Weird behavior from issue #21 is fixed!

* New feature, courtesy of Mashie: If you try to set a key that's used for an existing method (eg `conf.inspect = 'whatever'`), you know get a fail-fast exception raised, rather than weird gotcha behavior down the line. This caught a number of people including me, I think it's good.

What do you think? A fairly major refactor, but that behavior in #21 in Ruby 2.1 is really a problem for me, and this seemed a reasonable solution that is probably 99% if not 100% backwards compat (there are probably other weird edge case differences not caught by existing tests too, but they are going to be weird edge cases if they weren't caught by existing tests!)