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.

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);
 
 				});