Render progress for DAP progress events

[isidorn]

Related debug adapter protocol issue microsoft/debug-adapter-protocol#92

fyi @weinand

[weinand]

@isidorn I have added experimental DAP progress support to debugProtocol.d.ts.

I suggest that we start prototyping the debug progress UI for the use case "progress independent from a specific request". This is the important use case that @chuckries and @DanTup are looking for.

I will start this effort by adding this use case to Mock Debug.

[isidorn]

@weinand sounds good. Let me know when Mock Debug has adopted this.
Progress independent from a request: I suggest to render in as the spinning bar at the top of the Debug Viewlet.
Alternatives are:

• status bar entry (personally dislike this)
• progress badge in debug activity bar
• global workbench progress (too global imho)

[weinand]

@isidorn Yes, let's start by showing "unspecific progress" at the top of the Debug Viewlet.
If we will start showing the percentage we could show it in the status bar.

A shortcoming of the "top of the Debug Viewlet" location is that the user won't be able to cancel it, right?

[isidorn]

@weinand while we render progress we could also render a X or some other cnacel action in the debug viewlet title area. Pressing that would send a cancel. Check out search for long running search operations does somethign similar.

[weinand]

@isidorn I've added progress support to Mock Debug via this commit.
Running "progress" in the REPL starts a sequence of 100 progress events over a period of 50 seconds (but only if the client frontend sends a supportsProgressReporting capability).
The progress is cancellable.

[weinand]

@isidorn some random notes:

• yes, debug should have a similar cancelling UI as search (Mock Debug should have everything in place to support cancel)
• Mock debug provides the (mandatory) "title" attribute and the optional "message" attribute. We'll have to decide how they are displayed.
• overlapping progress: you can produce concurrent progress sequences with Mock debug by running multiple "progress" commands in the REPL within 50 seconds. I see two strategies for how to deal with concurrent progress sequences:
  • we could stack them and show only the top (newest) one (cleanest)
  • we could maintain just one progress and every new progress overwrites the current (kludge)
• next stage of the prototype: what to do with request-specific-progress. For now I suggest that we treat it identical as non-request-specific progress.

[isidorn]

@weinand thanks for adding the progess support for Mock debug. I plan to jump on this today.

1. Cool, will add a cancel title action
2. Maybe we show a hidden notification for every progress, and once the user clicks on the notification center he can se the title and the message
3. Idealy we would show progress as long as there is any progress event. Canceling cancels the last one
4. Yeah for now we will just render in the debug viewlet. Once there are specific use cases we can think more

[isidorn]

I have pushed the inital support for this.
When the progress is reported we show the progress bar in the debug viewlet and the progress can be canceled. Note the following:

• We use a "stop" icon which I think works for the ocassion, attached is an icon that search uses, but I do not think it would be necessery to "scope the icon to debug", since it is already in the viewlet fyi @misolori maybe instead of the X in the circle we should use the do not park sign that search uses
• When the user presses the Cancel we cancel the last progress that arrived
• We show progress as long as any progress is left
• If a session goes down we stop all the progress associated with that session
• Tooltip of the Cancel action shows the title of the progress

What is not covered by this work and we can tackle as seperate issues once clients adopt this:

• We ignore the ProgressUpdateEvent since VS Code progress UI does not support reporting progress status
• We do not show the message of the progress (could be done as a hidden progress notification)
• We only show progress in the debug viewlet, we do not show any scoped progress

Note that I had to do a slight change in mock debug such that it no longer waits after a progress has been canceled microsoft/vscode-mock-debug@29ec169
For reference here's the Cancel progress icon search is using

Try it out and let me know what you think.

[isidorn]

Based on the great feedback from @bpasero we have decided to show progress via progress notifications.
Since this is similar to the save participant. On top of that show progress bar in debug viewlet (but not the cancel button).
Good part of this approach is that notifications can show the message and have multiple progress managment.
Once we have view cancelalble progress in workbench think of adopting.
Progress notifications will show the progress in status bar even when they are hidden.

[isidorn]

We have done the changes and now the progress cancelation is much easier to manage by users imho.

There is still some work in progress:

• silent notifications should still show in the status bar even before opened for first time
• message of the protocol event should be rendered, not only the title Progress notifications: visually distinguish between title and message #92948
• we should respect the progressUpdate event

Try it out and let me know what you think.

[isidorn]

@weinand I have pushed a commit such that we now also respect the progressUpdate events.

[gjsjohnmurray]

Nice! How about also setting options.source to something, so the message gets a [source] prefix that lets the user know where the message is coming from?

[bpasero]

That should be the extension I guess that sets the progress, I like that 👍

[weinand]

@gjsjohnmurray excellent suggestion.

@isidorn Since the DAP does not know anything about extensions, we cannot put something in the spec that says that the extension's name has to go into the progress' "title" attribute.

I suggest that we let VS Code add the extension name by using a format string like "${debugExtensionName}: ${progressTitle}"

[isidorn]

@weinand @gjsjohnmurray makes sense, good sugggestion.
Though I just used the ${debugExtensionName} since the progressTitle is already set on the message so to avoid duplication.

[weinand]

The "source" in the notification works nicely:

Since we should not use too much space in the status bar, we could show the "source" when hovering over the status item?

[isidorn]

I already talked to @bpasero about this and I think we should not show the message in the status bar. Only the title. This way it is way too wide.
On hover we should show the message or the source.

[gjsjohnmurray]

On hover we should show the message or the source.

Or perhaps {message} [{source}]

For example progress: 53 [Mock Debug]

[weinand]

In the status item we could start with the progress "title", and if there is a progress "message", we would overwrite the title with the message (so we only show one thing).
On hover we could show the "title" and the "source" as @gjsjohnmurray suggested.

[bpasero]

The status bar is capable of showing title and message at the same time as far as I can tell. It would simply show it as title: message and I think it is good to show both and not just the message because there is a chance that it becomes harder for the user to understand what is going on.