Single step in debugger collapses all variables being watched

[o-lim]

• VSCode Version: 1.11.x
• OS Version: Ubuntu 14.04
• Language: C++
• Debugger: gdb

When stepping over an instruction, any expanded variables in the watch window are automatically collapsed, force the user to to expand any variable of interest to view it's contents. This is particularly annoying when you need to observe how a nested variable (2 or more levels deep) is updated across several lines of code.

[alexdima]

@o-lim Which extension do you use for debugging ?

[o-lim]

I use the Microsoft C/C++ extension

[weinand]

Please file this issue against the C/C++ extension.

[o-lim]

@weinand: According to the guys at vscode-cpptools, this is a VSCode issue. See microsoft/vscode-cpptools#677

[weinand]

@isidorn any insights?

[pieandcakes]

@weinand I don't think the protocol allows for me to say that it should be expanded when I return the evaluation value. Any ideas?

[weinand]

The expansion state of a Variable or Watch tree is preserved across steps if the variablesReference (https://github.com/Microsoft/vscode-debugadapter-node/blob/master/protocol/src/debugProtocol.ts#L1177) doesn't change.
This works fine for node-debug, mock-debug, and mono-debug which proves that the VS Code debugger UI correctly supports this.
If it doesn't work for the C/C++ debugger, then I would check that the variablesReference remains stable across stepping.

@pieandcakes no, the protocol for Variable access does not allow to specify the expand state for evaluation values. But I do not see how this would help to solve this issue: the debug adapter should not have to care about the expansion state of the tree. This is only VS Code's business.

[gregg-miskelly]

@weinand how do you implement keeping the variablesReference the same? Ex: Do you have a Dictionary from name -> integer? Do you ever clear it out? Shouldn't VS Code remember the expansion state via something less fragile then the variableReference numbers (ex: the names of the variables that are expanded and then the names of the children of those variables).

[isidorn]

I have just checked the code on the vscode side I confirm what @weinand commented.

Some more details:
Tree expansion state is preserved if the id of the element is preserved.
The toplevel watch expression should remain expanded / collapsed no matter the reference, since for watch expressions we use a random UUID as an ID.
The children of that watch expression are Variables and for them we use the reference to determine the id in the following way "variable:${parent.getId()}:${name}:${reference}".

And yes the protocol should not touch upon tree expansion so the C++ extension should try to make the references stable where it makes sense

[isidorn]

@gregg-miskelly one of the reasons why vscode also uses the reference when determing the id is that there were cases when there are mutliple varialbes with the same name in the same "scope". Even though this is illegal. We have to prevent that as having two elements with the same ID seriously breaks the tree.

An alternative would be that vscode filters variables it recieves by name uniqness and then only uses tha name and the parentId to get the id. I am not sure if this would work in practice and would have to first try it out.
However since the current approach seems to work fine for all other debug adapters I would suggest that the C++ extension first try to fix this and if unable then we can look into changing the id computation on the vscode side.

[gregg-miskelly]

@isidorn How do you implement this in other debug adapters? Ex: Do you have a Dictionary? Or do you just get lucky? In the case of the C++ and C# debug adapters, reference numbers are just calculated with a simple counter. So the first thing that VS Code asks for that needs a reference during a particular break state session will be 1, the second will be 2, etc. I guess I could imagine some complicated logic for trying to keep reference numbers the same, but to me at least, this doesn't seem like it should be the back end's job. If you really want to do it that way, I would suggest at least explaining this in the protocol documentation.

I understand how multiple variables of the same name makes this more complicated, but I would suggest using name+child_number/local_number rather than reference.

[weinand]

@gregg-miskelly no, we don't use dictionaries. The node DA resets the counter whenever entering break state (stopped event).

[gregg-miskelly]

@weinand So don't you have the same problem then? Or am I missing something.

Theoretical repro steps:

1. Evaluate something is expandable in the watch window, this should return variableReference:1
2. Remove that from the watch window
3. Evaluate something new is the watch window that is expandable to at least two levels, this returns variableReference:2
4. Expand that. This return an expandable child with variableReference:3
5. Expand that
6. Step
7. Watch window item is reevaluated. This time it will return variableReference:1
8. Watch window item is expanded since the unique GUID is used
9. Watch window item return expandable child, but this time it has variableReference:2
10. Since the variableReference numbers don't match up, expansion state will be lost