From 7f359bb293ccec5aae8e42b74805c7c4d3060ee6 Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Tue, 13 Feb 2024 16:18:29 -0800 Subject: [PATCH] debug: allow editing visualizers in debug tree (#205163) * debug: allow editing visualizers in debug tree * fix visibility lint --- .../contrib/debug/browser/baseDebugView.ts | 36 ++++--- .../contrib/debug/browser/debugHover.ts | 4 +- .../contrib/debug/browser/variablesView.ts | 50 ++++++---- .../debug/browser/watchExpressionsView.ts | 8 +- .../workbench/contrib/debug/common/debug.ts | 7 +- .../contrib/debug/common/debugModel.ts | 33 +++++-- .../contrib/debug/common/debugViewModel.ts | 19 +++- .../contrib/debug/common/debugVisualizers.ts | 96 ++++++------------- .../vscode.proposed.debugVisualization.d.ts | 2 +- 9 files changed, 135 insertions(+), 120 deletions(-) diff --git a/src/vs/workbench/contrib/debug/browser/baseDebugView.ts b/src/vs/workbench/contrib/debug/browser/baseDebugView.ts index d55c8cefd4d..ac626b82f40 100644 --- a/src/vs/workbench/contrib/debug/browser/baseDebugView.ts +++ b/src/vs/workbench/contrib/debug/browser/baseDebugView.ts @@ -10,17 +10,18 @@ import { HighlightedLabel, IHighlight } from 'vs/base/browser/ui/highlightedlabe import { IInputValidationOptions, InputBox } from 'vs/base/browser/ui/inputbox/inputBox'; import { IAsyncDataSource, ITreeNode, ITreeRenderer } from 'vs/base/browser/ui/tree/tree'; import { Codicon } from 'vs/base/common/codicons'; -import { ThemeIcon } from 'vs/base/common/themables'; -import { createMatches, FuzzyScore } from 'vs/base/common/filters'; +import { FuzzyScore, createMatches } from 'vs/base/common/filters'; import { createSingleCallFunction } from 'vs/base/common/functional'; import { KeyCode } from 'vs/base/common/keyCodes'; -import { DisposableStore, dispose, IDisposable, toDisposable } from 'vs/base/common/lifecycle'; +import { DisposableStore, IDisposable, dispose, toDisposable } from 'vs/base/common/lifecycle'; +import { ThemeIcon } from 'vs/base/common/themables'; import { localize } from 'vs/nls'; import { IContextViewService } from 'vs/platform/contextview/browser/contextView'; import { defaultInputBoxStyles } from 'vs/platform/theme/browser/defaultStyles'; import { LinkDetector } from 'vs/workbench/contrib/debug/browser/linkDetector'; import { IDebugService, IExpression, IExpressionValue } from 'vs/workbench/contrib/debug/common/debug'; import { Expression, ExpressionContainer, Variable } from 'vs/workbench/contrib/debug/common/debugModel'; +import { IDebugVisualizerService } from 'vs/workbench/contrib/debug/common/debugVisualizers'; import { ReplEvaluationResult } from 'vs/workbench/contrib/debug/common/replModel'; const MAX_VALUE_RENDER_LENGTH_IN_VIEWLET = 1024; @@ -146,24 +147,31 @@ export interface IExpressionTemplateData { } export abstract class AbstractExpressionDataSource implements IAsyncDataSource { - constructor(@IDebugService protected debugService: IDebugService) { } + constructor( + @IDebugService protected debugService: IDebugService, + @IDebugVisualizerService protected debugVisualizer: IDebugVisualizerService, + ) { } public abstract hasChildren(element: Input | Element): boolean; - public getChildren(element: Input | Element): Promise { + public async getChildren(element: Input | Element): Promise { const vm = this.debugService.getViewModel(); - return this.doGetChildren(element).then(r => { - let dup: Element[] | undefined; - for (let i = 0; i < r.length; i++) { - const visualized = vm.getVisualizedExpression(r[i] as IExpression); - if (visualized) { - dup ||= r.slice(); - dup[i] = visualized as Element; + const children = await this.doGetChildren(element); + return Promise.all(children.map(async r => { + const vizOrTree = vm.getVisualizedExpression(r as IExpression); + if (typeof vizOrTree === 'string') { + const viz = await this.debugVisualizer.getVisualizedNodeFor(vizOrTree, r); + if (viz) { + vm.setVisualizedExpression(r, viz); + return viz as IExpression as Element; } + } else if (vizOrTree) { + return vizOrTree as Element; } - return dup || r; - }); + + return r; + })); } protected abstract doGetChildren(element: Input | Element): Promise; diff --git a/src/vs/workbench/contrib/debug/browser/debugHover.ts b/src/vs/workbench/contrib/debug/browser/debugHover.ts index c0e1b530b3e..65ba580cbd4 100644 --- a/src/vs/workbench/contrib/debug/browser/debugHover.ts +++ b/src/vs/workbench/contrib/debug/browser/debugHover.ts @@ -127,7 +127,7 @@ export class DebugHoverWidget implements IContentWidget { this.treeContainer.setAttribute('role', 'tree'); const tip = dom.append(this.complexValueContainer, $('.tip')); tip.textContent = nls.localize({ key: 'quickTip', comment: ['"switch to editor language hover" means to show the programming language hover widget instead of the debug hover'] }, 'Hold {0} key to switch to editor language hover', isMacintosh ? 'Option' : 'Alt'); - const dataSource = new DebugHoverDataSource(this.debugService); + const dataSource = this.instantiationService.createInstance(DebugHoverDataSource); const linkeDetector = this.instantiationService.createInstance(LinkDetector); this.tree = >this.instantiationService.createInstance(WorkbenchAsyncDataTree, 'DebugHover', this.treeContainer, new DebugHoverDelegate(), [ this.instantiationService.createInstance(VariablesRenderer, linkeDetector), @@ -414,7 +414,7 @@ class DebugHoverDataSource extends AbstractExpressionDataSource { + protected override doGetChildren(element: IExpression): Promise { return element.getChildren(); } } diff --git a/src/vs/workbench/contrib/debug/browser/variablesView.ts b/src/vs/workbench/contrib/debug/browser/variablesView.ts index ef5e0cfb7d0..03c81f3ff8a 100644 --- a/src/vs/workbench/contrib/debug/browser/variablesView.ts +++ b/src/vs/workbench/contrib/debug/browser/variablesView.ts @@ -126,7 +126,7 @@ export class VariablesView extends ViewPane { new ScopesRenderer(), new ScopeErrorRenderer(), ], - new VariablesDataSource(this.debugService), { + this.instantiationService.createInstance(VariablesDataSource), { accessibilityProvider: new VariablesAccessibilityProvider(), identityProvider: { getId: (element: IExpression | IScope) => element.getId() }, keyboardNavigationLabelProvider: { getKeyboardNavigationLabel: (e: IExpression | IScope) => e.name }, @@ -171,7 +171,7 @@ export class VariablesView extends ViewPane { let horizontalScrolling: boolean | undefined; this._register(this.debugService.getViewModel().onDidSelectExpression(e => { const variable = e?.expression; - if (variable instanceof Variable && !e?.settingWatch) { + if (variable && this.tree.hasNode(variable)) { horizontalScrolling = this.tree.options.horizontalScrolling; if (horizontalScrolling) { this.tree.updateOptions({ horizontalScrolling: false }); @@ -210,12 +210,24 @@ export class VariablesView extends ViewPane { } private onMouseDblClick(e: ITreeMouseEvent): void { - const session = this.debugService.getViewModel().focusedSession; - if (session && e.element instanceof Variable && session.capabilities.supportsSetVariable && !e.element.presentationHint?.attributes?.includes('readOnly') && !e.element.presentationHint?.lazy) { + if (this.canSetExpressionValue(e.element)) { this.debugService.getViewModel().setSelectedExpression(e.element, false); } } + private canSetExpressionValue(e: IExpression | IScope | null): e is IExpression { + const session = this.debugService.getViewModel().focusedSession; + if (!session) { + return false; + } + + if (e instanceof VisualizedExpression) { + return !!e.treeItem.canEdit; + } + + return e instanceof Variable && !e.presentationHint?.attributes?.includes('readOnly') && !e.presentationHint?.lazy; + } + private async onContextMenu(e: ITreeContextMenuEvent): Promise { const variable = e.element; if (!(variable instanceof Variable) || !variable.value) { @@ -415,7 +427,7 @@ export class VisualizedVariableRenderer extends AbstractExpressionsRenderer { */ public static rendererOnVisualizationRange(model: IViewModel, tree: AsyncDataTree): IDisposable { return model.onDidChangeVisualization(({ original }) => { - if (!tree.hasElement(original)) { + if (!tree.hasNode(original)) { return; } @@ -461,24 +473,21 @@ export class VisualizedVariableRenderer extends AbstractExpressionsRenderer { } protected override getInputBoxOptions(expression: IExpression): IInputBoxOptions | undefined { - const variable = expression; + const viz = expression; return { initialValue: expression.value, ariaLabel: localize('variableValueAriaLabel', "Type new variable value"), validationOptions: { - validation: () => variable.errorMessage ? ({ content: variable.errorMessage }) : null + validation: () => viz.errorMessage ? ({ content: viz.errorMessage }) : null }, onFinish: (value: string, success: boolean) => { - variable.errorMessage = undefined; - const focusedStackFrame = this.debugService.getViewModel().focusedStackFrame; - if (success && variable.value !== value && focusedStackFrame) { - variable.setVariable(value, focusedStackFrame) - // Need to force watch expressions and variables to update since a variable change can have an effect on both - .then(() => { - // Do not refresh scopes due to a node limitation #15520 - forgetScopes = false; - this.debugService.getViewModel().updateViews(); - }); + viz.errorMessage = undefined; + if (success) { + viz.edit(value).then(() => { + // Do not refresh scopes due to a node limitation #15520 + forgetScopes = false; + this.debugService.getViewModel().updateViews(); + }); } } }; @@ -494,7 +503,10 @@ export class VisualizedVariableRenderer extends AbstractExpressionsRenderer { createAndFillInContextMenuActions(menu, { arg: context, shouldForwardArgs: false }, { primary, secondary: [] }, 'inline'); if (viz.original) { - primary.push(new Action('debugViz', localize('removeVisualizer', 'Remove Visualizer'), ThemeIcon.asClassName(Codicon.close), undefined, () => this.debugService.getViewModel().setVisualizedExpression(viz.original!, undefined))); + const action = new Action('debugViz', localize('removeVisualizer', 'Remove Visualizer'), ThemeIcon.asClassName(Codicon.eye), true, () => this.debugService.getViewModel().setVisualizedExpression(viz.original!, undefined)); + action.checked = true; + primary.push(action); + actionBar.domNode.style.display = 'initial'; } actionBar.clear(); actionBar.context = context; @@ -601,7 +613,7 @@ export class VariablesRenderer extends AbstractExpressionsRenderer { if (resolved.type === DebugVisualizationType.Command) { viz.execute(); } else { - const replacement = await this.visualization.setVisualizedNodeFor(resolved.id, expression); + const replacement = await this.visualization.getVisualizedNodeFor(resolved.id, expression); if (replacement) { this.debugService.getViewModel().setVisualizedExpression(expression, replacement); } diff --git a/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts b/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts index 9bdf8801c54..8ab1bd56d16 100644 --- a/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts +++ b/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts @@ -93,7 +93,7 @@ export class WatchExpressionsView extends ViewPane { this.instantiationService.createInstance(VariablesRenderer, linkDetector), this.instantiationService.createInstance(VisualizedVariableRenderer, linkDetector), ], - new WatchExpressionsDataSource(this.debugService), { + this.instantiationService.createInstance(WatchExpressionsDataSource), { accessibilityProvider: new WatchExpressionsAccessibilityProvider(), identityProvider: { getId: (element: IExpression) => element.getId() }, keyboardNavigationLabelProvider: { @@ -157,7 +157,7 @@ export class WatchExpressionsView extends ViewPane { let horizontalScrolling: boolean | undefined; this._register(this.debugService.getViewModel().onDidSelectExpression(e => { const expression = e?.expression; - if (expression instanceof Expression || (expression instanceof Variable && e?.settingWatch)) { + if (expression && this.tree.hasElement(expression)) { horizontalScrolling = this.tree.options.horizontalScrolling; if (horizontalScrolling) { this.tree.updateOptions({ horizontalScrolling: false }); @@ -204,7 +204,7 @@ export class WatchExpressionsView extends ViewPane { const element = e.element; // double click on primitive value: open input box to be able to select and copy value. const selectedExpression = this.debugService.getViewModel().getSelectedExpression(); - if (element instanceof Expression && element !== selectedExpression?.expression) { + if ((element instanceof Expression && element !== selectedExpression?.expression) || (element instanceof VisualizedExpression && element.treeItem.canEdit)) { this.debugService.getViewModel().setSelectedExpression(element, false); } else if (!element) { // Double click in watch panel triggers to add a new watch expression @@ -259,7 +259,7 @@ class WatchExpressionsDataSource extends AbstractExpressionDataSource> { + protected override doGetChildren(element: IDebugService | IExpression): Promise> { if (isDebugService(element)) { const debugService = element as IDebugService; const watchExpressions = debugService.getModel().getWatchExpressions(); diff --git a/src/vs/workbench/contrib/debug/common/debug.ts b/src/vs/workbench/contrib/debug/common/debug.ts index 9dcab53e240..039bcc1fb10 100644 --- a/src/vs/workbench/contrib/debug/common/debug.ts +++ b/src/vs/workbench/contrib/debug/common/debug.ts @@ -634,8 +634,9 @@ export interface IViewModel extends ITreeElement { */ readonly focusedStackFrame: IStackFrame | undefined; - setVisualizedExpression(original: IExpression, visualized: IExpression | undefined): void; - getVisualizedExpression(expression: IExpression): IExpression | undefined; + setVisualizedExpression(original: IExpression, visualized: IExpression & { treeId: string } | undefined): void; + /** Returns the visualized expression if loaded, or a tree it should be visualized with, or undefined */ + getVisualizedExpression(expression: IExpression): IExpression | string | undefined; getSelectedExpression(): { expression: IExpression; settingWatch: boolean } | undefined; setSelectedExpression(expression: IExpression | undefined, settingWatch: boolean): void; updateViews(): void; @@ -1265,7 +1266,7 @@ export interface IReplOptions { export interface IDebugVisualizationContext { variable: DebugProtocol.Variable; - containerId?: string; + containerId?: number; frameId?: number; threadId: number; sessionId: string; diff --git a/src/vs/workbench/contrib/debug/common/debugModel.ts b/src/vs/workbench/contrib/debug/common/debugModel.ts index 373ebfa9189..98618a40cf4 100644 --- a/src/vs/workbench/contrib/debug/common/debugModel.ts +++ b/src/vs/workbench/contrib/debug/common/debugModel.ts @@ -246,9 +246,7 @@ function handleSetResponse(expression: ExpressionContainer, response: DebugProto } export class VisualizedExpression implements IExpression { - public readonly name: string; - public readonly hasChildren: boolean; - public readonly value: string; + public errorMessage?: string; private readonly id = generateUuid(); evaluateLazy(): Promise { @@ -262,15 +260,34 @@ export class VisualizedExpression implements IExpression { return this.id; } + get name() { + return this.treeItem.label; + } + + get value() { + return this.treeItem.description || ''; + } + + get hasChildren() { + return this.treeItem.collapsibleState !== DebugTreeItemCollapsibleState.None; + } + constructor( private readonly visualizer: IDebugVisualizerService, - private readonly treeId: string, + public readonly treeId: string, public readonly treeItem: IDebugVisualizationTreeItem, public readonly original?: Variable, - ) { - this.name = treeItem.label; - this.hasChildren = treeItem.collapsibleState !== DebugTreeItemCollapsibleState.None; - this.value = treeItem.description || ''; + ) { } + + /** Edits the value, sets the {@link errorMessage} and returns false if unsuccessful */ + public async edit(newValue: string) { + try { + await this.visualizer.editTreeItem(this.treeId, this.treeItem, newValue); + return true; + } catch (e) { + this.errorMessage = e.message; + return false; + } } } diff --git a/src/vs/workbench/contrib/debug/common/debugViewModel.ts b/src/vs/workbench/contrib/debug/common/debugViewModel.ts index 9c4ee1217ab..4b0959a97a8 100644 --- a/src/vs/workbench/contrib/debug/common/debugViewModel.ts +++ b/src/vs/workbench/contrib/debug/common/debugViewModel.ts @@ -24,6 +24,7 @@ export class ViewModel implements IViewModel { private readonly _onWillUpdateViews = new Emitter(); private readonly _onDidChangeVisualization = new Emitter<{ original: IExpression; replacement: IExpression }>(); private readonly visualized = new WeakMap(); + private readonly preferredVisualizers = new Map(); private expressionSelectedContextKey!: IContextKey; private loadedScriptsSupportedContextKey!: IContextKey; private stepBackSupportedContextKey!: IContextKey; @@ -165,23 +166,33 @@ export class ViewModel implements IViewModel { this.multiSessionDebug.set(isMultiSessionView); } - setVisualizedExpression(original: IExpression, visualized: IExpression | undefined): void { + setVisualizedExpression(original: IExpression, visualized: IExpression & { treeId: string } | undefined): void { const current = this.visualized.get(original) || original; - + const key = this.getPreferredVisualizedKey(original); if (visualized) { this.visualized.set(original, visualized); + this.preferredVisualizers.set(key, visualized.treeId); } else { this.visualized.delete(original); + this.preferredVisualizers.delete(key); } this._onDidChangeVisualization.fire({ original: current, replacement: visualized || original }); } - getVisualizedExpression(expression: IExpression): IExpression | undefined { - return this.visualized.get(expression); + getVisualizedExpression(expression: IExpression): IExpression | string | undefined { + return this.visualized.get(expression) || this.preferredVisualizers.get(this.getPreferredVisualizedKey(expression)); } async evaluateLazyExpression(expression: IExpressionContainer): Promise { await expression.evaluateLazy(); this._onDidEvaluateLazyExpression.fire(expression); } + + private getPreferredVisualizedKey(expr: IExpression) { + return JSON.stringify([ + expr.name, + expr.type, + !!expr.memoryReference, + ].join('\0')); + } } diff --git a/src/vs/workbench/contrib/debug/common/debugVisualizers.ts b/src/vs/workbench/contrib/debug/common/debugVisualizers.ts index 2b171d5f673..45d19aee6f9 100644 --- a/src/vs/workbench/contrib/debug/common/debugVisualizers.ts +++ b/src/vs/workbench/contrib/debug/common/debugVisualizers.ts @@ -79,18 +79,17 @@ export interface IDebugVisualizerService { /** * Sets that a certa tree should be used for the visualized node */ - setVisualizedNodeFor(treeId: string, expr: IExpression): Promise; - - /** - * Gets a visualized node for the given expression if the user has preferred - * to visualize it that way. - */ - getVisualizedNodeFor(expr: IExpression): Promise; + getVisualizedNodeFor(treeId: string, expr: IExpression): Promise; /** * Gets children for a visualized tree node. */ getVisualizedChildren(treeId: string, treeElementId: number): Promise; + + /** + * Gets children for a visualized tree node. + */ + editTreeItem(treeId: string, item: IDebugVisualizationTreeItem, newValue: string): Promise; } const emptyRef: IReference = { object: [], dispose: () => { } }; @@ -101,7 +100,6 @@ export class DebugVisualizerService implements IDebugVisualizerService { private readonly handles = new Map(); private readonly trees = new Map(); private readonly didActivate = new Map>(); - private readonly preferredTrees = new Map(); private registrations: { expr: ContextKeyExpression; id: string; extensionId: ExtensionIdentifier }[] = []; constructor( @@ -126,29 +124,7 @@ export class DebugVisualizerService implements IDebugVisualizerService { return emptyRef; } - const context: IDebugVisualizationContext = { - sessionId: variable.getSession()?.getId() || '', - containerId: variable.parent.getId(), - threadId, - variable: { - name: variable.name, - value: variable.value, - type: variable.type, - evaluateName: variable.evaluateName, - variablesReference: variable.reference || 0, - indexedVariables: variable.indexedVariables, - memoryReference: variable.memoryReference, - namedVariables: variable.namedVariables, - presentationHint: variable.presentationHint, - } - }; - - for (let p: IExpressionContainer = variable; p instanceof Variable; p = p.parent) { - if (p.parent instanceof Scope) { - context.frameId = p.parent.stackFrame.frameId; - } - } - + const context = this.getVariableContext(threadId, variable); const overlay = getContextForVariable(this.contextKeyService, variable, [ [CONTEXT_VARIABLE_NAME.key, variable.name], [CONTEXT_VARIABLE_VALUE.key, variable.value], @@ -205,22 +181,7 @@ export class DebugVisualizerService implements IDebugVisualizerService { } /** @inheritdoc */ - public async setVisualizedNodeFor(treeId: string, expr: IExpression): Promise { - return this.getOrSetNodeFor(expr, treeId); - } - - /** @inheritdoc */ - public async getVisualizedNodeFor(expr: IExpression): Promise { - return this.getOrSetNodeFor(expr); - } - - /** @inheritdoc */ - public async getVisualizedChildren(treeId: string, treeElementId: number): Promise { - const children = await this.trees.get(treeId)?.getChildren(treeElementId) || []; - return children.map(c => new VisualizedExpression(this, treeId, c, undefined)); - } - - private async getOrSetNodeFor(expr: IExpression, useTreeKey?: string): Promise { + public async getVisualizedNodeFor(treeId: string, expr: IExpression): Promise { if (!(expr instanceof Variable)) { return; } @@ -230,37 +191,42 @@ export class DebugVisualizerService implements IDebugVisualizerService { return; } - const exprPreferKey = useTreeKey || this.getPreferredTreeKey(expr); - const tree = exprPreferKey && this.trees.get(exprPreferKey); + const tree = this.trees.get(treeId); if (!tree) { return; } - const treeItem = await tree.getTreeItem(this.getVariableContext(threadId, expr)); - if (!treeItem) { + try { + const treeItem = await tree.getTreeItem(this.getVariableContext(threadId, expr)); + if (!treeItem) { + return; + } + + return new VisualizedExpression(this, treeId, treeItem, expr); + } catch (e) { + this.logService.warn('Failed to get visualized node', e); return; } - - if (useTreeKey) { - this.preferredTrees.set(exprPreferKey, exprPreferKey); - } - - return new VisualizedExpression(this, exprPreferKey, treeItem, expr); } - private getPreferredTreeKey(expr: Variable) { - return JSON.stringify([ - expr.name, - expr.value, - expr.type, - !!expr.memoryReference, - ].join('\0')); + /** @inheritdoc */ + public async getVisualizedChildren(treeId: string, treeElementId: number): Promise { + const children = await this.trees.get(treeId)?.getChildren(treeElementId) || []; + return children.map(c => new VisualizedExpression(this, treeId, c, undefined)); + } + + /** @inheritdoc */ + public async editTreeItem(treeId: string, treeItem: IDebugVisualizationTreeItem, newValue: string): Promise { + const newItem = await this.trees.get(treeId)?.editItem?.(treeItem.id, newValue); + if (newItem) { + Object.assign(treeItem, newItem); // replace in-place so rerenders work + } } private getVariableContext(threadId: number, variable: Variable) { const context: IDebugVisualizationContext = { sessionId: variable.getSession()?.getId() || '', - containerId: variable.parent.getId(), + containerId: (variable.parent instanceof Variable ? variable.reference : undefined), threadId, variable: { name: variable.name, diff --git a/src/vscode-dts/vscode.proposed.debugVisualization.d.ts b/src/vscode-dts/vscode.proposed.debugVisualization.d.ts index 0fc115f7619..cb12eaa00e3 100644 --- a/src/vscode-dts/vscode.proposed.debugVisualization.d.ts +++ b/src/vscode-dts/vscode.proposed.debugVisualization.d.ts @@ -150,7 +150,7 @@ declare module 'vscode' { * that came from user evaluations in the Debug Console. * @see https://microsoft.github.io/debug-adapter-protocol/specification#Types_Variable */ - containerId?: string; + containerId?: number; /** * The ID of the Debug Adapter Protocol StackFrame in which the variable was found, * for variables that came from scopes in a stack frame.