-
Notifications
You must be signed in to change notification settings - Fork 120
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
Change to explicit method call in completion #369
Conversation
lib/irb/completion.rb
Outdated
gv = eval("send(:global_variables)", bind).collect{|m| m.to_s}.push("true", "false", "nil") | ||
lv = eval("send(:local_variables)", bind).collect{|m| m.to_s} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Under Ruby 2.6, private methods cannot be called with self.
, so #send
is used.
Ensure that methods are called even when local variables are defined. see: ruby#368
Thanks for the support. class Foo
def methods
binding.irb
1
end
end
Foo.new.methods If we can use |
…ocal_variables #instance_variables.
Thanks for comment. |
test/irb/test_completion.rb
Outdated
str_example = '' | ||
str_example.clear # suppress "assigned but unused variable" warning | ||
@str_example = '' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like str_example
and @str_example
are test subjects, and the rest are for making sure irb
won't confuse them when completion?
If that's the case, can we add comments to mention that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks comment.
I added comment 697708d.
lib/irb/completion.rb
Outdated
using Module.new { | ||
refine ::Binding do | ||
def eval_methods | ||
eval("::Kernel.instance_method(:methods).bind(self).call") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of wrapping the entire chain inside eval
, can we only wrap the necessary part?
def eval_methods
::Kernel.instance_method(:methods).bind(eval("self")).call
end
def eval_private_methods
::Kernel.instance_method(:private_methods).bind(eval("self")).call
end
def eval_instance_variables
::Kernel.instance_method(:instance_variables).bind(eval("self")).call
end
def eval_global_variables
::Kernel.instance_method(:global_variables).bind(eval("self")).call
end
def eval_class_constants
::Module.instance_method(:constants).bind(eval("self.class")).call
end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks comments :)
I fixed it.
78ef990
@osyo-manga I think the PR is almost ready. Can you do the small adjustments I mentioned above? Thx |
@osyo-manga Given that this issue crashes irb, I want to make sure it's fixed and tested before Ruby 3.2. So I opened #404 based on your commits. Let me know if you prefer making those changes yourself, I'll close mine 🙂 |
Co-authored-by: Stan Lo <stan001212@gmail.com>
@st0012 Sorry for the delay in replying. |
@osyo-manga Thank you 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution!
Ensure that methods are called even when local variables are defined. see: ruby/irb#368 ruby/irb@c34d54b8bb
Thank for merge :) |
Ensure that methods are called even when local variables are defined.
see: #368