From d5b9ba86f23cc732c6c065d32a6ec944c735860b Mon Sep 17 00:00:00 2001 From: Unknown Date: Fri, 6 Apr 2018 11:37:24 -0700 Subject: [PATCH 1/8] Changes to remove status from packages.ts --- gulpfile.ts | 4 +- src/CSharpExtDownloader.ts | 13 ++-- src/downloader.helper.ts | 20 ------ src/observers/BaseStatusBarItemObserver.ts | 7 ++- src/observers/CsharpLoggerObserver.ts | 24 +++++-- src/observers/OmnisharpStatusBarObserver.ts | 22 +++++-- src/observers/ProjectStatusBarObserver.ts | 4 +- src/omnisharp/OmnisharpDownloader.ts | 15 ++--- src/omnisharp/OmnisharpPackageCreator.ts | 3 +- src/omnisharp/loggingEvents.ts | 17 ++++- src/packages.ts | 69 +++++++-------------- 11 files changed, 94 insertions(+), 104 deletions(-) diff --git a/gulpfile.ts b/gulpfile.ts index fa6b04e1d7..4e01fc3478 100644 --- a/gulpfile.ts +++ b/gulpfile.ts @@ -60,9 +60,9 @@ function install(platformInfo, packageJSON) { eventStream.subscribe(stdoutObserver.post); const debuggerUtil = new debugUtil.CoreClrDebugUtil(path.resolve('.')); - return packageManager.DownloadPackages(eventStream, undefined, undefined, undefined) + return packageManager.DownloadPackages(eventStream, undefined, undefined) .then(() => { - return packageManager.InstallPackages(eventStream, undefined); + return packageManager.InstallPackages(eventStream); }) .then(() => { return util.touchInstallFile(util.InstallFileType.Lock); diff --git a/src/CSharpExtDownloader.ts b/src/CSharpExtDownloader.ts index 9c7363e242..6d3ee4a57c 100644 --- a/src/CSharpExtDownloader.ts +++ b/src/CSharpExtDownloader.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as util from './common'; -import { GetNetworkConfiguration, GetStatus } from './downloader.helper'; +import { GetNetworkConfiguration } from './downloader.helper'; import { PackageManager } from './packages'; import { PlatformInformation } from './platform'; import { PackageInstallation, LogPlatformInfo, InstallationSuccess, InstallationFailure } from './omnisharp/loggingEvents'; @@ -21,9 +21,7 @@ export class CSharpExtDownloader { } public async installRuntimeDependencies(): Promise { - this.eventStream.post(new PackageInstallation("C# dependencies" )); - - let status = GetStatus(); + this.eventStream.post(new PackageInstallation("C# dependencies")); let installationStage = 'touchBeginFile'; let success = false; @@ -32,17 +30,17 @@ export class CSharpExtDownloader { let packageManager = new PackageManager(this.platformInfo, this.packageJSON); // Display platform information and RID - this.eventStream.post(new LogPlatformInfo(this.platformInfo )); + this.eventStream.post(new LogPlatformInfo(this.platformInfo)); installationStage = 'downloadPackages'; let networkConfiguration = GetNetworkConfiguration(); const proxy = networkConfiguration.Proxy; const strictSSL = networkConfiguration.StrictSSL; - await packageManager.DownloadPackages(this.eventStream, status, proxy, strictSSL); + await packageManager.DownloadPackages(this.eventStream, proxy, strictSSL); installationStage = 'installPackages'; - await packageManager.InstallPackages(this.eventStream, status); + await packageManager.InstallPackages(this.eventStream); installationStage = 'touchLockFile'; await util.touchInstallFile(util.InstallFileType.Lock); @@ -54,7 +52,6 @@ export class CSharpExtDownloader { this.eventStream.post(new InstallationFailure(installationStage, error)); } finally { - status.dispose(); // We do this step at the end so that we clean up the begin file in the case that we hit above catch block // Attach a an empty catch to this so that errors here do not propogate try { diff --git a/src/downloader.helper.ts b/src/downloader.helper.ts index e17eb5f7fb..0789755c9a 100644 --- a/src/downloader.helper.ts +++ b/src/downloader.helper.ts @@ -3,30 +3,10 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ import * as vscode from 'vscode'; -import { Status } from './packages'; export function GetNetworkConfiguration() { const config = vscode.workspace.getConfiguration(); const proxy = config.get('http.proxy'); const strictSSL = config.get('http.proxyStrictSSL', true); return { Proxy: proxy, StrictSSL: strictSSL }; -} - -export function GetStatus(): Status { - let statusItem = vscode.window.createStatusBarItem(vscode.StatusBarAlignment.Right); - let status: Status = { - setMessage: text => { - statusItem.text = text; - statusItem.show(); - }, - setDetail: text => { - statusItem.tooltip = text; - statusItem.show(); - }, - dispose: () => { - statusItem.dispose(); - } - }; - - return status; } \ No newline at end of file diff --git a/src/observers/BaseStatusBarItemObserver.ts b/src/observers/BaseStatusBarItemObserver.ts index fa47c107a4..974e9360ad 100644 --- a/src/observers/BaseStatusBarItemObserver.ts +++ b/src/observers/BaseStatusBarItemObserver.ts @@ -11,7 +11,7 @@ export abstract class BaseStatusBarItemObserver { constructor(private statusBarItem: StatusBarItem) { } - public SetAndShowStatusBar(text: string, command: string, color?: string) { + public SetTextAndShowStatusBar(text: string, command?: string, color?: string) { this.statusBarItem.text = text; this.statusBarItem.command = command; this.statusBarItem.color = color; @@ -25,5 +25,10 @@ export abstract class BaseStatusBarItemObserver { this.statusBarItem.hide(); } + public SetToolTipAndShowStatusBar(text: string) { + this.statusBarItem.tooltip = text; + this.statusBarItem.show(); + } + abstract post: (event: BaseEvent) => void; } \ No newline at end of file diff --git a/src/observers/CsharpLoggerObserver.ts b/src/observers/CsharpLoggerObserver.ts index 882c9d55e4..9618b33a14 100644 --- a/src/observers/CsharpLoggerObserver.ts +++ b/src/observers/CsharpLoggerObserver.ts @@ -45,9 +45,23 @@ export class CsharpLoggerObserver extends BaseLoggerObserver { case Event.ProjectJsonDeprecatedWarning.name: this.logger.appendLine("Warning: project.json is no longer a supported project format for .NET Core applications. Update to the latest version of .NET Core (https://aka.ms/netcoredownload) and use 'dotnet migrate' to upgrade your project (see https://aka.ms/netcoremigrate for details)."); break; + case Event.DownloadFallBack.name: + this.handleDownloadFallback(event); + break; + case Event.DownloadSizeObtained.name: + this.handleDownloadSizeObtained(event); + break; } } + private handleDownloadSizeObtained(event: Event.DownloadSizeObtained) { + this.logger.append(`(${Math.ceil(event.packageSize / 1024)} KB) `); + } + + private handleDownloadFallback(event: Event.DownloadFallBack) { + this.logger.append(`\tRetrying from '${event.fallbackUrl}' `); + } + private handleEventWithMessage(event: Event.EventWithMessage) { this.logger.appendLine(event.message); } @@ -81,19 +95,17 @@ export class CsharpLoggerObserver extends BaseLoggerObserver { private handleDownloadProgress(event: Event.DownloadProgress) { let newDots = Math.ceil(event.downloadPercentage / 5); - if (newDots > this.dots) { - this.logger.append('.'.repeat(newDots - this.dots)); - this.dots = newDots; - } + this.logger.append('.'.repeat(newDots - this.dots)); + this.dots = newDots; } private handleDownloadStart(event: Event.DownloadStart) { - this.logger.append(event.message); + this.logger.append(`Downloading package '${event.packageDescription}'... `); this.dots = 0; } private handleInstallationProgress(event: Event.InstallationProgress) { - this.logger.appendLine(event.message); + this.logger.appendLine(`Installing package '${event.packageDescription}'`); this.logger.appendLine(); } } \ No newline at end of file diff --git a/src/observers/OmnisharpStatusBarObserver.ts b/src/observers/OmnisharpStatusBarObserver.ts index 3258bb9666..560547d73f 100644 --- a/src/observers/OmnisharpStatusBarObserver.ts +++ b/src/observers/OmnisharpStatusBarObserver.ts @@ -3,26 +3,38 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { OmnisharpServerOnServerError, BaseEvent, OmnisharpOnBeforeServerInstall, OmnisharpOnBeforeServerStart, OmnisharpServerOnStop, OmnisharpServerOnStart } from "../omnisharp/loggingEvents"; +import { OmnisharpServerOnServerError, BaseEvent, OmnisharpOnBeforeServerInstall, OmnisharpOnBeforeServerStart, OmnisharpServerOnStop, OmnisharpServerOnStart, DownloadStart, InstallationProgress, DownloadProgress } from "../omnisharp/loggingEvents"; import { BaseStatusBarItemObserver } from './BaseStatusBarItemObserver'; export class OmnisharpStatusBarObserver extends BaseStatusBarItemObserver { public post = (event: BaseEvent) => { switch (event.constructor.name) { case OmnisharpServerOnServerError.name: - this.SetAndShowStatusBar('$(flame) Error starting OmniSharp', 'o.showOutput', ''); + this.SetTextAndShowStatusBar('$(flame) Error starting OmniSharp', 'o.showOutput', ''); break; case OmnisharpOnBeforeServerInstall.name: - this.SetAndShowStatusBar('$(flame) Installing OmniSharp...', 'o.showOutput', ''); + this.SetTextAndShowStatusBar('$(flame) Installing OmniSharp...', 'o.showOutput', ''); break; case OmnisharpOnBeforeServerStart.name: - this.SetAndShowStatusBar('$(flame) Starting...', 'o.showOutput', ''); + this.SetTextAndShowStatusBar('$(flame) Starting...', 'o.showOutput', ''); break; case OmnisharpServerOnStop.name: this.ResetAndHideStatusBar(); break; case OmnisharpServerOnStart.name: - this.SetAndShowStatusBar('$(flame) Running', 'o.showOutput', ''); + this.SetTextAndShowStatusBar('$(flame) Running', 'o.showOutput', ''); + break; + case DownloadStart.name: + this.SetTextAndShowStatusBar("$(cloud-download) Downloading packages"); + this.SetToolTipAndShowStatusBar(`Downloading package '${(event).packageDescription}...' `); + break; + case InstallationProgress.name: + this.SetTextAndShowStatusBar("$(desktop-download) Installing packages..."); + this.SetToolTipAndShowStatusBar(`Installing package '${(event).packageDescription}'`); + break; + case DownloadProgress.name: + let progressEvent = event; + this.SetToolTipAndShowStatusBar(`Downloading package '${progressEvent.packageDescription}'... ${progressEvent.downloadPercentage}%`); break; } } diff --git a/src/observers/ProjectStatusBarObserver.ts b/src/observers/ProjectStatusBarObserver.ts index fd37b7b20e..91a62eda1d 100644 --- a/src/observers/ProjectStatusBarObserver.ts +++ b/src/observers/ProjectStatusBarObserver.ts @@ -12,7 +12,7 @@ export class ProjectStatusBarObserver extends BaseStatusBarItemObserver { public post = (event: BaseEvent) => { switch (event.constructor.name) { case OmnisharpOnMultipleLaunchTargets.name: - this.SetAndShowStatusBar('$(file-submodule) Select project', 'o.pickProjectAndStart', 'rgb(90, 218, 90)'); + this.SetTextAndShowStatusBar('$(file-submodule) Select project', 'o.pickProjectAndStart', 'rgb(90, 218, 90)'); break; case OmnisharpServerOnStop.name: this.ResetAndHideStatusBar(); @@ -27,7 +27,7 @@ export class ProjectStatusBarObserver extends BaseStatusBarItemObserver { let info = event.info; if (info.MsBuild && info.MsBuild.SolutionPath) { label = basename(info.MsBuild.SolutionPath); //workspace.getRelativePath(info.MsBuild.SolutionPath); - this.SetAndShowStatusBar('$(file-directory) ' + label, 'o.pickProjectAndStart'); + this.SetTextAndShowStatusBar('$(file-directory) ' + label, 'o.pickProjectAndStart'); } else { this.ResetAndHideStatusBar(); diff --git a/src/omnisharp/OmnisharpDownloader.ts b/src/omnisharp/OmnisharpDownloader.ts index 7df37eadb1..25fe811417 100644 --- a/src/omnisharp/OmnisharpDownloader.ts +++ b/src/omnisharp/OmnisharpDownloader.ts @@ -3,9 +3,9 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { GetNetworkConfiguration, GetStatus } from '../downloader.helper'; +import { GetNetworkConfiguration } from '../downloader.helper'; import { GetPackagesFromVersion, GetVersionFilePackage } from './OmnisharpPackageCreator'; -import { Package, PackageManager, Status } from '../packages'; +import { Package, PackageManager } from '../packages'; import { PlatformInformation } from '../platform'; import { PackageInstallation, LogPlatformInfo, InstallationSuccess, InstallationFailure, InstallationProgress } from './loggingEvents'; import { EventStream } from '../EventStream'; @@ -16,7 +16,6 @@ export interface IPackageManagerFactory { } export class OmnisharpDownloader { - private status: Status; private proxy: string; private strictSSL: boolean; private packageManager: PackageManager; @@ -27,7 +26,6 @@ export class OmnisharpDownloader { private platformInfo: PlatformInformation, packageManagerFactory: IPackageManagerFactory = defaultPackageManagerFactory) { - this.status = GetStatus(); let networkConfiguration = GetNetworkConfiguration(); this.proxy = networkConfiguration.Proxy; this.strictSSL = networkConfiguration.StrictSSL; @@ -47,10 +45,10 @@ export class OmnisharpDownloader { installationStage = 'downloadPackages'; // Specify the packages that the package manager needs to download this.packageManager.SetVersionPackagesForDownload(packages); - await this.packageManager.DownloadPackages(this.eventStream, this.status, this.proxy, this.strictSSL); + await this.packageManager.DownloadPackages(this.eventStream, this.proxy, this.strictSSL); installationStage = 'installPackages'; - await this.packageManager.InstallPackages(this.eventStream, this.status); + await this.packageManager.InstallPackages(this.eventStream); this.eventStream.post(new InstallationSuccess()); } @@ -58,9 +56,6 @@ export class OmnisharpDownloader { this.eventStream.post(new InstallationFailure(installationStage, error)); throw error;// throw the error up to the server } - finally { - this.status.dispose(); - } } public async GetLatestVersion(serverUrl: string, latestVersionFileServerPath: string): Promise { @@ -70,7 +65,7 @@ export class OmnisharpDownloader { //The package manager needs a package format to download, hence we form a package for the latest version file let filePackage = GetVersionFilePackage(serverUrl, latestVersionFileServerPath); //Fetch the latest version information from the file - return await this.packageManager.GetLatestVersionFromFile(this.eventStream, this.status, this.proxy, this.strictSSL, filePackage); + return await this.packageManager.GetLatestVersionFromFile(this.eventStream, this.proxy, this.strictSSL, filePackage); } catch (error) { this.eventStream.post(new InstallationFailure(installationStage, error)); diff --git a/src/omnisharp/OmnisharpPackageCreator.ts b/src/omnisharp/OmnisharpPackageCreator.ts index fb0d6bb2da..a3f6c51097 100644 --- a/src/omnisharp/OmnisharpPackageCreator.ts +++ b/src/omnisharp/OmnisharpPackageCreator.ts @@ -47,7 +47,8 @@ function GetPackage(inputPackage: Package, serverUrl: string, version: string, i "description": `${inputPackage.description}, Version = ${version}`, "url": `${serverUrl}/releases/${version}/omnisharp-${inputPackage.platformId}.zip`, "installPath": `${installPath}/${version}`, - "installTestPath": `./${installPath}/${version}/${installBinary}` + "installTestPath": `./${installPath}/${version}/${installBinary}`, + "fallBackUrl": null //setting to null so that we dont use the fallback url of the default packages }; return versionPackage; diff --git a/src/omnisharp/loggingEvents.ts b/src/omnisharp/loggingEvents.ts index df1fc2c732..13e9683d3a 100644 --- a/src/omnisharp/loggingEvents.ts +++ b/src/omnisharp/loggingEvents.ts @@ -39,7 +39,7 @@ export class LogPlatformInfo implements BaseEvent { } export class InstallationProgress implements BaseEvent { - constructor(public stage: string, public message: string) { } + constructor(public stage: string, public packageDescription: string) { } } export class InstallationFailure implements BaseEvent { @@ -47,7 +47,7 @@ export class InstallationFailure implements BaseEvent { } export class DownloadProgress implements BaseEvent { - constructor(public downloadPercentage: number) { } + constructor(public downloadPercentage: number, public packageDescription: string) { } } export class OmnisharpFailure implements BaseEvent { @@ -106,12 +106,23 @@ export class EventWithMessage implements BaseEvent { constructor(public message: string) { } } +export class DownloadStart implements BaseEvent { + constructor(public packageDescription: string) { } +} + +export class DownloadFallBack implements BaseEvent { + constructor(public fallbackUrl: string) { } +} + +export class DownloadSizeObtained implements BaseEvent { + constructor(public packageSize: number) { } +} + export class DebuggerPrerequisiteFailure extends EventWithMessage { } export class DebuggerPrerequisiteWarning extends EventWithMessage { } export class CommandDotNetRestoreProgress extends EventWithMessage { } export class CommandDotNetRestoreSucceeded extends EventWithMessage { } export class CommandDotNetRestoreFailed extends EventWithMessage { } -export class DownloadStart extends EventWithMessage { } export class DownloadSuccess extends EventWithMessage { } export class DownloadFailure extends EventWithMessage { } export class OmnisharpServerOnStdErr extends EventWithMessage { } diff --git a/src/packages.ts b/src/packages.ts index 6006f194ac..338226e459 100644 --- a/src/packages.ts +++ b/src/packages.ts @@ -13,7 +13,7 @@ import * as yauzl from 'yauzl'; import * as util from './common'; import { PlatformInformation } from './platform'; import { getProxyAgent } from './proxy'; -import { DownloadSuccess, DownloadStart, DownloadFailure, DownloadProgress, InstallationProgress } from './omnisharp/loggingEvents'; +import { DownloadSuccess, DownloadStart, DownloadFailure, DownloadProgress, InstallationProgress, DownloadFallBack, DownloadSizeObtained } from './omnisharp/loggingEvents'; import { EventStream } from './EventStream'; export interface Package { @@ -31,12 +31,6 @@ export interface Package { installTestPath?: string; } -export interface Status { - setMessage: (text: string) => void; - setDetail: (text: string) => void; - dispose?: () => void; -} - export class PackageError extends Error { // Do not put PII (personally identifiable information) in the 'message' field as it will be logged to telemetry constructor(public message: string, @@ -57,17 +51,17 @@ export class PackageManager { tmp.setGracefulCleanup(); } - public DownloadPackages(eventStream: EventStream, status: Status, proxy: string, strictSSL: boolean): Promise { + public DownloadPackages(eventStream: EventStream, proxy: string, strictSSL: boolean): Promise { return this.GetPackages() .then(packages => { - return util.buildPromiseChain(packages, pkg => maybeDownloadPackage(pkg, eventStream, status, proxy, strictSSL)); + return util.buildPromiseChain(packages, pkg => maybeDownloadPackage(pkg, eventStream, proxy, strictSSL)); }); } - public InstallPackages(eventStream: EventStream, status: Status): Promise { + public InstallPackages(eventStream: EventStream): Promise { return this.GetPackages() .then(packages => { - return util.buildPromiseChain(packages, pkg => installPackage(pkg, eventStream, status)); + return util.buildPromiseChain(packages, pkg => installPackage(pkg, eventStream)); }); } @@ -113,10 +107,10 @@ export class PackageManager { resolvePackageBinaries(this.allPackages); } - public async GetLatestVersionFromFile(eventStream: EventStream, status: Status, proxy: string, strictSSL: boolean, filePackage: Package): Promise { + public async GetLatestVersionFromFile(eventStream: EventStream, proxy: string, strictSSL: boolean, filePackage: Package): Promise { try { let latestVersion: string; - await maybeDownloadPackage(filePackage, eventStream, status, proxy, strictSSL); + await maybeDownloadPackage(filePackage, eventStream, proxy, strictSSL); if (filePackage.tmpFile) { latestVersion = fs.readFileSync(filePackage.tmpFile.name, 'utf8'); //Delete the temporary file created @@ -149,29 +143,19 @@ function getBaseInstallPath(pkg: Package): string { return basePath; } -function getNoopStatus(): Status { - return { - setMessage: text => { }, - setDetail: text => { } - }; -} - -function maybeDownloadPackage(pkg: Package, eventStream: EventStream, status: Status, proxy: string, strictSSL: boolean): Promise { +function maybeDownloadPackage(pkg: Package, eventStream: EventStream, proxy: string, strictSSL: boolean): Promise { return doesPackageTestPathExist(pkg).then((exists: boolean) => { if (!exists) { - return downloadPackage(pkg, eventStream, status, proxy, strictSSL); + return downloadPackage(pkg, eventStream, proxy, strictSSL); } else { eventStream.post(new DownloadSuccess(`Skipping package '${pkg.description}' (already downloaded).`)); } }); } -function downloadPackage(pkg: Package, eventStream: EventStream, status: Status, proxy: string, strictSSL: boolean): Promise { - status = status || getNoopStatus(); +function downloadPackage(pkg: Package, eventStream: EventStream, proxy: string, strictSSL: boolean): Promise { - eventStream.post(new DownloadStart(`Downloading package '${pkg.description}' ` )); - status.setMessage("$(cloud-download) Downloading packages"); - status.setDetail(`Downloading package '${pkg.description}'...`); + eventStream.post(new DownloadStart(pkg.description)); return new Promise((resolve, reject) => { tmp.file({ prefix: 'package-' }, (err, path, fd, cleanupCallback) => { @@ -184,17 +168,17 @@ function downloadPackage(pkg: Package, eventStream: EventStream, status: Status, }).then(tmpResult => { pkg.tmpFile = tmpResult; - let result = downloadFile(pkg.url, pkg, eventStream, status, proxy, strictSSL) - .then(() => eventStream.post(new DownloadSuccess(` Done!` ))); + let result = downloadFile(pkg.url, pkg, eventStream, proxy, strictSSL) + .then(() => eventStream.post(new DownloadSuccess(` Done!`))); // If the package has a fallback Url, and downloading from the primary Url failed, try again from // the fallback. This is used for debugger packages as some users have had issues downloading from // the CDN link. if (pkg.fallbackUrl) { result = result.catch((primaryUrlError) => { - eventStream.post(new DownloadStart(`\tRetrying from '${pkg.fallbackUrl}' `)); - return downloadFile(pkg.fallbackUrl, pkg, eventStream, status, proxy, strictSSL) - .then(() => eventStream.post(new DownloadSuccess(' Done!' ))) + eventStream.post(new DownloadFallBack(pkg.fallbackUrl)); + return downloadFile(pkg.fallbackUrl, pkg, eventStream, proxy, strictSSL) + .then(() => eventStream.post(new DownloadSuccess(' Done!'))) .catch(() => primaryUrlError); }); } @@ -203,7 +187,7 @@ function downloadPackage(pkg: Package, eventStream: EventStream, status: Status, }); } -function downloadFile(urlString: string, pkg: Package, eventStream: EventStream, status: Status, proxy: string, strictSSL: boolean): Promise { +function downloadFile(urlString: string, pkg: Package, eventStream: EventStream, proxy: string, strictSSL: boolean): Promise { const url = parseUrl(urlString); const options: https.RequestOptions = { @@ -221,12 +205,12 @@ function downloadFile(urlString: string, pkg: Package, eventStream: EventStream, let request = https.request(options, response => { if (response.statusCode === 301 || response.statusCode === 302) { // Redirect - download from new location - return resolve(downloadFile(response.headers.location, pkg, eventStream, status, proxy, strictSSL)); + return resolve(downloadFile(response.headers.location, pkg, eventStream, proxy, strictSSL)); } if (response.statusCode != 200) { // Download failed - print error message - eventStream.post(new DownloadFailure(`failed (error code '${response.statusCode}')` )); + eventStream.post(new DownloadFailure(`failed (error code '${response.statusCode}')`)); return reject(new PackageError(response.statusCode.toString(), pkg)); } @@ -236,7 +220,7 @@ function downloadFile(urlString: string, pkg: Package, eventStream: EventStream, let downloadPercentage = 0; let tmpFile = fs.createWriteStream(null, { fd: pkg.tmpFile.fd }); - eventStream.post(new DownloadStart(`(${Math.ceil(packageSize / 1024)} KB) ` )); + eventStream.post(new DownloadSizeObtained(packageSize)); response.on('data', data => { downloadedBytes += data.length; @@ -244,11 +228,9 @@ function downloadFile(urlString: string, pkg: Package, eventStream: EventStream, // Update status bar item with percentage let newPercentage = Math.ceil(100 * (downloadedBytes / packageSize)); if (newPercentage !== downloadPercentage) { - status.setDetail(`Downloading package '${pkg.description}'... ${downloadPercentage}%`); downloadPercentage = newPercentage; + eventStream.post(new DownloadProgress(downloadPercentage, pkg.description)); } - - eventStream.post(new DownloadProgress(downloadPercentage)); }); response.on('end', () => { @@ -272,19 +254,14 @@ function downloadFile(urlString: string, pkg: Package, eventStream: EventStream, }); } -function installPackage(pkg: Package, eventStream: EventStream, status?: Status): Promise { +function installPackage(pkg: Package, eventStream: EventStream): Promise { const installationStage = 'installPackages'; if (!pkg.tmpFile) { // Download of this package was skipped, so there is nothing to install return Promise.resolve(); } - status = status || getNoopStatus(); - - eventStream.post(new InstallationProgress(installationStage, `Installing package '${pkg.description}'`)); - - status.setMessage("$(desktop-download) Installing packages..."); - status.setDetail(`Installing package '${pkg.description}'`); + eventStream.post(new InstallationProgress(installationStage, pkg.description)); return new Promise((resolve, baseReject) => { const reject = (err) => { From c8b5e769892d8b05b8d6872b2a7370cdf9502a03 Mon Sep 17 00:00:00 2001 From: Unknown Date: Fri, 6 Apr 2018 12:48:46 -0700 Subject: [PATCH 2/8] Tests failing --- .../logging/CsharpLoggerObserver.test.ts | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/test/unitTests/logging/CsharpLoggerObserver.test.ts b/test/unitTests/logging/CsharpLoggerObserver.test.ts index 96d46237b0..ee4a6ce4bb 100644 --- a/test/unitTests/logging/CsharpLoggerObserver.test.ts +++ b/test/unitTests/logging/CsharpLoggerObserver.test.ts @@ -53,7 +53,8 @@ suite("CsharpLoggerObserver", () => { }); }); - suite('Download',() => { + suite('Download', () => { + let packageName = "somePackage"; [ { events: [], @@ -64,31 +65,31 @@ suite("CsharpLoggerObserver", () => { expected: "Started" }, { - events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(100)], + events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(100, packageName)], expected: "Started...................." }, { - events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(10), new Event.DownloadProgress(50), new Event.DownloadProgress(100)], + events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(10, packageName), new Event.DownloadProgress(50, packageName), new Event.DownloadProgress(100, packageName)], expected: "Started...................." }, { - events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(10), new Event.DownloadProgress(50)], + events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(10, packageName), new Event.DownloadProgress(50, packageName)], expected: "Started.........." }, { - events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(50)], + events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(50, packageName)], expected: "Started.........." }, { - events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(50), new Event.DownloadProgress(50), new Event.DownloadProgress(50)], + events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(50, packageName), new Event.DownloadProgress(50, packageName), new Event.DownloadProgress(50, packageName)], expected: "Started.........." }, { - events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(100), new Event.DownloadSuccess("Done")], + events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(100, packageName), new Event.DownloadSuccess("Done")], expected: "Started....................Done\n" }, { - events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(50), new Event.DownloadFailure("Failed")], + events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(50, packageName), new Event.DownloadFailure("Failed")], expected: "Started..........Failed\n" }, ].forEach((element) => { @@ -141,9 +142,9 @@ suite("CsharpLoggerObserver", () => { }); test(`InstallationProgress: Progress message is logged`, () => { - let event = new Event.InstallationProgress("someStage", "someMessage"); + let event = new Event.InstallationProgress("someStage", "somPackage"); observer.post(event); - expect(logOutput).to.contain(event.message); + expect(logOutput).to.contain(event.packageDescription); }); test('PackageInstallation: Package name is logged', () => { From 0a956b98fa83c71fa38fd51082eaec86df31bbe7 Mon Sep 17 00:00:00 2001 From: Akshita Agarwal Date: Fri, 6 Apr 2018 16:07:20 -0700 Subject: [PATCH 3/8] Added tests --- src/observers/CsharpLoggerObserver.ts | 4 +- .../logging/CsharpLoggerObserver.test.ts | 40 +++++++++++-------- .../OmnisharpStatusBarObserver.test.ts | 29 ++++++++++++-- 3 files changed, 51 insertions(+), 22 deletions(-) diff --git a/src/observers/CsharpLoggerObserver.ts b/src/observers/CsharpLoggerObserver.ts index 9618b33a14..8940aae173 100644 --- a/src/observers/CsharpLoggerObserver.ts +++ b/src/observers/CsharpLoggerObserver.ts @@ -55,7 +55,7 @@ export class CsharpLoggerObserver extends BaseLoggerObserver { } private handleDownloadSizeObtained(event: Event.DownloadSizeObtained) { - this.logger.append(`(${Math.ceil(event.packageSize / 1024)} KB) `); + this.logger.append(`(${Math.ceil(event.packageSize / 1024)} KB)`); } private handleDownloadFallback(event: Event.DownloadFallBack) { @@ -100,7 +100,7 @@ export class CsharpLoggerObserver extends BaseLoggerObserver { } private handleDownloadStart(event: Event.DownloadStart) { - this.logger.append(`Downloading package '${event.packageDescription}'... `); + this.logger.append(`Downloading package '${event.packageDescription}' `); this.dots = 0; } diff --git a/test/unitTests/logging/CsharpLoggerObserver.test.ts b/test/unitTests/logging/CsharpLoggerObserver.test.ts index ee4a6ce4bb..4b2414a56d 100644 --- a/test/unitTests/logging/CsharpLoggerObserver.test.ts +++ b/test/unitTests/logging/CsharpLoggerObserver.test.ts @@ -61,37 +61,37 @@ suite("CsharpLoggerObserver", () => { expected: "" }, { - events: [new Event.DownloadStart("Started")], - expected: "Started" + events: [new Event.DownloadStart("somePackage")], + expected: "Downloading package 'somePackage' " }, { - events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(100, packageName)], - expected: "Started...................." + events: [new Event.DownloadStart("somePackage"), new Event.DownloadSizeObtained(500), new Event.DownloadProgress(100, packageName)], + expected: "Downloading package 'somePackage' (1 KB)...................." }, { - events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(10, packageName), new Event.DownloadProgress(50, packageName), new Event.DownloadProgress(100, packageName)], - expected: "Started...................." + events: [new Event.DownloadStart("somePackage"), new Event.DownloadSizeObtained(500), new Event.DownloadProgress(10, packageName), new Event.DownloadProgress(50, packageName), new Event.DownloadProgress(100, packageName)], + expected: "Downloading package 'somePackage' (1 KB)...................." }, { - events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(10, packageName), new Event.DownloadProgress(50, packageName)], - expected: "Started.........." + events: [new Event.DownloadStart("somePackage"), new Event.DownloadSizeObtained(500), new Event.DownloadProgress(10, packageName), new Event.DownloadProgress(50, packageName)], + expected: "Downloading package 'somePackage' (1 KB).........." }, { - events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(50, packageName)], - expected: "Started.........." + events: [new Event.DownloadStart("somePackage"), new Event.DownloadSizeObtained(2000), new Event.DownloadProgress(50, packageName)], + expected: "Downloading package 'somePackage' (2 KB).........." }, { - events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(50, packageName), new Event.DownloadProgress(50, packageName), new Event.DownloadProgress(50, packageName)], - expected: "Started.........." + events: [new Event.DownloadStart("somePackage"), new Event.DownloadSizeObtained(2000), new Event.DownloadProgress(50, packageName), new Event.DownloadProgress(50, packageName), new Event.DownloadProgress(50, packageName)], + expected: "Downloading package 'somePackage' (2 KB).........." }, { - events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(100, packageName), new Event.DownloadSuccess("Done")], - expected: "Started....................Done\n" + events: [new Event.DownloadStart("somePackage"), new Event.DownloadSizeObtained(3000), new Event.DownloadProgress(100, packageName), new Event.DownloadSuccess("Done")], + expected: "Downloading package 'somePackage' (3 KB)....................Done\n" }, { - events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(50, packageName), new Event.DownloadFailure("Failed")], - expected: "Started..........Failed\n" - }, + events: [new Event.DownloadStart("somePackage"), new Event.DownloadSizeObtained(4000), new Event.DownloadProgress(50, packageName), new Event.DownloadFailure("Failed")], + expected: "Downloading package 'somePackage' (4 KB)..........Failed\n" + } ].forEach((element) => { test(`Prints the download status to the logger as ${element.expected}`, () => { let logOutput = ""; @@ -152,4 +152,10 @@ suite("CsharpLoggerObserver", () => { observer.post(event); expect(logOutput).to.contain(event.packageInfo); }); + + test('DownloadFallBack: The fallbackurl is logged', () => { + let event = new Event.DownloadFallBack("somrurl"); + observer.post(event); + expect(logOutput).to.contain(event.fallbackUrl); + }); }); diff --git a/test/unitTests/logging/OmnisharpStatusBarObserver.test.ts b/test/unitTests/logging/OmnisharpStatusBarObserver.test.ts index 35b56199af..a66257128e 100644 --- a/test/unitTests/logging/OmnisharpStatusBarObserver.test.ts +++ b/test/unitTests/logging/OmnisharpStatusBarObserver.test.ts @@ -4,19 +4,21 @@ *--------------------------------------------------------------------------------------------*/ import { DocumentSelector, StatusBarItem } from '../../../src/vscodeAdapter'; -import { OmnisharpOnBeforeServerInstall, OmnisharpOnBeforeServerStart, OmnisharpOnMultipleLaunchTargets, OmnisharpServerOnServerError, OmnisharpServerOnStart, OmnisharpServerOnStop } from '../../../src/omnisharp/loggingEvents'; +import { OmnisharpOnBeforeServerInstall, OmnisharpOnBeforeServerStart, OmnisharpOnMultipleLaunchTargets, OmnisharpServerOnServerError, OmnisharpServerOnStart, OmnisharpServerOnStop, DownloadStart, InstallationProgress, DownloadProgress } from '../../../src/omnisharp/loggingEvents'; import { expect, should } from 'chai'; import { OmnisharpStatusBarObserver } from '../../../src/observers/OmnisharpStatusBarObserver'; import { getFakeVsCode, getWorkspaceInformationUpdated, getMSBuildWorkspaceInformation } from './Fakes'; suite('OmnisharpStatusBarObserver', () => { suiteSetup(() => should()); - let output = ''; let showCalled: boolean; let hideCalled: boolean; setup(() => { - output = ''; + statusBarItem.text = undefined; + statusBarItem.color = undefined; + statusBarItem.command = undefined; + statusBarItem.tooltip = undefined; showCalled = false; hideCalled = false; }); @@ -68,4 +70,25 @@ suite('OmnisharpStatusBarObserver', () => { expect(statusBarItem.command).to.be.undefined; expect(statusBarItem.color).to.be.undefined; }); + + test('DownloadStart: Text and tooltip are set ', () => { + let event = new DownloadStart("somePackage"); + observer.post(event); + expect(statusBarItem.text).to.contain("Downloading packages"); + expect(statusBarItem.tooltip).to.contain(event.packageDescription); + }); + + test('InstallationProgress: Text and tooltip are set', () => { + let event = new InstallationProgress("someStage", "somePackage"); + observer.post(event); + expect(statusBarItem.text).to.contain("Installing packages"); + expect(statusBarItem.tooltip).to.contain(event.packageDescription); + }); + + test('DownloadProgress: Tooltip contains package description and download percentage', () => { + let event = new DownloadProgress(50, "somePackage"); + observer.post(event); + expect(statusBarItem.tooltip).to.contain(event.packageDescription); + expect(statusBarItem.tooltip).to.contain(event.downloadPercentage); + }); }); \ No newline at end of file From 0db6bddd7bc6625653705dc5088de583829aee78 Mon Sep 17 00:00:00 2001 From: Akshita Agarwal Date: Fri, 6 Apr 2018 16:43:59 -0700 Subject: [PATCH 4/8] Used tooltip and changed flame color --- src/observers/BaseStatusBarItemObserver.ts | 9 +++------ src/observers/OmnisharpStatusBarObserver.ts | 16 +++++++--------- src/observers/ProjectStatusBarObserver.ts | 4 ++-- .../logging/OmnisharpStatusBarObserver.test.ts | 9 ++++++--- 4 files changed, 18 insertions(+), 20 deletions(-) diff --git a/src/observers/BaseStatusBarItemObserver.ts b/src/observers/BaseStatusBarItemObserver.ts index 974e9360ad..610f38170d 100644 --- a/src/observers/BaseStatusBarItemObserver.ts +++ b/src/observers/BaseStatusBarItemObserver.ts @@ -11,10 +11,11 @@ export abstract class BaseStatusBarItemObserver { constructor(private statusBarItem: StatusBarItem) { } - public SetTextAndShowStatusBar(text: string, command?: string, color?: string) { + public SetAndShowStatusBar(text: string, command: string, color: string, tooltip: string) { this.statusBarItem.text = text; this.statusBarItem.command = command; this.statusBarItem.color = color; + this.statusBarItem.tooltip = tooltip; this.statusBarItem.show(); } @@ -22,13 +23,9 @@ export abstract class BaseStatusBarItemObserver { this.statusBarItem.text = undefined; this.statusBarItem.command = undefined; this.statusBarItem.color = undefined; + this.statusBarItem.tooltip = undefined; this.statusBarItem.hide(); } - public SetToolTipAndShowStatusBar(text: string) { - this.statusBarItem.tooltip = text; - this.statusBarItem.show(); - } - abstract post: (event: BaseEvent) => void; } \ No newline at end of file diff --git a/src/observers/OmnisharpStatusBarObserver.ts b/src/observers/OmnisharpStatusBarObserver.ts index 560547d73f..feb0e32273 100644 --- a/src/observers/OmnisharpStatusBarObserver.ts +++ b/src/observers/OmnisharpStatusBarObserver.ts @@ -10,31 +10,29 @@ export class OmnisharpStatusBarObserver extends BaseStatusBarItemObserver { public post = (event: BaseEvent) => { switch (event.constructor.name) { case OmnisharpServerOnServerError.name: - this.SetTextAndShowStatusBar('$(flame) Error starting OmniSharp', 'o.showOutput', ''); + this.SetAndShowStatusBar('$(flame)', 'o.showOutput', 'rgb(218,0,0)', 'Error starting OmniSharp'); break; case OmnisharpOnBeforeServerInstall.name: - this.SetTextAndShowStatusBar('$(flame) Installing OmniSharp...', 'o.showOutput', ''); + this.SetAndShowStatusBar('$(flame) Installing OmniSharp...', 'o.showOutput', '', ''); break; case OmnisharpOnBeforeServerStart.name: - this.SetTextAndShowStatusBar('$(flame) Starting...', 'o.showOutput', ''); + this.SetAndShowStatusBar('$(flame)', 'o.showOutput', 'rgb(218,218,0)', 'Starting OmniSharp server'); break; case OmnisharpServerOnStop.name: this.ResetAndHideStatusBar(); break; case OmnisharpServerOnStart.name: - this.SetTextAndShowStatusBar('$(flame) Running', 'o.showOutput', ''); + this.SetAndShowStatusBar('$(flame)', 'o.showOutput', 'rgb(0, 218, 0)', 'OmniSharp server is Running'); break; case DownloadStart.name: - this.SetTextAndShowStatusBar("$(cloud-download) Downloading packages"); - this.SetToolTipAndShowStatusBar(`Downloading package '${(event).packageDescription}...' `); + this.SetAndShowStatusBar("$(cloud-download) Downloading packages", '', '', `Downloading package '${(event).packageDescription}...' `); break; case InstallationProgress.name: - this.SetTextAndShowStatusBar("$(desktop-download) Installing packages..."); - this.SetToolTipAndShowStatusBar(`Installing package '${(event).packageDescription}'`); + this.SetAndShowStatusBar("$(desktop-download) Installing packages...", '', '', `Installing package '${(event).packageDescription}'`); break; case DownloadProgress.name: let progressEvent = event; - this.SetToolTipAndShowStatusBar(`Downloading package '${progressEvent.packageDescription}'... ${progressEvent.downloadPercentage}%`); + this.SetAndShowStatusBar("$(cloud-download) Downloading packages", '', '', `Downloading package '${progressEvent.packageDescription}'... ${progressEvent.downloadPercentage}%`); break; } } diff --git a/src/observers/ProjectStatusBarObserver.ts b/src/observers/ProjectStatusBarObserver.ts index 91a62eda1d..3479c596db 100644 --- a/src/observers/ProjectStatusBarObserver.ts +++ b/src/observers/ProjectStatusBarObserver.ts @@ -12,7 +12,7 @@ export class ProjectStatusBarObserver extends BaseStatusBarItemObserver { public post = (event: BaseEvent) => { switch (event.constructor.name) { case OmnisharpOnMultipleLaunchTargets.name: - this.SetTextAndShowStatusBar('$(file-submodule) Select project', 'o.pickProjectAndStart', 'rgb(90, 218, 90)'); + this.SetAndShowStatusBar('$(file-submodule) Select project', 'o.pickProjectAndStart', 'rgb(90, 218, 90)', ''); break; case OmnisharpServerOnStop.name: this.ResetAndHideStatusBar(); @@ -27,7 +27,7 @@ export class ProjectStatusBarObserver extends BaseStatusBarItemObserver { let info = event.info; if (info.MsBuild && info.MsBuild.SolutionPath) { label = basename(info.MsBuild.SolutionPath); //workspace.getRelativePath(info.MsBuild.SolutionPath); - this.SetTextAndShowStatusBar('$(file-directory) ' + label, 'o.pickProjectAndStart'); + this.SetAndShowStatusBar('$(file-directory) ' + label, 'o.pickProjectAndStart', '', ''); } else { this.ResetAndHideStatusBar(); diff --git a/test/unitTests/logging/OmnisharpStatusBarObserver.test.ts b/test/unitTests/logging/OmnisharpStatusBarObserver.test.ts index a66257128e..01cba9f0da 100644 --- a/test/unitTests/logging/OmnisharpStatusBarObserver.test.ts +++ b/test/unitTests/logging/OmnisharpStatusBarObserver.test.ts @@ -34,8 +34,9 @@ suite('OmnisharpStatusBarObserver', () => { let event = new OmnisharpServerOnServerError("someError"); observer.post(event); expect(showCalled).to.be.true; - expect(statusBarItem.text).to.equal(`$(flame) Error starting OmniSharp`); + expect(statusBarItem.text).to.equal(`$(flame)`); expect(statusBarItem.command).to.equal('o.showOutput'); + expect(statusBarItem.tooltip).to.equal('Error starting OmniSharp'); }); test('OnBeforeServerInstall: Status bar is shown with the installation text', () => { @@ -50,16 +51,18 @@ suite('OmnisharpStatusBarObserver', () => { let event = new OmnisharpOnBeforeServerStart(); observer.post(event); expect(showCalled).to.be.true; - expect(statusBarItem.text).to.be.equal('$(flame) Starting...'); + expect(statusBarItem.text).to.be.equal('$(flame)'); expect(statusBarItem.command).to.equal('o.showOutput'); + expect(statusBarItem.tooltip).to.equal('Starting OmniSharp server'); }); test('OnServerStart: Status bar is shown with the flame and "Running" text', () => { let event = new OmnisharpServerOnStart(); observer.post(event); expect(showCalled).to.be.true; - expect(statusBarItem.text).to.be.equal('$(flame) Running'); + expect(statusBarItem.text).to.be.equal('$(flame)'); expect(statusBarItem.command).to.equal('o.showOutput'); + expect(statusBarItem.tooltip).to.be.equal('OmniSharp server is Running'); }); test('OnServerStop: Status bar is hidden and the attributes are set to undefined', () => { From c7c8168aa1188bbc53b073e595a8240743c3caa7 Mon Sep 17 00:00:00 2001 From: Akshita Agarwal Date: Fri, 6 Apr 2018 16:52:31 -0700 Subject: [PATCH 5/8] fix merge problems --- tasks/offlinePackagingTasks.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tasks/offlinePackagingTasks.ts b/tasks/offlinePackagingTasks.ts index 8be7c44da3..aeafc23583 100644 --- a/tasks/offlinePackagingTasks.ts +++ b/tasks/offlinePackagingTasks.ts @@ -98,9 +98,9 @@ function install(platformInfo, packageJSON) { eventStream.subscribe(stdoutObserver.post); const debuggerUtil = new debugUtil.CoreClrDebugUtil(path.resolve('.')); - return packageManager.DownloadPackages(eventStream, undefined, undefined, undefined) + return packageManager.DownloadPackages(eventStream, undefined, undefined) .then(() => { - return packageManager.InstallPackages(eventStream, undefined); + return packageManager.InstallPackages(eventStream); }) .then(() => { From ab4e7354fa94aaea44601888ef1c373f5d871fab Mon Sep 17 00:00:00 2001 From: Akshita Agarwal Date: Mon, 9 Apr 2018 13:54:38 -0700 Subject: [PATCH 6/8] PR feedback --- src/observers/BaseStatusBarItemObserver.ts | 2 +- src/observers/OmnisharpStatusBarObserver.ts | 4 ++-- src/observers/ProjectStatusBarObserver.ts | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/observers/BaseStatusBarItemObserver.ts b/src/observers/BaseStatusBarItemObserver.ts index 610f38170d..3919f163d5 100644 --- a/src/observers/BaseStatusBarItemObserver.ts +++ b/src/observers/BaseStatusBarItemObserver.ts @@ -11,7 +11,7 @@ export abstract class BaseStatusBarItemObserver { constructor(private statusBarItem: StatusBarItem) { } - public SetAndShowStatusBar(text: string, command: string, color: string, tooltip: string) { + public SetAndShowStatusBar(text: string, command: string, color?: string, tooltip?: string) { this.statusBarItem.text = text; this.statusBarItem.command = command; this.statusBarItem.color = color; diff --git a/src/observers/OmnisharpStatusBarObserver.ts b/src/observers/OmnisharpStatusBarObserver.ts index feb0e32273..2d5cab6614 100644 --- a/src/observers/OmnisharpStatusBarObserver.ts +++ b/src/observers/OmnisharpStatusBarObserver.ts @@ -13,7 +13,7 @@ export class OmnisharpStatusBarObserver extends BaseStatusBarItemObserver { this.SetAndShowStatusBar('$(flame)', 'o.showOutput', 'rgb(218,0,0)', 'Error starting OmniSharp'); break; case OmnisharpOnBeforeServerInstall.name: - this.SetAndShowStatusBar('$(flame) Installing OmniSharp...', 'o.showOutput', '', ''); + this.SetAndShowStatusBar('$(flame) Installing OmniSharp...', 'o.showOutput'); break; case OmnisharpOnBeforeServerStart.name: this.SetAndShowStatusBar('$(flame)', 'o.showOutput', 'rgb(218,218,0)', 'Starting OmniSharp server'); @@ -22,7 +22,7 @@ export class OmnisharpStatusBarObserver extends BaseStatusBarItemObserver { this.ResetAndHideStatusBar(); break; case OmnisharpServerOnStart.name: - this.SetAndShowStatusBar('$(flame)', 'o.showOutput', 'rgb(0, 218, 0)', 'OmniSharp server is Running'); + this.SetAndShowStatusBar('$(flame)', 'o.showOutput', 'rgb(0, 218, 0)', 'OmniSharp server is running'); break; case DownloadStart.name: this.SetAndShowStatusBar("$(cloud-download) Downloading packages", '', '', `Downloading package '${(event).packageDescription}...' `); diff --git a/src/observers/ProjectStatusBarObserver.ts b/src/observers/ProjectStatusBarObserver.ts index 3479c596db..fd37b7b20e 100644 --- a/src/observers/ProjectStatusBarObserver.ts +++ b/src/observers/ProjectStatusBarObserver.ts @@ -12,7 +12,7 @@ export class ProjectStatusBarObserver extends BaseStatusBarItemObserver { public post = (event: BaseEvent) => { switch (event.constructor.name) { case OmnisharpOnMultipleLaunchTargets.name: - this.SetAndShowStatusBar('$(file-submodule) Select project', 'o.pickProjectAndStart', 'rgb(90, 218, 90)', ''); + this.SetAndShowStatusBar('$(file-submodule) Select project', 'o.pickProjectAndStart', 'rgb(90, 218, 90)'); break; case OmnisharpServerOnStop.name: this.ResetAndHideStatusBar(); @@ -27,7 +27,7 @@ export class ProjectStatusBarObserver extends BaseStatusBarItemObserver { let info = event.info; if (info.MsBuild && info.MsBuild.SolutionPath) { label = basename(info.MsBuild.SolutionPath); //workspace.getRelativePath(info.MsBuild.SolutionPath); - this.SetAndShowStatusBar('$(file-directory) ' + label, 'o.pickProjectAndStart', '', ''); + this.SetAndShowStatusBar('$(file-directory) ' + label, 'o.pickProjectAndStart'); } else { this.ResetAndHideStatusBar(); From 796fa805999fb5db0eefb25c5a420edfa0a76fe4 Mon Sep 17 00:00:00 2001 From: Akshita Agarwal Date: Mon, 9 Apr 2018 14:32:46 -0700 Subject: [PATCH 7/8] Removed implicit any --- src/omnisharp/OmnisharpPackageCreator.ts | 2 +- src/packages.ts | 2 +- test/featureTests/OmnisharpPackageCreator.test.ts | 3 ++- test/unitTests/logging/OmnisharpStatusBarObserver.test.ts | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/omnisharp/OmnisharpPackageCreator.ts b/src/omnisharp/OmnisharpPackageCreator.ts index a3f6c51097..76774f113a 100644 --- a/src/omnisharp/OmnisharpPackageCreator.ts +++ b/src/omnisharp/OmnisharpPackageCreator.ts @@ -48,7 +48,7 @@ function GetPackage(inputPackage: Package, serverUrl: string, version: string, i "url": `${serverUrl}/releases/${version}/omnisharp-${inputPackage.platformId}.zip`, "installPath": `${installPath}/${version}`, "installTestPath": `./${installPath}/${version}/${installBinary}`, - "fallBackUrl": null //setting to null so that we dont use the fallback url of the default packages + "fallbackUrl": "" //setting to empty so that we dont use the fallback url of the default packages }; return versionPackage; diff --git a/src/packages.ts b/src/packages.ts index 8c887dc1ea..210a9a32fb 100644 --- a/src/packages.ts +++ b/src/packages.ts @@ -176,7 +176,7 @@ async function downloadPackage(pkg: Package, eventStream: EventStream, proxy: st // the CDN link. if (pkg.fallbackUrl) { result = result.catch(async (primaryUrlError) => { - eventStream.post(new DownloadStart(`\tRetrying from '${pkg.fallbackUrl}' `)); + eventStream.post(new DownloadFallBack(pkg.fallbackUrl)); return downloadFile(pkg.fallbackUrl, pkg, eventStream, proxy, strictSSL) .then(() => eventStream.post(new DownloadSuccess(' Done!' ))) .catch(() => primaryUrlError); diff --git a/test/featureTests/OmnisharpPackageCreator.test.ts b/test/featureTests/OmnisharpPackageCreator.test.ts index c2f54b1c2d..4ae3ae1653 100644 --- a/test/featureTests/OmnisharpPackageCreator.test.ts +++ b/test/featureTests/OmnisharpPackageCreator.test.ts @@ -36,13 +36,14 @@ suite("GetOmnisharpPackage : Output package depends on the input package and oth expect(fn).to.throw('Invalid version'); }); - test('Architectures, binaries and platforms do not change', () => { + test('Architectures, binaries and platforms do not change and fallback url is empty', () => { let testPackage = inputPackages.find(element => (element.platformId && element.platformId == "os-architecture")); let resultPackage = SetBinaryAndGetPackage(testPackage, serverUrl, version, installPath); resultPackage.architectures.should.equal(testPackage.architectures); assert.equal(resultPackage.binaries, testPackage.binaries); resultPackage.platforms.should.equal(testPackage.platforms); + expect(resultPackage.fallbackUrl).should.be.empty; }); test('Version information is appended to the description', () => { diff --git a/test/unitTests/logging/OmnisharpStatusBarObserver.test.ts b/test/unitTests/logging/OmnisharpStatusBarObserver.test.ts index da7f484210..afd03f758c 100644 --- a/test/unitTests/logging/OmnisharpStatusBarObserver.test.ts +++ b/test/unitTests/logging/OmnisharpStatusBarObserver.test.ts @@ -61,7 +61,7 @@ suite('OmnisharpStatusBarObserver', () => { expect(showCalled).to.be.true; expect(statusBarItem.text).to.be.equal('$(flame)'); expect(statusBarItem.command).to.equal('o.showOutput'); - expect(statusBarItem.tooltip).to.be.equal('OmniSharp server is Running'); + expect(statusBarItem.tooltip).to.be.equal('OmniSharp server is running'); }); test('OnServerStop: Status bar is hidden and the attributes are set to undefined', () => { From 3c04121652e502aeafaceaa8089527aedca9eeaa Mon Sep 17 00:00:00 2001 From: Akshita Agarwal Date: Mon, 9 Apr 2018 14:42:46 -0700 Subject: [PATCH 8/8] Changes --- test/featureTests/OmnisharpPackageCreator.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/featureTests/OmnisharpPackageCreator.test.ts b/test/featureTests/OmnisharpPackageCreator.test.ts index 4ae3ae1653..09e0961825 100644 --- a/test/featureTests/OmnisharpPackageCreator.test.ts +++ b/test/featureTests/OmnisharpPackageCreator.test.ts @@ -43,7 +43,7 @@ suite("GetOmnisharpPackage : Output package depends on the input package and oth resultPackage.architectures.should.equal(testPackage.architectures); assert.equal(resultPackage.binaries, testPackage.binaries); resultPackage.platforms.should.equal(testPackage.platforms); - expect(resultPackage.fallbackUrl).should.be.empty; + expect(resultPackage.fallbackUrl).to.be.empty; }); test('Version information is appended to the description', () => {