Skip to content

Commit

Permalink
Merge pull request #182883 from microsoft/tyriar/179476
Browse files Browse the repository at this point in the history 
EnvironmentVariableMutatorOptions API proposal
  • Loading branch information
@Tyriar
Tyriar authored May 23, 2023
2 parents b7e813f + 438eb68 commit f043f49
Show file tree
Hide file tree
Showing 8 changed files with 175 additions and 90 deletions.
12 changes: 6 additions & 6 deletions extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -850,17 +850,17 @@ import { assertNoRpc, poll } from '../utils';
collection.prepend('C', '~c2~');

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

// Verify forEach
const entries: [string, EnvironmentVariableMutator][] = [];
collection.forEach((v, m) => entries.push([v, m]));
deepStrictEqual(entries, [
['A', { value: '~a2~', type: EnvironmentVariableMutatorType.Replace, scope: undefined }],
['B', { value: '~b2~', type: EnvironmentVariableMutatorType.Append, scope: undefined }],
['C', { value: '~c2~', type: EnvironmentVariableMutatorType.Prepend, scope: undefined }]
['A', { value: '~a2~', type: EnvironmentVariableMutatorType.Replace, scope: undefined, options: {} }],
['B', { value: '~b2~', type: EnvironmentVariableMutatorType.Append, scope: undefined, options: {} }],
['C', { value: '~c2~', type: EnvironmentVariableMutatorType.Prepend, scope: undefined, options: {} }]
]);
});
});
Expand Down
12 changes: 6 additions & 6 deletions src/vs/platform/terminal/common/environmentVariable.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,24 +11,24 @@ export enum EnvironmentVariableMutatorType {
Append = 2,
Prepend = 3
}
// export enum EnvironmentVariableMutatorTiming {
// AtSpawn = 1,
// AfterShellIntegration = 2
// // TODO: Do we need a both?
// }
export interface IEnvironmentVariableMutator {
readonly variable: string;
readonly value: string;
readonly type: EnvironmentVariableMutatorType;
readonly scope?: EnvironmentVariableScope;
// readonly timing?: EnvironmentVariableMutatorTiming;
readonly options?: IEnvironmentVariableMutatorOptions;
}

export interface IEnvironmentDescriptionMutator {
readonly description: string | undefined;
readonly scope?: EnvironmentVariableScope;
}

export interface IEnvironmentVariableMutatorOptions {
applyAtProcessCreation?: boolean;
applyAtShellIntegration?: boolean;
}

export type EnvironmentVariableScope = {
workspaceFolder?: IWorkspaceFolderData;
};
Expand Down
50 changes: 29 additions & 21 deletions src/vs/platform/terminal/common/environmentVariableCollection.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ import { EnvironmentVariableMutatorType, EnvironmentVariableScope, IEnvironmentV

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

// const mutatorTypeToLabelMap: Map<EnvironmentVariableMutatorType, string> = new Map([
// [EnvironmentVariableMutatorType.Append, 'APPEND'],
// [EnvironmentVariableMutatorType.Prepend, 'PREPEND'],
// [EnvironmentVariableMutatorType.Replace, 'REPLACE']
// ]);
const mutatorTypeToLabelMap: Map<EnvironmentVariableMutatorType, string> = new Map([
[EnvironmentVariableMutatorType.Append, 'APPEND'],
[EnvironmentVariableMutatorType.Prepend, 'PREPEND'],
[EnvironmentVariableMutatorType.Replace, 'REPLACE']
]);

export class MergedEnvironmentVariableCollection implements IMergedEnvironmentVariableCollection {
private readonly map: Map<string, IExtensionOwnedEnvironmentVariableMutator[]> = new Map();
Expand Down Expand Up @@ -46,7 +46,8 @@ export class MergedEnvironmentVariableCollection implements IMergedEnvironmentVa
value: mutator.value,
type: mutator.type,
scope: mutator.scope,
variable: mutator.variable
variable: mutator.variable,
options: mutator.options
};
if (!extensionMutator.scope) {
delete extensionMutator.scope; // Convenient for tests
Expand All @@ -69,26 +70,33 @@ export class MergedEnvironmentVariableCollection implements IMergedEnvironmentVa
const actualVariable = isWindows ? lowerToActualVariableNames![variable.toLowerCase()] || variable : variable;
for (const mutator of mutators) {
const value = variableResolver ? await variableResolver(mutator.value) : mutator.value;
// if (mutator.timing === EnvironmentVariableMutatorTiming.AfterShellIntegration) {
// const key = `VSCODE_ENV_${mutatorTypeToLabelMap.get(mutator.type)!}`;
// env[key] = (env[key] ? env[key] + ':' : '') + variable + '=' + value;
// continue;
// }
switch (mutator.type) {
case EnvironmentVariableMutatorType.Append:
env[actualVariable] = (env[actualVariable] || '') + value;
break;
case EnvironmentVariableMutatorType.Prepend:
env[actualVariable] = value + (env[actualVariable] || '');
break;
case EnvironmentVariableMutatorType.Replace:
env[actualVariable] = value;
break;
// Default: true
if (mutator.options?.applyAtProcessCreation ?? true) {
switch (mutator.type) {
case EnvironmentVariableMutatorType.Append:
env[actualVariable] = (env[actualVariable] || '') + value;
break;
case EnvironmentVariableMutatorType.Prepend:
env[actualVariable] = value + (env[actualVariable] || '');
break;
case EnvironmentVariableMutatorType.Replace:
env[actualVariable] = value;
break;
}
}
// Default: false
if (mutator.options?.applyAtShellIntegration ?? false) {
const key = `VSCODE_ENV_${mutatorTypeToLabelMap.get(mutator.type)!}`;
env[key] = (env[key] ? env[key] + ':' : '') + variable + '=' + this._encodeColons(value);
}
}
}
}

private _encodeColons(value: string): string {
return value.replaceAll(':', '\\x3a');
}

diff(other: IMergedEnvironmentVariableCollection, scope: EnvironmentVariableScope | undefined): IMergedEnvironmentVariableCollectionDiff | undefined {
const added: Map<string, IExtensionOwnedEnvironmentVariableMutator[]> = new Map();
const changed: Map<string, IExtensionOwnedEnvironmentVariableMutator[]> = new Map();
Expand Down
56 changes: 38 additions & 18 deletions src/vs/workbench/api/common/extHostTerminalService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { withNullAsUndefined } from 'vs/base/common/types';
import { Promises } from 'vs/base/common/async';
import { EditorGroupColumn } from 'vs/workbench/services/editor/common/editorGroupColumn';
import { ViewColumn } from 'vs/workbench/api/common/extHostTypeConverters';
import { isProposedApiEnabled } from 'vs/workbench/services/extensions/common/extensions';

export interface IExtHostTerminalService extends ExtHostTerminalServiceShape, IDisposable {

Expand Down Expand Up @@ -826,7 +827,7 @@ export abstract class BaseExtHostTerminalService extends Disposable implements I
public getEnvironmentVariableCollection(extension: IExtensionDescription): IEnvironmentVariableCollection {
let collection = this._environmentVariableCollections.get(extension.identifier.value);
if (!collection) {
collection = new EnvironmentVariableCollection();
collection = new EnvironmentVariableCollection(extension);
this._setEnvironmentVariableCollection(extension.identifier.value, collection);
}
return collection.getScopedEnvironmentVariableCollection(undefined);
Expand All @@ -841,7 +842,7 @@ export abstract class BaseExtHostTerminalService extends Disposable implements I
public $initEnvironmentVariableCollections(collections: [string, ISerializableEnvironmentVariableCollection][]): void {
collections.forEach(entry => {
const extensionIdentifier = entry[0];
const collection = new EnvironmentVariableCollection(entry[1]);
const collection = new EnvironmentVariableCollection(undefined, entry[1]);
this._setEnvironmentVariableCollection(extensionIdentifier, collection);
});
}
Expand Down Expand Up @@ -883,6 +884,11 @@ class EnvironmentVariableCollection {
get onDidChangeCollection(): Event<void> { return this._onDidChangeCollection && this._onDidChangeCollection.event; }

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

replace(variable: string, value: string, scope: vscode.EnvironmentVariableScope | undefined): void {
this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Replace, scope });
replace(variable: string, value: string, options: vscode.EnvironmentVariableMutatorOptions | undefined, scope: vscode.EnvironmentVariableScope | undefined): void {
if (this._extension && options) {
isProposedApiEnabled(this._extension, 'envCollectionOptions');
}
this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Replace, options: options ?? {}, scope });
}

append(variable: string, value: string, scope: vscode.EnvironmentVariableScope | undefined): void {
this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Append, scope });
append(variable: string, value: string, options: vscode.EnvironmentVariableMutatorOptions | undefined, scope: vscode.EnvironmentVariableScope | undefined): void {
if (this._extension && options) {
isProposedApiEnabled(this._extension, 'envCollectionOptions');
}
this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Append, options: options ?? {}, scope });
}

prepend(variable: string, value: string, scope: vscode.EnvironmentVariableScope | undefined): void {
this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Prepend, scope });
prepend(variable: string, value: string, options: vscode.EnvironmentVariableMutatorOptions | undefined, scope: vscode.EnvironmentVariableScope | undefined): void {
if (this._extension && options) {
isProposedApiEnabled(this._extension, 'envCollectionOptions');
}
this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Prepend, options: options ?? {}, scope });
}

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

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

public getVariableMap(scope: vscode.EnvironmentVariableScope | undefined): Map<string, IEnvironmentVariableMutator> {
const map = new Map<string, IEnvironmentVariableMutator>();
public getVariableMap(scope: vscode.EnvironmentVariableScope | undefined): Map<string, vscode.EnvironmentVariableMutator> {
const map = new Map<string, vscode.EnvironmentVariableMutator>();
for (const [key, value] of this.map) {
if (this.getScopeKey(value.scope) === this.getScopeKey(scope)) {
map.set(key, value);
map.set(key, convertMutator(value));
}
}
return map;
Expand Down Expand Up @@ -1022,24 +1041,24 @@ class ScopedEnvironmentVariableCollection implements vscode.EnvironmentVariableC
return this.collection.getScopedEnvironmentVariableCollection(this.scope);
}

replace(variable: string, value: string): void {
this.collection.replace(variable, value, this.scope);
replace(variable: string, value: string, options?: vscode.EnvironmentVariableMutatorOptions | undefined): void {
this.collection.replace(variable, value, options, this.scope);
}

append(variable: string, value: string): void {
this.collection.append(variable, value, this.scope);
append(variable: string, value: string, options?: vscode.EnvironmentVariableMutatorOptions | undefined): void {
this.collection.append(variable, value, options, this.scope);
}

prepend(variable: string, value: string): void {
this.collection.prepend(variable, value, this.scope);
prepend(variable: string, value: string, options?: vscode.EnvironmentVariableMutatorOptions | undefined): void {
this.collection.prepend(variable, value, options, this.scope);
}

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

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

[Symbol.iterator](): IterableIterator<[variable: string, mutator: vscode.EnvironmentVariableMutator]> {
Expand Down Expand Up @@ -1102,6 +1121,7 @@ function asTerminalColor(color?: vscode.ThemeColor): ThemeColor | undefined {
function convertMutator(mutator: IEnvironmentVariableMutator): vscode.EnvironmentVariableMutator {
const newMutator = { ...mutator };
newMutator.scope = newMutator.scope ?? undefined;
newMutator.options = newMutator.options ?? undefined;
delete (newMutator as any).variable;
return newMutator as vscode.EnvironmentVariableMutator;
}
Loading

0 comments on commit f043f49

Please sign in to comment.