Starting env var collection UX

src/vs/workbench/api/node/extHostTerminalService.ts

@@ -49,6 +49,11 @@ export class ExtHostTerminalService extends BaseExtHostTerminalService {
 		this._updateLastActiveWorkspace();
 		this._updateVariableResolver();
 		this._registerListeners();
+
+		setTimeout(() => {
+			const c = this.getEnvironmentVariableCollection({ identifier: { value: 'test' } } as any);
+			c.replace('FOO', 'BAR');
+		}, 5000);
 	}
 
 	public createTerminal(name?: string, shellPath?: string, shellArgs?: string[] | string): vscode.Terminal {

src/vs/workbench/contrib/terminal/browser/terminalInstance.ts

@@ -95,7 +95,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
 
 	private _messageTitleDisposable: IDisposable | undefined;
 
-	private _widgetManager: TerminalWidgetManager | undefined;
+	private _widgetManager: TerminalWidgetManager = new TerminalWidgetManager();
 	private _linkManager: TerminalLinkManager | undefined;
 	private _commandTrackerAddon: CommandTrackerAddon | undefined;
 	private _navigationModeAddon: INavigationMode & ITerminalAddon | undefined;
@@ -582,9 +582,8 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
 			this._refreshSelectionContextKey();
 		}));
 
-		const widgetManager = new TerminalWidgetManager(this._wrapperElement);
-		this._widgetManager = widgetManager;
-		this._processManager.onProcessReady(() => this._linkManager?.setWidgetManager(widgetManager));
+		this._widgetManager.attachToElement(this._wrapperElement);
+		this._processManager.onProcessReady(() => this._linkManager?.setWidgetManager(this._widgetManager));
 
 		const computedStyle = window.getComputedStyle(this._container);
 		const width = parseInt(computedStyle.getPropertyValue('width').replace('px', ''), 10);
@@ -720,7 +719,6 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
 		dispose(this._commandTrackerAddon);
 		this._commandTrackerAddon = undefined;
 		dispose(this._widgetManager);
-		this._widgetManager = undefined;
 
 		if (this._xterm && this._xterm.element) {
 			this._hadFocusOnExit = dom.hasClass(this._xterm.element, 'focus');
@@ -789,6 +787,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
 		this.focus();
 		this._xterm.paste(await this._clipboardService.readText());
 	}
+
 	public async sendText(text: string, addNewLine: boolean): Promise<void> {
 		// Normalize line endings to 'enter' press.
 		text = text.replace(TerminalInstance.EOL_REGEX, '\r');
@@ -871,6 +870,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
 		this._processManager.onProcessData(data => this._onData.fire(data));
 		this._processManager.onProcessOverrideDimensions(e => this.setDimensions(e));
 		this._processManager.onProcessResolvedShellLaunchConfig(e => this._setResolvedShellLaunchConfig(e));
+		this._processManager.onEnvironmentVariableInfoChanged(e => this._widgetManager?.showEnvironmentVariableInfo);
 
 		if (this._shellLaunchConfig.name) {
 			this.setTitle(this._shellLaunchConfig.name, TitleEventSource.Api);

src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts

@@ -23,8 +23,9 @@ import { IConfigurationService } from 'vs/platform/configuration/common/configur
 import { IRemoteAgentService } from 'vs/workbench/services/remote/common/remoteAgentService';
 import { Disposable } from 'vs/base/common/lifecycle';
 import { withNullAsUndefined } from 'vs/base/common/types';
-import { IEnvironmentVariableService, IMergedEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariable';
+import { IEnvironmentVariableService, IMergedEnvironmentVariableCollection, IEnvironmentVariableInfo } from 'vs/workbench/contrib/terminal/common/environmentVariable';
 import { IRemotePathService } from 'vs/workbench/services/path/common/remotePathService';
+import { EnvironmentVariableInfoStale } from 'vs/workbench/contrib/terminal/common/environmentVariableInfo';
 
 /** The amount of time to consider terminal errors to be related to the launch */
 const LAUNCHING_DURATION = 500;
@@ -77,6 +78,8 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
 	public get onProcessOverrideDimensions(): Event<ITerminalDimensions | undefined> { return this._onProcessOverrideDimensions.event; }
 	private readonly _onProcessOverrideShellLaunchConfig = this._register(new Emitter<IShellLaunchConfig>());
 	public get onProcessResolvedShellLaunchConfig(): Event<IShellLaunchConfig> { return this._onProcessOverrideShellLaunchConfig.event; }
+	private readonly _onEnvironmentVariableInfoChange = this._register(new Emitter<IEnvironmentVariableInfo>());
+	public get onEnvironmentVariableInfoChanged(): Event<IEnvironmentVariableInfo> { return this._onEnvironmentVariableInfoChange.event; }
 
 	constructor(
 		private readonly _terminalId: number,
@@ -320,10 +323,13 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
 
 	private _onEnvironmentVariableCollectionChange(newCollection: IMergedEnvironmentVariableCollection): void {
 		// TODO: React to changes in environment variable collections
-		// const newAdditions = this._extEnvironmentVariableCollection!.getNewAdditions(newCollection);
-		// if (newAdditions === undefined) {
-		// 	return;
-		// }
+		const diff = this._extEnvironmentVariableCollection!.diff(newCollection);
+		if (diff === undefined) {
+			return;
+		}
+		console.log('environments differ', diff);
+		const info = new EnvironmentVariableInfoStale();
+		this._onEnvironmentVariableInfoChange.fire(info);
 		// const promptChoices: IPromptChoice[] = [
 		// 	{
 		// 		label: nls.localize('apply', "Apply"),

src/vs/workbench/contrib/terminal/browser/terminalWidgetManager.ts

@@ -6,6 +6,7 @@
 import { IDisposable, dispose, DisposableStore } from 'vs/base/common/lifecycle';
 import { IMarkdownString } from 'vs/base/common/htmlContent';
 import { renderMarkdown } from 'vs/base/browser/markdownRenderer';
+import { IEnvironmentVariableInfo } from 'vs/workbench/contrib/terminal/common/environmentVariable';
 
 export enum WidgetVerticalAlignment {
 	Bottom,
@@ -21,14 +22,18 @@ export class TerminalWidgetManager implements IDisposable {
 	private _messageWidget: MessageWidget | undefined;
 	private readonly _messageListeners = new DisposableStore();
 
-	constructor(
-		terminalWrapper: HTMLElement,
-	) {
-		this._container = document.createElement('div');
-		this._container.classList.add('terminal-widget-overlay');
-		terminalWrapper.appendChild(this._container);
+	private _environmentVariableInfo: IEnvironmentVariableInfo | undefined;
 
-		this._initTerminalHeightWatcher(terminalWrapper);
+	constructor() {
+	}
+
+	public attachToElement(terminalWrapper: HTMLElement) {
+		if (!this._container) {
+			this._container = document.createElement('div');
+			this._container.classList.add('terminal-widget-overlay');
+			terminalWrapper.appendChild(this._container);
+			this._initTerminalHeightWatcher(terminalWrapper);
+		}
 	}
 
 	public dispose(): void {
@@ -50,6 +55,17 @@ export class TerminalWidgetManager implements IDisposable {
 		mutationObserver.observe(this._xtermViewport, { attributes: true, attributeFilter: ['style'] });
 	}
 
+	public showEnvironmentVariableInfo(info: IEnvironmentVariableInfo): IDisposable {
+		this._environmentVariableInfo = info;
+		return {
+			dispose: () => {
+				if (this._environmentVariableInfo === info) {
+					this._environmentVariableInfo = undefined;
+				}
+			}
+		};
+	}
+
 	public showMessage(left: number, y: number, text: IMarkdownString, verticalAlignment: WidgetVerticalAlignment = WidgetVerticalAlignment.Bottom, linkHandler: (url: string) => void): void {
 		if (!this._container || this._messageWidget?.mouseOver) {
 			return;

src/vs/workbench/contrib/terminal/common/environmentVariable.ts

@@ -51,9 +51,10 @@ export interface IMergedEnvironmentVariableCollection {
 	applyToProcessEnvironment(env: IProcessEnvironment): void;
 
 	/**
-	 * Generates a diff of this connection against another.
+	 * Generates a diff of this connection against another. Returns undefined if the collections are
+	 * the same.
 	 */
-	diff(other: IMergedEnvironmentVariableCollection): IMergedEnvironmentVariableCollectionDiff;
+	diff(other: IMergedEnvironmentVariableCollection): IMergedEnvironmentVariableCollectionDiff | undefined;
 }
 
 /**
@@ -97,3 +98,11 @@ export interface IEnvironmentVariableService {
  * Third: Type
  */
 export type ISerializableEnvironmentVariableCollection = [string, IEnvironmentVariableMutator][];
+
+export interface IEnvironmentVariableInfo {
+	// TODO: Should this be MarkdownString?
+	getInfo(): string;
+
+	// TODO: Is this the right type for an icon?
+	getIcon(): string;
+}

src/vs/workbench/contrib/terminal/common/environmentVariableCollection.ts

@@ -65,7 +65,7 @@ export class MergedEnvironmentVariableCollection implements IMergedEnvironmentVa
 		});
 	}
 
-	diff(other: IMergedEnvironmentVariableCollection): IMergedEnvironmentVariableCollectionDiff {
+	diff(other: IMergedEnvironmentVariableCollection): IMergedEnvironmentVariableCollectionDiff | undefined {
 		const added: Map<string, IExtensionOwnedEnvironmentVariableMutator[]> = new Map();
 		const changed: Map<string, IExtensionOwnedEnvironmentVariableMutator[]> = new Map();
 		const removed: Map<string, IExtensionOwnedEnvironmentVariableMutator[]> = new Map();
@@ -97,6 +97,10 @@ export class MergedEnvironmentVariableCollection implements IMergedEnvironmentVa
 			}
 		});
 
+		if (added.size === 0 && changed.size === 0 && removed.size === 0) {
+			return undefined;
+		}
+
 		return { added, changed, removed };
 	}
 }

src/vs/workbench/contrib/terminal/common/environmentVariableInfo.ts

+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import { IEnvironmentVariableInfo } from 'vs/workbench/contrib/terminal/common/environmentVariable';
+
+export class EnvironmentVariableInfoStale implements IEnvironmentVariableInfo {
+	getInfo(): string {
+		return 'stale';
+	}
+	getIcon(): string {
+		return 'warning';
+	}
+}
+
+export class EnvironmentVariableInfoChangesActive implements IEnvironmentVariableInfo {
+	getInfo(): string {
+		return 'info';
+	}
+	getIcon(): string {
+		return 'info';
+	}
+}

src/vs/workbench/contrib/terminal/common/terminal.ts

@@ -11,6 +11,7 @@ import { createDecorator } from 'vs/platform/instantiation/common/instantiation'
 import { URI } from 'vs/base/common/uri';
 import { OperatingSystem } from 'vs/base/common/platform';
 import { IOpenFileRequest } from 'vs/platform/windows/common/windows';
+import { IEnvironmentVariableInfo } from 'vs/workbench/contrib/terminal/common/environmentVariable';
 
 export const TERMINAL_VIEW_ID = 'workbench.panel.terminal';
 
@@ -302,6 +303,7 @@ export interface ITerminalProcessManager extends IDisposable {
 	readonly onProcessExit: Event<number | undefined>;
 	readonly onProcessOverrideDimensions: Event<ITerminalDimensions | undefined>;
 	readonly onProcessResolvedShellLaunchConfig: Event<IShellLaunchConfig>;
+	readonly onEnvironmentVariableInfoChanged: Event<IEnvironmentVariableInfo>;
 
 	dispose(immediate?: boolean): void;
 	createProcess(shellLaunchConfig: IShellLaunchConfig, cols: number, rows: number, isScreenReaderModeEnabled: boolean): Promise<void>;

src/vs/workbench/contrib/terminal/test/common/environmentVariableCollection.test.ts

@@ -156,6 +156,24 @@ suite('EnvironmentVariable - MergedEnvironmentVariableCollection', () => {
 	});
 
 	suite('diff', () => {
+		test('should return undefined when collectinos are the same', () => {
+			const merged1 = new MergedEnvironmentVariableCollection(new Map([
+				['ext1', {
+					map: deserializeEnvironmentVariableCollection([
+						['A', { value: 'a', type: EnvironmentVariableMutatorType.Replace }]
+					])
+				}]
+			]));
+			const merged2 = new MergedEnvironmentVariableCollection(new Map([
+				['ext1', {
+					map: deserializeEnvironmentVariableCollection([
+						['A', { value: 'a', type: EnvironmentVariableMutatorType.Replace }]
+					])
+				}]
+			]));
+			const diff = merged1.diff(merged2);
+			strictEqual(diff, undefined);
+		});
 		test('should generate added diffs from when the first entry is added', () => {
 			const merged1 = new MergedEnvironmentVariableCollection(new Map([]));
 			const merged2 = new MergedEnvironmentVariableCollection(new Map([
@@ -165,7 +183,7 @@ suite('EnvironmentVariable - MergedEnvironmentVariableCollection', () => {
 					])
 				}]
 			]));
-			const diff = merged1.diff(merged2);
+			const diff = merged1.diff(merged2)!;
 			strictEqual(diff.changed.size, 0);
 			strictEqual(diff.removed.size, 0);
 			const entries = [...diff.added.entries()];
@@ -190,7 +208,7 @@ suite('EnvironmentVariable - MergedEnvironmentVariableCollection', () => {
 					])
 				}]
 			]));
-			const diff = merged1.diff(merged2);
+			const diff = merged1.diff(merged2)!;
 			strictEqual(diff.changed.size, 0);
 			strictEqual(diff.removed.size, 0);
 			const entries = [...diff.added.entries()];
@@ -220,7 +238,7 @@ suite('EnvironmentVariable - MergedEnvironmentVariableCollection', () => {
 					])
 				}]
 			]));
-			const diff = merged1.diff(merged2);
+			const diff = merged1.diff(merged2)!;
 			strictEqual(diff.changed.size, 0);
 			strictEqual(diff.removed.size, 0);
 			deepStrictEqual([...diff.added.entries()], [
@@ -240,7 +258,7 @@ suite('EnvironmentVariable - MergedEnvironmentVariableCollection', () => {
 					])
 				}]
 			]));
-			const diff2 = merged1.diff(merged3);
+			const diff2 = merged1.diff(merged3)!;
 			strictEqual(diff2.changed.size, 0);
 			strictEqual(diff2.removed.size, 0);
 			deepStrictEqual([...diff.added.entries()], [...diff2.added.entries()], 'Swapping the order of the entries in the other collection should yield the same result');
@@ -267,7 +285,7 @@ suite('EnvironmentVariable - MergedEnvironmentVariableCollection', () => {
 					])
 				}]
 			]));
-			const diff = merged1.diff(merged4);
+			const diff = merged1.diff(merged4)!;
 			strictEqual(diff.changed.size, 0);
 			strictEqual(diff.removed.size, 0);
 			deepStrictEqual([...diff.added.entries()], [], 'Replace should ignore any entries after it');
@@ -289,7 +307,7 @@ suite('EnvironmentVariable - MergedEnvironmentVariableCollection', () => {
 					])
 				}]
 			]));
-			const diff = merged1.diff(merged2);
+			const diff = merged1.diff(merged2)!;
 			strictEqual(diff.changed.size, 0);
 			strictEqual(diff.added.size, 0);
 			deepStrictEqual([...diff.removed.entries()], [
@@ -314,7 +332,7 @@ suite('EnvironmentVariable - MergedEnvironmentVariableCollection', () => {
 					])
 				}]
 			]));
-			const diff = merged1.diff(merged2);
+			const diff = merged1.diff(merged2)!;
 			strictEqual(diff.added.size, 0);
 			strictEqual(diff.removed.size, 0);
 			deepStrictEqual([...diff.changed.entries()], [
@@ -340,7 +358,7 @@ suite('EnvironmentVariable - MergedEnvironmentVariableCollection', () => {
 					])
 				}]
 			]));
-			const diff = merged1.diff(merged2);
+			const diff = merged1.diff(merged2)!;
 			deepStrictEqual([...diff.added.entries()], [
 				['C', [{ extensionIdentifier: 'ext1', value: 'c', type: EnvironmentVariableMutatorType.Append }]],
 			]);