From 04e0b3441647a894d73ba5bb36e45550b79de46f Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Tue, 5 May 2020 21:15:32 -0700 Subject: [PATCH 1/6] Fix path --- src/client/activation/node/languageClientFactory.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/activation/node/languageClientFactory.ts b/src/client/activation/node/languageClientFactory.ts index 8490c4910041..d607afdd5d31 100644 --- a/src/client/activation/node/languageClientFactory.ts +++ b/src/client/activation/node/languageClientFactory.ts @@ -20,7 +20,7 @@ export class NodeLanguageClientFactory implements ILanguageClientFactory { constructor( @inject(IFileSystem) private readonly fs: IFileSystem, @inject(ILanguageServerFolderService) private readonly languageServerFolderService: ILanguageServerFolderService - ) {} + ) { } public async createLanguageClient( resource: Resource, From 4477900742b0dea66a9c1219a8e16b5254f2f69d Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Thu, 18 Jun 2020 11:40:57 -0700 Subject: [PATCH 2/6] Actually fix settings --- src/client/testing/common/updateTestSettings.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/client/testing/common/updateTestSettings.ts b/src/client/testing/common/updateTestSettings.ts index 1f5c79faf32c..1a309a66c1b5 100644 --- a/src/client/testing/common/updateTestSettings.ts +++ b/src/client/testing/common/updateTestSettings.ts @@ -31,7 +31,7 @@ export class UpdateTestSettingService implements IExtensionActivationService { const filesToBeFixed = await this.getFilesToBeFixed(resource); await Promise.all(filesToBeFixed.map((file) => this.fixSettingInFile(file))); } - public getSettingsFiles(resource: Resource) { + public getSettingsFiles(resource: Resource): string[] { const settingsFiles: string[] = []; if (this.application.userSettingsFile) { settingsFiles.push(this.application.userSettingsFile); @@ -42,7 +42,7 @@ export class UpdateTestSettingService implements IExtensionActivationService { } return settingsFiles; } - public async getFilesToBeFixed(resource: Resource) { + public async getFilesToBeFixed(resource: Resource): Promise { const files = this.getSettingsFiles(resource); const result = await Promise.all( files.map(async (file) => { @@ -87,10 +87,11 @@ export class UpdateTestSettingService implements IExtensionActivationService { return fileContents; } - public async doesFileNeedToBeFixed(filePath: string) { + public async doesFileNeedToBeFixed(filePath: string): Promise { try { const contents = await this.fs.readFile(filePath); return ( + contents.indexOf('python.jediEnabled') > 0 || contents.indexOf('python.unitTest.') > 0 || contents.indexOf('.pyTest') > 0 || contents.indexOf('.pep8') > 0 From d0d50dedd938bb5380c1413c25b6150dc420a938 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Thu, 18 Jun 2020 11:45:41 -0700 Subject: [PATCH 3/6] Add news --- news/2 Fixes/12429.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/2 Fixes/12429.md diff --git a/news/2 Fixes/12429.md b/news/2 Fixes/12429.md new file mode 100644 index 000000000000..4fea2a4f8f5c --- /dev/null +++ b/news/2 Fixes/12429.md @@ -0,0 +1 @@ +Fixed issue when `python.jediEnabled` setting was not removed and `python.languageServer` setting was not updated. From 03aa5f92820ac00a443d1b0cec174f8b0ad5e98c Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Thu, 18 Jun 2020 11:53:02 -0700 Subject: [PATCH 4/6] Add test --- .../diagnostics/checks/updateTestSettings.unit.test.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts b/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts index 819b27b9f8cb..c9a5bd704ee0 100644 --- a/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts +++ b/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts @@ -168,6 +168,12 @@ suite('Application Diagnostics - Check Test Settings', () => { assert.ok(!needsToBeFixed); verify(fs.readFile(__filename)).once(); }); + test('Verify `python.jediEnabled` is found in user settings', async () => { + when(fs.readFile(__filename)).thenResolve('"python.jediEnabled": false'); + const needsToBeFixed = await diagnosticService.doesFileNeedToBeFixed(__filename); + assert.ok(needsToBeFixed); + verify(fs.readFile(__filename)).once(); + }); [ { From e4a032f43211c9082e5ae4f0db96deee6160471f Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Thu, 18 Jun 2020 11:54:54 -0700 Subject: [PATCH 5/6] Format --- src/client/activation/node/languageClientFactory.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/activation/node/languageClientFactory.ts b/src/client/activation/node/languageClientFactory.ts index 8bd72f7d60b0..a8d9cbdab41d 100644 --- a/src/client/activation/node/languageClientFactory.ts +++ b/src/client/activation/node/languageClientFactory.ts @@ -20,7 +20,7 @@ export class NodeLanguageClientFactory implements ILanguageClientFactory { constructor( @inject(IFileSystem) private readonly fs: IFileSystem, @inject(ILanguageServerFolderService) private readonly languageServerFolderService: ILanguageServerFolderService - ) { } + ) {} public async createLanguageClient( resource: Resource, From 27eeec76f8519ed0beb45274db2cd9200bde1f26 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Thu, 18 Jun 2020 16:29:44 -0700 Subject: [PATCH 6/6] Suppress 'jediEnabled' removal --- src/client/testing/common/updateTestSettings.ts | 13 +++---------- .../checks/updateTestSettings.unit.test.ts | 12 ++++++------ 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/src/client/testing/common/updateTestSettings.ts b/src/client/testing/common/updateTestSettings.ts index 1a309a66c1b5..b7b60aad8ad8 100644 --- a/src/client/testing/common/updateTestSettings.ts +++ b/src/client/testing/common/updateTestSettings.ts @@ -107,13 +107,13 @@ export class UpdateTestSettingService implements IExtensionActivationService { // - `true` or missing then set to `languageServer: Jedi`. // - `false` and `languageServer` is present, do nothing. // - `false` and `languageServer` is NOT present, set `languageServer` to `Microsoft`. - // `jediEnabled` is then removed. + // `jediEnabled` is NOT removed since JSONC parser may also remove comments. const jediEnabledPath = ['python.jediEnabled']; const languageServerPath = ['python.languageServer']; try { - let ast = parseTree(fileContent); - let jediEnabledNode = findNodeAtLocation(ast, jediEnabledPath); + const ast = parseTree(fileContent); + const jediEnabledNode = findNodeAtLocation(ast, jediEnabledPath); const jediEnabled = jediEnabledNode ? getNodeValue(jediEnabledNode) : true; const languageServerNode = findNodeAtLocation(ast, languageServerPath); const formattingOptions: FormattingOptions = { @@ -135,13 +135,6 @@ export class UpdateTestSettingService implements IExtensionActivationService { } fileContent = applyEdits(fileContent, edits); - // Remove jediEnabled - ast = parseTree(fileContent); - jediEnabledNode = findNodeAtLocation(ast, jediEnabledPath); - if (jediEnabledNode) { - edits = modify(fileContent, jediEnabledPath, undefined, { formattingOptions }); - fileContent = applyEdits(fileContent, edits); - } // tslint:disable-next-line:no-empty } catch {} return fileContent; diff --git a/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts b/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts index c9a5bd704ee0..d70dd05728ef 100644 --- a/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts +++ b/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts @@ -224,32 +224,32 @@ suite('Application Diagnostics - Check Test Settings', () => { { testTitle: 'jediEnabled: true, no languageServer setting', contents: '{ "python.jediEnabled": true }', - expectedContent: '{"python.languageServer": "Jedi"}' + expectedContent: '{ "python.jediEnabled": true, "python.languageServer": "Jedi"}' }, { testTitle: 'jediEnabled: true, languageServer setting present', contents: '{ "python.jediEnabled": true }', - expectedContent: '{"python.languageServer": "Jedi"}' + expectedContent: '{ "python.jediEnabled": true, "python.languageServer": "Jedi"}' }, { testTitle: 'jediEnabled: false, no languageServer setting', contents: '{ "python.jediEnabled": false }', - expectedContent: '{"python.languageServer": "Microsoft"}' + expectedContent: '{ "python.jediEnabled": false, "python.languageServer": "Microsoft"}' }, { testTitle: 'jediEnabled: false, languageServer is Microsoft', contents: '{ "python.jediEnabled": false, "python.languageServer": "Microsoft" }', - expectedContent: '{"python.languageServer": "Microsoft"}' + expectedContent: '{ "python.jediEnabled": false, "python.languageServer": "Microsoft"}' }, { testTitle: 'jediEnabled: false, languageServer is None', contents: '{ "python.jediEnabled": false, "python.languageServer": "None" }', - expectedContent: '{"python.languageServer": "None"}' + expectedContent: '{ "python.jediEnabled": false, "python.languageServer": "None"}' }, { testTitle: 'jediEnabled: false, languageServer is Jedi', contents: '{ "python.jediEnabled": false, "python.languageServer": "Jedi" }', - expectedContent: '{"python.languageServer": "Jedi"}' + expectedContent: '{ "python.jediEnabled": false, "python.languageServer": "Jedi"}' } ].forEach((item) => { test(item.testTitle, async () => {