From 48af4b4042f1227732df40293347af476ad49fd0 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 13 Dec 2021 20:26:33 +0530 Subject: [PATCH 01/24] Group interpreters in interpreter quick picker using separators --- src/client/interpreter/autoSelection/index.ts | 4 +- .../configuration/environmentTypeComparer.ts | 74 +++++++++++-------- .../commands/setInterpreter.ts | 63 ++++++++++++++-- .../interpreterSelector.ts | 2 +- .../pythonEnvironments/base/info/env.ts | 27 +++++-- .../pythonEnvironments/base/info/index.ts | 9 +++ .../environmentTypeComparer.unit.test.ts | 20 ++--- 7 files changed, 143 insertions(+), 56 deletions(-) diff --git a/src/client/interpreter/autoSelection/index.ts b/src/client/interpreter/autoSelection/index.ts index 50d4d4343c34..baf8a637f32d 100644 --- a/src/client/interpreter/autoSelection/index.ts +++ b/src/client/interpreter/autoSelection/index.ts @@ -14,7 +14,7 @@ import { compareSemVerLikeVersions } from '../../pythonEnvironments/base/info/py import { PythonEnvironment } from '../../pythonEnvironments/info'; import { sendTelemetryEvent } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; -import { EnvTypeHeuristic, getEnvTypeHeuristic } from '../configuration/environmentTypeComparer'; +import { EnvLocationHeuristic, getEnvLocationHeuristic } from '../configuration/environmentTypeComparer'; import { IInterpreterComparer } from '../configuration/types'; import { IInterpreterHelper, IInterpreterService } from '../contracts'; import { IInterpreterAutoSelectionService, IInterpreterAutoSelectionProxyService } from './types'; @@ -208,7 +208,7 @@ export class InterpreterAutoSelectionService implements IInterpreterAutoSelectio // or fallback on a globally-installed interpreter, and we don't want want to suggest a global environment // because we would have to add a way to match environments to a workspace. const filteredInterpreters = interpreters.filter( - (i) => getEnvTypeHeuristic(i, workspaceUri?.folderUri.fsPath || '') !== EnvTypeHeuristic.Global, + (i) => getEnvLocationHeuristic(i, workspaceUri?.folderUri.fsPath || '') !== EnvLocationHeuristic.Global, ); filteredInterpreters.sort(this.envTypeComparer.compare.bind(this.envTypeComparer)); diff --git a/src/client/interpreter/configuration/environmentTypeComparer.ts b/src/client/interpreter/configuration/environmentTypeComparer.ts index ea3d8e0a10eb..277d90daf2d3 100644 --- a/src/client/interpreter/configuration/environmentTypeComparer.ts +++ b/src/client/interpreter/configuration/environmentTypeComparer.ts @@ -9,16 +9,15 @@ import { PythonVersion } from '../../pythonEnvironments/info/pythonVersion'; import { IInterpreterHelper } from '../contracts'; import { IInterpreterComparer } from './types'; -/* - * Enum description: - * - Local environments (.venv); - * - Global environments (pipenv, conda); - * - Globally-installed interpreters (/usr/bin/python3, Windows Store). - */ -export enum EnvTypeHeuristic { +export enum EnvLocationHeuristic { + /** + * Environments inside the workspace. + */ Local = 1, + /** + * Environments outside the workspace. + */ Global = 2, - GlobalInterpreters = 3, } @injectable() @@ -41,8 +40,14 @@ export class EnvironmentTypeComparer implements IInterpreterComparer { * Always sort with newest version of Python first within each subgroup. */ public compare(a: PythonEnvironment, b: PythonEnvironment): number { + // Check environment location. + const envLocationComparison = compareEnvironmentLocation(a, b, this.workspaceFolderPath); + if (envLocationComparison !== 0) { + return envLocationComparison; + } + // Check environment type. - const envTypeComparison = compareEnvironmentType(a, b, this.workspaceFolderPath); + const envTypeComparison = compareEnvironmentType(a, b); if (envTypeComparison !== 0) { return envTypeComparison; } @@ -151,11 +156,11 @@ function comparePythonVersionDescending(a: PythonVersion | undefined, b: PythonV } /** - * Compare 2 environment types: return 0 if they are the same, -1 if a comes before b, 1 otherwise. + * Compare 2 environment locations: return 0 if they are the same, -1 if a comes before b, 1 otherwise. */ -function compareEnvironmentType(a: PythonEnvironment, b: PythonEnvironment, workspacePath: string): number { - const aHeuristic = getEnvTypeHeuristic(a, workspacePath); - const bHeuristic = getEnvTypeHeuristic(b, workspacePath); +function compareEnvironmentLocation(a: PythonEnvironment, b: PythonEnvironment, workspacePath: string): number { + const aHeuristic = getEnvLocationHeuristic(a, workspacePath); + const bHeuristic = getEnvLocationHeuristic(b, workspacePath); return Math.sign(aHeuristic - bHeuristic); } @@ -163,28 +168,37 @@ function compareEnvironmentType(a: PythonEnvironment, b: PythonEnvironment, work /** * Return a heuristic value depending on the environment type. */ -export function getEnvTypeHeuristic(environment: PythonEnvironment, workspacePath: string): EnvTypeHeuristic { - const { envType } = environment; - +export function getEnvLocationHeuristic(environment: PythonEnvironment, workspacePath: string): EnvLocationHeuristic { if ( workspacePath.length > 0 && ((environment.envPath && isParentPath(environment.envPath, workspacePath)) || (environment.path && isParentPath(environment.path, workspacePath))) ) { - return EnvTypeHeuristic.Local; + return EnvLocationHeuristic.Local; } + return EnvLocationHeuristic.Global; +} - switch (envType) { - case EnvironmentType.Venv: - case EnvironmentType.Conda: - case EnvironmentType.VirtualEnv: - case EnvironmentType.VirtualEnvWrapper: - case EnvironmentType.Pipenv: - case EnvironmentType.Poetry: - return EnvTypeHeuristic.Global; - // The default case covers global environments. - // For now this includes: pyenv, Windows Store, Global, System and Unknown environment types. - default: - return EnvTypeHeuristic.GlobalInterpreters; - } +/** + * Compare 2 environment types: return 0 if they are the same, -1 if a comes before b, 1 otherwise. + */ +function compareEnvironmentType(a: PythonEnvironment, b: PythonEnvironment): number { + const envTypeByPriority = getPrioritizedEnvironmentType(); + return envTypeByPriority.indexOf(a.envType) - envTypeByPriority.indexOf(b.envType); +} + +function getPrioritizedEnvironmentType(): EnvironmentType[] { + return [ + EnvironmentType.Conda, + EnvironmentType.Poetry, + EnvironmentType.Pipenv, + EnvironmentType.VirtualEnvWrapper, + EnvironmentType.Venv, + EnvironmentType.VirtualEnv, + EnvironmentType.Pyenv, + EnvironmentType.WindowsStore, + EnvironmentType.Global, + EnvironmentType.System, + EnvironmentType.Unknown, + ]; } diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index 047e26c80fe4..8dd53810ce17 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -6,10 +6,10 @@ import { inject, injectable } from 'inversify'; import { cloneDeep } from 'lodash'; import * as path from 'path'; -import { QuickPick, QuickPickItem } from 'vscode'; +import { QuickPick, QuickPickItem, QuickPickItemKind } from 'vscode'; import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../../../common/application/types'; import { Commands, Octicons } from '../../../../common/constants'; -import { arePathsSame } from '../../../../common/platform/fs-paths'; +import { arePathsSame, isParentPath } from '../../../../common/platform/fs-paths'; import { IPlatformService } from '../../../../common/platform/types'; import { IConfigurationService, IPathUtils, Resource } from '../../../../common/types'; import { getIcon } from '../../../../common/utils/icons'; @@ -22,6 +22,7 @@ import { } from '../../../../common/utils/multiStepInput'; import { SystemVariables } from '../../../../common/variables/systemVariables'; import { REFRESH_BUTTON_ICON } from '../../../../debugger/extension/attachQuickPick/types'; +import { EnvironmentType } from '../../../../pythonEnvironments/info'; import { captureTelemetry, sendTelemetryEvent } from '../../../../telemetry'; import { EventName } from '../../../../telemetry/constants'; import { IInterpreterService, PythonEnvironmentsChangedEvent } from '../../../contracts'; @@ -36,7 +37,7 @@ import { BaseInterpreterSelectorCommand } from './base'; const untildify = require('untildify'); export type InterpreterStateArgs = { path?: string; workspace: Resource }; -type QuickPickType = IInterpreterQuickPickItem | ISpecialQuickPickItem; +type QuickPickType = IInterpreterQuickPickItem | ISpecialQuickPickItem | QuickPickItem; function isInterpreterQuickPickItem(item: QuickPickType): item is IInterpreterQuickPickItem { return 'interpreter' in item; @@ -149,15 +150,23 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { if (defaultInterpreterPathSuggestion) { suggestions.push(defaultInterpreterPathSuggestion); } - const interpreterSuggestions = this.interpreterSelector.getSuggestions(resource); + const interpreterSuggestions = this.getSuggestions(resource); this.setRecommendedItem(interpreterSuggestions, resource); suggestions.push(...interpreterSuggestions); return suggestions; } + private getSuggestions(resource: Resource): QuickPickType[] { + const workspaceFolder = this.workspaceService.getWorkspaceFolder(resource); + const items = this.interpreterSelector.getSuggestions(resource); + return getGroupedQuickPickItems(items, workspaceFolder?.uri.fsPath); + } + private getActiveItem(resource: Resource, suggestions: QuickPickType[]) { const currentPythonPath = this.configurationService.getSettings(resource).pythonPath; - const activeInterpreter = suggestions.filter((i) => i.path === currentPythonPath); + const activeInterpreter = suggestions.filter( + (i) => isInterpreterQuickPickItem(i) && i.path === currentPythonPath, + ); if (activeInterpreter.length > 0) { return activeInterpreter[0]; } @@ -253,7 +262,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { // List is in the final state, so first suggestion is the recommended one. const recommended = cloneDeep(interpreterSuggestions[0]); recommended.label = `${Octicons.Star} ${recommended.label}`; - recommended.description = Common.recommended(); + recommended.description = `${recommended.description ?? ''} - ${Common.recommended()}`; const index = items.findIndex( (item) => isInterpreterQuickPickItem(item) && @@ -361,3 +370,45 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { return undefined; } } + +export enum EnvGroups { + Workspace = 'Workspace', + Conda = 'Conda', + Global = 'Global', + VirtualEnv = 'VirtualEnv', + PipEnv = 'PipEnv', + Pyenv = 'Pyenv', + Venv = 'Venv', + Poetry = 'Poetry', + VirtualEnvWrapper = 'VirtualEnvWrapper', +} + +function getGroupedQuickPickItems(items: IInterpreterQuickPickItem[], workspacePath?: string): QuickPickType[] { + const updatedItems: QuickPickType[] = []; + let previousGroup: EnvGroups | undefined; + for (const item of items) { + const currentGroup = getGroup(item, workspacePath); + if (!previousGroup || currentGroup !== previousGroup) { + const separatorItem: QuickPickItem = { label: currentGroup, kind: QuickPickItemKind.Separator }; + updatedItems.push(separatorItem); + previousGroup = currentGroup; + } + updatedItems.push(item); + } + return updatedItems; +} + +function getGroup(item: IInterpreterQuickPickItem, workspacePath?: string) { + if (workspacePath && isParentPath(item.path, workspacePath)) { + return EnvGroups.Workspace; + } + switch (item.interpreter.envType) { + case EnvironmentType.Global: + case EnvironmentType.System: + case EnvironmentType.Unknown: + case EnvironmentType.WindowsStore: + return EnvGroups.Global; + default: + return EnvGroups[item.interpreter.envType]; + } +} diff --git a/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts b/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts index 6f814678b3a3..611526bdb3e0 100644 --- a/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts +++ b/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts @@ -43,7 +43,7 @@ export class InterpreterSelector implements IInterpreterSelector { const cachedPrefix = suggestion.cachedEntry ? '(cached) ' : ''; return { label: suggestion.displayName || 'Python', - detail: `${cachedPrefix}${detail}`, + description: `${cachedPrefix}${detail}`, path: suggestion.path, interpreter: suggestion, }; diff --git a/src/client/pythonEnvironments/base/info/env.ts b/src/client/pythonEnvironments/base/info/env.ts index 33f2bb55a8ff..daf201601930 100644 --- a/src/client/pythonEnvironments/base/info/env.ts +++ b/src/client/pythonEnvironments/base/info/env.ts @@ -11,7 +11,15 @@ import { arePathsSame } from '../../common/externalDependencies'; import { getKindDisplayName } from './envKind'; import { areIdenticalVersion, areSimilarVersions, getVersionDisplayString, isVersionEmpty } from './pythonVersion'; -import { PythonEnvInfo, PythonEnvKind, PythonEnvSource, PythonReleaseLevel, PythonVersion } from '.'; +import { + globallyInstalledEnvKinds, + PythonEnvInfo, + PythonEnvKind, + PythonEnvSource, + PythonReleaseLevel, + PythonVersion, + virtualEnvKinds, +} from '.'; /** * Create a new info object with all values empty. @@ -124,13 +132,16 @@ export function getEnvDisplayString(env: PythonEnvInfo): string { function buildEnvDisplayString(env: PythonEnvInfo): string { // main parts + const shouldDisplayKind = env.searchLocation || globallyInstalledEnvKinds.includes(env.kind); const displayNameParts: string[] = ['Python']; if (env.version && !isVersionEmpty(env.version)) { displayNameParts.push(getVersionDisplayString(env.version)); } - const archName = getArchitectureDisplayName(env.arch); - if (archName !== '') { - displayNameParts.push(archName); + if (!virtualEnvKinds.includes(env.kind)) { + const archName = getArchitectureDisplayName(env.arch); + if (archName !== '') { + displayNameParts.push(archName); + } } // Note that currently we do not use env.distro in the display name. @@ -140,9 +151,11 @@ function buildEnvDisplayString(env: PythonEnvInfo): string { if (env.name && env.name !== '') { envSuffixParts.push(`'${env.name}'`); } - const kindName = getKindDisplayName(env.kind); - if (kindName !== '') { - envSuffixParts.push(kindName); + if (shouldDisplayKind) { + const kindName = getKindDisplayName(env.kind); + if (kindName !== '') { + envSuffixParts.push(kindName); + } } const envSuffix = envSuffixParts.length === 0 ? '' : `(${envSuffixParts.join(': ')})`; diff --git a/src/client/pythonEnvironments/base/info/index.ts b/src/client/pythonEnvironments/base/info/index.ts index 0f3e02f67cf7..df1af1e0ec85 100644 --- a/src/client/pythonEnvironments/base/info/index.ts +++ b/src/client/pythonEnvironments/base/info/index.ts @@ -36,6 +36,15 @@ export const virtualEnvKinds = [ PythonEnvKind.Conda, PythonEnvKind.VirtualEnv, ]; + +export const globallyInstalledEnvKinds = [ + PythonEnvKind.OtherGlobal, + PythonEnvKind.Unknown, + PythonEnvKind.WindowsStore, + PythonEnvKind.System, + PythonEnvKind.Custom, +]; + /** * Information about a file. */ diff --git a/src/test/configuration/environmentTypeComparer.unit.test.ts b/src/test/configuration/environmentTypeComparer.unit.test.ts index c52a2aea584d..f8dfac271eb9 100644 --- a/src/test/configuration/environmentTypeComparer.unit.test.ts +++ b/src/test/configuration/environmentTypeComparer.unit.test.ts @@ -6,8 +6,8 @@ import * as path from 'path'; import * as sinon from 'sinon'; import { EnvironmentTypeComparer, - EnvTypeHeuristic, - getEnvTypeHeuristic, + EnvLocationHeuristic, + getEnvLocationHeuristic, } from '../../client/interpreter/configuration/environmentTypeComparer'; import { IInterpreterHelper } from '../../client/interpreter/contracts'; import { EnvironmentType, PythonEnvironment } from '../../client/pythonEnvironments/info'; @@ -250,9 +250,9 @@ suite('getEnvTypeHeuristic tests', () => { version: { major: 3, minor: 10, patch: 2 }, } as PythonEnvironment; - const envTypeHeuristic = getEnvTypeHeuristic(environment, workspacePath); + const envTypeHeuristic = getEnvLocationHeuristic(environment, workspacePath); - assert.strictEqual(envTypeHeuristic, EnvTypeHeuristic.Local); + assert.strictEqual(envTypeHeuristic, EnvLocationHeuristic.Local); }); test('If the path to an environment does not start with the workspace path it should be marked as global', () => { @@ -262,9 +262,9 @@ suite('getEnvTypeHeuristic tests', () => { version: { major: 3, minor: 10, patch: 2 }, } as PythonEnvironment; - const envTypeHeuristic = getEnvTypeHeuristic(environment, workspacePath); + const envTypeHeuristic = getEnvLocationHeuristic(environment, workspacePath); - assert.strictEqual(envTypeHeuristic, EnvTypeHeuristic.Global); + assert.strictEqual(envTypeHeuristic, EnvLocationHeuristic.Global); }); test('If envPath is not set, fallback to path', () => { @@ -274,9 +274,9 @@ suite('getEnvTypeHeuristic tests', () => { version: { major: 3, minor: 10, patch: 2 }, } as PythonEnvironment; - const envTypeHeuristic = getEnvTypeHeuristic(environment, workspacePath); + const envTypeHeuristic = getEnvLocationHeuristic(environment, workspacePath); - assert.strictEqual(envTypeHeuristic, EnvTypeHeuristic.Local); + assert.strictEqual(envTypeHeuristic, EnvLocationHeuristic.Local); }); }); @@ -296,9 +296,9 @@ suite('getEnvTypeHeuristic tests', () => { version: { major: 3, minor: 10, patch: 2 }, } as PythonEnvironment; - const envTypeHeuristic = getEnvTypeHeuristic(environment, workspacePath); + const envTypeHeuristic = getEnvLocationHeuristic(environment, workspacePath); - assert.strictEqual(envTypeHeuristic, EnvTypeHeuristic.GlobalInterpreters); + assert.strictEqual(envTypeHeuristic, EnvLocationHeuristic.GlobalInterpreters); }); }); }); From 47a4e0efe6be1d42ad94355fb6d76c6c6dd16994 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 13 Dec 2021 22:20:23 +0530 Subject: [PATCH 02/24] Do not group while the list is loading --- .../commands/setInterpreter.ts | 8 ++++++- .../interpreterSelector.ts | 12 ++++++---- src/client/interpreter/configuration/types.ts | 8 +++++-- .../pythonEnvironments/base/info/env.ts | 12 +++++----- .../pythonEnvironments/base/info/index.ts | 2 ++ .../base/locators/composite/envsResolver.ts | 4 ++-- .../base/locators/composite/resolverUtils.ts | 4 ++-- src/client/pythonEnvironments/info/index.ts | 1 + src/client/pythonEnvironments/legacyIOC.ts | 1 + .../base/info/env.unit.test.ts | 6 ++--- .../composite/resolverUtils.unit.test.ts | 22 +++++++++---------- 11 files changed, 50 insertions(+), 30 deletions(-) diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index 8dd53810ce17..2442dd5b07d8 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -158,7 +158,11 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { private getSuggestions(resource: Resource): QuickPickType[] { const workspaceFolder = this.workspaceService.getWorkspaceFolder(resource); - const items = this.interpreterSelector.getSuggestions(resource); + const items = this.interpreterSelector.getSuggestions(resource, !!this.interpreterService.refreshPromise); + if (this.interpreterService.refreshPromise) { + // We cannot put items in groups while the list is loading as group of an item can change. + return items; + } return getGroupedQuickPickItems(items, workspaceFolder?.uri.fsPath); } @@ -242,6 +246,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { const newSuggestion: QuickPickType = this.interpreterSelector.suggestionToQuickPickItem( event.new, resource, + true, ); if (envIndex === -1) { updatedItems.push(newSuggestion); @@ -381,6 +386,7 @@ export enum EnvGroups { Venv = 'Venv', Poetry = 'Poetry', VirtualEnvWrapper = 'VirtualEnvWrapper', + NewlyDiscovered = 'NewlyDiscovered', } function getGroupedQuickPickItems(items: IInterpreterQuickPickItem[], workspacePath?: string): QuickPickType[] { diff --git a/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts b/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts index 611526bdb3e0..fcbb6b87ff04 100644 --- a/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts +++ b/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts @@ -24,11 +24,11 @@ export class InterpreterSelector implements IInterpreterSelector { this.disposables.forEach((disposable) => disposable.dispose()); } - public getSuggestions(resource: Resource): IInterpreterQuickPickItem[] { + public getSuggestions(resource: Resource, useFullDisplayName = false): IInterpreterQuickPickItem[] { const interpreters = this.interpreterManager.getInterpreters(resource); interpreters.sort(this.envTypeComparer.compare.bind(this.envTypeComparer)); - return interpreters.map((item) => this.suggestionToQuickPickItem(item, resource)); + return interpreters.map((item) => this.suggestionToQuickPickItem(item, resource, useFullDisplayName)); } public async getAllSuggestions(resource: Resource): Promise { @@ -38,11 +38,15 @@ export class InterpreterSelector implements IInterpreterSelector { return Promise.all(interpreters.map((item) => this.suggestionToQuickPickItem(item, resource))); } - public suggestionToQuickPickItem(suggestion: PythonEnvironment, workspaceUri?: Uri): IInterpreterQuickPickItem { + public suggestionToQuickPickItem( + suggestion: PythonEnvironment, + workspaceUri?: Uri, + useDetailedName = false, + ): IInterpreterQuickPickItem { const detail = this.pathUtils.getDisplayName(suggestion.path, workspaceUri ? workspaceUri.fsPath : undefined); const cachedPrefix = suggestion.cachedEntry ? '(cached) ' : ''; return { - label: suggestion.displayName || 'Python', + label: (useDetailedName ? suggestion.detailedDisplayName : suggestion.displayName) || 'Python', description: `${cachedPrefix}${detail}`, path: suggestion.path, interpreter: suggestion, diff --git a/src/client/interpreter/configuration/types.ts b/src/client/interpreter/configuration/types.ts index d9f03ddc2bae..27742634c6c3 100644 --- a/src/client/interpreter/configuration/types.ts +++ b/src/client/interpreter/configuration/types.ts @@ -26,8 +26,12 @@ export interface IPythonPathUpdaterServiceManager { export const IInterpreterSelector = Symbol('IInterpreterSelector'); export interface IInterpreterSelector extends Disposable { getAllSuggestions(resource: Resource): Promise; - getSuggestions(resource: Resource): IInterpreterQuickPickItem[]; - suggestionToQuickPickItem(suggestion: PythonEnvironment, workspaceUri?: Uri | undefined): IInterpreterQuickPickItem; + getSuggestions(resource: Resource, useFullDisplayName?: boolean): IInterpreterQuickPickItem[]; + suggestionToQuickPickItem( + suggestion: PythonEnvironment, + workspaceUri?: Uri | undefined, + useDetailedName?: boolean, + ): IInterpreterQuickPickItem; } export interface IInterpreterQuickPickItem extends QuickPickItem { diff --git a/src/client/pythonEnvironments/base/info/env.ts b/src/client/pythonEnvironments/base/info/env.ts index daf201601930..3cc7021b4ffc 100644 --- a/src/client/pythonEnvironments/base/info/env.ts +++ b/src/client/pythonEnvironments/base/info/env.ts @@ -126,18 +126,20 @@ function updateEnv( * The format is `Python (: )` * E.g. `Python 3.5.1 32-bit (myenv2: virtualenv)` */ -export function getEnvDisplayString(env: PythonEnvInfo): string { - return buildEnvDisplayString(env); +export function setEnvDisplayString(env: PythonEnvInfo): void { + env.display = buildEnvDisplayString(env); + env.detailedDisplayName = buildEnvDisplayString(env, true); } -function buildEnvDisplayString(env: PythonEnvInfo): string { +function buildEnvDisplayString(env: PythonEnvInfo, getAllDetails = false): string { // main parts - const shouldDisplayKind = env.searchLocation || globallyInstalledEnvKinds.includes(env.kind); + const shouldDisplayKind = getAllDetails || env.searchLocation || globallyInstalledEnvKinds.includes(env.kind); + const shouldDisplayArch = getAllDetails || !virtualEnvKinds.includes(env.kind); const displayNameParts: string[] = ['Python']; if (env.version && !isVersionEmpty(env.version)) { displayNameParts.push(getVersionDisplayString(env.version)); } - if (!virtualEnvKinds.includes(env.kind)) { + if (shouldDisplayArch) { const archName = getArchitectureDisplayName(env.arch); if (archName !== '') { displayNameParts.push(archName); diff --git a/src/client/pythonEnvironments/base/info/index.ts b/src/client/pythonEnvironments/base/info/index.ts index df1af1e0ec85..e18788cb1c22 100644 --- a/src/client/pythonEnvironments/base/info/index.ts +++ b/src/client/pythonEnvironments/base/info/index.ts @@ -177,11 +177,13 @@ type _PythonEnvInfo = PythonEnvBaseInfo & PythonBuildInfo; * * @prop distro - the installed Python distro that this env is using or belongs to * @prop display - the text to use when showing the env to users + * @prop detailedDisplayName - display name containing all details * @prop searchLocation - the root under which a locator found this env, if any */ export type PythonEnvInfo = _PythonEnvInfo & { distro: PythonDistroInfo; display?: string; + detailedDisplayName?: string; searchLocation?: Uri; }; diff --git a/src/client/pythonEnvironments/base/locators/composite/envsResolver.ts b/src/client/pythonEnvironments/base/locators/composite/envsResolver.ts index bf07dd6b3ec0..f92f542f7c13 100644 --- a/src/client/pythonEnvironments/base/locators/composite/envsResolver.ts +++ b/src/client/pythonEnvironments/base/locators/composite/envsResolver.ts @@ -6,7 +6,7 @@ import { Event, EventEmitter } from 'vscode'; import { identifyEnvironment } from '../../../common/environmentIdentifier'; import { IEnvironmentInfoService } from '../../info/environmentInfoService'; import { PythonEnvInfo } from '../../info'; -import { getEnvDisplayString } from '../../info/env'; +import { setEnvDisplayString } from '../../info/env'; import { InterpreterInformation } from '../../info/interpreter'; import { BasicEnvInfo, @@ -145,6 +145,6 @@ function getResolvedEnv(interpreterInfo: InterpreterInformation, environment: Py resolvedEnv.executable.sysPrefix = interpreterInfo.executable.sysPrefix; resolvedEnv.arch = interpreterInfo.arch; // Display name should be set after all the properties as we need other properties to build display name. - resolvedEnv.display = getEnvDisplayString(resolvedEnv); + setEnvDisplayString(resolvedEnv); return resolvedEnv; } diff --git a/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts b/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts index fe1f548fe09b..a3e00e35d232 100644 --- a/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts +++ b/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts @@ -5,7 +5,7 @@ import * as path from 'path'; import { Uri } from 'vscode'; import { uniq } from 'lodash'; import { PythonEnvInfo, PythonEnvKind, PythonEnvSource, UNKNOWN_PYTHON_VERSION, virtualEnvKinds } from '../../info'; -import { buildEnvInfo, comparePythonVersionSpecificity, getEnvDisplayString, getEnvMatcher } from '../../info/env'; +import { buildEnvInfo, comparePythonVersionSpecificity, setEnvDisplayString, getEnvMatcher } from '../../info/env'; import { getEnvironmentDirFromPath, getInterpreterPathFromDir, @@ -52,7 +52,7 @@ export async function resolveBasicEnv({ kind, executablePath, source }: BasicEnv // We can update env further using information we can get from the Windows registry. await updateEnvUsingRegistry(resolvedEnv); } - resolvedEnv.display = getEnvDisplayString(resolvedEnv); + setEnvDisplayString(resolvedEnv); return resolvedEnv; } diff --git a/src/client/pythonEnvironments/info/index.ts b/src/client/pythonEnvironments/info/index.ts index 187c2755f7f9..24e334a85477 100644 --- a/src/client/pythonEnvironments/info/index.ts +++ b/src/client/pythonEnvironments/info/index.ts @@ -68,6 +68,7 @@ export type InterpreterInformation = { export type PythonEnvironment = InterpreterInformation & { companyDisplayName?: string; displayName?: string; + detailedDisplayName?: string; envType: EnvironmentType; envName?: string; envPath?: string; diff --git a/src/client/pythonEnvironments/legacyIOC.ts b/src/client/pythonEnvironments/legacyIOC.ts index 7c2e774b4740..97ac3f6bac90 100644 --- a/src/client/pythonEnvironments/legacyIOC.ts +++ b/src/client/pythonEnvironments/legacyIOC.ts @@ -75,6 +75,7 @@ function convertEnvInfo(info: PythonEnvInfo): PythonEnvironment { env.companyDisplayName = distro.org; } env.displayName = info.display; + env.detailedDisplayName = info.detailedDisplayName; // We do not worry about using distro.defaultDisplayName. return env; diff --git a/src/test/pythonEnvironments/base/info/env.unit.test.ts b/src/test/pythonEnvironments/base/info/env.unit.test.ts index e29d054a4480..b436ad99fec2 100644 --- a/src/test/pythonEnvironments/base/info/env.unit.test.ts +++ b/src/test/pythonEnvironments/base/info/env.unit.test.ts @@ -5,7 +5,7 @@ import * as assert from 'assert'; import { Architecture } from '../../../../client/common/utils/platform'; import { parseVersionInfo } from '../../../../client/common/utils/version'; import { PythonEnvInfo, PythonDistroInfo, PythonEnvKind } from '../../../../client/pythonEnvironments/base/info'; -import { getEnvDisplayString } from '../../../../client/pythonEnvironments/base/info/env'; +import { setEnvDisplayString } from '../../../../client/pythonEnvironments/base/info/env'; import { createLocatedEnv } from '../common'; suite('pyenvs info - getEnvDisplayString()', () => { @@ -58,9 +58,9 @@ suite('pyenvs info - getEnvDisplayString()', () => { ]; tests.forEach(([env, expected]) => { test(`"${expected}"`, () => { - const result = getEnvDisplayString(env); + setEnvDisplayString(env); - assert.strictEqual(result, expected); + assert.equal(env.display, expected); }); }); }); diff --git a/src/test/pythonEnvironments/base/locators/composite/resolverUtils.unit.test.ts b/src/test/pythonEnvironments/base/locators/composite/resolverUtils.unit.test.ts index 556072dd0804..bd12004e31ef 100644 --- a/src/test/pythonEnvironments/base/locators/composite/resolverUtils.unit.test.ts +++ b/src/test/pythonEnvironments/base/locators/composite/resolverUtils.unit.test.ts @@ -14,7 +14,7 @@ import { PythonVersion, UNKNOWN_PYTHON_VERSION, } from '../../../../../client/pythonEnvironments/base/info'; -import { buildEnvInfo, getEnvDisplayString } from '../../../../../client/pythonEnvironments/base/info/env'; +import { buildEnvInfo, setEnvDisplayString } from '../../../../../client/pythonEnvironments/base/info/env'; import { InterpreterInformation } from '../../../../../client/pythonEnvironments/base/info/interpreter'; import { parseVersion } from '../../../../../client/pythonEnvironments/base/info/pythonVersion'; import { TEST_LAYOUT_ROOT } from '../../../common/commonTestConstants'; @@ -61,7 +61,7 @@ suite('Resolver Utils', () => { }); envInfo.location = path.join(testPyenvVersionsDir, '3.9.0'); envInfo.name = '3.9.0'; - envInfo.display = getEnvDisplayString(envInfo); + setEnvDisplayString(envInfo); return envInfo; } @@ -125,7 +125,7 @@ suite('Resolver Utils', () => { source: [PythonEnvSource.PathEnvVar], ...createExpectedInterpreterInfo(python38path), }; - expected.display = getEnvDisplayString(expected); + setEnvDisplayString(expected); const actual = await resolveBasicEnv({ executablePath: python38path, @@ -147,7 +147,7 @@ suite('Resolver Utils', () => { source: [PythonEnvSource.PathEnvVar], ...createExpectedInterpreterInfo(python38path), }; - expected.display = getEnvDisplayString(expected); + setEnvDisplayString(expected); const actual = await resolveBasicEnv({ executablePath: python38path, @@ -183,7 +183,7 @@ suite('Resolver Utils', () => { fileInfo: undefined, name: 'base', }); - info.display = getEnvDisplayString(info); + setEnvDisplayString(info); return info; } function createSimpleEnvInfo( @@ -210,7 +210,7 @@ suite('Resolver Utils', () => { searchLocation: undefined, source: [], }; - info.display = getEnvDisplayString(info); + setEnvDisplayString(info); return info; } @@ -307,7 +307,7 @@ suite('Resolver Utils', () => { searchLocation: Uri.file(path.dirname(location)), source: [], }; - info.display = getEnvDisplayString(info); + setEnvDisplayString(info); return info; } @@ -362,7 +362,7 @@ suite('Resolver Utils', () => { searchLocation: undefined, source: [], }; - info.display = getEnvDisplayString(info); + setEnvDisplayString(info); return info; } @@ -550,7 +550,7 @@ suite('Resolver Utils', () => { org: 'PythonCore', source: [PythonEnvSource.WindowsRegistry], }); - expected.display = getEnvDisplayString(expected); + setEnvDisplayString(expected); expected.distro.defaultDisplayName = 'Python 3.9 (64-bit)'; assertEnvEqual(actual, expected); }); @@ -570,7 +570,7 @@ suite('Resolver Utils', () => { org: 'PythonCodingPack', // Provided by registry source: [PythonEnvSource.WindowsRegistry, PythonEnvSource.PathEnvVar], }); - expected.display = getEnvDisplayString(expected); + setEnvDisplayString(expected); expected.distro.defaultDisplayName = 'Python 3.8 (32-bit)'; assertEnvEqual(actual, expected); }); @@ -597,7 +597,7 @@ suite('Resolver Utils', () => { name: 'conda3', source: [PythonEnvSource.WindowsRegistry], }); - expected.display = getEnvDisplayString(expected); + setEnvDisplayString(expected); expected.distro.defaultDisplayName = 'Anaconda py38_4.8.3'; assertEnvEqual(actual, expected); }); From 872715c2fb0377e3a10dbb2e3a58fee306bafcc8 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 14 Dec 2021 16:59:29 +0530 Subject: [PATCH 03/24] Cleanup --- .../commands/setInterpreter.ts | 42 ++++++++++++++----- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index 2442dd5b07d8..340994ff906e 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -9,6 +9,7 @@ import * as path from 'path'; import { QuickPick, QuickPickItem, QuickPickItemKind } from 'vscode'; import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../../../common/application/types'; import { Commands, Octicons } from '../../../../common/constants'; +import { FileChangeType } from '../../../../common/platform/fileSystemWatcher'; import { arePathsSame, isParentPath } from '../../../../common/platform/fs-paths'; import { IPlatformService } from '../../../../common/platform/types'; import { IConfigurationService, IPathUtils, Resource } from '../../../../common/types'; @@ -243,12 +244,15 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { ); } if (event.new) { - const newSuggestion: QuickPickType = this.interpreterSelector.suggestionToQuickPickItem( - event.new, - resource, - true, - ); + const newSuggestion = this.interpreterSelector.suggestionToQuickPickItem(event.new, resource, true); if (envIndex === -1) { + if (event.type === FileChangeType.Created) { + addSeparatorIfApplicable( + updatedItems, + newSuggestion, + this.workspaceService.getWorkspaceFolder(resource)?.uri.fsPath, + ); + } updatedItems.push(newSuggestion); } else { updatedItems[envIndex] = newSuggestion; @@ -393,17 +397,33 @@ function getGroupedQuickPickItems(items: IInterpreterQuickPickItem[], workspaceP const updatedItems: QuickPickType[] = []; let previousGroup: EnvGroups | undefined; for (const item of items) { - const currentGroup = getGroup(item, workspacePath); - if (!previousGroup || currentGroup !== previousGroup) { - const separatorItem: QuickPickItem = { label: currentGroup, kind: QuickPickItemKind.Separator }; - updatedItems.push(separatorItem); - previousGroup = currentGroup; - } + addSeparatorIfApplicable(updatedItems, item, workspacePath, previousGroup); updatedItems.push(item); } return updatedItems; } +function addSeparatorIfApplicable( + items: QuickPickType[], + newItem: IInterpreterQuickPickItem, + workspacePath?: string, + previousGroup?: EnvGroups | undefined, +) { + if (!previousGroup) { + const lastItem = items.length ? items[items.length - 1] : undefined; + previousGroup = + items.length && lastItem && isInterpreterQuickPickItem(lastItem) + ? getGroup(lastItem, workspacePath) + : undefined; + } + const currentGroup = getGroup(newItem, workspacePath); + if (!previousGroup || currentGroup !== previousGroup) { + const separatorItem: QuickPickItem = { label: currentGroup, kind: QuickPickItemKind.Separator }; + items.push(separatorItem); + previousGroup = currentGroup; + } +} + function getGroup(item: IInterpreterQuickPickItem, workspacePath?: string) { if (workspacePath && isParentPath(item.path, workspacePath)) { return EnvGroups.Workspace; From bafea218a10d099be372e08ef2eb58b8884a3ae8 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 14 Dec 2021 17:38:43 +0530 Subject: [PATCH 04/24] Ensure full display name does not have arch for non-global type envs --- .../interpreterSelector/commands/setInterpreter.ts | 14 +++++++++++--- src/client/interpreter/display/index.ts | 4 ++-- src/client/pythonEnvironments/base/info/env.ts | 2 +- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index 340994ff906e..12519d82e6fe 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -9,7 +9,6 @@ import * as path from 'path'; import { QuickPick, QuickPickItem, QuickPickItemKind } from 'vscode'; import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../../../common/application/types'; import { Commands, Octicons } from '../../../../common/constants'; -import { FileChangeType } from '../../../../common/platform/fileSystemWatcher'; import { arePathsSame, isParentPath } from '../../../../common/platform/fs-paths'; import { IPlatformService } from '../../../../common/platform/types'; import { IConfigurationService, IPathUtils, Resource } from '../../../../common/types'; @@ -48,6 +47,10 @@ function isSpecialQuickPickItem(item: QuickPickType): item is ISpecialQuickPickI return 'alwaysShow' in item; } +function isSeparatorItem(item: QuickPickType): item is QuickPickItem { + return 'kind' in item && item.kind === QuickPickItemKind.Separator; +} + @injectable() export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { private readonly manualEntrySuggestion: ISpecialQuickPickItem = { @@ -236,6 +239,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { resource: Resource, ): QuickPickType[] { const updatedItems = [...items.values()]; + const areItemsGrouped = items.find((item) => isSeparatorItem(item)); const env = event.old ?? event.new; let envIndex = -1; if (env) { @@ -244,9 +248,13 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { ); } if (event.new) { - const newSuggestion = this.interpreterSelector.suggestionToQuickPickItem(event.new, resource, true); + const newSuggestion = this.interpreterSelector.suggestionToQuickPickItem( + event.new, + resource, + !areItemsGrouped, + ); if (envIndex === -1) { - if (event.type === FileChangeType.Created) { + if (areItemsGrouped) { addSeparatorIfApplicable( updatedItems, newSuggestion, diff --git a/src/client/interpreter/display/index.ts b/src/client/interpreter/display/index.ts index 0382641e954b..ea09edefa89d 100644 --- a/src/client/interpreter/display/index.ts +++ b/src/client/interpreter/display/index.ts @@ -119,7 +119,7 @@ export class InterpreterDisplay implements IInterpreterDisplay, IExtensionSingle ); this.interpreterPath = interpreter.path; } - let text = interpreter.displayName!; + let text = interpreter.detailedDisplayName!; if (this.experiments.inExperimentSync(InterpreterStatusBarPosition.Pinned)) { text = text.startsWith('Python') ? text.substring('Python'.length).trim() : text; } @@ -142,7 +142,7 @@ export class InterpreterDisplay implements IInterpreterDisplay, IExtensionSingle ); this.interpreterPath = interpreter.path; } - let text = interpreter.displayName!; + let text = interpreter.detailedDisplayName!; text = text.startsWith('Python') ? text.substring('Python'.length).trim() : text; this.languageStatus.text = text; this.currentlySelectedInterpreterPath = interpreter.path; diff --git a/src/client/pythonEnvironments/base/info/env.ts b/src/client/pythonEnvironments/base/info/env.ts index 3cc7021b4ffc..86f5c16b02e3 100644 --- a/src/client/pythonEnvironments/base/info/env.ts +++ b/src/client/pythonEnvironments/base/info/env.ts @@ -134,7 +134,7 @@ export function setEnvDisplayString(env: PythonEnvInfo): void { function buildEnvDisplayString(env: PythonEnvInfo, getAllDetails = false): string { // main parts const shouldDisplayKind = getAllDetails || env.searchLocation || globallyInstalledEnvKinds.includes(env.kind); - const shouldDisplayArch = getAllDetails || !virtualEnvKinds.includes(env.kind); + const shouldDisplayArch = !virtualEnvKinds.includes(env.kind); const displayNameParts: string[] = ['Python']; if (env.version && !isVersionEmpty(env.version)) { displayNameParts.push(getVersionDisplayString(env.version)); From d849bfbac2bd4d55e05ea058c120e6687adf8a8c Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 7 Jan 2022 23:52:29 +0530 Subject: [PATCH 05/24] tes --- .../interpreterSelector/commands/setInterpreter.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index 12519d82e6fe..1c3100214a22 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -405,7 +405,7 @@ function getGroupedQuickPickItems(items: IInterpreterQuickPickItem[], workspaceP const updatedItems: QuickPickType[] = []; let previousGroup: EnvGroups | undefined; for (const item of items) { - addSeparatorIfApplicable(updatedItems, item, workspacePath, previousGroup); + previousGroup = addSeparatorIfApplicable(updatedItems, item, workspacePath, previousGroup); updatedItems.push(item); } return updatedItems; @@ -430,6 +430,7 @@ function addSeparatorIfApplicable( items.push(separatorItem); previousGroup = currentGroup; } + return previousGroup; } function getGroup(item: IInterpreterQuickPickItem, workspacePath?: string) { From 5f3e2106df24ca4cdc7df3ca3329eb5decd5ee1e Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sat, 8 Jan 2022 00:51:12 +0530 Subject: [PATCH 06/24] Recommend the same interpreter we autoselect --- src/client/interpreter/autoSelection/index.ts | 18 ++++++----------- .../configuration/environmentTypeComparer.ts | 20 ++++++++++++++++++- .../commands/setInterpreter.ts | 10 ++++++++-- .../interpreterSelector.ts | 13 ++++++++++++ src/client/interpreter/configuration/types.ts | 5 +++++ src/client/pythonEnvironments/info/index.ts | 9 +++++++++ .../environmentTypeComparer.unit.test.ts | 2 +- 7 files changed, 61 insertions(+), 16 deletions(-) diff --git a/src/client/interpreter/autoSelection/index.ts b/src/client/interpreter/autoSelection/index.ts index baf8a637f32d..3a23422e53fd 100644 --- a/src/client/interpreter/autoSelection/index.ts +++ b/src/client/interpreter/autoSelection/index.ts @@ -14,7 +14,6 @@ import { compareSemVerLikeVersions } from '../../pythonEnvironments/base/info/py import { PythonEnvironment } from '../../pythonEnvironments/info'; import { sendTelemetryEvent } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; -import { EnvLocationHeuristic, getEnvLocationHeuristic } from '../configuration/environmentTypeComparer'; import { IInterpreterComparer } from '../configuration/types'; import { IInterpreterHelper, IInterpreterService } from '../contracts'; import { IInterpreterAutoSelectionService, IInterpreterAutoSelectionProxyService } from './types'; @@ -204,19 +203,14 @@ export class InterpreterAutoSelectionService implements IInterpreterAutoSelectio const interpreters = await this.interpreterService.getAllInterpreters(resource); const workspaceUri = this.interpreterHelper.getActiveWorkspaceUri(resource); - // When auto-selecting an intepreter for a workspace, we either want to return a local one - // or fallback on a globally-installed interpreter, and we don't want want to suggest a global environment - // because we would have to add a way to match environments to a workspace. - const filteredInterpreters = interpreters.filter( - (i) => getEnvLocationHeuristic(i, workspaceUri?.folderUri.fsPath || '') !== EnvLocationHeuristic.Global, - ); - - filteredInterpreters.sort(this.envTypeComparer.compare.bind(this.envTypeComparer)); - + const recommendedInterpreter = this.envTypeComparer.getRecommended(interpreters); + if (!recommendedInterpreter) { + return; + } if (workspaceUri) { - this.setWorkspaceInterpreter(workspaceUri.folderUri, filteredInterpreters[0]); + this.setWorkspaceInterpreter(workspaceUri.folderUri, recommendedInterpreter); } else { - this.setGlobalInterpreter(filteredInterpreters[0]); + this.setGlobalInterpreter(recommendedInterpreter); } queriedState.updateValue(true); diff --git a/src/client/interpreter/configuration/environmentTypeComparer.ts b/src/client/interpreter/configuration/environmentTypeComparer.ts index 277d90daf2d3..9adf7d9960ba 100644 --- a/src/client/interpreter/configuration/environmentTypeComparer.ts +++ b/src/client/interpreter/configuration/environmentTypeComparer.ts @@ -3,8 +3,9 @@ import { injectable, inject } from 'inversify'; import { getArchitectureDisplayName } from '../../common/platform/registry'; +import { Resource } from '../../common/types'; import { isParentPath } from '../../pythonEnvironments/common/externalDependencies'; -import { EnvironmentType, PythonEnvironment } from '../../pythonEnvironments/info'; +import { EnvironmentType, PythonEnvironment, virtualEnvTypes } from '../../pythonEnvironments/info'; import { PythonVersion } from '../../pythonEnvironments/info/pythonVersion'; import { IInterpreterHelper } from '../contracts'; import { IInterpreterComparer } from './types'; @@ -81,6 +82,23 @@ export class EnvironmentTypeComparer implements IInterpreterComparer { return nameA > nameB ? 1 : -1; } + + public getRecommended(interpreters: PythonEnvironment[], workspaceUri?: Resource): PythonEnvironment | undefined { + // When recommending an intepreter for a workspace, we either want to return a local one + // or fallback on a globally-installed interpreter, and we don't want want to suggest a global environment + // because we would have to add a way to match environments to a workspace. + const filteredInterpreters = interpreters.filter((i) => { + if (getEnvLocationHeuristic(i, workspaceUri?.fsPath || '') === EnvLocationHeuristic.Local) { + return true; + } + if (virtualEnvTypes.includes(i.envType)) { + return false; + } + return true; + }); + filteredInterpreters.sort(this.compare.bind(this)); + return filteredInterpreters.length ? filteredInterpreters[0] : undefined; + } } function getSortName(info: PythonEnvironment, interpreterHelper: IInterpreterHelper): string { diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index 1c3100214a22..637d71db5074 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -276,8 +276,14 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { private setRecommendedItem(items: QuickPickType[], resource: Resource) { const interpreterSuggestions = this.interpreterSelector.getSuggestions(resource); if (!this.interpreterService.refreshPromise && interpreterSuggestions.length > 0) { - // List is in the final state, so first suggestion is the recommended one. - const recommended = cloneDeep(interpreterSuggestions[0]); + const suggestion = this.interpreterSelector.getRecommendedSuggestion( + interpreterSuggestions, + this.workspaceService.getWorkspaceFolder(resource)?.uri, + ); + if (!suggestion) { + return; + } + const recommended = cloneDeep(suggestion); recommended.label = `${Octicons.Star} ${recommended.label}`; recommended.description = `${recommended.description ?? ''} - ${Common.recommended()}`; const index = items.findIndex( diff --git a/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts b/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts index fcbb6b87ff04..6496286406b5 100644 --- a/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts +++ b/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts @@ -5,6 +5,7 @@ import { inject, injectable } from 'inversify'; import { Disposable, Uri } from 'vscode'; +import { arePathsSame } from '../../../common/platform/fs-paths'; import { IPathUtils, Resource } from '../../../common/types'; import { PythonEnvironment } from '../../../pythonEnvironments/info'; import { IInterpreterService } from '../../contracts'; @@ -52,4 +53,16 @@ export class InterpreterSelector implements IInterpreterSelector { interpreter: suggestion, }; } + + public getRecommendedSuggestion( + suggestions: IInterpreterQuickPickItem[], + resource: Resource, + ): IInterpreterQuickPickItem | undefined { + const envs = this.interpreterManager.getInterpreters(resource); + const recommendedEnv = this.envTypeComparer.getRecommended(envs); + if (!recommendedEnv) { + return undefined; + } + return suggestions.find((item) => arePathsSame(item.interpreter.path, recommendedEnv.path)); + } } diff --git a/src/client/interpreter/configuration/types.ts b/src/client/interpreter/configuration/types.ts index 27742634c6c3..2cec501174cb 100644 --- a/src/client/interpreter/configuration/types.ts +++ b/src/client/interpreter/configuration/types.ts @@ -25,6 +25,10 @@ export interface IPythonPathUpdaterServiceManager { export const IInterpreterSelector = Symbol('IInterpreterSelector'); export interface IInterpreterSelector extends Disposable { + getRecommendedSuggestion( + suggestions: IInterpreterQuickPickItem[], + resource: Resource, + ): IInterpreterQuickPickItem | undefined; getAllSuggestions(resource: Resource): Promise; getSuggestions(resource: Resource, useFullDisplayName?: boolean): IInterpreterQuickPickItem[]; suggestionToQuickPickItem( @@ -56,4 +60,5 @@ export interface ISpecialQuickPickItem { export const IInterpreterComparer = Symbol('IInterpreterComparer'); export interface IInterpreterComparer { compare(a: PythonEnvironment, b: PythonEnvironment): number; + getRecommended(interpreters: PythonEnvironment[], workspaceUri?: Resource): PythonEnvironment | undefined; } diff --git a/src/client/pythonEnvironments/info/index.ts b/src/client/pythonEnvironments/info/index.ts index 24e334a85477..0bed9b91179a 100644 --- a/src/client/pythonEnvironments/info/index.ts +++ b/src/client/pythonEnvironments/info/index.ts @@ -23,6 +23,15 @@ export enum EnvironmentType { System = 'System', } +export const virtualEnvTypes = [ + EnvironmentType.Poetry, + EnvironmentType.Pipenv, + EnvironmentType.Venv, + EnvironmentType.VirtualEnvWrapper, + EnvironmentType.Conda, + EnvironmentType.VirtualEnv, +]; + /** * The IModuleInstaller implementations. */ diff --git a/src/test/configuration/environmentTypeComparer.unit.test.ts b/src/test/configuration/environmentTypeComparer.unit.test.ts index f8dfac271eb9..e3b909378b63 100644 --- a/src/test/configuration/environmentTypeComparer.unit.test.ts +++ b/src/test/configuration/environmentTypeComparer.unit.test.ts @@ -298,7 +298,7 @@ suite('getEnvTypeHeuristic tests', () => { const envTypeHeuristic = getEnvLocationHeuristic(environment, workspacePath); - assert.strictEqual(envTypeHeuristic, EnvLocationHeuristic.GlobalInterpreters); + assert.strictEqual(envTypeHeuristic, EnvLocationHeuristic.Global); }); }); }); From a0ac6add15808f06f44941570e3fcd71ba373674 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sat, 8 Jan 2022 17:31:33 +0530 Subject: [PATCH 07/24] Fix env resikver tst --- .../base/locators/composite/envsResolver.unit.test.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/test/pythonEnvironments/base/locators/composite/envsResolver.unit.test.ts b/src/test/pythonEnvironments/base/locators/composite/envsResolver.unit.test.ts index ddc0510d03fa..3d6dcd528912 100644 --- a/src/test/pythonEnvironments/base/locators/composite/envsResolver.unit.test.ts +++ b/src/test/pythonEnvironments/base/locators/composite/envsResolver.unit.test.ts @@ -57,6 +57,7 @@ suite('Python envs locator - Environments Resolver', () => { updatedEnv.executable.sysPrefix = 'path'; updatedEnv.arch = Architecture.x64; updatedEnv.display = expectedDisplay; + updatedEnv.detailedDisplayName = expectedDisplay; return updatedEnv; } @@ -79,6 +80,7 @@ suite('Python envs locator - Environments Resolver', () => { mtime: -1, }, display, + detailedDisplayName: display, version, arch: Architecture.Unknown, distro: { org: '' }, @@ -151,7 +153,7 @@ suite('Python envs locator - Environments Resolver', () => { const envs = await getEnvsWithUpdates(iterator); assertEnvsEqual(envs, [ - createExpectedEnvInfo(resolvedEnvReturnedByBasicResolver, "Python 3.8.3 64-bit ('win1': venv)"), + createExpectedEnvInfo(resolvedEnvReturnedByBasicResolver, "Python 3.8.3 ('win1': venv)"), ]); }); @@ -215,10 +217,7 @@ suite('Python envs locator - Environments Resolver', () => { // Assert assertEnvsEqual(envs, [ - createExpectedEnvInfo( - resolvedUpdatedEnvReturnedByBasicResolver, - "Python 3.8.3 64-bit ('win1': poetry)", - ), + createExpectedEnvInfo(resolvedUpdatedEnvReturnedByBasicResolver, "Python 3.8.3 ('win1': poetry)"), ]); didUpdate.dispose(); }); @@ -276,7 +275,7 @@ suite('Python envs locator - Environments Resolver', () => { assertEnvEqual( expected, - createExpectedEnvInfo(resolvedEnvReturnedByBasicResolver, "Python 3.8.3 64-bit ('win1': venv)"), + createExpectedEnvInfo(resolvedEnvReturnedByBasicResolver, "Python 3.8.3 ('win1': venv)"), ); }); From ad498218adab7a57d6bae3f472a088b7c7288d3e Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sat, 8 Jan 2022 17:55:54 +0530 Subject: [PATCH 08/24] Fix display name testS --- .../base/info/env.unit.test.ts | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/test/pythonEnvironments/base/info/env.unit.test.ts b/src/test/pythonEnvironments/base/info/env.unit.test.ts index b436ad99fec2..6b82a3292adf 100644 --- a/src/test/pythonEnvironments/base/info/env.unit.test.ts +++ b/src/test/pythonEnvironments/base/info/env.unit.test.ts @@ -41,26 +41,27 @@ suite('pyenvs info - getEnvDisplayString()', () => { env.display = info.display; return env; } - const tests: [PythonEnvInfo, string][] = [ - [getEnv({}), 'Python'], - [getEnv({ version, arch, name, kind, distro }), "Python 3.8.1 64-bit ('my-env': venv)"], + const tests: [PythonEnvInfo, string, string][] = [ + [getEnv({}), 'Python', 'Python'], + [getEnv({ version, arch, name, kind, distro }), "Python 3.8.1 ('my-env')", "Python 3.8.1 ('my-env': venv)"], // without "suffix" info - [getEnv({ version }), 'Python 3.8.1'], - [getEnv({ arch }), 'Python 64-bit'], - [getEnv({ version, arch }), 'Python 3.8.1 64-bit'], + [getEnv({ version }), 'Python 3.8.1', 'Python 3.8.1'], + [getEnv({ arch }), 'Python 64-bit', 'Python 64-bit'], + [getEnv({ version, arch }), 'Python 3.8.1 64-bit', 'Python 3.8.1 64-bit'], // with "suffix" info - [getEnv({ name }), "Python ('my-env')"], - [getEnv({ kind }), 'Python (venv)'], - [getEnv({ name, kind }), "Python ('my-env': venv)"], + [getEnv({ name }), "Python ('my-env')", "Python ('my-env')"], + [getEnv({ kind }), 'Python', 'Python (venv)'], + [getEnv({ name, kind }), "Python ('my-env')", "Python ('my-env': venv)"], // env.location is ignored. - [getEnv({ location }), 'Python'], - [getEnv({ name, location }), "Python ('my-env')"], + [getEnv({ location }), 'Python', 'Python'], + [getEnv({ name, location }), "Python ('my-env')", "Python ('my-env')"], ]; - tests.forEach(([env, expected]) => { - test(`"${expected}"`, () => { + tests.forEach(([env, expectedDisplay, expectedDetailedDisplay]) => { + test(`"${expectedDisplay}"`, () => { setEnvDisplayString(env); - assert.equal(env.display, expected); + assert.equal(env.display, expectedDisplay); + assert.equal(env.detailedDisplayName, expectedDetailedDisplay); }); }); }); From f3fe240c97ce48f0056dac4da4360687371431c2 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sat, 8 Jan 2022 18:11:14 +0530 Subject: [PATCH 09/24] Fix status bar display --- src/test/interpreters/display.unit.test.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/test/interpreters/display.unit.test.ts b/src/test/interpreters/display.unit.test.ts index 9ee927a7262c..9561aad47836 100644 --- a/src/test/interpreters/display.unit.test.ts +++ b/src/test/interpreters/display.unit.test.ts @@ -36,7 +36,7 @@ import { EnvironmentType, PythonEnvironment } from '../../client/pythonEnvironme const info: PythonEnvironment = { architecture: Architecture.Unknown, companyDisplayName: '', - displayName: '', + detailedDisplayName: '', envName: '', path: '', envType: EnvironmentType.Unknown, @@ -171,7 +171,7 @@ suite('Interpreters Display', () => { const workspaceFolder = Uri.file('workspace'); const activeInterpreter: PythonEnvironment = { ...info, - displayName: 'Dummy_Display_Name', + detailedDisplayName: 'Dummy_Display_Name', envType: EnvironmentType.Unknown, path: path.join('user', 'development', 'env', 'bin', 'python'), }; @@ -187,7 +187,7 @@ suite('Interpreters Display', () => { if (inExperiment === InterpreterStatusBarPosition.Unpinned) { languageStatusItem.verify( - (s) => (s.text = TypeMoq.It.isValue(activeInterpreter.displayName)!), + (s) => (s.text = TypeMoq.It.isValue(activeInterpreter.detailedDisplayName)!), TypeMoq.Times.once(), ); languageStatusItem.verify( @@ -196,7 +196,7 @@ suite('Interpreters Display', () => { ); } else { statusBar.verify( - (s) => (s.text = TypeMoq.It.isValue(activeInterpreter.displayName)!), + (s) => (s.text = TypeMoq.It.isValue(activeInterpreter.detailedDisplayName)!), TypeMoq.Times.once(), ); statusBar.verify( @@ -210,7 +210,7 @@ suite('Interpreters Display', () => { const workspaceFolder = Uri.file('workspace'); const activeInterpreter: PythonEnvironment = { ...info, - displayName: 'Dummy_Display_Name', + detailedDisplayName: 'Dummy_Display_Name', envType: EnvironmentType.Unknown, path: path.join('user', 'development', 'env', 'bin', 'python'), }; @@ -237,7 +237,7 @@ suite('Interpreters Display', () => { setupWorkspaceFolder(resource, workspaceFolder); const pythonInterpreter: PythonEnvironment = ({ - displayName, + detailedDisplayName: displayName, path: pythonPath, } as any) as PythonEnvironment; interpreterService @@ -299,7 +299,7 @@ suite('Interpreters Display', () => { const pythonPath = path.join('user', 'development', 'env', 'bin', 'python'); const activeInterpreter: PythonEnvironment = { ...info, - displayName: 'Dummy_Display_Name', + detailedDisplayName: 'Dummy_Display_Name', envType: EnvironmentType.Unknown, companyDisplayName: 'Company Name', path: pythonPath, @@ -322,7 +322,7 @@ suite('Interpreters Display', () => { interpreterService.verifyAll(); if (inExperiment === InterpreterStatusBarPosition.Unpinned) { languageStatusItem.verify( - (s) => (s.text = TypeMoq.It.isValue(activeInterpreter.displayName)!), + (s) => (s.text = TypeMoq.It.isValue(activeInterpreter.detailedDisplayName)!), TypeMoq.Times.once(), ); languageStatusItem.verify( @@ -331,7 +331,7 @@ suite('Interpreters Display', () => { ); } else { statusBar.verify( - (s) => (s.text = TypeMoq.It.isValue(activeInterpreter.displayName)!), + (s) => (s.text = TypeMoq.It.isValue(activeInterpreter.detailedDisplayName)!), TypeMoq.Times.once(), ); statusBar.verify((s) => (s.tooltip = TypeMoq.It.isValue(pythonPath)!), TypeMoq.Times.atLeastOnce()); @@ -348,7 +348,7 @@ suite('Interpreters Display', () => { const workspaceFolder = Uri.file('workspace'); const activeInterpreter: PythonEnvironment = { ...info, - displayName: 'Dummy_Display_Name', + detailedDisplayName: 'Dummy_Display_Name', envType: EnvironmentType.Unknown, path: path.join('user', 'development', 'env', 'bin', 'python'), }; From cb937444d4d10672df31c0475744dd018ce16707 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sat, 8 Jan 2022 18:43:27 +0530 Subject: [PATCH 10/24] Fix autoselection tests --- src/client/interpreter/autoSelection/index.ts | 2 +- .../interpreter/configuration/environmentTypeComparer.ts | 5 +++-- .../configuration/interpreterSelector/interpreterSelector.ts | 2 +- src/client/interpreter/configuration/types.ts | 2 +- src/test/interpreters/autoSelection/index.unit.test.ts | 5 +++++ 5 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/client/interpreter/autoSelection/index.ts b/src/client/interpreter/autoSelection/index.ts index 3a23422e53fd..3117ad1826fb 100644 --- a/src/client/interpreter/autoSelection/index.ts +++ b/src/client/interpreter/autoSelection/index.ts @@ -203,7 +203,7 @@ export class InterpreterAutoSelectionService implements IInterpreterAutoSelectio const interpreters = await this.interpreterService.getAllInterpreters(resource); const workspaceUri = this.interpreterHelper.getActiveWorkspaceUri(resource); - const recommendedInterpreter = this.envTypeComparer.getRecommended(interpreters); + const recommendedInterpreter = this.envTypeComparer.getRecommended(interpreters, workspaceUri?.folderUri); if (!recommendedInterpreter) { return; } diff --git a/src/client/interpreter/configuration/environmentTypeComparer.ts b/src/client/interpreter/configuration/environmentTypeComparer.ts index 9adf7d9960ba..3cfa08723a07 100644 --- a/src/client/interpreter/configuration/environmentTypeComparer.ts +++ b/src/client/interpreter/configuration/environmentTypeComparer.ts @@ -83,12 +83,13 @@ export class EnvironmentTypeComparer implements IInterpreterComparer { return nameA > nameB ? 1 : -1; } - public getRecommended(interpreters: PythonEnvironment[], workspaceUri?: Resource): PythonEnvironment | undefined { + public getRecommended(interpreters: PythonEnvironment[], resource: Resource): PythonEnvironment | undefined { // When recommending an intepreter for a workspace, we either want to return a local one // or fallback on a globally-installed interpreter, and we don't want want to suggest a global environment // because we would have to add a way to match environments to a workspace. + const workspaceUri = this.interpreterHelper.getActiveWorkspaceUri(resource); const filteredInterpreters = interpreters.filter((i) => { - if (getEnvLocationHeuristic(i, workspaceUri?.fsPath || '') === EnvLocationHeuristic.Local) { + if (getEnvLocationHeuristic(i, workspaceUri?.folderUri.fsPath || '') === EnvLocationHeuristic.Local) { return true; } if (virtualEnvTypes.includes(i.envType)) { diff --git a/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts b/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts index 6496286406b5..3d25eb59da79 100644 --- a/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts +++ b/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts @@ -59,7 +59,7 @@ export class InterpreterSelector implements IInterpreterSelector { resource: Resource, ): IInterpreterQuickPickItem | undefined { const envs = this.interpreterManager.getInterpreters(resource); - const recommendedEnv = this.envTypeComparer.getRecommended(envs); + const recommendedEnv = this.envTypeComparer.getRecommended(envs, resource); if (!recommendedEnv) { return undefined; } diff --git a/src/client/interpreter/configuration/types.ts b/src/client/interpreter/configuration/types.ts index 2cec501174cb..72c5856361fa 100644 --- a/src/client/interpreter/configuration/types.ts +++ b/src/client/interpreter/configuration/types.ts @@ -60,5 +60,5 @@ export interface ISpecialQuickPickItem { export const IInterpreterComparer = Symbol('IInterpreterComparer'); export interface IInterpreterComparer { compare(a: PythonEnvironment, b: PythonEnvironment): number; - getRecommended(interpreters: PythonEnvironment[], workspaceUri?: Resource): PythonEnvironment | undefined; + getRecommended(interpreters: PythonEnvironment[], resource: Resource): PythonEnvironment | undefined; } diff --git a/src/test/interpreters/autoSelection/index.unit.test.ts b/src/test/interpreters/autoSelection/index.unit.test.ts index 2f4c6eabd575..c9636ec176db 100644 --- a/src/test/interpreters/autoSelection/index.unit.test.ts +++ b/src/test/interpreters/autoSelection/index.unit.test.ts @@ -88,6 +88,11 @@ suite('Interpreters - Auto Selection', () => { envPath: path.join('some', 'pipenv', 'env'), version: { major: 3, minor: 10, patch: 0 }, } as PythonEnvironment, + { + envType: EnvironmentType.Pyenv, + envPath: path.join('some', 'pipenv', 'env'), + version: { major: 3, minor: 5, patch: 0 }, + } as PythonEnvironment, ]), ); From 815ea70849bd4746198eba20a1edd214338459a5 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sat, 8 Jan 2022 20:09:19 +0530 Subject: [PATCH 11/24] Fix interpreter selector tests --- .../interpreterSelector/interpreterSelector.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts b/src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts index 851d38492cb0..65ab41c1751c 100644 --- a/src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts +++ b/src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts @@ -169,11 +169,11 @@ suite('Interpreters - selector', () => { const expected: InterpreterQuickPickItem[] = [ new InterpreterQuickPickItem('two', path.join(workspacePath, '.venv', 'bin', 'python')), + new InterpreterQuickPickItem('four', path.join('a', 'conda', 'environment')), new InterpreterQuickPickItem( 'one', path.join('path', 'to', 'another', 'workspace', '.venv', 'bin', 'python'), ), - new InterpreterQuickPickItem('four', path.join('a', 'conda', 'environment')), new InterpreterQuickPickItem('three', path.join('a', 'global', 'env', 'python')), ]; From 676fbc8a70310105b4df714c74b0a95d961d46b5 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sat, 8 Jan 2022 20:28:06 +0530 Subject: [PATCH 12/24] Fix sorting/do not prioritize conda envs --- .../configuration/environmentTypeComparer.ts | 25 ++++++++----------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/client/interpreter/configuration/environmentTypeComparer.ts b/src/client/interpreter/configuration/environmentTypeComparer.ts index 3cfa08723a07..cfb8efad23c6 100644 --- a/src/client/interpreter/configuration/environmentTypeComparer.ts +++ b/src/client/interpreter/configuration/environmentTypeComparer.ts @@ -59,20 +59,15 @@ export class EnvironmentTypeComparer implements IInterpreterComparer { return versionComparison; } - // Prioritize non-Conda environments. - if (isCondaEnvironment(a) && !isCondaEnvironment(b)) { + // If we have the "base" Conda env, put it last in its Python version subgroup. + if (isBaseCondaEnvironment(a)) { return 1; } - if (!isCondaEnvironment(a) && isCondaEnvironment(b)) { + if (isBaseCondaEnvironment(b)) { return -1; } - // If we have the "base" Conda env, put it last in its Python version subgroup. - if (isBaseCondaEnvironment(a)) { - return 1; - } - // Check alphabetical order. const nameA = getSortName(a, this.interpreterHelper); const nameB = getSortName(b, this.interpreterHelper); @@ -137,12 +132,11 @@ function getSortName(info: PythonEnvironment, interpreterHelper: IInterpreterHel return `${sortNameParts.join(' ')} ${envSuffix}`.trim(); } -function isCondaEnvironment(environment: PythonEnvironment): boolean { - return environment.envType === EnvironmentType.Conda; -} - function isBaseCondaEnvironment(environment: PythonEnvironment): boolean { - return isCondaEnvironment(environment) && (environment.envName === 'base' || environment.envName === 'miniconda'); + return ( + environment.envType === EnvironmentType.Conda && + (environment.envName === 'base' || environment.envName === 'miniconda') + ); } /** @@ -203,17 +197,18 @@ export function getEnvLocationHeuristic(environment: PythonEnvironment, workspac */ function compareEnvironmentType(a: PythonEnvironment, b: PythonEnvironment): number { const envTypeByPriority = getPrioritizedEnvironmentType(); - return envTypeByPriority.indexOf(a.envType) - envTypeByPriority.indexOf(b.envType); + return Math.sign(envTypeByPriority.indexOf(a.envType) - envTypeByPriority.indexOf(b.envType)); } function getPrioritizedEnvironmentType(): EnvironmentType[] { return [ - EnvironmentType.Conda, + // Prioritize non-Conda environments. EnvironmentType.Poetry, EnvironmentType.Pipenv, EnvironmentType.VirtualEnvWrapper, EnvironmentType.Venv, EnvironmentType.VirtualEnv, + EnvironmentType.Conda, EnvironmentType.Pyenv, EnvironmentType.WindowsStore, EnvironmentType.Global, From 42aba8bce67c1ff18b83de0bd5ce78cdec090205 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sun, 9 Jan 2022 01:56:33 +0530 Subject: [PATCH 13/24] Recommend Pipenv & Poetry envs --- .../interpreter/configuration/environmentTypeComparer.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/client/interpreter/configuration/environmentTypeComparer.ts b/src/client/interpreter/configuration/environmentTypeComparer.ts index cfb8efad23c6..2a62663740cc 100644 --- a/src/client/interpreter/configuration/environmentTypeComparer.ts +++ b/src/client/interpreter/configuration/environmentTypeComparer.ts @@ -87,7 +87,9 @@ export class EnvironmentTypeComparer implements IInterpreterComparer { if (getEnvLocationHeuristic(i, workspaceUri?.folderUri.fsPath || '') === EnvLocationHeuristic.Local) { return true; } - if (virtualEnvTypes.includes(i.envType)) { + const virtualEnvTypesRelatedToWorkspace = [EnvironmentType.Pipenv, EnvironmentType.Poetry]; + if (virtualEnvTypes.includes(i.envType) && !virtualEnvTypesRelatedToWorkspace.includes(i.envType)) { + // We're not sure if these envs were created for the workspace, so do not recommend them. return false; } return true; From 6d9a5cdd32c429d4d547ef793a3de5e877ea4051 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 10 Jan 2022 21:00:45 +0530 Subject: [PATCH 14/24] Do not autoselect poetry/pipenv envs --- .../interpreter/configuration/environmentTypeComparer.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/client/interpreter/configuration/environmentTypeComparer.ts b/src/client/interpreter/configuration/environmentTypeComparer.ts index 2a62663740cc..03b536e8a42c 100644 --- a/src/client/interpreter/configuration/environmentTypeComparer.ts +++ b/src/client/interpreter/configuration/environmentTypeComparer.ts @@ -87,8 +87,7 @@ export class EnvironmentTypeComparer implements IInterpreterComparer { if (getEnvLocationHeuristic(i, workspaceUri?.folderUri.fsPath || '') === EnvLocationHeuristic.Local) { return true; } - const virtualEnvTypesRelatedToWorkspace = [EnvironmentType.Pipenv, EnvironmentType.Poetry]; - if (virtualEnvTypes.includes(i.envType) && !virtualEnvTypesRelatedToWorkspace.includes(i.envType)) { + if (virtualEnvTypes.includes(i.envType)) { // We're not sure if these envs were created for the workspace, so do not recommend them. return false; } From 732466f6004d423a96f15b646242a989ea1d0845 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 10 Jan 2022 21:15:20 +0530 Subject: [PATCH 15/24] Fix interpreter selector tests --- .../interpreterSelector/interpreterSelector.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts b/src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts index 65ab41c1751c..851d38492cb0 100644 --- a/src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts +++ b/src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts @@ -169,11 +169,11 @@ suite('Interpreters - selector', () => { const expected: InterpreterQuickPickItem[] = [ new InterpreterQuickPickItem('two', path.join(workspacePath, '.venv', 'bin', 'python')), - new InterpreterQuickPickItem('four', path.join('a', 'conda', 'environment')), new InterpreterQuickPickItem( 'one', path.join('path', 'to', 'another', 'workspace', '.venv', 'bin', 'python'), ), + new InterpreterQuickPickItem('four', path.join('a', 'conda', 'environment')), new InterpreterQuickPickItem('three', path.join('a', 'global', 'env', 'python')), ]; From 4ea9c3f36c85ae9c8e365a326e384988f46c8541 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 11 Jan 2022 01:52:21 +0530 Subject: [PATCH 16/24] Fix set interpreter tests --- .../commands/setInterpreter.ts | 2 +- .../activation/partialModeStatus.unit.test.ts | 5 +- .../commands/setInterpreter.unit.test.ts | 68 ++++++++++++++----- src/test/mocks/vsc/index.ts | 5 ++ src/test/vscode-mock.ts | 1 + 5 files changed, 62 insertions(+), 19 deletions(-) diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index 637d71db5074..a2369d5b0ac8 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -274,7 +274,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { } private setRecommendedItem(items: QuickPickType[], resource: Resource) { - const interpreterSuggestions = this.interpreterSelector.getSuggestions(resource); + const interpreterSuggestions = this.interpreterSelector.getSuggestions(resource, false); if (!this.interpreterService.refreshPromise && interpreterSuggestions.length > 0) { const suggestion = this.interpreterSelector.getRecommendedSuggestion( interpreterSuggestions, diff --git a/src/test/activation/partialModeStatus.unit.test.ts b/src/test/activation/partialModeStatus.unit.test.ts index 7f1f0ada49c7..28f134379c87 100644 --- a/src/test/activation/partialModeStatus.unit.test.ts +++ b/src/test/activation/partialModeStatus.unit.test.ts @@ -44,11 +44,14 @@ suite('Partial Mode Status', async () => { parse: (s: string) => s, }, } as unknown) as typeof vscodeTypes; - rewiremock.disable(); rewiremock.enable(); rewiremock('vscode').with(vscodeMock); }); + teardown(() => { + rewiremock.disable(); + }); + test("No item is created if workspace is trusted and isn't virtual", async () => { workspaceService.setup((w) => w.isTrusted).returns(() => true); workspaceService.setup((w) => w.isVirtualWorkspace).returns(() => false); diff --git a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts index dd3f1454baa8..36a8bca981a5 100644 --- a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts +++ b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts @@ -6,7 +6,7 @@ import { expect } from 'chai'; import * as path from 'path'; import * as sinon from 'sinon'; import * as TypeMoq from 'typemoq'; -import { ConfigurationTarget, OpenDialogOptions, QuickPick, QuickPickItem, Uri } from 'vscode'; +import { ConfigurationTarget, OpenDialogOptions, QuickPick, QuickPickItem, QuickPickItemKind, Uri } from 'vscode'; import { cloneDeep } from 'lodash'; import { instance, mock, verify, when } from 'ts-mockito'; import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../../../client/common/application/types'; @@ -21,6 +21,7 @@ import { IQuickPickParameters, } from '../../../../client/common/utils/multiStepInput'; import { + EnvGroups, InterpreterStateArgs, SetInterpreterCommand, } from '../../../../client/interpreter/configuration/interpreterSelector/commands/setInterpreter'; @@ -29,7 +30,7 @@ import { IInterpreterSelector, IPythonPathUpdaterServiceManager, } from '../../../../client/interpreter/configuration/types'; -import { PythonEnvironment } from '../../../../client/pythonEnvironments/info'; +import { EnvironmentType, PythonEnvironment } from '../../../../client/pythonEnvironments/info'; import { EventName } from '../../../../client/telemetry/constants'; import * as Telemetry from '../../../../client/telemetry'; import { MockWorkspaceConfiguration } from '../../../mocks/mockWorkspaceConfig'; @@ -100,11 +101,11 @@ suite('Set Interpreter Command', () => { const interpreterPath = 'path/to/interpreter'; const item: IInterpreterQuickPickItem = { - description: '', + description: interpreterPath, detail: '', label: 'This is the selected Python path', path: interpreterPath, - interpreter: { path: interpreterPath } as PythonEnvironment, + interpreter: { path: interpreterPath, envType: EnvironmentType.Conda } as PythonEnvironment, }; const defaultInterpreterPath = 'defaultInterpreterPath'; const defaultInterpreterPathSuggestion = { @@ -115,11 +116,11 @@ suite('Set Interpreter Command', () => { }; const refreshedItem: IInterpreterQuickPickItem = { - description: '', + description: interpreterPath, detail: '', label: 'Refreshed path', path: interpreterPath, - interpreter: { path: interpreterPath } as PythonEnvironment, + interpreter: { path: interpreterPath, envType: EnvironmentType.Conda } as PythonEnvironment, }; const expectedEnterInterpreterPathSuggestion = { label: `${Octicons.Add} ${InterpreterQuickPickList.enterPath.label()}`, @@ -141,7 +142,13 @@ suite('Set Interpreter Command', () => { properties, }; }); - interpreterSelector.setup((i) => i.getSuggestions(TypeMoq.It.isAny())).returns(() => [item]); + interpreterSelector + .setup((i) => i.getSuggestions(TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns(() => [item]); + interpreterSelector + .setup((i) => i.getRecommendedSuggestion(TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns(() => item); + pythonSettings.setup((p) => p.pythonPath).returns(() => currentPythonPath); pythonSettings.setup((p) => p.defaultInterpreterPath).returns(() => defaultInterpreterPath); @@ -190,8 +197,14 @@ suite('Set Interpreter Command', () => { const multiStepInput = TypeMoq.Mock.ofType>(); const recommended = cloneDeep(item); recommended.label = `${Octicons.Star} ${item.label}`; - recommended.description = Common.recommended(); - const suggestions = [expectedEnterInterpreterPathSuggestion, defaultInterpreterPathSuggestion, recommended]; + recommended.description = `${interpreterPath} - ${Common.recommended()}`; + const separator = { label: EnvGroups.Conda, kind: QuickPickItemKind.Separator }; + const suggestions = [ + expectedEnterInterpreterPathSuggestion, + defaultInterpreterPathSuggestion, + separator, + recommended, + ]; const expectedParameters: IQuickPickParameters = { placeholder: InterpreterQuickPickList.quickPickListPlaceholder().format(currentPythonPath), items: suggestions, @@ -264,7 +277,8 @@ suite('Set Interpreter Command', () => { const multiStepInput = TypeMoq.Mock.ofType>(); const recommended = cloneDeep(item); recommended.label = `${Octicons.Star} ${item.label}`; - recommended.description = Common.recommended(); + recommended.description = `${interpreterPath} - ${Common.recommended()}`; + const separator = { label: EnvGroups.Conda, kind: QuickPickItemKind.Separator }; const defaultPathSuggestion = { label: `${Octicons.Gear} ${InterpreterQuickPickList.defaultInterpreterPath.label()}`, @@ -273,7 +287,7 @@ suite('Set Interpreter Command', () => { alwaysShow: true, }; - const suggestions = [expectedEnterInterpreterPathSuggestion, defaultPathSuggestion, recommended]; + const suggestions = [expectedEnterInterpreterPathSuggestion, defaultPathSuggestion, separator, recommended]; const expectedParameters: IQuickPickParameters = { placeholder: InterpreterQuickPickList.quickPickListPlaceholder().format(currentPythonPath), items: suggestions, @@ -347,13 +361,14 @@ suite('Set Interpreter Command', () => { expect(onChangedCallback).to.not.equal(undefined, 'Callback not set'); multiStepInput.verifyAll(); + const separator = { label: EnvGroups.Conda, kind: QuickPickItemKind.Separator }; const quickPick = { - items: [expectedEnterInterpreterPathSuggestion, defaultInterpreterPathSuggestion, item], + items: [expectedEnterInterpreterPathSuggestion, defaultInterpreterPathSuggestion, separator, item], activeItems: [item], busy: false, }; interpreterSelector - .setup((i) => i.suggestionToQuickPickItem(TypeMoq.It.isAny(), undefined)) + .setup((i) => i.suggestionToQuickPickItem(TypeMoq.It.isAny(), undefined, false)) .returns(() => refreshedItem); const changeEvent: PythonEnvironmentsChangedEvent = { @@ -365,7 +380,12 @@ suite('Set Interpreter Command', () => { assert.deepStrictEqual( quickPick, { - items: [expectedEnterInterpreterPathSuggestion, defaultInterpreterPathSuggestion, refreshedItem], + items: [ + expectedEnterInterpreterPathSuggestion, + defaultInterpreterPathSuggestion, + separator, + refreshedItem, + ], activeItems: [refreshedItem], busy: true, }, @@ -373,7 +393,16 @@ suite('Set Interpreter Command', () => { ); // Refresh is over; set the final states accordingly - interpreterSelector.setup((i) => i.getSuggestions(TypeMoq.It.isAny())).returns(() => [refreshedItem]); + interpreterSelector.reset(); + interpreterSelector + .setup((i) => i.getSuggestions(TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns(() => [refreshedItem]); + interpreterSelector + .setup((i) => i.getRecommendedSuggestion(TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns(() => refreshedItem); + interpreterSelector + .setup((i) => i.suggestionToQuickPickItem(TypeMoq.It.isAny(), undefined, false)) + .returns(() => refreshedItem); when(interpreterService.refreshPromise).thenReturn(undefined); refreshPromiseDeferred.resolve(); @@ -381,12 +410,17 @@ suite('Set Interpreter Command', () => { const recommended = cloneDeep(refreshedItem); recommended.label = `${Octicons.Star} ${refreshedItem.label}`; - recommended.description = Common.recommended(); + recommended.description = `${interpreterPath} - ${Common.recommended()}`; assert.deepStrictEqual( quickPick, { // Refresh has finished, so recommend an interpreter - items: [expectedEnterInterpreterPathSuggestion, defaultInterpreterPathSuggestion, recommended], + items: [ + expectedEnterInterpreterPathSuggestion, + defaultInterpreterPathSuggestion, + separator, + recommended, + ], activeItems: [recommended], // Refresh has finished, so quickpick busy indicator should go away busy: false, diff --git a/src/test/mocks/vsc/index.ts b/src/test/mocks/vsc/index.ts index bdffedc2ead3..fcef8af923d1 100644 --- a/src/test/mocks/vsc/index.ts +++ b/src/test/mocks/vsc/index.ts @@ -35,6 +35,11 @@ export enum LanguageStatusSeverity { Error = 2, } +export enum QuickPickItemKind { + Separator = -1, + Default = 1, +} + export class Disposable { constructor(private callOnDispose: () => void) {} diff --git a/src/test/vscode-mock.ts b/src/test/vscode-mock.ts index 53a255a184d4..c103f82e5455 100644 --- a/src/test/vscode-mock.ts +++ b/src/test/vscode-mock.ts @@ -107,6 +107,7 @@ mockedVSCode.FileType = vscodeMocks.FileType; mockedVSCode.UIKind = vscodeMocks.UIKind; mockedVSCode.FileSystemError = vscodeMocks.vscMockExtHostedTypes.FileSystemError; mockedVSCode.LanguageStatusSeverity = vscodeMocks.LanguageStatusSeverity; +mockedVSCode.QuickPickItemKind = vscodeMocks.QuickPickItemKind; (mockedVSCode as any).NotebookCellKind = vscodeMocks.vscMockExtHostedTypes.NotebookCellKind; (mockedVSCode as any).CellOutputKind = vscodeMocks.vscMockExtHostedTypes.CellOutputKind; (mockedVSCode as any).NotebookCellRunState = vscodeMocks.vscMockExtHostedTypes.NotebookCellRunState; From 1e9078e90680778650725a334d8871aea74b4fef Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 11 Jan 2022 01:57:51 +0530 Subject: [PATCH 17/24] News --- news/1 Enhancements/17944.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/1 Enhancements/17944.md diff --git a/news/1 Enhancements/17944.md b/news/1 Enhancements/17944.md new file mode 100644 index 000000000000..e875616b3ecd --- /dev/null +++ b/news/1 Enhancements/17944.md @@ -0,0 +1 @@ +Group interpreters in interpreter quick picker using separators. From af473cd7ee4d4a8875b17ec2c0ea1f3d533204d8 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 11 Jan 2022 13:56:20 +0530 Subject: [PATCH 18/24] Add test for grouping --- .../commands/setInterpreter.ts | 1 - .../commands/setInterpreter.unit.test.ts | 117 +++++++++++++++++- 2 files changed, 116 insertions(+), 2 deletions(-) diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index a2369d5b0ac8..ea961f592599 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -404,7 +404,6 @@ export enum EnvGroups { Venv = 'Venv', Poetry = 'Poetry', VirtualEnvWrapper = 'VirtualEnvWrapper', - NewlyDiscovered = 'NewlyDiscovered', } function getGroupedQuickPickItems(items: IInterpreterQuickPickItem[], workspacePath?: string): QuickPickType[] { diff --git a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts index 36a8bca981a5..45fcae39d1bf 100644 --- a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts +++ b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts @@ -6,7 +6,15 @@ import { expect } from 'chai'; import * as path from 'path'; import * as sinon from 'sinon'; import * as TypeMoq from 'typemoq'; -import { ConfigurationTarget, OpenDialogOptions, QuickPick, QuickPickItem, QuickPickItemKind, Uri } from 'vscode'; +import { + ConfigurationTarget, + OpenDialogOptions, + QuickPick, + QuickPickItem, + QuickPickItemKind, + Uri, + WorkspaceFolder, +} from 'vscode'; import { cloneDeep } from 'lodash'; import { instance, mock, verify, when } from 'ts-mockito'; import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../../../client/common/application/types'; @@ -127,6 +135,7 @@ suite('Set Interpreter Command', () => { alwaysShow: true, }; const currentPythonPath = 'python'; + const workspacePath = 'path/to/workspace'; setup(() => { _enterOrBrowseInterpreterPath = sinon.stub( @@ -161,6 +170,10 @@ suite('Set Interpreter Command', () => { }), ); + workspace + .setup((w) => w.getWorkspaceFolder(TypeMoq.It.isAny())) + .returns(() => (({ uri: { fsPath: workspacePath } } as unknown) as WorkspaceFolder)); + setInterpreterCommand = new SetInterpreterCommand( appShell.object, new PathUtils(false), @@ -233,6 +246,108 @@ suite('Set Interpreter Command', () => { assert.deepStrictEqual(actualParameters, expectedParameters, 'Params not equal'); }); + test('Items displayed should be grouped if no refresh is going on', async () => { + const state: InterpreterStateArgs = { path: 'some path', workspace: undefined }; + const multiStepInput = TypeMoq.Mock.ofType>(); + const interpreterItems: IInterpreterQuickPickItem[] = [ + { + description: `${workspacePath}/interpreterPath1`, + detail: '', + label: 'This is the selected Python path', + path: `${workspacePath}/interpreterPath1`, + interpreter: { + path: `${workspacePath}/interpreterPath1`, + envType: EnvironmentType.Venv, + } as PythonEnvironment, + }, + { + description: 'interpreterPath2', + detail: '', + label: 'This is the selected Python path', + path: 'interpreterPath2', + interpreter: { + path: 'interpreterPath2', + envType: EnvironmentType.VirtualEnvWrapper, + } as PythonEnvironment, + }, + { + description: 'interpreterPath3', + detail: '', + label: 'This is the selected Python path', + path: 'interpreterPath3', + interpreter: { + path: 'interpreterPath3', + envType: EnvironmentType.VirtualEnvWrapper, + } as PythonEnvironment, + }, + { + description: 'interpreterPath4', + detail: '', + label: 'This is the selected Python path', + path: 'interpreterPath4', + interpreter: { path: 'interpreterPath4', envType: EnvironmentType.Conda } as PythonEnvironment, + }, + item, + { + description: 'interpreterPath5', + detail: '', + label: 'This is the selected Python path', + path: 'interpreterPath5', + interpreter: { path: 'interpreterPath5', envType: EnvironmentType.Global } as PythonEnvironment, + }, + ]; + interpreterSelector.reset(); + interpreterSelector + .setup((i) => i.getSuggestions(TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns(() => interpreterItems); + interpreterSelector + .setup((i) => i.getRecommendedSuggestion(TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns(() => item); + const recommended = cloneDeep(item); + recommended.label = `${Octicons.Star} ${item.label}`; + recommended.description = `${interpreterPath} - ${Common.recommended()}`; + const suggestions = [ + expectedEnterInterpreterPathSuggestion, + defaultInterpreterPathSuggestion, + { label: EnvGroups.Workspace, kind: QuickPickItemKind.Separator }, + interpreterItems[0], + { label: EnvGroups.VirtualEnvWrapper, kind: QuickPickItemKind.Separator }, + interpreterItems[1], + interpreterItems[2], + { label: EnvGroups.Conda, kind: QuickPickItemKind.Separator }, + interpreterItems[3], + recommended, + { label: EnvGroups.Global, kind: QuickPickItemKind.Separator }, + interpreterItems[5], + ]; + const expectedParameters: IQuickPickParameters = { + placeholder: InterpreterQuickPickList.quickPickListPlaceholder().format(currentPythonPath), + items: suggestions, + activeItem: recommended, + matchOnDetail: true, + matchOnDescription: true, + title: InterpreterQuickPickList.browsePath.openButtonLabel(), + sortByLabel: true, + keepScrollPosition: true, + }; + let actualParameters: IQuickPickParameters | undefined; + multiStepInput + .setup((i) => i.showQuickPick(TypeMoq.It.isAny())) + .callback((options) => { + actualParameters = options; + }) + .returns(() => Promise.resolve((undefined as unknown) as QuickPickItem)); + + await setInterpreterCommand._pickInterpreter(multiStepInput.object, state); + + expect(actualParameters).to.not.equal(undefined, 'Parameters not set'); + const refreshButtonCallback = actualParameters!.customButtonSetup?.callback; + expect(refreshButtonCallback).to.not.equal(undefined, 'Callback not set'); + delete actualParameters!.customButtonSetup; + delete actualParameters!.onChangeItem; + assert.deepStrictEqual(actualParameters?.items, expectedParameters.items, 'Params not equal'); + }); + test('If system variables are used in the default interpreter path, make sure they are resolved when the path is displayed', async () => { // Create a SetInterpreterCommand instance from scratch, and use a different defaultInterpreterPath from the rest of the tests. const workspaceDefaultInterpreterPath = '${workspaceFolder}/defaultInterpreterPath'; From e58c0f0c98de813b4cb67986baa82a0b65a30275 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 11 Jan 2022 15:34:49 +0530 Subject: [PATCH 19/24] Events to add an environment are handled appropriately if a grouped list is present --- .../commands/setInterpreter.unit.test.ts | 54 ++++++++++++++++++- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts index 45fcae39d1bf..27c94a462cf5 100644 --- a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts +++ b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts @@ -455,7 +455,7 @@ suite('Set Interpreter Command', () => { verify(interpreterService.triggerRefresh()).once(); }); - test('If an event to update quickpick is received, the quickpick is updated accordingly', async () => { + test('Events to update quickpick updates the quickpick accordingly', async () => { const state: InterpreterStateArgs = { path: 'some path', workspace: undefined }; const multiStepInput = TypeMoq.Mock.ofType>(); let actualParameters: IQuickPickParameters | undefined; @@ -516,7 +516,9 @@ suite('Set Interpreter Command', () => { .setup((i) => i.getRecommendedSuggestion(TypeMoq.It.isAny(), TypeMoq.It.isAny())) .returns(() => refreshedItem); interpreterSelector - .setup((i) => i.suggestionToQuickPickItem(TypeMoq.It.isAny(), undefined, false)) + .setup((i) => + i.suggestionToQuickPickItem(TypeMoq.It.isValue(refreshedItem.interpreter), undefined, false), + ) .returns(() => refreshedItem); when(interpreterService.refreshPromise).thenReturn(undefined); @@ -542,6 +544,54 @@ suite('Set Interpreter Command', () => { }, 'Quickpick not updated correctly after refresh has finished', ); + + const newItem = { + description: `${workspacePath}/interpreterPath1`, + detail: '', + label: 'This is the selected Python path', + path: `${workspacePath}/interpreterPath1`, + interpreter: { + path: `${workspacePath}/interpreterPath1`, + envType: EnvironmentType.Venv, + } as PythonEnvironment, + }; + const changeEvent2: PythonEnvironmentsChangedEvent = { + old: undefined, + new: newItem.interpreter, + }; + interpreterSelector.reset(); + interpreterSelector + .setup((i) => i.getSuggestions(TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns(() => [refreshedItem, newItem]); + interpreterSelector + .setup((i) => i.getRecommendedSuggestion(TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns(() => refreshedItem); + interpreterSelector + .setup((i) => + i.suggestionToQuickPickItem(TypeMoq.It.isValue(refreshedItem.interpreter), undefined, false), + ) + .returns(() => refreshedItem); + interpreterSelector + .setup((i) => i.suggestionToQuickPickItem(TypeMoq.It.isValue(newItem.interpreter), undefined, false)) + .returns(() => newItem); + await onChangedCallback!(changeEvent2, (quickPick as unknown) as QuickPick); // Invoke callback, meaning that the items are supposed to change. + + assert.deepStrictEqual( + quickPick, + { + items: [ + expectedEnterInterpreterPathSuggestion, + defaultInterpreterPathSuggestion, + separator, + recommended, + { label: EnvGroups.Workspace, kind: QuickPickItemKind.Separator }, + newItem, + ], + activeItems: [recommended], + busy: false, + }, + 'Quickpick not updated correctly', + ); }); test('If an item is selected, update state and return', async () => { From 0b56fb635db3085f6862b379e53281dde169813d Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 12 Jan 2022 14:20:55 +0530 Subject: [PATCH 20/24] Code reviews --- .../interpreterSelector/commands/setInterpreter.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index ea961f592599..0fcea6222ccb 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -425,9 +425,7 @@ function addSeparatorIfApplicable( if (!previousGroup) { const lastItem = items.length ? items[items.length - 1] : undefined; previousGroup = - items.length && lastItem && isInterpreterQuickPickItem(lastItem) - ? getGroup(lastItem, workspacePath) - : undefined; + lastItem && isInterpreterQuickPickItem(lastItem) ? getGroup(lastItem, workspacePath) : undefined; } const currentGroup = getGroup(newItem, workspacePath); if (!previousGroup || currentGroup !== previousGroup) { From d73bdd09fde2fa953a74362db94104ee2100a1a6 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 13 Jan 2022 20:59:55 +0530 Subject: [PATCH 21/24] Add a new group for recommended interpreter --- .../commands/setInterpreter.ts | 55 +++++++++++++------ 1 file changed, 37 insertions(+), 18 deletions(-) diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index 0fcea6222ccb..c7bac6bcd19a 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -51,6 +51,18 @@ function isSeparatorItem(item: QuickPickType): item is QuickPickItem { return 'kind' in item && item.kind === QuickPickItemKind.Separator; } +export enum EnvGroups { + Workspace = 'Workspace', + Conda = 'Conda', + Global = 'Global', + VirtualEnv = 'VirtualEnv', + PipEnv = 'PipEnv', + Pyenv = 'Pyenv', + Venv = 'Venv', + Poetry = 'Poetry', + VirtualEnvWrapper = 'VirtualEnvWrapper', + Recommended = 'Recommended', +} @injectable() export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { private readonly manualEntrySuggestion: ISpecialQuickPickItem = { @@ -167,7 +179,15 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { // We cannot put items in groups while the list is loading as group of an item can change. return items; } - return getGroupedQuickPickItems(items, workspaceFolder?.uri.fsPath); + const itemsWithFullName = this.interpreterSelector.getSuggestions(resource, true); + const recommended = this.interpreterSelector.getRecommendedSuggestion( + itemsWithFullName, + this.workspaceService.getWorkspaceFolder(resource)?.uri, + ); + if (recommended && arePathsSame(items[0].interpreter.path, recommended.interpreter.path)) { + items.shift(); + } + return getGroupedQuickPickItems(items, recommended, workspaceFolder?.uri.fsPath); } private getActiveItem(resource: Resource, suggestions: QuickPickType[]) { @@ -192,7 +212,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { if (defaultInterpreterPathValue && defaultInterpreterPathValue !== 'python') { return { label: `${Octicons.Gear} ${InterpreterQuickPickList.defaultInterpreterPath.label()}`, - detail: this.pathUtils.getDisplayName( + description: this.pathUtils.getDisplayName( defaultInterpreterPathValue, resource ? resource.fsPath : undefined, ), @@ -274,7 +294,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { } private setRecommendedItem(items: QuickPickType[], resource: Resource) { - const interpreterSuggestions = this.interpreterSelector.getSuggestions(resource, false); + const interpreterSuggestions = this.interpreterSelector.getSuggestions(resource, true); if (!this.interpreterService.refreshPromise && interpreterSuggestions.length > 0) { const suggestion = this.interpreterSelector.getRecommendedSuggestion( interpreterSuggestions, @@ -283,9 +303,13 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { if (!suggestion) { return; } + const areItemsGrouped = items.find((item) => isSeparatorItem(item) && item.label === EnvGroups.Recommended); const recommended = cloneDeep(suggestion); recommended.label = `${Octicons.Star} ${recommended.label}`; - recommended.description = `${recommended.description ?? ''} - ${Common.recommended()}`; + recommended.description = areItemsGrouped + ? // No need to add a tag as "Recommended" group already exists. + recommended.description + : `${recommended.description ?? ''} - ${Common.recommended()}`; const index = items.findIndex( (item) => isInterpreterQuickPickItem(item) && @@ -394,21 +418,16 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { } } -export enum EnvGroups { - Workspace = 'Workspace', - Conda = 'Conda', - Global = 'Global', - VirtualEnv = 'VirtualEnv', - PipEnv = 'PipEnv', - Pyenv = 'Pyenv', - Venv = 'Venv', - Poetry = 'Poetry', - VirtualEnvWrapper = 'VirtualEnvWrapper', -} - -function getGroupedQuickPickItems(items: IInterpreterQuickPickItem[], workspacePath?: string): QuickPickType[] { +function getGroupedQuickPickItems( + items: IInterpreterQuickPickItem[], + recommended: IInterpreterQuickPickItem | undefined, + workspacePath?: string, +): QuickPickType[] { const updatedItems: QuickPickType[] = []; - let previousGroup: EnvGroups | undefined; + if (recommended) { + updatedItems.push({ label: EnvGroups.Recommended, kind: QuickPickItemKind.Separator }, recommended); + } + let previousGroup: EnvGroups | undefined = EnvGroups.Recommended; for (const item of items) { previousGroup = addSeparatorIfApplicable(updatedItems, item, workspacePath, previousGroup); updatedItems.push(item); From 9d2fb279466910e54973d3c0a56cee8269245942 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 13 Jan 2022 22:07:29 +0530 Subject: [PATCH 22/24] Localize group names --- package.nls.json | 2 ++ src/client/common/utils/localize.ts | 2 ++ .../commands/setInterpreter.ts | 27 ++++++++++--------- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/package.nls.json b/package.nls.json index ab5baa1a454e..12071875424b 100644 --- a/package.nls.json +++ b/package.nls.json @@ -53,6 +53,8 @@ "Interpreters.DiscoveringInterpreters": "Discovering Python Interpreters", "Interpreters.condaInheritEnvMessage": "We noticed you're using a conda environment. If you are experiencing issues with this environment in the integrated terminal, we recommend that you let the Python extension change \"terminal.integrated.inheritEnv\" to false in your user settings.", "Logging.CurrentWorkingDirectory": "cwd:", + "InterpreterQuickPickList.workspaceGroupName": "Workspace", + "InterpreterQuickPickList.globalGroupName": "Global", "InterpreterQuickPickList.quickPickListPlaceholder": "Current: {0}", "InterpreterQuickPickList.enterPath.label": "Enter interpreter path...", "InterpreterQuickPickList.enterPath.placeholder": "Enter path to a Python interpreter.", diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 9030d256cc74..14d32d7c0426 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -292,6 +292,8 @@ export namespace Interpreters { } export namespace InterpreterQuickPickList { + export const globalGroupName = localize('InterpreterQuickPickList.globalGroupName', 'Global'); + export const workspaceGroupName = localize('InterpreterQuickPickList.workspaceGroupName', 'Workspace'); export const quickPickListPlaceholder = localize( 'InterpreterQuickPickList.quickPickListPlaceholder', 'Current: {0}', diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index c7bac6bcd19a..c853c62cd62b 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -51,17 +51,18 @@ function isSeparatorItem(item: QuickPickType): item is QuickPickItem { return 'kind' in item && item.kind === QuickPickItemKind.Separator; } -export enum EnvGroups { - Workspace = 'Workspace', - Conda = 'Conda', - Global = 'Global', - VirtualEnv = 'VirtualEnv', - PipEnv = 'PipEnv', - Pyenv = 'Pyenv', - Venv = 'Venv', - Poetry = 'Poetry', - VirtualEnvWrapper = 'VirtualEnvWrapper', - Recommended = 'Recommended', +// eslint-disable-next-line @typescript-eslint/no-namespace +export namespace EnvGroups { + export const Workspace = InterpreterQuickPickList.workspaceGroupName(); + export const Conda = 'Conda'; + export const Global = InterpreterQuickPickList.globalGroupName(); + export const VirtualEnv = 'VirtualEnv'; + export const PipEnv = 'PipEnv'; + export const Pyenv = 'Pyenv'; + export const Venv = 'Venv'; + export const Poetry = 'Poetry'; + export const VirtualEnvWrapper = 'VirtualEnvWrapper'; + export const Recommended = Common.recommended(); } @injectable() export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { @@ -427,7 +428,7 @@ function getGroupedQuickPickItems( if (recommended) { updatedItems.push({ label: EnvGroups.Recommended, kind: QuickPickItemKind.Separator }, recommended); } - let previousGroup: EnvGroups | undefined = EnvGroups.Recommended; + let previousGroup = EnvGroups.Recommended; for (const item of items) { previousGroup = addSeparatorIfApplicable(updatedItems, item, workspacePath, previousGroup); updatedItems.push(item); @@ -439,7 +440,7 @@ function addSeparatorIfApplicable( items: QuickPickType[], newItem: IInterpreterQuickPickItem, workspacePath?: string, - previousGroup?: EnvGroups | undefined, + previousGroup?: string | undefined, ) { if (!previousGroup) { const lastItem = items.length ? items[items.length - 1] : undefined; From aeefac131e8a5c36c16d7574c39a53ee7590e005 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 13 Jan 2022 22:14:23 +0530 Subject: [PATCH 23/24] Fix unit tests --- .../commands/setInterpreter.unit.test.ts | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts index 27c94a462cf5..0b91a5f9445e 100644 --- a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts +++ b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts @@ -118,7 +118,7 @@ suite('Set Interpreter Command', () => { const defaultInterpreterPath = 'defaultInterpreterPath'; const defaultInterpreterPathSuggestion = { label: `${Octicons.Gear} ${InterpreterQuickPickList.defaultInterpreterPath.label()}`, - detail: defaultInterpreterPath, + description: defaultInterpreterPath, path: defaultInterpreterPath, alwaysShow: true, }; @@ -210,12 +210,11 @@ suite('Set Interpreter Command', () => { const multiStepInput = TypeMoq.Mock.ofType>(); const recommended = cloneDeep(item); recommended.label = `${Octicons.Star} ${item.label}`; - recommended.description = `${interpreterPath} - ${Common.recommended()}`; - const separator = { label: EnvGroups.Conda, kind: QuickPickItemKind.Separator }; + recommended.description = interpreterPath; const suggestions = [ expectedEnterInterpreterPathSuggestion, defaultInterpreterPathSuggestion, - separator, + { kind: QuickPickItemKind.Separator, label: EnvGroups.Recommended }, recommended, ]; const expectedParameters: IQuickPickParameters = { @@ -305,10 +304,12 @@ suite('Set Interpreter Command', () => { .returns(() => item); const recommended = cloneDeep(item); recommended.label = `${Octicons.Star} ${item.label}`; - recommended.description = `${interpreterPath} - ${Common.recommended()}`; + recommended.description = interpreterPath; const suggestions = [ expectedEnterInterpreterPathSuggestion, defaultInterpreterPathSuggestion, + { kind: QuickPickItemKind.Separator, label: EnvGroups.Recommended }, + recommended, { label: EnvGroups.Workspace, kind: QuickPickItemKind.Separator }, interpreterItems[0], { label: EnvGroups.VirtualEnvWrapper, kind: QuickPickItemKind.Separator }, @@ -316,7 +317,7 @@ suite('Set Interpreter Command', () => { interpreterItems[2], { label: EnvGroups.Conda, kind: QuickPickItemKind.Separator }, interpreterItems[3], - recommended, + item, { label: EnvGroups.Global, kind: QuickPickItemKind.Separator }, interpreterItems[5], ]; @@ -392,12 +393,12 @@ suite('Set Interpreter Command', () => { const multiStepInput = TypeMoq.Mock.ofType>(); const recommended = cloneDeep(item); recommended.label = `${Octicons.Star} ${item.label}`; - recommended.description = `${interpreterPath} - ${Common.recommended()}`; - const separator = { label: EnvGroups.Conda, kind: QuickPickItemKind.Separator }; + recommended.description = interpreterPath; + const separator = { label: EnvGroups.Recommended, kind: QuickPickItemKind.Separator }; const defaultPathSuggestion = { label: `${Octicons.Gear} ${InterpreterQuickPickList.defaultInterpreterPath.label()}`, - detail: expandedDetail, + description: expandedDetail, path: expandedPath, alwaysShow: true, }; From 38212b9a34d3b245197c570c5d3725ab3fb48ba6 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 13 Jan 2022 23:27:53 +0530 Subject: [PATCH 24/24] Change quickpick list placeholder --- package.nls.json | 2 +- package.nls.zh-cn.json | 1 - package.nls.zh-tw.json | 1 - src/client/common/utils/localize.ts | 2 +- 4 files changed, 2 insertions(+), 4 deletions(-) diff --git a/package.nls.json b/package.nls.json index 12071875424b..df3ba64f848f 100644 --- a/package.nls.json +++ b/package.nls.json @@ -55,7 +55,7 @@ "Logging.CurrentWorkingDirectory": "cwd:", "InterpreterQuickPickList.workspaceGroupName": "Workspace", "InterpreterQuickPickList.globalGroupName": "Global", - "InterpreterQuickPickList.quickPickListPlaceholder": "Current: {0}", + "InterpreterQuickPickList.quickPickListPlaceholder": "Selected Interpreter: {0}", "InterpreterQuickPickList.enterPath.label": "Enter interpreter path...", "InterpreterQuickPickList.enterPath.placeholder": "Enter path to a Python interpreter.", "InterpreterQuickPickList.refreshInterpreterList": "Refresh Interpreter list", diff --git a/package.nls.zh-cn.json b/package.nls.zh-cn.json index 968db6618692..f229b7f37907 100644 --- a/package.nls.zh-cn.json +++ b/package.nls.zh-cn.json @@ -49,7 +49,6 @@ "Interpreters.pythonInterpreterPath": "Python 解释器路径: {0}", "Interpreters.condaInheritEnvMessage": "您正在使用 conda 环境，如果您在集成终端中遇到相关问题，建议您允许 Python 扩展将用户设置中的 \"terminal.integrated.inheritEnv\" 改为 false。", "Logging.CurrentWorkingDirectory": "cwd:", - "InterpreterQuickPickList.quickPickListPlaceholder": "当前: {0}", "InterpreterQuickPickList.enterPath.label": "输入解释器路径...", "InterpreterQuickPickList.enterPath.placeholder": "请输入 Python 解释器的路径。", "InterpreterQuickPickList.refreshInterpreterList": "刷新解释器列表", diff --git a/package.nls.zh-tw.json b/package.nls.zh-tw.json index 96c50d19393c..88adaf77a987 100644 --- a/package.nls.zh-tw.json +++ b/package.nls.zh-tw.json @@ -51,7 +51,6 @@ "Interpreters.entireWorkspace": "完整工作區", "Interpreters.pythonInterpreterPath": "Python 直譯器路徑: {0}", "Logging.CurrentWorkingDirectory": "cwd:", - "InterpreterQuickPickList.quickPickListPlaceholder": "目前: {0}", "InterpreterQuickPickList.enterPath.label": "輸入直譯器路徑...", "InterpreterQuickPickList.enterPath.placeholder": "輸入 Python 直譯器的路徑。", "InterpreterQuickPickList.refreshInterpreterList": "重新整理直譯器清單", diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 14d32d7c0426..0f244ac1171a 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -296,7 +296,7 @@ export namespace InterpreterQuickPickList { export const workspaceGroupName = localize('InterpreterQuickPickList.workspaceGroupName', 'Workspace'); export const quickPickListPlaceholder = localize( 'InterpreterQuickPickList.quickPickListPlaceholder', - 'Current: {0}', + 'Selected Interpreter: {0}', ); export const enterPath = { label: localize('InterpreterQuickPickList.enterPath.label', 'Enter interpreter path...'),