Fix watch view context menu when Variables pane is hidden #274414
Conversation
…sion 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.
bryanchen-d
requested review from
connor4312,
Copilot and
roblourens
and removed request for
Copilot
There was a problem hiding this comment.
Pull Request Overview
This pull request adds support for the optional frameId parameter to the dataBreakpointInfo method in the debug adapter protocol. This allows the debug adapter to resolve data breakpoint information for expressions within the context of a specific stack frame.
- Updates the
dataBreakpointInfomethod signature across interfaces, implementations, and mocks to include the optionalframeIdparameter - Implements logic to properly pass
frameIdfor complex expressions orvariablesReferencefor simple variables when requesting data breakpoint information - Adds error handling to silently continue if
dataBreakpointInfofails - Removes trailing comma in a function call
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/vs/workbench/contrib/debug/common/debug.ts | Updates the IDebugSession.dataBreakpointInfo method signature to include the optional frameId parameter |
| src/vs/workbench/contrib/debug/browser/debugSession.ts | Updates the dataBreakpointInfo implementation to accept and pass the frameId parameter to the underlying debug adapter |
| src/vs/workbench/contrib/debug/test/common/mockDebug.ts | Updates the mock implementation to match the new method signature |
| src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts | Implements the logic to determine when to use variablesReference vs frameId based on expression type, adds error handling, and removes a trailing comma |
| } | ||
| } | ||
| const expressionName = 'evaluateName' in expression ? expression.evaluateName as string : expression.name; | ||
| dataBreakpointInfoResponse = await session.dataBreakpointInfo(expressionName, undefined, stackFrame?.frameId); |
There was a problem hiding this comment.
you had another change we should keep here:
- If there's an
expression.evaluateName, then calling it asevaluateName, undefined, frameIdis good - If there's no
expression.evaluateName, we should call it asname, expression.parent.reference, frameIdwhen available
evaluateName is absolute, but variable.name is relative to its parent, so the parent reference should be preserved when possible
There was a problem hiding this comment.
From the strong typing and also from my debugging, expression.parent is only avaiable when expression instanceof Variable is true. Can we assume when there is no evaluateName , the expression is a Variable?
There was a problem hiding this comment.
From what I see in debugging, when there is a top level expression, for example, if user type s or s.x in the expression, there is no evaluateName, nor expression.parent.
There was a problem hiding this comment.
For the top level expression, there will not be an expression.parent. For properties of the object/expression result, there will be a parent.
expression.parent is only avaiable when expression instanceof Variable
Yes, if this is the case we should pass in the parent refrence when there is no evaluateName
Problem
The context menu in the Watch view fails to appear during debugging when the Variables pane is hidden/collapsed. This prevents users from accessing debugging actions like "Break on Value Change", "Copy Value", etc.
Fixes: #273721
Root Cause
When the Variables pane is hidden, we never fetches scope variables from the debug adapter. When the Watch view tries to get data breakpoint info for expressions, the debug adapter doesn't have the necessary variable context and throws "Unable to find child property" errors.
Solution
Before requesting data breakpoint info, ensure the debug adapter has the necessary context:
For nested variable
s(like x within expanded s):variablesReferenceFor simple expressions (like
sora):variablesReferenceFor complex expressions (like
s.x):frameIdand let the adapter parse itError handling:
dataBreakpointInfo()calls in try-catch