Skip to content

Standardize interpreter display format #2540

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
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
4 changes: 1 addition & 3 deletions src/client/debugger/PythonProcess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@
import * as net from "net";
import { ChildProcess } from 'child_process';
import { EventEmitter } from "events";
import { IPythonProcess, IPythonThread, IPythonEvaluationResult, IPythonStackFrame } from "./Common/Contracts";
import { IPythonBreakpoint, IBreakpointCommand, IChildEnumCommand } from "./Common/Contracts";
import { PythonEvaluationResultReprKind, IExecutionCommand, enum_EXCEPTION_STATE } from "./Common/Contracts";
import { IPythonProcess, IPythonThread, IPythonEvaluationResult, IPythonStackFrame, IPythonBreakpoint, IBreakpointCommand, IChildEnumCommand, PythonEvaluationResultReprKind, IExecutionCommand, enum_EXCEPTION_STATE } from "./Common/Contracts";
import { Commands } from "./ProxyCommands";
import { IdDispenser } from "../common/idDispenser";
import { PythonProcessCallbackHandler } from "./PythonProcessCallbackHandler";
Expand Down
58 changes: 58 additions & 0 deletions src/client/interpreter/configuration/interpreterComparer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// 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 { IInterpreterHelper, PythonInterpreter } from '../contracts';
import { IInterpreterComparer } from './types';

@injectable()
export class InterpreterComparer implements IInterpreterComparer {
constructor(@inject(IInterpreterHelper) private readonly interpreterHelper: IInterpreterHelper) {
}
public compare(a: PythonInterpreter, b: PythonInterpreter): number {
const nameA = this.getSortName(a);
const nameB = this.getSortName(b);
if (nameA === nameB) {
return 0;
}
return nameA > nameB ? 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.architecture) {
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
53 changes: 11 additions & 42 deletions src/client/interpreter/display/index.ts
Original file line number Diff line number Diff line change
@@ -1,32 +1,24 @@
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 { IDisposableRegistry, IPathUtils } from '../../common/types';
import { IServiceContainer } from '../../ioc/types';
import { IInterpreterDisplay, IInterpreterHelper, IInterpreterService, PythonInterpreter } from '../contracts';
import { IVirtualEnvironmentManager } from '../virtualEnvs/types';
import { IInterpreterDisplay, IInterpreterHelper, IInterpreterService } from '../contracts';

// tslint:disable-next-line:completed-docs
@injectable()
export class InterpreterDisplay implements IInterpreterDisplay {
private readonly statusBar: StatusBarItem;
private readonly interpreterService: IInterpreterService;
private readonly virtualEnvMgr: IVirtualEnvironmentManager;
private readonly fileSystem: IFileSystem;
private readonly configurationService: IConfigurationService;
private readonly helper: IInterpreterHelper;
private readonly workspaceService: IWorkspaceService;
private readonly pathUtils: IPathUtils;
private readonly interpreterService: IInterpreterService;

constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) {
this.interpreterService = serviceContainer.get<IInterpreterService>(IInterpreterService);
this.virtualEnvMgr = serviceContainer.get<IVirtualEnvironmentManager>(IVirtualEnvironmentManager);
this.fileSystem = serviceContainer.get<IFileSystem>(IFileSystem);
this.configurationService = serviceContainer.get<IConfigurationService>(IConfigurationService);
this.helper = serviceContainer.get<IInterpreterHelper>(IInterpreterHelper);
this.workspaceService = serviceContainer.get<IWorkspaceService>(IWorkspaceService);
this.pathUtils = serviceContainer.get<IPathUtils>(IPathUtils);
this.interpreterService = serviceContainer.get<IInterpreterService>(IInterpreterService);

const application = serviceContainer.get<IApplicationShell>(IApplicationShell);
const disposableRegistry = serviceContainer.get<Disposable[]>(IDisposableRegistry);
Expand All @@ -47,39 +39,16 @@ export class InterpreterDisplay implements IInterpreterDisplay {
await this.updateDisplay(resource);
}
private async updateDisplay(workspaceFolder?: Uri) {
const interpreters = await this.interpreterService.getInterpreters(workspaceFolder);
const interpreter = await this.interpreterService.getActiveInterpreter(workspaceFolder);
const pythonPath = interpreter ? interpreter.path : this.configurationService.getSettings(workspaceFolder).pythonPath;

this.statusBar.color = '';
this.statusBar.tooltip = pythonPath;
if (interpreter) {
// tslint:disable-next-line:no-non-null-assertion
this.statusBar.color = '';
this.statusBar.tooltip = this.pathUtils.getDisplayName(interpreter.path, workspaceFolder ? workspaceFolder.fsPath : undefined);
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}`;

if (!interpreterExists && !details && interpreters.length > 0) {
this.statusBar.color = 'yellow';
this.statusBar.text = '$(alert) Select Python Environment';
}
});
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);
}
}
Loading