Dape compatibility

[iNecas]

When trying to get DebugAdapter running with https://github.com/svaante/dape, I came across some issues and capabilities I needed to support it fully, both with launch and attach requests.

[davidanthoff]

I tried to understand what this would fix, but I'm not succeeding :) I think the idea here is that we don't handle some message correctly that could contain a null value? Which one? My sense is that the DAP only allows null in very few cases that we actually don't support in the first place? I think I just need to get a bit more context to understand this PR.

[iNecas]

A sepecific case that I've hit was originated here https://github.com/svaante/dape/blob/master/dape.el#L2418 (but there are other similar cases). I first meant to send a PR to dape to use boolean instead, but then, when looking at the specification https://microsoft.github.io/debug-adapter-protocol/specification#Requests_Disconnect, there is a difference between false value and null/missing key (letting the implementation to make a choice). Given dape works fine with other DAP implementations, making the code here a bit more resilient looked like a better approach.

[davidanthoff]

I don't think sending null as the value is spec conformant, though? The spec conformant way is to just not include the terminateDebuggee element in the JSON, and I think that is already supported on our end?

[iNecas]

You are right. Not TypeScripter myself, but given the DAP protocol is defined via TS, it seems it doesn't like null values (based on some short experiement).

I will need to send a PR to dape to fix that.

Note the issue with null values is just one change needed. The other two were:

1. allow to customize the Capabilities when initiating the server
2. handle missing arguments (this is different than the null values issues above, as in this case, the arguments are really omitted and there are quite a couple of arguments? places in the DAP spec

Would the other changes be acceptable, any changes suggested there?

[davidanthoff]

allow to customize the Capabilities when initiating the server

Can you explain a bit more why that would be useful? In my mind the list of supported features is kind of hard coded simply by the choice of what features we implement?

handle missing arguments (this is different than the null values issues above, as in this case, the arguments are really omitted and there are quite a couple of arguments? places in the DAP spec

So, I think technically that is already supported, albeit in a bit of an inconsistent way.

DebugAdapter.jl/src/protocol/dap_message_defs.jl

Line 46 in d82fe8d

const configuration_done_request_type = DAPRPC.RequestType("configurationDone", Union{ConfigurationDoneArguments,Nothing}, ConfigurationDoneResponseArguments)

is an example, i.e. the way to do that is to use a Union{T,Nothing}. Having said that, I guess it would actually be more consistent to use Missing instead there, as we have used that throughout for the ? TypeScript case.

[iNecas]

Can you explain a bit more why that would be useful? In my mind the list of supported features is kind of hard coded simply by the choice of what features we implement?

The particular case I've hit was at the end of attach request session. Dape looks at the capabilities to determine, whether to send the terminateRequest https://github.com/svaante/dape/blob/master/dape.el#L2390. It however doesn't seem to make sense in the attach case and that's what I observed in lldb dap implementation was they report supportsTerminateRequest to be false in this case.

Thanks for the pointers about the alternative way of handling the missing arguments, I will try looking at leveraging that approach.