Fix watch view context menu when Variables pane is hidden

[bryanchen-d]

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):

• Fetch the parent's children to populate adapter state
• Pass the parent's variablesReference

For simple expressions (like s or a):

• Get the local scope from the focused stack frame
• Fetch the scope's variables to populate adapter state
• Pass the scope's variablesReference

For complex expressions (like s.x):

• Pass the expression with frameId and let the adapter parse it
• Fall back gracefully if not supported

Error handling:

• Wrap dataBreakpointInfo() calls in try-catch
• On failure, silently continue without data breakpoint menu items

[Copilot]

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 dataBreakpointInfo method signature across interfaces, implementations, and mocks to include the optional frameId parameter
• Implements logic to properly pass frameId for complex expressions or variablesReference for simple variables when requesting data breakpoint information
• Adds error handling to silently continue if dataBreakpointInfo fails
• 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

[Copilot]

The expression detection logic using simple string contains checks for '.' and '[' is fragile. This approach won't correctly handle all valid JavaScript/TypeScript expressions (e.g., member access via bracket notation with complex keys, or variables with these characters in their names). Consider using a more robust expression parsing method or documenting the limitations of this approach.

[Copilot]

The logic to find the local scope by checking !s.expensive is unreliable and based on an assumption that may not hold for all debug adapters. Consider using a more explicit identifier for the local scope (e.g., checking scope name or type) or documenting this assumption more clearly with its potential failure cases.

Suggested change

	const localScope = scopes.find(s => !s.expensive); // Local scope is typically not expensive
	let localScope = scopes.find(s => typeof s.name === 'string' && s.name.toLowerCase() === 'local');
	if (!localScope) {
	localScope = scopes.find(s => typeof s.name === 'string' && s.name.toLowerCase() === 'locals');
	}
	// Fallback: if not found by name, use the old logic (not expensive)
	if (!localScope) {
	localScope = scopes.find(s => !s.expensive);
	}

[connor4312]

I think passing the frameId is good but I'm confused why we need to look at scopes or check if the expression is nested. evaluateName is always expected to be based on the stackframe, so a scope reference is not needed. If there isn't an evaluate name, we should evaluate expression.name in the parent context which is expression.parent.

[bryanchen-d]

When the Variables window is off, the scopes are not populated so here we have to populate the scope and pass it in.
From my test, if we don't pass a scope reference here, the dataBreakpointInfo() throws "Unable to find child property".

Regarding the expression.parent, from my debug, it is undefined for a top-level expression like s.x (accessing a member variable in a struct object) or a (a simple variable)

[connor4312]

I'm still not sure why we need scopes. There's nothing in DAP that ties scopes to the dataBreakpointInfo. It needs only a name, and optionally variablesReference/frameId. We didn't pass the frameId before, and you fixed that here, which I think is good.

Regarding the expression.parent, from my debug, it is undefined for a top-level expression like s.x (accessing a member variable in a struct object) or a (a simple variable)

Yea, that is intended. It's only defined for children of DAP Variables.

[connor4312]

comment