Skip to content

Commit 7984076

Browse files
kimadelinekimadeline
authored
Leave pipenv in a corner until the user decides to select an interpreter (microsoft#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
1 parent 5aaca27 commit 7984076

24 files changed

Lines changed: 308 additions & 210 deletions

File tree

‎src/client/activation/activationManager.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ export class ExtensionActivationManager implements IExtensionActivationManager {
7373
this.experiments.sendTelemetryIfInExperiment(DeprecatePythonPath.control);
7474

7575
// Get latest interpreter list in the background.
76-
this.interpreterService.getInterpreters(resource, { onActivation: true }).ignoreErrors();
76+
this.interpreterService.getInterpreters(resource).ignoreErrors();
7777

7878
await sendActivationTelemetry(this.fileSystem, this.workspaceService, resource);
7979

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ export class InvalidMacPythonInterpreterService extends BaseDiagnosticsService {
101101
return [];
102102
}
103103

104-
const interpreters = await this.interpreterService.getInterpreters(resource, { onActivation: true });
104+
const interpreters = await this.interpreterService.getInterpreters(resource);
105105
if (interpreters.filter((i) => !this.helper.isMacDefaultPythonPath(i.path)).length === 0) {
106106
return [
107107
new InvalidMacPythonInterpreterDiagnostic(

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,16 @@
33

44
'use strict';
55

6-
import { inject, injectable } from 'inversify';
6+
import { inject, injectable, named } from 'inversify';
77
import { Uri } from 'vscode';
88
import '../../../common/extensions';
9-
import { IInterpreterService, InterpreterType, IPipEnvService } from '../../../interpreter/contracts';
9+
import {
10+
IInterpreterLocatorService,
11+
IInterpreterService,
12+
InterpreterType,
13+
IPipEnvService,
14+
PIPENV_SERVICE
15+
} from '../../../interpreter/contracts';
1016
import { IWorkspaceService } from '../../application/types';
1117
import { IFileSystem } from '../../platform/types';
1218
import { ITerminalActivationCommandProvider, TerminalShellType } from '../types';
@@ -15,7 +21,9 @@ import { ITerminalActivationCommandProvider, TerminalShellType } from '../types'
1521
export class PipEnvActivationCommandProvider implements ITerminalActivationCommandProvider {
1622
constructor(
1723
@inject(IInterpreterService) private readonly interpreterService: IInterpreterService,
18-
@inject(IPipEnvService) private readonly pipenvService: IPipEnvService,
24+
@inject(IInterpreterLocatorService)
25+
@named(PIPENV_SERVICE)
26+
private readonly pipenvService: IPipEnvService,
1927
@inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService,
2028
@inject(IFileSystem) private readonly fs: IFileSystem
2129
) {}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ export class SystemWideInterpretersAutoSelectionRule extends BaseRuleService {
2525
resource: Resource,
2626
manager?: IInterpreterAutoSelectionService
2727
): Promise<NextAction> {
28-
const interpreters = await this.interpreterService.getInterpreters(resource, { onActivation: true });
28+
const interpreters = await this.interpreterService.getInterpreters(resource);
2929
// Exclude non-local interpreters.
3030
const filteredInterpreters = interpreters.filter(
3131
(int) =>

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export class InterpreterSelector implements IInterpreterSelector {
2727
}
2828

2929
public async getSuggestions(resource: Resource) {
30-
let interpreters = await this.interpreterManager.getInterpreters(resource);
30+
let interpreters = await this.interpreterManager.getInterpreters(resource, { onSuggestion: true });
3131
if (this.experimentsManager.inExperiment(DeprecatePythonPath.experiment)) {
3232
interpreters = interpreters.filter((item) => this.interpreterSecurityService.isSafe(item) !== false);
3333
}

‎src/client/interpreter/contracts.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ export interface IVirtualEnvironmentsSearchPathProvider {
2828
}
2929

3030
export type GetInterpreterOptions = {
31-
onActivation?: boolean;
31+
onSuggestion?: boolean;
3232
};
3333

3434
export type GetInterpreterLocatorOptions = GetInterpreterOptions & { ignoreCache?: boolean };
@@ -38,6 +38,7 @@ export const IInterpreterLocatorService = Symbol('IInterpreterLocatorService');
3838
export interface IInterpreterLocatorService extends Disposable {
3939
readonly onLocating: Event<Promise<PythonInterpreter[]>>;
4040
readonly hasInterpreters: Promise<boolean>;
41+
didTriggerInterpreterSuggestions?: boolean;
4142
getInterpreters(resource?: Uri, options?: GetInterpreterLocatorOptions): Promise<PythonInterpreter[]>;
4243
}
4344

@@ -126,7 +127,7 @@ export interface IInterpreterHelper {
126127
}
127128

128129
export const IPipEnvService = Symbol('IPipEnvService');
129-
export interface IPipEnvService {
130+
export interface IPipEnvService extends IInterpreterLocatorService {
130131
executable: string;
131132
isRelatedPipEnvironment(dir: string, pythonPath: string): Promise<boolean>;
132133
}

‎src/client/interpreter/locators/index.ts‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,13 @@ const flatten = require('lodash/flatten') as typeof import('lodash/flatten');
3030
*/
3131
@injectable()
3232
export class PythonInterpreterLocatorService implements IInterpreterLocatorService {
33+
public didTriggerInterpreterSuggestions: boolean;
34+
3335
private readonly disposables: Disposable[] = [];
3436
private readonly platform: IPlatformService;
3537
private readonly interpreterLocatorHelper: IInterpreterLocatorHelper;
3638
private readonly _hasInterpreters: Deferred<boolean>;
39+
3740
constructor(
3841
@inject(IServiceContainer) private serviceContainer: IServiceContainer,
3942
@inject(InterpreterFilter) private readonly interpreterFilter: IInterpreterFilter
@@ -42,6 +45,7 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi
4245
serviceContainer.get<Disposable[]>(IDisposableRegistry).push(this);
4346
this.platform = serviceContainer.get<IPlatformService>(IPlatformService);
4447
this.interpreterLocatorHelper = serviceContainer.get<IInterpreterLocatorHelper>(IInterpreterLocatorHelper);
48+
this.didTriggerInterpreterSuggestions = false;
4549
}
4650
/**
4751
* This class should never emit events when we're locating.
@@ -106,18 +110,27 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi
106110
// The order is important because the data sources at the bottom of the list do not contain all,
107111
// the information about the interpreters (e.g. type, environment name, etc).
108112
// This way, the items returned from the top of the list will win, when we combine the items returned.
109-
const keys: [string | undefined, OSType | undefined][] = [
113+
const keys: [string, OSType | undefined][] = [
110114
[WINDOWS_REGISTRY_SERVICE, OSType.Windows],
111115
[CONDA_ENV_SERVICE, undefined],
112116
[CONDA_ENV_FILE_SERVICE, undefined],
113-
options?.onActivation ? [undefined, undefined] : [PIPENV_SERVICE, undefined],
117+
[PIPENV_SERVICE, undefined],
114118
[GLOBAL_VIRTUAL_ENV_SERVICE, undefined],
115119
[WORKSPACE_VIRTUAL_ENV_SERVICE, undefined],
116120
[KNOWN_PATH_SERVICE, undefined],
117121
[CURRENT_PATH_SERVICE, undefined]
118122
];
119-
return keys
120-
.filter((item) => item[0] !== undefined && (item[1] === undefined || item[1] === this.platform.osType))
123+
124+
const locators = keys
125+
.filter((item) => item[1] === undefined || item[1] === this.platform.osType)
121126
.map((item) => this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, item[0]));
127+
128+
// Set it to true the first time the user selects an interpreter
129+
if (!this.didTriggerInterpreterSuggestions && options?.onSuggestion === true) {
130+
this.didTriggerInterpreterSuggestions = true;
131+
locators.forEach((locator) => (locator.didTriggerInterpreterSuggestions = true));
132+
}
133+
134+
return locators;
122135
}
123136
}

‎src/client/interpreter/locators/services/cacheableLocatorService.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,24 @@ export abstract class CacheableLocatorService implements IInterpreterLocatorServ
7070
private readonly handlersAddedToResource = new Set<string>();
7171
private readonly cacheKeyPrefix: string;
7272
private readonly locating = new EventEmitter<Promise<PythonInterpreter[]>>();
73+
private _didTriggerInterpreterSuggestions: boolean;
74+
7375
constructor(
7476
@unmanaged() private readonly name: string,
7577
@unmanaged() protected readonly serviceContainer: IServiceContainer,
7678
@unmanaged() private cachePerWorkspace: boolean = false
7779
) {
7880
this._hasInterpreters = createDeferred<boolean>();
7981
this.cacheKeyPrefix = `INTERPRETERS_CACHE_v3_${name}`;
82+
this._didTriggerInterpreterSuggestions = false;
83+
}
84+
85+
public get didTriggerInterpreterSuggestions(): boolean {
86+
return this._didTriggerInterpreterSuggestions;
87+
}
88+
89+
public set didTriggerInterpreterSuggestions(value: boolean) {
90+
this._didTriggerInterpreterSuggestions = value;
8091
}
8192

8293
public get onLocating(): Event<Promise<PythonInterpreter[]>> {

‎src/client/interpreter/locators/services/pipEnvService.ts‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,15 @@ export class PipEnvService extends CacheableLocatorService implements IPipEnvSer
4343
this.configService = this.serviceContainer.get<IConfigurationService>(IConfigurationService);
4444
this.pipEnvServiceHelper = this.serviceContainer.get<IPipEnvServiceHelper>(IPipEnvServiceHelper);
4545
}
46+
4647
// tslint:disable-next-line:no-empty
4748
public dispose() {}
49+
4850
public async isRelatedPipEnvironment(dir: string, pythonPath: string): Promise<boolean> {
51+
if (!this.didTriggerInterpreterSuggestions) {
52+
return false;
53+
}
54+
4955
// In PipEnv, the name of the cwd is used as a prefix in the virtual env.
5056
if (pythonPath.indexOf(`${path.sep}${path.basename(dir)}-`) === -1) {
5157
return false;
@@ -55,10 +61,14 @@ export class PipEnvService extends CacheableLocatorService implements IPipEnvSer
5561
}
5662

5763
public get executable(): string {
58-
return this.configService.getSettings().pipenvPath;
64+
return this.didTriggerInterpreterSuggestions ? this.configService.getSettings().pipenvPath : '';
5965
}
6066

6167
public async getInterpreters(resource?: Uri, options?: GetInterpreterLocatorOptions): Promise<PythonInterpreter[]> {
68+
if (!this.didTriggerInterpreterSuggestions) {
69+
return [];
70+
}
71+
6272
const stopwatch = new StopWatch();
6373
const startDiscoveryTime = stopwatch.elapsedTime;
6474

@@ -71,6 +81,10 @@ export class PipEnvService extends CacheableLocatorService implements IPipEnvSer
7181
}
7282

7383
protected getInterpretersImplementation(resource?: Uri): Promise<PythonInterpreter[]> {
84+
if (!this.didTriggerInterpreterSuggestions) {
85+
return Promise.resolve([]);
86+
}
87+
7488
const pipenvCwd = this.getPipenvWorkingDirectory(resource);
7589
if (!pipenvCwd) {
7690
return Promise.resolve([]);
@@ -146,6 +160,7 @@ export class PipEnvService extends CacheableLocatorService implements IPipEnvSer
146160
}
147161
}
148162
}
163+
149164
private async checkIfPipFileExists(cwd: string): Promise<boolean> {
150165
const currentProcess = this.serviceContainer.get<ICurrentProcess>(ICurrentProcess);
151166
const pipFileName = currentProcess.env[pipEnvFileNameVariable];

‎src/client/interpreter/serviceRegistry.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ import {
6060
IInterpreterWatcherBuilder,
6161
IKnownSearchPathsForInterpreters,
6262
INTERPRETER_LOCATOR_SERVICE,
63-
IPipEnvService,
6463
IShebangCodeLensProvider,
6564
IVirtualEnvironmentsSearchPathProvider,
6665
KNOWN_PATH_SERVICE,
@@ -200,7 +199,6 @@ export function registerInterpreterTypes(serviceManager: IServiceManager) {
200199
WORKSPACE_VIRTUAL_ENV_SERVICE
201200
);
202201
serviceManager.addSingleton<IInterpreterLocatorService>(IInterpreterLocatorService, PipEnvService, PIPENV_SERVICE);
203-
serviceManager.addSingleton<IInterpreterLocatorService>(IPipEnvService, PipEnvService);
204202

205203
serviceManager.addSingleton<IInterpreterLocatorService>(
206204
IInterpreterLocatorService,

0 commit comments

Comments
 (0)