Support for editing 'accessType' property of data breakpoints

[yannickowow]

Regarding #83649 ( @isidorn )

As discussed in previous issue about DataBreakpoints support, it can be great if VS Code can surface a user interface to create and edit Data Breakpoints regarding to their attributes, such as read, write, readWrite for access type and condition/hitCondition.

Thanks

[isidorn]

@yannickowow thanks for your request and thanks for your PR, I will comment here for both.

Overall I think this makes a lot of sense, however since not a lot of debug extension use data breakpoints I would suggest the following approach:

• Add the ability to edit condition and hit condition of Data breakpoints in VS Code. That way data breakpoints will be aligned with other breakpoints
• Look into making the breakpoints menu contributable. That way extensions can contribute read, write, readWrite commands for data breakpoints

If we decide to go down this approach then my suggestions are the following:

• let's trim down this PR Support for DataBreakpointAccessType condition #117835 to make it minimalistic and that it only adds condition and hit condition commands
• We can decide what should be the context for the breakpoint for contributable commands. It could be a

 { 
type: 'breakpoint' | 'exceptionBreakpoint' | 'functionBreakpoint' | 'dataBreakpoint',
name?: string;
id?: string;

• I can make the breakpoints menu contributable
• With those context the extension can easily identify on what breakpoint the user is triggering the action. And the extension could use the quick pick for input of a command

@weinand @yannickowow Let me know what you think. I personally like this approach because:

1. less code is added to vscode
2. we are opening up breakpoints view to contributions which can benefit other extensions as well

[weinand]

@isidorn Why is there a need for having contributable commands for DataBreakpoint access types?

The DataBreakpointInfoResponses returned by the DataBreakpointInfoRequest are the possible data breakpoint candidates and as such offer a set of 0-3 possible DataBreakpointAccessTypes: none (=write), read, write, readWrite.

Today VS Code only show this context menu action (which is basically for the "write" access type):

When starting to respect the DataBreakpointAccessType we just have to provide the possible menu actions (or a submenu), e.g. "Break when value changes", "Break when value is read", "Break when value is read or changes". Picking one of them just fills in the DataBreakpointAccessType in the newly created data breakpoint.

[yannickowow]

Actually, my PR is "updating" the current Break When Value Changes behaviour depending on DataBreakpointAccessTypes attributes (as you can see in the screenshot in the previous reply from weinand).

In my opinion, adding contributable commands for breakpoints can be great (!), but I do not think it will (or can) replace integrated commands, in order to respect DAP implementations. Removing currently supported commands might suggests a DAP update, or it might suggests that Visual Studio Code will not fully implement this protocol...

To do my implementation, I modified current supported command to be aware of accessTypes attributes. This issue was also mentioned here (#83649) :

• VS Code didn't respect the accessTypes attribute of DataBreakpointInfoResponse. When it returns an array ['read', 'write', 'readWrite'], right click the variable, the menu only shows Break When Value Changes.

When AccessTypes returns ['readWrite', 'write']

When returning ['read']

I did assume the previous implementation was a "default" one, and then returning an empty array is equivalent to "write" ("Break When Value Changes")

Regards.

[weinand]

@yannickowow Yes, a missing "accessTypes" property or an empty array means "write".
So what you did for the Variable view's context menu looks good to me (and is all we need).

And you are right, today VS Code does not fully implement the spec.

(Note to myself: why is there a "readWrite" value if "accessTypes" is already an array which allows to specify ["read", "write"]...)

[isidorn]

I do agree with what you are saying, that in order for VS Code to fully implement the Spec these should be integrated commands.
What I was trying to say is: I am not sure how popular these commands are going to be, and VS Code already does not fully implement the DAP spec, some parts of the DAP spec we simply do not add to VS Code.
So my question is how important is it that VS Code implements the full DAP data breakpoints spec?

I leave this decision to @weinand, just my hunch was that this was better suited in an extension and that the initial Data breakpoint support that we already have is enough - I did not get a single user (not extension writer) issue regarding data breakpoints for example so I am doubting their popularity. Once users start using data breakpoints I would be more passionate about adding further data breakpoints features. Though I might be wrong, just sharing my thoughts.

[weinand]

@isidorn The "Julia" team is asking for "accessTypes" support. I will dig out the issue...

From the VS Code roadmap:

Expose more UI for DAP features that are currently not surfaced in the VS Code debugging UI.

In this specific case I think the missing functionality of our current implementation is very small: and optionally showing the three missing actions does not involve lots of code. On the contrary: delegating the implementation to an extension would require much more effort on our side because of missing extension APIs.

[isidorn]

Makes sense to me. I was just raising my concerns.
Anyways let me know how I can best help here.

[yannickowow]

Actually, I may have some questions about updating accessType attribute in DataBreakpoint...
When modifying a DataBreakpoint, since accessPoint is unique, I don't know if it is ok to show it with "check" rather than a possible radiobutton...

[weinand]

@isidorn it was not the "Julia" team asking for this but "Java": it is the first item on this list of DataBreakpoint issues #83649