Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow arrays in Variables view in Debugger to stay open when updated #74031

Closed
tommai78101 opened this issue May 21, 2019 · 11 comments
Closed

Allow arrays in Variables view in Debugger to stay open when updated #74031

tommai78101 opened this issue May 21, 2019 · 11 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues verified Verification succeeded
Milestone

Comments

@tommai78101
Copy link

This is an extension to #2399.

I see that the discussion above mentioned about the flickering when the variables are updated when stepping over code.

However, it seems the flickering can cause expanded (inspected) variables to become closed after an update. You can see how I have to to keep reopening the panel to view the contents of an array in the Variables view when debugging and stepping over some code.

GIF

I would like to request a feature to keep the expanded (inspected) variables to stay expanded (inspected) when a "Step Over" command is issued in VS Code.

Thanks for reading.

@weinand
Copy link
Contributor

weinand commented May 21, 2019

@isidorn In general this is an issue with the reference-ID-generation in the debug adapter.
However this time it looks like there is a bigger problem.

I can reproduce this with the node-debugger.
Use this code:

let a = [];

for(i = 0; i < 100; i++) {
	a[i] = i;
}
  • Set a breakpoint on the statement inside the for-loop.
  • start debugging
  • expand array
  • press "continue"

Observe:
array stays expanded (as expected).

  • press "step"

Observe:
array collapses.

@weinand weinand assigned isidorn and unassigned weinand May 21, 2019
@weinand weinand added debug Debug viewlet, configurations, breakpoints, adapter issues bug Issue identified by VS Code Team member as probable bug labels May 21, 2019
@weinand weinand added this to the May 2019 milestone May 21, 2019
@isidorn
Copy link
Contributor

isidorn commented May 23, 2019

@weinand the issue here is that the stack frame of the frame changes. Before pressing "step" the stackFrame has the id 1000 and after pressing "step" the stackFrame has the id 1001.
For the vscode ui these two frames are then considered different.

In order to fix this the debug adapter should return consistent frameId for same frames across steps.
As an alternative vscode could use the name of the stack frame when generating the id. Though using the current approach seems better to me.

@isidorn
Copy link
Contributor

isidorn commented May 28, 2019

Assigning to @weinand and @roblourens to see if the stackFrame id can be preserved and removing the milestone.

@isidorn isidorn assigned roblourens and weinand and unassigned isidorn May 28, 2019
@isidorn isidorn removed this from the May 2019 milestone May 28, 2019
@weinand
Copy link
Contributor

weinand commented May 28, 2019

@isidorn the original problem occurred in the Java debugger. Fixing this in node.js debugging doesn't help that much for Java.
Please explain the Java folks what to do.

@weinand weinand assigned isidorn and unassigned roblourens and weinand May 28, 2019
@isidorn
Copy link
Contributor

isidorn commented May 28, 2019

Dear Java folks,

VSCode is using the tree to represent the variables in the variables view. In order for us to keep the expansion state we need to uniquely identify each element in the tree.
For that we compute a unique ID for each element: Scope, Variable and so on.
Currently for the scope ID computation we use the StackFrame.id
In case you are interested here is where we compute the ids pointer
We use that to be consistent since each element takes into account its parents id.

In practice we could remove this. And that each scope does not take the parents ID into consideration. We can only do this because in vscode ui we never show multiple scopes from multiple stack frames at the same time in the tree. Our variables view is always scoped per StackFrame.

So we have two options to fix this.

  1. Java makes sure that the StackFrame id does not change when it does not need to
  2. VSCode no longer takes into account the stackFrame id when computing the scope id - this is a change which I could only push in debt week and only after self hosting on it for a milestone I would be comfortable with shipping this

Bottom line: I like the current id computation which is dependent on the parent and I would prefer to got with 1). If that is not possible than I can look into 2)

ping @testforstephen

@isidorn isidorn added under-discussion Issue is under discussion for relevance, priority, approach and removed bug Issue identified by VS Code Team member as probable bug labels May 28, 2019
@testforstephen
Copy link

i have two questions here:

  1. In my environment (VS Code 1.34.0), sometimes the expandable status is kept after step, sometimes it collapses all indexable variables. The behavior is not consistent, looks like a bug.
  2. For java debug, the JVM debug interface didn't provide us much information about the stack frame is changed or not during the step. We can only guess whether the stack frames are changed by comparing the total stack frame count with the last count. If the stack frame count doesn't change after step, it may indicate we can reuse the same frame id for the stack frame with the same depth. But this approach looks like a hack. I prefer the vscode client to deal with it.

@isidorn
Copy link
Contributor

isidorn commented May 31, 2019

  1. This is 99% becuase the id coming from the adapter changed. As I have mentioned in my previous comment, for each item in the variables view we compute an ID, this ID is based on the ID of its parent chain, and for each element we compute the ID using data coming from the debug adapter. If an element like for example StackFrame has an ID then we will use that to compute our own ID. Thus if that changes, our ID changes and the expansions status is not kept
  2. Ok, let's try that I no longer take into account StackFrame.id for the scope. I will push this next week and you can try it out and let me know how it goes.

@isidorn isidorn added this to the June 2019 milestone May 31, 2019
@isidorn
Copy link
Contributor

isidorn commented Jun 3, 2019

I have pushed these changes. The insiders in about a week should have these chagnes
@tommai78101 @testforstephen let me know how it behaves for you know. Looking at the sample @weinand provided this is fixed now. Adding bug label so we verify this.

@isidorn isidorn added bug Issue identified by VS Code Team member as probable bug and removed under-discussion Issue is under discussion for relevance, priority, approach labels Jun 3, 2019
@isidorn isidorn closed this as completed in 217d993 Jun 3, 2019
@tommai78101
Copy link
Author

@isidorn I may not likely be available to test this change, since I am preparing for a long, family vacation trip coming up really soon. I won't be available to test this until July 2019. Just letting everyone know in advance.

@testforstephen
Copy link

testforstephen commented Jun 10, 2019

I tried in the 1.36.0-insider with 2019-06-07T05:19:15.188Z version, the expanding status is kept after step. It works.

@isidorn
Copy link
Contributor

isidorn commented Jun 10, 2019

Great, adding verified label. Thanks for trying it out.

@isidorn isidorn added the verified Verification succeeded label Jun 10, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Jul 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

5 participants