EnvironmentVariableCollection API enhancements

[Tyriar]

This issue tracks the overall improvements for the upcoming API improvements so they're all in one place:

• Allow EnvironmentVariableCollection API to apply changes via shell integration #179476
• Allow specifying workspace-specific EnvironmentVariableCollection mutators #171173
• Allow to provide description to environment collection API #171108

We don't currently have a proposal for the first issue but the combined proposal for the other two is currently:

export interface EnvironmentVariableMutator {
	readonly type: EnvironmentVariableMutatorType;
	readonly value: string;
	readonly scope: EnvironmentVariableScope | undefined;
}

export interface EnvironmentVariableCollection extends Iterable<[variable: string, mutator: EnvironmentVariableMutator]> {
	/**
	 * Sets a description for the environment variable collection, this will be used to describe the changes in the UI.
	 * @param description A description for the environment variable collection.
	 * @param scope The scope to which this description applies to. If unspecified, operation applies to all
	 * workspace folders.
	 */
	setDescription(description: string | MarkdownString | undefined, scope?: EnvironmentVariableScope): void;

	replace(variable: string, value: string, scope?: EnvironmentVariableScope): void;
	append(variable: string, value: string, scope?: EnvironmentVariableScope): void;
	prepend(variable: string, value: string, scope?: EnvironmentVariableScope): void;
	get(variable: string, scope?: EnvironmentVariableScope): EnvironmentVariableMutator | undefined;
	delete(variable: string, scope?: EnvironmentVariableScope): void;
	clear(scope?: EnvironmentVariableScope): void;
}

export type EnvironmentVariableScope = {
	/**
	* The workspace folder to which this collection applies to. If unspecified, collection applies to all workspace folders.
	*/
	workspaceFolder?: WorkspaceFolder;
};

I have given feedback about scope being everywhere and maybe we should have a getScopedEnvironmentVariableCollection API on ExtensionContext instead of peppering it everywhere.

[karrtikr]

Alternative proposal:

export interface ExtensionContext {
	/**
	 * Gets the extension's environment variable collection for this workspace, enabling changes
	 * to be applied to terminal environment variables.
	 * 
	 * @param scope The scope to which the environment variable collection applies to.
	 */
	getEnvironmentVariableCollection(scope?: EnvironmentVariableScope): EnvironmentVariableCollection;
}

export type EnvironmentVariableScope = {
	/**
	* Any specific workspace folder to get collection for. If unspecified, collection applicable to all workspace folders is returned.
	*/
	workspaceFolder?: WorkspaceFolder;
};

export interface EnvironmentVariableCollection extends Iterable<[variable: string, mutator: EnvironmentVariableMutator]> {
	/**
	 * A description for the environment variable collection, this will be used to describe the
	 * changes in the UI.
	 */
	description: string | MarkdownString | undefined;
}

Note the following already exists, so we'll be duplicating APIs, in cases where scope is not required:

export interface ExtensionContext {
	/**
	 * Gets the extension's environment variable collection for this workspace, enabling changes
	 * to be applied to terminal environment variables.
	 */
	readonly environmentVariableCollection: EnvironmentVariableCollection;
}

[karrtikr]

Another thing to note is:

• Existing API extends Iterable<[variable: string, mutator: EnvironmentVariableMutator]> only makes sense with the second proposal.
• In the future we may also be adding options parameter to EnvironmentVariableCollection.replace() API.

[Tyriar]

Comments/open questions:

readonly scope: EnvironmentVariableScope | undefined;

This instead?

readonly scope?: EnvironmentVariableScope

getEnvironmentVariableCollection(scope?: EnvironmentVariableScope): EnvironmentVariableCollection;

Do we need some way to enumerate and delete collections?

getEnvironmentVariableCollection(scope?: EnvironmentVariableScope): EnvironmentVariableCollection;

My preference if we go this route:

getScopedEnvironmentVariableCollection(scope: EnvironmentVariableScope): EnvironmentVariableCollection;

export interface EnvironmentVariableCollection extends Iterable<[variable: string, mutator: EnvironmentVariableMutator]> {
	/**
	 * A description for the environment variable collection, this will be used to describe the
	 * changes in the UI.
	 */
	description: string | MarkdownString | undefined;
}

Do we need a note here about only taking the first line of the description?

[Tyriar]

Feedback from API sync:

• Workspace scope
  • How does the default and scoped collections interact? @karrtikr and I chatted about this; they are completely separate objects and are both applied, similar to how 2 extension collections both apply. We need to make sure this is explained well in the jsdoc
  • getScopedEnvironmentVariableCollection should live under EnvironmentVariableCollection, maybe as getScoped?
  • We can use EnvironmentVariableCollection & { getScoped(workspace) } as the type, or Exclude<EnvironmentVariableCollection, 'getScoped'>. Whatever way is preferred in the future API sync
  • Still not sure if we need a way to get all scoped collections and/or delete them, let's assume no for now as an extension can always do getScoped(workspace).clear()
• Description
  • Let's finalize the description property 🚀

[karrtikr]

Updated proposal

export interface ExtensionContext {
	/**
	 * Gets the extension's environment variable collection for this workspace, enabling changes
	 * to be applied to terminal environment variables.
	 *
	 * @param scope The scope to which the environment variable collection applies to.
	 */
	readonly environmentVariableCollection: EnvironmentVariableCollection & { getScopedEnvironmentVariableCollection(scope: EnvironmentVariableScope): EnvironmentVariableCollection };
}

export type EnvironmentVariableScope = {
	/**
	* Any specific workspace folder to get collection for. If unspecified, collection applicable to all workspace folders is returned.
	*/
	workspaceFolder?: WorkspaceFolder;
};

export interface EnvironmentVariableCollection extends Iterable<[variable: string, mutator: EnvironmentVariableMutator]> {
	/**
	 * A description for the environment variable collection, this will be used to describe the
	 * changes in the UI.
	 */
	description: string | MarkdownString | undefined;
}

[karrtikr]

Closing in favor of #171173 as the other proposal is finalized.