From 61388545b12ba18afb8af05ce9067f356cae8103 Mon Sep 17 00:00:00 2001 From: Henry Boisdequin <65845077+henryboisdequin@users.noreply.github.com> Date: Sun, 17 Jan 2021 11:13:12 +0530 Subject: [PATCH 1/5] enable/disable linting command --- .vscode/settings.json | 2 +- package.nls.json | 2 +- src/client/linters/linterCommands.ts | 5 +++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index de42fdbe8eb1..7ff9f6d43599 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -41,7 +41,7 @@ "editor.formatOnSave": true }, "typescript.tsdk": "./node_modules/typescript/lib", // we want to use the TS server from our node_modules folder to control its version - "python.linting.enabled": false, + "python.linting.enabled": true, "python.testing.promptToConfigure": false, "python.workspaceSymbols.enabled": false, "python.formatting.provider": "black", diff --git a/package.nls.json b/package.nls.json index bbcd6ecc83e7..3ccb4afbcdcb 100644 --- a/package.nls.json +++ b/package.nls.json @@ -30,7 +30,7 @@ "python.command.python.execSelectionInDjangoShell.title": "Run Selection/Line in Django Shell", "python.command.python.goToPythonObject.title": "Go to Python Object", "python.command.python.setLinter.title": "Select Linter", - "python.command.python.enableLinting.title": "Enable Linting", + "python.command.python.enableLinting.title": "Enable/Disable Linting", "python.command.python.runLinting.title": "Run Linting", "python.command.python.enableSourceMapSupport.title": "Enable Source Map Support For Extension Debugging", "python.command.python.startPage.open.title": "Open Start Page", diff --git a/src/client/linters/linterCommands.ts b/src/client/linters/linterCommands.ts index e93ca15f3197..c423a0f33a22 100644 --- a/src/client/linters/linterCommands.ts +++ b/src/client/linters/linterCommands.ts @@ -81,7 +81,7 @@ export class LinterCommands implements IDisposable { } public async enableLintingAsync(): Promise { - const options = ['on', 'off']; + const options = ['Enable', 'Disable']; const current = (await this.linterManager.isLintingEnabled(true, this.settingsUri)) ? options[0] : options[1]; const quickPickOptions: QuickPickOptions = { @@ -91,8 +91,9 @@ export class LinterCommands implements IDisposable { }; const selection = await this.appShell.showQuickPick(options, quickPickOptions); + if (selection !== undefined) { - const enable = selection === options[0]; + const enable: boolean = selection === options[0]; await this.linterManager.enableLintingAsync(enable, this.settingsUri); } } From 031e88b290609de5ba79cc94fd02e2c4375331b9 Mon Sep 17 00:00:00 2001 From: Henry Boisdequin <65845077+henryboisdequin@users.noreply.github.com> Date: Tue, 19 Jan 2021 06:55:46 +0530 Subject: [PATCH 2/5] tests modified for renaming --- .vscode/settings.json | 2 +- src/test/linters/linterCommands.unit.test.ts | 33 ++++++++++++-------- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 7ff9f6d43599..de42fdbe8eb1 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -41,7 +41,7 @@ "editor.formatOnSave": true }, "typescript.tsdk": "./node_modules/typescript/lib", // we want to use the TS server from our node_modules folder to control its version - "python.linting.enabled": true, + "python.linting.enabled": false, "python.testing.promptToConfigure": false, "python.workspaceSymbols.enabled": false, "python.formatting.provider": "black", diff --git a/src/test/linters/linterCommands.unit.test.ts b/src/test/linters/linterCommands.unit.test.ts index 0043a7cbec3b..318aed6094d8 100644 --- a/src/test/linters/linterCommands.unit.test.ts +++ b/src/test/linters/linterCommands.unit.test.ts @@ -53,12 +53,15 @@ suite('Linting - Linter Commands', () => { expect(result).to.be.equal('Hello'); }); - async function testEnableLintingWithCurrentState(currentState: boolean, selectedState: 'on' | 'off' | undefined) { + async function testEnableLintingWithCurrentState( + currentState: boolean, + selectedState: 'Enabled' | 'Disabled' | undefined, + ) { when(manager.isLintingEnabled(true, anything())).thenResolve(currentState); const expectedQuickPickOptions = { matchOnDetail: true, matchOnDescription: true, - placeHolder: `current: ${currentState ? 'on' : 'off'}`, + placeHolder: `current: ${currentState ? 'Enabled' : 'Disabled'}`, }; when(shell.showQuickPick(anything(), anything())).thenResolve(selectedState as any); @@ -67,29 +70,33 @@ suite('Linting - Linter Commands', () => { verify(shell.showQuickPick(anything(), anything())).once(); const options = capture(shell.showQuickPick).last()[0]; const quickPickOptions = capture(shell.showQuickPick).last()[1]; - expect(options).to.deep.equal(['on', 'off']); + expect(options).to.deep.equal(['Enabled', 'Disabled']); expect(quickPickOptions).to.deep.equal(expectedQuickPickOptions); if (selectedState) { - verify(manager.enableLintingAsync(selectedState === 'on', anything())).once(); + verify(manager.enableLintingAsync(selectedState === 'Enabled', anything())).once(); } else { verify(manager.enableLintingAsync(anything(), anything())).never(); } } - test("Enable linting should check if linting is enabled, and display current state of 'on' and select nothing", async () => { + test("Enable linting should check if linting is enabled, and display current state of 'Enabled' and select nothing", async () => { await testEnableLintingWithCurrentState(true, undefined); }); - test("Enable linting should check if linting is enabled, and display current state of 'on' and select 'on'", async () => { - await testEnableLintingWithCurrentState(true, 'on'); + + test("Enable linting should check if linting is enabled, and display current state of 'Enabled' and select 'Enabled'", async () => { + await testEnableLintingWithCurrentState(true, 'Enabled'); }); - test("Enable linting should check if linting is enabled, and display current state of 'on' and select 'off'", async () => { - await testEnableLintingWithCurrentState(true, 'off'); + + test("Enable linting should check if linting is enabled, and display current state of 'Enabled' and select 'Disabled'", async () => { + await testEnableLintingWithCurrentState(true, 'Disabled'); }); - test("Enable linting should check if linting is enabled, and display current state of 'off' and select 'on'", async () => { - await testEnableLintingWithCurrentState(true, 'on'); + + test("Enable linting should check if linting is enabled, and display current state of 'Disabled' and select 'Enabled'", async () => { + await testEnableLintingWithCurrentState(true, 'Enabled'); }); - test("Enable linting should check if linting is enabled, and display current state of 'off' and select 'off'", async () => { - await testEnableLintingWithCurrentState(true, 'off'); + + test("Enable linting should check if linting is enabled, and display current state of 'Disabled' and select 'Disabled'", async () => { + await testEnableLintingWithCurrentState(true, 'Disabled'); }); test('Set Linter should display a quickpick', async () => { From bd2a3a648781a1af8d43ba7115746dd980b8a31d Mon Sep 17 00:00:00 2001 From: Henry Boisdequin <65845077+henryboisdequin@users.noreply.github.com> Date: Tue, 19 Jan 2021 06:59:29 +0530 Subject: [PATCH 3/5] fixed bug --- src/test/linters/linterCommands.unit.test.ts | 29 +++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/test/linters/linterCommands.unit.test.ts b/src/test/linters/linterCommands.unit.test.ts index 318aed6094d8..cd345db31ba1 100644 --- a/src/test/linters/linterCommands.unit.test.ts +++ b/src/test/linters/linterCommands.unit.test.ts @@ -55,13 +55,13 @@ suite('Linting - Linter Commands', () => { async function testEnableLintingWithCurrentState( currentState: boolean, - selectedState: 'Enabled' | 'Disabled' | undefined, + selectedState: 'Enable' | 'Disable' | undefined, ) { when(manager.isLintingEnabled(true, anything())).thenResolve(currentState); const expectedQuickPickOptions = { matchOnDetail: true, matchOnDescription: true, - placeHolder: `current: ${currentState ? 'Enabled' : 'Disabled'}`, + placeHolder: `current: ${currentState ? 'Enable' : 'Disable'}`, }; when(shell.showQuickPick(anything(), anything())).thenResolve(selectedState as any); @@ -70,33 +70,33 @@ suite('Linting - Linter Commands', () => { verify(shell.showQuickPick(anything(), anything())).once(); const options = capture(shell.showQuickPick).last()[0]; const quickPickOptions = capture(shell.showQuickPick).last()[1]; - expect(options).to.deep.equal(['Enabled', 'Disabled']); + expect(options).to.deep.equal(['Enable', 'Disable']); expect(quickPickOptions).to.deep.equal(expectedQuickPickOptions); if (selectedState) { - verify(manager.enableLintingAsync(selectedState === 'Enabled', anything())).once(); + verify(manager.enableLintingAsync(selectedState === 'Enable', anything())).once(); } else { verify(manager.enableLintingAsync(anything(), anything())).never(); } } - test("Enable linting should check if linting is enabled, and display current state of 'Enabled' and select nothing", async () => { + test("Enable linting should check if linting is enabled, and display current state of 'Enable' and select nothing", async () => { await testEnableLintingWithCurrentState(true, undefined); }); - test("Enable linting should check if linting is enabled, and display current state of 'Enabled' and select 'Enabled'", async () => { - await testEnableLintingWithCurrentState(true, 'Enabled'); + test("Enable linting should check if linting is enabled, and display current state of 'Enable' and select 'Enable'", async () => { + await testEnableLintingWithCurrentState(true, 'Enable'); }); - test("Enable linting should check if linting is enabled, and display current state of 'Enabled' and select 'Disabled'", async () => { - await testEnableLintingWithCurrentState(true, 'Disabled'); + test("Enable linting should check if linting is enabled, and display current state of 'Enable' and select 'Disable'", async () => { + await testEnableLintingWithCurrentState(true, 'Disable'); }); - test("Enable linting should check if linting is enabled, and display current state of 'Disabled' and select 'Enabled'", async () => { - await testEnableLintingWithCurrentState(true, 'Enabled'); + test("Enable linting should check if linting is enabled, and display current state of 'Disable' and select 'Enable'", async () => { + await testEnableLintingWithCurrentState(true, 'Enable'); }); - test("Enable linting should check if linting is enabled, and display current state of 'Disabled' and select 'Disabled'", async () => { - await testEnableLintingWithCurrentState(true, 'Disabled'); + test("Enable linting should check if linting is enabled, and display current state of 'Disable' and select 'Disable'", async () => { + await testEnableLintingWithCurrentState(true, 'Disable'); }); test('Set Linter should display a quickpick', async () => { @@ -115,6 +115,7 @@ suite('Linting - Linter Commands', () => { const quickPickOptions = capture(shell.showQuickPick).last()[1]; expect(quickPickOptions).to.deep.equal(expectedQuickPickOptions); }); + test('Set Linter should display a quickpick and currently active linter when only one is enabled', async () => { const linterId = 'Hello World'; const activeLinters: ILinterInfo[] = [{ id: linterId } as any]; @@ -133,6 +134,7 @@ suite('Linting - Linter Commands', () => { const quickPickOptions = capture(shell.showQuickPick).last()[1]; expect(quickPickOptions).to.deep.equal(expectedQuickPickOptions); }); + test('Set Linter should display a quickpick and with message about multiple linters being enabled', async () => { const activeLinters: ILinterInfo[] = [{ id: 'linterId' } as any, { id: 'linterId2' } as any]; when(manager.getAllLinterInfos()).thenReturn([]); @@ -150,6 +152,7 @@ suite('Linting - Linter Commands', () => { const quickPickOptions = capture(shell.showQuickPick).last()[1]; expect(quickPickOptions).to.deep.equal(expectedQuickPickOptions); }); + test('Selecting a linter should display warning message about multiple linters', async () => { const linters: ILinterInfo[] = [{ id: '1' }, { id: '2' }, { id: '3', product: 'Three' }] as any; const activeLinters: ILinterInfo[] = [{ id: '1' }, { id: '3' }] as any; From bc2143b1608252f52990523bff3d8e7826be7a9c Mon Sep 17 00:00:00 2001 From: Henry Boisdequin <65845077+henryboisdequin@users.noreply.github.com> Date: Wed, 20 Jan 2021 06:36:39 +0530 Subject: [PATCH 4/5] added news --- news/2 Fixes/8800.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/2 Fixes/8800.md diff --git a/news/2 Fixes/8800.md b/news/2 Fixes/8800.md new file mode 100644 index 000000000000..597e0f3694d8 --- /dev/null +++ b/news/2 Fixes/8800.md @@ -0,0 +1 @@ +Refactored the Enable Linting command to provide the user with a choice of "Enable" or "Disable" linting to make it more intuitive. From 10b02815c4d29774e6635441db38934cb028bab5 Mon Sep 17 00:00:00 2001 From: Henry Boisdequin <65845077+henryboisdequin@users.noreply.github.com> Date: Thu, 21 Jan 2021 06:35:53 +0530 Subject: [PATCH 5/5] Update news/2 Fixes/8800.md Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> --- news/2 Fixes/8800.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/news/2 Fixes/8800.md b/news/2 Fixes/8800.md index 597e0f3694d8..6c13a526e96f 100644 --- a/news/2 Fixes/8800.md +++ b/news/2 Fixes/8800.md @@ -1 +1 @@ -Refactored the Enable Linting command to provide the user with a choice of "Enable" or "Disable" linting to make it more intuitive. +Refactored the Enable Linting command to provide the user with a choice of "Enable" or "Disable" linting to make it more intuitive. (thanks [henryboisdequin](https://github.com/henryboisdequin))