Skip to content

Conversation

@muno92
Copy link
Contributor

@muno92 muno92 commented May 7, 2025

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.

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)}"
Copy link
Member

@tompng tompng May 7, 2025

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
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
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.

assert_match(/=> 2\| puts "hello"/, output)
end

def test_debug_class_to_s_return_string
Copy link
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
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
Copy link
Member

@tompng tompng left a comment

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
33 checks passed
@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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Exception when using irb:rdbg where objectto_s might return nil

3 participants