diff options
author | Connor Peet <connor@peet.io> | 2022-06-08 20:00:18 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-06-08 20:00:18 +0300 |
commit | a7d2cfe5c56ac960584c9b8a7127fd5450792aa4 (patch) | |
tree | db68ee62e3648db073a4b6579001efaeea5938b0 /src/vs | |
parent | 1b093f9bf78194a1fe7fdd408e4679811f63a16e (diff) |
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.
Diffstat (limited to 'src/vs')
-rw-r--r-- | src/vs/workbench/contrib/debug/browser/variablesView.ts | 4 | ||||
-rw-r--r-- | src/vs/workbench/contrib/debug/common/debugModel.ts | 20 |
2 files changed, 15 insertions, 9 deletions
diff --git a/src/vs/workbench/contrib/debug/browser/variablesView.ts b/src/vs/workbench/contrib/debug/browser/variablesView.ts index 01aac2fa0b6..d4e995fb9f1 100644 --- a/src/vs/workbench/contrib/debug/browser/variablesView.ts +++ b/src/vs/workbench/contrib/debug/browser/variablesView.ts @@ -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); } diff --git a/src/vs/workbench/contrib/debug/common/debugModel.ts b/src/vs/workbench/contrib/debug/common/debugModel.ts index c05fc04c956..4b20e069cd2 100644 --- a/src/vs/workbench/contrib/debug/common/debugModel.ts +++ b/src/vs/workbench/contrib/debug/common/debugModel.ts @@ -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); }); |