Skip to content

Make ls command work for BasicObjects#1177

Open
eikes wants to merge 4 commits intoruby:masterfrom
eikes:fix-ls-for-basic-objects
Open

Make ls command work for BasicObjects#1177
eikes wants to merge 4 commits intoruby:masterfrom
eikes:fix-ls-for-basic-objects

Conversation

@eikes
Copy link
Contributor

@eikes eikes commented Feb 25, 2026

When working with the builder gem I noticed that the irb inspection of the Builder::XmlMarkup instances was not working as expected. Upon further investigation it turns out, that the reason is that this class inherits from BasicObject directly and not from the more common Object.

You can see the problem here:

4.0.1 :001 > ls BasicObject.new
/gems/irb-1.17.0/lib/irb/command/ls.rb:58:in 'IRB::Command::Ls#execute': undefined method 'class' for an instance of BasicObject (NoMethodError)

        klass  = (obj.class == Class || obj.class == Module ? obj : obj.class)
                     ^^^^^^

I have also created a new ls command test file containing a failing test which you can see below: https://github.com/ruby/irb/actions/runs/22390651625/job/64811518726

The test is fixed with the second commit of this PR by using a slightly different way to inspect the class. singleton_class and instance_variables of an object.

PS: The prism-latest test failure is because the syntax of ruby 4.1 allows trailing commas in method definitions and this change is addressed here: #1178


klass = (obj.class == Class || obj.class == Module ? obj : obj.class)
klass = Kernel.instance_method(:class).bind(obj).call
is_class_or_module = klass == Class || klass == Module
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can just do Kernel.instance_method(:is_a?).bind(obj).call(Module)? If an object is a Class, it's definitely a Module as well.

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 didn't think about that, but it's true! I just kept it as it was but I'll change it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not necessarily easier to read though:

-        is_class_or_module = klass == Class || klass == Module
+        is_class_or_module = Kernel.instance_method(:is_a?).bind(obj).call(Module)

Copy link
Member

Choose a reason for hiding this comment

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

How about Module === obj

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's neat! I changed it 👍

@eikes eikes force-pushed the fix-ls-for-basic-objects branch from bc0baa1 to 301bf3f Compare February 25, 2026 17:05
@eikes
Copy link
Contributor Author

eikes commented Feb 25, 2026

Just rebased it on master to make the prism test pass...

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