Skip to content

Commit 576d50d

Browse files
DonJayamanneDonJayamanne
authored
Refactor parsing of environment variables (after 439) (#466)
Refactor how environment variables are parsed and used in tools, language server and debugger Fixes DonJayamanne#316 (PYTHONPATH not used as extra path for auto complete) Fixes #183 (changes to .env file requires restarts of vscode) Fixes #436 (Path variables not inherited when debugging) Fixes #435 (Virtual environment name is same as the default environment file)
1 parent aec4fb3 commit 576d50d

28 files changed

+1115
-394
lines changed

‎gulpfile.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ function getFilesToProcess(options) {
309309
// If we need only modified files, then filter the glob.
310310
if (options && options.mode === 'changes') {
311311
return gulp.src(all, gulpSrcOptions)
312-
.pipe(gitmodified('M', 'A', 'D', 'R', 'C', 'U', '??'));
312+
.pipe(gitmodified(['M', 'A', 'D', 'R', 'C', 'U', '??']));
313313
}
314314

315315
if (options && options.mode === 'staged') {

‎package-lock.json‎

Lines changed: 115 additions & 110 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1559,6 +1559,7 @@
15591559
"@types/mocha": "^2.2.43",
15601560
"@types/node": "^6.0.40",
15611561
"@types/semver": "^5.4.0",
1562+
"@types/shortid": "0.0.29",
15621563
"@types/sinon": "^2.3.2",
15631564
"@types/uuid": "^3.3.27",
15641565
"@types/winreg": "^1.2.30",
@@ -1578,6 +1579,7 @@
15781579
"mocha": "^2.3.3",
15791580
"relative": "^3.0.2",
15801581
"retyped-diff-match-patch-tsd-ambient": "^1.0.0-0",
1582+
"shortid": "^2.2.8",
15811583
"sinon": "^2.3.6",
15821584
"tslint": "^5.7.0",
15831585
"tslint-eslint-rules": "^4.1.1",

‎src/client/common/decorators.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import * as _ from 'lodash';
2+
3+
type AsyncVoidAction = (...params: {}[]) => Promise<void>;
4+
type VoidAction = (...params: {}[]) => void;
5+
6+
/**
7+
* Debounces a function execution. Function must return either a void or a promise that resolves to a void.
8+
* @export
9+
* @param {number} [wait] Wait time.
10+
* @returns void
11+
*/
12+
export function debounce(wait?: number) {
13+
// tslint:disable-next-line:no-any no-function-expression
14+
return function (_target: any, _propertyName: string, descriptor: TypedPropertyDescriptor<VoidAction> | TypedPropertyDescriptor<AsyncVoidAction>) {
15+
const originalMethod = descriptor.value!;
16+
// tslint:disable-next-line:no-invalid-this no-any
17+
(descriptor as any).value = _.debounce(function () { return originalMethod.apply(this, arguments); }, wait);
18+
};
19+
}
20+
21+
/**
22+
* Swallows exceptions thrown by a function. Function must return either a void or a promise that resolves to a void.
23+
* @export
24+
* @param {string} [scopeName] Scope for the error message to be logged along with the error.
25+
* @returns void
26+
*/
27+
export function swallowExceptions(scopeName: string) {
28+
// tslint:disable-next-line:no-any no-function-expression
29+
return function (_target: any, propertyName: string, descriptor: TypedPropertyDescriptor<VoidAction> | TypedPropertyDescriptor<AsyncVoidAction>) {
30+
const originalMethod = descriptor.value!;
31+
const errorMessage = `Python Extension (Error in ${scopeName}, method:${propertyName}):`;
32+
// tslint:disable-next-line:no-any no-function-expression
33+
descriptor.value = function (...args: any[]) {
34+
try {
35+
// tslint:disable-next-line:no-invalid-this no-use-before-declare no-unsafe-any
36+
const result = originalMethod.apply(this, args);
37+
38+
// If method being wrapped returns a promise then wait and swallow errors.
39+
if (result && typeof result.then === 'function' && typeof result.catch === 'function') {
40+
return (result as Promise<void>).catch(error => console.error(errorMessage, error));
41+
}
42+
} catch (error) {
43+
console.error(errorMessage, error);
44+
}
45+
};
46+
};
47+
}

‎src/client/common/process/currentProcess.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import { injectable } from 'inversify';
2+
import { ICurrentProcess } from '../types';
3+
import { EnvironmentVariables } from '../variables/types';
4+
5+
@injectable()
6+
export class CurrentProcess implements ICurrentProcess {
7+
public get env(): EnvironmentVariables {
8+
return process.env;
9+
}
10+
}

‎src/client/common/process/pythonExecutionFactory.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
1414
@inject(IEnvironmentVariablesProvider) private envVarsService: IEnvironmentVariablesProvider) { }
1515
public async create(resource?: Uri): Promise<IPythonExecutionService> {
1616
const settings = PythonSettings.getInstance(resource);
17-
return this.envVarsService.getEnvironmentVariables(true, resource)
17+
return this.envVarsService.getEnvironmentVariables(resource)
1818
.then(customEnvVars => {
1919
return new PythonExecutionService(this.procService, settings.pythonPath, customEnvVars);
2020
});

‎src/client/common/serviceRegistry.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,10 @@ import { IS_64_BIT, IS_WINDOWS } from './platform/constants';
1212
import { PathUtils } from './platform/pathUtils';
1313
import { RegistryImplementation } from './platform/registry';
1414
import { IRegistry } from './platform/types';
15+
import { CurrentProcess } from './process/currentProcess';
1516
import { TerminalService } from './terminal/service';
1617
import { ITerminalService } from './terminal/types';
17-
import { IInstaller, ILogger, IPathUtils, IPersistentStateFactory, Is64Bit, IsWindows } from './types';
18+
import { ICurrentProcess, IInstaller, ILogger, IPathUtils, IPersistentStateFactory, Is64Bit, IsWindows } from './types';
1819

1920
export function registerTypes(serviceManager: IServiceManager) {
2021
serviceManager.addSingletonInstance<boolean>(IsWindows, IS_WINDOWS);
@@ -27,6 +28,7 @@ export function registerTypes(serviceManager: IServiceManager) {
2728
serviceManager.addSingleton<ILogger>(ILogger, Logger);
2829
serviceManager.addSingleton<ITerminalService>(ITerminalService, TerminalService);
2930
serviceManager.addSingleton<IPathUtils>(IPathUtils, PathUtils);
31+
serviceManager.addSingleton<ICurrentProcess>(ICurrentProcess, CurrentProcess);
3032

3133
if (IS_WINDOWS) {
3234
serviceManager.addSingleton<IRegistry>(IRegistry, RegistryImplementation);

‎src/client/common/types.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// Licensed under the MIT License.
44

55
import { Uri } from 'vscode';
6+
import { EnvironmentVariables } from './variables/types';
67
export const IOutputChannel = Symbol('IOutputChannel');
78
export const IDocumentSymbolProvider = Symbol('IDocumentSymbolProvider');
89
export const IsWindows = Symbol('IS_WINDOWS');
@@ -80,3 +81,8 @@ export const IPathUtils = Symbol('IPathUtils');
8081
export interface IPathUtils {
8182
getPathVariableName(): 'Path' | 'PATH';
8283
}
84+
85+
export const ICurrentProcess = Symbol('ICurrentProcess');
86+
export interface ICurrentProcess {
87+
env: EnvironmentVariables;
88+
}

‎src/client/common/utils.ts‎

Lines changed: 1 addition & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,19 @@
11
'use strict';
22
// tslint:disable: no-any one-line no-suspicious-comment prefer-template prefer-const no-unnecessary-callback-wrapper no-function-expression no-string-literal no-control-regex no-shadowed-variable
3-
// TODO: Cleanup this place
4-
// Add options for execPythonFile
53

64
import * as child_process from 'child_process';
75
import * as fs from 'fs';
8-
import * as fsExtra from 'fs-extra';
96
import * as os from 'os';
107
import * as path from 'path';
11-
import { Position, Range, TextDocument, Uri } from 'vscode';
12-
import * as settings from './configSettings';
13-
import { parseEnvFile } from './envFileParser';
8+
import { Position, Range, TextDocument } from 'vscode';
149

1510
export const IS_WINDOWS = /^win/.test(process.platform);
1611
export const Is_64Bit = os.arch() === 'x64';
1712
export const PATH_VARIABLE_NAME = IS_WINDOWS ? 'Path' : 'PATH';
1813

19-
const PathValidity: Map<string, boolean> = new Map<string, boolean>();
20-
export function validatePath(filePath: string): Promise<string> {
21-
if (filePath.length === 0) {
22-
return Promise.resolve('');
23-
}
24-
if (PathValidity.has(filePath)) {
25-
return Promise.resolve(PathValidity.get(filePath) ? filePath : '');
26-
}
27-
return new Promise<string>(resolve => {
28-
fs.exists(filePath, exists => {
29-
PathValidity.set(filePath, exists);
30-
return resolve(exists ? filePath : '');
31-
});
32-
});
33-
}
3414
export function fsExistsAsync(filePath: string): Promise<boolean> {
3515
return new Promise<boolean>(resolve => {
3616
fs.exists(filePath, exists => {
37-
PathValidity.set(filePath, exists);
3817
return resolve(exists);
3918
});
4019
});
@@ -110,45 +89,6 @@ export function getSubDirectories(rootDir: string): Promise<string[]> {
11089
});
11190
}
11291

113-
export async function getCustomEnvVars(resource?: Uri): Promise<{} | undefined | null> {
114-
const envFile = settings.PythonSettings.getInstance(resource).envFile;
115-
if (typeof envFile !== 'string' || envFile.length === 0) {
116-
return null;
117-
}
118-
const exists = await fsExtra.pathExists(envFile);
119-
if (!exists) {
120-
return null;
121-
}
122-
try {
123-
const vars = parseEnvFile(envFile);
124-
if (vars && typeof vars === 'object' && Object.keys(vars).length > 0) {
125-
return vars;
126-
}
127-
} catch (ex) {
128-
console.error('Failed to parse env file', ex);
129-
}
130-
return null;
131-
}
132-
export function getCustomEnvVarsSync(resource?: Uri): {} | undefined | null {
133-
const envFile = settings.PythonSettings.getInstance(resource).envFile;
134-
if (typeof envFile !== 'string' || envFile.length === 0) {
135-
return null;
136-
}
137-
const exists = fsExtra.pathExistsSync(envFile);
138-
if (!exists) {
139-
return null;
140-
}
141-
try {
142-
const vars = parseEnvFile(envFile);
143-
if (vars && typeof vars === 'object' && Object.keys(vars).length > 0) {
144-
return vars;
145-
}
146-
} catch (ex) {
147-
console.error('Failed to parse env file', ex);
148-
}
149-
return null;
150-
}
151-
15292
export function getWindowsLineEndingCount(document: TextDocument, offset: Number) {
15393
const eolPattern = new RegExp('\r\n', 'g');
15494
const readBlock = 1024;

‎src/client/common/variables/environment.ts‎

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,15 @@ export class EnvironmentVariablesService implements IEnvironmentVariablesService
1818
if (!exists) {
1919
return undefined;
2020
}
21+
if (!fs.lstatSync(filePath).isFile()) {
22+
return undefined;
23+
}
2124
return new Promise<EnvironmentVariables | undefined>((resolve, reject) => {
2225
fs.readFile(filePath, 'utf8', (error, data) => {
2326
if (error) {
2427
return reject(error);
2528
}
26-
const vars = parseEnvironmentVariables(data)!;
27-
if (!vars || Object.keys(vars).length === 0) {
28-
return resolve(undefined);
29-
}
30-
this.appendPythonPath(vars, process.env.PYTHONPATH);
31-
this.appendPath(vars, process.env[this.pathVariable]);
32-
resolve(vars);
29+
resolve(parseEnvironmentVariables(data));
3330
});
3431
});
3532
}
@@ -47,28 +44,25 @@ export class EnvironmentVariablesService implements IEnvironmentVariablesService
4744
}
4845
});
4946
}
50-
public prependPythonPath(vars: EnvironmentVariables, ...pythonPaths: string[]) {
51-
return this.appendOrPrependPaths(vars, 'PYTHONPATH', false, ...pythonPaths);
52-
}
5347
public appendPythonPath(vars: EnvironmentVariables, ...pythonPaths: string[]) {
54-
return this.appendOrPrependPaths(vars, 'PYTHONPATH', true, ...pythonPaths);
55-
}
56-
public prependPath(vars: EnvironmentVariables, ...paths: string[]) {
57-
return this.appendOrPrependPaths(vars, this.pathVariable, false, ...paths);
48+
return this.appendPaths(vars, 'PYTHONPATH', ...pythonPaths);
5849
}
5950
public appendPath(vars: EnvironmentVariables, ...paths: string[]) {
60-
return this.appendOrPrependPaths(vars, this.pathVariable, true, ...paths);
51+
return this.appendPaths(vars, this.pathVariable, ...paths);
6152
}
62-
private appendOrPrependPaths(vars: EnvironmentVariables, variableName: 'PATH' | 'Path' | 'PYTHONPATH', append: boolean, ...pythonPaths: string[]) {
63-
const pathToInsert = pythonPaths.filter(item => typeof item === 'string' && item.length > 0).join(path.delimiter);
64-
if (pathToInsert.length === 0) {
53+
private appendPaths(vars: EnvironmentVariables, variableName: 'PATH' | 'Path' | 'PYTHONPATH', ...pathsToAppend: string[]) {
54+
const valueToAppend = pathsToAppend
55+
.filter(item => typeof item === 'string' && item.trim().length > 0)
56+
.map(item => item.trim())
57+
.join(path.delimiter);
58+
if (valueToAppend.length === 0) {
6559
return vars;
6660
}
6761

6862
if (typeof vars[variableName] === 'string' && vars[variableName].length > 0) {
69-
vars[variableName] = append ? (vars[variableName] + path.delimiter + pathToInsert) : (pathToInsert + path.delimiter + vars[variableName]);
63+
vars[variableName] = vars[variableName] + path.delimiter + valueToAppend;
7064
} else {
71-
vars[variableName] = pathToInsert;
65+
vars[variableName] = valueToAppend;
7266
}
7367
return vars;
7468
}

0 commit comments

Comments
 (0)