debug: preserve scope expand state better (#151522)

Previously, we only expanded the top scope if there were no expanded
scopes lower down. I think this is too consevative. Chrome Devtools
doesn't do this, and I don't think we should either.

Second, we had some logic to preserve the expanded scopes. However,
this was being based on the scope index, which if you step inside a
function and have blocks with the same name, is likely to not be
preserved between loads. Instead, form the scope ID based on a quick
hash of its name _and_ location, which is preserved better.

I think this should solve some of @jrieken's papercuts.
This commit is contained in:
Connor Peet 2022-06-08 19:00:18 +02:00 committed by GitHub
parent 1b093f9bf7
commit a7d2cfe5c5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 15 additions and 9 deletions

View file

@ -96,10 +96,10 @@ export class VariablesView extends ViewPane {
const viewState = this.savedViewState.get(stackFrame.getId());
await this.tree.setInput(stackFrame, viewState);
// Automatically expand the first scope if it is not expensive and if all scopes are collapsed
// Automatically expand the first non-expensive scope
const scopes = await stackFrame.getScopes();
const toExpand = scopes.find(s => !s.expensive);
if (toExpand && (scopes.every(s => this.tree.isCollapsed(s)) || !this.autoExpandedScopes.has(toExpand.getId()))) {
if (toExpand) {
this.autoExpandedScopes.add(toExpand.getId());
await this.tree.expand(toExpand);
}

View file

@ -7,6 +7,7 @@ import { distinct, lastIndex } from 'vs/base/common/arrays';
import { RunOnceScheduler } from 'vs/base/common/async';
import { decodeBase64, encodeBase64, VSBuffer } from 'vs/base/common/buffer';
import { CancellationTokenSource } from 'vs/base/common/cancellation';
import { stringHash } from 'vs/base/common/hash';
import { Emitter, Event } from 'vs/base/common/event';
import { Disposable } from 'vs/base/common/lifecycle';
import { mixin } from 'vs/base/common/objects';
@ -351,7 +352,7 @@ export class Scope extends ExpressionContainer implements IScope {
constructor(
stackFrame: IStackFrame,
index: number,
id: number,
public readonly name: string,
reference: number,
public expensive: boolean,
@ -359,7 +360,7 @@ export class Scope extends ExpressionContainer implements IScope {
indexedVariables?: number,
public readonly range?: IRange
) {
super(stackFrame.thread.session, stackFrame.thread.threadId, reference, `scope:${name}:${index}`, namedVariables, indexedVariables);
super(stackFrame.thread.session, stackFrame.thread.threadId, reference, `scope:${name}:${id}`, namedVariables, indexedVariables);
}
override toString(): string {
@ -417,12 +418,17 @@ export class StackFrame implements IStackFrame {
return [];
}
const scopeNameIndexes = new Map<string, number>();
const usedIds = new Set<number>();
return response.body.scopes.map(rs => {
const previousIndex = scopeNameIndexes.get(rs.name);
const index = typeof previousIndex === 'number' ? previousIndex + 1 : 0;
scopeNameIndexes.set(rs.name, index);
return new Scope(this, index, rs.name, rs.variablesReference, rs.expensive, rs.namedVariables, rs.indexedVariables,
// form the id based on the name and location so that it's the
// same across multiple pauses to retain expansion state
let id = 0;
do {
id = stringHash(`${rs.name}:${rs.line}:${rs.column}`, id);
} while (usedIds.has(id));
usedIds.add(id);
return new Scope(this, id, rs.name, rs.variablesReference, rs.expensive, rs.namedVariables, rs.indexedVariables,
rs.line && rs.column && rs.endLine && rs.endColumn ? new Range(rs.line, rs.column, rs.endLine, rs.endColumn) : undefined);
});