Leverage notification progress support for progress API

[bpasero]

fixes #44090

@jrieken my attempt to support to show progress through notifications with:

• discrete progress (via percentage) support
• cancellation (via CancellationToken) support

Some comments:

• I did not add this to vscode.proposed because the changes are rather minimal and an evolution of our existing API, so I hope we can keep this as stable when pushing
• We should decide if we want the new progress location to be named differently if we ever decide to change it (e.g. should it be ProgressLocation.Prominent?). We could also decide to just use this new way of notification for our existing ProgressLocation.Window. One advantage is that the user can easily see and manage multiple progress tasks running at the same time if presented as notificiation.

Working Extension Sample

vscode.window.withProgress({
    location: vscode.ProgressLocation.Notification,
    title: "this is cool"
}, (progress, token) => {
    token.onCancellationRequested(() => {
        console.log("CANCELED")
    });

    setTimeout(() => {
        progress.report({ percentage: 10});
    }, 1000);

    setTimeout(() => {
        progress.report({ percentage: 50});
    }, 2000);

    setTimeout(() => {
        progress.report({ percentage: 100});
    }, 3000);

    var p = new Promise(resolve => {
        setTimeout(() => {
            resolve();
        }, 5000);
    });

    return p;
});

[jrieken]

I decided to report discrete progress through a combination of total and worked vs. supporting percentage. I find it easier to express the overall progress with chunks of work that are complete vs. having to compute the percentage.

cc @aeschli as IProgressMonitor veteran, but I find it weird to report absolute values. Doesn't it make it hard to reorder work or run work in parallel. E.g. download two files in parallel and have a third operation, all take 1/3. Which of the downloads says worked: 33 and which says worked: 66? Wouldn't that require an explicit sync-step?

Most importantly it prevents that the progress would ever go back

Like how? I can always call worked: 50 and then worked: 30, right? Do you mean an implementation check that ensures the value of worked always grows? That should be possible with other numbers, right?

[bpasero]

@jrieken

but I find it weird to report absolute values. Doesn't it make it hard to reorder work or run work in parallel. E.g. download two files in parallel and have a third operation, all take 1/3. Which of the downloads says worked: 33 and which says worked: 66? Wouldn't that require an explicit sync-step?

That could be total: 3 and worked: 1 per each operation that finishes since worked is relative always, no?

Btw it seems to me that IProgressMonitor in Eclipse works exactly like that (absolute total number of work and then relative chunks of work that can be reported when completed).

Like how? I can always call worked: 50 and then worked: 30, right? Do you mean an implementation check that ensures the value of worked always grows? That should be possible with other numbers, right?

Well this is always a relative value to the value already reported, never an absolute one (as the percentage one would probably be). So if you call worked: 50 and then worked: 30 the total value reflected in the progress bar would be 80.

[jrieken]

That could be total: 3 and worked: 1 per each operation that finishes since worked is relative always, no?

That makes sense but then your sample doesn't because worked: 10, worked: 50, worked: 70 is not total: 100.

What I don't like is that the total and worked information isn't contained nicely. Once piece knows the total and the Progress knows the increment...

[bpasero]

@jrieken but isn't a model where you provide percentage as absolute value equally hard for each sub-task to come up with? What if 2 tasks finish in parallel, what progress should be reported in terms of absolute percentage? I think its easier if the total: 2 and each task increments this by 1 when they finished to contribute to the overall progress.

Agree that you can easily fool the API by reporting more work done than actually present in total. I think the Eclipse API would have the same issue. From their docs:

void worked(int work): Notifies that a given number of work unit of the main task has been completed. Note that this amount represents an installment, as opposed to a cumulative amount of work done to date.
totalWork - the total number of work units into which the main task is been subdivided. If the value is UNKNOWN the implementation is free to indicate progress in a way which doesn't require the total number of work units in advance.

I think one concept that Eclipse has, that we are not surfacing is being able to create SubProgressMonitors where you can basically from the outside define a chunk of work and then pass it on to some task that just thinks the monitor is going from 0-100% and reports progress based on that scale. I am not sure if we want to go down that path though...

[jrieken]

Well, the question is if reported-values are increments or absolute values and I believe we both want increments, in my model total is just always 100 (therefore not needed and called percentage). From reading the eclipse doc, esp this:

amount represents an installment, as opposed to a cumulative amount of work done

I understand that worked is an absolute value and not increments.

So, with total: 100 and with worked: 10, worked: 50, worked: 70 you have have 10%, 60%, and 130% and I believe eclipse gives you 10%, 50%, and 70% (and that's why there are SubProgressMonitors)

[bpasero]

@jrieken

I believe we both want increments

Yes

in my model total is just always 100 (therefore not needed and called percentage)

We can do that.

The remaining question is, what happens when some work is done and you report more work done. For example, let's assume there are 3 tasks A, B, C.

• A finishes and reports 20% worked
• B finishes and reports 50% worked
• C finishes and reports 30% worked

Model 1: add percentage to overall work done
In this case we simply sum up the percentages until we reach 100%.

Model 2: add percentage relative to remaining work
In this case, the first task reports 20% so we end up at 20% overall work done and 80% remaining. Now the second task reports 50% done so we add 40% (80/2) and we and up with 60% overall work done and 40% remaining. And so on...

I think Model 1 is easier to use and explain to be honest.

[jrieken]

Model 1 - no question

[bpasero]

@jrieken pushed via 91ba743

The progress is infinite until the first time a percentage is reported.

[bpasero]

Remaining items up for discussion:

• should this just replace ProgressLocation.Window to keep things simpler for extension authors
• should the cancellation be optional (ProgressOptions.cancellable) so that extension authors are not forced to implement cancellation support

I personally think we should just remove the status bar entry in favour of showing it as a notification because:

• status bar space is already limited
• the notification area allows for more progress-related things (discrete progress, cancellation)
• multiple progresses can be shown at the same time
• you can just get rid of progress messages quickly by closing the notification

I am unsure about conditionally enabling the cancel button via option. If we decide to replace the status progress with the notification, it would mean that each extension author that implements this API has to wire in the cancellation. Since we cannot expect extension authors to do this, the cancellation should probably be opt-in.

[jrieken]

should the cancellation be optional (ProgressOptions.cancellable) so that extension authors are not forced to implement cancellation support

You cannot really force them to honour the token. Yes, we can have an option to signal the UI to not show the cancel button and then pass a null-token.

I personally think we should just remove the status bar entry in favour of showing it as a notification

Not sure - a message that pops into the editor area and an ambient message in the status bar are quite different. For instance, the progress message that TypeScript downloads d.ts-files is something I wouldn't wanna see in the editor area.

[bpasero]

@jrieken fair enough, I have pushed ProgressOptions.cancellable to opt-in to showing the cancel button (as opposed to opt-out). I also made the documentation more clear around the fact that both cancellation and discrete progress are currently only supported in ProgressLocation.Notification.