Skip to content

Standardize interpreter display format #2525

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

Closed
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/2 Fixes/1352.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improvements to the display format of interpreter information in the list of interpreters.
8 changes: 6 additions & 2 deletions src/client/common/installer/pythonInstallation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,28 @@

import { IInterpreterHelper, IInterpreterLocatorService, IInterpreterService, INTERPRETER_LOCATOR_SERVICE, InterpreterType } from '../../interpreter/contracts';
import { IServiceContainer } from '../../ioc/types';
import { IApplicationShell } from '../application/types';
import { IApplicationShell, IWorkspaceService } from '../application/types';
import { IPlatformService } from '../platform/types';
import { IPythonSettings } from '../types';

export class PythonInstaller {
private locator: IInterpreterLocatorService;
private shell: IApplicationShell;
private workspaceService: IWorkspaceService;

constructor(private serviceContainer: IServiceContainer) {
this.locator = serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, INTERPRETER_LOCATOR_SERVICE);
this.shell = serviceContainer.get<IApplicationShell>(IApplicationShell);
this.workspaceService = serviceContainer.get<IWorkspaceService>(IWorkspaceService);
}

public async checkPythonInstallation(settings: IPythonSettings): Promise<boolean> {
if (settings.disableInstallationChecks === true) {
return true;
}
const interpreters = await this.locator.getInterpreters();

const workspaceUri = this.workspaceService.hasWorkspaceFolders ? this.workspaceService.workspaceFolders![0].uri : undefined;
const interpreters = await this.locator.getInterpreters(workspaceUri);
if (interpreters.length > 0) {
const platform = this.serviceContainer.get<IPlatformService>(IPlatformService);
const helper = this.serviceContainer.get<IInterpreterHelper>(IInterpreterHelper);
Expand Down
17 changes: 15 additions & 2 deletions src/client/common/platform/pathUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,15 @@ import { inject, injectable } from 'inversify';
import * as path from 'path';
import { IPathUtils, IsWindows } from '../types';
import { NON_WINDOWS_PATH_VARIABLE_NAME, WINDOWS_PATH_VARIABLE_NAME } from './constants';
// tslint:disable-next-line:no-var-requires no-require-imports
const untildify = require('untildify');

@injectable()
export class PathUtils implements IPathUtils {
constructor(@inject(IsWindows) private isWindows: boolean) { }
public readonly home = '';
constructor(@inject(IsWindows) private isWindows: boolean) {
this.home = untildify('~');
}
public get delimiter(): string {
return path.delimiter;
}
Expand All @@ -16,5 +21,13 @@ export class PathUtils implements IPathUtils {
public basename(pathValue: string, ext?: string): string {
return path.basename(pathValue, ext);
}

public getDisplayName(pathValue: string, cwd?: string): string {
if (cwd && pathValue.startsWith(cwd)) {
return `.${path.sep}${path.relative(cwd, pathValue)}`;
} else if (pathValue.startsWith(this.home)) {
return `~${path.sep}${path.relative(this.home, pathValue)}`;
} else {
return pathValue;
}
}
}
2 changes: 1 addition & 1 deletion src/client/common/platform/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export class RegistryImplementation implements IRegistry {
}
}

export function getArchitectureDislayName(arch?: Architecture) {
export function getArchitectureDisplayName(arch?: Architecture) {
switch (arch) {
case Architecture.x64:
return '64-bit';
Expand Down
2 changes: 2 additions & 0 deletions src/client/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,10 @@ export const IPathUtils = Symbol('IPathUtils');

export interface IPathUtils {
readonly delimiter: string;
readonly home: string;
getPathVariableName(): 'Path' | 'PATH';
basename(pathValue: string, ext?: string): string;
getDisplayName(pathValue: string, cwd?: string): string;
}

export const IRandom = Symbol('IRandom');
Expand Down
57 changes: 57 additions & 0 deletions src/client/interpreter/configuration/interpreterComparer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

'use strict';

import { inject, injectable } from 'inversify';
import { getArchitectureDisplayName } from '../../common/platform/registry';
import { IServiceContainer } from '../../ioc/types';
import { IInterpreterHelper, PythonInterpreter } from '../contracts';
import { IInterpreterComparer } from './types';

@injectable()
export class InterpreterComparer implements IInterpreterComparer {
private readonly interpreterHelper: IInterpreterHelper;

constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) {
this.interpreterHelper = this.serviceContainer.get<IInterpreterHelper>(IInterpreterHelper);
}
public compare(a: PythonInterpreter, b: PythonInterpreter): number {
return this.getSortName(a) > this.getSortName(b) ? 1 : -1;
}
private getSortName(info: PythonInterpreter): string {
const sortNameParts: string[] = [];
const envSuffixParts: string[] = [];

// Sort order for interpreters is:
// * Version
// * Architecture
// * Interpreter Type
// * Environment name
if (info.version_info && info.version_info.length > 0) {
sortNameParts.push(info.version_info.slice(0, 3).join('.'));
}
if (info.version_info) {
sortNameParts.push(getArchitectureDisplayName(info.architecture));
}
if (info.companyDisplayName && info.companyDisplayName.length > 0) {
sortNameParts.push(info.companyDisplayName.trim());
} else {
sortNameParts.push('Python');
}

if (info.type) {
const name = this.interpreterHelper.getInterpreterTypeDisplayName(info.type);
if (name) {
envSuffixParts.push(name);
}
}
if (info.envName && info.envName.length > 0) {
envSuffixParts.push(info.envName);
}

const envSuffix = envSuffixParts.length === 0 ? '' :
`(${envSuffixParts.join(': ')})`;
return `${sortNameParts.join(' ')} ${envSuffix}`.trim();
}
}
23 changes: 9 additions & 14 deletions src/client/interpreter/configuration/interpreterSelector.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { inject, injectable } from 'inversify';
import * as path from 'path';
import { ConfigurationTarget, Disposable, QuickPickItem, QuickPickOptions, Uri } from 'vscode';
import { IApplicationShell, ICommandManager, IDocumentManager, IWorkspaceService } from '../../common/application/types';
import * as settings from '../../common/configSettings';
import { Commands } from '../../common/constants';
import { IPathUtils } from '../../common/types';
import { IServiceContainer } from '../../ioc/types';
import { IInterpreterService, IShebangCodeLensProvider, PythonInterpreter, WorkspacePythonPath } from '../contracts';
import { IInterpreterSelector, IPythonPathUpdaterServiceManager } from './types';
import { IInterpreterComparer, IInterpreterSelector, IPythonPathUpdaterServiceManager } from './types';

export interface IInterpreterQuickPickItem extends QuickPickItem {
path: string;
Expand All @@ -19,12 +19,16 @@ export class InterpreterSelector implements IInterpreterSelector {
private readonly workspaceService: IWorkspaceService;
private readonly applicationShell: IApplicationShell;
private readonly documentManager: IDocumentManager;
private readonly pathUtils: IPathUtils;
private readonly interpreterComparer: IInterpreterComparer;

constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) {
this.interpreterManager = serviceContainer.get<IInterpreterService>(IInterpreterService);
this.workspaceService = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
this.applicationShell = this.serviceContainer.get<IApplicationShell>(IApplicationShell);
this.documentManager = this.serviceContainer.get<IDocumentManager>(IDocumentManager);
this.pathUtils = this.serviceContainer.get<IPathUtils>(IPathUtils);
this.interpreterComparer = this.serviceContainer.get<IInterpreterComparer>(IInterpreterComparer);

const commandManager = serviceContainer.get<ICommandManager>(ICommandManager);
this.disposables.push(commandManager.registerCommand(Commands.Set_Interpreter, this.setInterpreter.bind(this)));
Expand All @@ -36,11 +40,9 @@ export class InterpreterSelector implements IInterpreterSelector {

public async getSuggestions(resourceUri?: Uri) {
const interpreters = await this.interpreterManager.getInterpreters(resourceUri);
// tslint:disable-next-line:no-non-null-assertion
interpreters.sort((a, b) => a.displayName! > b.displayName! ? 1 : -1);
interpreters.sort(this.interpreterComparer.compare.bind(this.interpreterComparer));
return Promise.all(interpreters.map(item => this.suggestionToQuickPickItem(item, resourceUri)));
}

private async getWorkspaceToSetPythonPath(): Promise<WorkspacePythonPath | undefined> {
if (!Array.isArray(this.workspaceService.workspaceFolders) || this.workspaceService.workspaceFolders.length === 0) {
return undefined;
Expand All @@ -56,15 +58,11 @@ export class InterpreterSelector implements IInterpreterSelector {
}

private async suggestionToQuickPickItem(suggestion: PythonInterpreter, workspaceUri?: Uri): Promise<IInterpreterQuickPickItem> {
let detail = suggestion.path;
if (workspaceUri && suggestion.path.startsWith(workspaceUri.fsPath)) {
detail = `.${path.sep}${path.relative(workspaceUri.fsPath, suggestion.path)}`;
}
const detail = this.pathUtils.getDisplayName(suggestion.path, workspaceUri ? workspaceUri.fsPath : undefined);
const cachedPrefix = suggestion.cachedEntry ? '(cached) ' : '';
return {
// tslint:disable-next-line:no-non-null-assertion
label: suggestion.displayName!,
description: suggestion.companyDisplayName || '',
detail: `${cachedPrefix}${detail}`,
path: suggestion.path
};
Expand All @@ -84,10 +82,7 @@ export class InterpreterSelector implements IInterpreterSelector {
}

const suggestions = await this.getSuggestions(wkspace);
let currentPythonPath = settings.PythonSettings.getInstance().pythonPath;
if (wkspace && currentPythonPath.startsWith(wkspace.fsPath)) {
currentPythonPath = `.${path.sep}${path.relative(wkspace.fsPath, currentPythonPath)}`;
}
const currentPythonPath = this.pathUtils.getDisplayName(settings.PythonSettings.getInstance().pythonPath, wkspace ? wkspace.fsPath : undefined);
const quickPickOptions: QuickPickOptions = {
matchOnDetail: true,
matchOnDescription: true,
Expand Down
6 changes: 6 additions & 0 deletions src/client/interpreter/configuration/types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ConfigurationTarget, Disposable, Uri } from 'vscode';
import { PythonInterpreter } from '../contracts';

export interface IPythonPathUpdaterService {
updatePythonPath(pythonPath: string): Promise<void>;
Expand All @@ -20,3 +21,8 @@ export const IInterpreterSelector = Symbol('IInterpreterSelector');
export interface IInterpreterSelector extends Disposable {

}

export const IInterpreterComparer = Symbol('IInterpreterComparer');
export interface IInterpreterComparer {
compare(a: PythonInterpreter, b: PythonInterpreter): number;
}
5 changes: 4 additions & 1 deletion src/client/interpreter/contracts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,11 @@ export interface IInterpreterService {
getInterpreters(resource?: Uri): Promise<PythonInterpreter[]>;
autoSetInterpreter(): Promise<void>;
getActiveInterpreter(resource?: Uri): Promise<PythonInterpreter | undefined>;
getInterpreterDetails(pythonPath: string): Promise<undefined | Partial<PythonInterpreter>>;
getInterpreterDetails(pythonPath: string, resoure?: Uri): Promise<undefined | PythonInterpreter>;
refresh(): Promise<void>;
initialize(): void;
getDisplayName(interpreter: Partial<PythonInterpreter>): Promise<string>;
shouldAutoSetInterpreter(): Promise<boolean>;
}

export const IInterpreterDisplay = Symbol('IInterpreterDisplay');
Expand All @@ -98,6 +100,7 @@ export interface IInterpreterHelper {
getActiveWorkspaceUri(): WorkspacePythonPath | undefined;
getInterpreterInformation(pythonPath: string): Promise<undefined | Partial<PythonInterpreter>>;
isMacDefaultPythonPath(pythonPath: string): Boolean;
getInterpreterTypeDisplayName(interpreterType: InterpreterType): string | undefined;
}

export const IPipEnvService = Symbol('IPipEnvService');
Expand Down
48 changes: 23 additions & 25 deletions src/client/interpreter/display/index.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import { inject, injectable } from 'inversify';
import { EOL } from 'os';
import * as path from 'path';
import { Disposable, StatusBarAlignment, StatusBarItem, Uri } from 'vscode';
import { IApplicationShell, IWorkspaceService } from '../../common/application/types';
import { IFileSystem } from '../../common/platform/types';
import { IConfigurationService, IDisposableRegistry } from '../../common/types';
import { IConfigurationService, IDisposableRegistry, IPathUtils } from '../../common/types';
import { IServiceContainer } from '../../ioc/types';
import { IInterpreterDisplay, IInterpreterHelper, IInterpreterService, PythonInterpreter } from '../contracts';
import { IVirtualEnvironmentManager } from '../virtualEnvs/types';
Expand All @@ -19,6 +17,7 @@ export class InterpreterDisplay implements IInterpreterDisplay {
private readonly configurationService: IConfigurationService;
private readonly helper: IInterpreterHelper;
private readonly workspaceService: IWorkspaceService;
private readonly pathUtils: IPathUtils;

constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) {
this.interpreterService = serviceContainer.get<IInterpreterService>(IInterpreterService);
Expand All @@ -27,6 +26,7 @@ export class InterpreterDisplay implements IInterpreterDisplay {
this.configurationService = serviceContainer.get<IConfigurationService>(IConfigurationService);
this.helper = serviceContainer.get<IInterpreterHelper>(IInterpreterHelper);
this.workspaceService = serviceContainer.get<IWorkspaceService>(IWorkspaceService);
this.pathUtils = serviceContainer.get<IPathUtils>(IPathUtils);

const application = serviceContainer.get<IApplicationShell>(IApplicationShell);
const disposableRegistry = serviceContainer.get<Disposable[]>(IDisposableRegistry);
Expand All @@ -52,34 +52,32 @@ export class InterpreterDisplay implements IInterpreterDisplay {
const pythonPath = interpreter ? interpreter.path : this.configurationService.getSettings(workspaceFolder).pythonPath;

this.statusBar.color = '';
this.statusBar.tooltip = pythonPath;
this.statusBar.tooltip = this.pathUtils.getDisplayName(pythonPath, workspaceFolder ? workspaceFolder.fsPath : undefined);
if (interpreter) {
// tslint:disable-next-line:no-non-null-assertion
this.statusBar.text = interpreter.displayName!;
if (interpreter.companyDisplayName) {
const toolTipSuffix = `${EOL}${interpreter.companyDisplayName}`;
this.statusBar.tooltip += toolTipSuffix;
}
} else {
await Promise.all([
this.fileSystem.fileExists(pythonPath),
this.helper.getInterpreterInformation(pythonPath).catch<Partial<PythonInterpreter>>(() => undefined),
this.getVirtualEnvironmentName(pythonPath).catch<string>(() => '')
])
.then(([interpreterExists, details, virtualEnvName]) => {
const defaultDisplayName = `${path.basename(pythonPath)} [Environment]`;
const dislayNameSuffix = virtualEnvName.length > 0 ? ` (${virtualEnvName})` : '';
this.statusBar.text = `${details ? details.version : defaultDisplayName}${dislayNameSuffix}`;
this.statusBar.show();
return;
}

const [interpreterExists, details, virtualEnvName] = await Promise.all([
this.fileSystem.fileExists(pythonPath),
this.helper.getInterpreterInformation(pythonPath).catch<Partial<PythonInterpreter> | undefined>(() => undefined),
this.getVirtualEnvironmentName(pythonPath, workspaceFolder).catch<string>(() => '')
]);
if (details) {
const displayName = await this.interpreterService.getDisplayName({ ...details, envName: virtualEnvName });
this.statusBar.text = displayName;
}

if (!interpreterExists && !details && interpreters.length > 0) {
this.statusBar.color = 'yellow';
this.statusBar.text = '$(alert) Select Python Environment';
}
});
if (!interpreterExists && !details && interpreters.length > 0) {
this.statusBar.tooltip = '';
this.statusBar.color = 'yellow';
this.statusBar.text = '$(alert) Select Python Environment';
}
this.statusBar.show();
}
private async getVirtualEnvironmentName(pythonPath: string): Promise<string> {
return this.virtualEnvMgr.getEnvironmentName(pythonPath);
private async getVirtualEnvironmentName(pythonPath: string, resource?: Uri): Promise<string> {
return this.virtualEnvMgr.getEnvironmentName(pythonPath, resource);
}
}
26 changes: 24 additions & 2 deletions src/client/interpreter/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { IFileSystem } from '../common/platform/types';
import { InterpreterInfomation, IPythonExecutionFactory } from '../common/process/types';
import { IPersistentStateFactory } from '../common/types';
import { IServiceContainer } from '../ioc/types';
import { IInterpreterHelper, PythonInterpreter, WorkspacePythonPath } from './contracts';
import { IInterpreterHelper, InterpreterType, PythonInterpreter, WorkspacePythonPath } from './contracts';

const EXPITY_DURATION = 24 * 60 * 60 * 1000;
type CachedPythonInterpreter = Partial<PythonInterpreter> & { fileHash: string };
Expand Down Expand Up @@ -46,7 +46,7 @@ export class InterpreterHelper implements IInterpreterHelper {
public async getInterpreterInformation(pythonPath: string): Promise<undefined | Partial<PythonInterpreter>> {
let fileHash = await this.fs.getFileHash(pythonPath).catch(() => '');
fileHash = fileHash ? fileHash : '';
const store = this.persistentFactory.createGlobalPersistentState<CachedPythonInterpreter>(pythonPath, undefined, EXPITY_DURATION);
const store = this.persistentFactory.createGlobalPersistentState<CachedPythonInterpreter>(`${pythonPath}.v1`, undefined, EXPITY_DURATION);
if (store.value && (!fileHash || store.value.fileHash === fileHash)) {
return store.value;
}
Expand All @@ -71,4 +71,26 @@ export class InterpreterHelper implements IInterpreterHelper {
public isMacDefaultPythonPath(pythonPath: string) {
return pythonPath === 'python' || pythonPath === '/usr/bin/python';
}
public getInterpreterTypeDisplayName(interpreterType: InterpreterType) {
switch (interpreterType) {
case InterpreterType.Conda: {
return 'conda';
}
case InterpreterType.PipEnv: {
return 'pipenv';
}
case InterpreterType.Pyenv: {
return 'pyenv';
}
case InterpreterType.Venv: {
return 'venv';
}
case InterpreterType.VirtualEnv: {
return 'virtualEnv';
}
default: {
return '';
}
}
}
}
Loading