@@ -6,18 +6,20 @@ import { CancellationToken, OutputChannel, Uri } from 'vscode';
66import '../../common/extensions' ;
77import { IInterpreterService } from '../../interpreter/contracts' ;
88import { IServiceContainer } from '../../ioc/types' ;
9- import { LinterId } from '../../linters/types' ;
9+ import { ILinterManager , LinterId } from '../../linters/types' ;
1010import { PythonEnvironment } from '../../pythonEnvironments/info' ;
1111import { sendTelemetryEvent } from '../../telemetry' ;
1212import { EventName } from '../../telemetry/constants' ;
1313import { IApplicationShell , ICommandManager , IWorkspaceService } from '../application/types' ;
1414import { Commands , STANDARD_OUTPUT_CHANNEL } from '../constants' ;
15- import { traceError } from '../logger' ;
15+ import { LinterInstallationPromptVariants } from '../experiments/groups' ;
16+ import { traceError , traceInfo } from '../logger' ;
1617import { IPlatformService } from '../platform/types' ;
1718import { IProcessServiceFactory , IPythonExecutionFactory } from '../process/types' ;
1819import { ITerminalServiceFactory } from '../terminal/types' ;
1920import {
2021 IConfigurationService ,
22+ IExperimentsManager ,
2123 IInstaller ,
2224 InstallerResponse ,
2325 IOutputChannel ,
@@ -26,7 +28,7 @@ import {
2628 Product ,
2729 ProductType
2830} from '../types' ;
29- import { Installer } from '../utils/localize' ;
31+ import { Common , Installer , Linters } from '../utils/localize' ;
3032import { isResource , noop } from '../utils/misc' ;
3133import { ProductNames } from './productNames' ;
3234import {
@@ -39,14 +41,14 @@ import {
3941
4042export { Product } from '../types' ;
4143
42- export const CTagsInsllationScript =
44+ export const CTagsInstallationScript =
4345 os . platform ( ) === 'darwin' ? 'brew install ctags' : 'sudo apt-get install exuberant-ctags' ;
4446
4547export abstract class BaseInstaller {
4648 private static readonly PromptPromises = new Map < string , Promise < InstallerResponse > > ( ) ;
4749 protected readonly appShell : IApplicationShell ;
4850 protected readonly configService : IConfigurationService ;
49- private readonly workspaceService : IWorkspaceService ;
51+ protected readonly workspaceService : IWorkspaceService ;
5052 private readonly productService : IProductService ;
5153
5254 constructor ( protected serviceContainer : IServiceContainer , protected outputChannel : OutputChannel ) {
@@ -168,8 +170,8 @@ export class CTagsInstaller extends BaseInstaller {
168170 . get < ITerminalServiceFactory > ( ITerminalServiceFactory )
169171 . getTerminalService ( resource ) ;
170172 terminalService
171- . sendCommand ( CTagsInsllationScript , [ ] )
172- . catch ( ( ex ) => traceError ( `Failed to install ctags. Script sent '${ CTagsInsllationScript } ', ${ ex } ` ) ) ;
173+ . sendCommand ( CTagsInstallationScript , [ ] )
174+ . catch ( ( ex ) => traceError ( `Failed to install ctags. Script sent '${ CTagsInstallationScript } ', ${ ex } ` ) ) ;
173175 }
174176 return InstallerResponse . Ignore ;
175177 }
@@ -230,24 +232,162 @@ export class FormatterInstaller extends BaseInstaller {
230232}
231233
232234export class LinterInstaller extends BaseInstaller {
235+ // This is a hack, really we should be handling this in a service that
236+ // controls the prompts we show. The issue here was that if we show
237+ // a prompt to install pylint and flake8, and user selects flake8
238+ // we immediately show this prompt again saying install flake8, while the
239+ // installation is on going.
240+ private static promptSeen : boolean = false ;
241+ private readonly experimentsManager : IExperimentsManager ;
242+ private readonly linterManager : ILinterManager ;
243+
244+ constructor ( protected serviceContainer : IServiceContainer , protected outputChannel : OutputChannel ) {
245+ super ( serviceContainer , outputChannel ) ;
246+ this . experimentsManager = serviceContainer . get < IExperimentsManager > ( IExperimentsManager ) ;
247+ this . linterManager = serviceContainer . get < ILinterManager > ( ILinterManager ) ;
248+ }
249+
250+ public static reset ( ) {
251+ // Read notes where this is defined.
252+ LinterInstaller . promptSeen = false ;
253+ }
254+
233255 protected async promptToInstallImplementation (
234256 product : Product ,
235257 resource ?: Uri ,
236258 cancel ?: CancellationToken
237259 ) : Promise < InstallerResponse > {
260+ // This is a hack, really we should be handling this in a service that
261+ // controls the prompts we show. The issue here was that if we show
262+ // a prompt to install pylint and flake8, and user selects flake8
263+ // we immediately show this prompt again saying install flake8, while the
264+ // installation is on going.
265+ if ( LinterInstaller . promptSeen ) {
266+ return InstallerResponse . Ignore ;
267+ }
268+
269+ LinterInstaller . promptSeen = true ;
270+
271+ // Conditions to use experiment prompt:
272+ // 1. There should be no linter set in any scope
273+ // 2. The default linter should be pylint
274+
275+ if ( ! this . isLinterSetInAnyScope ( ) && product === Product . pylint ) {
276+ if ( this . experimentsManager . inExperiment ( LinterInstallationPromptVariants . noPrompt ) ) {
277+ // We won't show a prompt, so tell the extension to treat as though user
278+ // ignored the prompt.
279+ sendTelemetryEvent ( EventName . LINTER_INSTALL_PROMPT , undefined , {
280+ prompt : 'noPrompt'
281+ } ) ;
282+
283+ const productName = ProductNames . get ( product ) ! ;
284+ traceInfo ( `Linter ${ productName } is not installed.` ) ;
285+
286+ return InstallerResponse . Ignore ;
287+ } else if ( this . experimentsManager . inExperiment ( LinterInstallationPromptVariants . pylintFirst ) ) {
288+ return this . newPromptForInstallation ( true , resource , cancel ) ;
289+ } else if ( this . experimentsManager . inExperiment ( LinterInstallationPromptVariants . flake8First ) ) {
290+ return this . newPromptForInstallation ( false , resource , cancel ) ;
291+ }
292+ }
293+
294+ sendTelemetryEvent ( EventName . LINTER_INSTALL_PROMPT , undefined , {
295+ prompt : 'old'
296+ } ) ;
297+ return this . oldPromptForInstallation ( product , resource , cancel ) ;
298+ }
299+
300+ /**
301+ * For installers that want to avoid prompting the user over and over, they can make use of a
302+ * persisted true/false value representing user responses to 'stop showing this prompt'. This method
303+ * gets the persisted value given the installer-defined key.
304+ *
305+ * @param key Key to use to get a persisted response value, each installer must define this for themselves.
306+ * @returns Boolean: The current state of the stored response key given.
307+ */
308+ protected getStoredResponse ( key : string ) : boolean {
309+ const factory = this . serviceContainer . get < IPersistentStateFactory > ( IPersistentStateFactory ) ;
310+ const state = factory . createGlobalPersistentState < boolean | undefined > ( key , undefined ) ;
311+ return state . value === true ;
312+ }
313+
314+ private async newPromptForInstallation ( pylintFirst : boolean , resource ?: Uri , cancel ?: CancellationToken ) {
315+ const productName = ProductNames . get ( Product . pylint ) ! ;
316+
317+ // User has already set to ignore this prompt
318+ const disableLinterInstallPromptKey = `${ productName } _DisableLinterInstallPrompt` ;
319+ if ( this . getStoredResponse ( disableLinterInstallPromptKey ) === true ) {
320+ return InstallerResponse . Ignore ;
321+ }
322+
323+ // Check if the linter settings has Pylint or flake8 pointing to executables.
324+ // If the settings point to executables then we can't install. Defer to old Prompt.
325+ if (
326+ ! this . isExecutableAModule ( Product . pylint , resource ) ||
327+ ! this . isExecutableAModule ( Product . flake8 , resource )
328+ ) {
329+ return this . oldPromptForInstallation ( Product . pylint , resource , cancel ) ;
330+ }
331+
332+ const installPylint = Linters . installPylint ( ) ;
333+ const installFlake8 = Linters . installFlake8 ( ) ;
334+ const doNotShowAgain = Common . doNotShowAgain ( ) ;
335+
336+ const options = pylintFirst
337+ ? [ installPylint , installFlake8 , doNotShowAgain ]
338+ : [ installFlake8 , installPylint , doNotShowAgain ] ;
339+ const message = Linters . installMessage ( ) ;
340+ const prompt = pylintFirst ? 'pylintFirst' : 'flake8first' ;
341+
342+ sendTelemetryEvent ( EventName . LINTER_INSTALL_PROMPT , undefined , {
343+ prompt
344+ } ) ;
345+
346+ const response = await this . appShell . showInformationMessage ( message , ...options ) ;
347+
348+ if ( response === installPylint ) {
349+ sendTelemetryEvent ( EventName . LINTER_INSTALL_PROMPT_ACTION , undefined , {
350+ prompt,
351+ action : 'installPylint'
352+ } ) ;
353+ return this . install ( Product . pylint , resource , cancel ) ;
354+ } else if ( response === installFlake8 ) {
355+ sendTelemetryEvent ( EventName . LINTER_INSTALL_PROMPT_ACTION , undefined , {
356+ prompt,
357+ action : 'installFlake8'
358+ } ) ;
359+ await this . linterManager . setActiveLintersAsync ( [ Product . flake8 ] , resource ) ;
360+ return this . install ( Product . flake8 , resource , cancel ) ;
361+ } else if ( response === doNotShowAgain ) {
362+ sendTelemetryEvent ( EventName . LINTER_INSTALL_PROMPT_ACTION , undefined , {
363+ prompt,
364+ action : 'disablePrompt'
365+ } ) ;
366+ await this . setStoredResponse ( disableLinterInstallPromptKey , true ) ;
367+ return InstallerResponse . Ignore ;
368+ }
369+
370+ sendTelemetryEvent ( EventName . LINTER_INSTALL_PROMPT_ACTION , undefined , {
371+ prompt,
372+ action : 'close'
373+ } ) ;
374+ return InstallerResponse . Ignore ;
375+ }
376+
377+ private async oldPromptForInstallation ( product : Product , resource ?: Uri , cancel ?: CancellationToken ) {
238378 const isPylint = product === Product . pylint ;
239379
240380 const productName = ProductNames . get ( product ) ! ;
241- const install = 'Install' ;
242- const disableInstallPrompt = 'Do not show again' ;
381+ const install = Common . install ( ) ;
382+ const doNotShowAgain = Common . doNotShowAgain ( ) ;
243383 const disableLinterInstallPromptKey = `${ productName } _DisableLinterInstallPrompt` ;
244- const selectLinter = 'Select Linter' ;
384+ const selectLinter = Linters . selectLinter ( ) ;
245385
246386 if ( isPylint && this . getStoredResponse ( disableLinterInstallPromptKey ) === true ) {
247387 return InstallerResponse . Ignore ;
248388 }
249389
250- const options = isPylint ? [ selectLinter , disableInstallPrompt ] : [ selectLinter ] ;
390+ const options = isPylint ? [ selectLinter , doNotShowAgain ] : [ selectLinter ] ;
251391
252392 let message = `Linter ${ productName } is not installed.` ;
253393 if ( this . isExecutableAModule ( product , resource ) ) {
@@ -263,7 +403,7 @@ export class LinterInstaller extends BaseInstaller {
263403 action : 'install'
264404 } ) ;
265405 return this . install ( product , resource , cancel ) ;
266- } else if ( response === disableInstallPrompt ) {
406+ } else if ( response === doNotShowAgain ) {
267407 await this . setStoredResponse ( disableLinterInstallPromptKey , true ) ;
268408 sendTelemetryEvent ( EventName . LINTER_NOT_INSTALLED_PROMPT , undefined , {
269409 tool : productName as LinterId ,
@@ -280,18 +420,34 @@ export class LinterInstaller extends BaseInstaller {
280420 return InstallerResponse . Ignore ;
281421 }
282422
283- /**
284- * For installers that want to avoid prompting the user over and over, they can make use of a
285- * persisted true/false value representing user responses to 'stop showing this prompt'. This method
286- * gets the persisted value given the installer-defined key.
287- *
288- * @param key Key to use to get a persisted response value, each installer must define this for themselves.
289- * @returns Boolean: The current state of the stored response key given.
290- */
291- protected getStoredResponse ( key : string ) : boolean {
292- const factory = this . serviceContainer . get < IPersistentStateFactory > ( IPersistentStateFactory ) ;
293- const state = factory . createGlobalPersistentState < boolean | undefined > ( key , undefined ) ;
294- return state . value === true ;
423+ private isLinterSetInAnyScope ( ) {
424+ const config = this . workspaceService . getConfiguration ( 'python' ) ;
425+ if ( config ) {
426+ const keys = [
427+ 'linting.pylintEnabled' ,
428+ 'linting.flake8Enabled' ,
429+ 'linting.banditEnabled' ,
430+ 'linting.mypyEnabled' ,
431+ 'linting.pycodestyleEnabled' ,
432+ 'linting.prospectorEnabled' ,
433+ 'linting.pydocstyleEnabled' ,
434+ 'linting.pylamaEnabled'
435+ ] ;
436+
437+ const values = keys . map ( ( key ) => {
438+ const value = config . inspect < boolean > ( key ) ;
439+ if ( value ) {
440+ if ( value . globalValue || value . workspaceValue || value . workspaceFolderValue ) {
441+ return 'linter set' ;
442+ }
443+ }
444+ return 'no info' ;
445+ } ) ;
446+
447+ return values . includes ( 'linter set' ) ;
448+ }
449+
450+ return false ;
295451 }
296452
297453 /**
0 commit comments