Replace spinner with custom icon when downloading MPLS/Insiders build (#10687)

* replace default Progress with custom icon when downloading
* extract formatProgressMessageWithState method for readability
* add news entry for #10495
* move icon display logic to `downloadFileWithStatusBarProgress`
* move RequestProgressState to fileDownloader and move formatProgressMessageWithState at module level
* fix try block to also catch download initialization errors
* add withProgressCustomIcon method
* use withProgressCustomIcon in fileDownloader.ts
* fix withProgressCustomIcon
* adapt fileDownloader unit tests
* add doc for icon param
* drop param `options` in withProgressCustomIcon
* update test_plan
Co-authored-by: Erwan de Lépinau <[email protected]>

Author: ErwanDL

.github/test_plan.md

@@ -135,6 +135,7 @@ SPAM='hello ${WHO}'
 #### Language server
 
 -   [ ] LS is downloaded using HTTP (no SSL) when the "http.proxyStrictSSL" setting is false
+-   [ ] An item with a cloud icon appears in the status bar indicating progress while downloading the language server
 -   [ ] Installing [`requests`](https://pypi.org/project/requests/) in virtual environment is detected
     -   [ ] Import of `requests` without package installed is flagged as unresolved
     -   [ ] Create a virtual environment

news/1 Enhancements/10495.md

@@ -0,0 +1 @@
+Use specific icons when downloading MPLS and Insiders instead of the spinner.

src/client/common/application/applicationShell.ts

@@ -7,6 +7,7 @@
 import { injectable } from 'inversify';
 import {
     CancellationToken,
+    CancellationTokenSource,
     Disposable,
     env,
     Event,
@@ -121,6 +122,23 @@ export class ApplicationShell implements IApplicationShell {
     ): Thenable<R> {
         return window.withProgress<R>(options, task);
     }
+    public withProgressCustomIcon<R>(
+        icon: string,
+        task: (progress: Progress<{ message?: string; increment?: number }>, token: CancellationToken) => Thenable<R>
+    ): Thenable<R> {
+        const token = new CancellationTokenSource().token;
+        const statusBarProgress = this.createStatusBarItem(StatusBarAlignment.Left);
+        const progress = {
+            report: (value: { message?: string; increment?: number }) => {
+                statusBarProgress.text = `${icon} ${value.message}`;
+            }
+        };
+        statusBarProgress.show();
+        return task(progress, token).then((result) => {
+            statusBarProgress.dispose();
+            return result;
+        });
+    }
     public createQuickPick<T extends QuickPickItem>(): QuickPick<T> {
         return window.createQuickPick<T>();
     }

src/client/common/application/types.ts

@@ -374,6 +374,33 @@ export interface IApplicationShell {
         task: (progress: Progress<{ message?: string; increment?: number }>, token: CancellationToken) => Thenable<R>
     ): Thenable<R>;
 
+    /**
+     * Show progress in the status bar with a custom icon instead of the default spinner.
+     * Progress is shown while running the given callback and while the promise it returned isn't resolved nor rejected.
+     * At the moment, progress can only be displayed in the status bar when using this method. If you want to
+     * display it elsewhere, use `withProgress`.
+     *
+     * @param icon A valid Octicon.
+     *
+     * @param task A callback returning a promise. Progress state can be reported with
+     * the provided [progress](#Progress)-object.
+     *
+     * To report discrete progress, use `increment` to indicate how much work has been completed. Each call with
+     * a `increment` value will be summed up and reflected as overall progress until 100% is reached (a value of
+     * e.g. `10` accounts for `10%` of work done).
+     * Note that currently only `ProgressLocation.Notification` is capable of showing discrete progress.
+     *
+     * To monitor if the operation has been cancelled by the user, use the provided [`CancellationToken`](#CancellationToken).
+     * Note that currently only `ProgressLocation.Notification` is supporting to show a cancel button to cancel the
+     * long running operation.
+     *
+     * @return The thenable the task-callback returned.
+     */
+    withProgressCustomIcon<R>(
+        icon: string,
+        task: (progress: Progress<{ message?: string; increment?: number }>, token: CancellationToken) => Thenable<R>
+    ): Thenable<R>;
+
     /**
      * Create a [TreeView](#TreeView) for the view contributed using the extension point `views`.
      * @param viewId Id of the view contributed using the extension point `views`.

src/client/common/constants.ts

@@ -69,6 +69,8 @@ export namespace Octicons {
     export const Test_Fail = '$(alert)';
     export const Test_Error = '$(x)';
     export const Test_Skip = '$(circle-slash)';
+    export const Downloading = '$(cloud-download)';
+    export const Installing = '$(desktop-download)';
 }
 
 export const Button_Text_Tests_View_Output = 'View Output';

src/client/common/net/fileDownloader.ts

@@ -5,8 +5,9 @@
 
 import { inject, injectable } from 'inversify';
 import * as requestTypes from 'request';
-import { Progress, ProgressLocation } from 'vscode';
+import { Progress } from 'vscode';
 import { IApplicationShell } from '../application/types';
+import { Octicons } from '../constants';
 import { IFileSystem, WriteStream } from '../platform/types';
 import { DownloadOptions, IFileDownloader, IHttpClient } from '../types';
 import { Http } from '../utils/localize';
@@ -40,12 +41,13 @@ export class FileDownloader implements IFileDownloader {
         progressMessage: string,
         tmpFilePath: string
     ): Promise<void> {
-        await this.appShell.withProgress({ location: ProgressLocation.Window }, async (progress) => {
+        await this.appShell.withProgressCustomIcon(Octicons.Downloading, async (progress) => {
             const req = await this.httpClient.downloadFile(uri);
             const fileStream = this.fs.createWriteStream(tmpFilePath);
             return this.displayDownloadProgress(uri, progress, req, fileStream, progressMessage);
         });
     }
+
     public async displayDownloadProgress(
         uri: string,
         progress: Progress<{ message?: string; increment?: number }>,
@@ -64,17 +66,8 @@ export class FileDownloader implements IFileDownloader {
             // tslint:disable-next-line: no-require-imports
             const requestProgress = require('request-progress');
             requestProgress(request)
-                // tslint:disable-next-line: no-any
-                .on('progress', (state: any) => {
-                    const received = Math.round(state.size.transferred / 1024);
-                    const total = Math.round(state.size.total / 1024);
-                    const percentage = Math.round(100 * state.percent);
-                    const message = Http.downloadingFileProgress().format(
-                        progressMessagePrefix,
-                        received.toString(),
-                        total.toString(),
-                        percentage.toString()
-                    );
+                .on('progress', (state: RequestProgressState) => {
+                    const message = formatProgressMessageWithState(progressMessagePrefix, state);
                     progress.report({ message });
                 })
                 // Handle errors from download.
@@ -86,3 +79,29 @@ export class FileDownloader implements IFileDownloader {
         });
     }
 }
+
+type RequestProgressState = {
+    percent: number;
+    speed: number;
+    size: {
+        total: number;
+        transferred: number;
+    };
+    time: {
+        elapsed: number;
+        remaining: number;
+    };
+};
+
+function formatProgressMessageWithState(progressMessagePrefix: string, state: RequestProgressState): string {
+    const received = Math.round(state.size.transferred / 1024);
+    const total = Math.round(state.size.total / 1024);
+    const percentage = Math.round(100 * state.percent);
+
+    return Http.downloadingFileProgress().format(
+        progressMessagePrefix,
+        received.toString(),
+        total.toString(),
+        percentage.toString()
+    );
+}

src/test/common/net/fileDownloader.unit.test.ts

@@ -111,7 +111,7 @@ suite('File Downloader', () => {
             const progressReportStub = sinon.stub();
             const progressReporter: Progress<ProgressReporterData> = { report: progressReportStub };
             const tmpFilePath = await fs.createTemporaryFile('.json');
-            when(appShell.withProgress(anything(), anything())).thenCall((_, cb) => cb(progressReporter));
+            when(appShell.withProgressCustomIcon(anything(), anything())).thenCall((_, cb) => cb(progressReporter));
 
             fileDownloader = new FileDownloader(instance(httpClient), fs, instance(appShell));
             await fileDownloader.downloadFileWithStatusBarProgress(uri, 'hello', tmpFilePath.filePath);
@@ -125,7 +125,7 @@ suite('File Downloader', () => {
             nock('https://python.extension').get('/package.json').reply(500);
             const progressReportStub = sinon.stub();
             const progressReporter: Progress<ProgressReporterData> = { report: progressReportStub };
-            when(appShell.withProgress(anything(), anything())).thenCall((_, cb) => cb(progressReporter));
+            when(appShell.withProgressCustomIcon(anything(), anything())).thenCall((_, cb) => cb(progressReporter));
             const tmpFilePath = await fs.createTemporaryFile('.json');
 
             fileDownloader = new FileDownloader(instance(httpClient), fs, instance(appShell));
@@ -142,7 +142,7 @@ suite('File Downloader', () => {
                 .reply(200, () => fsExtra.createReadStream(packageJsonFile));
             const progressReportStub = sinon.stub();
             const progressReporter: Progress<ProgressReporterData> = { report: progressReportStub };
-            when(appShell.withProgress(anything(), anything())).thenCall((_, cb) => cb(progressReporter));
+            when(appShell.withProgressCustomIcon(anything(), anything())).thenCall((_, cb) => cb(progressReporter));
 
             // Use bogus files that cannot be created (on windows, invalid drives, on mac & linux use invalid home directories).
             const invalidFileName = new PlatformService().isWindows
@@ -161,7 +161,7 @@ suite('File Downloader', () => {
                 .reply(200, () => fsExtra.createReadStream(packageJsonFile));
             const progressReportStub = sinon.stub();
             const progressReporter: Progress<ProgressReporterData> = { report: progressReportStub };
-            when(appShell.withProgress(anything(), anything())).thenCall((_, cb) => cb(progressReporter));
+            when(appShell.withProgressCustomIcon(anything(), anything())).thenCall((_, cb) => cb(progressReporter));
             // Create a file stream that will throw an error when written to (use ErroringMemoryStream).
             const tmpFilePath = 'bogus file';
             const fileSystem = mock(FileSystem);
@@ -188,7 +188,7 @@ suite('File Downloader', () => {
                 ]);
             const progressReportStub = sinon.stub();
             const progressReporter: Progress<ProgressReporterData> = { report: progressReportStub };
-            when(appShell.withProgress(anything(), anything())).thenCall((_, cb) => cb(progressReporter));
+            when(appShell.withProgressCustomIcon(anything(), anything())).thenCall((_, cb) => cb(progressReporter));
             const tmpFilePath = await fs.createTemporaryFile('.json');
             // Mock request-progress to throttle 1ms, so we can get progress messages.
             // I.e. report progress every 1ms. (however since download is delayed to 10ms,