Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clash: cannot nest twice even when key is Clash class #240

Closed

Conversation

bartoszkopinski
Copy link
Member

Hello,

In the example found in README:

https://github.com/intridea/hashie#clash

The suggestion to access one of the nested hashes is by using bang methods:

c = Hashie::Clash.new
c.where!.nested(10)._end!
# => {:where=>{:nested=>10}}

However, a second attempt to do the same results in ChainError exception:

c = Hashie::Clash.new
c.where!.nested(10)._end!
c.where!.another(10)._end!
Hashie::Clash::ChainError: Tried to chain into a non-hash key.

Of course this is a naive example, as the issue trying to solve is merge nested elements:

c = Hashie::Clash.new
c.where(location: { near: [1, 2] })
# => {:where=>{:location=>{:near=>[1, 2]}}}
c.where(location: { distance: "100mi" })
# => {:where=>{:location=>{:distance=>"100mi"}}}

There is no way, without using the nested (bang) methods to workaround this limitation.

Looking at the logic of the code:

https://github.com/intridea/hashie/blob/59069f13ea067a319584a56674cc02de887b999f/lib/hashie/clash.rb#L73-L76

Seems that one scenario is missing, which is the case when self[key] is already a Clash instance.

Wanted to confirm if is the desired behavior, there are alternatives or a pull request might be in order 馃榿

Thank you!

@dblock dblock added the bug? label Nov 18, 2014
@dblock
Copy link
Member

dblock commented Nov 18, 2014

Right on @luislavena, you're totally right.

@dblock dblock added confirmed bug and removed bug? labels Nov 18, 2014
Added inheritance

Added multiple bang notation merging

Minor performace improvements - ends_with? is faster than regexp
@dblock
Copy link
Member

dblock commented Dec 8, 2014

This just needs a CHANGELOG entry, please (via --amend), and I'll merge, please?

@luislavena
Copy link
Author

@dblock want me to provide the changelog item?

btw, thanks for working on a fix for this!

@dblock
Copy link
Member

dblock commented Dec 11, 2014

Yes, something I can merge.

@dblock
Copy link
Member

dblock commented Oct 25, 2015

Bump @luislavena

@luislavena
Copy link
Author

@dblock just to be clear: take your change, change the changelog and then send as a pull request?

@dblock
Copy link
Member

dblock commented Oct 26, 2015

@luislavena I just want you to finish this PR! It needs a CHANGELOG entry.

@luislavena
Copy link
Author

Hello @dblock

Really sorry to upset you. I do understand you want to get this change merged, however while I originally reported this issue, wasn't me who contributed the change (or turned the issue into a pull request)

The user that provided the change is @bartoszkopinski and the changes were in his fork.

To be honest is the first time that a reporter of the issue is being asked to provide the CHANGELOG entry description, even when wasn't me who provided the patch.

Anyway, I went ahead and forked, pulled the changes from @bartoszkopinski branch (bk-240-clash-fixes) and amend into this commit:

master...luislavena:bk-240-clash-fixes

Not sure I should be submitting another pull request for the same and also don't want to take credit for the change made by @bartoszkopinski .

Please let me know if I missed something.

Thank you.

@dblock
Copy link
Member

dblock commented Oct 26, 2015

My bad @luislavena I didn't notice that :)

@dblock
Copy link
Member

dblock commented Oct 26, 2015

Just make a PR with that? You can amend on behalf of the original author.

@luislavena
Copy link
Author

@dblock done, see #321, rebased.

@dblock
Copy link
Member

dblock commented Oct 27, 2015

Closed via #321.

@dblock dblock closed this Oct 27, 2015
jsonn pushed a commit to jsonn/pkgsrc that referenced this pull request Oct 15, 2016
[3.4.6]: hashie/hashie@v3.4.5...v3.4.6

### Fixed

* [#368](hashie/hashie#368): Since `hashie/mash` can be required alone, require its dependencies - [@jrafanie](https://github.com/jrafanie).

## [3.4.5] - 2016-09-16

[3.4.5]: hashie/hashie@v3.4.4...v3.4.5

### Added

* [#337](hashie/hashie#337), [#331](hashie/hashie#331): `Hashie::Mash#load` accepts a `Pathname` object - [@gipcompany](https://github.com/gipcompany).

### Deprecated

* [#366](hashie/hashie#366): Hashie is no longer tested on Ruby < 2 - [@dblock](https://github.com/dblock).

### Fixed

* [#358](hashie/hashie#358): Fixed support for Array#dig - [@modosc](https://github.com/modosc).
* [#365](hashie/hashie#365): Ensured ActiveSupport::HashWithIndifferentAccess is defined before use in #deep_locate  - [@mikejarema](https://github.com/mikejarema).

### Miscellanous

* [#366](hashie/hashie#366): Added Danger, PR linter - [@dblock](https://github.com/dblock).

## [3.4.4] - 2016-04-29

[3.4.4]: hashie/hashie@v3.4.3...v3.4.4

### Added

* [#349](hashie/hashie#349): Convert `Hashie::Mash#dig` arguments for Ruby 2.3.0 - [@k0kubun](https://github.com/k0kubun).

### Fixed

* [#240](hashie/hashie#240): Fixed nesting twice with Clash keys - [@bartoszkopinski](https://github.com/bartoszkopinski).
* [#317](hashie/hashie#317): Ensure `Hashie::Extensions::MethodQuery` methods return boolean values - [@michaelherold](https://github.com/michaelherold).
* [#319](hashie/hashie#319): Fix a regression from 3.4.1 where `Hashie::Extensions::DeepFind` is no longer indifference-aware - [@michaelherold](https://github.com/michaelherold).
* [#322](hashie/hashie#322): Fixed `reverse_merge` issue with `Mash` subclasses - [@marshall-lee](https://github.com/marshall-lee).
* [#346](hashie/hashie#346): Fixed `merge` breaking indifferent access - [@docwhat](https://github.com/docwhat), [@michaelherold](https://github.com/michaelherold).
* [#350](hashie/hashie#350): Fixed from string translations used with `IgnoreUndeclared` - [@marshall-lee](https://github.com/marshall-lee).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants