Skip to content

Fix nil error on debugger prompt#1097

Merged
tompng merged 3 commits into
ruby:masterfrom
muno92:fix-nil-error-on-debugger-prompt
May 8, 2025
Merged

Fix nil error on debugger prompt#1097
tompng merged 3 commits into
ruby:masterfrom
muno92:fix-nil-error-on-debugger-prompt

Conversation

@muno92

@muno92 muno92 commented May 7, 2025

Copy link
Copy Markdown
Contributor

Fix #1089

This change ensures that when a class's to_s method returns nil, it is handled in the same way as if to_s had raised an error.

Comment thread lib/irb.rb Outdated
@context.irb_name
when "m"
main_str = @context.safe_method_call_on_main(:to_s) rescue "!#{$!.class}"
main_str = main_str || "!#{@context.safe_method_call_on_main(:class)}"

@tompng tompng May 7, 2025

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 think ! in "!RuntimeError" is expressing that stringifying main raised RuntimeError, not that main is a RuntimeError.
So we don't need ! prefixed to a non-error class name.

I think main_str can be just an empty string. Just like when to_s returned an empty string.
How about this?

main_str = "#{@context.safe_method_call_on_main(:to_s)}" rescue "!#{$!.class}"

This way, it can handle other erroneous case such as to_s returned Object.new

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.

Or this one

main_str = "!TypeError" unless String === main_str

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you for reviewing.

I misunderstood meaning of prompt.

And "#{@context.safe_method_call_on_main(:to_s)}" looks better!
So I changed.

Comment thread test/irb/test_debugger_integration.rb Outdated
assert_match(/=> 2\| puts "hello"/, output)
end

def test_debug_class_to_s_return_string

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.

This test case is not needed.
It is testing a common normal case that to_s is defined and it returns string. I think this case is already tested in another test case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I removed this test case.

@st0012 st0012 added the bug Something isn't working label May 8, 2025

@tompng tompng left a comment

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.

Thank you 👍

@tompng tompng merged commit f5254a3 into ruby:master May 8, 2025
@muno92 muno92 deleted the fix-nil-error-on-debugger-prompt branch May 8, 2025 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

3 participants