CR feedback

Author: akshita31

src/features/structureProvider.ts

@@ -1,3 +1,8 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
 import { FoldingRangeProvider, TextDocument, FoldingContext, CancellationToken, FoldingRange, FoldingRangeKind } from "vscode";
 import AbstractSupport from './abstractProvider';
 import { blockStructure } from "../omnisharp/utils";

src/packageManager/AbsolutePath.ts

@@ -12,7 +12,7 @@ export class AbsolutePath{
         }
     }
 
-    public static getAbsolutePath(pathToPrepend: string, path: string): AbsolutePath {
-        return new AbsolutePath(resolve(pathToPrepend, path));
+    public static getAbsolutePath(...pathSegments: string[]): AbsolutePath {
+        return new AbsolutePath(resolve(...pathSegments));
     }
 }

src/packageManager/InstallablePackage.ts renamed to src/packageManager/AbsolutePathPackage.ts

@@ -3,10 +3,11 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { IPackage, Package } from "./Package";
+import { Package } from "./Package";
+import { IPackage } from "./IPackage";
 import { AbsolutePath } from "./AbsolutePath";
 
-export class InstallablePackage implements IPackage{
+export class AbsolutePathPackage implements IPackage{
     constructor(public description: string,
         public url: string,
         public platforms: string[],
@@ -18,8 +19,8 @@ export class InstallablePackage implements IPackage{
         public platformId?: string) {
     }
 
-    public static getInstallablePackage(pkg: Package, extensionPath: string) {
-        return new InstallablePackage(
+    public static getAbsolutePathPackage(pkg: Package, extensionPath: string) {
+        return new AbsolutePathPackage(
             pkg.description,
             pkg.url,
             pkg.platforms,

src/packageManager/IPackage.ts

@@ -0,0 +1,13 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+export interface IPackage {
+    description: string;
+    url: string;
+    fallbackUrl?: string;
+    platforms: string[];
+    architectures: string[];
+    platformId?: string;
+}

src/packageManager/Package.ts

@@ -3,14 +3,7 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-export interface IPackage {
-    description: string;
-    url: string;
-    fallbackUrl?: string;
-    platforms: string[];
-    architectures: string[];
-    platformId?: string;
-}
+import { IPackage } from "./IPackage";
 
 export interface Package extends IPackage{
     installPath?: string;

src/packageManager/PackageError.ts

@@ -4,7 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 import { NestedError } from '../NestedError';
-import { IPackage } from './Package';
+import { IPackage } from "./IPackage";
 
 export class PackageError extends NestedError {
     // Do not put PII (personally identifiable information) in the 'message' field as it will be logged to telemetry

src/packageManager/PackageFilterer.ts

@@ -6,16 +6,16 @@
 import { PlatformInformation } from "../platform";
 import * as util from '../common';
 import { PackageError } from "./PackageError";
-import { InstallablePackage } from "./InstallablePackage";
+import { AbsolutePathPackage } from "./AbsolutePathPackage";
 
 const { filterAsync } = require('node-filter-async');
 
-export async function filterPackages(packages: InstallablePackage[], platformInfo: PlatformInformation): Promise<InstallablePackage[]> {
+export async function filterPackages(packages: AbsolutePathPackage[], platformInfo: PlatformInformation): Promise<AbsolutePathPackage[]> {
     let platformPackages = filterPlatformPackages(packages, platformInfo);
     return filterAlreadyInstalledPackages(platformPackages);
 }
 
-function filterPlatformPackages(packages: InstallablePackage[], platformInfo: PlatformInformation) {
+function filterPlatformPackages(packages: AbsolutePathPackage[], platformInfo: PlatformInformation) {
     if (packages) {
         return packages.filter(pkg => {
             if (pkg.architectures && pkg.architectures.indexOf(platformInfo.architecture) === -1) {
@@ -34,8 +34,8 @@ function filterPlatformPackages(packages: InstallablePackage[], platformInfo: Pl
     }
 }
 
-async function filterAlreadyInstalledPackages(packages: InstallablePackage[]): Promise<InstallablePackage[]> {
-    return filterAsync(packages, async (pkg: InstallablePackage) => {
+async function filterAlreadyInstalledPackages(packages: AbsolutePathPackage[]): Promise<AbsolutePathPackage[]> {
+    return filterAsync(packages, async (pkg: AbsolutePathPackage) => {
         //If the file is present at the install test path then filter it
         let testPath = pkg.installTestPath;
         if (!testPath) {

src/packageManager/PackageManager.ts

@@ -12,11 +12,11 @@ import { InstallZip } from './ZipInstaller';
 import { EventStream } from '../EventStream';
 import { NetworkSettingsProvider } from "../NetworkSettings";
 import { filterPackages } from "./PackageFilterer";
-import { InstallablePackage } from "./InstallablePackage";
+import { AbsolutePathPackage } from "./AbsolutePathPackage";
 
 export async function DownloadAndInstallPackages(packages: Package[], provider: NetworkSettingsProvider, platformInfo: PlatformInformation, eventStream: EventStream, extensionPath: string) {
-    let installablePackages = packages.map(pkg => InstallablePackage.getInstallablePackage(pkg, extensionPath));
-    let filteredPackages = await filterPackages(installablePackages, platformInfo);
+    let absolutePathPackages = packages.map(pkg => AbsolutePathPackage.getAbsolutePathPackage(pkg, extensionPath));
+    let filteredPackages = await filterPackages(absolutePathPackages, platformInfo);
     if (filteredPackages) {
         for (let pkg of filteredPackages) {
             try {

test/unitTests/Packages/PackageFilterer.test.ts

@@ -7,14 +7,14 @@ import { CreateTmpFile, TmpAsset } from "../../../src/CreateTmpAsset";
 import { PlatformInformation } from "../../../src/platform";
 import { filterPackages } from "../../../src/packageManager/PackageFilterer";
 import { Package } from '../../../src/packageManager/Package';
-import { InstallablePackage } from '../../../src/packageManager/InstallablePackage';
+import { AbsolutePathPackage } from '../../../src/packageManager/AbsolutePathPackage';
 import { AbsolutePath } from '../../../src/packageManager/AbsolutePath';
 
 let expect = chai.expect;
 
 suite('PackageFilterer', () => {
     let tmpFile: TmpAsset;
-    let installablePackages: InstallablePackage[];
+    let absolutePathPackage: AbsolutePathPackage[];
     const extensionPath = "/ExtensionPath";
     const packages = <Package[]>[
         {   
@@ -63,13 +63,13 @@ suite('PackageFilterer', () => {
 
     setup(async () => {
         tmpFile = await CreateTmpFile();
-        installablePackages = packages.map(pkg => InstallablePackage.getInstallablePackage(pkg, extensionPath));
-        installablePackages[1].installTestPath = new AbsolutePath(tmpFile.name);
+        absolutePathPackage = packages.map(pkg => AbsolutePathPackage.getAbsolutePathPackage(pkg, extensionPath));
+        absolutePathPackage[1].installTestPath = new AbsolutePath(tmpFile.name);
     });
 
     test('Filters the packages based on Platform Information', async () => {
         let platformInfo = new PlatformInformation("platform2", "architecture2");
-        let filteredPackages = await filterPackages(installablePackages, platformInfo);
+        let filteredPackages = await filterPackages(absolutePathPackage, platformInfo);
         expect(filteredPackages.length).to.be.equal(1);
         expect(filteredPackages[0].description).to.be.equal("Platfrom2-Architecture2 uninstalled package");
         expect(filteredPackages[0].platforms[0]).to.be.equal("platform2");
@@ -78,7 +78,7 @@ suite('PackageFilterer', () => {
 
     test('Returns only uninstalled packages', async () => {
         let platformInfo = new PlatformInformation("platform1", "architecture1");
-        let filteredPackages = await filterPackages(installablePackages, platformInfo);
+        let filteredPackages = await filterPackages(absolutePathPackage, platformInfo);
         expect(filteredPackages.length).to.be.equal(1);
         expect(filteredPackages[0].description).to.be.equal("Platfrom1-Architecture1 uninstalled package");
         expect(filteredPackages[0].platforms[0]).to.be.equal("platform1");
@@ -87,7 +87,7 @@ suite('PackageFilterer', () => {
 
     test('Doesnot filter the package if install test path is not specified', async () => {
         let platformInfo = new PlatformInformation("platform3", "architecture3");
-        let filteredPackages = await filterPackages(installablePackages, platformInfo);
+        let filteredPackages = await filterPackages(absolutePathPackage, platformInfo);
         expect(filteredPackages.length).to.be.equal(1);
         expect(filteredPackages[0].description).to.be.equal("Platfrom3-Architecture3 with no installTestPath specified");
         expect(filteredPackages[0].platforms[0]).to.be.equal("platform3");