-
Notifications
You must be signed in to change notification settings - Fork 119
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
Fix debug command in nomultiline mode #1006
Conversation
lib/irb/context.rb
Outdated
@@ -649,6 +649,21 @@ def parse_command(code) | |||
end | |||
end | |||
|
|||
def colorize_code(input, complete:) |
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 feel colorize_input
express the intent better and is less likely to be confused with Color.colorize_code
?
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 better, but I have one concern. colorize_input colorizes output. I think it is an input, but Reline's api calls it output.
Do you think the code below is acceptable? IMO... maybe yes?
Reline.output_modifier_proc = proc do |output, complete:|
IRB.CurrentContext.colorize_input(output, complete: complete)
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.
In IRB's context, it is input indeed. And in Reline's context, I think calling it both input or output are technically correct? Though since there's no input_modifier_proc
, I feel it'd be a better name for the library user's perspective.
In this case, I feel a comment to clarify it would be enough, and maybe rename the output
variable to input
?
Thank you! |
(ruby/irb#1006) * Fix debug command in nomultiline mode * context.colorize_code -> context.colorize_input ruby/irb@71f4d6bfb5
Fixes #909 and #1003
Fix this bug
rendering test conflicts with #1001