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

Unroll extension method generation #882

Merged
merged 2 commits into from
Feb 23, 2024

Conversation

st0012
Copy link
Member

@st0012 st0012 commented Feb 23, 2024

Given we only have 2 remaining extension setter methods, both of which only take 1 argument and don't have any alias, the current method generation logic is overly complicated.

This PR simplifies the method generation logic by simply defining the methods directly in the IRB::Context class.

I also found that use_loader is broken and fixed it in a separate commit.

Given we only have 2 remaining extension setter methods, both of which
only take 1 argument and don't have any alias, the current method generation
logic is overly complicated.

This commit simplifies the method generation logic by simply defining
the methods directly in the `IRB::Context` class.
@tompng tompng merged commit 67eba54 into master Feb 23, 2024
57 checks passed
@tompng tompng deleted the remove-dynamic-extention-method-generation branch February 23, 2024 10:02
matzbot pushed a commit to ruby/ruby that referenced this pull request Feb 23, 2024
(ruby/irb#882)

* Unroll extension method generation

Given we only have 2 remaining extension setter methods, both of which
only take 1 argument and don't have any alias, the current method generation
logic is overly complicated.

This commit simplifies the method generation logic by simply defining
the methods directly in the `IRB::Context` class.

* Fix use_loader extension

ruby/irb@67eba5401b
@@ -49,7 +49,7 @@ def use_loader=(opt)
if IRB.conf[:USE_LOADER] != opt
IRB.conf[:USE_LOADER] = opt
if opt
if !$".include?("irb/cmd/load")
if !$".include?("irb/command/load")
Copy link
Member

Choose a reason for hiding this comment

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

This if seems doing nothing from the beginning.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! #885

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

Successfully merging this pull request may close these issues.

3 participants