#1288 installer config and tests (#1302)

* update to use latest api

* config changes for multiroot workspace

* linting support with multi roots

* multi root support for formatters

* determine workspace root path

* revert change

* support multiple configs per workspace folder

* modify formatters to use resource specific settings

* modified installer to pass resource for workspace resolution

* null test in installer

* canges to config settings to support multiroot workspace

* changes to code refactoring to support workspace symbols

* oops

* modified to settings are resolved using document uri

* unit tests for multi root support

* fix unittests for multiroot

* exclude files

* add new line

* config changes for multiroot workspace

* new lines and enabled multi root linter tests

* fix sys variables

* added unit test to resolve ${workspaceRoot} in settings.json

* fixed code review comments

* fixed code review comments

.gitignore

@@ -4,7 +4,9 @@ node_modules
 *.pyc
 .vscode/.ropeproject/**
 src/test/.vscode/**
+**/.vscode/tags
 **/testFiles/**/.cache/**
 *.noseids
 .vscode-test
 __pycache__
+npm-debug.log

.vscode/launch.json

@@ -67,6 +67,23 @@
 			],
 			"preLaunchTask": "npm"
 		},
+		{
+			"name": "Launch Multiroot Tests",
+			"type": "extensionHost",
+			"request": "launch",
+			"runtimeExecutable": "${execPath}",
+			"args": [
+				"${workspaceRoot}/src/test/multiRootWkspc/multi.code-workspace",
+				"--extensionDevelopmentPath=${workspaceRoot}",
+				"--extensionTestsPath=${workspaceRoot}/out/test"
+			],
+			"stopOnEntry": false,
+			"sourceMaps": true,
+			"outFiles": [
+				"${workspaceRoot}/out/**/*.js"
+			],
+			"preLaunchTask": "npm"
+		},
 		{
 			"name": "Python",
 			"type": "python",

.vscode/settings.json

@@ -1,10 +1,11 @@
 // Place your settings in this file to overwrite default and user settings.
 {
 	"files.exclude": {
-		"out": false, // set this to true to hide the "out" folder with the compiled JS files
+		"out": true, // set this to true to hide the "out" folder with the compiled JS files
 		"**/*.pyc": true,
 		"**/__pycache__": true,
-		"node_modules": true
+		"node_modules": true,
+		".vscode-test":true
 	},
 	"search.exclude": {
 		"out": true // set this to false to include "out" folder in search results

package.json

@@ -1593,7 +1593,7 @@
     "vscode:prepublish": "tsc -p ./ && webpack",
     "compile": "webpack && tsc -watch -p ./",
     "postinstall": "node ./node_modules/vscode/bin/install",
-    "test": "node ./node_modules/vscode/bin/test"
+    "test": "node ./node_modules/vscode/bin/test && node ./out/test/multiRootTest.js"
   },
   "dependencies": {
     "anser": "^1.1.0",

src/client/common/configSettings.ts

@@ -146,6 +146,12 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
 
         this.initializeSettings();
     }
+    public static dispose() {
+        if (!IS_TEST_EXECUTION) {
+            throw new Error('Dispose can only be called from unit tests');
+        }
+        PythonSettings.pythonSettings.clear();
+    }
     public static getInstance(resource?: Uri): PythonSettings {
         const workspaceFolder = resource ? vscode.workspace.getWorkspaceFolder(resource) : undefined;
         let workspaceFolderUri: Uri = workspaceFolder ? workspaceFolder.uri : undefined;
@@ -160,15 +166,15 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
         return PythonSettings.pythonSettings.get(workspaceFolderKey);
     }
     private initializeSettings() {
-        const systemVariables: SystemVariables = new SystemVariables();
-        const workspaceRoot = (IS_TEST_EXECUTION || !this.workspaceRoot) ? __dirname : this.workspaceRoot.fsPath;
+        const workspaceRoot = this.workspaceRoot.fsPath;
+        const systemVariables: SystemVariables = new SystemVariables(this.workspaceRoot ? this.workspaceRoot.fsPath : undefined);
         const pythonSettings = vscode.workspace.getConfiguration('python', this.workspaceRoot);
         this.pythonPath = systemVariables.resolveAny(pythonSettings.get<string>('pythonPath'))!;
-        this.pythonPath = getAbsolutePath(this.pythonPath, IS_TEST_EXECUTION ? __dirname : workspaceRoot);
+        this.pythonPath = getAbsolutePath(this.pythonPath, workspaceRoot);
         this.venvPath = systemVariables.resolveAny(pythonSettings.get<string>('venvPath'))!;
         this.jediPath = systemVariables.resolveAny(pythonSettings.get<string>('jediPath'))!;
         if (typeof this.jediPath === 'string' && this.jediPath.length > 0) {
-            this.jediPath = getAbsolutePath(systemVariables.resolveAny(this.jediPath), IS_TEST_EXECUTION ? __dirname : workspaceRoot);
+            this.jediPath = getAbsolutePath(systemVariables.resolveAny(this.jediPath), workspaceRoot);
         }
         else {
             this.jediPath = '';

src/client/common/editor.ts

@@ -75,7 +75,7 @@ export function getTextEditsFromPatch(before: string, patch: string): TextEdit[]
 
     return textEdits;
 }
-export function getWorkspaceEditsFromPatch(filePatches: string[]): WorkspaceEdit {
+export function getWorkspaceEditsFromPatch(filePatches: string[], workspaceRoot?:string): WorkspaceEdit {
     const workspaceEdit = new WorkspaceEdit();
     filePatches.forEach(patch => {
         const indexOfAtAt = patch.indexOf('@@');
@@ -101,7 +101,7 @@ export function getWorkspaceEditsFromPatch(filePatches: string[]): WorkspaceEdit
         }
 
         let fileName = fileNameLines[0].substring(fileNameLines[0].indexOf(' a') + 3).trim();
-        fileName = path.isAbsolute(fileName) ? fileName : path.resolve(vscode.workspace.rootPath, fileName);
+        fileName = workspaceRoot && !path.isAbsolute(fileName) ? path.resolve(workspaceRoot, fileName) : fileName;
         if (!fs.existsSync(fileName)) {
             return;
         }

src/client/common/installer.ts

@@ -1,9 +1,10 @@
+import { error } from './logger';
 import * as vscode from 'vscode';
 import * as settings from './configSettings';
 import * as os from 'os';
+import { commands, ConfigurationTarget, Disposable, OutputChannel, Terminal, Uri, window, workspace } from 'vscode';
 import { isNotInstalledError } from './helpers';
 import { execPythonFile, getFullyQualifiedPythonInterpreterPath } from './utils';
-import { Documentation } from './constants';
 
 export enum Product {
     pytest,
@@ -111,6 +112,9 @@ SettingToDisableProduct.set(Product.pydocstyle, 'linting.pydocstyleEnabled');
 SettingToDisableProduct.set(Product.pylint, 'linting.pylintEnabled');
 SettingToDisableProduct.set(Product.pytest, 'unitTest.pyTestEnabled');
 
+const ProductInstallationPrompt = new Map<Product, string>();
+ProductInstallationPrompt.set(Product.ctags, 'Install CTags to enable Python workspace symbols');
+
 enum ProductType {
     Linter,
     Formatter,
@@ -142,6 +146,11 @@ ProductTypes.set(Product.autopep8, ProductType.Formatter);
 ProductTypes.set(Product.yapf, ProductType.Formatter);
 ProductTypes.set(Product.rope, ProductType.RefactoringLibrary);
 
+export enum InstallerResponse {
+    Installed,
+    Disabled,
+    Ignore
+}
 export class Installer implements vscode.Disposable {
     private static terminal: vscode.Terminal | undefined | null;
     private disposables: vscode.Disposable[] = [];
@@ -155,12 +164,14 @@ export class Installer implements vscode.Disposable {
     public dispose() {
         this.disposables.forEach(d => d.dispose());
     }
-    public shouldDisplayPrompt(product: Product) {
+    private shouldDisplayPrompt(product: Product) {
         const productName = ProductNames.get(product)!;
-        return settings.PythonSettings.getInstance().disablePromptForFeatures.indexOf(productName) === -1;
+        const pythonConfig = workspace.getConfiguration('python');
+        const disablePromptForFeatures = pythonConfig.get('disablePromptForFeatures', [] as string[]);
+        return disablePromptForFeatures.indexOf(productName) === -1;
     }
 
-    async promptToInstall(product: Product) {
+    public async promptToInstall(product: Product, resource?: Uri): Promise<InstallerResponse> {
         const productType = ProductTypes.get(product)!;
         const productTypeName = ProductTypeNames.get(productType);
         const productName = ProductNames.get(product)!;
@@ -173,10 +184,10 @@ export class Installer implements vscode.Disposable {
             else {
                 console.warn(message);
             }
-            return;
+            return InstallerResponse.Ignore;
         }
 
-        const installOption = 'Install ' + productName;
+        const installOption = ProductInstallationPrompt.has(product) ? ProductInstallationPrompt.get(product) : 'Install ' + productName;
         const disableOption = 'Disable ' + productTypeName;
         const dontShowAgain = `Don't show this prompt again`;
         const alternateFormatter = product === Product.autopep8 ? 'yapf' : 'autopep8';
@@ -189,46 +200,53 @@ export class Installer implements vscode.Disposable {
         if (SettingToDisableProduct.has(product)) {
             options.push(...[disableOption, dontShowAgain]);
         }
-        return vscode.window.showErrorMessage(`${productTypeName} ${productName} is not installed`, ...options).then(item => {
-            switch (item) {
-                case installOption: {
-                    return this.install(product);
-                }
-                case disableOption: {
-                    if (Linters.indexOf(product) >= 0) {
-                        return disableLinter(product);
-                    }
-                    else {
-                        const pythonConfig = vscode.workspace.getConfiguration('python');
-                        const settingToDisable = SettingToDisableProduct.get(product)!;
-                        return pythonConfig.update(settingToDisable, false);
-                    }
-                }
-                case useOtherFormatter: {
-                    const pythonConfig = vscode.workspace.getConfiguration('python');
-                    return pythonConfig.update('formatting.provider', alternateFormatter);
-                }
-                case dontShowAgain: {
-                    const pythonConfig = vscode.workspace.getConfiguration('python');
-                    const features = pythonConfig.get('disablePromptForFeatures', [] as string[]);
-                    features.push(productName);
-                    return pythonConfig.update('disablePromptForFeatures', features, true);
+        const item = await window.showErrorMessage(`${productTypeName} ${productName} is not installed`, ...options);
+        switch (item) {
+            case installOption: {
+                return this.install(product, resource);
+            }
+            case disableOption: {
+                if (Linters.indexOf(product) >= 0) {
+                    return this.disableLinter(product, resource).then(() => InstallerResponse.Disabled);
                 }
-                case 'Help': {
-                    return Promise.resolve();
+                else {
+                    const settingToDisable = SettingToDisableProduct.get(product)!;
+                    return this.updateSetting(settingToDisable, false, resource).then(() => InstallerResponse.Disabled);
                 }
             }
-        });
+            case useOtherFormatter: {
+                return this.updateSetting('formatting.provider', alternateFormatter, resource)
+                    .then(() => InstallerResponse.Installed);
+            }
+            case dontShowAgain: {
+                const pythonConfig = workspace.getConfiguration('python');
+                const features = pythonConfig.get('disablePromptForFeatures', [] as string[]);
+                features.push(productName);
+                return pythonConfig.update('disablePromptForFeatures', features, true).then(() => InstallerResponse.Ignore);
+            }
+            default: {
+                throw new Error('Invalid selection');
+            }
+        }
     }
-
-    install(product: Product): Promise<any> {
+    public async install(product: Product, resource?: Uri): Promise<InstallerResponse> {
         if (!this.outputChannel && !Installer.terminal) {
-            Installer.terminal = vscode.window.createTerminal('Python Installer');
+            Installer.terminal = window.createTerminal('Python Installer');
         }
 
-        if (product === Product.ctags && os.platform() === 'win32') {
-            vscode.commands.executeCommand('python.displayHelp', Documentation.Workspace.InstallOnWindows);
-            return Promise.resolve();
+        if (product === Product.ctags && settings.IS_WINDOWS) {
+            if (this.outputChannel) {
+                this.outputChannel.appendLine('Install Universal Ctags Win32 to enable support for Workspace Symbols');
+                this.outputChannel.appendLine('Download the CTags binary from the Universal CTags site.');
+                this.outputChannel.appendLine('Option 1: Extract ctags.exe from the downloaded zip to any folder within your PATH so that Visual Studio Code can run it.');
+                this.outputChannel.appendLine('Option 2: Extract to any folder and add the path to this folder to the command setting.');
+                this.outputChannel.appendLine('Option 3: Extract to any folder and define that path in the python.workspaceSymbols.ctagsPath setting of your user settings file (settings.json).');
+                this.outputChannel.show();
+            }
+            else {
+                window.showInformationMessage('Install Universal Ctags and set it in your path or define the path in your python.workspaceSymbols.ctagsPath settings');
+            }
+            return InstallerResponse.Ignore;
         }
 
         let installArgs = ProductInstallScripts.get(product)!;
@@ -241,19 +259,19 @@ export class Installer implements vscode.Disposable {
                 installArgs.splice(2, 0, '--proxy');
             }
         }
+        let installationPromise: Promise<any>;
         if (this.outputChannel && installArgs[0] === '-m') {
             // Errors are just displayed to the user
             this.outputChannel.show();
-            return execPythonFile(settings.PythonSettings.getInstance().pythonPath, installArgs, vscode.workspace.rootPath!, true, (data) => {
-                this.outputChannel!.append(data);
-            });
+            installationPromise = execPythonFile(settings.PythonSettings.getInstance(resource).pythonPath,
+                installArgs, getCwdForInstallScript(resource), true, (data) => { this.outputChannel!.append(data); });
         }
         else {
             // When using terminal get the fully qualitified path
             // Cuz people may launch vs code from terminal when they have activated the appropriate virtual env
             // Problem is terminal doesn't use the currently activated virtual env
             // Must have something to do with the process being launched in the terminal
-            return getFullyQualifiedPythonInterpreterPath()
+            installationPromise = getFullyQualifiedPythonInterpreterPath()
                 .then(pythonPath => {
                     let installScript = installArgs.join(' ');
 
@@ -269,42 +287,68 @@ export class Installer implements vscode.Disposable {
                     Installer.terminal!.show(false);
                 });
         }
+
+        return installationPromise
+            .then(() => this.isInstalled(product))
+            .then(isInstalled => isInstalled ? InstallerResponse.Installed : InstallerResponse.Ignore);
     }
 
-    isInstalled(product: Product): Promise<boolean | undefined> {
-        return isProductInstalled(product);
+    public isInstalled(product: Product, resource?: Uri): Promise<boolean | undefined> {
+        return isProductInstalled(product, resource);
     }
 
-    uninstall(product: Product): Promise<any> {
-        return uninstallproduct(product);
+    public uninstall(product: Product, resource?: Uri): Promise<any> {
+        return uninstallproduct(product, resource);
+    }
+    public disableLinter(product: Product, resource: Uri) {
+        if (resource && !workspace.getWorkspaceFolder(resource)) {
+            const settingToDisable = SettingToDisableProduct.get(product)!;
+            const pythonConfig = workspace.getConfiguration('python', resource);
+            return pythonConfig.update(settingToDisable, false, ConfigurationTarget.Workspace);
+        }
+        else {
+            const pythonConfig = workspace.getConfiguration('python');
+            return pythonConfig.update('linting.enabledWithoutWorkspace', false, true);
+        }
+    }
+    private updateSetting(setting: string, value: any, resource?: Uri) {
+        if (resource && !workspace.getWorkspaceFolder(resource)) {
+            const pythonConfig = workspace.getConfiguration('python', resource);
+            return pythonConfig.update(setting, value, ConfigurationTarget.Workspace);
+        }
+        else {
+            const pythonConfig = workspace.getConfiguration('python');
+            return pythonConfig.update(setting, value, true);
+        }
     }
 }
 
-export function disableLinter(product: Product, global?: boolean) {
-    const pythonConfig = vscode.workspace.getConfiguration('python');
-    const settingToDisable = SettingToDisableProduct.get(product)!;
-    if (vscode.workspace.rootPath) {
-        return pythonConfig.update(settingToDisable, false, global);
+function getCwdForInstallScript(resource?: Uri) {
+    const workspaceFolder = resource ? workspace.getWorkspaceFolder(resource) : undefined;
+    if (workspaceFolder) {
+        return workspaceFolder.uri.fsPath;
     }
-    else {
-        return pythonConfig.update('linting.enabledWithoutWorkspace', false, true);
+    if (Array.isArray(workspace.workspaceFolders) && workspace.workspaceFolders.length > 0) {
+        return workspace.workspaceFolders[0].uri.fsPath;
     }
+    return __dirname;
 }
 
-async function isProductInstalled(product: Product): Promise<boolean | undefined> {
+async function isProductInstalled(product: Product, resource?: Uri): Promise<boolean | undefined> {
     if (!ProductExecutableAndArgs.has(product)) {
         return;
     }
     const prodExec = ProductExecutableAndArgs.get(product)!;
-    return execPythonFile(prodExec.executable, prodExec.args.concat(['--version']), vscode.workspace.rootPath!, false)
-        .then(() => {
-            return true;
-        }).catch(reason => {
-            return !isNotInstalledError(reason);
-        });
+    const cwd = getCwdForInstallScript(resource);
+    return execPythonFile(prodExec.executable, prodExec.args.concat(['--version']), cwd, false)
+        .then(() => true)
+        .catch(reason => !isNotInstalledError(reason));
 }
 
-function uninstallproduct(product: Product): Promise<any> {
+function uninstallproduct(product: Product, resource?: Uri): Promise<any> {
+    if (!ProductUninstallScripts.has(product)) {
+        return Promise.resolve();
+    }
     const uninstallArgs = ProductUninstallScripts.get(product)!;
-    return execPythonFile('python', uninstallArgs, vscode.workspace.rootPath!, false);
-}
+    return execPythonFile('python', uninstallArgs, getCwdForInstallScript(resource), false);
+}

src/client/common/logger.ts

@@ -22,7 +22,9 @@ class Logger {
         Logger.writeLine(category, message);
     }
     static writeLine(category: string = "log", line: any) {
-        console[category](line);
+        if (process.env['PYTHON_DONJAYAMANNE_TEST'] !== '1') {
+            console[category](line);
+        }
         if (outChannel) {
             outChannel.appendLine(line);
         }

src/client/common/systemVariables.ts

@@ -133,11 +133,10 @@ export abstract class AbstractSystemVariables implements ISystemVariables {
 export class SystemVariables extends AbstractSystemVariables {
     private _workspaceRoot: string;
     private _workspaceRootFolderName: string;
-    private _execPath: string;
 
-    constructor() {
+    constructor(workspaceRoot?: string) {
         super();
-        this._workspaceRoot = typeof vscode.workspace.rootPath === 'string' ? vscode.workspace.rootPath : __dirname;
+        this._workspaceRoot = typeof workspaceRoot === 'string' ? workspaceRoot : __dirname;
         this._workspaceRootFolderName = Path.basename(this._workspaceRoot);
         Object.keys(process.env).forEach(key => {
             this[`env:${key}`] = this[`env.${key}`] = process.env[key];