Skip to content

Selective inspect of instance variables #13555

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

Merged
merged 1 commit into from
Jun 9, 2025

Conversation

byroot
Copy link
Member

@byroot byroot commented Jun 7, 2025

[Feature #21219]

Make Kernel#inspect ask which instance variables should be dumped by the result of #instance_variables_to_inspect.

@byroot byroot requested a review from nobu June 7, 2025 07:38

This comment has been minimized.

Make Kernel#inspect ask which instance variables should be dumped by
the result of `#instance_variables_to_inspect`.

Co-Authored-By: Jean Boussier <byroot@ruby-lang.org>
@byroot byroot force-pushed the instance_variables_to_inspect branch from 6ddd72d to e643b25 Compare June 7, 2025 07:59
@byroot byroot merged commit f4135fe into ruby:master Jun 9, 2025
85 checks passed
@byroot byroot deleted the instance_variables_to_inspect branch June 9, 2025 07:25
Copy link
Contributor

@joshuay03 joshuay03 left a comment

Choose a reason for hiding this comment

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

This is going to be so useful! I had a few suggestions while reading this in passing—let me know if you agree with them, and I'd be happy to send a PR.

Comment on lines +19 to +20
* `Kernel#inspect` now check for the existence of a `#instance_variables_to_inspect` method
allowing to control which instance variables are displayed in the `#inspect` string:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* `Kernel#inspect` now check for the existence of a `#instance_variables_to_inspect` method
allowing to control which instance variables are displayed in the `#inspect` string:
* `Kernel#inspect` now checks for the existence of a `#instance_variables_to_inspect` method,
allowing control over which instance variables are displayed in the `#inspect` string:

@@ -28,4 +28,34 @@ class << obj
end
obj.inspect.should be_kind_of(String)
end

ruby_version_is "3.5" do
it "calls #instance_variables_to_inspect private method to know which variables to display" do
Copy link
Contributor

@joshuay03 joshuay03 Jun 9, 2025

Choose a reason for hiding this comment

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

Reading this I assumed it had to be a private method, but the implementation tells me it doesn't. What do you think about removing the mention of private here and making the method public in the empty override case (see next suggestion) so both are covered?

Suggested change
it "calls #instance_variables_to_inspect private method to know which variables to display" do
it "calls #instance_variables_to_inspect method to know which variables to display" do

Copy link
Member Author

Choose a reason for hiding this comment

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

I assumed it had to be a private method,

No method ever have to be private. e.g. method_missing is supposed to be private, but nothing will break if it isn't.

@password = "hunter2"
end
obj.singleton_class.class_eval do
private def instance_variables_to_inspect = []
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private def instance_variables_to_inspect = []
def instance_variables_to_inspect = []

joshuay03 added a commit to joshuay03/ruby that referenced this pull request Jun 9, 2025
joshuay03 added a commit to joshuay03/ruby that referenced this pull request Jun 9, 2025
@joshuay03 joshuay03 mentioned this pull request Jun 9, 2025
@joshuay03
Copy link
Contributor

let me know if you agree with them, and I'd be happy to send a PR

Actually it's late here so I've just opened this if you decide to merge them: #13564

joshuay03 added a commit to joshuay03/ruby that referenced this pull request Jun 10, 2025
joshuay03 added a commit to joshuay03/ruby that referenced this pull request Jun 10, 2025
byroot pushed a commit that referenced this pull request Jun 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants