From 0b6372623ef8a7e19acb09c6fceacd162e4aa02d Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 21 Apr 2020 07:18:05 -0700 Subject: [PATCH 1/6] Expose interpreter path using API --- src/client/api.ts | 21 +++++++++++++++++++-- src/test/api.functional.test.ts | 24 ++++++++++++++++++++++-- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/src/client/api.ts b/src/client/api.ts index 6494862f8471..59fa96e2002e 100644 --- a/src/client/api.ts +++ b/src/client/api.ts @@ -6,7 +6,7 @@ import { isTestExecution } from './common/constants'; import { DebugAdapterNewPtvsd } from './common/experimentGroups'; import { traceError } from './common/logger'; -import { IExperimentsManager } from './common/types'; +import { IConfigurationService, IExperimentsManager, Resource } from './common/types'; import { getDebugpyLauncherArgs, getPtvsdLauncherScriptArgs } from './debugger/extension/adapter/remoteLaunchers'; import { IServiceContainer, IServiceManager } from './ioc/types'; @@ -34,6 +34,17 @@ export interface IExtensionApi { */ getRemoteLauncherCommand(host: string, port: number, waitUntilDebuggerAttaches: boolean): Promise; }; + /** + * Return internal settings within the extension stored in VSCode storage + */ + settings: { + /** + * Return the currently selected interpreter path. + * @param {Resource} [resource] + * @returns {string} + */ + getInterpreterPath(resource?: Resource): string; + }; } export function buildApi( @@ -41,8 +52,9 @@ export function buildApi( ready: Promise, serviceManager: IServiceManager, serviceContainer: IServiceContainer -) { +): IExtensionApi { const experimentsManager = serviceContainer.get(IExperimentsManager); + const configurationService = serviceContainer.get(IConfigurationService); const api = { // 'ready' will propagate the exception, but we must log it here first. ready: ready.catch((ex) => { @@ -71,6 +83,11 @@ export function buildApi( waitUntilDebuggerAttaches }); } + }, + settings: { + getInterpreterPath(resource?: Resource) { + return configurationService.getSettings(resource).pythonPath; + } } }; diff --git a/src/test/api.functional.test.ts b/src/test/api.functional.test.ts index 9f45c9f686ad..184746133153 100644 --- a/src/test/api.functional.test.ts +++ b/src/test/api.functional.test.ts @@ -8,15 +8,17 @@ import { expect } from 'chai'; import * as path from 'path'; import { anyString, instance, mock, when } from 'ts-mockito'; +import { Uri } from 'vscode'; import { buildApi } from '../client/api'; +import { ConfigurationService } from '../client/common/configuration/service'; import { EXTENSION_ROOT_DIR } from '../client/common/constants'; import { ExperimentsManager } from '../client/common/experiments'; -import { IExperimentsManager } from '../client/common/types'; +import { IConfigurationService, IExperimentsManager } from '../client/common/types'; import { ServiceContainer } from '../client/ioc/container'; import { ServiceManager } from '../client/ioc/serviceManager'; import { IServiceContainer, IServiceManager } from '../client/ioc/types'; -suite('Extension API - Debugger', () => { +suite('Extension API', () => { const expectedLauncherPath = `${EXTENSION_ROOT_DIR.fileToCommandArgument()}/pythonFiles/ptvsd_launcher.py`; const ptvsdPath = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'lib', 'python', 'debugpy', 'no_wheels', 'debugpy'); const ptvsdHost = 'somehost'; @@ -25,15 +27,33 @@ suite('Extension API - Debugger', () => { let serviceContainer: IServiceContainer; let serviceManager: IServiceManager; let experimentsManager: IExperimentsManager; + let configurationService: IConfigurationService; setup(() => { serviceContainer = mock(ServiceContainer); serviceManager = mock(ServiceManager); experimentsManager = mock(ExperimentsManager); + configurationService = mock(ConfigurationService); + when(serviceContainer.get(IConfigurationService)).thenReturn( + instance(configurationService) + ); when(serviceContainer.get(IExperimentsManager)).thenReturn(instance(experimentsManager)); }); + test('Test interpreter path settings API', async () => { + const resource = Uri.parse('a'); + when(configurationService.getSettings(resource)).thenReturn({ pythonPath: 'settingValue' } as any); + + const interpreterPath = buildApi( + Promise.resolve(), + instance(serviceManager), + instance(serviceContainer) + ).settings.getInterpreterPath(resource); + + expect(interpreterPath).to.equal('settingValue'); + }); + test('Test debug launcher args (no-wait and not in experiment)', async () => { const waitForAttach = false; when(experimentsManager.inExperiment(anyString())).thenReturn(false); From 27958d2560ab12aaa4bd39111c7461615169c661 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 21 Apr 2020 07:37:30 -0700 Subject: [PATCH 2/6] News entry --- news/1 Enhancements/11294.md | 1 + src/client/api.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 news/1 Enhancements/11294.md diff --git a/news/1 Enhancements/11294.md b/news/1 Enhancements/11294.md new file mode 100644 index 000000000000..7f7632e27034 --- /dev/null +++ b/news/1 Enhancements/11294.md @@ -0,0 +1 @@ +Expose currently selected interpreter path using API. diff --git a/src/client/api.ts b/src/client/api.ts index 59fa96e2002e..ed628640e68d 100644 --- a/src/client/api.ts +++ b/src/client/api.ts @@ -35,7 +35,7 @@ export interface IExtensionApi { getRemoteLauncherCommand(host: string, port: number, waitUntilDebuggerAttaches: boolean): Promise; }; /** - * Return internal settings within the extension stored in VSCode storage + * Return internal settings within the extension which are stored in VSCode storage */ settings: { /** From fa3e1e9f8f8492ece64e08eeac6226c874e33e56 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 21 Apr 2020 11:35:01 -0700 Subject: [PATCH 3/6] Add flag in package.json --- package.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/package.json b/package.json index 72219a305384..03ff346ff08d 100644 --- a/package.json +++ b/package.json @@ -3,6 +3,9 @@ "displayName": "Python", "description": "Linting, Debugging (multi-threaded, remote), Intellisense, Jupyter Notebooks, code formatting, refactoring, unit tests, snippets, and more.", "version": "2020.5.0-dev", + "featureFlags": { + "usingNewInterpreterStorage": true + }, "languageServerVersion": "0.5.30", "publisher": "ms-python", "enableProposedApi": false, From 5c85e516b15037e9b1ae7ea0b68f7b4d32947ece Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 21 Apr 2020 11:47:58 -0700 Subject: [PATCH 4/6] Update API description --- src/client/api.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/client/api.ts b/src/client/api.ts index ed628640e68d..c1d02c2c077a 100644 --- a/src/client/api.ts +++ b/src/client/api.ts @@ -39,8 +39,11 @@ export interface IExtensionApi { */ settings: { /** - * Return the currently selected interpreter path. - * @param {Resource} [resource] + * Returns the Python interpreter path corresponding to the specified resource, taking into account + * any workspace-specific settings for the workspace to which this resource belongs. + * @param {Resource} [resource] A resource for which the setting is asked for. + * * When no resource is provided, the setting scoped to the first workspace folder is returned. + * * If no folder is present, it returns the global setting. * @returns {string} */ getInterpreterPath(resource?: Resource): string; From b65c3db1b93b2f337451a8faf5c7b569e97b0d7c Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 22 Apr 2020 11:34:01 -0700 Subject: [PATCH 5/6] API changes after discussion --- src/client/api.ts | 18 +++++++++++++----- src/client/common/configSettings.ts | 1 + src/test/api.functional.test.ts | 19 ++++++++++++++++--- 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/src/client/api.ts b/src/client/api.ts index c1d02c2c077a..b1752e64ef84 100644 --- a/src/client/api.ts +++ b/src/client/api.ts @@ -39,14 +39,20 @@ export interface IExtensionApi { */ settings: { /** - * Returns the Python interpreter path corresponding to the specified resource, taking into account + * Returns the Python execution command corresponding to the specified resource, taking into account * any workspace-specific settings for the workspace to which this resource belongs. + * E.g of execution commands returned could be, + * * `['']` + * * `['']` + * * `['conda', 'run', 'python']` which is used to run from within Conda environments. + * or something similar for some other Python environments. * @param {Resource} [resource] A resource for which the setting is asked for. * * When no resource is provided, the setting scoped to the first workspace folder is returned. * * If no folder is present, it returns the global setting. - * @returns {string} + * @returns {(string[] | undefined)} When return value is `undefined`, it means no interpreter is set. + * Otherwise, join the items returned using space to construct the full execution command. */ - getInterpreterPath(resource?: Resource): string; + getExecutionCommand(resource?: Resource): string[] | undefined; }; } @@ -88,8 +94,10 @@ export function buildApi( } }, settings: { - getInterpreterPath(resource?: Resource) { - return configurationService.getSettings(resource).pythonPath; + getExecutionCommand(resource?: Resource) { + const pythonPath = configurationService.getSettings(resource).pythonPath; + // If pythonPath equals an empty string, no interpreter is set. + return pythonPath === '' ? undefined : [pythonPath]; } } }; diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index 32787c6fb4ad..e7d024358307 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -630,6 +630,7 @@ export class PythonSettings implements IPythonSettings { } } if (inExperiment && this.pythonPath === DEFAULT_INTERPRETER_SETTING) { + // If no interpreter is selected, set pythonPath to an empty string. // This is to ensure that we ask users to select an interpreter in case auto selected interpreter is not safe to select this.pythonPath = ''; } diff --git a/src/test/api.functional.test.ts b/src/test/api.functional.test.ts index 184746133153..ecc36798189b 100644 --- a/src/test/api.functional.test.ts +++ b/src/test/api.functional.test.ts @@ -41,7 +41,7 @@ suite('Extension API', () => { when(serviceContainer.get(IExperimentsManager)).thenReturn(instance(experimentsManager)); }); - test('Test interpreter path settings API', async () => { + test('Execution command settings API returns expected array if interpreter is set', async () => { const resource = Uri.parse('a'); when(configurationService.getSettings(resource)).thenReturn({ pythonPath: 'settingValue' } as any); @@ -49,9 +49,22 @@ suite('Extension API', () => { Promise.resolve(), instance(serviceManager), instance(serviceContainer) - ).settings.getInterpreterPath(resource); + ).settings.getExecutionCommand(resource); - expect(interpreterPath).to.equal('settingValue'); + expect(interpreterPath).to.equal(['settingValue']); + }); + + test('Execution command settings API returns `undefined` if interpreter is set', async () => { + const resource = Uri.parse('a'); + when(configurationService.getSettings(resource)).thenReturn({ pythonPath: '' } as any); + + const interpreterPath = buildApi( + Promise.resolve(), + instance(serviceManager), + instance(serviceContainer) + ).settings.getExecutionCommand(resource); + + expect(interpreterPath).to.equal(undefined, ''); }); test('Test debug launcher args (no-wait and not in experiment)', async () => { From b8046bd8360e2a21894415869a8c89cb2295b16f Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 22 Apr 2020 12:17:49 -0700 Subject: [PATCH 6/6] Oops --- src/test/api.functional.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/api.functional.test.ts b/src/test/api.functional.test.ts index ecc36798189b..25ece7cc9288 100644 --- a/src/test/api.functional.test.ts +++ b/src/test/api.functional.test.ts @@ -5,7 +5,7 @@ // tslint:disable:no-any max-func-body-length -import { expect } from 'chai'; +import { assert, expect } from 'chai'; import * as path from 'path'; import { anyString, instance, mock, when } from 'ts-mockito'; import { Uri } from 'vscode'; @@ -51,7 +51,7 @@ suite('Extension API', () => { instance(serviceContainer) ).settings.getExecutionCommand(resource); - expect(interpreterPath).to.equal(['settingValue']); + assert.deepEqual(interpreterPath, ['settingValue']); }); test('Execution command settings API returns `undefined` if interpreter is set', async () => {