Share variable inspection logic between CDP and DAP

[amomchilov]

The logic that decides how variables are presented is repeated between the CDP and DAP implementations. This is error prone and leads to inconsistencies, where improvements are made to one or the other, but not both. E.g.:

1. The special #dump member for String only exists in VSCode #869
2. This feature to hide special #class members only exists for VSCode #986
3. VSCode formats hash keys with value_inspect, but not Chrome
4. CDP adds #class appears at the end, whereas DAP adds #class to the start.

This PR extracts the variable inspection logic into a new VariableInspector class, which can be shared between VSCode and Chrome. This gives a central place to make future improvements.

I tried to make only the minimal behaviour changes necessary to unify the two implementations.

This PR then unlocks the ability to make some changes really nice/easily:

1. #1003
2. #1004
3. #1005

[ko1]

• I like an idea to unify CDP and DAP because of implementation.
• However I'm not sure we can merge it because of difference between CDP and DAP (I don't know the CDP details). Do you have a time to trouble shooting for CDP?
• The name "variable.rb" doesn't make sense for me because it's only for the representation (the name is too general). debug.gem has a policy to reduce the number of "require" to reduce the boot time. Can we merge it with variable_inspector.rb?

[amomchilov]

Do you have a time to trouble shooting for CDP?

I've already run this with CDP, it works great.

Can we merge it with variable_inspector.rb?

@ko1 Done. I still call it class Variable. It's a general name, but I don't have any better ideas. Please suggest if you have any.

[ono-max]

Also, some tests are failed in protocol part. Can you fix them? FYI: https://github.com/ruby/debug/blob/master/CONTRIBUTING.md

[amomchilov]

@ono-max I can have a look over the holiday break next week :)

[amomchilov]

Hey @ono-max, Happy new year!

I've fixed the protocol tests, and rebased onto the latest master.

There's 2 test failures in test/console/nested_break_test.rb left, but I don't see how they could be related.

[ono-max]

It seems to be that some protocol tests are still failed. Can you please check them? If you need help, please let me know 👍

I believe that https://github.com/ruby/debug/actions/runs/7405610000/job/20183471986?pr=1001 and https://github.com/ruby/debug/actions/runs/7405610000/job/20183471690?pr=1001 are not flakey tests.

[amomchilov]

@ono-max Hmmm I think we have one of those "it works on my machine" situations

$ bundle exec rake test_protocol

Loaded suite /Users/alex/.gem/ruby/3.3.0/gems/rake-13.0.6/lib/rake/rake_test_loader
Started
Finished in 1.438602 seconds.
--------------------------------------------------------------------------------
2 tests, 690 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
--------------------------------------------------------------------------------
1.39 tests/s, 479.63 assertions/s

I'm running on Ruby 3.3.0 on macOS Sonoma 14.2.1.

I suspect these errors are coming from the ancestors chains being in different in CI, but I don't know why that would be. Could you please help me investigate this? I don't have access to a Linux machine to compare against.

[ono-max]

I investigated the cause of the failed tests. I believe that the reason is because ancestor classes are changed from Ruby 3.1. Thus, we can skip the test if the Ruby version is lower than 3.1, e.g. 6f154ac

[amomchilov]

@ono-max Fantastic find! I pulled that commit into this PR, so you get credit for it :)

Is this ready to merge?

[ono-max]

I appreciate your dedicated efforts and hard work. @ko1-san will merge this PR 👍

[amomchilov]

@ono-max Thanks!

Could you run the CI one more time, so we're sure it's green before merging?

[amomchilov]

@ono-max I think the remaining test failures are just flakey tests. I can reproduce them Ruby 3.1.4, but they fail less than 1/10 times.

[amomchilov]

@ko1 Can you please review and merge this?