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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is ruby2_keywords also required here? #56

Closed
jochenseeber opened this issue Dec 23, 2020 · 5 comments
Closed

Is ruby2_keywords also required here? #56

jochenseeber opened this issue Dec 23, 2020 · 5 comments

Comments

@jochenseeber
Copy link
Contributor

jochenseeber commented Dec 23, 2020

#52 fixed keyword argument delegation (learned something new, thanks!)

Do we also need this here (cc @eregon)?

send(:define_method, :method_missing) do |method, *args, &block|

@eregon
Copy link
Contributor

eregon commented Dec 24, 2020

Yes, it's also needed, if the method called by receiver.__send__(method.to_sym, *args, &block) has keyword arguments.

It's a good idea to review all usages of rest arguments (e.g., *args) that are used for delegation, and might potentially call a method which has keyword arguments. In that case, ruby2_keywords should be used to correctly pass keyword arguments as-is, and not transforming them back to a positional Hash argument.

@ms-ati
Copy link
Owner

ms-ati commented Jan 4, 2021

cc @jochenseeber @eregon and others, if you'd consider proposing a PR to address this It would be greatly appreciated and welcome! :)

@jochenseeber
Copy link
Contributor Author

I can try and create a PR, but it will be 2 weeks before I have time.

@ms-ati
Copy link
Owner

ms-ati commented Jan 11, 2021

@jochenseeber is this closed by #62 ?

@ms-ati
Copy link
Owner

ms-ati commented Jan 13, 2021

@jochenseeber @eregon I'm going to mark this closed by #62 -- please do feel free to re-open this issue if I've misunderstood and it's not fixed.

Also, I've just released v1.3.5 to Rubygems with these changes!

@ms-ati ms-ati closed this as completed Jan 13, 2021
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

No branches or pull requests

3 participants