Cherry-pick pipenv changes and pythonpath prompt changes to release (#11700)

* Show the prompt again if user clicks on more info (#11664)

* Show the prompt again if user clicks on more info

* Review feedback

* Use Learn more as text for the link.

* Leave pipenv in a corner until the user decides to select an interpreter (#11654)

* add onSuggestion option
* Swap onActivation with onSuggestion
* Update unit tests
* Remove registration of IPipenvService
* Move didTriggerInterpreterSuggestions logic inside pipenv locator
* Fix existing unit tests
* Add new unit tests
* Replace typemoq any param with object
* Shorten the tests
* Fix warning
* Duplicate teardown

Co-authored-by: Kim-Adeline Miguel <[email protected]>

Author: karthiknadig

package.nls.json

@@ -240,9 +240,9 @@
     "DataScience.liveShareServiceFailure": "Failure starting '{0}' service during live share connection.",
     "DataScience.documentMismatch": "Cannot run cells, duplicate documents for {0} found.",
     "DataScience.pythonInteractiveCreateFailed": "Failure to create a 'Python Interactive' window. Try reinstalling the Python extension.",
-    "diagnostics.removePythonPathSettingsJson": "The setting \"python.pythonPath\" defined in your workspace settings is now deprecated. Do you want us to delete it? This will only remove the \"python.pythonPath\" entry from your settings.json.",
-    "diagnostics.removePythonPathCodeWorkspace": "The setting \"python.pythonPath\" defined in your workspace settings is now deprecated. Do you want us to delete it? This will only remove the \"python.pythonPath\" entry from your .code-workspace file.",
-    "diagnostics.removePythonPathCodeWorkspaceAndSettingsJson": "The setting \"python.pythonPath\" defined in your workspace settings is now deprecated. Do you want us to delete it? This will only remove the \"python.pythonPath\" entry from your settings.json and .code-workspace file.",
+    "diagnostics.removePythonPathSettingsJson": "The setting \"python.pythonPath\" defined in your settings.json is now deprecated. Do you want us to delete it from your settings.json only? [Learn more](https://aka.ms/AA7jfor).",
+    "diagnostics.removePythonPathCodeWorkspace": "The setting \"python.pythonPath\" defined in your workspace settings is now deprecated. Do you want us to delete it from your .code-workspace file only? [Learn more](https://aka.ms/AA7jfor).",
+    "diagnostics.removePythonPathCodeWorkspaceAndSettingsJson": "The setting \"python.pythonPath\" defined in your workspace settings is now deprecated. Do you want us to delete it from your .code-workspace file and settings.json? [Learn more](https://aka.ms/AA7jfor).",
     "diagnostics.warnSourceMaps": "Source map support is enabled in the Python Extension, this will adversely impact performance of the extension.",
     "diagnostics.disableSourceMaps": "Disable Source Map Support",
     "diagnostics.warnBeforeEnablingSourceMaps": "Enabling source map support in the Python Extension will adversely impact performance of the extension.",

src/client/activation/activationManager.ts

@@ -73,7 +73,7 @@ export class ExtensionActivationManager implements IExtensionActivationManager {
         this.experiments.sendTelemetryIfInExperiment(DeprecatePythonPath.control);
 
         // Get latest interpreter list in the background.
-        this.interpreterService.getInterpreters(resource, { onActivation: true }).ignoreErrors();
+        this.interpreterService.getInterpreters(resource).ignoreErrors();
 
         await sendActivationTelemetry(this.fileSystem, this.workspaceService, resource);
 

src/client/application/diagnostics/checks/macPythonInterpreter.ts

@@ -101,7 +101,7 @@ export class InvalidMacPythonInterpreterService extends BaseDiagnosticsService {
             return [];
         }
 
-        const interpreters = await this.interpreterService.getInterpreters(resource, { onActivation: true });
+        const interpreters = await this.interpreterService.getInterpreters(resource);
         if (interpreters.filter((i) => !this.helper.isMacDefaultPythonPath(i.path)).length === 0) {
             return [
                 new InvalidMacPythonInterpreterDiagnostic(

src/client/application/diagnostics/checks/pythonPathDeprecated.ts

@@ -9,7 +9,6 @@ import { IWorkspaceService } from '../../../common/application/types';
 import { DeprecatePythonPath } from '../../../common/experimentGroups';
 import { IDisposableRegistry, IExperimentsManager, Resource } from '../../../common/types';
 import { Common, Diagnostics } from '../../../common/utils/localize';
-import { learnMoreOnInterpreterSecurityURI } from '../../../interpreter/autoSelection/constants';
 import { IServiceContainer } from '../../../ioc/types';
 import { BaseDiagnostic, BaseDiagnosticsService } from '../base';
 import { IDiagnosticsCommandFactory } from '../commands/types';
@@ -97,13 +96,6 @@ export class PythonPathDeprecatedDiagnosticService extends BaseDiagnosticsServic
             {
                 prompt: Common.noIWillDoItLater()
             },
-            {
-                prompt: Common.moreInfo(),
-                command: commandFactory.createCommand(diagnostic, {
-                    type: 'launch',
-                    options: learnMoreOnInterpreterSecurityURI
-                })
-            },
             {
                 prompt: Common.doNotShowAgain(),
                 command: commandFactory.createCommand(diagnostic, { type: 'ignore', options: DiagnosticScope.Global })

src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts

@@ -3,10 +3,16 @@
 
 'use strict';
 
-import { inject, injectable } from 'inversify';
+import { inject, injectable, named } from 'inversify';
 import { Uri } from 'vscode';
 import '../../../common/extensions';
-import { IInterpreterService, InterpreterType, IPipEnvService } from '../../../interpreter/contracts';
+import {
+    IInterpreterLocatorService,
+    IInterpreterService,
+    InterpreterType,
+    IPipEnvService,
+    PIPENV_SERVICE
+} from '../../../interpreter/contracts';
 import { IWorkspaceService } from '../../application/types';
 import { IFileSystem } from '../../platform/types';
 import { ITerminalActivationCommandProvider, TerminalShellType } from '../types';
@@ -15,7 +21,9 @@ import { ITerminalActivationCommandProvider, TerminalShellType } from '../types'
 export class PipEnvActivationCommandProvider implements ITerminalActivationCommandProvider {
     constructor(
         @inject(IInterpreterService) private readonly interpreterService: IInterpreterService,
-        @inject(IPipEnvService) private readonly pipenvService: IPipEnvService,
+        @inject(IInterpreterLocatorService)
+        @named(PIPENV_SERVICE)
+        private readonly pipenvService: IPipEnvService,
         @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService,
         @inject(IFileSystem) private readonly fs: IFileSystem
     ) {}

src/client/common/utils/localize.ts

@@ -32,15 +32,15 @@ export namespace Diagnostics {
     );
     export const removePythonPathSettingsJson = localize(
         'diagnostics.removePythonPathSettingsJson',
-        'The setting "python.pythonPath" defined in your workspace settings is now deprecated. Do you want us to delete it? This will only remove the "python.pythonPath" entry from your settings.json.'
+        'The setting "python.pythonPath" defined in your settings.json is now deprecated. Do you want us to delete it from your settings.json only? [Learn more](https://aka.ms/AA7jfor).'
     );
     export const removePythonPathCodeWorkspace = localize(
         'diagnostics.removePythonPathCodeWorkspace',
-        'The setting "python.pythonPath" defined in your workspace settings is now deprecated. Do you want us to delete it? This will only remove the "python.pythonPath" entry from your .code-workspace file.'
+        'The setting "python.pythonPath" defined in your workspace settings is now deprecated. Do you want us to delete it from your .code-workspace file only? [Learn more](https://aka.ms/AA7jfor).'
     );
     export const removePythonPathCodeWorkspaceAndSettingsJson = localize(
         'diagnostics.removePythonPathCodeWorkspaceAndSettingsJson',
-        'The setting "python.pythonPath" defined in your workspace settings is now deprecated. Do you want us to delete it? This will only remove the "python.pythonPath" entry from your settings.json and .code-workspace file.'
+        'The setting "python.pythonPath" defined in your workspace settings is now deprecated. Do you want us to delete it from your .code-workspace file and settings.json? [Learn more](https://aka.ms/AA7jfor).'
     );
     export const invalidPythonPathInDebuggerSettings = localize(
         'diagnostics.invalidPythonPathInDebuggerSettings',

src/client/interpreter/autoSelection/rules/system.ts

@@ -25,7 +25,7 @@ export class SystemWideInterpretersAutoSelectionRule extends BaseRuleService {
         resource: Resource,
         manager?: IInterpreterAutoSelectionService
     ): Promise<NextAction> {
-        const interpreters = await this.interpreterService.getInterpreters(resource, { onActivation: true });
+        const interpreters = await this.interpreterService.getInterpreters(resource);
         // Exclude non-local interpreters.
         const filteredInterpreters = interpreters.filter(
             (int) =>

src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts

@@ -27,7 +27,7 @@ export class InterpreterSelector implements IInterpreterSelector {
     }
 
     public async getSuggestions(resource: Resource) {
-        let interpreters = await this.interpreterManager.getInterpreters(resource);
+        let interpreters = await this.interpreterManager.getInterpreters(resource, { onSuggestion: true });
         if (this.experimentsManager.inExperiment(DeprecatePythonPath.experiment)) {
             interpreters = interpreters.filter((item) => this.interpreterSecurityService.isSafe(item) !== false);
         }

src/client/interpreter/contracts.ts

@@ -28,7 +28,7 @@ export interface IVirtualEnvironmentsSearchPathProvider {
 }
 
 export type GetInterpreterOptions = {
-    onActivation?: boolean;
+    onSuggestion?: boolean;
 };
 
 export type GetInterpreterLocatorOptions = GetInterpreterOptions & { ignoreCache?: boolean };
@@ -38,6 +38,7 @@ export const IInterpreterLocatorService = Symbol('IInterpreterLocatorService');
 export interface IInterpreterLocatorService extends Disposable {
     readonly onLocating: Event<Promise<PythonInterpreter[]>>;
     readonly hasInterpreters: Promise<boolean>;
+    didTriggerInterpreterSuggestions?: boolean;
     getInterpreters(resource?: Uri, options?: GetInterpreterLocatorOptions): Promise<PythonInterpreter[]>;
 }
 
@@ -126,7 +127,7 @@ export interface IInterpreterHelper {
 }
 
 export const IPipEnvService = Symbol('IPipEnvService');
-export interface IPipEnvService {
+export interface IPipEnvService extends IInterpreterLocatorService {
     executable: string;
     isRelatedPipEnvironment(dir: string, pythonPath: string): Promise<boolean>;
 }

src/client/interpreter/locators/index.ts

@@ -30,10 +30,13 @@ const flatten = require('lodash/flatten') as typeof import('lodash/flatten');
  */
 @injectable()
 export class PythonInterpreterLocatorService implements IInterpreterLocatorService {
+    public didTriggerInterpreterSuggestions: boolean;
+
     private readonly disposables: Disposable[] = [];
     private readonly platform: IPlatformService;
     private readonly interpreterLocatorHelper: IInterpreterLocatorHelper;
     private readonly _hasInterpreters: Deferred<boolean>;
+
     constructor(
         @inject(IServiceContainer) private serviceContainer: IServiceContainer,
         @inject(InterpreterFilter) private readonly interpreterFilter: IInterpreterFilter
@@ -42,6 +45,7 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi
         serviceContainer.get<Disposable[]>(IDisposableRegistry).push(this);
         this.platform = serviceContainer.get<IPlatformService>(IPlatformService);
         this.interpreterLocatorHelper = serviceContainer.get<IInterpreterLocatorHelper>(IInterpreterLocatorHelper);
+        this.didTriggerInterpreterSuggestions = false;
     }
     /**
      * This class should never emit events when we're locating.
@@ -106,18 +110,27 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi
         // The order is important because the data sources at the bottom of the list do not contain all,
         //  the information about the interpreters (e.g. type, environment name, etc).
         // This way, the items returned from the top of the list will win, when we combine the items returned.
-        const keys: [string | undefined, OSType | undefined][] = [
+        const keys: [string, OSType | undefined][] = [
             [WINDOWS_REGISTRY_SERVICE, OSType.Windows],
             [CONDA_ENV_SERVICE, undefined],
             [CONDA_ENV_FILE_SERVICE, undefined],
-            options?.onActivation ? [undefined, undefined] : [PIPENV_SERVICE, undefined],
+            [PIPENV_SERVICE, undefined],
             [GLOBAL_VIRTUAL_ENV_SERVICE, undefined],
             [WORKSPACE_VIRTUAL_ENV_SERVICE, undefined],
             [KNOWN_PATH_SERVICE, undefined],
             [CURRENT_PATH_SERVICE, undefined]
         ];
-        return keys
-            .filter((item) => item[0] !== undefined && (item[1] === undefined || item[1] === this.platform.osType))
+
+        const locators = keys
+            .filter((item) => item[1] === undefined || item[1] === this.platform.osType)
             .map((item) => this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, item[0]));
+
+        // Set it to true the first time the user selects an interpreter
+        if (!this.didTriggerInterpreterSuggestions && options?.onSuggestion === true) {
+            this.didTriggerInterpreterSuggestions = true;
+            locators.forEach((locator) => (locator.didTriggerInterpreterSuggestions = true));
+        }
+
+        return locators;
     }
 }