-
Notifications
You must be signed in to change notification settings - Fork 141
Add --context-mode 5: evaluates inside Ruby::Box
#1142
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
Conversation
| @binding = TOPLEVEL_BINDING.dup | ||
| when 5 | ||
| puts 'Context-mode-5 (binding in new Ruby::Box) is experimental. It may be removed or changed without notice.' | ||
| @binding = Ruby::Box.new.eval('Kernel.binding') |
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.
How about we include a check for the Ruby version?
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.
I think it should be a constant defined check instead? If Ruby::Box doesn't exist, then we should warn that it's not available and that we'd fallback to the default behaviour.
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.
Add a defined?(Ruby::Box) check.
I think if Ruby::Box is unavailable, it's OK or even safe to fail instead of fallback to default.
There is one more error case. If Box is not enabled by ENV['RUBY_BOX'], class Ruby::Box is defined but calling new throws RuntimeError.
'Ruby::Box#initialize': Ruby Box is disabled. Set RUBY_BOX=1 environment variable to use Ruby::Box. (RuntimeError)
|
Can we have a few simple test cases running with this mode on? It can be used to detect |
2f8cc0e to
48a84fa
Compare
| when 5 # binding in Ruby::Box | ||
| unless defined?(Ruby::Box) | ||
| puts 'Context-mode 5 (binding in Ruby::Box) requires Ruby 4.0 or later.' | ||
| raise NameError, 'Ruby::Box not defined' |
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.
Immediate exit will make IRB.start rescue do_something not work as expected
|
|
||
| class ContextModeTest < TestIRB::IntegrationTestCase | ||
| # RUBY_BOX=1 may need more time to start IRB | ||
| TIMEOUT_SEC = TestIRB::IntegrationTestCase::TIMEOUT_SEC + 5 |
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.
I think this can be
| TIMEOUT_SEC = TestIRB::IntegrationTestCase::TIMEOUT_SEC + 5 | |
| TIMEOUT_SEC = self::TIMEOUT_SEC + 5 |
class Foo
TIMEOUT = 10
end
class Bar < Foo
TIMEOUT = self::TIMEOUT + 10
end
puts Bar::TIMEOUT # 20There 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.
Actually, it can be even shorter
TIMEOUT = TIMEOUT + 5or
TIMEOUT += 5but the visual of the code, especially the last one, looks like a constant reassignment. (It's not, though)
So my code is... a bit redundant but I'd like to use the redundant form here.
A playground to try monkey patching core methods.
Related to: #143 #235 #960