From 4ce04ac2ca3a24aad85d697f707f875b0b394c96 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Fri, 19 Aug 2022 16:33:37 -0700 Subject: [PATCH 01/12] WIP --- src/client/activation/node/analysisOptions.ts | 11 +++++++++++ src/client/common/application/workspace.ts | 2 +- src/client/common/configSettings.ts | 6 ++++-- src/client/common/configuration/service.ts | 2 +- src/client/common/types.ts | 1 + .../languageServer/pylanceLSExtensionManager.ts | 2 ++ .../activation/node/analysisOptions.unit.test.ts | 14 ++++++++++++-- 7 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/client/activation/node/analysisOptions.ts b/src/client/activation/node/analysisOptions.ts index f06ed52b7b54..46a8a6befbaa 100644 --- a/src/client/activation/node/analysisOptions.ts +++ b/src/client/activation/node/analysisOptions.ts @@ -3,6 +3,7 @@ import { LanguageClientOptions } from 'vscode-languageclient'; import { IWorkspaceService } from '../../common/application/types'; +import { IConfigurationService, IExperimentService } from '../../common/types'; import { LanguageServerAnalysisOptionsBase } from '../common/analysisOptions'; import { ILanguageServerOutputChannel } from '../types'; @@ -13,6 +14,8 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt constructor( lsOutputChannel: ILanguageServerOutputChannel, workspace: IWorkspaceService, + private readonly configurationService: IConfigurationService, + private readonly experimentService: IExperimentService, private readonly lspNotebooksExperiment: LspNotebooksExperiment, ) { super(lsOutputChannel, workspace); @@ -20,11 +23,19 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt // eslint-disable-next-line class-methods-use-this protected async getInitializationOptions(): Promise { + const inExperiment = await this.experimentService.inExperiment('pylanceAutoIndent'); + const pythonSettings = this.configurationService.getSettings(); + if (pythonSettings.formatOnType === undefined) { + this.configurationService.updateSectionSetting('editor', 'formatOnType', true); + } + const enableAutoIndent = inExperiment && pythonSettings.formatOnType !== false; + return ({ experimentationSupport: true, trustedWorkspaceSupport: true, lspNotebooksSupport: this.lspNotebooksExperiment.isInNotebooksExperiment(), lspInteractiveWindowSupport: this.lspNotebooksExperiment.isInNotebooksExperimentWithInteractiveWindowSupport(), + autoIndentSupport: enableAutoIndent, } as unknown) as LanguageClientOptions; } } diff --git a/src/client/common/application/workspace.ts b/src/client/common/application/workspace.ts index 69d5dff965a3..57965d78ab2f 100644 --- a/src/client/common/application/workspace.ts +++ b/src/client/common/application/workspace.ts @@ -40,7 +40,7 @@ export class WorkspaceService implements IWorkspaceService { return workspace.workspaceFile; } public getConfiguration(section?: string, resource?: Uri): WorkspaceConfiguration { - return workspace.getConfiguration(section, resource || null); + return workspace.getConfiguration(section, { uri: resource, languageId: 'python' }); } public getWorkspaceFolder(uri: Resource): WorkspaceFolder | undefined { return uri ? workspace.getWorkspaceFolder(uri) : undefined; diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index 52ed1cbeda79..c484f972f3c2 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -130,6 +130,8 @@ export class PythonSettings implements IPythonSettings { public languageServerIsDefault = true; + public formatOnType: boolean | undefined = undefined; + protected readonly changed = new EventEmitter(); private static readonly configChanged = new EventEmitter(); @@ -181,8 +183,8 @@ export class PythonSettings implements IPythonSettings { // eslint-disable-next-line @typescript-eslint/no-explicit-any const config = workspace.getConfiguration('editor', resource || (null as any)); - const formatOnType = config ? config.get('formatOnType', false) : false; - sendTelemetryEvent(EventName.FORMAT_ON_TYPE, undefined, { enabled: formatOnType }); + settings.formatOnType = config ? config.get('formatOnType', undefined) : undefined; + sendTelemetryEvent(EventName.FORMAT_ON_TYPE, undefined, { enabled: settings.formatOnType ?? false }); } return PythonSettings.pythonSettings.get(workspaceFolderKey)!; diff --git a/src/client/common/configuration/service.ts b/src/client/common/configuration/service.ts index 219c8727ca16..f4280ace9a46 100644 --- a/src/client/common/configuration/service.ts +++ b/src/client/common/configuration/service.ts @@ -66,7 +66,7 @@ export class ConfigurationService implements IConfigurationService { ) { return; } - await configSection.update(setting, value, configTarget); + await configSection.update(setting, value, configTarget, /* overrideInLanguage */ true); await this.verifySetting(configSection, configTarget, setting, value); } diff --git a/src/client/common/types.ts b/src/client/common/types.ts index 571a9a01b8a2..88e40bd39b51 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -202,6 +202,7 @@ export interface IPythonSettings { readonly languageServerIsDefault: boolean; readonly defaultInterpreterPath: string; readonly tensorBoard: ITensorBoardSettings | undefined; + readonly formatOnType: boolean | undefined; register(): void; } diff --git a/src/client/languageServer/pylanceLSExtensionManager.ts b/src/client/languageServer/pylanceLSExtensionManager.ts index faa1bb75c4bc..8fdfaec3e522 100644 --- a/src/client/languageServer/pylanceLSExtensionManager.ts +++ b/src/client/languageServer/pylanceLSExtensionManager.ts @@ -58,6 +58,8 @@ export class PylanceLSExtensionManager extends LanguageServerCapabilities this.analysisOptions = new NodeLanguageServerAnalysisOptions( outputChannel, workspaceService, + configurationService, + experimentService, lspNotebooksExperiment, ); this.clientFactory = new NodeLanguageClientFactory(fileSystem, extensions); diff --git a/src/test/activation/node/analysisOptions.unit.test.ts b/src/test/activation/node/analysisOptions.unit.test.ts index 0518fac170e9..d663aca96f2c 100644 --- a/src/test/activation/node/analysisOptions.unit.test.ts +++ b/src/test/activation/node/analysisOptions.unit.test.ts @@ -10,7 +10,7 @@ import { LspNotebooksExperiment } from '../../../client/activation/node/lspNoteb import { ILanguageServerOutputChannel } from '../../../client/activation/types'; import { IWorkspaceService } from '../../../client/common/application/types'; import { PYTHON, PYTHON_LANGUAGE } from '../../../client/common/constants'; -import { IOutputChannel } from '../../../client/common/types'; +import { IConfigurationService, IExperimentService, IOutputChannel } from '../../../client/common/types'; suite('Pylance Language Server - Analysis Options', () => { class TestClass extends NodeLanguageServerAnalysisOptions { @@ -32,6 +32,8 @@ suite('Pylance Language Server - Analysis Options', () => { let outputChannel: IOutputChannel; let lsOutputChannel: typemoq.IMock; let workspace: typemoq.IMock; + let configurationService: IConfigurationService; + let experimentService: IExperimentService; let lspNotebooksExperiment: typemoq.IMock; setup(() => { @@ -40,9 +42,17 @@ suite('Pylance Language Server - Analysis Options', () => { workspace.setup((w) => w.isVirtualWorkspace).returns(() => false); lsOutputChannel = typemoq.Mock.ofType(); lsOutputChannel.setup((l) => l.channel).returns(() => outputChannel); + configurationService = typemoq.Mock.ofType().object; + experimentService = typemoq.Mock.ofType().object; lspNotebooksExperiment = typemoq.Mock.ofType(); lspNotebooksExperiment.setup((l) => l.isInNotebooksExperiment()).returns(() => false); - analysisOptions = new TestClass(lsOutputChannel.object, workspace.object, lspNotebooksExperiment.object); + analysisOptions = new TestClass( + lsOutputChannel.object, + workspace.object, + configurationService, + experimentService, + lspNotebooksExperiment.object, + ); }); test('Workspace folder is undefined', () => { From caea5778d9b06942348194ad1d705ce9411c5071 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Fri, 19 Aug 2022 21:30:31 -0700 Subject: [PATCH 02/12] Update check and set working --- src/client/activation/node/analysisOptions.ts | 40 ++++++++++++++----- src/client/common/application/types.ts | 3 +- src/client/common/application/workspace.ts | 12 +++++- src/client/common/configSettings.ts | 6 +-- src/client/common/configuration/service.ts | 2 +- src/client/common/types.ts | 1 - .../pylanceLSExtensionManager.ts | 1 - .../node/analysisOptions.unit.test.ts | 5 +-- 8 files changed, 46 insertions(+), 24 deletions(-) diff --git a/src/client/activation/node/analysisOptions.ts b/src/client/activation/node/analysisOptions.ts index 46a8a6befbaa..e5003112b334 100644 --- a/src/client/activation/node/analysisOptions.ts +++ b/src/client/activation/node/analysisOptions.ts @@ -1,9 +1,10 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +import { ConfigurationTarget } from 'vscode'; import { LanguageClientOptions } from 'vscode-languageclient'; import { IWorkspaceService } from '../../common/application/types'; -import { IConfigurationService, IExperimentService } from '../../common/types'; +import { IExperimentService } from '../../common/types'; import { LanguageServerAnalysisOptionsBase } from '../common/analysisOptions'; import { ILanguageServerOutputChannel } from '../types'; @@ -14,7 +15,6 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt constructor( lsOutputChannel: ILanguageServerOutputChannel, workspace: IWorkspaceService, - private readonly configurationService: IConfigurationService, private readonly experimentService: IExperimentService, private readonly lspNotebooksExperiment: LspNotebooksExperiment, ) { @@ -23,19 +23,39 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt // eslint-disable-next-line class-methods-use-this protected async getInitializationOptions(): Promise { - const inExperiment = await this.experimentService.inExperiment('pylanceAutoIndent'); - const pythonSettings = this.configurationService.getSettings(); - if (pythonSettings.formatOnType === undefined) { - this.configurationService.updateSectionSetting('editor', 'formatOnType', true); - } - const enableAutoIndent = inExperiment && pythonSettings.formatOnType !== false; - return ({ experimentationSupport: true, trustedWorkspaceSupport: true, lspNotebooksSupport: this.lspNotebooksExperiment.isInNotebooksExperiment(), lspInteractiveWindowSupport: this.lspNotebooksExperiment.isInNotebooksExperimentWithInteractiveWindowSupport(), - autoIndentSupport: enableAutoIndent, + autoIndentSupport: await this.isAutoIndentEnabled(), } as unknown) as LanguageClientOptions; } + + private async isAutoIndentEnabled() { + const editorConfig = this.workspace.getConfiguration('editor', undefined, /* languageSpecific */ true); + const formatOnTypeSetting = 'formatOnType'; + const formatOnTypeEffectiveValue = editorConfig.get(formatOnTypeSetting); + const formatOnTypeInspect = editorConfig.inspect(formatOnTypeSetting); + + let formatOnTypeSetForPython = false; + if (formatOnTypeInspect?.languageIds) { + formatOnTypeSetForPython = formatOnTypeInspect.languageIds.indexOf('python') >= 0; + } + + let enableAutoIndent = formatOnTypeEffectiveValue; + + const inExperiment = await this.experimentService.inExperiment('pylanceAutoIndent'); + if (inExperiment && !formatOnTypeEffectiveValue && !formatOnTypeSetForPython) { + editorConfig.update( + formatOnTypeSetting, + /* value */ true, + ConfigurationTarget.Global, + /* overrideInLanguage */ true, + ); + enableAutoIndent = true; + } + + return enableAutoIndent; + } } diff --git a/src/client/common/application/types.ts b/src/client/common/application/types.ts index a91aeed75b04..256404810237 100644 --- a/src/client/common/application/types.ts +++ b/src/client/common/application/types.ts @@ -837,9 +837,10 @@ export interface IWorkspaceService { * * @param section A dot-separated identifier. * @param resource A resource for which the configuration is asked for + * @param languageSpecific Should the [python] language-specific settings be obtained? * @return The full configuration or a subset. */ - getConfiguration(section?: string, resource?: Uri): WorkspaceConfiguration; + getConfiguration(section?: string, resource?: Uri, languageSpecific?: boolean): WorkspaceConfiguration; /** * Opens an untitled text document. The editor will prompt the user for a file diff --git a/src/client/common/application/workspace.ts b/src/client/common/application/workspace.ts index 57965d78ab2f..0a5fd8d81816 100644 --- a/src/client/common/application/workspace.ts +++ b/src/client/common/application/workspace.ts @@ -39,8 +39,16 @@ export class WorkspaceService implements IWorkspaceService { public get workspaceFile() { return workspace.workspaceFile; } - public getConfiguration(section?: string, resource?: Uri): WorkspaceConfiguration { - return workspace.getConfiguration(section, { uri: resource, languageId: 'python' }); + public getConfiguration( + section?: string, + resource?: Uri, + languageSpecific: boolean = false, + ): WorkspaceConfiguration { + if (languageSpecific) { + return workspace.getConfiguration(section, { uri: resource, languageId: 'python' }); + } else { + return workspace.getConfiguration(section, resource); + } } public getWorkspaceFolder(uri: Resource): WorkspaceFolder | undefined { return uri ? workspace.getWorkspaceFolder(uri) : undefined; diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index c484f972f3c2..52ed1cbeda79 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -130,8 +130,6 @@ export class PythonSettings implements IPythonSettings { public languageServerIsDefault = true; - public formatOnType: boolean | undefined = undefined; - protected readonly changed = new EventEmitter(); private static readonly configChanged = new EventEmitter(); @@ -183,8 +181,8 @@ export class PythonSettings implements IPythonSettings { // eslint-disable-next-line @typescript-eslint/no-explicit-any const config = workspace.getConfiguration('editor', resource || (null as any)); - settings.formatOnType = config ? config.get('formatOnType', undefined) : undefined; - sendTelemetryEvent(EventName.FORMAT_ON_TYPE, undefined, { enabled: settings.formatOnType ?? false }); + const formatOnType = config ? config.get('formatOnType', false) : false; + sendTelemetryEvent(EventName.FORMAT_ON_TYPE, undefined, { enabled: formatOnType }); } return PythonSettings.pythonSettings.get(workspaceFolderKey)!; diff --git a/src/client/common/configuration/service.ts b/src/client/common/configuration/service.ts index f4280ace9a46..219c8727ca16 100644 --- a/src/client/common/configuration/service.ts +++ b/src/client/common/configuration/service.ts @@ -66,7 +66,7 @@ export class ConfigurationService implements IConfigurationService { ) { return; } - await configSection.update(setting, value, configTarget, /* overrideInLanguage */ true); + await configSection.update(setting, value, configTarget); await this.verifySetting(configSection, configTarget, setting, value); } diff --git a/src/client/common/types.ts b/src/client/common/types.ts index 88e40bd39b51..571a9a01b8a2 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -202,7 +202,6 @@ export interface IPythonSettings { readonly languageServerIsDefault: boolean; readonly defaultInterpreterPath: string; readonly tensorBoard: ITensorBoardSettings | undefined; - readonly formatOnType: boolean | undefined; register(): void; } diff --git a/src/client/languageServer/pylanceLSExtensionManager.ts b/src/client/languageServer/pylanceLSExtensionManager.ts index 8fdfaec3e522..2cc74308feea 100644 --- a/src/client/languageServer/pylanceLSExtensionManager.ts +++ b/src/client/languageServer/pylanceLSExtensionManager.ts @@ -58,7 +58,6 @@ export class PylanceLSExtensionManager extends LanguageServerCapabilities this.analysisOptions = new NodeLanguageServerAnalysisOptions( outputChannel, workspaceService, - configurationService, experimentService, lspNotebooksExperiment, ); diff --git a/src/test/activation/node/analysisOptions.unit.test.ts b/src/test/activation/node/analysisOptions.unit.test.ts index d663aca96f2c..6ff72512e02f 100644 --- a/src/test/activation/node/analysisOptions.unit.test.ts +++ b/src/test/activation/node/analysisOptions.unit.test.ts @@ -10,7 +10,7 @@ import { LspNotebooksExperiment } from '../../../client/activation/node/lspNoteb import { ILanguageServerOutputChannel } from '../../../client/activation/types'; import { IWorkspaceService } from '../../../client/common/application/types'; import { PYTHON, PYTHON_LANGUAGE } from '../../../client/common/constants'; -import { IConfigurationService, IExperimentService, IOutputChannel } from '../../../client/common/types'; +import { IExperimentService, IOutputChannel } from '../../../client/common/types'; suite('Pylance Language Server - Analysis Options', () => { class TestClass extends NodeLanguageServerAnalysisOptions { @@ -32,7 +32,6 @@ suite('Pylance Language Server - Analysis Options', () => { let outputChannel: IOutputChannel; let lsOutputChannel: typemoq.IMock; let workspace: typemoq.IMock; - let configurationService: IConfigurationService; let experimentService: IExperimentService; let lspNotebooksExperiment: typemoq.IMock; @@ -42,14 +41,12 @@ suite('Pylance Language Server - Analysis Options', () => { workspace.setup((w) => w.isVirtualWorkspace).returns(() => false); lsOutputChannel = typemoq.Mock.ofType(); lsOutputChannel.setup((l) => l.channel).returns(() => outputChannel); - configurationService = typemoq.Mock.ofType().object; experimentService = typemoq.Mock.ofType().object; lspNotebooksExperiment = typemoq.Mock.ofType(); lspNotebooksExperiment.setup((l) => l.isInNotebooksExperiment()).returns(() => false); analysisOptions = new TestClass( lsOutputChannel.object, workspace.object, - configurationService, experimentService, lspNotebooksExperiment.object, ); From a48d7e0d47720cc43394177a34a696c96e8490fc Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Fri, 19 Aug 2022 21:43:44 -0700 Subject: [PATCH 03/12] Unset formatOnType if user is not in experiment --- src/client/activation/node/analysisOptions.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/client/activation/node/analysisOptions.ts b/src/client/activation/node/analysisOptions.ts index e5003112b334..5d22b9ac7ffd 100644 --- a/src/client/activation/node/analysisOptions.ts +++ b/src/client/activation/node/analysisOptions.ts @@ -54,6 +54,13 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt /* overrideInLanguage */ true, ); enableAutoIndent = true; + } else if (!inExperiment) { + editorConfig.update( + formatOnTypeSetting, + /* value */ undefined, + ConfigurationTarget.Global, + /* overrideInLanguage */ true, + ); } return enableAutoIndent; From d1438acb1d97f4799481f797d342c518dee8136b Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Mon, 22 Aug 2022 14:25:24 -0700 Subject: [PATCH 04/12] Add missing awaits --- src/client/activation/node/analysisOptions.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/activation/node/analysisOptions.ts b/src/client/activation/node/analysisOptions.ts index 5d22b9ac7ffd..dcd577f06c45 100644 --- a/src/client/activation/node/analysisOptions.ts +++ b/src/client/activation/node/analysisOptions.ts @@ -47,7 +47,7 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt const inExperiment = await this.experimentService.inExperiment('pylanceAutoIndent'); if (inExperiment && !formatOnTypeEffectiveValue && !formatOnTypeSetForPython) { - editorConfig.update( + await editorConfig.update( formatOnTypeSetting, /* value */ true, ConfigurationTarget.Global, @@ -55,7 +55,7 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt ); enableAutoIndent = true; } else if (!inExperiment) { - editorConfig.update( + await editorConfig.update( formatOnTypeSetting, /* value */ undefined, ConfigurationTarget.Global, From 8d94f3a71e3a5f92327b1c89bbfd4a48dae8850d Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Mon, 22 Aug 2022 15:11:08 -0700 Subject: [PATCH 05/12] Fix analysisOptions unit tests --- src/test/activation/node/analysisOptions.unit.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/test/activation/node/analysisOptions.unit.test.ts b/src/test/activation/node/analysisOptions.unit.test.ts index 6ff72512e02f..8d16f0c0d9c9 100644 --- a/src/test/activation/node/analysisOptions.unit.test.ts +++ b/src/test/activation/node/analysisOptions.unit.test.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import { assert, expect } from 'chai'; import * as typemoq from 'typemoq'; -import { WorkspaceFolder } from 'vscode'; +import { WorkspaceConfiguration, WorkspaceFolder } from 'vscode'; import { DocumentFilter } from 'vscode-languageclient/node'; import { NodeLanguageServerAnalysisOptions } from '../../../client/activation/node/analysisOptions'; @@ -39,6 +39,9 @@ suite('Pylance Language Server - Analysis Options', () => { outputChannel = typemoq.Mock.ofType().object; workspace = typemoq.Mock.ofType(); workspace.setup((w) => w.isVirtualWorkspace).returns(() => false); + const workspaceConfig = typemoq.Mock.ofType(); + workspace.setup((w) => w.getConfiguration('editor', undefined, true)).returns(() => workspaceConfig.object); + workspaceConfig.setup((w) => w.get('formatOnType')).returns(() => true); lsOutputChannel = typemoq.Mock.ofType(); lsOutputChannel.setup((l) => l.channel).returns(() => outputChannel); experimentService = typemoq.Mock.ofType().object; From 6f7e8b8e7d00579cb78146442b993351be98d082 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Mon, 22 Aug 2022 16:51:28 -0700 Subject: [PATCH 06/12] Simplify experiment logic --- src/client/activation/node/analysisOptions.ts | 37 +++++++++++-------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/src/client/activation/node/analysisOptions.ts b/src/client/activation/node/analysisOptions.ts index dcd577f06c45..e0377f88219b 100644 --- a/src/client/activation/node/analysisOptions.ts +++ b/src/client/activation/node/analysisOptions.ts @@ -10,6 +10,9 @@ import { LanguageServerAnalysisOptionsBase } from '../common/analysisOptions'; import { ILanguageServerOutputChannel } from '../types'; import { LspNotebooksExperiment } from './lspNotebooksExperiment'; +const editorConfigSection = 'editor'; +const formatOnTypeConfigSetting = 'formatOnType'; + export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOptionsBase { // eslint-disable-next-line @typescript-eslint/no-useless-constructor constructor( @@ -33,36 +36,38 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt } private async isAutoIndentEnabled() { - const editorConfig = this.workspace.getConfiguration('editor', undefined, /* languageSpecific */ true); - const formatOnTypeSetting = 'formatOnType'; - const formatOnTypeEffectiveValue = editorConfig.get(formatOnTypeSetting); - const formatOnTypeInspect = editorConfig.inspect(formatOnTypeSetting); + const editorConfig = this.getPythonSpecificEditorSection(); + const formatOnTypeEffectiveValue = editorConfig.get(formatOnTypeConfigSetting); + const formatOnTypeInspect = editorConfig.inspect(formatOnTypeConfigSetting); let formatOnTypeSetForPython = false; if (formatOnTypeInspect?.languageIds) { formatOnTypeSetForPython = formatOnTypeInspect.languageIds.indexOf('python') >= 0; } - let enableAutoIndent = formatOnTypeEffectiveValue; + if (formatOnTypeSetForPython && !formatOnTypeEffectiveValue) { + // User has explicitly disabled auto-indent + return false; + } const inExperiment = await this.experimentService.inExperiment('pylanceAutoIndent'); - if (inExperiment && !formatOnTypeEffectiveValue && !formatOnTypeSetForPython) { - await editorConfig.update( - formatOnTypeSetting, - /* value */ true, - ConfigurationTarget.Global, - /* overrideInLanguage */ true, - ); - enableAutoIndent = true; - } else if (!inExperiment) { + + let enableAutoIndent = formatOnTypeEffectiveValue; + if (inExperiment !== formatOnTypeSetForPython) { await editorConfig.update( - formatOnTypeSetting, - /* value */ undefined, + formatOnTypeConfigSetting, + inExperiment ? true : undefined, ConfigurationTarget.Global, /* overrideInLanguage */ true, ); + + enableAutoIndent = this.getPythonSpecificEditorSection().get(formatOnTypeConfigSetting); } return enableAutoIndent; } + + private getPythonSpecificEditorSection() { + return this.workspace.getConfiguration(editorConfigSection, undefined, /* languageSpecific */ true); + } } From a5be883581a9ecd0c662489587f4fe20b6432e4c Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Mon, 22 Aug 2022 17:31:09 -0700 Subject: [PATCH 07/12] Only enable auto-indent if user is in experiment; simplify language setting check --- src/client/activation/node/analysisOptions.ts | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/client/activation/node/analysisOptions.ts b/src/client/activation/node/analysisOptions.ts index e0377f88219b..24e932da22de 100644 --- a/src/client/activation/node/analysisOptions.ts +++ b/src/client/activation/node/analysisOptions.ts @@ -37,22 +37,12 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt private async isAutoIndentEnabled() { const editorConfig = this.getPythonSpecificEditorSection(); - const formatOnTypeEffectiveValue = editorConfig.get(formatOnTypeConfigSetting); + let formatOnTypeEffectiveValue = editorConfig.get(formatOnTypeConfigSetting); const formatOnTypeInspect = editorConfig.inspect(formatOnTypeConfigSetting); - - let formatOnTypeSetForPython = false; - if (formatOnTypeInspect?.languageIds) { - formatOnTypeSetForPython = formatOnTypeInspect.languageIds.indexOf('python') >= 0; - } - - if (formatOnTypeSetForPython && !formatOnTypeEffectiveValue) { - // User has explicitly disabled auto-indent - return false; - } + const formatOnTypeSetForPython = formatOnTypeInspect?.globalLanguageValue !== undefined; const inExperiment = await this.experimentService.inExperiment('pylanceAutoIndent'); - let enableAutoIndent = formatOnTypeEffectiveValue; if (inExperiment !== formatOnTypeSetForPython) { await editorConfig.update( formatOnTypeConfigSetting, @@ -61,10 +51,10 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt /* overrideInLanguage */ true, ); - enableAutoIndent = this.getPythonSpecificEditorSection().get(formatOnTypeConfigSetting); + formatOnTypeEffectiveValue = this.getPythonSpecificEditorSection().get(formatOnTypeConfigSetting); } - return enableAutoIndent; + return inExperiment && formatOnTypeEffectiveValue; } private getPythonSpecificEditorSection() { From 1763d0fe7d1735dfd77f73a082068dfb6813c3de Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Mon, 22 Aug 2022 17:35:21 -0700 Subject: [PATCH 08/12] Add back explicit opt-out check --- src/client/activation/node/analysisOptions.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/client/activation/node/analysisOptions.ts b/src/client/activation/node/analysisOptions.ts index 24e932da22de..e4106508d5fe 100644 --- a/src/client/activation/node/analysisOptions.ts +++ b/src/client/activation/node/analysisOptions.ts @@ -41,6 +41,11 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt const formatOnTypeInspect = editorConfig.inspect(formatOnTypeConfigSetting); const formatOnTypeSetForPython = formatOnTypeInspect?.globalLanguageValue !== undefined; + if (formatOnTypeInspect?.globalLanguageValue === false) { + // User has explicitly disabled auto-indent + return false; + } + const inExperiment = await this.experimentService.inExperiment('pylanceAutoIndent'); if (inExperiment !== formatOnTypeSetForPython) { From 736c9d62866c20e336754a7b082a88b079d14198 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Mon, 22 Aug 2022 17:56:13 -0700 Subject: [PATCH 09/12] Don't unset formatOnType if it's set to false for Python --- src/client/activation/node/analysisOptions.ts | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/client/activation/node/analysisOptions.ts b/src/client/activation/node/analysisOptions.ts index e4106508d5fe..d6954a96a52a 100644 --- a/src/client/activation/node/analysisOptions.ts +++ b/src/client/activation/node/analysisOptions.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { ConfigurationTarget } from 'vscode'; +import { ConfigurationTarget, WorkspaceConfiguration } from 'vscode'; import { LanguageClientOptions } from 'vscode-languageclient'; import { IWorkspaceService } from '../../common/application/types'; import { IExperimentService } from '../../common/types'; @@ -41,20 +41,14 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt const formatOnTypeInspect = editorConfig.inspect(formatOnTypeConfigSetting); const formatOnTypeSetForPython = formatOnTypeInspect?.globalLanguageValue !== undefined; - if (formatOnTypeInspect?.globalLanguageValue === false) { - // User has explicitly disabled auto-indent - return false; - } - const inExperiment = await this.experimentService.inExperiment('pylanceAutoIndent'); if (inExperiment !== formatOnTypeSetForPython) { - await editorConfig.update( - formatOnTypeConfigSetting, - inExperiment ? true : undefined, - ConfigurationTarget.Global, - /* overrideInLanguage */ true, - ); + if (inExperiment) { + await NodeLanguageServerAnalysisOptions.setPythonSpecificFormatOnType(editorConfig, true); + } else if (formatOnTypeInspect?.globalLanguageValue !== false) { + await NodeLanguageServerAnalysisOptions.setPythonSpecificFormatOnType(editorConfig, undefined); + } formatOnTypeEffectiveValue = this.getPythonSpecificEditorSection().get(formatOnTypeConfigSetting); } @@ -65,4 +59,16 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt private getPythonSpecificEditorSection() { return this.workspace.getConfiguration(editorConfigSection, undefined, /* languageSpecific */ true); } + + private static async setPythonSpecificFormatOnType( + editorConfig: WorkspaceConfiguration, + value: boolean | undefined, + ) { + await editorConfig.update( + formatOnTypeConfigSetting, + value, + ConfigurationTarget.Global, + /* overrideInLanguage */ true, + ); + } } From b7b49d6c7dc4b5b09ad9d1a245e389e485b80330 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Mon, 22 Aug 2022 23:30:14 -0700 Subject: [PATCH 10/12] Treat all internal pylance users as having the experiment enabled --- src/client/activation/node/analysisOptions.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/client/activation/node/analysisOptions.ts b/src/client/activation/node/analysisOptions.ts index d6954a96a52a..67f1c96a4e5c 100644 --- a/src/client/activation/node/analysisOptions.ts +++ b/src/client/activation/node/analysisOptions.ts @@ -1,9 +1,11 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { ConfigurationTarget, WorkspaceConfiguration } from 'vscode'; +import { ConfigurationTarget, extensions, WorkspaceConfiguration } from 'vscode'; import { LanguageClientOptions } from 'vscode-languageclient'; +import * as semver from 'semver'; import { IWorkspaceService } from '../../common/application/types'; +import { PYLANCE_EXTENSION_ID } from '../../common/constants'; import { IExperimentService } from '../../common/types'; import { LanguageServerAnalysisOptionsBase } from '../common/analysisOptions'; @@ -41,7 +43,7 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt const formatOnTypeInspect = editorConfig.inspect(formatOnTypeConfigSetting); const formatOnTypeSetForPython = formatOnTypeInspect?.globalLanguageValue !== undefined; - const inExperiment = await this.experimentService.inExperiment('pylanceAutoIndent'); + const inExperiment = await this.isInAutoIndentExperiment(); if (inExperiment !== formatOnTypeSetForPython) { if (inExperiment) { @@ -56,6 +58,15 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt return inExperiment && formatOnTypeEffectiveValue; } + private async isInAutoIndentExperiment(): Promise { + if (await this.experimentService.inExperiment('pylanceAutoIndent')) { + return true; + } + + const pylanceVersion = extensions.getExtension(PYLANCE_EXTENSION_ID)?.packageJSON.version; + return pylanceVersion && semver.prerelease(pylanceVersion)?.includes('dev'); + } + private getPythonSpecificEditorSection() { return this.workspace.getConfiguration(editorConfigSection, undefined, /* languageSpecific */ true); } From edada584fc4f38b7eb299a7a1119e3752a637bd8 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Mon, 22 Aug 2022 23:31:24 -0700 Subject: [PATCH 11/12] Give Pylance the Python-specific view of formatOnType config setting --- .../languageClientMiddlewareBase.ts | 6 ++++ .../node/languageClientMiddleware.ts | 29 +++++++++++++++++-- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/client/activation/languageClientMiddlewareBase.ts b/src/client/activation/languageClientMiddlewareBase.ts index 29a49b8bd35d..67ec24701189 100644 --- a/src/client/activation/languageClientMiddlewareBase.ts +++ b/src/client/activation/languageClientMiddlewareBase.ts @@ -10,6 +10,7 @@ import { Middleware, ResponseError, } from 'vscode-languageclient'; +import { ConfigurationItem } from 'vscode-languageserver-protocol'; import { HiddenFilePrefix } from '../common/constants'; import { IConfigurationService } from '../common/types'; @@ -96,6 +97,8 @@ export class LanguageClientMiddlewareBase implements Middleware { settingDict._envPYTHONPATH = envPYTHONPATH; } } + + this.configurationHook(item, settings[i] as LSPObject); } return settings; @@ -107,6 +110,9 @@ export class LanguageClientMiddlewareBase implements Middleware { return undefined; } + // eslint-disable-next-line class-methods-use-this, @typescript-eslint/no-empty-function + protected configurationHook(_item: ConfigurationItem, _settings: LSPObject): void {} + private get connected(): Promise { return this.connectedPromise.promise; } diff --git a/src/client/activation/node/languageClientMiddleware.ts b/src/client/activation/node/languageClientMiddleware.ts index e1e9cb447bc1..9c1d4c468191 100644 --- a/src/client/activation/node/languageClientMiddleware.ts +++ b/src/client/activation/node/languageClientMiddleware.ts @@ -2,8 +2,8 @@ // Licensed under the MIT License. import { Uri } from 'vscode'; -import { LanguageClient } from 'vscode-languageclient/node'; -import { IJupyterExtensionDependencyManager } from '../../common/application/types'; +import { ConfigurationItem, LanguageClient, LSPObject } from 'vscode-languageclient/node'; +import { IJupyterExtensionDependencyManager, IWorkspaceService } from '../../common/application/types'; import { IServiceContainer } from '../../ioc/types'; import { JupyterExtensionIntegration } from '../../jupyter/jupyterIntegration'; import { traceLog } from '../../logging'; @@ -19,6 +19,8 @@ export class NodeLanguageClientMiddleware extends LanguageClientMiddleware { private readonly jupyterExtensionIntegration: JupyterExtensionIntegration; + private readonly workspaceService: IWorkspaceService; + public constructor( serviceContainer: IServiceContainer, private getClient: () => LanguageClient | undefined, @@ -26,6 +28,8 @@ export class NodeLanguageClientMiddleware extends LanguageClientMiddleware { ) { super(serviceContainer, LanguageServerType.Node, serverVersion); + this.workspaceService = serviceContainer.get(IWorkspaceService); + this.lspNotebooksExperiment = serviceContainer.get(LspNotebooksExperiment); this.setupHidingMiddleware(serviceContainer); @@ -82,4 +86,25 @@ export class NodeLanguageClientMiddleware extends LanguageClientMiddleware { return result; } + + // eslint-disable-next-line class-methods-use-this + protected configurationHook(item: ConfigurationItem, settings: LSPObject): void { + if (item.section === 'editor') { + if (this.workspaceService) { + // Get editor.formatOnType using Python language id so [python] setting + // will be honored if present. + const editorConfig = this.workspaceService.getConfiguration( + item.section, + undefined, + /* languageSpecific */ true, + ); + + const settingDict: LSPObject & { formatOnType?: boolean } = settings as LSPObject & { + formatOnType: boolean; + }; + + settingDict.formatOnType = editorConfig.get('formatOnType'); + } + } + } } From 6dda6bfb5da61b4a9e9ae2886682fcb06bac7723 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Wed, 24 Aug 2022 16:32:21 -0700 Subject: [PATCH 12/12] Fix constant name format --- src/client/activation/node/analysisOptions.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/client/activation/node/analysisOptions.ts b/src/client/activation/node/analysisOptions.ts index 67f1c96a4e5c..a410405d3131 100644 --- a/src/client/activation/node/analysisOptions.ts +++ b/src/client/activation/node/analysisOptions.ts @@ -12,8 +12,8 @@ import { LanguageServerAnalysisOptionsBase } from '../common/analysisOptions'; import { ILanguageServerOutputChannel } from '../types'; import { LspNotebooksExperiment } from './lspNotebooksExperiment'; -const editorConfigSection = 'editor'; -const formatOnTypeConfigSetting = 'formatOnType'; +const EDITOR_CONFIG_SECTION = 'editor'; +const FORMAT_ON_TYPE_CONFIG_SETTING = 'formatOnType'; export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOptionsBase { // eslint-disable-next-line @typescript-eslint/no-useless-constructor @@ -39,8 +39,8 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt private async isAutoIndentEnabled() { const editorConfig = this.getPythonSpecificEditorSection(); - let formatOnTypeEffectiveValue = editorConfig.get(formatOnTypeConfigSetting); - const formatOnTypeInspect = editorConfig.inspect(formatOnTypeConfigSetting); + let formatOnTypeEffectiveValue = editorConfig.get(FORMAT_ON_TYPE_CONFIG_SETTING); + const formatOnTypeInspect = editorConfig.inspect(FORMAT_ON_TYPE_CONFIG_SETTING); const formatOnTypeSetForPython = formatOnTypeInspect?.globalLanguageValue !== undefined; const inExperiment = await this.isInAutoIndentExperiment(); @@ -52,7 +52,7 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt await NodeLanguageServerAnalysisOptions.setPythonSpecificFormatOnType(editorConfig, undefined); } - formatOnTypeEffectiveValue = this.getPythonSpecificEditorSection().get(formatOnTypeConfigSetting); + formatOnTypeEffectiveValue = this.getPythonSpecificEditorSection().get(FORMAT_ON_TYPE_CONFIG_SETTING); } return inExperiment && formatOnTypeEffectiveValue; @@ -68,7 +68,7 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt } private getPythonSpecificEditorSection() { - return this.workspace.getConfiguration(editorConfigSection, undefined, /* languageSpecific */ true); + return this.workspace.getConfiguration(EDITOR_CONFIG_SECTION, undefined, /* languageSpecific */ true); } private static async setPythonSpecificFormatOnType( @@ -76,7 +76,7 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt value: boolean | undefined, ) { await editorConfig.update( - formatOnTypeConfigSetting, + FORMAT_ON_TYPE_CONFIG_SETTING, value, ConfigurationTarget.Global, /* overrideInLanguage */ true,