Proposal: Refactor server_dap to use command-registration style

[andyw8]

The current implementation of server_dap.rb uses a very long case statement.

We (the Ruby Developer Experience team at Shopify) would like to propose changing this better align with the register_command approach already used in sessions.rb. This will allow the long case statement to be avoided, and will provide better consistency within the project.

Please take a look at a suggested approach. The behaviour should be identical to as it was before. We look forward to hearing your feedback.

We recommend viewing the PR with whitespace changes ignored: https://github.com/ruby/debug/pull/931/files?w=1

[ko1]

Thank you for the patch.
However, current usage of case/when is intentional and consistency is not acceptable reason.
I will close it if there is no reasons.

[andyw8]

@ko1 Thank you for the feedback. I will give some additional context about why we wanted to make this change:

Within the Shopify Ruby DX team, we aim to build a better understanding of how Shopify developers are using tooling, by adding telemetry/metrics reporting.

For debug, we want to learn which particular commands developers are making use of. Our idea was to override the process method in our applications to allow this. However, since the process method is very long, that would be difficult to maintain, hence we wanted to first refactor.

We are curious, what's the reason for preferring case here, but the registration approach in sessions.rb?

Regardless, there is a different approach we could take that would result in a much smaller change. We could extract the body of the while into a separate method, which would make it much easier to override process:

def process
  while req = recv_request
    process_request(req)
  end
end

def process_request(req)
  case req['command']
    when 'launch'
    # ...
    # etc. for all the existing `when` clauses
  end
end

Would you be open to something like that?

[andyw8]

@ko1 Just wondering if you have any further thoughts on this?

[ko1]

However, since the process method is very long, that would be difficult to maintain, hence we wanted to first refactor.

I don't think "difficult to maintain" on this case but it is my preferences.
Trivial reason is case/when with strings will be compiled to special optimized bytecodes so it is slight faster.

Regardless, there is a different approach we could take that would result in a much smaller change. We could extract the body of the while into a separate method, which would make it much easier to override process:

It is acceptable so I'll make another PR for it and close this PR. Thank you.