Allow debug extensions to show diagnostic information in the breakpoints view

[connor4312]

js-debug has a breakpoint helper to diagnose issues. It's there and there's some heuristics for when we show a notification, but notifications are disruptive, thus the heuristics are conservative, and thus not many people know about the tool. Something that could be helpful is a contribution point that would show markdown text below the breakpoints in the BREAKPOINTS view. js-debug could embed a command link there to open the diagnostic tool, and use a when clause to show it at appropriate times.

[rebornix]

👍

I was bugging @connor4312 for unbound breakpoints in a project which has a wrong webpack output config. Due to the misconfig, the source maps are wrong. It would be helpful the unbound breakpoint has some tooltip/hover as suggested above and send to me a diagnostic/troubleshooting view.

[roblourens]

I think a link in the breakpoint hover message would be more discoverable

[rebornix]

The hover message of breakpoint is markdown, we can double check if it's command enabled.

[roblourens]

Is it ok to just allow command links in the markdown hover @connor4312 ? I think that's discoverable, I hardly ever look at the breakpoints view in the debug pane.

[weinand]

The initial comment proposed to "show markdown text below the breakpoints in the BREAKPOINTS view" which seems to be discoverable.

Having the additional information in the breakpoint hover makes sense too, but that location is not very discoverable.
Are users really hovering over unbound (=grey) breakpoints to read the hover message?

If we go this route we should use a visual cue (e.g. a small 'i' badge in one corner of the grey breakpoint icon) to make users aware of the additional info. For this we might have to extend DAP so that a client can detect that there is "additional info".

[roblourens]

Are users really hovering over unbound (=grey) breakpoints to read the hover message?

To me this is a natural thing to do, like hovering squiggles in the editor. When something is confusing, I hover it to get more context. To me it's less natural to set a breakpoint, see that it didn't work, then go find a different view to get context on it.

[roblourens]

On the other hand, command links in breakpoint messages are a vscode-specific concept in DAP, we might not like that.

We can explore the breakpoints view text direction too. Are there any other uses for it except this kind of hint?

[weinand]

I'm not saying that we should show the additional information in the breakpoints view only. I'd like the additional info in the breakpoint hover.

My point was that most VS Code debugger users won't hover over grey breakpoints in the gutter because they already know that it means "unbound" and they do not expect to find more information there. This is different to problem icons where only the hover reveals the real info.

An 'i' badge on the breakpoint icon could help here.

[roblourens]

If we want info in the breakpoint hover, we should add a flag to Breakpoint in DAP to indicate whether the message is markdown. Can look at MarkdownString in vscode.d.ts

The message also shows up in a native tooltip in the breakpoints view, so I'd want to switch to the custom hover for that tree, I guess.

[roblourens]

Here's a mockup of some text in the breakpoints view.

I think this would have to be a new contribution point, with js-debug contributing a static message and turning it on and off with a context key. It doesn't fit into viewsWelcome. It doesn't make sense to do this as a one-off just for the breakpoint view, so does anyone know of an API or API proposal that is similar to this?

[weinand]

some questions/comments:

• @connor4312 do you envision the "diagnostic information" in the future become specific for individual breakpoints ("fix this breakpoint") or will it continue to be just a link that points to a single web page or tool? The answer to this question determines whether we should focus on showing "diagnostic information" per breakpoint or for the breakpoints view as a whole.
• @roblourens I assume that the message is shown in the breakpoints view when a debug session starts and if at least one breakpoint can not be validated. And the message will disappear when the sessions end. This will result in flicker because all the breakpoint options ("Caught Exceptions" etc.) and the list of breakpoints will shift down/up. If we would address issue #126608, then even more flicker will occur when switching between debug sessions.
• @roblourens Showing at the top of the breakpoints view has the (minor) problem that the message is shown above the breakpoint options but it applies to breakpoints listed below the breakpoint options. This might be confusing.

BTW, here is the original discussion for "Improving discoverability of js-debug diagnostic tool" and the corresponding js-debug feature "diagnostic tool".

[connor4312]

do you envision the "diagnostic information" in the future become specific for individual breakpoints ("fix this breakpoint") or will it continue to be just a link that points to a single web page or tool? The answer to this question determines whether we should focus on showing "diagnostic information" per breakpoint or for the breakpoints view as a whole.

The latter.

Re previous discussion: I also have never intentionally hovered over an unbound breakpoint, but maybe that's just me. I was invisioning text in the view like what Rob has in his screenshots.

I assume that the message is shown in the breakpoints view when a debug session starts and if at least one breakpoint can not be validated. And the message will disappear when the sessions end. This will result in flicker because all the breakpoint options ("Caught Exceptions" etc.) and the list of breakpoints will shift down/up. If we would address issue #126608, then even more flicker will occur when switching between debug sessions.

I think this should be a package.json contribution with visibility driven by a when clause. That would avoid flicker and would let debuggers do their best to show the text only when they 'know' there's something wrong.

[weinand]

@connor4312 how would that "when" clause look like?

Please note that breakpoints in the breakpoints view are owned by VS Code and are not associated with a particular debugger. So there could be two concurrent debug sessions (e.g. one Python, one js-debug) and both Python and JS/TS breakpoints...

[connor4312]

In js-debug I would probably use some custom context key that I set from the extension side when I detect things are amiss.

Please note that breakpoints in the breakpoints view are owned by VS Code and are not associated with a particular debugger. So there could be two concurrent debug sessions (e.g. one Python, one js-debug) and both Python and JS/TS breakpoints...

Indeed. I don't have a good answer for this, maybe the message is shown below or otherwise visually associated with the first unbound breakpoint from that extension?

[weinand]

When designing the semantics of the new contribution point, we should consider VS Code's possible strategies around the fix for #126608:

• with fix: VS Code could automatically switch the message based on the active debug session.
• without fix: VS Code could show the message of the most recently started debug session (this is basically what we do today for the exception options shown in the Breakpoint view).

In both cases the "when" clause should only express the condition where "things are amiss", but not when to show/hide.

Maybe it is even better to not support a "when" clause at all and just provide extension API like "showMessage"/"clearMessage"

[roblourens]

Thanks for pointing out that other issue.

A dynamic showMessage API feels better to me than an entire contribution point, if it doesn't generalize more. Maybe on DebugSession. Or would it make any sense to have this in DAP? There could be a message on the SetBreakpointsResponse.

[weinand]

I think we have 4 options, 3 DAP-based and 1 extension-API based:

• DAP: the SetBreakpoints request returns Breakpoint objects with information about the verified or unverified breakpoints. Every Breakpoint object has a message property with this description: "An optional message about the state of the breakpoint. This is shown to the user and can be used to explain why a breakpoint could not be verified." Using this message would require a diagnostic information UI per breakpoint (as pointed out by @connor4312 this approach is too fine grained for the breakpoint diagnostic information provided by js-debug).
• DAP: as suggested by @roblourens we could introduce a message property on the SetBreakpointsResponse but this request is still per resource (file). So multiple messages could be returned when starting debugging and a UI would have to deal with that. So this is again too fine grained for the breakpoint diagnostic information provided by js-debug.
• DAP: we could introduce a new DA capability that provides a single piece of diagnostic information to the client and the client would show that whenever unverified breakpoints exist for a debug session. Since this information is static and declarative the DA would not have to actively show/hide it. Instead the client could do the "right thing" (even after #126608 has been fixed). Currently we do not support markdown strings in DAP. So if we deem markdown as important then we would have to design a markdown-story for DAP.
• API: showBreakpointsInfoMessage would supply the client with a markdown string whenever breakpoints are amiss. The client would show the information with the same logic as option 3. Here we would not have the "markdown" problem, but instead the problem that the extension does not have easy access to information about unverified breakpoints (because the DA is a separate process).

@roblourens @connor4312 did I miss anything?

My preference is option 4.

[connor4312]

Option 1 is actually okay for js-debug. I would just show the same message for every breakpoint, though, inviting users to use the diagnostic tool. While I can try to diagnose why a single breakpoint isn't working, doing so at runtime when the breakpoint is initially set is expensive and I would not do so without the second level action of opening the tool. However, the message could not be interpreted as command-link-capable markdown, so this would not be a particularly good experience.

I think an API works for this.

[roblourens]

Thinking about an extension API, it's possible for a DA to talk to the EH through some channel, and I know some adapters do this already, do you think that this is common, or will the breakpoint info be totally inaccessible from the EH for some popular DAs? But since the purpose of this is specifically to drive vscode UI, it makes sense to me to have it in extension API.

Does this go on the debug session, or is it global? I think session. And does it return a disposable to hide the message, with multiple calls stacking up, or should we just allow one message per session, and call it setBreakpointsInfoMessage

[connor4312]

The common way for DA's to communicate is to use a custom DAP call and then onDidReceiveDebugSessionCustomEvent in the EH.

going on a debug session makes the most sense to me. Probably one message per session.

[weinand]

@roblourens I prefer your suggested setBreakpointsInfoMessage API over my showBreakpointsInfoMessage because it puts VS Code into control of showing/hiding the message (and multiplexing for multiple debug sessions).

"... or will the breakpoint info be totally inaccessible from the EH"

no, a DA can either send a custom event to the extension or the extension can install a DebugAdapterTracker to intercept the setBreakpoints response.

[roblourens]

And @connor4312 said that we should make it clear that only the extension that owns the DebugSession should use this API. I won't try to restrict any other extension from calling it and possibly wiping out the message that the DA extension set, but that would basically be misuse of the API, any API can be misused.

[roblourens]

Another suggestion from Kai:

Instead of showing the message outright, we could show some icon indicator like a lightbulb, or warning icon like the Terminal uses. When you hover or click it, we show a box with the message. This wouldn't take up vertical space, so you wouldn't have any resize flashing from the message appearing/hiding or switching DebugSessions like mentioned above. And in case we did want to enable showing multiple messages, this would be a nicer format for that.

I was wondering how much flickering there might be. When will this message be shown? Anytime there is an unbound breakpoint? If I set a breakpoint and press F5, will it be shown until the sourcemaps are loaded and the breakpoint is bound?

[roblourens]

A place where we have a custom icon in a viewpane header is for extensions:

I can add a hover icon in the same place, and it looks like this

I think this looks ok, any thoughts?

I talked with @sbatten about doing this today, we decided to try to fit something in the body of the viewpane rather than customizing the header. I tried a floating icon that would overlap the breakpoints list, but I just don't think that is going to look good, and it might block a button or detail on a breakpoint row. And anything else in the view body is going to change the height, which we don't want to do.

[gjsjohnmurray]

think this looks ok, any thoughts?

I like how it looks.

[weinand]

I like this too!
and if we want to make this stand out a bit more, we could append it to the header's label. IMO this makes sense because the icon is always visible like the label, and not only on hover.

[connor4312]

Looks great!

[roblourens]

We will try using a static message declared in the debugger contribution. Example:

"uiMessages": {
  "unverifiedBreakpoints": "There was a problem configuring your breakpoints. Click [here](command:js-debug.showTroubleshootingView) to troubleshoot your launch configuration."
}

In this case, vscode is in control of when and where to show this message.

[roblourens]

Code for the basic feature is in main. More code to add this message to the breakpoint hover, and to filter breakpoints by the debugger's supported language, is in a pending PR. Since the PR in js-debug will be shipped in june, and this isn't actually exposed anywhere for May, I will move this to June for testing.