Skip to content

Make use of new execution layer instead of spawning processes #425

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 38 commits into from
Dec 15, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
ecc1ca9
Fix Microsoft/vscode#37627 (#1368)
octref Nov 3, 2017
7c5778c
Version 0.7.0 of extension (#1381)
DonJayamanne Nov 9, 2017
9d1bf82
Update README.md
DonJayamanne Nov 9, 2017
ffba179
Update README.md
DonJayamanne Nov 9, 2017
905c713
sync fork with upstream
DonJayamanne Nov 10, 2017
acc2109
fix readme
DonJayamanne Nov 10, 2017
d470523
Merge branch 'master' of https://github.com/Microsoft/vscode-python
DonJayamanne Nov 16, 2017
d392e8b
merged upstream
DonJayamanne Nov 16, 2017
92f775f
Merge remote-tracking branch 'upstream/master'
DonJayamanne Nov 20, 2017
32a6e53
Merge remote-tracking branch 'upstream/master'
DonJayamanne Nov 21, 2017
4b30f2c
Merge remote-tracking branch 'upstream/master'
DonJayamanne Nov 22, 2017
e396752
Merge remote-tracking branch 'upstream/master'
DonJayamanne Nov 22, 2017
eff4792
Merge remote-tracking branch 'upstream/master'
DonJayamanne Nov 28, 2017
4553c28
Merge remote-tracking branch 'upstream/master'
DonJayamanne Nov 28, 2017
3c6520a
Merge remote-tracking branch 'upstream/master'
DonJayamanne Nov 28, 2017
966e516
Merge remote-tracking branch 'upstream/master'
DonJayamanne Nov 28, 2017
63d2d65
Merge remote-tracking branch 'upstream/master'
DonJayamanne Nov 28, 2017
f6d469e
Merge remote-tracking branch 'upstream/master'
DonJayamanne Nov 28, 2017
029e055
Merge remote-tracking branch 'upstream/master'
DonJayamanne Nov 30, 2017
e8c71c0
Merge remote-tracking branch 'upstream/master'
DonJayamanne Nov 30, 2017
51cf9d2
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 1, 2017
7aadc43
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 1, 2017
f0f5c59
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 4, 2017
b2b9da9
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 4, 2017
30a4091
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 5, 2017
b16d2f9
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 6, 2017
c8db345
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 7, 2017
0df7f16
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 8, 2017
3ccc881
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 9, 2017
bb0709e
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 11, 2017
2c19004
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 11, 2017
8f224ab
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 11, 2017
41b7080
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 12, 2017
dab38dc
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 12, 2017
ae22dd4
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 12, 2017
d2340d2
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 14, 2017
bcb8879
use new exec engine instead of spawning manually
DonJayamanne Dec 14, 2017
c5cc573
fixed code review comments
DonJayamanne Dec 15, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/client/providers/renameProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export class PythonRenameProvider implements vscode.RenameProvider {
const workspaceRoot = workspaceFolder ? workspaceFolder.uri.fsPath : __dirname;
const pythonSettings = PythonSettings.getInstance(workspaceFolder ? workspaceFolder.uri : undefined);

const proxy = new RefactorProxy(EXTENSION_DIR, pythonSettings, workspaceRoot);
const proxy = new RefactorProxy(EXTENSION_DIR, pythonSettings, workspaceRoot, this.serviceContainer);
return proxy.rename<RenameResponse>(document, newName, document.uri.fsPath, range).then(response => {
const fileDiffs = response.results.map(fileChanges => fileChanges.diff);
return getWorkspaceEditsFromPatch(fileDiffs, workspaceRoot);
Expand Down
12 changes: 6 additions & 6 deletions src/client/providers/simpleRefactorProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export function activateSimplePythonRefactorProvider(context: vscode.ExtensionCo
vscode.window.activeTextEditor!,
vscode.window.activeTextEditor!.selection,
// tslint:disable-next-line:no-empty
outputChannel).catch(() => { });
outputChannel, serviceContainer).catch(() => { });
sendTelemetryWhenDone(REFACTOR_EXTRACT_VAR, promise, stopWatch);
});
context.subscriptions.push(disposable);
Expand All @@ -33,7 +33,7 @@ export function activateSimplePythonRefactorProvider(context: vscode.ExtensionCo
vscode.window.activeTextEditor!,
vscode.window.activeTextEditor!.selection,
// tslint:disable-next-line:no-empty
outputChannel).catch(() => { });
outputChannel, serviceContainer).catch(() => { });
sendTelemetryWhenDone(REFACTOR_EXTRACT_FUNCTION, promise, stopWatch);
});
context.subscriptions.push(disposable);
Expand All @@ -42,7 +42,7 @@ export function activateSimplePythonRefactorProvider(context: vscode.ExtensionCo
// Exported for unit testing
export function extractVariable(extensionDir: string, textEditor: vscode.TextEditor, range: vscode.Range,
// tslint:disable-next-line:no-any
outputChannel: vscode.OutputChannel): Promise<any> {
outputChannel: vscode.OutputChannel, serviceContainer: IServiceContainer): Promise<any> {

let workspaceFolder = vscode.workspace.getWorkspaceFolder(textEditor.document.uri);
if (!workspaceFolder && Array.isArray(vscode.workspace.workspaceFolders) && vscode.workspace.workspaceFolders.length > 0) {
Expand All @@ -53,7 +53,7 @@ export function extractVariable(extensionDir: string, textEditor: vscode.TextEdi

return validateDocumentForRefactor(textEditor).then(() => {
const newName = `newvariable${new Date().getMilliseconds().toString()}`;
const proxy = new RefactorProxy(extensionDir, pythonSettings, workspaceRoot);
const proxy = new RefactorProxy(extensionDir, pythonSettings, workspaceRoot, serviceContainer);
const rename = proxy.extractVariable<RenameResponse>(textEditor.document, newName, textEditor.document.uri.fsPath, range, textEditor.options).then(response => {
return response.results[0].diff;
});
Expand All @@ -65,7 +65,7 @@ export function extractVariable(extensionDir: string, textEditor: vscode.TextEdi
// Exported for unit testing
export function extractMethod(extensionDir: string, textEditor: vscode.TextEditor, range: vscode.Range,
// tslint:disable-next-line:no-any
outputChannel: vscode.OutputChannel): Promise<any> {
outputChannel: vscode.OutputChannel, serviceContainer: IServiceContainer): Promise<any> {

let workspaceFolder = vscode.workspace.getWorkspaceFolder(textEditor.document.uri);
if (!workspaceFolder && Array.isArray(vscode.workspace.workspaceFolders) && vscode.workspace.workspaceFolders.length > 0) {
Expand All @@ -76,7 +76,7 @@ export function extractMethod(extensionDir: string, textEditor: vscode.TextEdito

return validateDocumentForRefactor(textEditor).then(() => {
const newName = `newmethod${new Date().getMilliseconds().toString()}`;
const proxy = new RefactorProxy(extensionDir, pythonSettings, workspaceRoot);
const proxy = new RefactorProxy(extensionDir, pythonSettings, workspaceRoot, serviceContainer);
const rename = proxy.extractMethod<RenameResponse>(textEditor.document, newName, textEditor.document.uri.fsPath, range, textEditor.options).then(response => {
return response.results[0].diff;
});
Expand Down
154 changes: 73 additions & 81 deletions src/client/refactor/proxy.ts
Original file line number Diff line number Diff line change
@@ -1,33 +1,37 @@
'use strict';
// tslint:disable:no-any no-empty member-ordering prefer-const prefer-template no-var-self

import * as child_process from 'child_process';
import * as path from 'path';
import * as vscode from 'vscode';
import { Uri } from 'vscode';
import { IPythonSettings } from '../common/configSettings';
import { mergeEnvVariables } from '../common/envFileParser';
import { getCustomEnvVarsSync, getWindowsLineEndingCount, IS_WINDOWS } from '../common/utils';
import '../common/extensions';
import { createDeferred, Deferred } from '../common/helpers';
import { IPythonExecutionFactory } from '../common/process/types';
import { getWindowsLineEndingCount, IS_WINDOWS } from '../common/utils';
import { IServiceContainer } from '../ioc/types';

export class RefactorProxy extends vscode.Disposable {
private _process: child_process.ChildProcess;
private _process?: child_process.ChildProcess;
private _extensionDir: string;
private _previousOutData: string = '';
private _previousStdErrData: string = '';
private _startedSuccessfully: boolean = false;
private _commandResolve: (value?: any | PromiseLike<any>) => void;
private _commandResolve?: (value?: any | PromiseLike<any>) => void;
private _commandReject: (reason?: any) => void;
private _initializeReject: (reason?: any) => void;
constructor(extensionDir: string, private pythonSettings: IPythonSettings, private workspaceRoot: string) {
private initialized: Deferred<void>;
constructor(extensionDir: string, private pythonSettings: IPythonSettings, private workspaceRoot: string,
private serviceContainer: IServiceContainer) {
super(() => { });
this._extensionDir = extensionDir;
}

dispose() {
public dispose() {
try {
this._process.kill();
this._process!.kill();
} catch (ex) {
}
catch (ex) {
}
this._process = null;
this._process = undefined;
}
private getOffsetAt(document: vscode.TextDocument, position: vscode.Position): number {
if (!IS_WINDOWS) {
Expand All @@ -43,52 +47,52 @@ export class RefactorProxy extends vscode.Disposable {

return offset - winEols;
}
rename<T>(document: vscode.TextDocument, name: string, filePath: string, range: vscode.Range, options?: vscode.TextEditorOptions): Promise<T> {
public rename<T>(document: vscode.TextDocument, name: string, filePath: string, range: vscode.Range, options?: vscode.TextEditorOptions): Promise<T> {
if (!options) {
options = vscode.window.activeTextEditor.options;
options = vscode.window.activeTextEditor!.options;
}
let command = {
"lookup": "rename",
"file": filePath,
"start": this.getOffsetAt(document, range.start).toString(),
"id": "1",
"name": name,
"indent_size": options.tabSize
const command = {
lookup: 'rename',
file: filePath,
start: this.getOffsetAt(document, range.start).toString(),
id: '1',
name: name,
indent_size: options.tabSize
};

return this.sendCommand<T>(JSON.stringify(command));
}
extractVariable<T>(document: vscode.TextDocument, name: string, filePath: string, range: vscode.Range, options?: vscode.TextEditorOptions): Promise<T> {
public extractVariable<T>(document: vscode.TextDocument, name: string, filePath: string, range: vscode.Range, options?: vscode.TextEditorOptions): Promise<T> {
if (!options) {
options = vscode.window.activeTextEditor.options;
options = vscode.window.activeTextEditor!.options;
}
let command = {
"lookup": "extract_variable",
"file": filePath,
"start": this.getOffsetAt(document, range.start).toString(),
"end": this.getOffsetAt(document, range.end).toString(),
"id": "1",
"name": name,
"indent_size": options.tabSize
const command = {
lookup: 'extract_variable',
file: filePath,
start: this.getOffsetAt(document, range.start).toString(),
end: this.getOffsetAt(document, range.end).toString(),
id: '1',
name: name,
indent_size: options.tabSize
};
return this.sendCommand<T>(JSON.stringify(command));
}
extractMethod<T>(document: vscode.TextDocument, name: string, filePath: string, range: vscode.Range, options?: vscode.TextEditorOptions): Promise<T> {
public extractMethod<T>(document: vscode.TextDocument, name: string, filePath: string, range: vscode.Range, options?: vscode.TextEditorOptions): Promise<T> {
if (!options) {
options = vscode.window.activeTextEditor.options;
options = vscode.window.activeTextEditor!.options;
}
// Ensure last line is an empty line
if (!document.lineAt(document.lineCount - 1).isEmptyOrWhitespace && range.start.line === document.lineCount - 1) {
return Promise.reject<T>('Missing blank line at the end of document (PEP8).');
}
let command = {
"lookup": "extract_method",
"file": filePath,
"start": this.getOffsetAt(document, range.start).toString(),
"end": this.getOffsetAt(document, range.end).toString(),
"id": "1",
"name": name,
"indent_size": options.tabSize
const command = {
lookup: 'extract_method',
file: filePath,
start: this.getOffsetAt(document, range.start).toString(),
end: this.getOffsetAt(document, range.end).toString(),
id: '1',
name: name,
indent_size: options.tabSize
};
return this.sendCommand<T>(JSON.stringify(command));
}
Expand All @@ -98,39 +102,30 @@ export class RefactorProxy extends vscode.Disposable {
return new Promise<T>((resolve, reject) => {
this._commandResolve = resolve;
this._commandReject = reject;
this._process.stdin.write(command + '\n');
this._process!.stdin.write(command + '\n');
});
});
}
private initialize(pythonPath: string): Promise<string> {
return new Promise<any>((resolve, reject) => {
this._initializeReject = reject;
let environmentVariables: Object & { [key: string]: string } = { 'PYTHONUNBUFFERED': '1' };
let customEnvironmentVars = getCustomEnvVarsSync(vscode.Uri.file(this.workspaceRoot));
if (customEnvironmentVars) {
environmentVariables = mergeEnvVariables(environmentVariables, customEnvironmentVars);
private async initialize(pythonPath: string): Promise<void> {
const pythonProc = await this.serviceContainer.get<IPythonExecutionFactory>(IPythonExecutionFactory).create(Uri.file(this.workspaceRoot));
this.initialized = createDeferred<void>();
const args = ['refactor.py', this.workspaceRoot];
const cwd = path.join(this._extensionDir, 'pythonFiles');
const result = pythonProc.execObservable(args, { cwd });
this._process = result.proc;
result.out.subscribe(output => {
if (output.source === 'stdout') {
if (!this._startedSuccessfully && output.out.startsWith('STARTED')) {
this._startedSuccessfully = true;
return this.initialized.resolve();
}
this.onData(output.out);
} else {
this.handleStdError(output.out);
}
environmentVariables = mergeEnvVariables(environmentVariables);
this._process = child_process.spawn(pythonPath, ['refactor.py', this.workspaceRoot],
{
cwd: path.join(this._extensionDir, 'pythonFiles'),
env: environmentVariables
});
this._process.stderr.setEncoding('utf8');
this._process.stderr.on('data', this.handleStdError.bind(this));
this._process.on('error', this.handleError.bind(this));
}, error => this.handleError(error));

let that = this;
this._process.stdout.setEncoding('utf8');
this._process.stdout.on('data', (data: string) => {
let dataStr: string = data + '';
if (!that._startedSuccessfully && dataStr.startsWith('STARTED')) {
that._startedSuccessfully = true;
return resolve();
}
that.onData(data);
});
});
return this.initialized.promise;
}
private handleStdError(data: string) {
// Possible there was an exception in parsing the data returned
Expand All @@ -140,34 +135,32 @@ export class RefactorProxy extends vscode.Disposable {
try {
errorResponse = dataStr.split(/\r?\n/g).filter(line => line.length > 0).map(resp => JSON.parse(resp));
this._previousStdErrData = '';
}
catch (ex) {
} catch (ex) {
console.error(ex);
// Possible we've only received part of the data, hence don't clear previousData
return;
}
if (typeof errorResponse[0].message !== 'string' || errorResponse[0].message.length === 0) {
errorResponse[0].message = errorResponse[0].traceback.split(/\r?\n/g).pop();
errorResponse[0].message = errorResponse[0].traceback.splitLines().pop()!;
}
let errorMessage = errorResponse[0].message + '\n' + errorResponse[0].traceback;

if (this._startedSuccessfully) {
this._commandReject(`Refactor failed. ${errorMessage}`);
}
else {
} else {
if (typeof errorResponse[0].type === 'string' && errorResponse[0].type === 'ModuleNotFoundError') {
this._initializeReject('Not installed');
this.initialized.reject('Not installed');
return;
}

this._initializeReject(`Refactor failed. ${errorMessage}`);
this.initialized.reject(`Refactor failed. ${errorMessage}`);
}
}
private handleError(error: Error) {
if (this._startedSuccessfully) {
return this._commandReject(error);
}
this._initializeReject(error);
this.initialized.reject(error);
}
private onData(data: string) {
if (!this._commandResolve) { return; }
Expand All @@ -179,13 +172,12 @@ export class RefactorProxy extends vscode.Disposable {
try {
response = dataStr.split(/\r?\n/g).filter(line => line.length > 0).map(resp => JSON.parse(resp));
this._previousOutData = '';
}
catch (ex) {
} catch (ex) {
// Possible we've only received part of the data, hence don't clear previousData
return;
}
this.dispose();
this._commandResolve(response[0]);
this._commandResolve = null;
this._commandResolve!(response[0]);
this._commandResolve = undefined;
}
}
Loading