Skip to content

Commit

Permalink
Merge pull request #179323 from microsoft/kartik/terminal
Browse files Browse the repository at this point in the history 
Add proposed API to specific workspace-specific environment variables via EnvironmentVariableCollection
  • Loading branch information
Kartik Raj authored Apr 20, 2023
2 parents 92d528b + 54965bb commit f80485d
Show file tree
Hide file tree
Showing 23 changed files with 545 additions and 201 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 });
deepStrictEqual(collection.get('B'), { value: '~b2~', type: EnvironmentVariableMutatorType.Append });
deepStrictEqual(collection.get('C'), { value: '~c2~', type: EnvironmentVariableMutatorType.Prepend });
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 });

// Verify forEach
const entries: [string, EnvironmentVariableMutator][] = [];
collection.forEach((v, m) => entries.push([v, m]));
deepStrictEqual(entries, [
['A', { value: '~a2~', type: EnvironmentVariableMutatorType.Replace }],
['B', { value: '~b2~', type: EnvironmentVariableMutatorType.Append }],
['C', { value: '~c2~', type: EnvironmentVariableMutatorType.Prepend }]
['A', { value: '~a2~', type: EnvironmentVariableMutatorType.Replace, scope: undefined }],
['B', { value: '~b2~', type: EnvironmentVariableMutatorType.Append, scope: undefined }],
['C', { value: '~c2~', type: EnvironmentVariableMutatorType.Prepend, scope: undefined }]
]);
});
});
Expand Down
18 changes: 14 additions & 4 deletions src/vs/platform/terminal/common/environmentVariable.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*--------------------------------------------------------------------------------------------*/

import { IProcessEnvironment } from 'vs/base/common/platform';
import { IWorkspaceFolderData } from 'vs/platform/workspace/common/workspace';

export enum EnvironmentVariableMutatorType {
Replace = 1,
Expand All @@ -16,11 +17,17 @@ export enum EnvironmentVariableMutatorType {
// // TODO: Do we need a both?
// }
export interface IEnvironmentVariableMutator {
readonly variable: string;
readonly value: string;
readonly type: EnvironmentVariableMutatorType;
readonly scope?: EnvironmentVariableScope;
// readonly timing?: EnvironmentVariableMutatorTiming;
}

export type EnvironmentVariableScope = {
workspaceFolder?: IWorkspaceFolderData;
};

export interface IEnvironmentVariableCollection {
readonly map: ReadonlyMap<string, IEnvironmentVariableMutator>;
}
Expand Down Expand Up @@ -49,18 +56,21 @@ type VariableResolver = (str: string) => Promise<string>;
*/
export interface IMergedEnvironmentVariableCollection {
readonly collections: ReadonlyMap<string, IEnvironmentVariableCollection>;
readonly map: ReadonlyMap<string, IExtensionOwnedEnvironmentVariableMutator[]>;

/**
* Gets the variable map for a given scope.
* @param scope The scope to get the variable map for. If undefined, the global scope is used.
*/
getVariableMap(scope: EnvironmentVariableScope | undefined): Map<string, IExtensionOwnedEnvironmentVariableMutator[]>;
/**
* Applies this collection to a process environment.
* @param variableResolver An optional function to use to resolve variables within the
* environment values.
*/
applyToProcessEnvironment(env: IProcessEnvironment, variableResolver?: VariableResolver): Promise<void>;
applyToProcessEnvironment(env: IProcessEnvironment, scope: EnvironmentVariableScope | undefined, variableResolver?: VariableResolver): Promise<void>;

/**
* Generates a diff of this collection against another. Returns undefined if the collections are
* the same.
*/
diff(other: IMergedEnvironmentVariableCollection): IMergedEnvironmentVariableCollectionDiff | undefined;
diff(other: IMergedEnvironmentVariableCollection, scope: EnvironmentVariableScope | undefined): IMergedEnvironmentVariableCollectionDiff | undefined;
}
75 changes: 54 additions & 21 deletions src/vs/platform/terminal/common/environmentVariableCollection.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*--------------------------------------------------------------------------------------------*/

import { IProcessEnvironment, isWindows } from 'vs/base/common/platform';
import { EnvironmentVariableMutatorType, IEnvironmentVariableCollection, IExtensionOwnedEnvironmentVariableMutator, IMergedEnvironmentVariableCollection, IMergedEnvironmentVariableCollectionDiff } from 'vs/platform/terminal/common/environmentVariable';
import { EnvironmentVariableMutatorType, EnvironmentVariableScope, IEnvironmentVariableCollection, IExtensionOwnedEnvironmentVariableMutator, IMergedEnvironmentVariableCollection, IMergedEnvironmentVariableCollectionDiff } from 'vs/platform/terminal/common/environmentVariable';

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

Expand All @@ -15,20 +15,21 @@ type VariableResolver = (str: string) => Promise<string>;
// ]);

export class MergedEnvironmentVariableCollection implements IMergedEnvironmentVariableCollection {
readonly map: Map<string, IExtensionOwnedEnvironmentVariableMutator[]> = new Map();
private readonly map: Map<string, IExtensionOwnedEnvironmentVariableMutator[]> = new Map();

constructor(
readonly collections: ReadonlyMap<string, IEnvironmentVariableCollection>
readonly collections: ReadonlyMap<string, IEnvironmentVariableCollection>,
) {
collections.forEach((collection, extensionIdentifier) => {
const it = collection.map.entries();
let next = it.next();
while (!next.done) {
const variable = next.value[0];
let entry = this.map.get(variable);
const mutator = next.value[1];
const key = next.value[0];
let entry = this.map.get(key);
if (!entry) {
entry = [];
this.map.set(variable, entry);
this.map.set(key, entry);
}

// If the first item in the entry is replace ignore any other entries as they would
Expand All @@ -38,26 +39,31 @@ export class MergedEnvironmentVariableCollection implements IMergedEnvironmentVa
continue;
}

// Mutators get applied in the reverse order than they are created
const mutator = next.value[1];
entry.unshift({
const extensionMutator = {
extensionIdentifier,
value: mutator.value,
type: mutator.type
});
type: mutator.type,
scope: mutator.scope,
variable: mutator.variable
};
if (!extensionMutator.scope) {
delete extensionMutator.scope; // Convenient for tests
}
// Mutators get applied in the reverse order than they are created
entry.unshift(extensionMutator);

next = it.next();
}
});
}

async applyToProcessEnvironment(env: IProcessEnvironment, variableResolver?: VariableResolver): Promise<void> {
async applyToProcessEnvironment(env: IProcessEnvironment, scope: EnvironmentVariableScope | undefined, variableResolver?: VariableResolver): Promise<void> {
let lowerToActualVariableNames: { [lowerKey: string]: string | undefined } | undefined;
if (isWindows) {
lowerToActualVariableNames = {};
Object.keys(env).forEach(e => lowerToActualVariableNames![e.toLowerCase()] = e);
}
for (const [variable, mutators] of this.map) {
for (const [variable, mutators] of this.getVariableMap(scope)) {
const actualVariable = isWindows ? lowerToActualVariableNames![variable.toLowerCase()] || variable : variable;
for (const mutator of mutators) {
const value = variableResolver ? await variableResolver(mutator.value) : mutator.value;
Expand All @@ -81,32 +87,32 @@ export class MergedEnvironmentVariableCollection implements IMergedEnvironmentVa
}
}

diff(other: IMergedEnvironmentVariableCollection): IMergedEnvironmentVariableCollectionDiff | undefined {
diff(other: IMergedEnvironmentVariableCollection, scope: EnvironmentVariableScope | undefined): IMergedEnvironmentVariableCollectionDiff | undefined {
const added: Map<string, IExtensionOwnedEnvironmentVariableMutator[]> = new Map();
const changed: Map<string, IExtensionOwnedEnvironmentVariableMutator[]> = new Map();
const removed: Map<string, IExtensionOwnedEnvironmentVariableMutator[]> = new Map();

// Find added
other.map.forEach((otherMutators, variable) => {
const currentMutators = this.map.get(variable);
other.getVariableMap(scope).forEach((otherMutators, variable) => {
const currentMutators = this.getVariableMap(scope).get(variable);
const result = getMissingMutatorsFromArray(otherMutators, currentMutators);
if (result) {
added.set(variable, result);
}
});

// Find removed
this.map.forEach((currentMutators, variable) => {
const otherMutators = other.map.get(variable);
this.getVariableMap(scope).forEach((currentMutators, variable) => {
const otherMutators = other.getVariableMap(scope).get(variable);
const result = getMissingMutatorsFromArray(currentMutators, otherMutators);
if (result) {
removed.set(variable, result);
}
});

// Find changed
this.map.forEach((currentMutators, variable) => {
const otherMutators = other.map.get(variable);
this.getVariableMap(scope).forEach((currentMutators, variable) => {
const otherMutators = other.getVariableMap(scope).get(variable);
const result = getChangedMutatorsFromArray(currentMutators, otherMutators);
if (result) {
changed.set(variable, result);
Expand All @@ -119,6 +125,33 @@ export class MergedEnvironmentVariableCollection implements IMergedEnvironmentVa

return { added, changed, removed };
}

getVariableMap(scope: EnvironmentVariableScope | undefined): Map<string, IExtensionOwnedEnvironmentVariableMutator[]> {
const result = new Map<string, IExtensionOwnedEnvironmentVariableMutator[]>();
this.map.forEach((mutators, _key) => {
const filteredMutators = mutators.filter(m => filterScope(m, scope));
if (filteredMutators.length > 0) {
// All of these mutators are for the same variable because they are in the same scope, hence choose anyone to form a key.
result.set(filteredMutators[0].variable, filteredMutators);
}
});
return result;
}
}

function filterScope(
mutator: IExtensionOwnedEnvironmentVariableMutator,
scope: EnvironmentVariableScope | undefined
): boolean {
if (!mutator.scope) {
return true;
}
// If a mutator is scoped to a workspace folder, only apply it if the workspace
// folder matches.
if (mutator.scope.workspaceFolder && scope?.workspaceFolder && mutator.scope.workspaceFolder.index === scope.workspaceFolder.index) {
return true;
}
return false;
}

function getMissingMutatorsFromArray(
Expand Down Expand Up @@ -162,7 +195,7 @@ function getChangedMutatorsFromArray(
const result: IExtensionOwnedEnvironmentVariableMutator[] = [];
current.forEach(mutator => {
const otherMutator = otherMutatorExtensions.get(mutator.extensionIdentifier);
if (otherMutator && (mutator.type !== otherMutator.type || mutator.value !== otherMutator.value)) {
if (otherMutator && (mutator.type !== otherMutator.type || mutator.value !== otherMutator.value || mutator.scope?.workspaceFolder?.index !== otherMutator.scope?.workspaceFolder?.index)) {
// Return the new result, not the old one
result.push(otherMutator);
}
Expand Down
2 changes: 2 additions & 0 deletions src/vs/platform/terminal/common/terminal.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { IGetTerminalLayoutInfoArgs, IProcessDetails, ISetTerminalLayoutInfoArgs
import { ThemeIcon } from 'vs/base/common/themables';
import { ISerializableEnvironmentVariableCollections } from 'vs/platform/terminal/common/environmentVariable';
import { RawContextKey } from 'vs/platform/contextkey/common/contextkey';
import { IWorkspaceFolder } from 'vs/platform/workspace/common/workspace';

export const terminalTabFocusContextKey = new RawContextKey<boolean>('terminalTabFocusMode', false, true);

Expand Down Expand Up @@ -600,6 +601,7 @@ export interface ITerminalProcessOptions {
};
windowsEnableConpty: boolean;
environmentVariableCollections: ISerializableEnvironmentVariableCollections | undefined;
workspaceFolder: IWorkspaceFolder | undefined;
}

export interface ITerminalEnvironment {
Expand Down
2 changes: 1 addition & 1 deletion src/vs/platform/terminal/node/terminalEnvironment.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ function addEnvMixinPathPrefix(options: ITerminalProcessOptions, envMixin: IProc
const merged = new MergedEnvironmentVariableCollection(deserialized);

// Get all prepend PATH entries
const pathEntry = merged.map.get('PATH');
const pathEntry = merged.getVariableMap({ workspaceFolder: options.workspaceFolder }).get('PATH');
const prependToPath: string[] = [];
if (pathEntry) {
for (const mutator of pathEntry) {
Expand Down
6 changes: 3 additions & 3 deletions src/vs/platform/terminal/test/node/terminalEnvironment.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import { IProductService } from 'vs/platform/product/common/productService';
import { ITerminalProcessOptions } from 'vs/platform/terminal/common/terminal';
import { getShellIntegrationInjection, getWindowsBuildNumber, IShellIntegrationConfigInjection } from 'vs/platform/terminal/node/terminalEnvironment';

const enabledProcessOptions: ITerminalProcessOptions = { shellIntegration: { enabled: true, suggestEnabled: false }, windowsEnableConpty: true, environmentVariableCollections: undefined };
const disabledProcessOptions: ITerminalProcessOptions = { shellIntegration: { enabled: false, suggestEnabled: false }, windowsEnableConpty: true, environmentVariableCollections: undefined };
const winptyProcessOptions: ITerminalProcessOptions = { shellIntegration: { enabled: true, suggestEnabled: false }, windowsEnableConpty: false, environmentVariableCollections: undefined };
const enabledProcessOptions: ITerminalProcessOptions = { shellIntegration: { enabled: true, suggestEnabled: false }, windowsEnableConpty: true, environmentVariableCollections: undefined, workspaceFolder: undefined };
const disabledProcessOptions: ITerminalProcessOptions = { shellIntegration: { enabled: false, suggestEnabled: false }, windowsEnableConpty: true, environmentVariableCollections: undefined, workspaceFolder: undefined };
const winptyProcessOptions: ITerminalProcessOptions = { shellIntegration: { enabled: true, suggestEnabled: false }, windowsEnableConpty: false, environmentVariableCollections: undefined, workspaceFolder: undefined };
const pwshExe = process.platform === 'win32' ? 'pwsh.exe' : 'pwsh';
const repoRoot = process.platform === 'win32' ? process.cwd()[0].toLowerCase() + process.cwd().substring(1) : process.cwd();
const logService = new NullLogService();
Expand Down
4 changes: 3 additions & 1 deletion src/vs/server/node/remoteTerminalChannel.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { IServerEnvironmentService } from 'vs/server/node/serverEnvironmentServi
import { IProductService } from 'vs/platform/product/common/productService';
import { IExtensionManagementService } from 'vs/platform/extensionManagement/common/extensionManagement';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
import { withNullAsUndefined } from 'vs/base/common/types';

class CustomVariableResolver extends AbstractVariableResolverService {
constructor(
Expand Down Expand Up @@ -237,7 +238,8 @@ export class RemoteTerminalChannel extends Disposable implements IServerChannel<
}
const envVariableCollections = new Map<string, IEnvironmentVariableCollection>(entries);
const mergedCollection = new MergedEnvironmentVariableCollection(envVariableCollections);
await mergedCollection.applyToProcessEnvironment(env, variableResolver);
const workspaceFolder = activeWorkspaceFolder ? withNullAsUndefined(activeWorkspaceFolder) : undefined;
await mergedCollection.applyToProcessEnvironment(env, { workspaceFolder }, variableResolver);
}

// Fork the process and listen for messages
Expand Down
Loading

0 comments on commit f80485d

Please sign in to comment.