8000 Update proposed API for env collection · microsoft/vscode-python@3d1709f · GitHub
[go: up one dir, main page]

Skip to content

Commit 3d1709f

Browse files
author
Kartik Raj
committed
Update proposed API for env collection
1 parent 021b362 commit 3d1709f

File tree

2 files changed

+80
-59
lines changed

2 files changed

+80
-59
lines changed

src/client/interpreter/activation/terminalEnvVarCollectionService.ts

Lines changed: 53 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,14 @@
33

44
import * as path from 'path';
55
import { inject, injectable } from 'inversify';
6-
import { ProgressOptions, ProgressLocation, MarkdownString, WorkspaceFolder } from 'vscode';
6+
import {
7+
ProgressOptions,
8+
ProgressLocation,
9+
MarkdownString,
10+
WorkspaceFolder,
11+
GlobalEnvironmentVariableCollection,
12+
EnvironmentVariableScope,
13+
} from 'vscode';
714
import { pathExists } from 'fs-extra';
815
import { IExtensionActivationService } from '../../activation/types';
916
import { IApplicationShell, IApplicationEnvironment, IWorkspaceService } from '../../common/application/types';
@@ -20,7 +27,7 @@ import {
2027
} from '../../common/types';
2128
import { Deferred, createDeferred } from '../../common/utils/async';
2229
import { Interpreters } from '../../common/utils/localize';
23-
import { traceDecoratorVerbose, traceVerbose, traceWarn } from '../../logging';
30+
import { traceDecoratorVerbose, traceError, traceVerbose, traceWarn } from '../../logging';
2431
import { IInterpreterService } from '../contracts';
2532
import { defaultShells } from './service';
2633
import { IEnvironmentActivationService, ITerminalEnvVarCollectionService } from './types';
@@ -61,52 +68,57 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
6168
) {}
6269

6370
public async activate(resource: Resource): Promise<void> {
64-
if (!inTerminalEnvVarExperiment(this.experimentService)) {
65-
this.context.environmentVariableCollection.clear();
66-
await this.handleMicroVenv(resource);
71+
// TODO: Remove try...catch once proposed APIs being used are finalized.
72+
try {
73+
if (!inTerminalEnvVarExperiment(this.experimentService)) {
74+
this.context.environmentVariableCollection.clear();
75+
await this.handleMicroVenv(resource);
76+
if (!this.registeredOnce) {
77+
this.interpreterService.onDidChangeInterpreter(
78+
async (r) => {
79+
await this.handleMicroVenv(r);
80+
},
81+
this,
82+
this.disposables,
83+
);
84+
this.registeredOnce = true;
85+
}
86+
return;
87+
}
6788
if (!this.registeredOnce) {
6889
this.interpreterService.onDidChangeInterpreter(
6990
async (r) => {
70-
await this.handleMicroVenv(r);
91+
this.showProgress();
92+
await this._applyCollection(r).ignoreErrors();
93+
this.hideProgress();
94+
},
95+
this,
96+
this.disposables,
97+
);
98+
this.applicationEnvironment.onDidChangeShell(
99+
async (shell: string) => {
100+
this.showProgress();
101+
this.processEnvVars = undefined;
102+
// Pass in the shell where known instead of relying on the application environment, because of bug
103+
// on VSCode: https://github.com/microsoft/vscode/issues/160694
104+
await this._applyCollection(undefined, shell).ignoreErrors();
105+
this.hideProgress();
71106
},
72107
this,
73108
this.disposables,
74109
);
75110
this.registeredOnce = true;
76111
}
77-
return;
78-
}
79-
if (!this.registeredOnce) {
80-
this.interpreterService.onDidChangeInterpreter(
81-
async (r) => {
82-
this.showProgress();
83-
await this._applyCollection(r).ignoreErrors();
84-
this.hideProgress();
85-
},
86-
this,
87-
this.disposables,
88-
);
89-
this.applicationEnvironment.onDidChangeShell(
90-
async (shell: string) => {
91-
this.showProgress();
92-
this.processEnvVars = undefined;
93-
// Pass in the shell where known instead of relying on the application environment, because of bug
94-
// on VSCode: https://github.com/microsoft/vscode/issues/160694
95-
await this._applyCollection(undefined, shell).ignoreErrors();
96-
this.hideProgress();
97-
},
98-
this,
99-
this.disposables,
100-
);
101-
this.registeredOnce = true;
112+
this._applyCollection(resource).ignoreErrors();
113+
} catch (ex) {
114+
traceError(`Activating terminal env collection failed`, ex);
102115
}
103-
this._applyCollection(resource).ignoreErrors();
104116
}
105117

106118
public async _applyCollection(resource: Resource, shell = this.applicationEnvironment.shell): Promise<void> {
107119
const workspaceFolder = this.getWorkspaceFolder(resource);
108120
const settings = this.configurationService.getSettings(resource);
109-
const envVarCollection = this.context.getEnvironmentVariableCollection({ workspaceFolder });
93D4
121+
const envVarCollection = this.getEnvironmentVariableCollection({ workspaceFolder });
110122
// Clear any previously set env vars from collection
111123
envVarCollection.clear();
112124
if (!settings.terminal.activateEnvironment) {
@@ -232,22 +244,27 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
232244
if (interpreter?.envType === EnvironmentType.Venv) {
233245
const activatePath = path.join(path.dirname(interpreter.path), 'activate');
234246
if (!(await pathExists(activatePath))) {
235-
const envVarCollection = this.context.getEnvironmentVariableCollection({ workspaceFolder });
247+
const envVarCollection = this.getEnvironmentVariableCollection({ workspaceFolder });
236248
const pathVarName = getSearchPathEnvVarNames()[0];
237249
envVarCollection.replace(
238250
'PATH',
239251
`${path.dirname(interpreter.path)}${path.delimiter}${process.env[pathVarName]}`,
240-
{ applyAtShellIntegration: true, applyAtProcessCreation: true },
252+
{ applyAtShellIntegration: true },
241253
);
242254
return;
243255
}
244-
this.context.getEnvironmentVariableCollection({ workspaceFolder }).clear();
245256
}
257+
this.getEnvironmentVariableCollection({ workspaceFolder }).clear();
246258
} catch (ex) {
247259
traceWarn(`Microvenv failed as it is using proposed API which is constantly changing`, ex);
248260
}
249261
}
250262

263+
private getEnvironmentVariableCollection(scope?: EnvironmentVariableScope) {
264+
const envVarCollection = this.context.environmentVariableCollection as GlobalEnvironmentVariableCollection;
265+
return scope?.workspaceFolder ? envVarCollection.getScoped(scope) : envVarCollection;
266+
}
267+
251268
private getWorkspaceFolder(resource: Resource): WorkspaceFolder | undefined {
252269
let workspaceFolder = this.workspaceService.getWorkspaceFolder(resource);
253270
if (

types/vscode.proposed.envCollectionWorkspace.d.ts

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4,30 +4,34 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
declare module 'vscode' {
7+
// https://github.com/microsoft/vscode/issues/171173
78

8-
// https://github.com/microsoft/vscode/issues/182069
9+
// export interface ExtensionContext {
10+
// /**
11+
// * Gets the extension's global environment variable collection for this workspace, enabling changes to be
12+
// * applied to terminal environment variables.
13+
// */
14+
// readonly environmentVariableCollection: GlobalEnvironmentVariableCollection;
15+
// }
916

10-
export interface ExtensionContext {
11-
/**
12-
* Gets the extension's environment variable collection for this workspace, enabling changes
13-
* to be applied to terminal environment variables.
14-
*
15-
* @deprecated Use {@link getEnvironmentVariableCollection} instead.
16-
*/
17-
readonly environmentVariableCollection: EnvironmentVariableCollection;
18-
/**
19-
* Gets the extension's environment variable collection for this workspace, enabling changes
20-
* to be applied to terminal environment variables.
21< E9AB /td>-
*
22-
* @param scope The scope to which the environment variable collection applies to.
23-
*/
24-
getEnvironmentVariableCollection(scope?: EnvironmentVariableScope): EnvironmentVariableCollection;
25-
}
17+
export interface GlobalEnvironmentVariableCollection extends EnvironmentVariableCollection {
18+
/**
19+
* Gets scope-specific environment variable collection for the extension. This enables alterations to
20+
* terminal environment variables solely within the designated scope, and is applied in addition to (and
21+
* after) the global collection.
22+
*
23+
* Each object obtained through this method is isolated and does not impact objects for other scopes,
24+
* including the global collection.
25+
*
26+
* @param scope The scope to which the environment variable collection applies to.
27+
*/
28+
getScoped(scope: EnvironmentVariableScope): EnvironmentVariableCollection;
29+
}
2630

27-
export type EnvironmentVariableScope = {
28-
/**
29-
* Any specific workspace folder to get collection for. If unspecified, collection applicable to all workspace folders is returned.
30-
*/
31-
workspaceFolder?: WorkspaceFolder;
32-
};
31+
export type EnvironmentVariableScope = {
32+
/**
33+
* Any specific workspace folder to get collection for. If unspecified, collection applicable to all workspace folders is returned.
34+
*/
35+
workspaceFolder?: WorkspaceFolder;
36+
};
3337
}

0 commit comments

Comments
 (0)
0