[Debug] Expose the contribution.menus for Variable view

[testforstephen]

The Java Debugger extension received much feedback to support customizing the Variable Formatter. For example:

• Show decimal/hex/oct for int.
• Show logical structure for Array/List/Map/Collections.
• Show toString() for Object.

But current VS Code doesn't provide the frontend UI or the protocol for changing the Variable Format dynamically. See the docs here, https://code.visualstudio.com/api/references/contribution-points#contributes.menus, it doesn't say the extension could contribute context menus to the debug Variable view.

The request is to expose the contribution.menus for Variable view, and carry the hovered variable type info in the menu context. Besides, the DAP should provide some approach to re-request the variable info after dynamically changing the variable format.

[isidorn]

We could easily expose a new debug/variables/context context menu.
The real question is what to pass as context to extension commands such that the extension can identify what variable the command is being operated on? In the variables view we have two element types Varialbes and Scopes.

Here's a proposal:

• For scope we can pass a {name, variableReference}
• For a variable we can pass {name, variableReference}

Not sure if that would be helpful. Could the extension identify the scope just by those, or some id of the stackFrame would be needed as well. I believe the some sort of stack frame ID would be needed
Same for the variable, does it need a parent chain to be added? For example evaluateName? Does it also need a scope id (which would also have the stack frame id).

[testforstephen]

When customize the format, we care more about the variable type instead of name. For different variable types, enable different format options in the context menu.

Here is the updated proposal:

• Don't see the use case for scope yet.
• For a variable, should pass { type, frameId }.
• Provide a VariableEvent for debugger to notify the client to refresh the whole variable list for the specified stack frame.

interface VariableEvent extends Event {
  event: 'variable';

  body: {
    /**
     * The stack frame id.
     */
    frameId: number;
  };
}

With the information above, it will allow the debugger to refresh the whole variables in the specified stack frame with the latest format.

If we want to apply the changed format to the selected variable only, we may need carry more variable details in the menu context. Currently i don't figure out which metadata is enough for the debugger to restore the specified variable instance in the back end. It looks like {name, variableReference} only works for the first level variable in the scope. For the indexed or named variable, we also need the parent variable information, that adds more complexity to the context menu. However, i feel like the case of support refreshing the selected variable format is low priority.

[testforstephen]

@isidorn is there any update on the proposal?

[isidorn]

@testforstephen I have discussed with @weinand and it is still not quite clear how to expose a proper context.
We will definetely not tackle this this milestone and @weinand can comment more

[weinand]

@testforstephen Before we can proceed here, we have to investigate/decide how we can represent the "variable" element to the action in the extension API.

Currently the abstractions used in the DAP are not surfaced anywhere in the extension API. If we start doing that, we have to design the whole DAP surface area. We cannot just surface a "variable".

We can plan this design work for a future milestone but there will be no support for this or the next release.

There is already another debt item for this work: #70742

[testforstephen]

Good to know the status, thanks. I'd love to get involved in the design discussion or anything else. Feel free to bug me.

[lurifaxel]

I would love to see this happen as it would open up for a completely new class of extensions for VS Code! For the use cases I'm thinking of the type, value and name of a variable would be enough but I realize you must consider all kinds of use cases.

Is there any update on this?

[weinand]

This feature request asks for two "sub features":

• enable context menu contributions in the VARIABLES view (predominantly on variables).
• API to make VS Code refetch variables in order to refresh them in the UI.

First a proposal for the context menu contributions:

As suggested by @isidorn the contribution selector is "debug/variables/context". That's the easy part.

More complex is the question what information to pass to the context menu command so that the extension code can identify the underlying variable. DAP's Variable type has all the information for rendering, but it does not uniquely identify the variable because it only knows its name but not the "container" in which it lives. This container is either a Scope object or it is again a Variable.

For this reason I propose to pass the following context object to the menu command (at least conceptually):

{
    container: Scope & Variable;     // intersection of properties of Scope and Variable
    variable: Variable;
}

However, there is a problem with this approach: the Scope and Variable types are defined in DAP and in general we do not surface (duplicate) all DAP types in VS Code's extension API. Instead we introduce opaque stand-in types that don't reveal the real properties but can be easily coerced to the DAP types when needed.
Existing examples for this approach are DebugProtocolMessage, DebugProtocolSource, DebugProtocolBreakpoint. Following this strategy I suggest to introduce two new stand-in types:

/**
 * A DebugProtocolVariableContainer is an opaque stand-in type for the intersection of the Scope and Variable types defined in the Debug Adapter Protocol. See https://microsoft.github.io/debug-adapter-protocol/specification#Types_Scope and https://microsoft.github.io/debug-adapter-protocol/specification#Types_Variable.
 */
export interface DebugProtocolVariableContainer {
  // Properties: the intersection of DAP's Scope and Variable types.
}
/**
 * A DebugProtocolVariable is an opaque stand-in type for the Variable type defined in the Debug Adapter Protocol. See https://microsoft.github.io/debug-adapter-protocol/specification#Types_Variable.
 */
export interface DebugProtocolVariable {
  // Properties: see details [here](https://microsoft.github.io/debug-adapter-protocol/specification#Base_Protocol_Variable).
}

With these definitions the API becomes:

{
    container: DebugProtocolVariableContainer;
    variable: DebugProtocolVariable;
}

Some open issues and design and implementation notes:

1. how is the context object used if a context menu command is run on a top-level Scope object? As {scope, undefined}?
2. is there a root-level context menu command on the "empty space" of the Variables view? If yes, how does the context object look like?
3. when large arrays are split into chunks, we cannot offer context menu actions on the intermediate "chunks" (e.g. the "1..99" nodes) because the chunk can not be expressed as a {container, variable}pair.
4. should we offer to contribute different menu commands for inner nodes and leaf nodes?
5. Since DAP has no explicit "VariableContainer" type, it is not possible to coerce VS Code's stand-in
   DebugProtocolVariableContainer directly into it. Instead an AND-type must be used. We should consider introducing a "VariableContainer" type in DAP which would help here and reduce some duplication in the DAP schema.
6. @testforstephen suggested to pass a type and a frameID into the context menu command. Since we do not want to surface individual DAP properties on the extension API, I've omitted them from the proposal. The type is available from the DebugProtocolVariable after coercing it to DAP's Variable type. The frameId is not easily available but the container's variablesReference together with the variable's name uniquely identify a variable which should make it possible to find its corresponding StackFrame. If this is too cumbersome, we could consider to pass the stack frame as a DebugProtocolStackFrame stand-in:

{
    stackFrame: DebugProtocolStackFrame;
    container: DebugProtocolVariableContainer;
    variable: DebugProtocolVariable;
}

@isidorn @testforstephen I'd appreciate your feedback.

[weinand]

Proposal for API to make VS Code refetch variables:

The original feature request asks for a new DAP event to trigger a refetch and refresh of variables in VS Code. This feature requests already exists as microsoft/debug-adapter-protocol#128 and I suggest to continue the discussion about the DAP request there.

But I think that a DAP mechanism is not enough because context menu commands are implemented in extensions and not in a debug adapter and there must be an easy way to notify the UI about changes that require to refetch variables and update the UI.

@isidorn any idea what we should here?

[isidorn]

@weinand thanks for your proposal, and sorry for the slow response

1. Yes, the container should be DebugProtocolVariableContainer | undefined. When it is undefined that means top level
2. There is no context menu in the VARIABLES view in the empty area. So no need to worry about this
3. There is currently no context menu on the chunk elements. So no need to worry about this
4. I think we should just have a context key which would say if the context menu is on a Scope or on a Variable and based on that the Extensions can decide where to place their commands
5. Makes sense. I actually already have a VariableContainer type on the vscode side, I just call it ExpressionContainer For reference
6. Refetch of variables: feels to me like we should introduce a DAP event VariablesEvent, similar to BreakpointsEvent. reason should be changed for now. In the future we could extend the VariablesEvent to support more things, but for now this feels like the most natural way to go. Alternatives are VS Code API and some hidden VS Code command. I would go for the command if this turns out to be an obscure use case. Edit: now I see that you mentioned that this should be triggered from the Extension. So I would first figure out how many use cases there are, if only one let's go with the command, if multiple let's introduce VS Code api.

Alternative name for container is parent. Though I prefer container.
Also the good thing about passing DAP types is that they are not cyclic - there are no pointers to other objects, thus they are easily JSON serialisable.

[weinand]

@isidorn thanks for your feedback.

4. I agree that it makes sense to have a way to distinguish Scopes and Variables. My question 4 was more about a way to distinguish variable containers ("inner nodes") from simple variables ("leafs"). So we might want to consider a when context for this too. Example: it does not make a lot of sense to open a tabular data view on a simple variable of type integer.
5. Great, then we are already conceptually aligned.
6. A DAP "VariablesEvent" makes sense, but the problem is that DAP Variables have no ID so the event cannot target a specific Variable or Scope. BreakpointsEvent are simpler because a Breakpoint has an optional ID (exactly for that purpose). If we would follow this example, we could introduce an optional Variable ID and could eliminate the variableContainer and stackFrame from the the command context because the ID would uniquely identify the variable. On the other hand are Variables short-living (in contrast to breakpoints which are long-living). So introducing an ID seems to be an overkill because after a single "step" all Variables are gone...
   I suggest that we move this discussion to Support for indicating custom debug focus change debug-adapter-protocol#128.

[isidorn]

4. Makes sense. I think we can introduce different context keys as the need arises. So based on extension needs I can introduce them

Introducing a Variable ID would be great for the UI since I could use it as well for preserving tree expansion state. Though I am not sure how hard it is for adapter to keep these ID's stable between steps. If the ID's are not stable between steps then it is not really an ID...
So I do agree that it is an overkill. For now I suggest to have a simple command debug.refreshVariables that the extensions can use. If it become popular then we make an official API.

Let's continue the discussion there...

[weinand]

@isidorn I suggest we start implementing this proposal:

The contribution selector is debug/variables/context

A "when" context variable viewItem with two values scope-item and variable-item so that a menu contribution can look like this:

  "contributes": {
    "menus": {
      "debug/variables/context": [
        {
          "command": "variables-view.showAsHexValue",
          "when": "debugConfigurationType == 'java' && viewItem == 'variable-item'"
        }
	  ]
    }
  }

I'll add the following new proposed API in vscode.proposed.d.ts:

/**
 * A DebugProtocolVariableContainer is an opaque stand-in type for the intersection of the Scope and Variable types defined in the Debug Adapter Protocol. See https://microsoft.github.io/debug-adapter-protocol/specification#Types_Scope and https://microsoft.github.io/debug-adapter-protocol/specification#Types_Variable.
 */
export interface DebugProtocolVariableContainer {
  // Properties: the intersection of DAP's Scope and Variable types.
}
/**
 * A DebugProtocolVariable is an opaque stand-in type for the Variable type defined in the Debug Adapter Protocol. See https://microsoft.github.io/debug-adapter-protocol/specification#Types_Variable.
 */
export interface DebugProtocolVariable {
  // Properties: see details [here](https://microsoft.github.io/debug-adapter-protocol/specification#Base_Protocol_Variable).
}

And the following context structure will be passed to the commands contributed to debug/variables/context:

{
    container: DebugProtocolVariableContainer;
    variable: DebugProtocolVariable;
}

[isidorn]

@weinand this makes sense. Please let me know once you add the proposed API in vscode.proposed.d.ts. And I can look into adding a new contributable menu and to make sure I pass the appropriate context to the extensions.

[weinand]

@isidorn I've added the two stand-in types DebugProtocolVariableContainer and DebugProtocolVariable to vscode.proposed.d.ts (but I doubt that you actually need them because command arguments are not really typed...).

[testforstephen]

@weinand @isidorn It's a great proposal. One remaining question is how to refresh Variables view with more granularity.

2. is there a root-level context menu command on the "empty space" of the Variables view? If yes, how does the context object look like?

This is useful if allowed to enable menu on "empty space". For example, if i can enable a formatter menu such as "Show hex" on empty space, i would expect to refresh the whole Variables view in current debug session. The context object can be undefined.

4. should we offer to contribute different menu commands for inner nodes and leaf nodes?

Allowed to contribute different menus on different kind of node is also helpful. For example, if the node is a List/Collections/Map object, i would like to add menus such as "Show as Object", "Show as List", etc. This will ask the "when" context key to contain more info. An ideal solution is to let the debugger to return a customized contextValue for each variable in the variable response.

Also, if i enabled a menu on an inner or leaf nodes, i would expect to apply the new formatter to the selected node only. Does debug.refreshVariables command support a specific variable node? I see you guys are discussing about Variable ID, that looks necessary to map the variable object with the UI tree node.

[weinand]

@testforstephen thanks for the feedback

• yes, I agree that having a context menu on the empty space would be a good thing.
• surfacing some property of the DAP Variable or DAP Scope in the "when" context is a great idea. @isidorn could we have a kind of generic "accessor" in the when context so that this becomes possible:

  "contributes": {
    "menus": {
      "debug/variables/context": [
        {
          "command": "variables-view.showAsDataTable",
          "when": "debugConfigurationType == 'java' && viewItem.payload.someCustomJavaProperty == 'array'"
        }
      ]
    }
  }

• for the time being there will be no debug.refreshVariables API because the variable formatting needs to be done in the debug adapter anyway and there we plan (and currently discuss) the variables event (see Support for indicating custom debug focus change debug-adapter-protocol#128)
• we do not plan to introduce a Variable ID.

[isidorn]

As agreed I have pushed a change to allow extensions to contribute to the variables context menu.
The ID of the menu is debug/variables/context.
You can see an image of my starter extension contributing a "Hello World" command.
As @weinand specified above the context passed is

{
    container: DebugProtocolVariableContainer;
    variable: DebugProtocolVariable;
}

Since context menu is not enabled on Scope for now I have decided to not have any context keys.
I will introduce appropriate context keys as the need arrises.
Also currently it is only possible to contribute to the end of the context menu. Not at the beggining. Same as context keys, if the need arrises I can add that.

@testforstephen it would be great if you can give this a try from next week and provide feedback
@weinand I can create a test plan item for this. Let me know what you think.

[isidorn]

@testforstephen looking at your initial comment and after discussing with @weinand it seems like you need the debugProtocolVariableType context. Which we can easily introduce. That would just respect whatever the type is returned by the debug protocol.

[weinand]

Yes, adding a debugProtocolVariableType context would be easy to do. The problem is that we do not have a useful property on the DAP Variable that could be surfaced as the debugProtocolVariableType. And introducing a new "official" property for this is a no-go because we do not want to have properties in the DAP spec that control the UI directly.

A better solution would be to support "custom" properties in "when" clauses.
With this mechanism Java could just add a property "customJavaVariableMenuSelector" with values like "array", "collection", "simple", etc. and the Java extension could add this contribution:

  "contributes": {
    "menus": {
      "debug/variables/context": [
        {
          "command": "variables-view.showAsDataTable",
          "when": "debugConfigurationType == 'java'
                              && viewItem.getCustomProperty('customJavaVariableMenuSelector') == 'array'"
        }
      ]
    }
  }

But this "dynamic" approach is not (yet) supported by VS Code.

Another static approach is to use a fixed but VS Code private property name that is not part of the DAP spec, e.g. "__vscode_menuaction_key". DAP stays clean and debug adapters can still contribute VS Code specific stuff. This is OK because the context menu actions in the package.json are VS Code specific too. And a debug adapter could even check the "clientId" and provide this property only for VS Code.

[testforstephen]

looking at your initial comment and after discussing with @weinand it seems like you need the debugProtocolVariableType context. Which we can easily introduce. That would just respect whatever the type is returned by the debug protocol.

The type context is useful for the primitive types, but not enough for the complex object. Usually the context menu applies to the category, not a specific type. From the type info, it's not easy for Java debugger to restore its category info. Because Java debugger just returns a simple type name for an object, such as List, but the backend requires the fully qualified name such as java.util.List to get accurate the instance. A new property will be more convenient.

Another static approach is to use a fixed but VS Code private property name that is not part of the DAP spec, e.g. "__vscode_menuaction_key". DAP stays clean and debug adapters can still contribute VS Code specific stuff. This is OK because the context menu actions in the package.json are VS Code specific too. And a debug adapter could even check the "clientId" and provide this property only for VS Code.

If introducing a new property officially to DAP is a little too far gone, a private property for VS Code only is OK for me.

@testforstephen it would be great if you can give this a try from next week and provide feedback.

sure, i will find some time at this week to have a try. Since the Variable refresh event is still on discussing, i can only check whether the menus are added, but cannot run the full e2e.

[weinand]

@testforstephen thanks for your feedback.

If introducing a new property officially to DAP is a little too far gone, a private property for VS Code only is OK for me.

Let's use a VS Code specific property then and find good names for the context and the Variable property.

Since we are already using two underscores ("__") as a prefix for VS Code specific properties, I suggest that we extend this prefix to "__vscode" (so that it becomes clear that this is for VS Code only) and then append a "ContextMenuSelector".
So the DAP Variable's property (to be supplied by the debug adapter) is __vscodeContextMenuSelector.

Now the context name:
@isidorn since you had suggested to use debugProtocolVariableType, I propose to just replace the "Type" by "ContextMenuSelector". With this we would arrive at the long but expressive debugProtocolVariableContextMenuSelector.

With this a menu contribution for an "array" Variable would look like this:

  "contributes": {
    "menus": {
      "debug/variables/context": [
        {
          "command": "variables-view.showArrayAsDataTable",
          "when": "debugConfigurationType == 'java' && debugProtocolVariableContextMenuSelector == 'array'"
        }
      ]
    }
  }

(...and if we think that debugProtocolVariableContextMenuSelector is too long, we could eliminate the debugProtocoland use variableContextMenuSelector)

@isidorn @testforstephen what do you think?

[isidorn]

@weinand I like this approach, I am just not sure about the name. I would go with the following
__debugProtocolVariableMenuContext

So instead of contextMenu I would say just menu - in case we introduce inline menus in the future this context key should be respected as well.
I would say context not selector because context is what we usualy use in vscode for arguments that get passed to commands.

Alternative is
__debugProtocolVariableMenuSelector

[weinand]

@isidorn there are actually two names:

• the property name used within the debug adapter: __vscodeContextMenuSelector
• and the corresponding context name: debugProtocolVariableContextMenuSelector

Using your suggestion, we would have these names then:

• the property name used within the debug adapter: __vscodeVariableMenuContext
• and the context name: debugProtocolVariableMenuContext

So the prefix indicates what is targeted ('vscode') and where it does come from ('debugProtocol').

[isidorn]

@weinand yes, that sounds good.

[isidorn]

Let me know if you would like me to add this context key on the vscode side. If you are happy with these names.

[weinand]

@isidorn yes please.
The debugProtocolVariableMenuContext makes it more interesting for debug extensions to give it a try, and we receive more feedback...

[isidorn]

I have tackled this via 4b54344 Try it out and let me know how it goes. So to recap:

• There is a new "hidden" attribute on the debug protocol Variable __vscodeVariableMenuContext which is a string
• There is a new context key on the VS Code side debugProtocolVariableMenuContext which has the content of this hidden variable, or is an empty string when a variable does not have this hidden property set

[testforstephen]

Have a try on the latest insider version, the private property __vscodeVariableMenuContext works well.

[isidorn]

@testforstephen Can you do me a small favor, get latest VS Code insiders and post a new picture of the context menu.
Reason: we have re-ordered the groups and I would like to put a nice picture with proper groups used in our Release Notes :)

More about the groups:

The Variables context menus has the following default groups which the extensions can contribute to:

• navigation: Commands related to navigation across VS Code. It is empty by default. This group always comes first.
• 1_view: Commands related to displaying variables in different view formats. Empty by default.
• 3_modifications: Commands related to modifications of variables. "Set Value" command
• 5_cutcopypaste: Commands related to cutting, copying and pasting of variables. "Copy Value" and "Copy as Expression" commands
• z_commands: Other commands that do not belong to the above categories. "Add to Watch" and "Break on Value changes" commands. This group comes last.

[testforstephen]

@isidorn Below is a new picture with formatter menus under 1_view group.

[isidorn]

@testforstephen Lovely, thanks a lot! That's going in our release notes.
@DonJayamanne let us know your feedback once you also try it out. Thanks!