From 1461c7d38225e5450d3d059b7872593a99d44df3 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 23 Aug 2023 07:54:24 -0700 Subject: [PATCH 1/3] Finalize EnvironmentVariableMutatorOptions API Fixes #179476 --- extensions/vscode-api-tests/package.json | 3 +- .../api/common/extHostTerminalService.ts | 9 --- .../common/extensionsApiProposals.ts | 1 - src/vscode-dts/vscode.d.ts | 33 ++++++++++- .../vscode.proposed.envCollectionOptions.d.ts | 55 ------------------- 5 files changed, 31 insertions(+), 70 deletions(-) delete mode 100644 src/vscode-dts/vscode.proposed.envCollectionOptions.d.ts diff --git a/extensions/vscode-api-tests/package.json b/extensions/vscode-api-tests/package.json index 9d50e393547ec..ac6fb719a90f4 100644 --- a/extensions/vscode-api-tests/package.json +++ b/extensions/vscode-api-tests/package.json @@ -52,8 +52,7 @@ "telemetry", "windowActivity", "interactiveUserActions", - "envCollectionWorkspace", - "envCollectionOptions" + "envCollectionWorkspace" ], "private": true, "activationEvents": [], diff --git a/src/vs/workbench/api/common/extHostTerminalService.ts b/src/vs/workbench/api/common/extHostTerminalService.ts index c7d8b2a83d0b1..c901a5cae3dc3 100644 --- a/src/vs/workbench/api/common/extHostTerminalService.ts +++ b/src/vs/workbench/api/common/extHostTerminalService.ts @@ -944,23 +944,14 @@ class UnifiedEnvironmentVariableCollection { } replace(variable: string, value: string, options: vscode.EnvironmentVariableMutatorOptions | undefined, scope: vscode.EnvironmentVariableScope | undefined): void { - if (this._extension && options) { - checkProposedApiEnabled(this._extension, 'envCollectionOptions'); - } this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Replace, options: options ?? { applyAtProcessCreation: true }, scope }); } append(variable: string, value: string, options: vscode.EnvironmentVariableMutatorOptions | undefined, scope: vscode.EnvironmentVariableScope | undefined): void { - if (this._extension && options) { - checkProposedApiEnabled(this._extension, 'envCollectionOptions'); - } this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Append, options: options ?? { applyAtProcessCreation: true }, scope }); } prepend(variable: string, value: string, options: vscode.EnvironmentVariableMutatorOptions | undefined, scope: vscode.EnvironmentVariableScope | undefined): void { - if (this._extension && options) { - checkProposedApiEnabled(this._extension, 'envCollectionOptions'); - } this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Prepend, options: options ?? { applyAtProcessCreation: true }, scope }); } diff --git a/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts b/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts index 03d80474dad3d..be411915ec1be 100644 --- a/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts +++ b/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts @@ -40,7 +40,6 @@ 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.d.ts b/src/vscode-dts/vscode.d.ts index db7d29f3b1b07..6c962d2e0fd57 100644 --- a/src/vscode-dts/vscode.d.ts +++ b/src/vscode-dts/vscode.d.ts @@ -11334,6 +11334,22 @@ declare module 'vscode' { Prepend = 3 } + /** + * Options applied to the mutator. + */ + export interface EnvironmentVariableMutatorOptions { + /** + * Apply to the environment just before the process is created. + */ + 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. + */ + applyAtShellIntegration?: boolean; + } + /** * A type of mutation and its value to be applied to an environment variable. */ @@ -11347,6 +11363,11 @@ declare module 'vscode' { * The value to use for the variable. */ readonly value: string; + + /** + * Options applied to the mutator. + */ + readonly options: EnvironmentVariableMutatorOptions; } /** @@ -11376,8 +11397,10 @@ declare module 'vscode' { * * @param variable The variable to replace. * @param value The value to replace the variable with. + * @param options Options applied to the mutator, when no options are provided this will + * default to `{ applyAtProcessCreation: true }`. */ - replace(variable: string, value: string): void; + replace(variable: string, value: string, options?: EnvironmentVariableMutatorOptions): void; /** * Append a value to an environment variable. @@ -11387,8 +11410,10 @@ declare module 'vscode' { * * @param variable The variable to append to. * @param value The value to append to the variable. + * @param options Options applied to the mutator, when no options are provided this will + * default to `{ applyAtProcessCreation: true }`. */ - append(variable: string, value: string): void; + append(variable: string, value: string, options?: EnvironmentVariableMutatorOptions): void; /** * Prepend a value to an environment variable. @@ -11398,8 +11423,10 @@ declare module 'vscode' { * * @param variable The variable to prepend. * @param value The value to prepend to the variable. + * @param options Options applied to the mutator, when no options are provided this will + * default to `{ applyAtProcessCreation: true }`. */ - prepend(variable: string, value: string): void; + prepend(variable: string, value: string, options?: EnvironmentVariableMutatorOptions): void; /** * Gets the mutator that this collection applies to a variable, if any. diff --git a/src/vscode-dts/vscode.proposed.envCollectionOptions.d.ts b/src/vscode-dts/vscode.proposed.envCollectionOptions.d.ts deleted file mode 100644 index 1de5bed146ecd..0000000000000 --- a/src/vscode-dts/vscode.proposed.envCollectionOptions.d.ts +++ /dev/null @@ -1,55 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * 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 - - /** - * Options applied to the mutator. - */ - export interface EnvironmentVariableMutatorOptions { - /** - * Apply to the environment just before the process is created. - */ - 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. - */ - 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]> { - /** - * @param options Options applied to the mutator, when not options are provided this will - * default to `{ applyAtProcessCreation: true }` - */ - replace(variable: string, value: string, options?: EnvironmentVariableMutatorOptions): void; - - /** - * @param options Options applied to the mutator, when not options are provided this will - * default to `{ applyAtProcessCreation: true }` - */ - append(variable: string, value: string, options?: EnvironmentVariableMutatorOptions): void; - - /** - * @param options Options applied to the mutator, when not options are provided this will - * default to `{ applyAtProcessCreation: true }` - */ - prepend(variable: string, value: string, options?: EnvironmentVariableMutatorOptions): void; - } -} From d0b353aa4d1d6c7ec62039628c22919b4f8ccfe9 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 24 Aug 2023 06:00:42 -0700 Subject: [PATCH 2/3] Add default values to EnvironmentVariableMutatorOptions --- src/vscode-dts/vscode.d.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/vscode-dts/vscode.d.ts b/src/vscode-dts/vscode.d.ts index 9904b311e50fd..d440e2d2fc166 100644 --- a/src/vscode-dts/vscode.d.ts +++ b/src/vscode-dts/vscode.d.ts @@ -11339,13 +11339,14 @@ declare module 'vscode' { */ export interface EnvironmentVariableMutatorOptions { /** - * Apply to the environment just before the process is created. + * Apply to the environment just before the process is created. Defaults to false. */ 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. + * the mutator if shell integration is disabled or not working for some reason. Defaults to + * false. */ applyAtShellIntegration?: boolean; } From 16cae5ea488e45d183859c3b5dcca7e7af46ae83 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 24 Aug 2023 15:35:08 -0700 Subject: [PATCH 3/3] Feedback --- src/vs/workbench/api/common/extHostTerminalService.ts | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/vs/workbench/api/common/extHostTerminalService.ts b/src/vs/workbench/api/common/extHostTerminalService.ts index ecc9e00556f2d..58bc387e60002 100644 --- a/src/vs/workbench/api/common/extHostTerminalService.ts +++ b/src/vs/workbench/api/common/extHostTerminalService.ts @@ -24,7 +24,6 @@ import { ThemeColor } from 'vs/base/common/themables'; import { Promises } from 'vs/base/common/async'; import { EditorGroupColumn } from 'vs/workbench/services/editor/common/editorGroupColumn'; import { TerminalQuickFix, ViewColumn } from 'vs/workbench/api/common/extHostTypeConverters'; -import { checkProposedApiEnabled } from 'vs/workbench/services/extensions/common/extensions'; import { IExtHostCommands } from 'vs/workbench/api/common/extHostCommands'; export interface IExtHostTerminalService extends ExtHostTerminalServiceShape, IDisposable { @@ -858,7 +857,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 UnifiedEnvironmentVariableCollection(extension); + collection = new UnifiedEnvironmentVariableCollection(); this._setEnvironmentVariableCollection(extension.identifier.value, collection); } return collection.getScopedEnvironmentVariableCollection(undefined); @@ -873,7 +872,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 UnifiedEnvironmentVariableCollection(undefined, entry[1]); + const collection = new UnifiedEnvironmentVariableCollection(entry[1]); this._setEnvironmentVariableCollection(extensionIdentifier, collection); }); } @@ -918,11 +917,6 @@ class UnifiedEnvironmentVariableCollection { 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);