Skip to content

Fix debug command in nomultiline mode#1006

Merged
tompng merged 2 commits into
ruby:masterfrom
tompng:debug_readline_bugfix
Sep 20, 2024
Merged

Fix debug command in nomultiline mode#1006
tompng merged 2 commits into
ruby:masterfrom
tompng:debug_readline_bugfix

Conversation

@tompng

@tompng tompng commented Sep 17, 2024

Copy link
Copy Markdown
Member

Fixes #909 and #1003

Fix this bug

% irb --readline
irb(main):001> binding.irb
irb(main):001> debug
/Users/tomoya.ishida/.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/irb-1.14.0/lib/irb/debug.rb:72:in `block in setup': undefined method `call' for nil (NoMethodError)

            irb_output_modifier_proc.call(output, complete: complete)
                                    ^^^^^

rendering test conflicts with #1001

@st0012 st0012 added the bug Something isn't working label Sep 17, 2024
Comment thread lib/irb/context.rb Outdated
end
end

def colorize_code(input, complete:)

Copy link
Copy Markdown
Member

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?

Copy link
Copy Markdown
Member Author

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

@st0012 st0012 Sep 17, 2024

Copy link
Copy Markdown
Member

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?

Comment thread lib/irb/debug.rb Outdated
@st0012

st0012 commented Sep 19, 2024

Copy link
Copy Markdown
Member

Thank you!

@tompng tompng merged commit 71f4d6b into ruby:master Sep 20, 2024
@tompng tompng deleted the debug_readline_bugfix branch September 20, 2024 10:13
matzbot pushed a commit to ruby/ruby that referenced this pull request Sep 20, 2024
(ruby/irb#1006)

* Fix debug command in nomultiline mode

* context.colorize_code -> context.colorize_input

ruby/irb@71f4d6bfb5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

2 participants