Skip to content
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

#1228 multiroot interpreter display #1339

Merged
merged 44 commits into from
Oct 26, 2017
Merged
Show file tree
Hide file tree
Changes from 42 commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
aad5ca0
modifications to fix tests to run under multi root setup
DonJayamanne Oct 16, 2017
a819f69
log errors
DonJayamanne Oct 16, 2017
3b48d84
fix return type
DonJayamanne Oct 17, 2017
b290a7e
fix linter messages
DonJayamanne Oct 17, 2017
1e979b9
fix linter errors
DonJayamanne Oct 17, 2017
0959155
changes to ensure code is formatted correctly
DonJayamanne Oct 17, 2017
5ad6d1f
fixed comments
DonJayamanne Oct 17, 2017
26241f2
delete unwanted file
DonJayamanne Oct 17, 2017
6f3aba4
hide unwanted folders
DonJayamanne Oct 17, 2017
35a9cbe
fixes to linters to run on multiroot setup
DonJayamanne Oct 17, 2017
f5e4006
udpate settings sequentially
DonJayamanne Oct 17, 2017
34aefa2
log the output
DonJayamanne Oct 17, 2017
3c58408
show errors in deleting dir
DonJayamanne Oct 17, 2017
252a52e
removed prospector test, to be completed in #1319
DonJayamanne Oct 17, 2017
6acaa7a
fixes to tests and sorting provider
DonJayamanne Oct 17, 2017
d137b75
fixed test for interpreter display
DonJayamanne Oct 17, 2017
65076f8
undo commenting of code
DonJayamanne Oct 17, 2017
630f83a
add new line
DonJayamanne Oct 17, 2017
0dad286
support multi root in interpreter display
DonJayamanne Oct 18, 2017
9a283c7
merged 1228MultiRootMaster
DonJayamanne Oct 18, 2017
9f90a42
fix linter
DonJayamanne Oct 18, 2017
31aa28f
changed package version
DonJayamanne Oct 18, 2017
63180f0
disabled multiroot test
DonJayamanne Oct 18, 2017
7bdc669
backwards compatible change
DonJayamanne Oct 18, 2017
5956a47
fix nose tests
DonJayamanne Oct 19, 2017
7c8049f
revert change
DonJayamanne Oct 19, 2017
0f6b142
enable test but disable it
DonJayamanne Oct 19, 2017
8570b90
multi root support in utils.ts
DonJayamanne Oct 23, 2017
71f3507
fixed #1328
DonJayamanne Oct 23, 2017
c339fab
retries for flaky unit tests
DonJayamanne Oct 24, 2017
b0d29da
retry beforeEach
DonJayamanne Oct 24, 2017
dfc7451
common retry decorator
DonJayamanne Oct 24, 2017
a8eda55
enable telemetry for extension loads
DonJayamanne Oct 24, 2017
32b6d85
disable jupyter tests in multiroot tests
DonJayamanne Oct 24, 2017
155e45f
clean up python Path before and after testsclean up python Path befor…
DonJayamanne Oct 24, 2017
df14e35
rename test env variable
DonJayamanne Oct 24, 2017
1b58b20
dispose cfg settings
DonJayamanne Oct 24, 2017
f9e8915
dispose cfg settings
DonJayamanne Oct 24, 2017
7d4f0f7
update comment
DonJayamanne Oct 24, 2017
4f880a1
clean up
DonJayamanne Oct 24, 2017
000a25a
rearrange to ensurfe launching ext is first debug option
DonJayamanne Oct 24, 2017
665a233
bug fix for display name
DonJayamanne Oct 24, 2017
22cc2e7
resolved code review comment
DonJayamanne Oct 26, 2017
3cbc067
Fixed typp
DonJayamanne Oct 26, 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
51 changes: 10 additions & 41 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
@@ -1,25 +1,7 @@
// A launch configuration that compiles the extension and then opens it inside a new window
{
"version": "0.1.0",
"configurations": [{
"name": "CompletionServer.ppy",
"type": "python",
"request": "launch",
"stopOnEntry": true,
"pythonPath": "python",
"program": "${workspaceRoot}/pythonFiles/completionServer.py",
"cwd": "${workspaceRoot}",
"env": {},
"args": [
"123"
],
"envFile": "${workspaceRoot}/.env",
"debugOptions": [
"WaitOnAbnormalExit",
"WaitOnNormalExit",
"RedirectOutput"
]
},
"configurations": [
{
"name": "Launch Extension",
"type": "extensionHost",
Expand Down Expand Up @@ -83,28 +65,15 @@
"${workspaceRoot}/out/**/*.js"
],
"preLaunchTask": "compile"
},
{
"name": "Python",
"type": "python",
"request": "launch",
"stopOnEntry": true,
"pythonPath": "python",
"program": "${file}",
"console": "integratedTerminal",
"args": [],
"debugOptions": [
"WaitOnAbnormalExit",
"WaitOnNormalExit",
"RedirectOutput"
],
"cwd": "${workspaceRoot}"
}
],
"compounds": [{
"name": "Extension + Debugger",
"configurations": [
"Launch Extension", "Launch Extension as debugServer"
]
}]
"compounds": [
{
"name": "Extension + Debugger",
"configurations": [
"Launch Extension",
"Launch Extension as debugServer"
]
}
]
}
2 changes: 1 addition & 1 deletion pythonFiles/PythonTools/ipythonServer.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
from queue import Empty, Queue # Python 3

DEBUG = os.environ.get('DEBUG_DJAYAMANNE_IPYTHON', '0') == '1'
TEST = os.environ.get('PYTHON_DONJAYAMANNE_TEST', '0') == '1'
TEST = os.environ.get('VSC_PYTHON_CI_TEST', '0') == '1'

# The great "support IPython 2, 3, 4" strat begins
if not TEST:
Expand Down
2 changes: 1 addition & 1 deletion pythonFiles/completionServer.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
from queue import Empty, Queue # Python 3

DEBUG = os.environ.get('DEBUG_DJAYAMANNE_IPYTHON', '0') == '1'
TEST = os.environ.get('PYTHON_DONJAYAMANNE_TEST', '0') == '1'
TEST = os.environ.get('VSC_PYTHON_CI_TEST', '0') == '1'

def _debug_write(out):
if DEBUG:
Expand Down
43 changes: 19 additions & 24 deletions src/client/common/configSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import { EventEmitter } from 'events';
import * as path from 'path';
import * as vscode from 'vscode';
import { Uri } from 'vscode';
import { InterpreterInfoCache } from './interpreterInfoCache';
import { SystemVariables } from './systemVariables';

// tslint:disable-next-line:no-require-imports no-var-requires
const untildify = require('untildify');

Expand Down Expand Up @@ -135,7 +137,7 @@ export interface JupyterSettings {
}

// tslint:disable-next-line:no-string-literal
const IS_TEST_EXECUTION = process.env['PYTHON_DONJAYAMANNE_TEST'] === '1';
const IS_TEST_EXECUTION = process.env['VSC_PYTHON_CI_TEST'] === '1';

// tslint:disable-next-line:completed-docs
export class PythonSettings extends EventEmitter implements IPythonSettings {
Expand Down Expand Up @@ -197,10 +199,12 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
// tslint:disable-next-line:no-unsafe-any
this.disposables.forEach(disposable => disposable.dispose());
this.disposables = [];
InterpreterInfoCache.clear();
}

// tslint:disable-next-line:cyclomatic-complexity max-func-body-length
private initializeSettings() {
InterpreterInfoCache.clear();
const workspaceRoot = this.workspaceRoot.fsPath;
const systemVariables: SystemVariables = new SystemVariables(this.workspaceRoot ? this.workspaceRoot.fsPath : undefined);
const pythonSettings = vscode.workspace.getConfiguration('python', this.workspaceRoot);
Expand All @@ -213,8 +217,7 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
this.jediPath = systemVariables.resolveAny(pythonSettings.get<string>('jediPath'))!;
if (typeof this.jediPath === 'string' && this.jediPath.length > 0) {
this.jediPath = getAbsolutePath(systemVariables.resolveAny(this.jediPath), workspaceRoot);
}
else {
} else {
this.jediPath = '';
}
// tslint:disable-next-line:no-backbone-get-set-outside-model no-non-null-assertion
Expand All @@ -230,16 +233,14 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
this.disablePromptForFeatures = Array.isArray(this.disablePromptForFeatures) ? this.disablePromptForFeatures : [];
if (this.linting) {
Object.assign<ILintingSettings, ILintingSettings>(this.linting, lintingSettings);
}
else {
} else {
this.linting = lintingSettings;
}
// tslint:disable-next-line:no-backbone-get-set-outside-model no-non-null-assertion
const sortImportSettings = systemVariables.resolveAny(pythonSettings.get<ISortImportSettings>('sortImports'))!;
if (this.sortImports) {
Object.assign<ISortImportSettings, ISortImportSettings>(this.sortImports, sortImportSettings);
}
else {
} else {
this.sortImports = sortImportSettings;
}
// Support for travis.
Expand Down Expand Up @@ -290,8 +291,7 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
const formattingSettings = systemVariables.resolveAny(pythonSettings.get<IFormattingSettings>('formatting'))!;
if (this.formatting) {
Object.assign<IFormattingSettings, IFormattingSettings>(this.formatting, formattingSettings);
}
else {
} else {
this.formatting = formattingSettings;
}
// Support for travis.
Expand All @@ -309,8 +309,7 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
const autoCompleteSettings = systemVariables.resolveAny(pythonSettings.get<IAutoCompeteSettings>('autoComplete'))!;
if (this.autoComplete) {
Object.assign<IAutoCompeteSettings, IAutoCompeteSettings>(this.autoComplete, autoCompleteSettings);
}
else {
} else {
this.autoComplete = autoCompleteSettings;
}
// Support for travis.
Expand All @@ -324,8 +323,7 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
const workspaceSymbolsSettings = systemVariables.resolveAny(pythonSettings.get<IWorkspaceSymbolSettings>('workspaceSymbols'))!;
if (this.workspaceSymbols) {
Object.assign<IWorkspaceSymbolSettings, IWorkspaceSymbolSettings>(this.workspaceSymbols, workspaceSymbolsSettings);
}
else {
} else {
this.workspaceSymbols = workspaceSymbolsSettings;
}
// Support for travis.
Expand All @@ -343,8 +341,7 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
const unitTestSettings = systemVariables.resolveAny(pythonSettings.get<IUnitTestSettings>('unitTest'))!;
if (this.unitTest) {
Object.assign<IUnitTestSettings, IUnitTestSettings>(this.unitTest, unitTestSettings);
}
else {
} else {
this.unitTest = unitTestSettings;
if (IS_TEST_EXECUTION && !this.unitTest) {
// tslint:disable-next-line:prefer-type-cast
Expand Down Expand Up @@ -381,8 +378,7 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
const terminalSettings = systemVariables.resolveAny(pythonSettings.get<ITerminalSettings>('terminal'))!;
if (this.terminal) {
Object.assign<ITerminalSettings, ITerminalSettings>(this.terminal, terminalSettings);
}
else {
} else {
this.terminal = terminalSettings;
if (IS_TEST_EXECUTION && !this.terminal) {
// tslint:disable-next-line:prefer-type-cast
Expand All @@ -402,7 +398,9 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
appendResults: true, defaultKernel: '', startupCode: []
};

this.emit('change');
// If workspace config changes, then we could have a cascading effect of on change events.
// Lets defer the change notification.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Let's"

setTimeout(() => this.emit('change'), 1);
}

public get pythonPath(): string {
Expand All @@ -416,8 +414,7 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
// E.g. virtual directory name.
try {
this._pythonPath = getPythonExecutable(value);
}
catch (ex) {
} catch (ex) {
this._pythonPath = value;
}
}
Expand Down Expand Up @@ -461,8 +458,7 @@ function getPythonExecutable(pythonPath: string): string {
if (isValidPythonPath(path.join(pythonPath, 'scripts', executableName))) {
return path.join(pythonPath, 'scripts', executableName);
}
}
else {
} else {
if (isValidPythonPath(path.join(pythonPath, executableName))) {
return path.join(pythonPath, executableName);
}
Expand All @@ -479,8 +475,7 @@ function isValidPythonPath(pythonPath: string): boolean {
try {
const output = child_process.execFileSync(pythonPath, ['-c', 'print(1234)'], { encoding: 'utf8' });
return output.startsWith('1234');
}
catch (ex) {
} catch (ex) {
return false;
}
}
10 changes: 5 additions & 5 deletions src/client/common/installer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -263,15 +263,15 @@ export class Installer implements vscode.Disposable {
if (this.outputChannel && installArgs[0] === '-m') {
// Errors are just displayed to the user
this.outputChannel.show();
installationPromise = execPythonFile(settings.PythonSettings.getInstance(resource).pythonPath,
installationPromise = execPythonFile(resource, 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
installationPromise = getFullyQualifiedPythonInterpreterPath()
installationPromise = getFullyQualifiedPythonInterpreterPath(resource)
.then(pythonPath => {
let installScript = installArgs.join(' ');

Expand Down Expand Up @@ -340,7 +340,7 @@ async function isProductInstalled(product: Product, resource?: Uri): Promise<boo
}
const prodExec = ProductExecutableAndArgs.get(product)!;
const cwd = getCwdForInstallScript(resource);
return execPythonFile(prodExec.executable, prodExec.args.concat(['--version']), cwd, false)
return execPythonFile(resource, prodExec.executable, prodExec.args.concat(['--version']), cwd, false)
.then(() => true)
.catch(reason => !isNotInstalledError(reason));
}
Expand All @@ -350,5 +350,5 @@ function uninstallproduct(product: Product, resource?: Uri): Promise<any> {
return Promise.resolve();
}
const uninstallArgs = ProductUninstallScripts.get(product)!;
return execPythonFile('python', uninstallArgs, getCwdForInstallScript(resource), false);
}
return execPythonFile(resource, 'python', uninstallArgs, getCwdForInstallScript(resource), false);
}
54 changes: 54 additions & 0 deletions src/client/common/interpreterInfoCache.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { Uri, workspace } from 'vscode';

type InterpreterCache = {
pythonInterpreterDirectory?: string;
pythonInterpreterPath?: string;
pythonSettingsPath?: string;
// tslint:disable-next-line:no-any
customEnvVariables?: any;
};

const cache = new Map<string, InterpreterCache>();

// tslint:disable-next-line:no-stateless-class
export class InterpreterInfoCache {
// tslint:disable-next-line:function-name
public static clear(): void {
cache.clear();
}
// tslint:disable-next-line:function-name
public static get(resource?: Uri) {
const cacheKey = InterpreterInfoCache.getCacheKey(resource) || '';
return cache.has(cacheKey) ? cache.get(cacheKey) : {};
}
// tslint:disable-next-line:function-name
public static setPaths(resource?: Uri, pythonSettingsPath?: string, pythonInterpreterPath?: string, pythonInterpreterDirectory?: string) {
InterpreterInfoCache.setCacheData('pythonInterpreterDirectory', resource, pythonInterpreterDirectory);
InterpreterInfoCache.setCacheData('pythonInterpreterPath', resource, pythonInterpreterPath);
InterpreterInfoCache.setCacheData('pythonSettingsPath', resource, pythonSettingsPath);
}

// tslint:disable-next-line:no-any function-name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So is this instead of declaring the variable as void?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it's to treat a variable as one that has no type information. (Kind of like removing type hints when a variable has had type hints defined)

public static setCustomEnvVariables(resource?: Uri, envVars?: any) {
// tslint:disable-next-line:no-any
InterpreterInfoCache.setCacheData('customEnvVariables', resource, envVars);
}
// tslint:disable-next-line:no-any function-name
private static setCacheData(property: keyof InterpreterCache, resource?: Uri, value?: string | any) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does declaring the type as string | any mean anything since any will override?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, can't understand why that was added. Yes you are right

const cacheKey = InterpreterInfoCache.getCacheKey(resource) || '';
// tslint:disable-next-line:prefer-type-cast
const data = cache.has(cacheKey) ? cache.get(cacheKey) : {} as InterpreterCache;
data[property] = value;
cache.set(cacheKey, data);
}
private static getCacheKey(resource?: Uri): string {
if (!Array.isArray(workspace.workspaceFolders) || workspace.workspaceFolders.length === 0) {
return '';
}
if (!resource || workspace.workspaceFolders.length === 1) {
return workspace.workspaceFolders[0].uri.fsPath;
}
const folder = workspace.getWorkspaceFolder(resource);
return folder ? folder.uri.fsPath : '';
}
}
2 changes: 1 addition & 1 deletion src/client/common/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class Logger {
Logger.writeLine(category, message);
}
static writeLine(category: string = "log", line: any) {
if (process.env['PYTHON_DONJAYAMANNE_TEST'] !== '1') {
if (process.env['VSC_PYTHON_CI_TEST'] !== '1') {
console[category](line);
}
if (outChannel) {
Expand Down
Loading