Extension API for enabling debug extensions to show a message in the breakpoints view

[roblourens]

In #142870 we are discussing UI for an extension to show a diagnostic message in the breakpoints view. An example of using this would be for a debug extension to prompt the user about the breakpoints troubleshooting tool. This issue is for the extension API.

We could put an API on the DebugSession or on the debug namespace. We could also only allow a single message at a time, with the advice that a well-behaved extension will only use it if it is responsible for this debug type, or fully allow multiple messages (I don't have a real use-case in mind for that).

Some possibilities

interface DebugSession {
    /** Set a message regarding breakpoints to be shown for this DebugSession. Will replace a previously-set message. Should only be called by the extension that "owns" this DebugSession kind. Set to an empty string to remove the message. */
    setBreakpointsInfoMessage(message: string | MarkdownString): void;
}

interface DebugSession {
    /** Show a message regarding breakpoints for this DebugSession. Returns a Disposable that hides the message. */
    showBreakpointsInfoMessage(message: string | MarkdownString): Disposable;
}

namespace debug {
    /** Set a message regarding breakpoints to be shown for this DebugSession. Will replace a previously-set message. Should only be called by the extension that "owns" this DebugSession kind. */
    setBreakpointsInfoMessage(session: DebugSession, message: string | MarkdownString): void;
}

cc @jrieken

[ashgti]

Should the message be a string|MarkdownString to allow for links?

[roblourens]

Yes, thanks

[jrieken]

Should only be called by the extension that "owns" this DebugSession kind.

Maybe I have trust issues but I wouldn't rely on that. A debug session is public via vscode.debug.activeDebugSession so any extension can exercise this API. I am unsure how debug session are created and if that would allow for a way to sneak some controller in. Other alternatives might be something like debug message collection that one creates and manages or at least some "additive behaviour" e.g show 5 messages if extensions call it 5 fives etc. Also, where is the "Breakpoint" in the breakpoint message? Don't we have an object-type for them? Should a message reference concrete breakpoints? Last question: is this always info or also other severities, e.g should there be a BreakpointMessageSeverity-type?

[roblourens]

I am unsure how debug session are created and if that would allow for a way to sneak some controller in.

Not really, they are created by vscode. We could check that extension IDs match, do we do that anywhere?

Other alternatives might be something like debug message collection that one creates and manages or at least some "additive behaviour" e.g show 5 messages if extensions call it 5 fives etc.

This is the showBreakpointsInfoMessage alternative, it adds a message and the Disposable hides it.

Also, where is the "Breakpoint" in the breakpoint message? Don't we have an object-type for them? Should a message reference concrete breakpoints?

The idea is that this isn't for a particular breakpoint but just a hint about some breakpoints in general. It shows in the title bar for the breakpoints view.

In general the concept is that this is a pretty simple message which is not expected to be used for rich detailed info, which is why I don't have a severity and why I prefer only showing a single message. If we allow multiple messages, then I am starting to reimplement the notifications pane and everything that entails, but I basically just want an icon with a hover tooltip.

@mjbvz also suggested adding a language status bar item for it, but I'm not sure whether this sort of info is expected to go in there.

[jrieken]

Not really, they are created by vscode. We could check that extension IDs match, do we do that anywhere?

Now, I am unsure why this is an API request and not a DAP request? Isn't the latter the better fit for something like this e.g the ownership/driver of data is clearly defined and doesn't come from "the side" via some API

[roblourens]

We discussed that but it doesn't seem to fit into DAP. Feels too targeted at driving a specific piece of vscode UI. This needs markdown and vscode command links which aren't in DAP.

[jrieken]

I would question that. From where I am, it feels much better "contained" within DAP. A good question might be how such messages are forwarded in LiveShare scenarios? You also want to reconsider allowing markdown - like you want to support links but I doubt that you want bullet lists, tables, or code listings in simple messages.

[roblourens]

I think that defining what markdown in DAP means is too heavy just for this little feature, unless there is some other compelling feature that needs markdown. Do you agree @connor4312 @weinand?

Besides that, @weinand described the options well here: #142870 (comment)

If this went in DAP, we would need something like option 3, except that I think js-debug's heuristic is more complex than "some breakpoints are unverified", so we would need a new DAP event.

[jrieken]

I think that defining what markdown in DAP means is too heavy just for this little feature, unless there is some other compelling feature that needs markdown. Do you agree @connor4312 @weinand?

@roblourens That's exactly what I am saying, you do not want to support markdown here, not in DAP and not for this little feature.

[weinand]

I agree that we do not want to introduce a "heavy" API for this "little" feature so I was thinking about lightweight alternatives...

Here is one:
We could piggyback on the existing notification mechanism (API: window.showInformationMessage) which already provides all the necessary functionality (styled text and embedded links and buttons).

But using notifications for the breakpoint information has (at least) two problems:

• it is too annoying to always have a notification pop up if a breakpoint could not be validated,
• the notification is too far away from the breakpoints view.

These problems could be easily addressed by introducing a new MessageOption that shows a notification icon at a specific location (here: header of breakpoints view) instead of the far right of the status bar. And pressing the icon shows the notification in the normal place (bottom right corner).

The new MessageOption property could be called context and since the mechanism is independent from breakpoints or debugging, it could be made available for other locations than the breakpoints view.

@roblourens @connor4312 @jrieken opinions?

[roblourens]

One issue with that is that it loses the connection to the DebugSession, and I really think it should be per-DebugSession. It's also not clear in that case how you get to the warning icon in the breakpoints view - can you get 3 icons depending on which show* API you called? I don't really like that.

Discussing this morning, it feels like DAP is the right place for this, since that is the real source of truth for this info. Also would be nice to have some flexibility in how exactly it is displayed in the client. Learned that LSP has a markdown concept: https://microsoft.github.io/language-server-protocol/specification#markupContent. @dbaeumer I'm wondering whether that has been a source of pain for LSP clients/servers or if it has not been an issue.

If we had a plain text API in DAP, at least to separate the markdown work item, would this still be useful? It can say "Run the xyz command ..."

On the other hand, occurs to me that the link we are trying to show here specifically only makes sense in vscode, because the very feature we are trying to prompt (a troubleshooting tool which is a custom vscode editor) only works in vscode, that is a point for extension API.

[dbaeumer]

@roblourens no, no source of pain. What needs to be noted is that LSP has a Markup content which supports plain text and mark down. So a client can announce that it doesn't understand markdown.

But the push is actually to support mark down in more areas not in less :-).

[jrieken]

These problems could be easily addressed by introducing a new MessageOption that shows a notification icon at a specific location

I generally like the idea, similar to the concept of the ProgressLocation there could be a "MessageLocation" which allows to show contextual messages. Maybe that has the potential to reduce the "notification randomness".

However, for this specific topic I think a targeted DAP concept makes more sense. It will allow to associate a message to a session and allow for more flexible UI, e.g show the warning in the breakpoints view and in other places like the debug toolbar or inside the editor etc. Such UI works better with "domain information" instead of a generic show message API

[weinand]

@roblourens and @jrieken: thanks for the feedback - here some comments:

• making the "breakpoint information feature" a DAP concept allows to fully implement it in VS Code debug core and does not require any new extension API (which I like). This means that we should move the discussion from this now obsolete API request back to the original #142870.
• Since conceptually DAP cannot be the driver of the UI we cannot add something like a "showBreakpointInformation" request to DAP. DAP can only provide the "strings" that the client needs for populating the UI. This brings us back to the options that I've already listed here.
• I agree with @roblourens that there is not much need for "markdown strings" in DAP, as we would only need them for embedding action links, which depend heavily on the client's UI and therefore would not make much sense in DAP. But just introducing new extension API for getting "action links" into this UI sounds again a bit heavy. Maybe we could contribute the breakpoint information as a markdown template statically in the package.json and VS Code would just parameterise the template with the error messages returned from DAP.
• VS Code will have to aggregate the information and show it behind an icon in the breakpoint viewer's header. Multiplexing this information across different debug sessions must be considered. Today we are not doing this, see #126608.
• tieing the breakpoint information to a debug session is not without problems: if a user runs a (short running) program without hitting a breakpoint, the debug session immediately ends and the breakpoint information will vanish. In order to keep it longer we will have to add our own "history" management. By using the existing notification mechanism (as suggested here) we would get that for free.

[roblourens]

Maybe we could contribute the breakpoint information as a markdown template statically in the package.json and VS Code would just parameterise the template with the error messages returned from DAP.

Would this go inside the debugger contribution? Needs to be localized. How would it work to reference a markdown template from a DAP message?

[weinand]

A DA cannot know anything about markdown templates because they are VS Code specific.
But VS Code core would use a template from the debugger contribution whenever it sees info messages returned from DAP requests. I don't think that VS Code could map different DAP response messages to different templates. So we would only support a single template.

[roblourens]

I don't really understand the suggestion. What would the message from DAP look like? How does the DA know that the client will use this template?

[weinand]

In case of a breakpoint problem the DAP returns any of the messages listed in the first two options here.
If VS Code receives one or more of these messages, it assumes that there is a problem and tries to find a markdown template in the corresponding debugger contribution of the package.json. If it finds one, it substitutes some pattern in the template with the individual messages returned from DAP (or with some aggregation of the messages) and constructs the final markdown. Then VS code shows the information icon in the BREAKPOINTS view header and installs the markdown as the text to show when the icon is clicked.

[roblourens]

We will probably not add extension API for this, so we can move the discussion back to #142870