From 4930e01ba8401787859ae6b21fb4e63cc872ed92 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 20 Jul 2022 15:21:09 -0700 Subject: [PATCH 1/6] Added setting to control when interpreter information is displayed in the status bar --- package.json | 16 ++++++++++++++++ package.nls.json | 4 ++++ src/client/common/configSettings.ts | 5 +++++ src/client/common/types.ts | 4 ++++ 4 files changed, 29 insertions(+) diff --git a/package.json b/package.json index cc8ac85836dc..fd49baae2e3b 100644 --- a/package.json +++ b/package.json @@ -587,6 +587,22 @@ "scope": "resource", "type": "string" }, + "python.interpreter.displayInfo": { + "default": "pythonContext", + "description": "%python.interpreter.displayInfo.description%", + "enum": [ + "never", + "pythonContext", + "always" + ], + "enumDescriptions": [ + "%python.interpreter.displayInfo.never.description%", + "%python.interpreter.displayInfo.pythonContext.description%", + "%python.interpreter.displayInfo.always.description%" + ], + "scope": "machine", + "type": "string" + }, "python.linting.flake8CategorySeverity.W": { "default": "Warning", "description": "%python.linting.flake8CategorySeverity.W.description%", diff --git a/package.nls.json b/package.nls.json index ac2d2176ee0e..b20d6b98fbc7 100644 --- a/package.nls.json +++ b/package.nls.json @@ -57,6 +57,10 @@ "python.linting.flake8Enabled.description": "Whether to lint Python files using flake8.", "python.linting.flake8Path.description": "Path to flake8, you can use a custom version of flake8 by modifying this setting to include the full path.", "python.linting.ignorePatterns.description": "Patterns used to exclude files or folders from being linted.", + "python.interpreter.displayInfo.description": "Controls when to display information of selected interpreter in the status bar.", + "python.interpreter.displayInfo.never.description": "Never display information of selected interpreter in the status bar.", + "python.interpreter.displayInfo.pythonContext.description": "Only display information of selected interpreter in the status bar if Python related files are opened.", + "python.interpreter.displayInfo.always.description": "Always display information of selected interpreter in the status bar.", "python.linting.lintOnSave.description": "Whether to lint Python files when saved.", "python.linting.maxNumberOfProblems.description": "Controls the maximum number of problems produced by the server.", "python.linting.mypyArgs.description": "Arguments passed in. Each argument is a separate item in the array.", diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index f52833b3ea02..59c6c3c32aef 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -30,7 +30,9 @@ import { IExperiments, IFormattingSettings, IInterpreterPathService, + IInterpreterSettings, ILintingSettings, + InterpreterDisplayInfo, IPythonSettings, ISortImportSettings, ITensorBoardSettings, @@ -88,6 +90,8 @@ export class PythonSettings implements IPythonSettings { public venvPath = ''; + public interpreter!: IInterpreterSettings; + public venvFolders: string[] = []; public condaPath = ''; @@ -246,6 +250,7 @@ export class PythonSettings implements IPythonSettings { const poetryPath = systemVariables.resolveAny(pythonSettings.get('poetryPath'))!; this.poetryPath = poetryPath && poetryPath.length > 0 ? getAbsolutePath(poetryPath, workspaceRoot) : poetryPath; + this.interpreter = pythonSettings.get('interpreter') ?? { displayInfo: 'pythonContext' }; // Get as a string and verify; don't just accept. let userLS = pythonSettings.get('languageServer'); userLS = systemVariables.resolveAny(userLS); diff --git a/src/client/common/types.ts b/src/client/common/types.ts index ddeb77f244d1..87d9673ca183 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -179,6 +179,7 @@ export interface ICurrentProcess { } export interface IPythonSettings { + readonly interpreter: IInterpreterSettings; readonly pythonPath: string; readonly venvPath: string; readonly venvFolders: string[]; @@ -233,6 +234,9 @@ export interface IMypyCategorySeverity { readonly error: DiagnosticSeverity; readonly note: DiagnosticSeverity; } +export interface IInterpreterSettings { + displayInfo: 'never' | 'pythonContext' | 'always'; +} export interface ILintingSettings { readonly enabled: boolean; From be3caa9a9b259b4283165aecfdeb4bc02fe024e0 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 20 Jul 2022 15:26:35 -0700 Subject: [PATCH 2/6] Oops --- src/client/common/configSettings.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index 59c6c3c32aef..27709d56b953 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -32,7 +32,6 @@ import { IInterpreterPathService, IInterpreterSettings, ILintingSettings, - InterpreterDisplayInfo, IPythonSettings, ISortImportSettings, ITensorBoardSettings, From f97a242de754c8bb68af103dc0aa6984b8195c50 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 21 Jul 2022 09:29:10 -0700 Subject: [PATCH 3/6] Team reviews --- package.json | 14 +++++++------- package.nls.json | 8 ++++---- src/client/common/configSettings.ts | 4 +++- src/client/common/types.ts | 2 +- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/package.json b/package.json index fd49baae2e3b..0c5bdb9d1aac 100644 --- a/package.json +++ b/package.json @@ -587,18 +587,18 @@ "scope": "resource", "type": "string" }, - "python.interpreter.displayInfo": { - "default": "pythonContext", - "description": "%python.interpreter.displayInfo.description%", + "python.interpreter.infoVisibility": { + "default": "onPythonRelated", + "description": "%python.interpreter.infoVisibility.description%", "enum": [ "never", - "pythonContext", + "onPythonRelated", "always" ], "enumDescriptions": [ - "%python.interpreter.displayInfo.never.description%", - "%python.interpreter.displayInfo.pythonContext.description%", - "%python.interpreter.displayInfo.always.description%" + "%python.interpreter.infoVisibility.never.description%", + "%python.interpreter.infoVisibility.onPythonRelated.description%", + "%python.interpreter.infoVisibility.always.description%" ], "scope": "machine", "type": "string" diff --git a/package.nls.json b/package.nls.json index b20d6b98fbc7..41b7205c7e6e 100644 --- a/package.nls.json +++ b/package.nls.json @@ -57,10 +57,10 @@ "python.linting.flake8Enabled.description": "Whether to lint Python files using flake8.", "python.linting.flake8Path.description": "Path to flake8, you can use a custom version of flake8 by modifying this setting to include the full path.", "python.linting.ignorePatterns.description": "Patterns used to exclude files or folders from being linted.", - "python.interpreter.displayInfo.description": "Controls when to display information of selected interpreter in the status bar.", - "python.interpreter.displayInfo.never.description": "Never display information of selected interpreter in the status bar.", - "python.interpreter.displayInfo.pythonContext.description": "Only display information of selected interpreter in the status bar if Python related files are opened.", - "python.interpreter.displayInfo.always.description": "Always display information of selected interpreter in the status bar.", + "python.interpreter.infoVisibility.description": "Controls when to display information of selected interpreter in the status bar.", + "python.interpreter.infoVisibility.never.description": "Never display information of selected interpreter in the status bar.", + "python.interpreter.infoVisibility.onPythonRelated.description": "Only display information of selected interpreter in the status bar if Python related files are opened.", + "python.interpreter.infoVisibility.always.description": "Always display information of selected interpreter in the status bar.", "python.linting.lintOnSave.description": "Whether to lint Python files when saved.", "python.linting.maxNumberOfProblems.description": "Controls the maximum number of problems produced by the server.", "python.linting.mypyArgs.description": "Arguments passed in. Each argument is a separate item in the array.", diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index 27709d56b953..b7789009c9c7 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -249,7 +249,9 @@ export class PythonSettings implements IPythonSettings { const poetryPath = systemVariables.resolveAny(pythonSettings.get('poetryPath'))!; this.poetryPath = poetryPath && poetryPath.length > 0 ? getAbsolutePath(poetryPath, workspaceRoot) : poetryPath; - this.interpreter = pythonSettings.get('interpreter') ?? { displayInfo: 'pythonContext' }; + this.interpreter = pythonSettings.get('interpreter') ?? { + infoVisibility: 'onPythonRelated', + }; // Get as a string and verify; don't just accept. let userLS = pythonSettings.get('languageServer'); userLS = systemVariables.resolveAny(userLS); diff --git a/src/client/common/types.ts b/src/client/common/types.ts index 87d9673ca183..66ff24a426a4 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -235,7 +235,7 @@ export interface IMypyCategorySeverity { readonly note: DiagnosticSeverity; } export interface IInterpreterSettings { - displayInfo: 'never' | 'pythonContext' | 'always'; + infoVisibility: 'never' | 'onPythonRelated' | 'always'; } export interface ILintingSettings { From c83729b8cba3fa46153e50a285cce59083283565 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 21 Jul 2022 14:30:15 -0700 Subject: [PATCH 4/6] Add feature --- src/client/common/configSettings.ts | 45 +++++++++++++------- src/client/common/configuration/service.ts | 7 ++- src/client/common/types.ts | 3 +- src/client/interpreter/interpreterService.ts | 33 ++++++++++++-- 4 files changed, 68 insertions(+), 20 deletions(-) diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index b7789009c9c7..7d2ef4c08992 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -45,10 +45,15 @@ import { getOSType, OSType } from './utils/platform'; const untildify = require('untildify'); export class PythonSettings implements IPythonSettings { - public get onDidChange(): Event { + private get onDidChange(): Event { return this.changed.event; } + // eslint-disable-next-line class-methods-use-this + public static onConfigChange(): Event { + return PythonSettings.configChanged.event; + } + public get pythonPath(): string { return this._pythonPath; } @@ -125,7 +130,9 @@ export class PythonSettings implements IPythonSettings { public languageServerIsDefault = true; - protected readonly changed = new EventEmitter(); + protected readonly changed = new EventEmitter(); + + private static readonly configChanged = new EventEmitter(); private workspaceRoot: Resource; @@ -169,6 +176,7 @@ export class PythonSettings implements IPythonSettings { defaultLS, ); PythonSettings.pythonSettings.set(workspaceFolderKey, settings); + settings.onDidChange((event) => this.configChanged.fire(event)); // Pass null to avoid VSC from complaining about not passing in a value. // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -518,28 +526,35 @@ export class PythonSettings implements IPythonSettings { this.initialize(); } - public initialize(): void { - const onDidChange = () => { - const currentConfig = this.workspace.getConfiguration('python', this.workspaceRoot); - this.update(currentConfig); + private onDidChanged(event?: ConfigurationChangeEvent) { + const currentConfig = this.workspace.getConfiguration('python', this.workspaceRoot); + this.update(currentConfig); - // If workspace config changes, then we could have a cascading effect of on change events. - // Let's defer the change notification. - this.debounceChangeNotification(); - }; + // If workspace config changes, then we could have a cascading effect of on change events. + // Let's defer the change notification. + this.debounceChangeNotification(event); + } + + public initialize(): void { this.disposables.push(this.workspace.onDidChangeWorkspaceFolders(this.onWorkspaceFoldersChanged, this)); this.disposables.push( - this.interpreterAutoSelectionService.onDidChangeAutoSelectedInterpreter(onDidChange.bind(this)), + this.interpreterAutoSelectionService.onDidChangeAutoSelectedInterpreter(() => { + this.onDidChanged(); + }), ); this.disposables.push( this.workspace.onDidChangeConfiguration((event: ConfigurationChangeEvent) => { if (event.affectsConfiguration('python')) { - onDidChange(); + this.onDidChanged(event); } }), ); if (this.interpreterPathService) { - this.disposables.push(this.interpreterPathService.onDidChange(onDidChange.bind(this))); + this.disposables.push( + this.interpreterPathService.onDidChange(() => { + this.onDidChanged(); + }), + ); } const initialConfig = this.workspace.getConfiguration('python', this.workspaceRoot); @@ -549,8 +564,8 @@ export class PythonSettings implements IPythonSettings { } @debounceSync(1) - protected debounceChangeNotification(): void { - this.changed.fire(); + protected debounceChangeNotification(event?: ConfigurationChangeEvent): void { + this.changed.fire(event); } private getPythonPath(systemVariables: SystemVariables, workspaceRoot: string | undefined) { diff --git a/src/client/common/configuration/service.ts b/src/client/common/configuration/service.ts index 2920e8c14721..219c8727ca16 100644 --- a/src/client/common/configuration/service.ts +++ b/src/client/common/configuration/service.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import { inject, injectable } from 'inversify'; -import { ConfigurationTarget, Uri, WorkspaceConfiguration } from 'vscode'; +import { ConfigurationTarget, Event, Uri, WorkspaceConfiguration, ConfigurationChangeEvent } from 'vscode'; import { IInterpreterAutoSelectionService } from '../../interpreter/autoSelection/types'; import { IServiceContainer } from '../../ioc/types'; import { IWorkspaceService } from '../application/types'; @@ -18,6 +18,11 @@ export class ConfigurationService implements IConfigurationService { this.workspaceService = this.serviceContainer.get(IWorkspaceService); } + // eslint-disable-next-line class-methods-use-this + public get onDidChange(): Event { + return PythonSettings.onConfigChange(); + } + public getSettings(resource?: Uri): IPythonSettings { const InterpreterAutoSelectionService = this.serviceContainer.get( IInterpreterAutoSelectionService, diff --git a/src/client/common/types.ts b/src/client/common/types.ts index 66ff24a426a4..571a9a01b8a2 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -7,6 +7,7 @@ import { Socket } from 'net'; import { Request as RequestResult } from 'request'; import { CancellationToken, + ConfigurationChangeEvent, ConfigurationTarget, DiagnosticSeverity, Disposable, @@ -196,7 +197,6 @@ export interface IPythonSettings { readonly envFile: string; readonly globalModuleInstallation: boolean; readonly pylanceLspNotebooksEnabled: boolean; - readonly onDidChange: Event; readonly experiments: IExperiments; readonly languageServer: LanguageServerType; readonly languageServerIsDefault: boolean; @@ -312,6 +312,7 @@ export interface IAutoCompleteSettings { export const IConfigurationService = Symbol('IConfigurationService'); export interface IConfigurationService { + readonly onDidChange: Event; getSettings(resource?: Uri): IPythonSettings; isTestExecution(): boolean; updateSetting(setting: string, value?: unknown, resource?: Uri, configTarget?: ConfigurationTarget): Promise; diff --git a/src/client/interpreter/interpreterService.ts b/src/client/interpreter/interpreterService.ts index 4d65edcb2e7e..1f563125162d 100644 --- a/src/client/interpreter/interpreterService.ts +++ b/src/client/interpreter/interpreterService.ts @@ -1,7 +1,15 @@ // eslint-disable-next-line max-classes-per-file import { inject, injectable } from 'inversify'; import * as pathUtils from 'path'; -import { Disposable, Event, EventEmitter, ProgressLocation, ProgressOptions, Uri } from 'vscode'; +import { + ConfigurationChangeEvent, + Disposable, + Event, + EventEmitter, + ProgressLocation, + ProgressOptions, + Uri, +} from 'vscode'; import '../common/extensions'; import { IApplicationShell, IDocumentManager } from '../common/application/types'; import { @@ -95,20 +103,39 @@ export class InterpreterService implements Disposable, IInterpreterService { const documentManager = this.serviceContainer.get(IDocumentManager); const interpreterDisplay = this.serviceContainer.get(IInterpreterDisplay); const filter = new (class implements IInterpreterStatusbarVisibilityFilter { - constructor(private readonly docManager: IDocumentManager) {} + constructor( + private readonly docManager: IDocumentManager, + private readonly configService: IConfigurationService, + private readonly disposablesReg: IDisposableRegistry, + ) { + this.disposablesReg.push( + this.configService.onDidChange(async (event: ConfigurationChangeEvent | undefined) => { + if (event?.affectsConfiguration('python.interpreter.infoVisibility')) { + this.interpreterVisibilityEmitter.fire(); + } + }), + ); + } public readonly interpreterVisibilityEmitter = new EventEmitter(); public readonly changed = this.interpreterVisibilityEmitter.event; get hidden() { + const visibility = this.configService.getSettings().interpreter.infoVisibility; + if (visibility === 'never') { + return true; + } + if (visibility === 'always') { + return false; + } const document = this.docManager.activeTextEditor?.document; if (document?.fileName.endsWith('settings.json')) { return false; } return document?.languageId !== PYTHON_LANGUAGE; } - })(documentManager); + })(documentManager, this.configService, disposables); interpreterDisplay.registerVisibilityFilter(filter); disposables.push( this.onDidChangeInterpreters((e): void => { From 21a74728b6a245fce50bd401a59b4d8f45d719fd Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 21 Jul 2022 14:47:20 -0700 Subject: [PATCH 5/6] Debounce change notification --- src/client/common/configSettings.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index 7d2ef4c08992..52ed1cbeda79 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -176,7 +176,7 @@ export class PythonSettings implements IPythonSettings { defaultLS, ); PythonSettings.pythonSettings.set(workspaceFolderKey, settings); - settings.onDidChange((event) => this.configChanged.fire(event)); + settings.onDidChange((event) => PythonSettings.debounceConfigChangeNotification(event)); // Pass null to avoid VSC from complaining about not passing in a value. // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -188,6 +188,12 @@ export class PythonSettings implements IPythonSettings { return PythonSettings.pythonSettings.get(workspaceFolderKey)!; } + @debounceSync(1) + // eslint-disable-next-line class-methods-use-this + protected static debounceConfigChangeNotification(event?: ConfigurationChangeEvent): void { + PythonSettings.configChanged.fire(event); + } + public static getSettingsUriAndTarget( resource: Uri | undefined, workspace?: IWorkspaceService, From 956ddcf9c769b0e136583c48e9bd7c719db62c21 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 21 Jul 2022 14:53:18 -0700 Subject: [PATCH 6/6] Add test --- .../configSettings/configSettings.unit.test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/test/common/configSettings/configSettings.unit.test.ts b/src/test/common/configSettings/configSettings.unit.test.ts index 92757a2f135c..c3c94e5411e4 100644 --- a/src/test/common/configSettings/configSettings.unit.test.ts +++ b/src/test/common/configSettings/configSettings.unit.test.ts @@ -19,6 +19,7 @@ import { IAutoCompleteSettings, IExperiments, IFormattingSettings, + IInterpreterSettings, ILintingSettings, ISortImportSettings, ITerminalSettings, @@ -109,6 +110,7 @@ suite('Python Settings', async () => { config.setup((c) => c.get('devOptions')).returns(() => sourceSettings.devOptions); // complex settings + config.setup((c) => c.get('interpreter')).returns(() => sourceSettings.interpreter); config.setup((c) => c.get('linting')).returns(() => sourceSettings.linting); config.setup((c) => c.get('sortImports')).returns(() => sourceSettings.sortImports); config.setup((c) => c.get('formatting')).returns(() => sourceSettings.formatting); @@ -145,6 +147,21 @@ suite('Python Settings', async () => { }); }); + test('Interpreter settings object', () => { + initializeConfig(expected); + config + .setup((c) => c.get('condaPath')) + .returns(() => expected.condaPath) + .verifiable(TypeMoq.Times.once()); + + settings.update(config.object); + + expect(settings.interpreter).to.deep.equal({ + infoVisibility: 'onPythonRelated', + }); + config.verifyAll(); + }); + test('condaPath updated', () => { expected.pythonPath = 'python3'; expected.condaPath = 'spam';