Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/2 Fixes/8800.md
Original file line number Diff line number Diff line change
@@ -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. (thanks [henryboisdequin](https://github.com/henryboisdequin))
2 changes: 1 addition & 1 deletion package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
5 changes: 3 additions & 2 deletions src/client/linters/linterCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export class LinterCommands implements IDisposable {
}

public async enableLintingAsync(): Promise<void> {
const options = ['on', 'off'];
const options = ['Enable', 'Disable'];
const current = (await this.linterManager.isLintingEnabled(true, this.settingsUri)) ? options[0] : options[1];

const quickPickOptions: QuickPickOptions = {
Expand All @@ -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);
}
}
Expand Down
36 changes: 23 additions & 13 deletions src/test/linters/linterCommands.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: 'Enable' | 'Disable' | undefined,
) {
when(manager.isLintingEnabled(true, anything())).thenResolve(currentState);
const expectedQuickPickOptions = {
matchOnDetail: true,
matchOnDescription: true,
placeHolder: `current: ${currentState ? 'on' : 'off'}`,
placeHolder: `current: ${currentState ? 'Enable' : 'Disable'}`,
};
when(shell.showQuickPick(anything(), anything())).thenResolve(selectedState as any);

Expand All @@ -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(['Enable', 'Disable']);
expect(quickPickOptions).to.deep.equal(expectedQuickPickOptions);

if (selectedState) {
verify(manager.enableLintingAsync(selectedState === 'on', 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 'on' 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 'on' and select 'on'", async () => {
await testEnableLintingWithCurrentState(true, 'on');

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 'on' and select 'off'", async () => {
await testEnableLintingWithCurrentState(true, 'off');

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 'off' and select 'on'", async () => {
await testEnableLintingWithCurrentState(true, 'on');

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 'off' and select 'off'", async () => {
await testEnableLintingWithCurrentState(true, 'off');

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 () => {
Expand All @@ -108,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];
Expand All @@ -126,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([]);
Expand All @@ -143,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;
Expand Down