react to changes to python path #216

src/client/common/configSettings.ts

@@ -2,6 +2,7 @@
 
 import * as vscode from 'vscode';
 import {SystemVariables} from './systemVariables';
+import {EventEmitter} from 'events';
 
 export interface IPythonSettings {
     pythonPath: string;
@@ -65,9 +66,10 @@ export interface IAutoCompeteSettings {
     extraPaths: string[];
 }
 const systemVariables: SystemVariables = new SystemVariables();
-export class PythonSettings implements IPythonSettings {
+export class PythonSettings extends EventEmitter implements IPythonSettings {
     private static pythonSettings: PythonSettings = new PythonSettings();
     constructor() {
+        super();
         if (PythonSettings.pythonSettings) {
             throw new Error('Singleton class, Use getInstance method');
         }
@@ -79,7 +81,7 @@ export class PythonSettings implements IPythonSettings {
     }
     public static getInstance(): PythonSettings {
         return PythonSettings.pythonSettings;
-    }
+    }    
     private initializeSettings() {
         let pythonSettings = vscode.workspace.getConfiguration('python');
         this.pythonPath = systemVariables.resolveAny(pythonSettings.get<string>('pythonPath'));
@@ -116,7 +118,9 @@ export class PythonSettings implements IPythonSettings {
         else {
             this.unitTest = unitTestSettings;
         }
-   }
+
+        this.emit('change');
+    }
 
     public pythonPath: string;
     public devOptions: any[];

src/client/common/utils.ts

@@ -1,25 +1,25 @@
-"use strict";
+'use strict';
 
-import * as path from "path";
-import * as fs from "fs";
-import * as child_process from "child_process";
-import * as settings from "./configSettings";
+import * as path from 'path';
+import * as fs from 'fs';
+import * as child_process from 'child_process';
+import * as settings from './configSettings';
 
 const IS_WINDOWS = /^win/.test(process.platform);
-const PATH_VARIABLE_NAME = IS_WINDOWS ? "Path" : "PATH";
+const PATH_VARIABLE_NAME = IS_WINDOWS ? 'Path' : 'PATH';
 
 const PathValidity: Map<string, boolean> = new Map<string, boolean>();
 export function validatePath(filePath: string): Promise<string> {
     if (filePath.length === 0) {
-        return Promise.resolve("");
+        return Promise.resolve('');
     }
     if (PathValidity.has(filePath)) {
-        return Promise.resolve(PathValidity.get(filePath) ? filePath : "");
+        return Promise.resolve(PathValidity.get(filePath) ? filePath : '');
     }
     return new Promise<string>(resolve => {
         fs.exists(filePath, exists => {
             PathValidity.set(filePath, exists);
-            return resolve(exists ? filePath : "");
+            return resolve(exists ? filePath : '');
         });
     });
 }
@@ -28,6 +28,12 @@ let pythonInterpretterDirectory: string = null;
 let previouslyIdentifiedPythonPath: string = null;
 let customEnvVariables: any = null;
 
+// If config settings change then clear env variables that we have cached
+// Remember, the path to the python interpreter can change, hence we need to re-set the paths
+settings.PythonSettings.getInstance().on('change', function () {
+    customEnvVariables = null;
+});
+
 export function getPythonInterpreterDirectory(): Promise<string> {
     // If we already have it and the python path hasn't changed, yay
     if (pythonInterpretterDirectory && previouslyIdentifiedPythonPath === settings.PythonSettings.getInstance().pythonPath) {
@@ -40,25 +46,25 @@ export function getPythonInterpreterDirectory(): Promise<string> {
         // Check if we have the path
         if (path.basename(pythonFileName) === pythonFileName) {
             // No path provided
-            return resolve("");
+            return resolve('');
         }
 
         // If we can execute the python, then get the path from the fullyqualitified name
-        child_process.execFile(pythonFileName, ["-c", "print(1234)"], (error, stdout, stderr) => {
+        child_process.execFile(pythonFileName, ['-c', 'print(1234)'], (error, stdout, stderr) => {
             // Yes this is a valid python path
-            if (stdout.startsWith("1234")) {
+            if (stdout.startsWith('1234')) {
                 return resolve(path.dirname(pythonFileName));
             }
             // No idea, didn't work, hence don't reject, but return empty path
-            resolve("");
+            resolve('');
         });
     }).then(value => {
         // Cache and return
         previouslyIdentifiedPythonPath = settings.PythonSettings.getInstance().pythonPath;
         return pythonInterpretterDirectory = value;
     }).catch(() => {
         // Don't care what the error is, all we know is that this doesn't work
-        return pythonInterpretterDirectory = "";
+        return pythonInterpretterDirectory = '';
     });
 }
 
@@ -78,9 +84,9 @@ export function execPythonFile(file: string, args: string[], cwd: string, includ
         if (customEnvVariables === null) {
             let pathValue = <string>process.env[PATH_VARIABLE_NAME];
             // Ensure to include the path of the current python 
-            let newPath = "";
+            let newPath = '';
             if (IS_WINDOWS) {
-                newPath = pyPath + "\\" + path.delimiter + path.join(pyPath, "Scripts\\") + path.delimiter + process.env[PATH_VARIABLE_NAME];
+                newPath = pyPath + '\\' + path.delimiter + path.join(pyPath, 'Scripts\\') + path.delimiter + process.env[PATH_VARIABLE_NAME];
                 // This needs to be done for windows
                 process.env[PATH_VARIABLE_NAME] = newPath;
             }
@@ -98,24 +104,24 @@ export function execPythonFile(file: string, args: string[], cwd: string, includ
 
 function handleResponse(file: string, includeErrorAsResponse: boolean, error: Error, stdout: string, stderr: string): Promise<string> {
     return new Promise<string>((resolve, reject) => {
-        if (typeof (error) === "object" && error !== null && ((<any>error).code === "ENOENT" || (<any>error).code === 127)) {
+        if (typeof (error) === 'object' && error !== null && ((<any>error).code === 'ENOENT' || (<any>error).code === 127)) {
             return reject(error);
         }
 
         // pylint:
         //      In the case of pylint we have some messages (such as config file not found and using default etc...) being returned in stderr
         //      These error messages are useless when using pylint   
         if (includeErrorAsResponse && (stdout.length > 0 || stderr.length > 0)) {
-            return resolve(stdout + "\n" + stderr);
+            return resolve(stdout + '\n' + stderr);
         }
 
         let hasErrors = (error && error.message.length > 0) || (stderr && stderr.length > 0);
-        if (hasErrors && (typeof stdout !== "string" || stdout.length === 0)) {
-            let errorMsg = (error && error.message) ? error.message : (stderr && stderr.length > 0 ? stderr + "" : "");
+        if (hasErrors && (typeof stdout !== 'string' || stdout.length === 0)) {
+            let errorMsg = (error && error.message) ? error.message : (stderr && stderr.length > 0 ? stderr + '' : '');
             return reject(errorMsg);
         }
 
-        resolve(stdout + "");
+        resolve(stdout + '');
     });
 }
 function execFileInternal(file: string, args: string[], options: child_process.ExecFileOptions, includeErrorAsResponse: boolean): Promise<string> {
@@ -127,7 +133,7 @@ function execFileInternal(file: string, args: string[], options: child_process.E
 }
 function execInternal(command: string, args: string[], options: child_process.ExecFileOptions, includeErrorAsResponse: boolean): Promise<string> {
     return new Promise<string>((resolve, reject) => {
-        child_process.exec([command].concat(args).join(" "), options, (error, stdout, stderr) => {
+        child_process.exec([command].concat(args).join(' '), options, (error, stdout, stderr) => {
             handleResponse(command, includeErrorAsResponse, error, stdout, stderr).then(resolve, reject);
         });
     });

src/client/providers/hoverProvider.ts

@@ -11,7 +11,7 @@ export class PythonHoverProvider implements vscode.HoverProvider {
     public constructor(context: vscode.ExtensionContext) {
         this.jediProxyHandler = new proxy.JediProxyHandler(context, null, PythonHoverProvider.parseData);
     }
-    private static parseData(data: proxy.ICompletionResult) {
+    private static parseData(data: proxy.ICompletionResult): vscode.Hover {
         if (data && data.items.length > 0) {
             var definition = data.items[0];
 

src/client/providers/jediProxy.ts

@@ -149,10 +149,33 @@ export class JediProxy extends vscode.Disposable {
     }
 }
 
+// keep track of the directory so we can re-spawn the process
+let pythonProcessCWD = "";
 function initialize(dir: string) {
+    pythonProcessCWD = dir;
     spawnProcess(path.join(dir, "pythonFiles"));
 }
 
+// Check if settings changes
+let lastKnownPythonInterpreter = pythonSettings.pythonPath;
+pythonSettings.on('change', onPythonSettingsChanged);
+
+function onPythonSettingsChanged() {
+    if (lastKnownPythonInterpreter === pythonSettings.pythonPath) {
+        return;
+    }
+    killProcess();
+    clearPendingRequests();
+    initialize(pythonProcessCWD);
+}
+
+function clearPendingRequests() {
+    commandQueue = [];
+    commands.forEach(item => {
+        item.resolve();
+    });
+    commands.clear();
+}
 var previousData = "";
 var commands = new Map<number, IExecutionCommand<ICommandResult>>();
 var commandQueue: number[] = [];
@@ -164,6 +187,7 @@ function killProcess() {
         }
     }
     catch (ex) { }
+    proc = null;
 }
 
 function handleError(source: string, errorMessage: string) {
@@ -200,8 +224,11 @@ function spawnProcess(dir: string) {
             previousData = "";
         }
         catch (ex) {
-            //Possible we've only received part of the data, hence don't clear previousData
-            handleError("stdout", ex.message);
+            // Possible we've only received part of the data, hence don't clear previousData
+            // Don't log errors when we haven't received the entire response
+            if (ex.message !== 'Unexpected end of input') {
+                handleError("stdout", ex.message);
+            }
             return;
         }
 
@@ -580,7 +607,7 @@ export class JediProxyHandler<R extends ICommandResult, T> {
     }
 
     private onResolved(data: R) {
-        if (this.lastToken.isCancellationRequested || data.requestId !== this.lastCommandId) {
+        if (this.lastToken.isCancellationRequested || !data || data.requestId !== this.lastCommandId) {
             this.promiseResolve(this.defaultCallbackData);
         }
         if (data) {

src/client/providers/referenceProvider.ts

@@ -9,7 +9,7 @@ export class PythonReferenceProvider implements vscode.ReferenceProvider {
     private jediProxyHandler: proxy.JediProxyHandler<proxy.IReferenceResult, vscode.Location[]>;
 
     public constructor(context: vscode.ExtensionContext) {
-        this.jediProxyHandler = new proxy.JediProxyHandler(context, null, PythonReferenceProvider.parseData);
+        this.jediProxyHandler = new proxy.JediProxyHandler(context, [], PythonReferenceProvider.parseData);
     }
     private static parseData(data: proxy.IReferenceResult): vscode.Location[] {
         if (data && data.references.length > 0) {

src/client/providers/symbolProvider.ts

@@ -8,7 +8,7 @@ export class PythonSymbolProvider implements vscode.DocumentSymbolProvider {
     private jediProxyHandler: proxy.JediProxyHandler<proxy.ISymbolResult, vscode.SymbolInformation[]>;
 
     public constructor(context: vscode.ExtensionContext, jediProxy: proxy.JediProxy = null) {
-        this.jediProxyHandler = new proxy.JediProxyHandler(context, null, PythonSymbolProvider.parseData, jediProxy);
+        this.jediProxyHandler = new proxy.JediProxyHandler(context, [], PythonSymbolProvider.parseData, jediProxy);
     }
     private static parseData(data: proxy.ISymbolResult): vscode.SymbolInformation[] {
         if (data) {