service/dap: add load config support to launch request

[polinasok]

Updates #1515

[aarzilli]

IMHO letting users set these values is going to be a hindrance for implementing autoloading, both in terms of code to support weird edge cases (like MaxArrayValues set to 0) and in terms of bug reports ("I set FollowPointers to false and now it crashes").

[polinasok]

What do you propose instead?
The current adapter allows users to set these. I guess the assumption has been that these are fair game for the frontend to expose to the user because they are part of the json-rpc interface.

[aarzilli]

What do you propose instead?

Expose MaxStringLen because it can't be done otherwise due to DAP limitations, and implement autoloading instead for everything else.

The current adapter allows users to set these. I guess the assumption has been that these are fair game for the frontend to expose to the user because they are part of the json-rpc interface.

They are, but I don't think it's an ergonomic way to do things in a graphical client.

[polinasok]

Could you elaborate please? What do you mean by DAP limitations here with respect to MaxStringLen vs others?

Here are some examples of how users take advantage of these config options:
https://itnext.io/golang-bits-better-debugging-in-vscode-599bc5b018da
https://ehotinger.com/blog/vscode-delve-debugging/
It's true that string comes up the most, but arrays are mentioned as well.
Here is a case where MaxVariableRecurse made a difference:
https://stackoverflow.com/questions/57414830/vscode-debug-not-display-map-values-in-variables-area-when-running-a-golang-debu
There were more cases of things not loading in the past, but some got fixed (microsoft/vscode-go#1010).

This, of course, ties into the intended support for cases when things are not fully loaded. The legacy adapter does the following in the Variables pane:

(1) String -- displays "+N more" and there is no link to load more

(2) Arrays, maps, slices -- displays the full length as part of the type at the top level, but then when expanded only shows a truncated list of elements. One would realize that there is less of them than the length, only if they count them, which is not very realistic for large structures. There is no explicit "+N more" indicator or a way to expand and load more.

(3) Structs -- displays the type with all fields at the top level, then a partial list of fields when you expand. Like with (2) there is no "+more" or "..." indicator or a way to click to load more.

(4) Pointers -- when expanded, is printed instead of value and there is no more expansion to load more.

(5) Interfaces -- here the MaxVariableRecurse is effectively ignored (unless it is -1, then the UI just hangs) because more and more data is loaded behind the scenes with evaluate requests as per your looking-into-variables doc as the user clicks > to expand data. I guess this is what you are referring to as autoloading, right?

(6) Struct elements - some cases auto-load (microsoft/vscode-go#1010) and some don't (https://stackoverflow.com/questions/57414830/vscode-debug-not-display-map-values-in-variables-area-when-running-a-golang-debu).

I have been considering the following:

(A) Allow users to load more data by using the config options in this change - sounds like you are firmly against this. But maybe your concerns can be addressed with a warning that this is a power option and to use with caution and that performance may suffer?

(B) Don't load fully at first, but give a way to expand further from the variables pane and not just for interfaces. Something like "+N more" with > that if clicked issues an additional request for the rest of the variable.

(C) For things like arrays and structs that require expanding anyway and are not fully displayed at the top level (unlike strings), implement full autoloading as part of expansion. If the user expands to see all elements, then actually load all of them with additional expression requests like is currently being done for interfaces. Btw, when adding autoloading for interfaces, vscode-go introduced autoloading for all types, but then quickly took it out in favor of the config (microsoft/vscode-go#2301).

(D) Provide an expression in the mouse-over that a user can use to issue an evaluate request themselves from the Debug Console.

It sounds like you are proposing partial (A) with only strings and (C). Do I have that right?

[aarzilli]

Could you elaborate please? What do you mean by DAP limitations here with respect to MaxStringLen vs others?

I don't think there's a way with DAP to show a button on incompletely loaded strings that loads more of them.

Here are some examples of how users take advantage of these config options:
https://itnext.io/golang-bits-better-debugging-in-vscode-599bc5b018da
https://ehotinger.com/blog/vscode-delve-debugging/
It's true that string comes up the most, but arrays are mentioned as well.
Here is a case where MaxVariableRecurse made a difference:
https://stackoverflow.com/questions/57414830/vscode-debug-not-display-map-values-in-variables-area-when-running-a-golang-debu
There were more cases of things not loading in the past, but some got fixed (microsoft/vscode-go#1010).

But if the adapter loaded automatically non-loaded variables when they are expanded this workarounds wouldn't be needed.

I have been considering the following:

(A) Allow users to load more data by using the config options in this change - sounds like you are firmly against this. But maybe your concerns can be addressed with a warning that this is a power option and to use with caution and that performance may suffer?

(B) Don't load fully at first, but give a way to expand further from the variables pane and not just for interfaces. Something like "+N more" with > that if clicked issues an additional request for the rest of the variable.

My assumption is that this is how things are supposed to be done, for VSCode. It's the most logical thing and what are the start and cound fields of VariablesArgument for if not this?

(C) For things like arrays and structs that require expanding anyway and are not fully displayed at the top level (unlike strings), implement full autoloading as part of expansion. If the user expands to see all elements, then actually load all of them with additional expression requests like is currently being done for interfaces. Btw, when adding autoloading for interfaces, vscode-go introduced autoloading for all types, but then quickly took it out in favor of the config (microsoft/vscode-go#2301).

You can't fully load a slice or a map because it could have millions of elements. That PR seems like a bad decision to me, but maybe I'm missing something.

(D) Provide an expression in the mouse-over that a user can use to issue an evaluate request themselves from the Debug Console.

It sounds like you are proposing partial (A) with only strings and (C). Do I have that right?

I think I'm proposing (B):

1. For strings there's a configuration option (unless it's possible to make it interactive)
2. For anything that isn't an array a slice or a map when the user expands it a request goes out to retrieve it
3. For arrays, slices and maps when the user expands them the first N elements are retrieved and an UI element is added to request loading more.