From 689c9126defdc7564b8deafb6f1d6481b1bcda30 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 18 May 2023 08:45:20 -0700 Subject: [PATCH 01/13] Initial impl of env var collection options Part of #179476 --- .../terminal/common/environmentVariable.ts | 12 ++--- .../common/environmentVariableCollection.ts | 47 +++++++++------- .../api/browser/mainThreadTerminalService.ts | 1 + .../api/common/extHostTerminalService.ts | 11 ++-- .../browser/media/shellIntegration.ps1 | 4 ++ .../common/extensionsApiProposals.ts | 1 + .../vscode.proposed.envCollectionOptions.d.ts | 54 +++++++++++++++++++ 7 files changed, 101 insertions(+), 29 deletions(-) create mode 100644 src/vscode-dts/vscode.proposed.envCollectionOptions.d.ts diff --git a/src/vs/platform/terminal/common/environmentVariable.ts b/src/vs/platform/terminal/common/environmentVariable.ts index 3df82accaae63..8ff5a08abc495 100644 --- a/src/vs/platform/terminal/common/environmentVariable.ts +++ b/src/vs/platform/terminal/common/environmentVariable.ts @@ -11,17 +11,12 @@ 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 { @@ -29,6 +24,11 @@ export interface IEnvironmentDescriptionMutator { readonly scope?: EnvironmentVariableScope; } +export interface IEnvironmentVariableMutatorOptions { + applyAtProcessCreation?: boolean; + applyAtShellIntegration?: boolean; +} + export type EnvironmentVariableScope = { workspaceFolder?: IWorkspaceFolderData; }; diff --git a/src/vs/platform/terminal/common/environmentVariableCollection.ts b/src/vs/platform/terminal/common/environmentVariableCollection.ts index 3c223347ceb33..af9c677286b87 100644 --- a/src/vs/platform/terminal/common/environmentVariableCollection.ts +++ b/src/vs/platform/terminal/common/environmentVariableCollection.ts @@ -8,11 +8,11 @@ import { EnvironmentVariableMutatorType, EnvironmentVariableScope, IEnvironmentV type VariableResolver = (str: string) => Promise; -// const mutatorTypeToLabelMap: Map = new Map([ -// [EnvironmentVariableMutatorType.Append, 'APPEND'], -// [EnvironmentVariableMutatorType.Prepend, 'PREPEND'], -// [EnvironmentVariableMutatorType.Replace, 'REPLACE'] -// ]); +const mutatorTypeToLabelMap: Map = new Map([ + [EnvironmentVariableMutatorType.Append, 'APPEND'], + [EnvironmentVariableMutatorType.Prepend, 'PREPEND'], + [EnvironmentVariableMutatorType.Replace, 'REPLACE'] +]); export class MergedEnvironmentVariableCollection implements IMergedEnvironmentVariableCollection { private readonly map: Map = new Map(); @@ -69,26 +69,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 = new Map(); const changed: Map = new Map(); diff --git a/src/vs/workbench/api/browser/mainThreadTerminalService.ts b/src/vs/workbench/api/browser/mainThreadTerminalService.ts index f43a403ff42a0..d11771537f0c4 100644 --- a/src/vs/workbench/api/browser/mainThreadTerminalService.ts +++ b/src/vs/workbench/api/browser/mainThreadTerminalService.ts @@ -407,6 +407,7 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape map: deserializeEnvironmentVariableCollection(collection), descriptionMap: deserializeEnvironmentDescriptionMap(descriptionMap) }; + console.log('translatedCollection', translatedCollection); this._environmentVariableService.set(extensionIdentifier, translatedCollection); } else { this._environmentVariableService.delete(extensionIdentifier); diff --git a/src/vs/workbench/api/common/extHostTerminalService.ts b/src/vs/workbench/api/common/extHostTerminalService.ts index c703131984652..89520de678770 100644 --- a/src/vs/workbench/api/common/extHostTerminalService.ts +++ b/src/vs/workbench/api/common/extHostTerminalService.ts @@ -892,15 +892,18 @@ class EnvironmentVariableCollection implements vscode.EnvironmentVariableCollect } replace(variable: string, value: string, scope?: vscode.EnvironmentVariableScope): void { - this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Replace, scope }); + // TODO: Implement options in API + this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Replace, scope, options: {} }); } append(variable: string, value: string, scope?: vscode.EnvironmentVariableScope): void { - this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Append, scope }); + // TODO: Implement options in API + this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Append, scope, options: {} }); } prepend(variable: string, value: string, scope?: vscode.EnvironmentVariableScope): void { - this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Prepend, scope }); + // TODO: Implement options in API + this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Prepend, scope, options: {} }); } private _setIfDiffers(variable: string, mutator: vscode.EnvironmentVariableMutator): void { @@ -920,6 +923,7 @@ class EnvironmentVariableCollection implements vscode.EnvironmentVariableCollect get(variable: string, scope?: vscode.EnvironmentVariableScope): 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; } @@ -1030,6 +1034,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; } diff --git a/src/vs/workbench/contrib/terminal/browser/media/shellIntegration.ps1 b/src/vs/workbench/contrib/terminal/browser/media/shellIntegration.ps1 index e32014490863e..616b9b9736ff2 100644 --- a/src/vs/workbench/contrib/terminal/browser/media/shellIntegration.ps1 +++ b/src/vs/workbench/contrib/terminal/browser/media/shellIntegration.ps1 @@ -21,6 +21,10 @@ $Global:__LastHistoryId = -1 $Nonce = $env:VSCODE_NONCE $env:VSCODE_NONCE = $null +Write-Host "REPLACE: " $env:VSCODE_ENV_REPLACE +Write-Host "PREPEND: " $env:VSCODE_ENV_PREPEND +Write-Host "APPEND: " $env:VSCODE_ENV_APPEND + if ($env:VSCODE_ENV_REPLACE) { $Split = $env:VSCODE_ENV_REPLACE.Split(":") foreach ($Item in $Split) { diff --git a/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts b/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts index c18e798edf8d6..9d70b48d6c738 100644 --- a/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts +++ b/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts @@ -33,6 +33,7 @@ export const allApiProposals = Object.freeze({ dropMetadata: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.dropMetadata.d.ts', editSessionIdentityProvider: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.editSessionIdentityProvider.d.ts', editorInsets: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.editorInsets.d.ts', + envCollectionOptions: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.envCollectionOptions.d.ts', envCollectionWorkspace: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.envCollectionWorkspace.d.ts', envShellEvent: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.envShellEvent.d.ts', extensionRuntime: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.extensionRuntime.d.ts', diff --git a/src/vscode-dts/vscode.proposed.envCollectionOptions.d.ts b/src/vscode-dts/vscode.proposed.envCollectionOptions.d.ts new file mode 100644 index 0000000000000..b55c72fe2143c --- /dev/null +++ b/src/vscode-dts/vscode.proposed.envCollectionOptions.d.ts @@ -0,0 +1,54 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +declare module 'vscode' { + + // https://github.com/microsoft/vscode/issues/179476 + + export interface EnvironmentVariableMutatorOptions { + /** + * Apply to the environment just before the process is created. + * + * Defaults to true. + */ + applyAtProcessCreation?: boolean; + + /** + * Apply to the environment in the shell integration script. Note that this _will not_ apply + * the mutator if shell integration is disabled or not working for some reason. + * + * Defaults to false. + */ + applyAtShellIntegration?: boolean; + } + + /** + * A type of mutation and its value to be applied to an environment variable. + */ + export interface EnvironmentVariableMutator { + /** + * Options applied to the mutator. + */ + readonly options: EnvironmentVariableMutatorOptions; + } + + export interface EnvironmentVariableCollection extends Iterable<[variable: string, mutator: EnvironmentVariableMutator]> { + // TODO: Uncomment when workspace/scopes is removed from the other proposal + // /** + // * @param options Options applied to the mutator. + // */ + // replace(variable: string, value: string, options?: EnvironmentVariableMutatorOptions): void; + + // /** + // * @param options Options applied to the mutator. + // */ + // append(variable: string, value: string, options?: EnvironmentVariableMutatorOptions): void; + + // /** + // * @param options Options applied to the mutator. + // */ + // prepend(variable: string, value: string, options?: EnvironmentVariableMutatorOptions): void; + } +} From f3fe4f9432307dd75902389eec4750a260220459 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 19 May 2023 07:10:33 -0700 Subject: [PATCH 02/13] Fix options in extension API pass through --- .../api/common/extHostTerminalService.ts | 27 ++++++++--------- .../vscode.proposed.envCollectionOptions.d.ts | 29 +++++++++---------- 2 files changed, 26 insertions(+), 30 deletions(-) diff --git a/src/vs/workbench/api/common/extHostTerminalService.ts b/src/vs/workbench/api/common/extHostTerminalService.ts index 757790615d367..c775d12a44753 100644 --- a/src/vs/workbench/api/common/extHostTerminalService.ts +++ b/src/vs/workbench/api/common/extHostTerminalService.ts @@ -899,19 +899,16 @@ class EnvironmentVariableCollection { return scopedCollection; } - replace(variable: string, value: string, scope: vscode.EnvironmentVariableScope | undefined): void { - // TODO: Implement options in API - this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Replace, scope, options: {} }); + replace(variable: string, value: string, options: vscode.EnvironmentVariableMutatorOptions | undefined, scope: vscode.EnvironmentVariableScope | undefined): void { + this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Replace, options: options ?? {}, scope }); } - append(variable: string, value: string, scope: vscode.EnvironmentVariableScope | undefined): void { - // TODO: Implement options in API - this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Append, scope, options: {} }); + append(variable: string, value: string, options: vscode.EnvironmentVariableMutatorOptions | undefined, scope: vscode.EnvironmentVariableScope | undefined): void { + this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Append, options: options ?? {}, scope }); } - prepend(variable: string, value: string, scope: vscode.EnvironmentVariableScope | undefined): void { - // TODO: Implement options in API - this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Prepend, scope, options: {} }); + prepend(variable: string, value: string, options: vscode.EnvironmentVariableMutatorOptions | undefined, scope: vscode.EnvironmentVariableScope | undefined): void { + this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Prepend, options: options ?? {}, scope }); } private _setIfDiffers(variable: string, mutator: vscode.EnvironmentVariableMutator & { scope: vscode.EnvironmentVariableScope | undefined }): void { @@ -1026,16 +1023,16 @@ 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 { diff --git a/src/vscode-dts/vscode.proposed.envCollectionOptions.d.ts b/src/vscode-dts/vscode.proposed.envCollectionOptions.d.ts index b55c72fe2143c..d9ac19d23c041 100644 --- a/src/vscode-dts/vscode.proposed.envCollectionOptions.d.ts +++ b/src/vscode-dts/vscode.proposed.envCollectionOptions.d.ts @@ -35,20 +35,19 @@ declare module 'vscode' { } export interface EnvironmentVariableCollection extends Iterable<[variable: string, mutator: EnvironmentVariableMutator]> { - // TODO: Uncomment when workspace/scopes is removed from the other proposal - // /** - // * @param options Options applied to the mutator. - // */ - // replace(variable: string, value: string, options?: EnvironmentVariableMutatorOptions): void; - - // /** - // * @param options Options applied to the mutator. - // */ - // append(variable: string, value: string, options?: EnvironmentVariableMutatorOptions): void; - - // /** - // * @param options Options applied to the mutator. - // */ - // prepend(variable: string, value: string, options?: EnvironmentVariableMutatorOptions): void; + /** + * @param options Options applied to the mutator. + */ + replace(variable: string, value: string, options?: EnvironmentVariableMutatorOptions): void; + + /** + * @param options Options applied to the mutator. + */ + append(variable: string, value: string, options?: EnvironmentVariableMutatorOptions): void; + + /** + * @param options Options applied to the mutator. + */ + prepend(variable: string, value: string, options?: EnvironmentVariableMutatorOptions): void; } } From 456dd5b317a332d99491af7683b24c723a5db98e Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 19 May 2023 07:17:10 -0700 Subject: [PATCH 03/13] Fix considering options in merged collection --- .../platform/terminal/common/environmentVariableCollection.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/vs/platform/terminal/common/environmentVariableCollection.ts b/src/vs/platform/terminal/common/environmentVariableCollection.ts index af9c677286b87..67b0e2a534beb 100644 --- a/src/vs/platform/terminal/common/environmentVariableCollection.ts +++ b/src/vs/platform/terminal/common/environmentVariableCollection.ts @@ -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 From 2830bd25f8ff95b9b1252332a861880f5c0e4177 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 19 May 2023 07:17:54 -0700 Subject: [PATCH 04/13] Remove test logs --- .../contrib/terminal/browser/media/shellIntegration.ps1 | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/browser/media/shellIntegration.ps1 b/src/vs/workbench/contrib/terminal/browser/media/shellIntegration.ps1 index 616b9b9736ff2..e32014490863e 100644 --- a/src/vs/workbench/contrib/terminal/browser/media/shellIntegration.ps1 +++ b/src/vs/workbench/contrib/terminal/browser/media/shellIntegration.ps1 @@ -21,10 +21,6 @@ $Global:__LastHistoryId = -1 $Nonce = $env:VSCODE_NONCE $env:VSCODE_NONCE = $null -Write-Host "REPLACE: " $env:VSCODE_ENV_REPLACE -Write-Host "PREPEND: " $env:VSCODE_ENV_PREPEND -Write-Host "APPEND: " $env:VSCODE_ENV_APPEND - if ($env:VSCODE_ENV_REPLACE) { $Split = $env:VSCODE_ENV_REPLACE.Split(":") foreach ($Item in $Split) { From cd5fe46e0be43a067c0314446de1c4fc50a3eb75 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 19 May 2023 07:24:43 -0700 Subject: [PATCH 05/13] Add proposed check to options --- .../api/common/extHostTerminalService.ts | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/api/common/extHostTerminalService.ts b/src/vs/workbench/api/common/extHostTerminalService.ts index c775d12a44753..b882f5c29c888 100644 --- a/src/vs/workbench/api/common/extHostTerminalService.ts +++ b/src/vs/workbench/api/common/extHostTerminalService.ts @@ -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 { @@ -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); @@ -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); }); } @@ -883,6 +884,11 @@ class EnvironmentVariableCollection { get onDidChangeCollection(): Event { 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); @@ -900,14 +906,23 @@ class EnvironmentVariableCollection { } 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, 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, 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 }); } From c5aa6c197bbd7c84f50a17721bee4cba0e28fb20 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 19 May 2023 07:30:28 -0700 Subject: [PATCH 06/13] Throw when invalid options are used --- src/vs/workbench/api/common/extHostTerminalService.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/vs/workbench/api/common/extHostTerminalService.ts b/src/vs/workbench/api/common/extHostTerminalService.ts index b882f5c29c888..60dbb38da3cfb 100644 --- a/src/vs/workbench/api/common/extHostTerminalService.ts +++ b/src/vs/workbench/api/common/extHostTerminalService.ts @@ -927,6 +927,9 @@ class EnvironmentVariableCollection { } 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 } From 8e9b5a258c4d258bdfc343a238cbd701dc4b6fa3 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 19 May 2023 07:37:12 -0700 Subject: [PATCH 07/13] Fix throw condition --- src/vs/workbench/api/common/extHostTerminalService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/api/common/extHostTerminalService.ts b/src/vs/workbench/api/common/extHostTerminalService.ts index 60dbb38da3cfb..78a1d4a52919f 100644 --- a/src/vs/workbench/api/common/extHostTerminalService.ts +++ b/src/vs/workbench/api/common/extHostTerminalService.ts @@ -927,7 +927,7 @@ class EnvironmentVariableCollection { } private _setIfDiffers(variable: string, mutator: vscode.EnvironmentVariableMutator & { scope: vscode.EnvironmentVariableScope | undefined }): void { - if (mutator.options && mutator.options.applyAtProcessCreation === false && mutator.options.applyAtShellIntegration) { + 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) { From 01926572836ae0c3bd77ec5f5c0bcf59c297d0f2 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 19 May 2023 07:38:40 -0700 Subject: [PATCH 08/13] Add missing comment to EnvironmentVariableMutatorOptions --- src/vscode-dts/vscode.proposed.envCollectionOptions.d.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/vscode-dts/vscode.proposed.envCollectionOptions.d.ts b/src/vscode-dts/vscode.proposed.envCollectionOptions.d.ts index d9ac19d23c041..d25a92725a4dd 100644 --- a/src/vscode-dts/vscode.proposed.envCollectionOptions.d.ts +++ b/src/vscode-dts/vscode.proposed.envCollectionOptions.d.ts @@ -7,6 +7,9 @@ declare module 'vscode' { // https://github.com/microsoft/vscode/issues/179476 + /** + * Options applied to the mutator. + */ export interface EnvironmentVariableMutatorOptions { /** * Apply to the environment just before the process is created. From 2fe1a30dad610c16090f7b388aa6479ea5db060a Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Tue, 23 May 2023 06:42:46 -0700 Subject: [PATCH 09/13] Fix test failures --- .../environmentVariableCollection.test.ts | 46 +++++++++---------- .../common/environmentVariableService.test.ts | 28 +++++------ 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/test/common/environmentVariableCollection.test.ts b/src/vs/workbench/contrib/terminal/test/common/environmentVariableCollection.test.ts index 40d0e4c339e92..e13f89af0e3d9 100644 --- a/src/vs/workbench/contrib/terminal/test/common/environmentVariableCollection.test.ts +++ b/src/vs/workbench/contrib/terminal/test/common/environmentVariableCollection.test.ts @@ -37,10 +37,10 @@ suite('EnvironmentVariable - MergedEnvironmentVariableCollection', () => { ])); deepStrictEqual([...merged.getVariableMap(undefined).entries()], [ ['A', [ - { extensionIdentifier: 'ext4', type: EnvironmentVariableMutatorType.Append, value: 'a4', variable: 'A' }, - { extensionIdentifier: 'ext3', type: EnvironmentVariableMutatorType.Prepend, value: 'a3', variable: 'A' }, - { extensionIdentifier: 'ext2', type: EnvironmentVariableMutatorType.Append, value: 'a2', variable: 'A' }, - { extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Prepend, value: 'a1', variable: 'A' } + { extensionIdentifier: 'ext4', type: EnvironmentVariableMutatorType.Append, value: 'a4', variable: 'A', options: undefined }, + { extensionIdentifier: 'ext3', type: EnvironmentVariableMutatorType.Prepend, value: 'a3', variable: 'A', options: undefined }, + { extensionIdentifier: 'ext2', type: EnvironmentVariableMutatorType.Append, value: 'a2', variable: 'A', options: undefined }, + { extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Prepend, value: 'a1', variable: 'A', options: undefined } ]] ]); }); @@ -70,9 +70,9 @@ suite('EnvironmentVariable - MergedEnvironmentVariableCollection', () => { ])); deepStrictEqual([...merged.getVariableMap(undefined).entries()], [ ['A', [ - { extensionIdentifier: 'ext3', type: EnvironmentVariableMutatorType.Replace, value: 'a3', variable: 'A' }, - { extensionIdentifier: 'ext2', type: EnvironmentVariableMutatorType.Append, value: 'a2', variable: 'A' }, - { extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Prepend, value: 'a1', variable: 'A' } + { extensionIdentifier: 'ext3', type: EnvironmentVariableMutatorType.Replace, value: 'a3', variable: 'A', options: undefined }, + { extensionIdentifier: 'ext2', type: EnvironmentVariableMutatorType.Append, value: 'a2', variable: 'A', options: undefined }, + { extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Prepend, value: 'a1', variable: 'A', options: undefined } ]] ], 'The ext4 entry should be removed as it comes after a Replace'); }); @@ -104,9 +104,9 @@ suite('EnvironmentVariable - MergedEnvironmentVariableCollection', () => { ])); deepStrictEqual([...merged.getVariableMap(scope2).entries()], [ ['A', [ - { extensionIdentifier: 'ext4', type: EnvironmentVariableMutatorType.Append, value: 'a4', variable: 'A' }, - { extensionIdentifier: 'ext3', type: EnvironmentVariableMutatorType.Prepend, value: 'a3', scope: scope2, variable: 'A' }, - { extensionIdentifier: 'ext2', type: EnvironmentVariableMutatorType.Append, value: 'a2', variable: 'A' }, + { extensionIdentifier: 'ext4', type: EnvironmentVariableMutatorType.Append, value: 'a4', variable: 'A', options: undefined }, + { extensionIdentifier: 'ext3', type: EnvironmentVariableMutatorType.Prepend, value: 'a3', scope: scope2, variable: 'A', options: undefined }, + { extensionIdentifier: 'ext2', type: EnvironmentVariableMutatorType.Append, value: 'a2', variable: 'A', options: undefined }, ]] ]); }); @@ -138,8 +138,8 @@ suite('EnvironmentVariable - MergedEnvironmentVariableCollection', () => { ])); deepStrictEqual([...merged.getVariableMap(undefined).entries()], [ ['A', [ - { extensionIdentifier: 'ext4', type: EnvironmentVariableMutatorType.Append, value: 'a4', variable: 'A' }, - { extensionIdentifier: 'ext2', type: EnvironmentVariableMutatorType.Append, value: 'a2', variable: 'A' }, + { extensionIdentifier: 'ext4', type: EnvironmentVariableMutatorType.Append, value: 'a4', variable: 'A', options: undefined }, + { extensionIdentifier: 'ext2', type: EnvironmentVariableMutatorType.Append, value: 'a2', variable: 'A', options: undefined }, ]] ]); }); @@ -322,7 +322,7 @@ suite('EnvironmentVariable - MergedEnvironmentVariableCollection', () => { strictEqual(diff.removed.size, 0); const entries = [...diff.added.entries()]; deepStrictEqual(entries, [ - ['A', [{ extensionIdentifier: 'ext1', value: 'a', type: EnvironmentVariableMutatorType.Replace, variable: 'A' }]] + ['A', [{ extensionIdentifier: 'ext1', value: 'a', type: EnvironmentVariableMutatorType.Replace, variable: 'A', options: undefined }]] ]); }); @@ -347,7 +347,7 @@ suite('EnvironmentVariable - MergedEnvironmentVariableCollection', () => { strictEqual(diff.removed.size, 0); const entries = [...diff.added.entries()]; deepStrictEqual(entries, [ - ['B', [{ extensionIdentifier: 'ext1', value: 'b', type: EnvironmentVariableMutatorType.Append, variable: 'B' }]] + ['B', [{ extensionIdentifier: 'ext1', value: 'b', type: EnvironmentVariableMutatorType.Append, variable: 'B', options: undefined }]] ]); }); @@ -376,7 +376,7 @@ suite('EnvironmentVariable - MergedEnvironmentVariableCollection', () => { strictEqual(diff.changed.size, 0); strictEqual(diff.removed.size, 0); deepStrictEqual([...diff.added.entries()], [ - ['A', [{ extensionIdentifier: 'ext2', value: 'a2', type: EnvironmentVariableMutatorType.Append, variable: 'A' }]] + ['A', [{ extensionIdentifier: 'ext2', value: 'a2', type: EnvironmentVariableMutatorType.Append, variable: 'A', options: undefined }]] ]); const merged3 = new MergedEnvironmentVariableCollection(new Map([ @@ -443,7 +443,7 @@ suite('EnvironmentVariable - MergedEnvironmentVariableCollection', () => { strictEqual(diff.changed.size, 0); strictEqual(diff.added.size, 0); deepStrictEqual([...diff.removed.entries()], [ - ['B', [{ extensionIdentifier: 'ext1', value: 'b', type: EnvironmentVariableMutatorType.Replace, variable: 'B' }]] + ['B', [{ extensionIdentifier: 'ext1', value: 'b', type: EnvironmentVariableMutatorType.Replace, variable: 'B', options: undefined }]] ]); }); @@ -468,8 +468,8 @@ suite('EnvironmentVariable - MergedEnvironmentVariableCollection', () => { strictEqual(diff.added.size, 0); strictEqual(diff.removed.size, 0); deepStrictEqual([...diff.changed.entries()], [ - ['A', [{ extensionIdentifier: 'ext1', value: 'a2', type: EnvironmentVariableMutatorType.Replace, variable: 'A' }]], - ['B', [{ extensionIdentifier: 'ext1', value: 'b', type: EnvironmentVariableMutatorType.Append, variable: 'B' }]] + ['A', [{ extensionIdentifier: 'ext1', value: 'a2', type: EnvironmentVariableMutatorType.Replace, variable: 'A', options: undefined }]], + ['B', [{ extensionIdentifier: 'ext1', value: 'b', type: EnvironmentVariableMutatorType.Append, variable: 'B', options: undefined }]] ]); }); @@ -492,13 +492,13 @@ suite('EnvironmentVariable - MergedEnvironmentVariableCollection', () => { ])); const diff = merged1.diff(merged2, undefined)!; deepStrictEqual([...diff.added.entries()], [ - ['C', [{ extensionIdentifier: 'ext1', value: 'c', type: EnvironmentVariableMutatorType.Append, variable: 'C' }]], + ['C', [{ extensionIdentifier: 'ext1', value: 'c', type: EnvironmentVariableMutatorType.Append, variable: 'C', options: undefined }]], ]); deepStrictEqual([...diff.removed.entries()], [ - ['B', [{ extensionIdentifier: 'ext1', value: 'b', type: EnvironmentVariableMutatorType.Prepend, variable: 'B' }]] + ['B', [{ extensionIdentifier: 'ext1', value: 'b', type: EnvironmentVariableMutatorType.Prepend, variable: 'B', options: undefined }]] ]); deepStrictEqual([...diff.changed.entries()], [ - ['A', [{ extensionIdentifier: 'ext1', value: 'a2', type: EnvironmentVariableMutatorType.Replace, variable: 'A' }]] + ['A', [{ extensionIdentifier: 'ext1', value: 'a2', type: EnvironmentVariableMutatorType.Replace, variable: 'A', options: undefined }]] ]); }); @@ -524,10 +524,10 @@ suite('EnvironmentVariable - MergedEnvironmentVariableCollection', () => { const diff = merged1.diff(merged2, scope1)!; strictEqual(diff.added.size, 0); deepStrictEqual([...diff.removed.entries()], [ - ['B', [{ extensionIdentifier: 'ext1', value: 'b', type: EnvironmentVariableMutatorType.Prepend, variable: 'B' }]] + ['B', [{ extensionIdentifier: 'ext1', value: 'b', type: EnvironmentVariableMutatorType.Prepend, variable: 'B', options: undefined }]] ]); deepStrictEqual([...diff.changed.entries()], [ - ['A', [{ extensionIdentifier: 'ext1', value: 'a2', type: EnvironmentVariableMutatorType.Replace, scope: scope1, variable: 'A' }]] + ['A', [{ extensionIdentifier: 'ext1', value: 'a2', type: EnvironmentVariableMutatorType.Replace, scope: scope1, variable: 'A', options: undefined }]] ]); }); }); diff --git a/src/vs/workbench/contrib/terminal/test/common/environmentVariableService.test.ts b/src/vs/workbench/contrib/terminal/test/common/environmentVariableService.test.ts index c1651e86f9ea1..edd6f43bd5116 100644 --- a/src/vs/workbench/contrib/terminal/test/common/environmentVariableService.test.ts +++ b/src/vs/workbench/contrib/terminal/test/common/environmentVariableService.test.ts @@ -54,18 +54,18 @@ suite('EnvironmentVariable - EnvironmentVariableService', () => { collection.set('C-key', { value: 'c', type: EnvironmentVariableMutatorType.Prepend, variable: 'C' }); environmentVariableService.set('ext1', { map: collection, persistent: true }); deepStrictEqual([...environmentVariableService.mergedCollection.getVariableMap(undefined).entries()], [ - ['A', [{ extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Replace, value: 'a', variable: 'A' }]], - ['B', [{ extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Append, value: 'b', variable: 'B' }]], - ['C', [{ extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Prepend, value: 'c', variable: 'C' }]] + ['A', [{ extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Replace, value: 'a', variable: 'A', options: undefined }]], + ['B', [{ extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Append, value: 'b', variable: 'B', options: undefined }]], + ['C', [{ extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Prepend, value: 'c', variable: 'C', options: undefined }]] ]); // Persist with old service, create a new service with the same storage service to verify restore environmentVariableService.persistCollections(); const service2: TestEnvironmentVariableService = instantiationService.createInstance(TestEnvironmentVariableService); deepStrictEqual([...service2.mergedCollection.getVariableMap(undefined).entries()], [ - ['A', [{ extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Replace, value: 'a', variable: 'A' }]], - ['B', [{ extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Append, value: 'b', variable: 'B' }]], - ['C', [{ extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Prepend, value: 'c', variable: 'C' }]] + ['A', [{ extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Replace, value: 'a', variable: 'A', options: undefined }]], + ['B', [{ extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Append, value: 'b', variable: 'B', options: undefined }]], + ['C', [{ extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Prepend, value: 'c', variable: 'C', options: undefined }]] ]); }); @@ -85,10 +85,10 @@ suite('EnvironmentVariable - EnvironmentVariableService', () => { environmentVariableService.set('ext3', { map: collection3, persistent: true }); deepStrictEqual([...environmentVariableService.mergedCollection.getVariableMap(undefined).entries()], [ ['A', [ - { extensionIdentifier: 'ext2', type: EnvironmentVariableMutatorType.Replace, value: 'a2', variable: 'A' }, - { extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Append, value: 'a1', variable: 'A' } + { extensionIdentifier: 'ext2', type: EnvironmentVariableMutatorType.Replace, value: 'a2', variable: 'A', options: undefined }, + { extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Append, value: 'a1', variable: 'A', options: undefined } ]], - ['B', [{ extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Replace, value: 'b1', variable: 'B' }]] + ['B', [{ extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Replace, value: 'b1', variable: 'B', options: undefined }]] ]); }); @@ -106,9 +106,9 @@ suite('EnvironmentVariable - EnvironmentVariableService', () => { // The entries should be ordered in the order they are applied deepStrictEqual([...environmentVariableService.mergedCollection.getVariableMap(undefined).entries()], [ ['A', [ - { extensionIdentifier: 'ext3', type: EnvironmentVariableMutatorType.Replace, value: 'a3', variable: 'A' }, - { extensionIdentifier: 'ext2', type: EnvironmentVariableMutatorType.Prepend, value: 'a2:', variable: 'A' }, - { extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Append, value: ':a1', variable: 'A' } + { extensionIdentifier: 'ext3', type: EnvironmentVariableMutatorType.Replace, value: 'a3', variable: 'A', options: undefined }, + { extensionIdentifier: 'ext2', type: EnvironmentVariableMutatorType.Prepend, value: 'a2:', variable: 'A', options: undefined }, + { extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Append, value: ':a1', variable: 'A', options: undefined } ]] ]); @@ -134,8 +134,8 @@ suite('EnvironmentVariable - EnvironmentVariableService', () => { // The entries should be ordered in the order they are applied deepStrictEqual([...environmentVariableService.mergedCollection.getVariableMap(scope1).entries()], [ ['A', [ - { extensionIdentifier: 'ext2', type: EnvironmentVariableMutatorType.Prepend, value: 'a2:', variable: 'A' }, - { extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Append, value: ':a1', scope: scope1, variable: 'A' } + { extensionIdentifier: 'ext2', type: EnvironmentVariableMutatorType.Prepend, value: 'a2:', variable: 'A', options: undefined }, + { extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Append, value: ':a1', scope: scope1, variable: 'A', options: undefined } ]] ]); From 35bdb61be58750c15e7060d18386fd8f6cb3d295 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Tue, 23 May 2023 06:47:21 -0700 Subject: [PATCH 10/13] Add mutator options to some tests --- .../test/common/environmentVariableCollection.test.ts | 4 ++-- .../terminal/test/common/environmentVariableService.test.ts | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/test/common/environmentVariableCollection.test.ts b/src/vs/workbench/contrib/terminal/test/common/environmentVariableCollection.test.ts index e13f89af0e3d9..cffe7c45081c1 100644 --- a/src/vs/workbench/contrib/terminal/test/common/environmentVariableCollection.test.ts +++ b/src/vs/workbench/contrib/terminal/test/common/environmentVariableCollection.test.ts @@ -31,13 +31,13 @@ suite('EnvironmentVariable - MergedEnvironmentVariableCollection', () => { }], ['ext4', { map: deserializeEnvironmentVariableCollection([ - ['A-key', { value: 'a4', type: EnvironmentVariableMutatorType.Append, variable: 'A' }] + ['A-key', { value: 'a4', type: EnvironmentVariableMutatorType.Append, variable: 'A', options: { applyAtProcessCreation: true, applyAtShellIntegration: true } }] ]) }] ])); deepStrictEqual([...merged.getVariableMap(undefined).entries()], [ ['A', [ - { extensionIdentifier: 'ext4', type: EnvironmentVariableMutatorType.Append, value: 'a4', variable: 'A', options: undefined }, + { extensionIdentifier: 'ext4', type: EnvironmentVariableMutatorType.Append, value: 'a4', variable: 'A', options: { applyAtProcessCreation: true, applyAtShellIntegration: true } }, { extensionIdentifier: 'ext3', type: EnvironmentVariableMutatorType.Prepend, value: 'a3', variable: 'A', options: undefined }, { extensionIdentifier: 'ext2', type: EnvironmentVariableMutatorType.Append, value: 'a2', variable: 'A', options: undefined }, { extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Prepend, value: 'a1', variable: 'A', options: undefined } diff --git a/src/vs/workbench/contrib/terminal/test/common/environmentVariableService.test.ts b/src/vs/workbench/contrib/terminal/test/common/environmentVariableService.test.ts index edd6f43bd5116..4d3d7a840c362 100644 --- a/src/vs/workbench/contrib/terminal/test/common/environmentVariableService.test.ts +++ b/src/vs/workbench/contrib/terminal/test/common/environmentVariableService.test.ts @@ -51,12 +51,12 @@ suite('EnvironmentVariable - EnvironmentVariableService', () => { const collection = new Map(); collection.set('A-key', { value: 'a', type: EnvironmentVariableMutatorType.Replace, variable: 'A' }); collection.set('B-key', { value: 'b', type: EnvironmentVariableMutatorType.Append, variable: 'B' }); - collection.set('C-key', { value: 'c', type: EnvironmentVariableMutatorType.Prepend, variable: 'C' }); + collection.set('C-key', { value: 'c', type: EnvironmentVariableMutatorType.Prepend, variable: 'C', options: { applyAtProcessCreation: true, applyAtShellIntegration: true } }); environmentVariableService.set('ext1', { map: collection, persistent: true }); deepStrictEqual([...environmentVariableService.mergedCollection.getVariableMap(undefined).entries()], [ ['A', [{ extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Replace, value: 'a', variable: 'A', options: undefined }]], ['B', [{ extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Append, value: 'b', variable: 'B', options: undefined }]], - ['C', [{ extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Prepend, value: 'c', variable: 'C', options: undefined }]] + ['C', [{ extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Prepend, value: 'c', variable: 'C', options: { applyAtProcessCreation: true, applyAtShellIntegration: true } }]] ]); // Persist with old service, create a new service with the same storage service to verify restore @@ -65,7 +65,7 @@ suite('EnvironmentVariable - EnvironmentVariableService', () => { deepStrictEqual([...service2.mergedCollection.getVariableMap(undefined).entries()], [ ['A', [{ extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Replace, value: 'a', variable: 'A', options: undefined }]], ['B', [{ extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Append, value: 'b', variable: 'B', options: undefined }]], - ['C', [{ extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Prepend, value: 'c', variable: 'C', options: undefined }]] + ['C', [{ extensionIdentifier: 'ext1', type: EnvironmentVariableMutatorType.Prepend, value: 'c', variable: 'C', options: { applyAtProcessCreation: true, applyAtShellIntegration: true } }]] ]); }); From 8b079fafeaf56f2c5b6301c5add5829e7d9b0841 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Tue, 23 May 2023 07:38:47 -0700 Subject: [PATCH 11/13] Remove unwanted log --- src/vs/workbench/api/browser/mainThreadTerminalService.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/vs/workbench/api/browser/mainThreadTerminalService.ts b/src/vs/workbench/api/browser/mainThreadTerminalService.ts index d11771537f0c4..f43a403ff42a0 100644 --- a/src/vs/workbench/api/browser/mainThreadTerminalService.ts +++ b/src/vs/workbench/api/browser/mainThreadTerminalService.ts @@ -407,7 +407,6 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape map: deserializeEnvironmentVariableCollection(collection), descriptionMap: deserializeEnvironmentDescriptionMap(descriptionMap) }; - console.log('translatedCollection', translatedCollection); this._environmentVariableService.set(extensionIdentifier, translatedCollection); } else { this._environmentVariableService.delete(extensionIdentifier); From 27293f19d28e908fb6971a23ac5414ea8ed3a7f2 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Tue, 23 May 2023 08:34:06 -0700 Subject: [PATCH 12/13] Fix env var collection api tests --- .../src/singlefolder-tests/terminal.test.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts index 36c1e66c85249..2be7496cb8635 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts @@ -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: undefined }); + deepStrictEqual(collection.get('B'), { value: '~b2~', type: EnvironmentVariableMutatorType.Append, scope: undefined, options: undefined }); + deepStrictEqual(collection.get('C'), { value: '~c2~', type: EnvironmentVariableMutatorType.Prepend, scope: undefined, options: undefined }); // 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: undefined }], + ['B', { value: '~b2~', type: EnvironmentVariableMutatorType.Append, scope: undefined, options: undefined }], + ['C', { value: '~c2~', type: EnvironmentVariableMutatorType.Prepend, scope: undefined, options: undefined }] ]); }); }); From 438eb68ef6277038b600811f90435b17fb731721 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Tue, 23 May 2023 09:02:42 -0700 Subject: [PATCH 13/13] Actually fix the tests, options is an empty object not undefined --- .../src/singlefolder-tests/terminal.test.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts index 2be7496cb8635..e902bcd04a7cc 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts @@ -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, options: undefined }); - deepStrictEqual(collection.get('B'), { value: '~b2~', type: EnvironmentVariableMutatorType.Append, scope: undefined, options: undefined }); - deepStrictEqual(collection.get('C'), { value: '~c2~', type: EnvironmentVariableMutatorType.Prepend, scope: undefined, options: 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, options: undefined }], - ['B', { value: '~b2~', type: EnvironmentVariableMutatorType.Append, scope: undefined, options: undefined }], - ['C', { value: '~c2~', type: EnvironmentVariableMutatorType.Prepend, scope: undefined, options: 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: {} }] ]); }); });