Skip to content

Conversation

@Jack12816
Copy link
Contributor

First things first: thank you for this awesome gem and your work! ❤️

- What is it good for

I accidentally hit funny errors when wrapping a ros object in a new row object like this: RecursiveOpenStruct.new(RecursiveOpenStruct.new({a: true})). Then I digged into this and found different behavior for OpenStruct.

Show more examples
# Reference how OpenStruct works:

» OpenStruct.new({a: true})
# => #<OpenStruct a=true>
» OpenStruct.new(OpenStruct.new({a: true}))
# => #<OpenStruct a=true>
» OpenStruct.new(OpenStruct.new({a: true})).a
# => true
» a = OpenStruct.new({a: true}); b = OpenStruct.new(a); a.object_id == b.object_id
# => false

# Before the patch:

» RecursiveOpenStruct.new({a: true})
# => #<RecursiveOpenStruct a=true>
» RecursiveOpenStruct.new(RecursiveOpenStruct.new({a: true}))
# => #<RecursiveOpenStruct:0x00007f84ebefe120 ...>
» RecursiveOpenStruct.new(RecursiveOpenStruct.new({a: true})).a
# => NoMethodError: undefined method `has_key?' for #<RecursiveOpenStruct a=true> (NoMethodError)
#                   from gems/recursive-open-struct-2.0.0/lib/recursive_open_struct.rb:205
#                   in 'RecursiveOpenStruct#_get_key_from_table_'
» a = RecursiveOpenStruct.new({a: true}); b = RecursiveOpenStruct.new(a); a.object_id == b.object_id
# => false

# After the patch:

» RecursiveOpenStruct.new({a: true})
# => #<RecursiveOpenStruct a=true>
» RecursiveOpenStruct.new(RecursiveOpenStruct.new({a: true}))
# => #<RecursiveOpenStruct a=true>
» RecursiveOpenStruct.new(RecursiveOpenStruct.new({a: true})).a
# => true
» a = RecursiveOpenStruct.new({a: true}); b = RecursiveOpenStruct.new(a); a.object_id == b.object_id
# => false

- What I did

I just added a safety guard on the class constructor to unpack ros/os (or children of them) objects first. This leads to deep copies.

- A picture of a cute animal (not mandatory but encouraged)
image

@aetherknight
Copy link
Owner

Tests are failing due to changes out side of this pull request. jruby-head is failing, I think because I'm pinning to an older version of the setup-ruby action. I'll get a fix for that added soon.

@aetherknight
Copy link
Owner

@Jack12816 when you get a chance, can you merge or rebase to my current main branch? It contains fixes for the CI test runs.

@Jack12816
Copy link
Contributor Author

Will do in a couple hours :)

Signed-off-by: Hermann Mayer <hermann.mayer92@gmail.com>
@Jack12816
Copy link
Contributor Author

@aetherknight I rebased against upstream main.

@Jack12816
Copy link
Contributor Author

I'll fix the broken specs tomorrow. I didn't tested the older rubies locally - sorry.

Signed-off-by: Hermann Mayer <hermann.mayer92@gmail.com>
@Jack12816
Copy link
Contributor Author

@aetherknight I fixed the specs and tested the fix locally against Ruby 3.2, 3.3, and 3.4.

@aetherknight
Copy link
Owner

Looks good. I'll get it merged now, and will make a release soon

@aetherknight aetherknight merged commit 5cb58e5 into aetherknight:main Dec 5, 2025
6 checks passed
@Jack12816
Copy link
Contributor Author

Awesome - thank you! 🎉

@aetherknight
Copy link
Owner

It should be live now, as v2.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants