From 748050fb1ae2529bce2f5f7cdfe8f382f0f35029 Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Fri, 31 Oct 2025 12:04:34 -0700 Subject: [PATCH 1/5] Support data breakpoint queries with frameId and improve watch expression data access - Add optional frameId parameter to IDebugSession.dataBreakpointInfo and implementations (DebugSession, MockSession) so adapters can resolve data breakpoint info with frame context. - Update watch expressions context logic to pass the DebugService into getContextForWatchExpressionMenuWithDataAccess. - Enhance data breakpoint lookup for watch items: - For Variable instances, ensure parent children are fetched and pass the parent's variablesReference. - For expressions, pass frameId for complex expressions (e.g. contains '.' or '[') so adapters can parse them. - For simple variable names, fetch the local scope's reference and variables before querying. - Wrap calls in try/catch to silently continue if dataBreakpointInfo fails. --- .../contrib/debug/browser/debugSession.ts | 4 +- .../debug/browser/watchExpressionsView.ts | 60 +++++++++++++++++-- .../workbench/contrib/debug/common/debug.ts | 2 +- .../contrib/debug/test/common/mockDebug.ts | 2 +- 4 files changed, 59 insertions(+), 9 deletions(-) diff --git a/src/vs/workbench/contrib/debug/browser/debugSession.ts b/src/vs/workbench/contrib/debug/browser/debugSession.ts index 74ec0c9785b..34cf1145a7a 100644 --- a/src/vs/workbench/contrib/debug/browser/debugSession.ts +++ b/src/vs/workbench/contrib/debug/browser/debugSession.ts @@ -566,8 +566,8 @@ export class DebugSession implements IDebugSession { return this._dataBreakpointInfo({ name: address, bytes, asAddress: true }); } - dataBreakpointInfo(name: string, variablesReference?: number): Promise<{ dataId: string | null; description: string; canPersist?: boolean } | undefined> { - return this._dataBreakpointInfo({ name, variablesReference }); + dataBreakpointInfo(name: string, variablesReference?: number, frameId?: number): Promise<{ dataId: string | null; description: string; canPersist?: boolean } | undefined> { + return this._dataBreakpointInfo({ name, variablesReference, frameId }); } private async _dataBreakpointInfo(args: DebugProtocol.DataBreakpointInfoArguments): Promise<{ dataId: string | null; description: string; canPersist?: boolean } | undefined> { diff --git a/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts b/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts index abbfb66f5f8..c2afc2ccd53 100644 --- a/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts +++ b/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts @@ -221,15 +221,14 @@ export class WatchExpressionsView extends ViewPane implements IDebugViewWithVari const selection = this.tree.getSelection(); - const contextKeyService = element && await getContextForWatchExpressionMenuWithDataAccess(this.contextKeyService, element); + const contextKeyService = element && await getContextForWatchExpressionMenuWithDataAccess(this.contextKeyService, element, this.debugService); const menu = this.menuService.getMenuActions(MenuId.DebugWatchContext, contextKeyService, { arg: element, shouldForwardArgs: false }); const { secondary } = getContextMenuActions(menu, 'inline'); - // const actions = getFlatContextMenuActions(this.menu.getActions({ arg: element, shouldForwardArgs: true })); this.contextMenuService.showContextMenu({ getAnchor: () => e.anchor, getActions: () => secondary, - getActionsContext: () => element && selection.includes(element) ? selection : element ? [element] : [], + getActionsContext: () => element && selection.includes(element) ? selection : element ? [element] : [] }); } } @@ -409,14 +408,65 @@ function getContextForWatchExpressionMenu(parentContext: IContextKeyService, exp /** * Gets a context key overlay that has context for the given expression, including data access info. */ -async function getContextForWatchExpressionMenuWithDataAccess(parentContext: IContextKeyService, expression: IExpression) { +async function getContextForWatchExpressionMenuWithDataAccess(parentContext: IContextKeyService, expression: IExpression, debugService: IDebugService) { const session = expression.getSession(); if (!session || !session.capabilities.supportsDataBreakpoints) { return getContextForWatchExpressionMenu(parentContext, expression); } const contextKeys: [string, unknown][] = []; - const dataBreakpointInfoResponse = await session.dataBreakpointInfo('evaluateName' in expression ? expression.evaluateName as string : expression.name); + + // Ensure variables are fetched from their container so the debug adapter knows about them. + // For Variables (nested items like s.x), the container is the parent expression. + // For Expressions (top-level items like s), the container is the local scope. + const stackFrame = debugService.getViewModel().focusedStackFrame; + let dataBreakpointInfoResponse; + + try { + if (expression instanceof Variable) { + // For nested variables, ensure the parent's children have been fetched + if ('getChildren' in expression.parent && typeof expression.parent.getChildren === 'function') { + await expression.parent.getChildren(); + } + + dataBreakpointInfoResponse = await session.dataBreakpointInfo(expression.name, expression.parent.reference); + } else { + const expressionName = 'evaluateName' in expression ? expression.evaluateName as string : expression.name; + + // For simple expressions (variable names), use the local scope reference + // For complex expressions (like s.x), pass frameId and let the adapter parse it + if (expressionName.includes('.') || expressionName.includes('[')) { + // Complex expression - pass frameId for context, adapter may parse the expression + dataBreakpointInfoResponse = await session.dataBreakpointInfo( + expressionName, + undefined, + stackFrame?.frameId + ); + } else { + // Simple variable name - get it from the local scope + let scopeReference = 0; + if (stackFrame) { + const scopes = await stackFrame.getScopes(); + const localScope = scopes.find(s => !s.expensive); // Local scope is typically not expensive + if (localScope) { + // Fetch the variables from the scope to ensure the debug adapter knows about them. + // getChildren() is cached, so this won't make duplicate requests if already fetched. + await localScope.getChildren(); + scopeReference = localScope.reference || 0; + } + } + + dataBreakpointInfoResponse = await session.dataBreakpointInfo( + expressionName, + scopeReference + ); + } + } + } catch (error) { + // If dataBreakpointInfo fails (e.g., expression not found, invalid expression), + // silently continue without data breakpoint support for this item + } + const dataBreakpointId = dataBreakpointInfoResponse?.dataId; const dataBreakpointAccessTypes = dataBreakpointInfoResponse?.accessTypes; setDataBreakpointInfoResponse(dataBreakpointInfoResponse); diff --git a/src/vs/workbench/contrib/debug/common/debug.ts b/src/vs/workbench/contrib/debug/common/debug.ts index 385ce2bfff9..ad83f3a447a 100644 --- a/src/vs/workbench/contrib/debug/common/debug.ts +++ b/src/vs/workbench/contrib/debug/common/debug.ts @@ -445,7 +445,7 @@ export interface IDebugSession extends ITreeElement, IDisposable { sendBreakpoints(modelUri: uri, bpts: IBreakpoint[], sourceModified: boolean): Promise; sendFunctionBreakpoints(fbps: IFunctionBreakpoint[]): Promise; - dataBreakpointInfo(name: string, variablesReference?: number): Promise; + dataBreakpointInfo(name: string, variablesReference?: number, frameId?: number): Promise; dataBytesBreakpointInfo(address: string, bytes: number): Promise; sendDataBreakpoints(dbps: IDataBreakpoint[]): Promise; sendInstructionBreakpoints(dbps: IInstructionBreakpoint[]): Promise; diff --git a/src/vs/workbench/contrib/debug/test/common/mockDebug.ts b/src/vs/workbench/contrib/debug/test/common/mockDebug.ts index e559c23fa11..0007966084a 100644 --- a/src/vs/workbench/contrib/debug/test/common/mockDebug.ts +++ b/src/vs/workbench/contrib/debug/test/common/mockDebug.ts @@ -235,7 +235,7 @@ export class MockSession implements IDebugSession { throw new Error('Method not implemented.'); } - dataBreakpointInfo(name: string, variablesReference?: number | undefined): Promise<{ dataId: string | null; description: string; canPersist?: boolean | undefined } | undefined> { + dataBreakpointInfo(name: string, variablesReference?: number | undefined, frameId?: number | undefined): Promise<{ dataId: string | null; description: string; canPersist?: boolean | undefined } | undefined> { throw new Error('Method not implemented.'); } From 1b2a2d28d89b62444165c6145572007cd90aa7c4 Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Tue, 4 Nov 2025 16:01:56 -0800 Subject: [PATCH 2/5] revert the logic resolving scope reference. --- .../debug/browser/watchExpressionsView.ts | 55 +++---------------- 1 file changed, 8 insertions(+), 47 deletions(-) diff --git a/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts b/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts index c2afc2ccd53..9e3a4847d27 100644 --- a/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts +++ b/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts @@ -23,6 +23,7 @@ import { ContextKeyExpr, IContextKey, IContextKeyService } from '../../../../pla import { IContextMenuService, IContextViewService } from '../../../../platform/contextview/browser/contextView.js'; import { IHoverService } from '../../../../platform/hover/browser/hover.js'; import { IInstantiationService, ServicesAccessor } from '../../../../platform/instantiation/common/instantiation.js'; +import { ILogService } from '../../../../platform/log/common/log.js'; import { IKeybindingService } from '../../../../platform/keybinding/common/keybinding.js'; import { KeybindingWeight } from '../../../../platform/keybinding/common/keybindingsRegistry.js'; import { WorkbenchAsyncDataTree } from '../../../../platform/list/browser/listService.js'; @@ -68,7 +69,8 @@ export class WatchExpressionsView extends ViewPane implements IDebugViewWithVari @IOpenerService openerService: IOpenerService, @IThemeService themeService: IThemeService, @IHoverService hoverService: IHoverService, - @IMenuService private readonly menuService: IMenuService + @IMenuService private readonly menuService: IMenuService, + @ILogService private readonly logService: ILogService ) { super(options, keybindingService, contextMenuService, configurationService, contextKeyService, viewDescriptorService, instantiationService, openerService, themeService, hoverService); @@ -221,7 +223,7 @@ export class WatchExpressionsView extends ViewPane implements IDebugViewWithVari const selection = this.tree.getSelection(); - const contextKeyService = element && await getContextForWatchExpressionMenuWithDataAccess(this.contextKeyService, element, this.debugService); + const contextKeyService = element && await getContextForWatchExpressionMenuWithDataAccess(this.contextKeyService, element, this.debugService, this.logService); const menu = this.menuService.getMenuActions(MenuId.DebugWatchContext, contextKeyService, { arg: element, shouldForwardArgs: false }); const { secondary } = getContextMenuActions(menu, 'inline'); @@ -408,63 +410,22 @@ function getContextForWatchExpressionMenu(parentContext: IContextKeyService, exp /** * Gets a context key overlay that has context for the given expression, including data access info. */ -async function getContextForWatchExpressionMenuWithDataAccess(parentContext: IContextKeyService, expression: IExpression, debugService: IDebugService) { +async function getContextForWatchExpressionMenuWithDataAccess(parentContext: IContextKeyService, expression: IExpression, debugService: IDebugService, logService: ILogService) { const session = expression.getSession(); if (!session || !session.capabilities.supportsDataBreakpoints) { return getContextForWatchExpressionMenu(parentContext, expression); } const contextKeys: [string, unknown][] = []; - - // Ensure variables are fetched from their container so the debug adapter knows about them. - // For Variables (nested items like s.x), the container is the parent expression. - // For Expressions (top-level items like s), the container is the local scope. const stackFrame = debugService.getViewModel().focusedStackFrame; let dataBreakpointInfoResponse; try { - if (expression instanceof Variable) { - // For nested variables, ensure the parent's children have been fetched - if ('getChildren' in expression.parent && typeof expression.parent.getChildren === 'function') { - await expression.parent.getChildren(); - } - - dataBreakpointInfoResponse = await session.dataBreakpointInfo(expression.name, expression.parent.reference); - } else { - const expressionName = 'evaluateName' in expression ? expression.evaluateName as string : expression.name; - - // For simple expressions (variable names), use the local scope reference - // For complex expressions (like s.x), pass frameId and let the adapter parse it - if (expressionName.includes('.') || expressionName.includes('[')) { - // Complex expression - pass frameId for context, adapter may parse the expression - dataBreakpointInfoResponse = await session.dataBreakpointInfo( - expressionName, - undefined, - stackFrame?.frameId - ); - } else { - // Simple variable name - get it from the local scope - let scopeReference = 0; - if (stackFrame) { - const scopes = await stackFrame.getScopes(); - const localScope = scopes.find(s => !s.expensive); // Local scope is typically not expensive - if (localScope) { - // Fetch the variables from the scope to ensure the debug adapter knows about them. - // getChildren() is cached, so this won't make duplicate requests if already fetched. - await localScope.getChildren(); - scopeReference = localScope.reference || 0; - } - } - - dataBreakpointInfoResponse = await session.dataBreakpointInfo( - expressionName, - scopeReference - ); - } - } + const expressionName = 'evaluateName' in expression ? expression.evaluateName as string : expression.name; + dataBreakpointInfoResponse = await session.dataBreakpointInfo(expressionName, undefined, stackFrame?.frameId); } catch (error) { - // If dataBreakpointInfo fails (e.g., expression not found, invalid expression), // silently continue without data breakpoint support for this item + logService.error('Failed to get data breakpoint info for watch expression:', error); } const dataBreakpointId = dataBreakpointInfoResponse?.dataId; From 49820f38c535ab80d8ff45edf9585b5473d6c70f Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Tue, 4 Nov 2025 17:07:37 -0800 Subject: [PATCH 3/5] Include parent reference when requesting data breakpoint info for watch variables --- .../debug/browser/watchExpressionsView.ts | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts b/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts index 9e3a4847d27..96504d51ad8 100644 --- a/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts +++ b/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts @@ -421,8 +421,22 @@ async function getContextForWatchExpressionMenuWithDataAccess(parentContext: ICo let dataBreakpointInfoResponse; try { - const expressionName = 'evaluateName' in expression ? expression.evaluateName as string : expression.name; - dataBreakpointInfoResponse = await session.dataBreakpointInfo(expressionName, undefined, stackFrame?.frameId); + // Per DAP spec: + // - For Variables (children in a tree): use variable.name + parent's variablesReference + // - For Expressions (top-level items): use expression string (evaluateName preferred) + frameId + if (expression instanceof Variable) { + dataBreakpointInfoResponse = await session.dataBreakpointInfo( + expression.name, + expression.parent.reference, + stackFrame?.frameId + ); + } else { + dataBreakpointInfoResponse = await session.dataBreakpointInfo( + 'evaluateName' in expression ? expression.evaluateName as string : expression.name, + undefined, // No parent reference for top-level expressions + stackFrame?.frameId + ); + } } catch (error) { // silently continue without data breakpoint support for this item logService.error('Failed to get data breakpoint info for watch expression:', error); From 0e4feffcc66dac9150d85e5dc46d45eee05dfc57 Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Thu, 6 Nov 2025 09:48:28 -0800 Subject: [PATCH 4/5] Prefer evaluateName for dataBreakpointInfo; use parent.reference for Variables; downgrade error log to trace --- .../debug/browser/watchExpressionsView.ts | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts b/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts index 96504d51ad8..e3501399168 100644 --- a/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts +++ b/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts @@ -422,24 +422,35 @@ async function getContextForWatchExpressionMenuWithDataAccess(parentContext: ICo try { // Per DAP spec: - // - For Variables (children in a tree): use variable.name + parent's variablesReference - // - For Expressions (top-level items): use expression string (evaluateName preferred) + frameId - if (expression instanceof Variable) { + // - If evaluateName is available: use it as an expression (top-level evaluation) + // - Otherwise, check if it's a Variable: use name + parent reference (container-relative) + // - Otherwise: use name as an expression + if ('evaluateName' in expression && expression.evaluateName) { + // Use evaluateName if available (more precise for evaluation context) + dataBreakpointInfoResponse = await session.dataBreakpointInfo( + expression.evaluateName as string, + undefined, + stackFrame?.frameId + ); + } else if (expression instanceof Variable) { + // Variable without evaluateName: use name relative to parent container dataBreakpointInfoResponse = await session.dataBreakpointInfo( expression.name, expression.parent.reference, stackFrame?.frameId ); } else { + // Expression without evaluateName: use name as the expression to evaluate dataBreakpointInfoResponse = await session.dataBreakpointInfo( - 'evaluateName' in expression ? expression.evaluateName as string : expression.name, - undefined, // No parent reference for top-level expressions + expression.name, + undefined, stackFrame?.frameId ); } } catch (error) { + // If dataBreakpointInfo fails (e.g., expression not found, invalid expression), // silently continue without data breakpoint support for this item - logService.error('Failed to get data breakpoint info for watch expression:', error); + logService.trace('Failed to get data breakpoint info for watch expression:', error); } const dataBreakpointId = dataBreakpointInfoResponse?.dataId; From a78fef71a424d48a7abe902c309f35192b629dee Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Thu, 6 Nov 2025 09:49:28 -0800 Subject: [PATCH 5/5] Log failure to get data breakpoint info as error for watch expressions --- src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts b/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts index e3501399168..195fa783b9c 100644 --- a/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts +++ b/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts @@ -448,9 +448,8 @@ async function getContextForWatchExpressionMenuWithDataAccess(parentContext: ICo ); } } catch (error) { - // If dataBreakpointInfo fails (e.g., expression not found, invalid expression), // silently continue without data breakpoint support for this item - logService.trace('Failed to get data breakpoint info for watch expression:', error); + logService.error('Failed to get data breakpoint info for watch expression:', error); } const dataBreakpointId = dataBreakpointInfoResponse?.dataId;