Skip to content

Commit f043f49

Browse files
TyriarTyriar
authored
Merge pull request microsoft#182883 from microsoft/tyriar/179476
EnvironmentVariableMutatorOptions API proposal
2 parents b7e813f + 438eb68 commit f043f49

File tree

8 files changed

+175
-90
lines changed

8 files changed

+175
-90
lines changed

‎extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -850,17 +850,17 @@ import { assertNoRpc, poll } from '../utils';
850850
collection.prepend('C', '~c2~');
851851

852852
// Verify get
853-
deepStrictEqual(collection.get('A'), { value: '~a2~', type: EnvironmentVariableMutatorType.Replace, scope: undefined });
854-
deepStrictEqual(collection.get('B'), { value: '~b2~', type: EnvironmentVariableMutatorType.Append, scope: undefined });
855-
deepStrictEqual(collection.get('C'), { value: '~c2~', type: EnvironmentVariableMutatorType.Prepend, scope: undefined });
853+
deepStrictEqual(collection.get('A'), { value: '~a2~', type: EnvironmentVariableMutatorType.Replace, scope: undefined, options: {} });
854+
deepStrictEqual(collection.get('B'), { value: '~b2~', type: EnvironmentVariableMutatorType.Append, scope: undefined, options: {} });
855+
deepStrictEqual(collection.get('C'), { value: '~c2~', type: EnvironmentVariableMutatorType.Prepend, scope: undefined, options: {} });
856856

857857
// Verify forEach
858858
const entries: [string, EnvironmentVariableMutator][] = [];
859859
collection.forEach((v, m) => entries.push([v, m]));
860860
deepStrictEqual(entries, [
861-
['A', { value: '~a2~', type: EnvironmentVariableMutatorType.Replace, scope: undefined }],
862-
['B', { value: '~b2~', type: EnvironmentVariableMutatorType.Append, scope: undefined }],
863-
['C', { value: '~c2~', type: EnvironmentVariableMutatorType.Prepend, scope: undefined }]
861+
['A', { value: '~a2~', type: EnvironmentVariableMutatorType.Replace, scope: undefined, options: {} }],
862+
['B', { value: '~b2~', type: EnvironmentVariableMutatorType.Append, scope: undefined, options: {} }],
863+
['C', { value: '~c2~', type: EnvironmentVariableMutatorType.Prepend, scope: undefined, options: {} }]
864864
]);
865865
});
866866
});

‎src/vs/platform/terminal/common/environmentVariable.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,24 +11,24 @@ export enum EnvironmentVariableMutatorType {
1111
Append = 2,
1212
Prepend = 3
1313
}
14-
// export enum EnvironmentVariableMutatorTiming {
15-
// AtSpawn = 1,
16-
// AfterShellIntegration = 2
17-
// // TODO: Do we need a both?
18-
// }
1914
export interface IEnvironmentVariableMutator {
2015
readonly variable: string;
2116
readonly value: string;
2217
readonly type: EnvironmentVariableMutatorType;
2318
readonly scope?: EnvironmentVariableScope;
24-
// readonly timing?: EnvironmentVariableMutatorTiming;
19+
readonly options?: IEnvironmentVariableMutatorOptions;
2520
}
2621

2722
export interface IEnvironmentDescriptionMutator {
2823
readonly description: string | undefined;
2924
readonly scope?: EnvironmentVariableScope;
3025
}
3126

27+
export interface IEnvironmentVariableMutatorOptions {
28+
applyAtProcessCreation?: boolean;
29+
applyAtShellIntegration?: boolean;
30+
}
31+
3232
export type EnvironmentVariableScope = {
3333
workspaceFolder?: IWorkspaceFolderData;
3434
};

‎src/vs/platform/terminal/common/environmentVariableCollection.ts

Lines changed: 29 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@ import { EnvironmentVariableMutatorType, EnvironmentVariableScope, IEnvironmentV
88

99
type VariableResolver = (str: string) => Promise<string>;
1010

11-
// const mutatorTypeToLabelMap: Map<EnvironmentVariableMutatorType, string> = new Map([
12-
// [EnvironmentVariableMutatorType.Append, 'APPEND'],
13-
// [EnvironmentVariableMutatorType.Prepend, 'PREPEND'],
14-
// [EnvironmentVariableMutatorType.Replace, 'REPLACE']
15-
// ]);
11+
const mutatorTypeToLabelMap: Map<EnvironmentVariableMutatorType, string> = new Map([
12+
[EnvironmentVariableMutatorType.Append, 'APPEND'],
13+
[EnvironmentVariableMutatorType.Prepend, 'PREPEND'],
14+
[EnvironmentVariableMutatorType.Replace, 'REPLACE']
15+
]);
1616

1717
export class MergedEnvironmentVariableCollection implements IMergedEnvironmentVariableCollection {
1818
private readonly map: Map<string, IExtensionOwnedEnvironmentVariableMutator[]> = new Map();
@@ -46,7 +46,8 @@ export class MergedEnvironmentVariableCollection implements IMergedEnvironmentVa
4646
value: mutator.value,
4747
type: mutator.type,
4848
scope: mutator.scope,
49-
variable: mutator.variable
49+
variable: mutator.variable,
50+
options: mutator.options
5051
};
5152
if (!extensionMutator.scope) {
5253
delete extensionMutator.scope; // Convenient for tests
@@ -69,26 +70,33 @@ export class MergedEnvironmentVariableCollection implements IMergedEnvironmentVa
6970
const actualVariable = isWindows ? lowerToActualVariableNames![variable.toLowerCase()] || variable : variable;
7071
for (const mutator of mutators) {
7172
const value = variableResolver ? await variableResolver(mutator.value) : mutator.value;
72-
// if (mutator.timing === EnvironmentVariableMutatorTiming.AfterShellIntegration) {
73-
// const key = `VSCODE_ENV_${mutatorTypeToLabelMap.get(mutator.type)!}`;
74-
// env[key] = (env[key] ? env[key] + ':' : '') + variable + '=' + value;
75-
// continue;
76-
// }
77-
switch (mutator.type) {
78-
case EnvironmentVariableMutatorType.Append:
79-
env[actualVariable] = (env[actualVariable] || '') + value;
80-
break;
81-
case EnvironmentVariableMutatorType.Prepend:
82-
env[actualVariable] = value + (env[actualVariable] || '');
83-
break;
84-
case EnvironmentVariableMutatorType.Replace:
85-
env[actualVariable] = value;
86-
break;
73+
// Default: true
74+
if (mutator.options?.applyAtProcessCreation ?? true) {
75+
switch (mutator.type) {
76+
case EnvironmentVariableMutatorType.Append:
77+
env[actualVariable] = (env[actualVariable] || '') + value;
78+
break;
79+
case EnvironmentVariableMutatorType.Prepend:
80+
env[actualVariable] = value + (env[actualVariable] || '');
81+
break;
82+
case EnvironmentVariableMutatorType.Replace:
83+
env[actualVariable] = value;
84+
break;
85+
}
86+
}
87+
// Default: false
88+
if (mutator.options?.applyAtShellIntegration ?? false) {
89+
const key = `VSCODE_ENV_${mutatorTypeToLabelMap.get(mutator.type)!}`;
90+
env[key] = (env[key] ? env[key] + ':' : '') + variable + '=' + this._encodeColons(value);
8791
}
8892
}
8993
}
9094
}
9195

96+
private _encodeColons(value: string): string {
97+
return value.replaceAll(':', '\\x3a');
98+
}
99+
92100
diff(other: IMergedEnvironmentVariableCollection, scope: EnvironmentVariableScope | undefined): IMergedEnvironmentVariableCollectionDiff | undefined {
93101
const added: Map<string, IExtensionOwnedEnvironmentVariableMutator[]> = new Map();
94102
const changed: Map<string, IExtensionOwnedEnvironmentVariableMutator[]> = new Map();

‎src/vs/workbench/api/common/extHostTerminalService.ts

Lines changed: 38 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { withNullAsUndefined } from 'vs/base/common/types';
2525
import { Promises } from 'vs/base/common/async';
2626
import { EditorGroupColumn } from 'vs/workbench/services/editor/common/editorGroupColumn';
2727
import { ViewColumn } from 'vs/workbench/api/common/extHostTypeConverters';
28+
import { isProposedApiEnabled } from 'vs/workbench/services/extensions/common/extensions';
2829

2930
export interface IExtHostTerminalService extends ExtHostTerminalServiceShape, IDisposable {
3031

@@ -826,7 +827,7 @@ export abstract class BaseExtHostTerminalService extends Disposable implements I
826827
public getEnvironmentVariableCollection(extension: IExtensionDescription): IEnvironmentVariableCollection {
827828
let collection = this._environmentVariableCollections.get(extension.identifier.value);
828829
if (!collection) {
829-
collection = new EnvironmentVariableCollection();
830+
collection = new EnvironmentVariableCollection(extension);
830831
this._setEnvironmentVariableCollection(extension.identifier.value, collection);
831832
}
832833
return collection.getScopedEnvironmentVariableCollection(undefined);
@@ -841,7 +842,7 @@ export abstract class BaseExtHostTerminalService extends Disposable implements I
841842
public $initEnvironmentVariableCollections(collections: [string, ISerializableEnvironmentVariableCollection][]): void {
842843
collections.forEach(entry => {
843844
const extensionIdentifier = entry[0];
844-
const collection = new EnvironmentVariableCollection(entry[1]);
845+
const collection = new EnvironmentVariableCollection(undefined, entry[1]);
845846
this._setEnvironmentVariableCollection(extensionIdentifier, collection);
846847
});
847848
}
@@ -883,6 +884,11 @@ class EnvironmentVariableCollection {
883884
get onDidChangeCollection(): Event<void> { return this._onDidChangeCollection && this._onDidChangeCollection.event; }
884885

885886
constructor(
887+
// HACK: Only check proposed options if extension is set (when the collection is not
888+
// restored by serialization). This saves us from getting the extension details and
889+
// shouldn't ever happen since you can only set them initially via the proposed check.
890+
// TODO: This should be removed when the env var extension API(s) are stabilized
891+
private readonly _extension: IExtensionDescription | undefined,
886892
serialized?: ISerializableEnvironmentVariableCollection
887893
) {
888894
this.map = new Map(serialized);
@@ -899,19 +905,31 @@ class EnvironmentVariableCollection {
899905
return scopedCollection;
900906
}
901907

902-
replace(variable: string, value: string, scope: vscode.EnvironmentVariableScope | undefined): void {
903-
this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Replace, scope });
908+
replace(variable: string, value: string, options: vscode.EnvironmentVariableMutatorOptions | undefined, scope: vscode.EnvironmentVariableScope | undefined): void {
909+
if (this._extension && options) {
910+
isProposedApiEnabled(this._extension, 'envCollectionOptions');
911+
}
912+
this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Replace, options: options ?? {}, scope });
904913
}
905914

906-
append(variable: string, value: string, scope: vscode.EnvironmentVariableScope | undefined): void {
907-
this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Append, scope });
915+
append(variable: string, value: string, options: vscode.EnvironmentVariableMutatorOptions | undefined, scope: vscode.EnvironmentVariableScope | undefined): void {
916+
if (this._extension && options) {
917+
isProposedApiEnabled(this._extension, 'envCollectionOptions');
918+
}
919+
this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Append, options: options ?? {}, scope });
908920
}
909921

910-
prepend(variable: string, value: string, scope: vscode.EnvironmentVariableScope | undefined): void {
911-
this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Prepend, scope });
922+
prepend(variable: string, value: string, options: vscode.EnvironmentVariableMutatorOptions | undefined, scope: vscode.EnvironmentVariableScope | undefined): void {
923+
if (this._extension && options) {
924+
isProposedApiEnabled(this._extension, 'envCollectionOptions');
925+
}
926+
this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Prepend, options: options ?? {}, scope });
912927
}
913928

914929
private _setIfDiffers(variable: string, mutator: vscode.EnvironmentVariableMutator & { scope: vscode.EnvironmentVariableScope | undefined }): void {
930+
if (mutator.options && mutator.options.applyAtProcessCreation === false && !mutator.options.applyAtShellIntegration) {
931+
throw new Error('EnvironmentVariableMutatorOptions must apply at either process creation or shell integration');
932+
}
915933
if (!mutator.scope) {
916934
delete (mutator as any).scope; // Convenient for tests
917935
}
@@ -928,6 +946,7 @@ class EnvironmentVariableCollection {
928946
get(variable: string, scope: vscode.EnvironmentVariableScope | undefined): vscode.EnvironmentVariableMutator | undefined {
929947
const key = this.getKey(variable, scope);
930948
const value = this.map.get(key);
949+
// TODO: Set options to defaults if needed
931950
return value ? convertMutator(value) : undefined;
932951
}
933952

@@ -944,11 +963,11 @@ class EnvironmentVariableCollection {
944963
return workspaceFolder ? workspaceFolder.uri.toString() : undefined;
945964
}
946965

947-
public getVariableMap(scope: vscode.EnvironmentVariableScope | undefined): Map<string, IEnvironmentVariableMutator> {
948-
const map = new Map<string, IEnvironmentVariableMutator>();
966+
public getVariableMap(scope: vscode.EnvironmentVariableScope | undefined): Map<string, vscode.EnvironmentVariableMutator> {
967+
const map = new Map<string, vscode.EnvironmentVariableMutator>();
949968
for (const [key, value] of this.map) {
950969
if (this.getScopeKey(value.scope) === this.getScopeKey(scope)) {
951-
map.set(key, value);
970+
map.set(key, convertMutator(value));
952971
}
953972
}
954973
return map;
@@ -1022,24 +1041,24 @@ class ScopedEnvironmentVariableCollection implements vscode.EnvironmentVariableC
10221041
return this.collection.getScopedEnvironmentVariableCollection(this.scope);
10231042
}
10241043

1025-
replace(variable: string, value: string): void {
1026-
this.collection.replace(variable, value, this.scope);
1044+
replace(variable: string, value: string, options?: vscode.EnvironmentVariableMutatorOptions | undefined): void {
1045+
this.collection.replace(variable, value, options, this.scope);
10271046
}
10281047

1029-
append(variable: string, value: string): void {
1030-
this.collection.append(variable, value, this.scope);
1048+
append(variable: string, value: string, options?: vscode.EnvironmentVariableMutatorOptions | undefined): void {
1049+
this.collection.append(variable, value, options, this.scope);
10311050
}
10321051

1033-
prepend(variable: string, value: string): void {
1034-
this.collection.prepend(variable, value, this.scope);
1052+
prepend(variable: string, value: string, options?: vscode.EnvironmentVariableMutatorOptions | undefined): void {
1053+
this.collection.prepend(variable, value, options, this.scope);
10351054
}
10361055

10371056
get(variable: string): vscode.EnvironmentVariableMutator | undefined {
10381057
return this.collection.get(variable, this.scope);
10391058
}
10401059

10411060
forEach(callback: (variable: string, mutator: vscode.EnvironmentVariableMutator, collection: vscode.EnvironmentVariableCollection) => any, thisArg?: any): void {
1042-
this.collection.getVariableMap(this.scope).forEach((value, variable) => callback.call(thisArg, variable, convertMutator(value), this), this.scope);
1061+
this.collection.getVariableMap(this.scope).forEach((value, variable) => callback.call(thisArg, variable, value, this), this.scope);
10431062
}
10441063

10451064
[Symbol.iterator](): IterableIterator<[variable: string, mutator: vscode.EnvironmentVariableMutator]> {
@@ -1102,6 +1121,7 @@ function asTerminalColor(color?: vscode.ThemeColor): ThemeColor | undefined {
11021121
function convertMutator(mutator: IEnvironmentVariableMutator): vscode.EnvironmentVariableMutator {
11031122
const newMutator = { ...mutator };
11041123
newMutator.scope = newMutator.scope ?? undefined;
1124+
newMutator.options = newMutator.options ?? undefined;
11051125
delete (newMutator as any).variable;
11061126
return newMutator as vscode.EnvironmentVariableMutator;
11071127
}

0 commit comments

Comments
 (0)