From 9540ade29a781b1890e0f6c410f4c39306c70121 Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Tue, 4 Dec 2018 13:56:09 -0800 Subject: [PATCH 1/6] Allow users to not show 'Install missing Linter' prompt. Fix for #3349 - Saves persisted value per linter/per workspace --- news/1 Enhancements/3349.md | 1 + .../common/installer/productInstaller.ts | 49 +++++++++++- .../installer.invalidPath.unit.test.ts | 14 +++- .../common/installer/installer.unit.test.ts | 79 +++++++++++++++++-- 4 files changed, 133 insertions(+), 10 deletions(-) create mode 100644 news/1 Enhancements/3349.md diff --git a/news/1 Enhancements/3349.md b/news/1 Enhancements/3349.md new file mode 100644 index 000000000000..b5b171dc7df6 --- /dev/null +++ b/news/1 Enhancements/3349.md @@ -0,0 +1 @@ +Allow users to request the 'Install missing Linter' prompt to not show again for a workspace. \ No newline at end of file diff --git a/src/client/common/installer/productInstaller.ts b/src/client/common/installer/productInstaller.ts index b0256848ea22..88c113a6e75e 100644 --- a/src/client/common/installer/productInstaller.ts +++ b/src/client/common/installer/productInstaller.ts @@ -11,7 +11,10 @@ import { STANDARD_OUTPUT_CHANNEL } from '../constants'; import { IPlatformService } from '../platform/types'; import { IProcessServiceFactory, IPythonExecutionFactory } from '../process/types'; import { ITerminalServiceFactory } from '../terminal/types'; -import { IConfigurationService, IInstaller, ILogger, InstallerResponse, IOutputChannel, ModuleNamePurpose, Product, ProductType } from '../types'; +import { + IConfigurationService, IInstaller, ILogger, InstallerResponse, IOutputChannel, + IPersistentStateFactory, ModuleNamePurpose, Product, ProductType +} from '../types'; import { ProductNames } from './productNames'; import { IInstallationChannelManager, IProductPathService, IProductService } from './types'; @@ -88,6 +91,38 @@ export abstract class BaseInstaller { .catch(() => false); } } + + /** + * For installers that want to avoid prompting the user over and over, they can make use of a + * persisted true/false value representing user responses to 'stop showing this prompt'. This method + * gets the persisted value given the installer-defined key. + * + * @param key Key to use to get a persisted response value, each installer must define this for themselves. + * @returns Boolean: The current state of the stored response key given. + */ + public getStoredResponse(key: string): boolean { + const factory = this.serviceContainer.get(IPersistentStateFactory); + const state = factory.createWorkspacePersistentState(key, undefined); + return state.value; + } + + /** + * For installers that want to avoid prompting the user over and over, they can make use of a + * persisted true/false value representing user responses to 'stop showing this prompt'. This + * method will set that persisted value given the installer-defined key. + * + * @param key Key to use to get a persisted response value, each installer must define this for themselves. + * @param value Boolean value to store for the user - if they choose to not be prompted again for instance. + * @returns Boolean: The current state of the stored response key given. + */ + public async setStoredResponse(key: string, value: boolean): Promise { + const factory = this.serviceContainer.get(IPersistentStateFactory); + const state = factory.createWorkspacePersistentState(key, undefined); + if (state && state.value !== value) { + state.updateValue(value); + } + } + protected abstract promptToInstallImplementation(product: Product, resource?: Uri): Promise; protected getExecutableNameFromSettings(product: Product, resource?: Uri): string { const productType = this.productService.getProductType(product); @@ -172,8 +207,14 @@ export class LinterInstaller extends BaseInstaller { const install = 'Install'; const disableAllLinting = 'Disable linting'; const disableThisLinter = `Disable ${productName}`; + const disableInstallPrompt = 'Do not show again'; + const disableLinterInstallPromptKey = `${productName}_DisableLinterInstallPrompt`; - const options = [disableThisLinter, disableAllLinting]; + if (this.getStoredResponse(disableLinterInstallPromptKey) === true) { + return InstallerResponse.Ignore; + } + + const options = [disableThisLinter, disableAllLinting, disableInstallPrompt]; let message = `Linter ${productName} is not installed.`; if (this.isExecutableAModule(product, resource)) { options.splice(0, 0, install); @@ -185,7 +226,11 @@ export class LinterInstaller extends BaseInstaller { const response = await this.appShell.showErrorMessage(message, ...options); if (response === install) { return this.install(product, resource); + } else if (response === disableInstallPrompt) { + this.setStoredResponse(disableLinterInstallPromptKey, true).ignoreErrors(); + return InstallerResponse.Ignore; } + const lm = this.serviceContainer.get(ILinterManager); if (response === disableAllLinting) { await lm.enableLintingAsync(false); diff --git a/src/test/common/installer/installer.invalidPath.unit.test.ts b/src/test/common/installer/installer.invalidPath.unit.test.ts index 7b26a3253d67..c8183a15c6bc 100644 --- a/src/test/common/installer/installer.invalidPath.unit.test.ts +++ b/src/test/common/installer/installer.invalidPath.unit.test.ts @@ -13,7 +13,7 @@ import '../../../client/common/extensions'; import { ProductInstaller } from '../../../client/common/installer/productInstaller'; import { ProductService } from '../../../client/common/installer/productService'; import { IProductPathService, IProductService } from '../../../client/common/installer/types'; -import { Product } from '../../../client/common/types'; +import { IPersistentState, IPersistentStateFactory, Product } from '../../../client/common/types'; import { getNamesAndValues } from '../../../client/common/utils/enum'; import { IServiceContainer } from '../../../client/ioc/types'; @@ -30,6 +30,8 @@ suite('Module Installer - Invalid Paths', () => { let app: TypeMoq.IMock; let workspaceService: TypeMoq.IMock; let productPathService: TypeMoq.IMock; + let persistentState: TypeMoq.IMock; + setup(() => { serviceContainer = TypeMoq.Mock.ofType(); const outputChannel = TypeMoq.Mock.ofType(); @@ -43,6 +45,9 @@ suite('Module Installer - Invalid Paths', () => { productPathService = TypeMoq.Mock.ofType(); serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IProductPathService), TypeMoq.It.isAny())).returns(() => productPathService.object); + persistentState = TypeMoq.Mock.ofType(); + serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IPersistentStateFactory), TypeMoq.It.isAny())).returns(() => persistentState.object); + installer = new ProductInstaller(serviceContainer.object, outputChannel.object); }); @@ -74,7 +79,12 @@ suite('Module Installer - Invalid Paths', () => { }) .returns(() => Promise.resolve(undefined)) .verifiable(TypeMoq.Times.exactly(1)); - + const persistValue = TypeMoq.Mock.ofType>(); + persistValue.setup(pv => pv.value).returns(() => false); + persistValue.setup(pv => pv.updateValue(TypeMoq.It.isValue(true))); + persistentState.setup(ps => + ps.createWorkspacePersistentState(TypeMoq.It.isAnyString(), TypeMoq.It.isValue(undefined)) + ).returns(() => persistValue.object); await installer.promptToInstall(product.value, resource); productPathService.verifyAll(); }); diff --git a/src/test/common/installer/installer.unit.test.ts b/src/test/common/installer/installer.unit.test.ts index 2187bef2da78..2427c2516268 100644 --- a/src/test/common/installer/installer.unit.test.ts +++ b/src/test/common/installer/installer.unit.test.ts @@ -11,15 +11,20 @@ import { IApplicationShell, IWorkspaceService } from '../../../client/common/app import '../../../client/common/extensions'; import { ProductInstaller } from '../../../client/common/installer/productInstaller'; import { ProductService } from '../../../client/common/installer/productService'; -import { IInstallationChannelManager, IModuleInstaller, IProductPathService, IProductService } from '../../../client/common/installer/types'; -import { IDisposableRegistry, ILogger, InstallerResponse, ModuleNamePurpose, Product } from '../../../client/common/types'; +import { + IInstallationChannelManager, IModuleInstaller, IProductPathService, IProductService +} from '../../../client/common/installer/types'; +import { + IDisposableRegistry, ILogger, InstallerResponse, IPersistentState, + IPersistentStateFactory, ModuleNamePurpose, Product, ProductType +} from '../../../client/common/types'; import { createDeferred, Deferred } from '../../../client/common/utils/async'; import { getNamesAndValues } from '../../../client/common/utils/enum'; import { IServiceContainer } from '../../../client/ioc/types'; use(chaiAsPromised); -suite('Module Installer', () => { +suite('Module Installer only', () => { [undefined, Uri.file('resource')].forEach(resource => { getNamesAndValues(Product).forEach(product => { let disposables: Disposable[] = []; @@ -30,6 +35,9 @@ suite('Module Installer', () => { let app: TypeMoq.IMock; let promptDeferred: Deferred; let workspaceService: TypeMoq.IMock; + let persistentStore: TypeMoq.IMock; + const productService = new ProductService(); + setup(() => { promptDeferred = createDeferred(); serviceContainer = TypeMoq.Mock.ofType(); @@ -37,13 +45,15 @@ suite('Module Installer', () => { disposables = []; serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IDisposableRegistry), TypeMoq.It.isAny())).returns(() => disposables); - serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IProductService), TypeMoq.It.isAny())).returns(() => new ProductService()); + serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IProductService), TypeMoq.It.isAny())).returns(() => productService); installationChannel = TypeMoq.Mock.ofType(); serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IInstallationChannelManager), TypeMoq.It.isAny())).returns(() => installationChannel.object); app = TypeMoq.Mock.ofType(); serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IApplicationShell), TypeMoq.It.isAny())).returns(() => app.object); workspaceService = TypeMoq.Mock.ofType(); serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IWorkspaceService), TypeMoq.It.isAny())).returns(() => workspaceService.object); + persistentStore = TypeMoq.Mock.ofType(); + serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IPersistentStateFactory), TypeMoq.It.isAny())).returns(() => persistentStore.object); moduleInstaller = TypeMoq.Mock.ofType(); // tslint:disable-next-line:no-any @@ -113,13 +123,22 @@ suite('Module Installer', () => { } }); if (product.value !== Product.unittest) { - test(`Ensure the prompt is displayed only once, untill the prompt is closed, ${product.name} (${resource ? 'With a resource' : 'without a resource'})`, async () => { + test(`Ensure the prompt is displayed only once, until the prompt is closed, ${product.name} (${resource ? 'With a resource' : 'without a resource'})`, async () => { workspaceService.setup(w => w.getWorkspaceFolder(TypeMoq.It.isValue(resource!))) .returns(() => TypeMoq.Mock.ofType().object) .verifiable(TypeMoq.Times.exactly(resource ? 5 : 0)); app.setup(a => a.showErrorMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) - .returns(() => promptDeferred.promise) + .returns( + () => { + return promptDeferred.promise; + }) .verifiable(TypeMoq.Times.once()); + const persistVal = TypeMoq.Mock.ofType>(); + persistVal.setup(p => p.value).returns(() => false); + persistVal.setup(p => p.updateValue(TypeMoq.It.isValue(true))); + persistentStore.setup(ps => + ps.createWorkspacePersistentState(TypeMoq.It.isAnyString(), TypeMoq.It.isValue(undefined)) + ).returns(() => persistVal.object); // Display first prompt. installer.promptToInstall(product.value, resource).ignoreErrors(); @@ -133,6 +152,48 @@ suite('Module Installer', () => { app.verifyAll(); workspaceService.verifyAll(); }); + if (productService.getProductType(product.value) === ProductType.Linter) { + test(`Ensure the install prompt is not displayed when the user requests it not be shown again, ${product.name} (${resource ? 'With a resource' : 'without a resource'})`, async () => { + workspaceService.setup(w => w.getWorkspaceFolder(TypeMoq.It.isValue(resource!))) + .returns(() => TypeMoq.Mock.ofType().object) + .verifiable(TypeMoq.Times.exactly(resource ? 2 : 0)); + app.setup(a => a.showErrorMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns( + async () => { + return 'Do not show again'; + }) + .verifiable(TypeMoq.Times.once()); + const persistVal = TypeMoq.Mock.ofType>(); + let mockPersistVal = false; + persistVal.setup(p => p.value).returns(() => { + return mockPersistVal; + }); + persistVal.setup(p => p.updateValue(TypeMoq.It.isValue(true))) + .returns(() => { + mockPersistVal = true; + return Promise.resolve(); + }).verifiable(TypeMoq.Times.once()); + persistentStore.setup(ps => + ps.createWorkspacePersistentState(TypeMoq.It.isAnyString(), TypeMoq.It.isValue(undefined)) + ).returns(() => { + return persistVal.object; + }).verifiable(TypeMoq.Times.exactly(3)); + + // Display first prompt. + const initialResponse = await installer.promptToInstall(product.value, resource); + + // Display a second prompt. + const secondResponse = await installer.promptToInstall(product.value, resource); + + expect(initialResponse).to.be.equal(InstallerResponse.Ignore); + expect(secondResponse).to.be.equal(InstallerResponse.Ignore); + + app.verifyAll(); + workspaceService.verifyAll(); + persistentStore.verifyAll(); + persistVal.verifyAll(); + }); + } test(`Ensure the prompt is displayed again when previous prompt has been closed, ${product.name} (${resource ? 'With a resource' : 'without a resource'})`, async () => { workspaceService.setup(w => w.getWorkspaceFolder(TypeMoq.It.isValue(resource!))) .returns(() => TypeMoq.Mock.ofType().object) @@ -140,6 +201,12 @@ suite('Module Installer', () => { app.setup(a => a.showErrorMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) .returns(() => Promise.resolve(undefined)) .verifiable(TypeMoq.Times.exactly(3)); + const persistVal = TypeMoq.Mock.ofType>(); + persistVal.setup(p => p.value).returns(() => false); + persistVal.setup(p => p.updateValue(TypeMoq.It.isValue(true))); + persistentStore.setup(ps => + ps.createWorkspacePersistentState(TypeMoq.It.isAnyString(), TypeMoq.It.isValue(undefined)) + ).returns(() => persistVal.object); await installer.promptToInstall(product.value, resource); await installer.promptToInstall(product.value, resource); From 4f4c33d417874808fb2627d224b226abf1117291 Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Tue, 4 Dec 2018 14:41:06 -0800 Subject: [PATCH 2/6] Fix up hygiene issue --- src/client/common/installer/productInstaller.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/common/installer/productInstaller.ts b/src/client/common/installer/productInstaller.ts index 88c113a6e75e..788059c62e56 100644 --- a/src/client/common/installer/productInstaller.ts +++ b/src/client/common/installer/productInstaller.ts @@ -119,7 +119,7 @@ export abstract class BaseInstaller { const factory = this.serviceContainer.get(IPersistentStateFactory); const state = factory.createWorkspacePersistentState(key, undefined); if (state && state.value !== value) { - state.updateValue(value); + await state.updateValue(value); } } From 8e73fafba9945f7ea3aaa2f926148c79d08efe54 Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Sat, 8 Dec 2018 14:35:54 -0800 Subject: [PATCH 3/6] Only for pylint, will be removed once LS is GA - add test to ensure it only shows for pylint --- .vscode/launch.json | 4 +- .../common/installer/productInstaller.ts | 71 ++++++++++--------- .../common/installer/installer.unit.test.ts | 59 ++++++++++++++- 3 files changed, 96 insertions(+), 38 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 11b9ecc5c193..8ab42ad59dab 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -124,7 +124,7 @@ "sourceMaps": true, "args": [ "timeout=300000", - "grep=" + "grep=Module Installer only" ], "outFiles": [ "${workspaceFolder}/out/**/*.js" @@ -166,4 +166,4 @@ ] } ] -} +} \ No newline at end of file diff --git a/src/client/common/installer/productInstaller.ts b/src/client/common/installer/productInstaller.ts index 788059c62e56..4af9e77077fc 100644 --- a/src/client/common/installer/productInstaller.ts +++ b/src/client/common/installer/productInstaller.ts @@ -92,37 +92,6 @@ export abstract class BaseInstaller { } } - /** - * For installers that want to avoid prompting the user over and over, they can make use of a - * persisted true/false value representing user responses to 'stop showing this prompt'. This method - * gets the persisted value given the installer-defined key. - * - * @param key Key to use to get a persisted response value, each installer must define this for themselves. - * @returns Boolean: The current state of the stored response key given. - */ - public getStoredResponse(key: string): boolean { - const factory = this.serviceContainer.get(IPersistentStateFactory); - const state = factory.createWorkspacePersistentState(key, undefined); - return state.value; - } - - /** - * For installers that want to avoid prompting the user over and over, they can make use of a - * persisted true/false value representing user responses to 'stop showing this prompt'. This - * method will set that persisted value given the installer-defined key. - * - * @param key Key to use to get a persisted response value, each installer must define this for themselves. - * @param value Boolean value to store for the user - if they choose to not be prompted again for instance. - * @returns Boolean: The current state of the stored response key given. - */ - public async setStoredResponse(key: string, value: boolean): Promise { - const factory = this.serviceContainer.get(IPersistentStateFactory); - const state = factory.createWorkspacePersistentState(key, undefined); - if (state && state.value !== value) { - await state.updateValue(value); - } - } - protected abstract promptToInstallImplementation(product: Product, resource?: Uri): Promise; protected getExecutableNameFromSettings(product: Product, resource?: Uri): string { const productType = this.productService.getProductType(product); @@ -203,6 +172,8 @@ export class FormatterInstaller extends BaseInstaller { export class LinterInstaller extends BaseInstaller { protected async promptToInstallImplementation(product: Product, resource?: Uri): Promise { + const isPylint = product === Product.pylint; + const productName = ProductNames.get(product)!; const install = 'Install'; const disableAllLinting = 'Disable linting'; @@ -210,11 +181,12 @@ export class LinterInstaller extends BaseInstaller { const disableInstallPrompt = 'Do not show again'; const disableLinterInstallPromptKey = `${productName}_DisableLinterInstallPrompt`; - if (this.getStoredResponse(disableLinterInstallPromptKey) === true) { + if (isPylint && this.getStoredResponse(disableLinterInstallPromptKey) === true) { return InstallerResponse.Ignore; } - const options = [disableThisLinter, disableAllLinting, disableInstallPrompt]; + const options = isPylint ? [disableThisLinter, disableAllLinting, disableInstallPrompt] : [disableThisLinter, disableAllLinting]; + let message = `Linter ${productName} is not installed.`; if (this.isExecutableAModule(product, resource)) { options.splice(0, 0, install); @@ -227,7 +199,7 @@ export class LinterInstaller extends BaseInstaller { if (response === install) { return this.install(product, resource); } else if (response === disableInstallPrompt) { - this.setStoredResponse(disableLinterInstallPromptKey, true).ignoreErrors(); + await this.setStoredResponse(disableLinterInstallPromptKey, true); return InstallerResponse.Ignore; } @@ -241,6 +213,37 @@ export class LinterInstaller extends BaseInstaller { } return InstallerResponse.Ignore; } + + /** + * For installers that want to avoid prompting the user over and over, they can make use of a + * persisted true/false value representing user responses to 'stop showing this prompt'. This method + * gets the persisted value given the installer-defined key. + * + * @param key Key to use to get a persisted response value, each installer must define this for themselves. + * @returns Boolean: The current state of the stored response key given. + */ + private getStoredResponse(key: string): boolean { + const factory = this.serviceContainer.get(IPersistentStateFactory); + const state = factory.createWorkspacePersistentState(key, undefined); + return state.value; + } + + /** + * For installers that want to avoid prompting the user over and over, they can make use of a + * persisted true/false value representing user responses to 'stop showing this prompt'. This + * method will set that persisted value given the installer-defined key. + * + * @param key Key to use to get a persisted response value, each installer must define this for themselves. + * @param value Boolean value to store for the user - if they choose to not be prompted again for instance. + * @returns Boolean: The current state of the stored response key given. + */ + private async setStoredResponse(key: string, value: boolean): Promise { + const factory = this.serviceContainer.get(IPersistentStateFactory); + const state = factory.createWorkspacePersistentState(key, undefined); + if (state && state.value !== value) { + await state.updateValue(value); + } + } } export class TestFrameworkInstaller extends BaseInstaller { diff --git a/src/test/common/installer/installer.unit.test.ts b/src/test/common/installer/installer.unit.test.ts index 2427c2516268..8dc3c25e1805 100644 --- a/src/test/common/installer/installer.unit.test.ts +++ b/src/test/common/installer/installer.unit.test.ts @@ -152,12 +152,18 @@ suite('Module Installer only', () => { app.verifyAll(); workspaceService.verifyAll(); }); - if (productService.getProductType(product.value) === ProductType.Linter) { + if (product.value === Product.pylint) { test(`Ensure the install prompt is not displayed when the user requests it not be shown again, ${product.name} (${resource ? 'With a resource' : 'without a resource'})`, async () => { workspaceService.setup(w => w.getWorkspaceFolder(TypeMoq.It.isValue(resource!))) .returns(() => TypeMoq.Mock.ofType().object) .verifiable(TypeMoq.Times.exactly(resource ? 2 : 0)); - app.setup(a => a.showErrorMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) + app.setup(a => + a.showErrorMessage( + TypeMoq.It.isAnyString(), + TypeMoq.It.isValue('Install'), + TypeMoq.It.isValue(`Disable ${product.name}`), + TypeMoq.It.isValue('Disable linting'), + TypeMoq.It.isValue('Do not show again'))) .returns( async () => { return 'Do not show again'; @@ -193,6 +199,55 @@ suite('Module Installer only', () => { persistentStore.verifyAll(); persistVal.verifyAll(); }); + } else if (productService.getProductType(product.value) === ProductType.Linter) { + test(`Ensure the 'do not show again' prompt isn't shown for non-pylint linters, ${product.name} (${resource ? 'With a resource' : 'without a resource'})`, async () => { + workspaceService.setup(w => w.getWorkspaceFolder(TypeMoq.It.isValue(resource!))) + .returns(() => TypeMoq.Mock.ofType().object); + app.setup(a => + a.showErrorMessage( + TypeMoq.It.isAnyString(), + TypeMoq.It.isValue('Install'), + TypeMoq.It.isValue(`Disable ${product.name}`), + TypeMoq.It.isValue('Disable linting'))) + .returns( + async () => { + return undefined; + }) + .verifiable(TypeMoq.Times.once()); + app.setup(a => + a.showErrorMessage( + TypeMoq.It.isAnyString(), + TypeMoq.It.isValue('Install'), + TypeMoq.It.isValue(`Disable ${product.name}`), + TypeMoq.It.isValue('Disable linting'), + TypeMoq.It.isValue('Do not show again'))) + .returns( + async () => { + return undefined; + }) + .verifiable(TypeMoq.Times.never()); + const persistVal = TypeMoq.Mock.ofType>(); + let mockPersistVal = false; + persistVal.setup(p => p.value).returns(() => { + return mockPersistVal; + }); + persistVal.setup(p => p.updateValue(TypeMoq.It.isValue(true))) + .returns(() => { + mockPersistVal = true; + return Promise.resolve(); + }); + persistentStore.setup(ps => + ps.createWorkspacePersistentState(TypeMoq.It.isAnyString(), TypeMoq.It.isValue(undefined)) + ).returns(() => { + return persistVal.object; + }); + + // Display the prompt. + await installer.promptToInstall(product.value, resource); + + // we're just ensuring the 'disable pylint' prompt never appears... + app.verifyAll(); + }); } test(`Ensure the prompt is displayed again when previous prompt has been closed, ${product.name} (${resource ? 'With a resource' : 'without a resource'})`, async () => { workspaceService.setup(w => w.getWorkspaceFolder(TypeMoq.It.isValue(resource!))) From a0f884865e96bbc5dfb3924d896edc5615850d51 Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Sat, 8 Dec 2018 14:38:03 -0800 Subject: [PATCH 4/6] Make the prompt to not install globally effective, not just per workspace. - update news --- news/1 Enhancements/3349.md | 2 +- src/client/common/installer/productInstaller.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/news/1 Enhancements/3349.md b/news/1 Enhancements/3349.md index b5b171dc7df6..4c29e1dc881f 100644 --- a/news/1 Enhancements/3349.md +++ b/news/1 Enhancements/3349.md @@ -1 +1 @@ -Allow users to request the 'Install missing Linter' prompt to not show again for a workspace. \ No newline at end of file +Allow users to request the 'Install missing Linter' prompt to not show again for pylint. \ No newline at end of file diff --git a/src/client/common/installer/productInstaller.ts b/src/client/common/installer/productInstaller.ts index 4af9e77077fc..902b789fb7e7 100644 --- a/src/client/common/installer/productInstaller.ts +++ b/src/client/common/installer/productInstaller.ts @@ -224,7 +224,7 @@ export class LinterInstaller extends BaseInstaller { */ private getStoredResponse(key: string): boolean { const factory = this.serviceContainer.get(IPersistentStateFactory); - const state = factory.createWorkspacePersistentState(key, undefined); + const state = factory.createGlobalPersistentState(key, undefined); return state.value; } @@ -239,7 +239,7 @@ export class LinterInstaller extends BaseInstaller { */ private async setStoredResponse(key: string, value: boolean): Promise { const factory = this.serviceContainer.get(IPersistentStateFactory); - const state = factory.createWorkspacePersistentState(key, undefined); + const state = factory.createGlobalPersistentState(key, undefined); if (state && state.value !== value) { await state.updateValue(value); } From 267f8a0ef57109513d995c05fc139c0f0446499c Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Sat, 8 Dec 2018 15:51:55 -0800 Subject: [PATCH 5/6] Fix up tests to use mock global instead of workspace persistent storage --- .../common/installer/installer.invalidPath.unit.test.ts | 2 +- src/test/common/installer/installer.unit.test.ts | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/test/common/installer/installer.invalidPath.unit.test.ts b/src/test/common/installer/installer.invalidPath.unit.test.ts index c8183a15c6bc..65fc3cb40765 100644 --- a/src/test/common/installer/installer.invalidPath.unit.test.ts +++ b/src/test/common/installer/installer.invalidPath.unit.test.ts @@ -83,7 +83,7 @@ suite('Module Installer - Invalid Paths', () => { persistValue.setup(pv => pv.value).returns(() => false); persistValue.setup(pv => pv.updateValue(TypeMoq.It.isValue(true))); persistentState.setup(ps => - ps.createWorkspacePersistentState(TypeMoq.It.isAnyString(), TypeMoq.It.isValue(undefined)) + ps.createGlobalPersistentState(TypeMoq.It.isAnyString(), TypeMoq.It.isValue(undefined)) ).returns(() => persistValue.object); await installer.promptToInstall(product.value, resource); productPathService.verifyAll(); diff --git a/src/test/common/installer/installer.unit.test.ts b/src/test/common/installer/installer.unit.test.ts index 8dc3c25e1805..9176c0d4a713 100644 --- a/src/test/common/installer/installer.unit.test.ts +++ b/src/test/common/installer/installer.unit.test.ts @@ -137,7 +137,7 @@ suite('Module Installer only', () => { persistVal.setup(p => p.value).returns(() => false); persistVal.setup(p => p.updateValue(TypeMoq.It.isValue(true))); persistentStore.setup(ps => - ps.createWorkspacePersistentState(TypeMoq.It.isAnyString(), TypeMoq.It.isValue(undefined)) + ps.createGlobalPersistentState(TypeMoq.It.isAnyString(), TypeMoq.It.isValue(undefined)) ).returns(() => persistVal.object); // Display first prompt. @@ -180,7 +180,7 @@ suite('Module Installer only', () => { return Promise.resolve(); }).verifiable(TypeMoq.Times.once()); persistentStore.setup(ps => - ps.createWorkspacePersistentState(TypeMoq.It.isAnyString(), TypeMoq.It.isValue(undefined)) + ps.createGlobalPersistentState(TypeMoq.It.isAnyString(), TypeMoq.It.isValue(undefined)) ).returns(() => { return persistVal.object; }).verifiable(TypeMoq.Times.exactly(3)); @@ -237,7 +237,7 @@ suite('Module Installer only', () => { return Promise.resolve(); }); persistentStore.setup(ps => - ps.createWorkspacePersistentState(TypeMoq.It.isAnyString(), TypeMoq.It.isValue(undefined)) + ps.createGlobalPersistentState(TypeMoq.It.isAnyString(), TypeMoq.It.isValue(undefined)) ).returns(() => { return persistVal.object; }); @@ -260,7 +260,7 @@ suite('Module Installer only', () => { persistVal.setup(p => p.value).returns(() => false); persistVal.setup(p => p.updateValue(TypeMoq.It.isValue(true))); persistentStore.setup(ps => - ps.createWorkspacePersistentState(TypeMoq.It.isAnyString(), TypeMoq.It.isValue(undefined)) + ps.createGlobalPersistentState(TypeMoq.It.isAnyString(), TypeMoq.It.isValue(undefined)) ).returns(() => persistVal.object); await installer.promptToInstall(product.value, resource); From 48cfd7250a49493637b6ffb3e150dcf14f0a0465 Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Mon, 10 Dec 2018 22:40:56 -0800 Subject: [PATCH 6/6] Remove grep from launch.json --- .vscode/launch.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 8ab42ad59dab..d8c2042447d7 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -124,7 +124,7 @@ "sourceMaps": true, "args": [ "timeout=300000", - "grep=Module Installer only" + "grep=" ], "outFiles": [ "${workspaceFolder}/out/**/*.js"