From 79c747b86bf2ddf9409e8201ce04286698e1c30e Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 10 Dec 2021 01:29:13 +0530 Subject: [PATCH 1/8] Move interpreter display status bar item to the right --- src/client/interpreter/display/index.ts | 2 +- src/test/interpreters/display.unit.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/interpreter/display/index.ts b/src/client/interpreter/display/index.ts index c30eb540b995..3737df675ea5 100644 --- a/src/client/interpreter/display/index.ts +++ b/src/client/interpreter/display/index.ts @@ -36,7 +36,7 @@ export class InterpreterDisplay implements IInterpreterDisplay { const application = serviceContainer.get(IApplicationShell); const disposableRegistry = serviceContainer.get(IDisposableRegistry); - this.statusBar = application.createStatusBarItem(StatusBarAlignment.Left, 100); + this.statusBar = application.createStatusBarItem(StatusBarAlignment.Right, 100); this.statusBar.command = 'python.setInterpreter'; disposableRegistry.push(this.statusBar); diff --git a/src/test/interpreters/display.unit.test.ts b/src/test/interpreters/display.unit.test.ts index 059a00fa5e27..fd4b24a9c8ca 100644 --- a/src/test/interpreters/display.unit.test.ts +++ b/src/test/interpreters/display.unit.test.ts @@ -82,7 +82,7 @@ suite('Interpreters Display', () => { .returns(() => interpreterHelper.object); serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IPathUtils))).returns(() => pathUtils.object); applicationShell - .setup((a) => a.createStatusBarItem(TypeMoq.It.isValue(StatusBarAlignment.Left), TypeMoq.It.isValue(100))) + .setup((a) => a.createStatusBarItem(TypeMoq.It.isValue(StatusBarAlignment.Right), TypeMoq.It.isValue(100))) .returns(() => statusBar.object); pathUtils.setup((p) => p.getDisplayName(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns((p) => p); createInterpreterDisplay(); From 01065aa8f94ba5ba7235f632530f65e7c5c69d4a Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 14 Dec 2021 00:57:25 +0530 Subject: [PATCH 2/8] Simulate pinning version in status bar --- src/client/interpreter/display/index.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/client/interpreter/display/index.ts b/src/client/interpreter/display/index.ts index 3737df675ea5..3c623184f34d 100644 --- a/src/client/interpreter/display/index.ts +++ b/src/client/interpreter/display/index.ts @@ -14,6 +14,11 @@ import { IInterpreterStatusbarVisibilityFilter, } from '../contracts'; +/** + * Based on https://github.com/microsoft/vscode-python/issues/18040#issuecomment-992567670. + * This is to ensure the item appears right after the Python language status item. + */ +const STATUS_BAR_ITEM_PRIORITY = 100.09999; @injectable() export class InterpreterDisplay implements IInterpreterDisplay { private readonly statusBar: StatusBarItem; @@ -36,7 +41,7 @@ export class InterpreterDisplay implements IInterpreterDisplay { const application = serviceContainer.get(IApplicationShell); const disposableRegistry = serviceContainer.get(IDisposableRegistry); - this.statusBar = application.createStatusBarItem(StatusBarAlignment.Right, 100); + this.statusBar = application.createStatusBarItem(StatusBarAlignment.Right, STATUS_BAR_ITEM_PRIORITY); this.statusBar.command = 'python.setInterpreter'; disposableRegistry.push(this.statusBar); @@ -83,7 +88,7 @@ export class InterpreterDisplay implements IInterpreterDisplay { ); this.interpreterPath = interpreter.path; } - this.statusBar.text = interpreter.displayName!; + this.statusBar.text = interpreter.displayName!.substring('Python '.length); this.currentlySelectedInterpreterPath = interpreter.path; } else { this.statusBar.tooltip = ''; From 839796e2d342ce1a93c51f882c8fc34faf8a3837 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 12 Jan 2022 18:57:21 +0530 Subject: [PATCH 3/8] Add pinning behind experiment --- src/client/common/experiments/groups.ts | 4 ++ src/client/interpreter/display/index.ts | 43 +++++++++++++++----- src/client/interpreter/interpreterService.ts | 40 ++++++++++++++++-- src/client/interpreter/serviceRegistry.ts | 1 + 4 files changed, 73 insertions(+), 15 deletions(-) diff --git a/src/client/common/experiments/groups.ts b/src/client/common/experiments/groups.ts index 7f2b9f74cef5..37359a2aea9f 100644 --- a/src/client/common/experiments/groups.ts +++ b/src/client/common/experiments/groups.ts @@ -2,3 +2,7 @@ export enum ShowExtensionSurveyPrompt { experiment = 'pythonSurveyNotification', } +export enum InterpreterStatusBarPosition { + Pinned = 'pythonInterpreterInfoPinned', + Unpinned = 'pythonInterpreterInfoUnpinned', +} diff --git a/src/client/interpreter/display/index.ts b/src/client/interpreter/display/index.ts index 3c623184f34d..a45587ccd145 100644 --- a/src/client/interpreter/display/index.ts +++ b/src/client/interpreter/display/index.ts @@ -1,8 +1,11 @@ import { inject, injectable } from 'inversify'; import { Disposable, StatusBarAlignment, StatusBarItem, Uri } from 'vscode'; +import { IExtensionSingleActivationService } from '../../activation/types'; import { IApplicationShell, IWorkspaceService } from '../../common/application/types'; +import { Commands } from '../../common/constants'; +import { InterpreterStatusBarPosition } from '../../common/experiments/groups'; import '../../common/extensions'; -import { IDisposableRegistry, IPathUtils, Resource } from '../../common/types'; +import { IDisposableRegistry, IExperimentService, IPathUtils, Resource } from '../../common/types'; import { Interpreters } from '../../common/utils/localize'; import { IServiceContainer } from '../../ioc/types'; import { traceLog } from '../../logging'; @@ -20,8 +23,12 @@ import { */ const STATUS_BAR_ITEM_PRIORITY = 100.09999; @injectable() -export class InterpreterDisplay implements IInterpreterDisplay { - private readonly statusBar: StatusBarItem; +export class InterpreterDisplay implements IInterpreterDisplay, IExtensionSingleActivationService { + public supportedWorkspaceTypes: { untrustedWorkspace: boolean; virtualWorkspace: boolean } = { + untrustedWorkspace: false, + virtualWorkspace: true, + }; + private statusBar!: StatusBarItem; private readonly helper: IInterpreterHelper; private readonly workspaceService: IWorkspaceService; private readonly pathUtils: IPathUtils; @@ -31,6 +38,8 @@ export class InterpreterDisplay implements IInterpreterDisplay { private interpreterPath: string | undefined; private statusBarCanBeDisplayed?: boolean; private visibilityFilters: IInterpreterStatusbarVisibilityFilter[] = []; + private disposableRegistry: Disposable[]; + private experiments: IExperimentService; constructor(@inject(IServiceContainer) private readonly serviceContainer: IServiceContainer) { this.helper = serviceContainer.get(IInterpreterHelper); @@ -38,19 +47,27 @@ export class InterpreterDisplay implements IInterpreterDisplay { this.pathUtils = serviceContainer.get(IPathUtils); this.interpreterService = serviceContainer.get(IInterpreterService); - const application = serviceContainer.get(IApplicationShell); - const disposableRegistry = serviceContainer.get(IDisposableRegistry); - - this.statusBar = application.createStatusBarItem(StatusBarAlignment.Right, STATUS_BAR_ITEM_PRIORITY); - this.statusBar.command = 'python.setInterpreter'; - disposableRegistry.push(this.statusBar); + this.disposableRegistry = serviceContainer.get(IDisposableRegistry); this.interpreterService.onDidChangeInterpreterInformation( this.onDidChangeInterpreterInformation, this, - disposableRegistry, + this.disposableRegistry, ); + this.experiments = this.serviceContainer.get(IExperimentService); + } + + public async activate(): Promise { + let [alignment, priority] = [StatusBarAlignment.Left, undefined]; + if (this.experiments.inExperimentSync(InterpreterStatusBarPosition.Pinned)) { + [alignment, priority] = [StatusBarAlignment.Right, STATUS_BAR_ITEM_PRIORITY]; + } + const application = this.serviceContainer.get(IApplicationShell); + this.statusBar = application.createStatusBarItem(alignment, priority); + this.statusBar.command = Commands.Set_Interpreter; + this.disposableRegistry.push(this.statusBar); } + public async refresh(resource?: Uri) { // Use the workspace Uri if available if (resource && this.workspaceService.getWorkspaceFolder(resource)) { @@ -88,7 +105,11 @@ export class InterpreterDisplay implements IInterpreterDisplay { ); this.interpreterPath = interpreter.path; } - this.statusBar.text = interpreter.displayName!.substring('Python '.length); + let text = interpreter.displayName!; + if (this.experiments.inExperimentSync(InterpreterStatusBarPosition.Pinned)) { + text = text.substring('Python '.length); + } + this.statusBar.text = text; this.currentlySelectedInterpreterPath = interpreter.path; } else { this.statusBar.tooltip = ''; diff --git a/src/client/interpreter/interpreterService.ts b/src/client/interpreter/interpreterService.ts index 1e795aee23cd..5b46c303804f 100644 --- a/src/client/interpreter/interpreterService.ts +++ b/src/client/interpreter/interpreterService.ts @@ -1,19 +1,28 @@ +// eslint-disable-next-line max-classes-per-file import { inject, injectable } from 'inversify'; import { Disposable, Event, EventEmitter, Uri } from 'vscode'; import '../common/extensions'; import { IDocumentManager } from '../common/application/types'; import { IPythonExecutionFactory } from '../common/process/types'; -import { IConfigurationService, IDisposableRegistry, IInterpreterPathService } from '../common/types'; +import { + IConfigurationService, + IDisposableRegistry, + IExperimentService, + IInterpreterPathService, +} from '../common/types'; import { IServiceContainer } from '../ioc/types'; import { PythonEnvironment } from '../pythonEnvironments/info'; import { IComponentAdapter, IInterpreterDisplay, IInterpreterService, + IInterpreterStatusbarVisibilityFilter, PythonEnvironmentsChangedEvent, } from './contracts'; import { PythonLocatorQuery } from '../pythonEnvironments/base/locator'; import { traceError } from '../logging'; +import { PYTHON_LANGUAGE } from '../common/constants'; +import { InterpreterStatusBarPosition } from '../common/experiments/groups'; type StoredPythonEnvironment = PythonEnvironment & { store?: boolean }; @@ -80,6 +89,22 @@ export class InterpreterService implements Disposable, IInterpreterService { public initialize(): void { const disposables = this.serviceContainer.get(IDisposableRegistry); const documentManager = this.serviceContainer.get(IDocumentManager); + const interpreterDisplay = this.serviceContainer.get(IInterpreterDisplay); + const filter = new (class implements IInterpreterStatusbarVisibilityFilter { + constructor(private readonly docManager: IDocumentManager) {} + + public readonly interpreterVisibilityEmitter = new EventEmitter(); + + public readonly changed = this.interpreterVisibilityEmitter.event; + + get hidden() { + return this.docManager.activeTextEditor?.document.languageId !== PYTHON_LANGUAGE; + } + })(documentManager); + const experiments = this.serviceContainer.get(IExperimentService); + if (experiments.inExperimentSync(InterpreterStatusBarPosition.Pinned)) { + interpreterDisplay.registerVisibilityFilter(filter); + } disposables.push( this.onDidChangeInterpreters((e) => { const interpreter = e.old ?? e.new; @@ -89,9 +114,16 @@ export class InterpreterService implements Disposable, IInterpreterService { }), ); disposables.push( - documentManager.onDidChangeActiveTextEditor((e) => - e && e.document ? this.refresh(e.document.uri) : undefined, - ), + documentManager.onDidOpenTextDocument(() => { + // To handle scenario when language mode is set to "python" + filter.interpreterVisibilityEmitter.fire(); + }), + documentManager.onDidChangeActiveTextEditor((e) => { + filter.interpreterVisibilityEmitter.fire(); + if (e && e.document) { + this.refresh(e.document.uri); + } + }), ); const pySettings = this.configService.getSettings(); this._pythonPathSetting = pySettings.pythonPath; diff --git a/src/client/interpreter/serviceRegistry.ts b/src/client/interpreter/serviceRegistry.ts index de27e1c914d7..bf586dd4e959 100644 --- a/src/client/interpreter/serviceRegistry.ts +++ b/src/client/interpreter/serviceRegistry.ts @@ -57,6 +57,7 @@ export function registerInterpreterTypes(serviceManager: IServiceManager): void serviceManager.addSingleton(IInterpreterService, InterpreterService); serviceManager.addSingleton(IInterpreterDisplay, InterpreterDisplay); + serviceManager.addBinding(IInterpreterDisplay, IExtensionSingleActivationService); serviceManager.addSingleton( IPythonPathUpdaterServiceFactory, From 98d718073fd12c46428d5b04a99aa1b2995954b0 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 12 Jan 2022 21:04:32 +0530 Subject: [PATCH 4/8] Fix tests --- src/client/common/experiments/service.ts | 4 ++++ src/client/interpreter/display/index.ts | 4 ++-- src/test/interpreters/display.unit.test.ts | 19 +++++++++++++------ .../interpreterService.unit.test.ts | 6 ++++++ 4 files changed, 25 insertions(+), 8 deletions(-) diff --git a/src/client/common/experiments/service.ts b/src/client/common/experiments/service.ts index fb6c856251e4..879470af56da 100644 --- a/src/client/common/experiments/service.ts +++ b/src/client/common/experiments/service.ts @@ -13,6 +13,7 @@ import { IApplicationEnvironment, IWorkspaceService } from '../application/types import { PVSC_EXTENSION_ID } from '../constants'; import { GLOBAL_MEMENTO, IExperimentService, IMemento } from '../types'; import { Experiments } from '../utils/localize'; +import { InterpreterStatusBarPosition } from './groups'; import { ExperimentationTelemetry } from './telemetry'; const EXP_MEMENTO_KEY = 'VSCode.ABExp.FeatureData'; @@ -108,6 +109,9 @@ export class ExperimentService implements IExperimentService { } public inExperimentSync(experiment: string): boolean { + if (experiment === InterpreterStatusBarPosition.Pinned) { + return true; + } if (!this.experimentationService) { return false; } diff --git a/src/client/interpreter/display/index.ts b/src/client/interpreter/display/index.ts index a45587ccd145..9eb573f850f7 100644 --- a/src/client/interpreter/display/index.ts +++ b/src/client/interpreter/display/index.ts @@ -58,7 +58,7 @@ export class InterpreterDisplay implements IInterpreterDisplay, IExtensionSingle } public async activate(): Promise { - let [alignment, priority] = [StatusBarAlignment.Left, undefined]; + let [alignment, priority] = [StatusBarAlignment.Left, 100]; if (this.experiments.inExperimentSync(InterpreterStatusBarPosition.Pinned)) { [alignment, priority] = [StatusBarAlignment.Right, STATUS_BAR_ITEM_PRIORITY]; } @@ -107,7 +107,7 @@ export class InterpreterDisplay implements IInterpreterDisplay, IExtensionSingle } let text = interpreter.displayName!; if (this.experiments.inExperimentSync(InterpreterStatusBarPosition.Pinned)) { - text = text.substring('Python '.length); + text = text.substring('Python'.length).trim(); } this.statusBar.text = text; this.currentlySelectedInterpreterPath = interpreter.path; diff --git a/src/test/interpreters/display.unit.test.ts b/src/test/interpreters/display.unit.test.ts index fd4b24a9c8ca..ead941641584 100644 --- a/src/test/interpreters/display.unit.test.ts +++ b/src/test/interpreters/display.unit.test.ts @@ -12,9 +12,11 @@ import { Uri, WorkspaceFolder, } from 'vscode'; +import { IExtensionSingleActivationService } from '../../client/activation/types'; import { IApplicationShell, IWorkspaceService } from '../../client/common/application/types'; +import { InterpreterStatusBarPosition } from '../../client/common/experiments/groups'; import { IFileSystem } from '../../client/common/platform/types'; -import { IDisposableRegistry, IPathUtils, ReadWrite } from '../../client/common/types'; +import { IDisposableRegistry, IExperimentService, IPathUtils, ReadWrite } from '../../client/common/types'; import { Interpreters } from '../../client/common/utils/localize'; import { Architecture } from '../../client/common/utils/platform'; import { @@ -48,16 +50,19 @@ suite('Interpreters Display', () => { let fileSystem: TypeMoq.IMock; let disposableRegistry: Disposable[]; let statusBar: TypeMoq.IMock; - let interpreterDisplay: IInterpreterDisplay; + let interpreterDisplay: IInterpreterDisplay & IExtensionSingleActivationService; let interpreterHelper: TypeMoq.IMock; + let experiments: TypeMoq.IMock; let pathUtils: TypeMoq.IMock; let traceLogStub: sinon.SinonStub; - setup(() => { + setup(async () => { serviceContainer = TypeMoq.Mock.ofType(); workspaceService = TypeMoq.Mock.ofType(); applicationShell = TypeMoq.Mock.ofType(); interpreterService = TypeMoq.Mock.ofType(); + experiments = TypeMoq.Mock.ofType(); + experiments.setup((e) => e.inExperimentSync(InterpreterStatusBarPosition.Pinned)).returns(() => false); fileSystem = TypeMoq.Mock.ofType(); interpreterHelper = TypeMoq.Mock.ofType(); disposableRegistry = []; @@ -75,6 +80,7 @@ suite('Interpreters Display', () => { serviceContainer .setup((c) => c.get(TypeMoq.It.isValue(IInterpreterService))) .returns(() => interpreterService.object); + serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IExperimentService))).returns(() => experiments.object); serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IFileSystem))).returns(() => fileSystem.object); serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IDisposableRegistry))).returns(() => disposableRegistry); serviceContainer @@ -82,18 +88,19 @@ suite('Interpreters Display', () => { .returns(() => interpreterHelper.object); serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IPathUtils))).returns(() => pathUtils.object); applicationShell - .setup((a) => a.createStatusBarItem(TypeMoq.It.isValue(StatusBarAlignment.Right), TypeMoq.It.isValue(100))) + .setup((a) => a.createStatusBarItem(TypeMoq.It.isValue(StatusBarAlignment.Left), TypeMoq.It.isValue(100))) .returns(() => statusBar.object); pathUtils.setup((p) => p.getDisplayName(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns((p) => p); - createInterpreterDisplay(); + await createInterpreterDisplay(); }); teardown(() => { sinon.restore(); }); - function createInterpreterDisplay(filters: IInterpreterStatusbarVisibilityFilter[] = []) { + async function createInterpreterDisplay(filters: IInterpreterStatusbarVisibilityFilter[] = []) { interpreterDisplay = new InterpreterDisplay(serviceContainer.object); + await interpreterDisplay.activate(); filters.forEach((f) => interpreterDisplay.registerVisibilityFilter(f)); } function setupWorkspaceFolder(resource: Uri, workspaceFolder?: Uri) { diff --git a/src/test/interpreters/interpreterService.unit.test.ts b/src/test/interpreters/interpreterService.unit.test.ts index 8dc1557ccde1..d3119f589815 100644 --- a/src/test/interpreters/interpreterService.unit.test.ts +++ b/src/test/interpreters/interpreterService.unit.test.ts @@ -10,11 +10,13 @@ import * as path from 'path'; import * as TypeMoq from 'typemoq'; import { ConfigurationTarget, Disposable, TextDocument, TextEditor, Uri, WorkspaceConfiguration } from 'vscode'; import { IDocumentManager, IWorkspaceService } from '../../client/common/application/types'; +import { InterpreterStatusBarPosition } from '../../client/common/experiments/groups'; import { IFileSystem } from '../../client/common/platform/types'; import { IPythonExecutionFactory, IPythonExecutionService } from '../../client/common/process/types'; import { IConfigurationService, IDisposableRegistry, + IExperimentService, IInterpreterPathService, InterpreterConfigurationScope, IPersistentStateFactory, @@ -53,6 +55,7 @@ suite('Interpreters service', () => { let configService: TypeMoq.IMock; let interpreterPathService: TypeMoq.IMock; let pythonSettings: TypeMoq.IMock; + let experiments: TypeMoq.IMock; function setupSuite() { const cont = new Container(); @@ -71,6 +74,8 @@ suite('Interpreters service', () => { pythonExecutionFactory = TypeMoq.Mock.ofType(); pythonExecutionService = TypeMoq.Mock.ofType(); configService = TypeMoq.Mock.ofType(); + experiments = TypeMoq.Mock.ofType(); + experiments.setup((e) => e.inExperimentSync(InterpreterStatusBarPosition.Pinned)).returns(() => false); pythonSettings = TypeMoq.Mock.ofType(); pythonSettings.setup((s) => s.pythonPath).returns(() => PYTHON_PATH); @@ -91,6 +96,7 @@ suite('Interpreters service', () => { return state as any; }); + serviceManager.addSingletonInstance(IExperimentService, experiments.object); serviceManager.addSingletonInstance(IDisposableRegistry, []); serviceManager.addSingletonInstance(IInterpreterHelper, helper.object); serviceManager.addSingletonInstance( From 96cedf7b2b3efa4151c3df140799785c5ee5af8a Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 12 Jan 2022 21:39:54 +0530 Subject: [PATCH 5/8] Add tests --- src/client/interpreter/display/index.ts | 2 +- src/test/interpreters/display.unit.test.ts | 492 +++++++++++---------- 2 files changed, 268 insertions(+), 226 deletions(-) diff --git a/src/client/interpreter/display/index.ts b/src/client/interpreter/display/index.ts index 9eb573f850f7..4df784b15068 100644 --- a/src/client/interpreter/display/index.ts +++ b/src/client/interpreter/display/index.ts @@ -107,7 +107,7 @@ export class InterpreterDisplay implements IInterpreterDisplay, IExtensionSingle } let text = interpreter.displayName!; if (this.experiments.inExperimentSync(InterpreterStatusBarPosition.Pinned)) { - text = text.substring('Python'.length).trim(); + text = text.startsWith('Python') ? text.substring('Python'.length).trim() : text; } this.statusBar.text = text; this.currentlySelectedInterpreterPath = interpreter.path; diff --git a/src/test/interpreters/display.unit.test.ts b/src/test/interpreters/display.unit.test.ts index ead941641584..a459f6c99ecf 100644 --- a/src/test/interpreters/display.unit.test.ts +++ b/src/test/interpreters/display.unit.test.ts @@ -42,7 +42,7 @@ const info: PythonEnvironment = { sysVersion: '', }; -suite('Interpreters Display', () => { +suite('xInterpreters Display', () => { let applicationShell: TypeMoq.IMock; let workspaceService: TypeMoq.IMock; let serviceContainer: TypeMoq.IMock; @@ -55,14 +55,19 @@ suite('Interpreters Display', () => { let experiments: TypeMoq.IMock; let pathUtils: TypeMoq.IMock; let traceLogStub: sinon.SinonStub; + async function createInterpreterDisplay(filters: IInterpreterStatusbarVisibilityFilter[] = []) { + interpreterDisplay = new InterpreterDisplay(serviceContainer.object); + await interpreterDisplay.activate(); + filters.forEach((f) => interpreterDisplay.registerVisibilityFilter(f)); + } - setup(async () => { + async function setupMocks(inExperiment = false) { serviceContainer = TypeMoq.Mock.ofType(); workspaceService = TypeMoq.Mock.ofType(); applicationShell = TypeMoq.Mock.ofType(); interpreterService = TypeMoq.Mock.ofType(); experiments = TypeMoq.Mock.ofType(); - experiments.setup((e) => e.inExperimentSync(InterpreterStatusBarPosition.Pinned)).returns(() => false); + experiments.setup((e) => e.inExperimentSync(InterpreterStatusBarPosition.Pinned)).returns(() => inExperiment); fileSystem = TypeMoq.Mock.ofType(); interpreterHelper = TypeMoq.Mock.ofType(); disposableRegistry = []; @@ -87,22 +92,21 @@ suite('Interpreters Display', () => { .setup((c) => c.get(TypeMoq.It.isValue(IInterpreterHelper))) .returns(() => interpreterHelper.object); serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IPathUtils))).returns(() => pathUtils.object); - applicationShell - .setup((a) => a.createStatusBarItem(TypeMoq.It.isValue(StatusBarAlignment.Left), TypeMoq.It.isValue(100))) - .returns(() => statusBar.object); + if (inExperiment) { + applicationShell + .setup((a) => a.createStatusBarItem(TypeMoq.It.isValue(StatusBarAlignment.Right), TypeMoq.It.isAny())) + .returns(() => statusBar.object); + } else { + applicationShell + .setup((a) => + a.createStatusBarItem(TypeMoq.It.isValue(StatusBarAlignment.Left), TypeMoq.It.isValue(100)), + ) + .returns(() => statusBar.object); + } pathUtils.setup((p) => p.getDisplayName(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns((p) => p); await createInterpreterDisplay(); - }); - - teardown(() => { - sinon.restore(); - }); - - async function createInterpreterDisplay(filters: IInterpreterStatusbarVisibilityFilter[] = []) { - interpreterDisplay = new InterpreterDisplay(serviceContainer.object); - await interpreterDisplay.activate(); - filters.forEach((f) => interpreterDisplay.registerVisibilityFilter(f)); } + function setupWorkspaceFolder(resource: Uri, workspaceFolder?: Uri) { if (workspaceFolder) { const mockFolder = TypeMoq.Mock.ofType(); @@ -114,215 +118,253 @@ suite('Interpreters Display', () => { workspaceService.setup((w) => w.getWorkspaceFolder(TypeMoq.It.isValue(resource))).returns(() => undefined); } } - test('Statusbar must be created and have command name initialized', () => { - statusBar.verify((s) => (s.command = TypeMoq.It.isValue('python.setInterpreter')), TypeMoq.Times.once()); - expect(disposableRegistry).to.be.lengthOf.above(0); - expect(disposableRegistry).contain(statusBar.object); - }); - test('Display name and tooltip must come from interpreter info', async () => { - const resource = Uri.file('x'); - const workspaceFolder = Uri.file('workspace'); - const activeInterpreter: PythonEnvironment = { - ...info, - displayName: 'Dummy_Display_Name', - envType: EnvironmentType.Unknown, - path: path.join('user', 'development', 'env', 'bin', 'python'), - }; - setupWorkspaceFolder(resource, workspaceFolder); - interpreterService.setup((i) => i.getInterpreters(TypeMoq.It.isValue(workspaceFolder))).returns(() => []); - interpreterService - .setup((i) => i.getActiveInterpreter(TypeMoq.It.isValue(workspaceFolder))) - .returns(() => Promise.resolve(activeInterpreter)); - - await interpreterDisplay.refresh(resource); - - statusBar.verify((s) => (s.text = TypeMoq.It.isValue(activeInterpreter.displayName)!), TypeMoq.Times.once()); - statusBar.verify((s) => (s.tooltip = TypeMoq.It.isValue(activeInterpreter.path)!), TypeMoq.Times.atLeastOnce()); - }); - test('Log the output channel if displayed needs to be updated with a new interpreter', async () => { - const resource = Uri.file('x'); - const workspaceFolder = Uri.file('workspace'); - const activeInterpreter: PythonEnvironment = { - ...info, - displayName: 'Dummy_Display_Name', - envType: EnvironmentType.Unknown, - path: path.join('user', 'development', 'env', 'bin', 'python'), - }; - pathUtils - .setup((p) => p.getDisplayName(TypeMoq.It.isAny(), TypeMoq.It.isAny())) - .returns(() => activeInterpreter.path); - setupWorkspaceFolder(resource, workspaceFolder); - interpreterService.setup((i) => i.getInterpreters(TypeMoq.It.isValue(workspaceFolder))).returns(() => []); - interpreterService - .setup((i) => i.getActiveInterpreter(TypeMoq.It.isValue(workspaceFolder))) - .returns(() => Promise.resolve(activeInterpreter)); - - await interpreterDisplay.refresh(resource); - traceLogStub.calledOnceWithExactly(Interpreters.pythonInterpreterPath().format(activeInterpreter.path)); - }); - test('If interpreter is not identified then tooltip should point to python Path', async () => { - const resource = Uri.file('x'); - const pythonPath = path.join('user', 'development', 'env', 'bin', 'python'); - const workspaceFolder = Uri.file('workspace'); - const displayName = 'This is the display name'; - - setupWorkspaceFolder(resource, workspaceFolder); - const pythonInterpreter: PythonEnvironment = ({ - displayName, - path: pythonPath, - } as any) as PythonEnvironment; - interpreterService - .setup((i) => i.getActiveInterpreter(TypeMoq.It.isValue(workspaceFolder))) - .returns(() => Promise.resolve(pythonInterpreter)); - - await interpreterDisplay.refresh(resource); - - statusBar.verify((s) => (s.tooltip = TypeMoq.It.isValue(pythonPath)), TypeMoq.Times.atLeastOnce()); - statusBar.verify((s) => (s.text = TypeMoq.It.isValue(displayName)), TypeMoq.Times.once()); - }); - test('If interpreter file does not exist then update status bar accordingly', async () => { - const resource = Uri.file('x'); - const pythonPath = path.join('user', 'development', 'env', 'bin', 'python'); - const workspaceFolder = Uri.file('workspace'); - setupWorkspaceFolder(resource, workspaceFolder); - - interpreterService - .setup((i) => i.getInterpreters(TypeMoq.It.isValue(workspaceFolder))) - .returns(() => [{} as any]); - interpreterService - .setup((i) => i.getActiveInterpreter(TypeMoq.It.isValue(workspaceFolder))) - .returns(() => Promise.resolve(undefined)); - fileSystem.setup((f) => f.fileExists(TypeMoq.It.isValue(pythonPath))).returns(() => Promise.resolve(false)); - interpreterHelper - .setup((v) => v.getInterpreterInformation(TypeMoq.It.isValue(pythonPath))) - .returns(() => Promise.resolve(undefined)); - - await interpreterDisplay.refresh(resource); - - statusBar.verify((s) => (s.color = TypeMoq.It.isValue('')), TypeMoq.Times.once()); - statusBar.verify( - (s) => (s.text = TypeMoq.It.isValue('$(alert) Select Python Interpreter')), - TypeMoq.Times.once(), - ); - }); - test('Ensure we try to identify the active workspace when a resource is not provided ', async () => { - const workspaceFolder = Uri.file('x'); - const resource = workspaceFolder; - const pythonPath = path.join('user', 'development', 'env', 'bin', 'python'); - const activeInterpreter: PythonEnvironment = { - ...info, - displayName: 'Dummy_Display_Name', - envType: EnvironmentType.Unknown, - companyDisplayName: 'Company Name', - path: pythonPath, - }; - fileSystem.setup((fs) => fs.fileExists(TypeMoq.It.isAny())).returns(() => Promise.resolve(true)); - interpreterService - .setup((i) => i.getActiveInterpreter(TypeMoq.It.isValue(resource))) - .returns(() => Promise.resolve(activeInterpreter)) - .verifiable(TypeMoq.Times.once()); - interpreterHelper - .setup((i) => i.getActiveWorkspaceUri(undefined)) - .returns(() => { - return { folderUri: workspaceFolder, configTarget: ConfigurationTarget.Workspace }; - }) - .verifiable(TypeMoq.Times.once()); - - await interpreterDisplay.refresh(); - - interpreterHelper.verifyAll(); - interpreterService.verifyAll(); - statusBar.verify((s) => (s.text = TypeMoq.It.isValue(activeInterpreter.displayName)!), TypeMoq.Times.once()); - statusBar.verify((s) => (s.tooltip = TypeMoq.It.isValue(pythonPath)!), TypeMoq.Times.atLeastOnce()); - }); - suite('Visibility', () => { - const resource = Uri.file('x'); - setup(() => { - const workspaceFolder = Uri.file('workspace'); - const activeInterpreter: PythonEnvironment = { - ...info, - displayName: 'Dummy_Display_Name', - envType: EnvironmentType.Unknown, - path: path.join('user', 'development', 'env', 'bin', 'python'), - }; - setupWorkspaceFolder(resource, workspaceFolder); - interpreterService.setup((i) => i.getInterpreters(TypeMoq.It.isValue(workspaceFolder))).returns(() => []); - interpreterService - .setup((i) => i.getActiveInterpreter(TypeMoq.It.isValue(workspaceFolder))) - .returns(() => Promise.resolve(activeInterpreter)); - }); - test('Status bar must be displayed', async () => { - await interpreterDisplay.refresh(resource); - - statusBar.verify((s) => s.show(), TypeMoq.Times.once()); - statusBar.verify((s) => s.hide(), TypeMoq.Times.never()); - }); - test('Status bar must not be displayed if a filter is registered that needs it to be hidden', async () => { - const filter1: IInterpreterStatusbarVisibilityFilter = { hidden: true }; - const filter2: IInterpreterStatusbarVisibilityFilter = { hidden: false }; - createInterpreterDisplay([filter1, filter2]); - - await interpreterDisplay.refresh(resource); - - statusBar.verify((s) => s.show(), TypeMoq.Times.never()); - statusBar.verify((s) => s.hide(), TypeMoq.Times.once()); - }); - test('Status bar must not be displayed if both filters need it to be hidden', async () => { - const filter1: IInterpreterStatusbarVisibilityFilter = { hidden: true }; - const filter2: IInterpreterStatusbarVisibilityFilter = { hidden: true }; - createInterpreterDisplay([filter1, filter2]); - - await interpreterDisplay.refresh(resource); - - statusBar.verify((s) => s.show(), TypeMoq.Times.never()); - statusBar.verify((s) => s.hide(), TypeMoq.Times.once()); - }); - test('Status bar must be displayed if both filter needs it to be displayed', async () => { - const filter1: IInterpreterStatusbarVisibilityFilter = { hidden: false }; - const filter2: IInterpreterStatusbarVisibilityFilter = { hidden: false }; - createInterpreterDisplay([filter1, filter2]); - - await interpreterDisplay.refresh(resource); - - statusBar.verify((s) => s.show(), TypeMoq.Times.once()); - statusBar.verify((s) => s.hide(), TypeMoq.Times.never()); - }); - test('Status bar must hidden if a filter triggers need for status bar to be hidden', async () => { - const event1 = new EventEmitter(); - const filter1: ReadWrite = { hidden: false, changed: event1.event }; - const event2 = new EventEmitter(); - const filter2: ReadWrite = { hidden: false, changed: event2.event }; - createInterpreterDisplay([filter1, filter2]); - - await interpreterDisplay.refresh(resource); - - statusBar.verify((s) => s.show(), TypeMoq.Times.once()); - statusBar.verify((s) => s.hide(), TypeMoq.Times.never()); - - // Filter one will now want the status bar to get hidden. - statusBar.reset(); - filter1.hidden = true; - event1.fire(); - - statusBar.verify((s) => s.show(), TypeMoq.Times.never()); - statusBar.verify((s) => s.hide(), TypeMoq.Times.once()); - - // Filter two now needs it to be displayed. - statusBar.reset(); - event2.fire(); - - // No changes. - statusBar.verify((s) => s.show(), TypeMoq.Times.never()); - statusBar.verify((s) => s.hide(), TypeMoq.Times.once()); - - // Filter two now needs it to be displayed & filter 1 will allow it to be displayed. - filter1.hidden = false; - statusBar.reset(); - event2.fire(); - - // No changes. - statusBar.verify((s) => s.show(), TypeMoq.Times.once()); - statusBar.verify((s) => s.hide(), TypeMoq.Times.never()); + [true, false].forEach((inExperiment) => { + suite(`When ${inExperiment ? 'in experiment' : 'not in experiment'}`, () => { + setup(async () => { + setupMocks(inExperiment); + }); + + teardown(() => { + sinon.restore(); + }); + test('Statusbar must be created and have command name initialized', () => { + statusBar.verify( + (s) => (s.command = TypeMoq.It.isValue('python.setInterpreter')), + TypeMoq.Times.once(), + ); + expect(disposableRegistry).to.be.lengthOf.above(0); + expect(disposableRegistry).contain(statusBar.object); + }); + test('Display name and tooltip must come from interpreter info', async () => { + const resource = Uri.file('x'); + const workspaceFolder = Uri.file('workspace'); + const activeInterpreter: PythonEnvironment = { + ...info, + displayName: 'Dummy_Display_Name', + envType: EnvironmentType.Unknown, + path: path.join('user', 'development', 'env', 'bin', 'python'), + }; + setupWorkspaceFolder(resource, workspaceFolder); + interpreterService + .setup((i) => i.getInterpreters(TypeMoq.It.isValue(workspaceFolder))) + .returns(() => []); + interpreterService + .setup((i) => i.getActiveInterpreter(TypeMoq.It.isValue(workspaceFolder))) + .returns(() => Promise.resolve(activeInterpreter)); + + await interpreterDisplay.refresh(resource); + + statusBar.verify( + (s) => (s.text = TypeMoq.It.isValue(activeInterpreter.displayName)!), + TypeMoq.Times.once(), + ); + statusBar.verify( + (s) => (s.tooltip = TypeMoq.It.isValue(activeInterpreter.path)!), + TypeMoq.Times.atLeastOnce(), + ); + }); + test('Log the output channel if displayed needs to be updated with a new interpreter', async () => { + const resource = Uri.file('x'); + const workspaceFolder = Uri.file('workspace'); + const activeInterpreter: PythonEnvironment = { + ...info, + displayName: 'Dummy_Display_Name', + envType: EnvironmentType.Unknown, + path: path.join('user', 'development', 'env', 'bin', 'python'), + }; + pathUtils + .setup((p) => p.getDisplayName(TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns(() => activeInterpreter.path); + setupWorkspaceFolder(resource, workspaceFolder); + interpreterService + .setup((i) => i.getInterpreters(TypeMoq.It.isValue(workspaceFolder))) + .returns(() => []); + interpreterService + .setup((i) => i.getActiveInterpreter(TypeMoq.It.isValue(workspaceFolder))) + .returns(() => Promise.resolve(activeInterpreter)); + + await interpreterDisplay.refresh(resource); + traceLogStub.calledOnceWithExactly(Interpreters.pythonInterpreterPath().format(activeInterpreter.path)); + }); + test('If interpreter is not identified then tooltip should point to python Path', async () => { + const resource = Uri.file('x'); + const pythonPath = path.join('user', 'development', 'env', 'bin', 'python'); + const workspaceFolder = Uri.file('workspace'); + const displayName = 'Python 3.10.1'; + const expectedDisplayName = inExperiment ? '3.10.1' : 'Python 3.10.1'; + + setupWorkspaceFolder(resource, workspaceFolder); + const pythonInterpreter: PythonEnvironment = ({ + displayName, + path: pythonPath, + } as any) as PythonEnvironment; + interpreterService + .setup((i) => i.getActiveInterpreter(TypeMoq.It.isValue(workspaceFolder))) + .returns(() => Promise.resolve(pythonInterpreter)); + + await interpreterDisplay.refresh(resource); + + statusBar.verify((s) => (s.tooltip = TypeMoq.It.isValue(pythonPath)), TypeMoq.Times.atLeastOnce()); + statusBar.verify((s) => (s.text = TypeMoq.It.isValue(expectedDisplayName)), TypeMoq.Times.once()); + }); + test('If interpreter file does not exist then update status bar accordingly', async () => { + const resource = Uri.file('x'); + const pythonPath = path.join('user', 'development', 'env', 'bin', 'python'); + const workspaceFolder = Uri.file('workspace'); + setupWorkspaceFolder(resource, workspaceFolder); + + interpreterService + .setup((i) => i.getInterpreters(TypeMoq.It.isValue(workspaceFolder))) + .returns(() => [{} as any]); + interpreterService + .setup((i) => i.getActiveInterpreter(TypeMoq.It.isValue(workspaceFolder))) + .returns(() => Promise.resolve(undefined)); + fileSystem + .setup((f) => f.fileExists(TypeMoq.It.isValue(pythonPath))) + .returns(() => Promise.resolve(false)); + interpreterHelper + .setup((v) => v.getInterpreterInformation(TypeMoq.It.isValue(pythonPath))) + .returns(() => Promise.resolve(undefined)); + + await interpreterDisplay.refresh(resource); + + statusBar.verify((s) => (s.color = TypeMoq.It.isValue('')), TypeMoq.Times.once()); + statusBar.verify( + (s) => (s.text = TypeMoq.It.isValue('$(alert) Select Python Interpreter')), + TypeMoq.Times.once(), + ); + }); + test('Ensure we try to identify the active workspace when a resource is not provided ', async () => { + const workspaceFolder = Uri.file('x'); + const resource = workspaceFolder; + const pythonPath = path.join('user', 'development', 'env', 'bin', 'python'); + const activeInterpreter: PythonEnvironment = { + ...info, + displayName: 'Dummy_Display_Name', + envType: EnvironmentType.Unknown, + companyDisplayName: 'Company Name', + path: pythonPath, + }; + fileSystem.setup((fs) => fs.fileExists(TypeMoq.It.isAny())).returns(() => Promise.resolve(true)); + interpreterService + .setup((i) => i.getActiveInterpreter(TypeMoq.It.isValue(resource))) + .returns(() => Promise.resolve(activeInterpreter)) + .verifiable(TypeMoq.Times.once()); + interpreterHelper + .setup((i) => i.getActiveWorkspaceUri(undefined)) + .returns(() => { + return { folderUri: workspaceFolder, configTarget: ConfigurationTarget.Workspace }; + }) + .verifiable(TypeMoq.Times.once()); + + await interpreterDisplay.refresh(); + + interpreterHelper.verifyAll(); + interpreterService.verifyAll(); + statusBar.verify( + (s) => (s.text = TypeMoq.It.isValue(activeInterpreter.displayName)!), + TypeMoq.Times.once(), + ); + statusBar.verify((s) => (s.tooltip = TypeMoq.It.isValue(pythonPath)!), TypeMoq.Times.atLeastOnce()); + }); + suite('Visibility', () => { + const resource = Uri.file('x'); + setup(() => { + const workspaceFolder = Uri.file('workspace'); + const activeInterpreter: PythonEnvironment = { + ...info, + displayName: 'Dummy_Display_Name', + envType: EnvironmentType.Unknown, + path: path.join('user', 'development', 'env', 'bin', 'python'), + }; + setupWorkspaceFolder(resource, workspaceFolder); + interpreterService + .setup((i) => i.getInterpreters(TypeMoq.It.isValue(workspaceFolder))) + .returns(() => []); + interpreterService + .setup((i) => i.getActiveInterpreter(TypeMoq.It.isValue(workspaceFolder))) + .returns(() => Promise.resolve(activeInterpreter)); + }); + test('Status bar must be displayed', async () => { + await interpreterDisplay.refresh(resource); + + statusBar.verify((s) => s.show(), TypeMoq.Times.once()); + statusBar.verify((s) => s.hide(), TypeMoq.Times.never()); + }); + test('Status bar must not be displayed if a filter is registered that needs it to be hidden', async () => { + const filter1: IInterpreterStatusbarVisibilityFilter = { hidden: true }; + const filter2: IInterpreterStatusbarVisibilityFilter = { hidden: false }; + createInterpreterDisplay([filter1, filter2]); + + await interpreterDisplay.refresh(resource); + + statusBar.verify((s) => s.show(), TypeMoq.Times.never()); + statusBar.verify((s) => s.hide(), TypeMoq.Times.once()); + }); + test('Status bar must not be displayed if both filters need it to be hidden', async () => { + const filter1: IInterpreterStatusbarVisibilityFilter = { hidden: true }; + const filter2: IInterpreterStatusbarVisibilityFilter = { hidden: true }; + createInterpreterDisplay([filter1, filter2]); + + await interpreterDisplay.refresh(resource); + + statusBar.verify((s) => s.show(), TypeMoq.Times.never()); + statusBar.verify((s) => s.hide(), TypeMoq.Times.once()); + }); + test('Status bar must be displayed if both filter needs it to be displayed', async () => { + const filter1: IInterpreterStatusbarVisibilityFilter = { hidden: false }; + const filter2: IInterpreterStatusbarVisibilityFilter = { hidden: false }; + createInterpreterDisplay([filter1, filter2]); + + await interpreterDisplay.refresh(resource); + + statusBar.verify((s) => s.show(), TypeMoq.Times.once()); + statusBar.verify((s) => s.hide(), TypeMoq.Times.never()); + }); + test('Status bar must hidden if a filter triggers need for status bar to be hidden', async () => { + const event1 = new EventEmitter(); + const filter1: ReadWrite = { + hidden: false, + changed: event1.event, + }; + const event2 = new EventEmitter(); + const filter2: ReadWrite = { + hidden: false, + changed: event2.event, + }; + createInterpreterDisplay([filter1, filter2]); + + await interpreterDisplay.refresh(resource); + + statusBar.verify((s) => s.show(), TypeMoq.Times.once()); + statusBar.verify((s) => s.hide(), TypeMoq.Times.never()); + + // Filter one will now want the status bar to get hidden. + statusBar.reset(); + filter1.hidden = true; + event1.fire(); + + statusBar.verify((s) => s.show(), TypeMoq.Times.never()); + statusBar.verify((s) => s.hide(), TypeMoq.Times.once()); + + // Filter two now needs it to be displayed. + statusBar.reset(); + event2.fire(); + + // No changes. + statusBar.verify((s) => s.show(), TypeMoq.Times.never()); + statusBar.verify((s) => s.hide(), TypeMoq.Times.once()); + + // Filter two now needs it to be displayed & filter 1 will allow it to be displayed. + filter1.hidden = false; + statusBar.reset(); + event2.fire(); + + // No changes. + statusBar.verify((s) => s.show(), TypeMoq.Times.once()); + statusBar.verify((s) => s.hide(), TypeMoq.Times.never()); + }); + }); }); }); }); From 9e490981b06d8e2f3d3aa47bb0f0d7979f0906af Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 13 Jan 2022 00:22:21 +0530 Subject: [PATCH 6/8] Add unpinned language status item experiment --- .../common/application/applicationShell.ts | 6 ++ src/client/common/application/types.ts | 2 + src/client/interpreter/display/index.ts | 96 +++++++++++++------ 3 files changed, 73 insertions(+), 31 deletions(-) diff --git a/src/client/common/application/applicationShell.ts b/src/client/common/application/applicationShell.ts index be16859ecb1f..1f243682f18c 100644 --- a/src/client/common/application/applicationShell.ts +++ b/src/client/common/application/applicationShell.ts @@ -7,10 +7,13 @@ import { CancellationToken, CancellationTokenSource, Disposable, + DocumentSelector, env, Event, InputBox, InputBoxOptions, + languages, + LanguageStatusItem, MessageItem, MessageOptions, OpenDialogOptions, @@ -149,4 +152,7 @@ export class ApplicationShell implements IApplicationShell { public createOutputChannel(name: string): OutputChannel { return window.createOutputChannel(name); } + public createLanguageStatusItem(id: string, selector: DocumentSelector): LanguageStatusItem { + return languages.createLanguageStatusItem(id, selector); + } } diff --git a/src/client/common/application/types.ts b/src/client/common/application/types.ts index 4b55301fd8d7..c8335e288447 100644 --- a/src/client/common/application/types.ts +++ b/src/client/common/application/types.ts @@ -24,6 +24,7 @@ import { GlobPattern, InputBox, InputBoxOptions, + LanguageStatusItem, MessageItem, MessageOptions, OpenDialogOptions, @@ -416,6 +417,7 @@ export interface IApplicationShell { * @param name Human-readable string which will be used to represent the channel in the UI. */ createOutputChannel(name: string): OutputChannel; + createLanguageStatusItem(id: string, selector: DocumentSelector): LanguageStatusItem; } export const ICommandManager = Symbol('ICommandManager'); diff --git a/src/client/interpreter/display/index.ts b/src/client/interpreter/display/index.ts index 4df784b15068..6bf018cbf376 100644 --- a/src/client/interpreter/display/index.ts +++ b/src/client/interpreter/display/index.ts @@ -1,12 +1,12 @@ import { inject, injectable } from 'inversify'; -import { Disposable, StatusBarAlignment, StatusBarItem, Uri } from 'vscode'; +import { Disposable, LanguageStatusItem, LanguageStatusSeverity, StatusBarAlignment, StatusBarItem, Uri } from 'vscode'; import { IExtensionSingleActivationService } from '../../activation/types'; import { IApplicationShell, IWorkspaceService } from '../../common/application/types'; -import { Commands } from '../../common/constants'; +import { Commands, PYTHON_LANGUAGE } from '../../common/constants'; import { InterpreterStatusBarPosition } from '../../common/experiments/groups'; import '../../common/extensions'; import { IDisposableRegistry, IExperimentService, IPathUtils, Resource } from '../../common/types'; -import { Interpreters } from '../../common/utils/localize'; +import { InterpreterQuickPickList, Interpreters } from '../../common/utils/localize'; import { IServiceContainer } from '../../ioc/types'; import { traceLog } from '../../logging'; import { PythonEnvironment } from '../../pythonEnvironments/info'; @@ -28,7 +28,8 @@ export class InterpreterDisplay implements IInterpreterDisplay, IExtensionSingle untrustedWorkspace: false, virtualWorkspace: true, }; - private statusBar!: StatusBarItem; + private statusBar: StatusBarItem | undefined; + private languageStatus: LanguageStatusItem | undefined; private readonly helper: IInterpreterHelper; private readonly workspaceService: IWorkspaceService; private readonly pathUtils: IPathUtils; @@ -58,14 +59,25 @@ export class InterpreterDisplay implements IInterpreterDisplay, IExtensionSingle } public async activate(): Promise { - let [alignment, priority] = [StatusBarAlignment.Left, 100]; - if (this.experiments.inExperimentSync(InterpreterStatusBarPosition.Pinned)) { - [alignment, priority] = [StatusBarAlignment.Right, STATUS_BAR_ITEM_PRIORITY]; - } const application = this.serviceContainer.get(IApplicationShell); - this.statusBar = application.createStatusBarItem(alignment, priority); - this.statusBar.command = Commands.Set_Interpreter; - this.disposableRegistry.push(this.statusBar); + if (this.experiments.inExperimentSync(InterpreterStatusBarPosition.Unpinned)) { + this.languageStatus = application.createLanguageStatusItem('python.selectedInterpreter', { + language: PYTHON_LANGUAGE, + }); + this.languageStatus.severity = LanguageStatusSeverity.Information; + this.languageStatus.command = { + title: InterpreterQuickPickList.browsePath.openButtonLabel(), + command: Commands.Set_Interpreter, + }; + } else { + let [alignment, priority] = [StatusBarAlignment.Left, 100]; + if (this.experiments.inExperimentSync(InterpreterStatusBarPosition.Pinned)) { + [alignment, priority] = [StatusBarAlignment.Right, STATUS_BAR_ITEM_PRIORITY]; + } + this.statusBar = application.createStatusBarItem(alignment, priority); + this.statusBar.command = Commands.Set_Interpreter; + this.disposableRegistry.push(this.statusBar); + } } public async refresh(resource?: Uri) { @@ -94,34 +106,56 @@ export class InterpreterDisplay implements IInterpreterDisplay, IExtensionSingle private async updateDisplay(workspaceFolder?: Uri) { const interpreter = await this.interpreterService.getActiveInterpreter(workspaceFolder); this.currentlySelectedWorkspaceFolder = workspaceFolder; - if (interpreter) { - this.statusBar.color = ''; - this.statusBar.tooltip = this.pathUtils.getDisplayName(interpreter.path, workspaceFolder?.fsPath); - if (this.interpreterPath !== interpreter.path) { - traceLog( - Interpreters.pythonInterpreterPath().format( - this.pathUtils.getDisplayName(interpreter.path, workspaceFolder?.fsPath), - ), - ); - this.interpreterPath = interpreter.path; + if (this.statusBar) { + if (interpreter) { + this.statusBar.color = ''; + this.statusBar.tooltip = this.pathUtils.getDisplayName(interpreter.path, workspaceFolder?.fsPath); + if (this.interpreterPath !== interpreter.path) { + traceLog( + Interpreters.pythonInterpreterPath().format( + this.pathUtils.getDisplayName(interpreter.path, workspaceFolder?.fsPath), + ), + ); + this.interpreterPath = interpreter.path; + } + let text = interpreter.displayName!; + if (this.experiments.inExperimentSync(InterpreterStatusBarPosition.Pinned)) { + text = text.startsWith('Python') ? text.substring('Python'.length).trim() : text; + } + this.statusBar.text = text; + this.currentlySelectedInterpreterPath = interpreter.path; + } else { + this.statusBar.tooltip = ''; + this.statusBar.color = ''; + this.statusBar.text = '$(alert) Select Python Interpreter'; + this.currentlySelectedInterpreterPath = undefined; } - let text = interpreter.displayName!; - if (this.experiments.inExperimentSync(InterpreterStatusBarPosition.Pinned)) { + } else if (this.languageStatus) { + if (interpreter) { + this.languageStatus.detail = this.pathUtils.getDisplayName(interpreter.path, workspaceFolder?.fsPath); + if (this.interpreterPath !== interpreter.path) { + traceLog( + Interpreters.pythonInterpreterPath().format( + this.pathUtils.getDisplayName(interpreter.path, workspaceFolder?.fsPath), + ), + ); + this.interpreterPath = interpreter.path; + } + let text = interpreter.displayName!; text = text.startsWith('Python') ? text.substring('Python'.length).trim() : text; + this.languageStatus.text = text; + this.currentlySelectedInterpreterPath = interpreter.path; + } else { + this.languageStatus.text = '$(alert) No Interpreter Selected'; + this.languageStatus.detail = undefined; + this.currentlySelectedInterpreterPath = undefined; } - this.statusBar.text = text; - this.currentlySelectedInterpreterPath = interpreter.path; - } else { - this.statusBar.tooltip = ''; - this.statusBar.color = ''; - this.statusBar.text = '$(alert) Select Python Interpreter'; - this.currentlySelectedInterpreterPath = undefined; } this.statusBarCanBeDisplayed = true; this.updateVisibility(); } private updateVisibility() { - if (!this.statusBarCanBeDisplayed) { + if (!this.statusBar || !this.statusBarCanBeDisplayed) { return; } if (this.visibilityFilters.length === 0 || this.visibilityFilters.every((filter) => !filter.hidden)) { From 58806383cb4099793b0410d75a8adc65c1342c93 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 13 Jan 2022 01:12:23 +0530 Subject: [PATCH 7/8] Add tests --- src/client/common/experiments/service.ts | 4 - src/client/interpreter/display/index.ts | 1 + src/test/interpreters/display.unit.test.ts | 141 ++++++++++++++++----- src/test/mocks/vsc/index.ts | 6 + src/test/vscode-mock.ts | 1 + 5 files changed, 116 insertions(+), 37 deletions(-) diff --git a/src/client/common/experiments/service.ts b/src/client/common/experiments/service.ts index 879470af56da..fb6c856251e4 100644 --- a/src/client/common/experiments/service.ts +++ b/src/client/common/experiments/service.ts @@ -13,7 +13,6 @@ import { IApplicationEnvironment, IWorkspaceService } from '../application/types import { PVSC_EXTENSION_ID } from '../constants'; import { GLOBAL_MEMENTO, IExperimentService, IMemento } from '../types'; import { Experiments } from '../utils/localize'; -import { InterpreterStatusBarPosition } from './groups'; import { ExperimentationTelemetry } from './telemetry'; const EXP_MEMENTO_KEY = 'VSCode.ABExp.FeatureData'; @@ -109,9 +108,6 @@ export class ExperimentService implements IExperimentService { } public inExperimentSync(experiment: string): boolean { - if (experiment === InterpreterStatusBarPosition.Pinned) { - return true; - } if (!this.experimentationService) { return false; } diff --git a/src/client/interpreter/display/index.ts b/src/client/interpreter/display/index.ts index 6bf018cbf376..0382641e954b 100644 --- a/src/client/interpreter/display/index.ts +++ b/src/client/interpreter/display/index.ts @@ -69,6 +69,7 @@ export class InterpreterDisplay implements IInterpreterDisplay, IExtensionSingle title: InterpreterQuickPickList.browsePath.openButtonLabel(), command: Commands.Set_Interpreter, }; + this.disposableRegistry.push(this.languageStatus); } else { let [alignment, priority] = [StatusBarAlignment.Left, 100]; if (this.experiments.inExperimentSync(InterpreterStatusBarPosition.Pinned)) { diff --git a/src/test/interpreters/display.unit.test.ts b/src/test/interpreters/display.unit.test.ts index a459f6c99ecf..9ee927a7262c 100644 --- a/src/test/interpreters/display.unit.test.ts +++ b/src/test/interpreters/display.unit.test.ts @@ -7,6 +7,8 @@ import { ConfigurationTarget, Disposable, EventEmitter, + LanguageStatusItem, + LanguageStatusSeverity, StatusBarAlignment, StatusBarItem, Uri, @@ -14,10 +16,11 @@ import { } from 'vscode'; import { IExtensionSingleActivationService } from '../../client/activation/types'; import { IApplicationShell, IWorkspaceService } from '../../client/common/application/types'; +import { Commands, PYTHON_LANGUAGE } from '../../client/common/constants'; import { InterpreterStatusBarPosition } from '../../client/common/experiments/groups'; import { IFileSystem } from '../../client/common/platform/types'; import { IDisposableRegistry, IExperimentService, IPathUtils, ReadWrite } from '../../client/common/types'; -import { Interpreters } from '../../client/common/utils/localize'; +import { InterpreterQuickPickList, Interpreters } from '../../client/common/utils/localize'; import { Architecture } from '../../client/common/utils/platform'; import { IInterpreterDisplay, @@ -42,7 +45,7 @@ const info: PythonEnvironment = { sysVersion: '', }; -suite('xInterpreters Display', () => { +suite('Interpreters Display', () => { let applicationShell: TypeMoq.IMock; let workspaceService: TypeMoq.IMock; let serviceContainer: TypeMoq.IMock; @@ -54,6 +57,7 @@ suite('xInterpreters Display', () => { let interpreterHelper: TypeMoq.IMock; let experiments: TypeMoq.IMock; let pathUtils: TypeMoq.IMock; + let languageStatusItem: TypeMoq.IMock; let traceLogStub: sinon.SinonStub; async function createInterpreterDisplay(filters: IInterpreterStatusbarVisibilityFilter[] = []) { interpreterDisplay = new InterpreterDisplay(serviceContainer.object); @@ -61,17 +65,22 @@ suite('xInterpreters Display', () => { filters.forEach((f) => interpreterDisplay.registerVisibilityFilter(f)); } - async function setupMocks(inExperiment = false) { + async function setupMocks(inExperiment: InterpreterStatusBarPosition | undefined) { serviceContainer = TypeMoq.Mock.ofType(); workspaceService = TypeMoq.Mock.ofType(); applicationShell = TypeMoq.Mock.ofType(); interpreterService = TypeMoq.Mock.ofType(); experiments = TypeMoq.Mock.ofType(); - experiments.setup((e) => e.inExperimentSync(InterpreterStatusBarPosition.Pinned)).returns(() => inExperiment); + if (inExperiment) { + experiments.setup((e) => e.inExperimentSync(inExperiment)).returns(() => true); + } else { + experiments.setup((e) => e.inExperimentSync(TypeMoq.It.isAny())).returns(() => false); + } fileSystem = TypeMoq.Mock.ofType(); interpreterHelper = TypeMoq.Mock.ofType(); disposableRegistry = []; statusBar = TypeMoq.Mock.ofType(); + languageStatusItem = TypeMoq.Mock.ofType(); pathUtils = TypeMoq.Mock.ofType(); traceLogStub = sinon.stub(logging, 'traceLog'); @@ -92,10 +101,16 @@ suite('xInterpreters Display', () => { .setup((c) => c.get(TypeMoq.It.isValue(IInterpreterHelper))) .returns(() => interpreterHelper.object); serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IPathUtils))).returns(() => pathUtils.object); - if (inExperiment) { + if (inExperiment === InterpreterStatusBarPosition.Pinned) { applicationShell .setup((a) => a.createStatusBarItem(TypeMoq.It.isValue(StatusBarAlignment.Right), TypeMoq.It.isAny())) .returns(() => statusBar.object); + } else if (inExperiment === InterpreterStatusBarPosition.Unpinned) { + applicationShell + .setup((a) => + a.createLanguageStatusItem(TypeMoq.It.isAny(), TypeMoq.It.isValue({ language: PYTHON_LANGUAGE })), + ) + .returns(() => languageStatusItem.object); } else { applicationShell .setup((a) => @@ -118,8 +133,8 @@ suite('xInterpreters Display', () => { workspaceService.setup((w) => w.getWorkspaceFolder(TypeMoq.It.isValue(resource))).returns(() => undefined); } } - [true, false].forEach((inExperiment) => { - suite(`When ${inExperiment ? 'in experiment' : 'not in experiment'}`, () => { + [InterpreterStatusBarPosition.Unpinned, InterpreterStatusBarPosition.Pinned, undefined].forEach((inExperiment) => { + suite(`When ${inExperiment ? `in experiment ${inExperiment}` : 'not in experiment'}`, () => { setup(async () => { setupMocks(inExperiment); }); @@ -128,12 +143,28 @@ suite('xInterpreters Display', () => { sinon.restore(); }); test('Statusbar must be created and have command name initialized', () => { - statusBar.verify( - (s) => (s.command = TypeMoq.It.isValue('python.setInterpreter')), - TypeMoq.Times.once(), - ); + if (inExperiment === InterpreterStatusBarPosition.Unpinned) { + languageStatusItem.verify( + (s) => (s.severity = TypeMoq.It.isValue(LanguageStatusSeverity.Information)), + TypeMoq.Times.once(), + ); + languageStatusItem.verify( + (s) => + (s.command = TypeMoq.It.isValue({ + title: InterpreterQuickPickList.browsePath.openButtonLabel(), + command: Commands.Set_Interpreter, + })), + TypeMoq.Times.once(), + ); + expect(disposableRegistry).contain(languageStatusItem.object); + } else { + statusBar.verify( + (s) => (s.command = TypeMoq.It.isValue('python.setInterpreter')), + TypeMoq.Times.once(), + ); + expect(disposableRegistry).contain(statusBar.object); + } expect(disposableRegistry).to.be.lengthOf.above(0); - expect(disposableRegistry).contain(statusBar.object); }); test('Display name and tooltip must come from interpreter info', async () => { const resource = Uri.file('x'); @@ -154,14 +185,25 @@ suite('xInterpreters Display', () => { await interpreterDisplay.refresh(resource); - statusBar.verify( - (s) => (s.text = TypeMoq.It.isValue(activeInterpreter.displayName)!), - TypeMoq.Times.once(), - ); - statusBar.verify( - (s) => (s.tooltip = TypeMoq.It.isValue(activeInterpreter.path)!), - TypeMoq.Times.atLeastOnce(), - ); + if (inExperiment === InterpreterStatusBarPosition.Unpinned) { + languageStatusItem.verify( + (s) => (s.text = TypeMoq.It.isValue(activeInterpreter.displayName)!), + TypeMoq.Times.once(), + ); + languageStatusItem.verify( + (s) => (s.detail = TypeMoq.It.isValue(activeInterpreter.path)!), + TypeMoq.Times.atLeastOnce(), + ); + } else { + statusBar.verify( + (s) => (s.text = TypeMoq.It.isValue(activeInterpreter.displayName)!), + TypeMoq.Times.once(), + ); + statusBar.verify( + (s) => (s.tooltip = TypeMoq.It.isValue(activeInterpreter.path)!), + TypeMoq.Times.atLeastOnce(), + ); + } }); test('Log the output channel if displayed needs to be updated with a new interpreter', async () => { const resource = Uri.file('x'); @@ -203,9 +245,19 @@ suite('xInterpreters Display', () => { .returns(() => Promise.resolve(pythonInterpreter)); await interpreterDisplay.refresh(resource); - - statusBar.verify((s) => (s.tooltip = TypeMoq.It.isValue(pythonPath)), TypeMoq.Times.atLeastOnce()); - statusBar.verify((s) => (s.text = TypeMoq.It.isValue(expectedDisplayName)), TypeMoq.Times.once()); + if (inExperiment === InterpreterStatusBarPosition.Unpinned) { + languageStatusItem.verify( + (s) => (s.detail = TypeMoq.It.isValue(pythonPath)), + TypeMoq.Times.atLeastOnce(), + ); + languageStatusItem.verify( + (s) => (s.text = TypeMoq.It.isValue(expectedDisplayName)), + TypeMoq.Times.once(), + ); + } else { + statusBar.verify((s) => (s.tooltip = TypeMoq.It.isValue(pythonPath)), TypeMoq.Times.atLeastOnce()); + statusBar.verify((s) => (s.text = TypeMoq.It.isValue(expectedDisplayName)), TypeMoq.Times.once()); + } }); test('If interpreter file does not exist then update status bar accordingly', async () => { const resource = Uri.file('x'); @@ -228,11 +280,18 @@ suite('xInterpreters Display', () => { await interpreterDisplay.refresh(resource); - statusBar.verify((s) => (s.color = TypeMoq.It.isValue('')), TypeMoq.Times.once()); - statusBar.verify( - (s) => (s.text = TypeMoq.It.isValue('$(alert) Select Python Interpreter')), - TypeMoq.Times.once(), - ); + if (inExperiment === InterpreterStatusBarPosition.Unpinned) { + languageStatusItem.verify( + (s) => (s.text = TypeMoq.It.isValue('$(alert) No Interpreter Selected')), + TypeMoq.Times.once(), + ); + } else { + statusBar.verify((s) => (s.color = TypeMoq.It.isValue('')), TypeMoq.Times.once()); + statusBar.verify( + (s) => (s.text = TypeMoq.It.isValue('$(alert) Select Python Interpreter')), + TypeMoq.Times.once(), + ); + } }); test('Ensure we try to identify the active workspace when a resource is not provided ', async () => { const workspaceFolder = Uri.file('x'); @@ -261,14 +320,30 @@ suite('xInterpreters Display', () => { interpreterHelper.verifyAll(); interpreterService.verifyAll(); - statusBar.verify( - (s) => (s.text = TypeMoq.It.isValue(activeInterpreter.displayName)!), - TypeMoq.Times.once(), - ); - statusBar.verify((s) => (s.tooltip = TypeMoq.It.isValue(pythonPath)!), TypeMoq.Times.atLeastOnce()); + if (inExperiment === InterpreterStatusBarPosition.Unpinned) { + languageStatusItem.verify( + (s) => (s.text = TypeMoq.It.isValue(activeInterpreter.displayName)!), + TypeMoq.Times.once(), + ); + languageStatusItem.verify( + (s) => (s.detail = TypeMoq.It.isValue(pythonPath)!), + TypeMoq.Times.atLeastOnce(), + ); + } else { + statusBar.verify( + (s) => (s.text = TypeMoq.It.isValue(activeInterpreter.displayName)!), + TypeMoq.Times.once(), + ); + statusBar.verify((s) => (s.tooltip = TypeMoq.It.isValue(pythonPath)!), TypeMoq.Times.atLeastOnce()); + } }); suite('Visibility', () => { const resource = Uri.file('x'); + suiteSetup(function () { + if (inExperiment === InterpreterStatusBarPosition.Unpinned) { + return this.skip(); + } + }); setup(() => { const workspaceFolder = Uri.file('workspace'); const activeInterpreter: PythonEnvironment = { diff --git a/src/test/mocks/vsc/index.ts b/src/test/mocks/vsc/index.ts index a0ac4549669a..bdffedc2ead3 100644 --- a/src/test/mocks/vsc/index.ts +++ b/src/test/mocks/vsc/index.ts @@ -29,6 +29,12 @@ export enum ExtensionKind { Workspace = 2, } +export enum LanguageStatusSeverity { + Information = 0, + Warning = 1, + Error = 2, +} + export class Disposable { constructor(private callOnDispose: () => void) {} diff --git a/src/test/vscode-mock.ts b/src/test/vscode-mock.ts index b458935231ab..53a255a184d4 100644 --- a/src/test/vscode-mock.ts +++ b/src/test/vscode-mock.ts @@ -106,6 +106,7 @@ mockedVSCode.QuickInputButtons = vscodeMocks.vscMockExtHostedTypes.QuickInputBut mockedVSCode.FileType = vscodeMocks.FileType; mockedVSCode.UIKind = vscodeMocks.UIKind; mockedVSCode.FileSystemError = vscodeMocks.vscMockExtHostedTypes.FileSystemError; +mockedVSCode.LanguageStatusSeverity = vscodeMocks.LanguageStatusSeverity; (mockedVSCode as any).NotebookCellKind = vscodeMocks.vscMockExtHostedTypes.NotebookCellKind; (mockedVSCode as any).CellOutputKind = vscodeMocks.vscMockExtHostedTypes.CellOutputKind; (mockedVSCode as any).NotebookCellRunState = vscodeMocks.vscMockExtHostedTypes.NotebookCellRunState; From eeddb063aa1dc39e29fcc139b95dbf5f04edc4a8 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 13 Jan 2022 14:39:27 +0530 Subject: [PATCH 8/8] Add news entry --- news/1 Enhancements/18282.md | 1 + news/1 Enhancements/18283.md | 1 + 2 files changed, 2 insertions(+) create mode 100644 news/1 Enhancements/18282.md create mode 100644 news/1 Enhancements/18283.md diff --git a/news/1 Enhancements/18282.md b/news/1 Enhancements/18282.md new file mode 100644 index 000000000000..a75cc521f1db --- /dev/null +++ b/news/1 Enhancements/18282.md @@ -0,0 +1 @@ +Move pinned interpreter status bar item towards the right behind `pythonInterpreterInfoPinned` experiment. diff --git a/news/1 Enhancements/18283.md b/news/1 Enhancements/18283.md new file mode 100644 index 000000000000..92cdc62e4f03 --- /dev/null +++ b/news/1 Enhancements/18283.md @@ -0,0 +1 @@ +Move interpreter status bar item into the `Python` language status item behind `pythonInterpreterInfoUnpinned` experiment.