Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Group interpreters in interpreter quick picker using separators #18171

Merged
merged 24 commits into from
Jan 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/1 Enhancements/17944.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Group interpreters in interpreter quick picker using separators.
4 changes: 3 additions & 1 deletion package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@
"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.quickPickListPlaceholder": "Current: {0}",
"InterpreterQuickPickList.workspaceGroupName": "Workspace",
"InterpreterQuickPickList.globalGroupName": "Global",
"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",
Expand Down
1 change: 0 additions & 1 deletion package.nls.zh-cn.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": "刷新解释器列表",
Expand Down
1 change: 0 additions & 1 deletion package.nls.zh-tw.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": "重新整理直譯器清單",
Expand Down
4 changes: 3 additions & 1 deletion src/client/common/utils/localize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,9 +292,11 @@ 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}',
'Selected Interpreter: {0}',
);
export const enterPath = {
label: localize('InterpreterQuickPickList.enterPath.label', 'Enter interpreter path...'),
Expand Down
18 changes: 6 additions & 12 deletions src/client/interpreter/autoSelection/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 { EnvTypeHeuristic, getEnvTypeHeuristic } from '../configuration/environmentTypeComparer';
import { IInterpreterComparer } from '../configuration/types';
import { IInterpreterHelper, IInterpreterService } from '../contracts';
import { IInterpreterAutoSelectionService, IInterpreterAutoSelectionProxyService } from './types';
Expand Down Expand Up @@ -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) => getEnvTypeHeuristic(i, workspaceUri?.folderUri.fsPath || '') !== EnvTypeHeuristic.Global,
);

filteredInterpreters.sort(this.envTypeComparer.compare.bind(this.envTypeComparer));

const recommendedInterpreter = this.envTypeComparer.getRecommended(interpreters, workspaceUri?.folderUri);
if (!recommendedInterpreter) {
return;
}
Comment on lines +206 to +209
Copy link
Author

Choose a reason for hiding this comment

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

Refactored it out as we also need this info while showing the recommended interpreter in the list.

if (workspaceUri) {
this.setWorkspaceInterpreter(workspaceUri.folderUri, filteredInterpreters[0]);
this.setWorkspaceInterpreter(workspaceUri.folderUri, recommendedInterpreter);
} else {
this.setGlobalInterpreter(filteredInterpreters[0]);
this.setGlobalInterpreter(recommendedInterpreter);
}

queriedState.updateValue(true);
Expand Down
117 changes: 73 additions & 44 deletions src/client/interpreter/configuration/environmentTypeComparer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,22 @@

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';

/*
* 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()
Expand All @@ -41,8 +41,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;
}
Expand All @@ -53,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;
}
Copy link
Author

Choose a reason for hiding this comment

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

This is handled by getPrioritizedEnvironmentType() instead.


// 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);
Expand All @@ -76,6 +77,25 @@ export class EnvironmentTypeComparer implements IInterpreterComparer {

return nameA > nameB ? 1 : -1;
}

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);
Copy link
Member

Choose a reason for hiding this comment

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

Might be a good item for debt week, we can remove the dependency on the interpreterHelper. What we really need here is the workspace uri, which is the real data. instead of depending on the object that provides that data.

Copy link
Author

Choose a reason for hiding this comment

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

Are you suggesting we directly query vscode.workspace.getWorkspaceFolder API instead of using interpreter helper? (and get rid of the interpreter helper)

Interpreter helper also handles the case when resource is undefined, I guess that's why it's useful.

const filteredInterpreters = interpreters.filter((i) => {
if (getEnvLocationHeuristic(i, workspaceUri?.folderUri.fsPath || '') === EnvLocationHeuristic.Local) {
return true;
}
if (virtualEnvTypes.includes(i.envType)) {
// We're not sure if these envs were created for the workspace, so do not recommend them.
return false;
}
return true;
});
filteredInterpreters.sort(this.compare.bind(this));
return filteredInterpreters.length ? filteredInterpreters[0] : undefined;
}
}

function getSortName(info: PythonEnvironment, interpreterHelper: IInterpreterHelper): string {
Expand Down Expand Up @@ -113,12 +133,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')
);
}

/**
Expand Down Expand Up @@ -151,40 +170,50 @@ 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);
}

/**
* 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;
}
Comment on lines -177 to -189
Copy link
Author

Choose a reason for hiding this comment

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

We want to make sure environments of the same type appear together, so they can be grouped. But in this all global environment types were assigned the same priority, so order was not guaranteed.

/**
* 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 Math.sign(envTypeByPriority.indexOf(a.envType) - envTypeByPriority.indexOf(b.envType));
}

function getPrioritizedEnvironmentType(): EnvironmentType[] {
return [
// Prioritize non-Conda environments.
EnvironmentType.Poetry,
EnvironmentType.Pipenv,
EnvironmentType.VirtualEnvWrapper,
EnvironmentType.Venv,
EnvironmentType.VirtualEnv,
EnvironmentType.Conda,
Copy link
Author

@karrtikr karrtikr Jan 11, 2022

Choose a reason for hiding this comment

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

Instead we assign each type a unique priority, conda being the last in the priority queue.

EnvironmentType.Pyenv,
EnvironmentType.WindowsStore,
EnvironmentType.Global,
EnvironmentType.System,
EnvironmentType.Unknown,
];
}
Loading