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

Added class methods support #4

Merged
merged 2 commits into from
Apr 4, 2024
Merged

Added class methods support #4

merged 2 commits into from
Apr 4, 2024

Conversation

hdf1996
Copy link
Contributor

@hdf1996 hdf1996 commented Apr 2, 2024

Summary

Solves #3 . it attempts to refactor the sig method to cover for class methods along with adding specs for it. Another early approach was to use a separate class_sig method, but I thought that could end up being too confusing

Also addresses some errors I found when check the gemspec while installing local dependencies, but let me know if that's not needed for whatever reason

send(:"__#{name}", *args, **kwargs)
end
meta.send :define_method, :singleton_method_added do |name|
next if name == :singleton_method_added
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't found this documented anywhere in ruby, but looks like defining singleton_method_added also calls itself. Tried isolating this on a simple example just in case define_method or send where causing it somehow, but it also happened on isolation

Comment on lines 28 to +42
meta.send :define_method, :method_added do |name|
meta.send :remove_method, :method_added
meta.send :remove_method, :singleton_method_added

alias_method :"__#{name}", name
define_method name do |*args, **kwargs, &block|
sig_args.each.with_index do |arg, i|
args[i] => ^arg
end

kwargs.each do |key, value|
value => ^(sig_kwargs[key])
end
sig_check.call(self, name, *args, **kwargs, &block)
end
end

result = if block
send(:"__#{name}", *args, **kwargs, &block)
else
send(:"__#{name}", *args, **kwargs)
end
meta.send :define_method, :singleton_method_added do |name|
next if name == :singleton_method_added

result => ^returns if returns != NULL
meta.send :remove_method, :singleton_method_added
meta.send :remove_method, :method_added
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, we wait for either a singleton method or an instance method, whatever happens first will remove both method_added callbacks

end

sig String, returns: Array
def find_by_name(name)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo? should be a class method I think.

@joelmoss
Copy link
Owner

joelmoss commented Apr 3, 2024

Other than the typo and a few rubocop failures, this looks great! If you can correct those, I can merge this.

Thanks so much.

@hdf1996
Copy link
Contributor Author

hdf1996 commented Apr 3, 2024

Cool! Just saw those rubocop offences while fixing the typo. I'll take a look and push an update here soon, Thanks!

@hdf1996 hdf1996 requested a review from joelmoss April 3, 2024 20:41
@joelmoss joelmoss merged commit db740ed into joelmoss:master Apr 4, 2024
2 checks passed
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