Merge pull request microsoft#182067 from microsoft/tyriar/181755

Fire onDidRemoveStatus if the status differs

Author: Tyriar

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

@@ -38,6 +38,8 @@ export interface ITerminalStatusList {
 
 	/**
 	 * Adds a status to the list.
+	 * @param status The status object. Ideally a single status object that does not change will be
+	 * shared as this call will no-op if the status is already set (checked by by object reference).
 	 * @param duration An optional duration in milliseconds of the status, when specified the status
 	 * will remove itself when the duration elapses unless the status gets re-added.
 	 */
@@ -87,6 +89,11 @@ export class TerminalStatusList extends Disposable implements ITerminalStatusLis
 			const timeout = window.setTimeout(() => this.remove(status), duration);
 			this._statusTimeouts.set(status.id, timeout);
 		}
+		const existingStatus = this._statuses.get(status.id);
+		if (existingStatus && existingStatus !== status) {
+			this._onDidRemoveStatus.fire(existingStatus);
+			this._statuses.delete(existingStatus.id);
+		}
 		if (!this._statuses.has(status.id)) {
 			const oldPrimary = this.primary;
 			this._statuses.set(status.id, status);
@@ -95,11 +102,6 @@ export class TerminalStatusList extends Disposable implements ITerminalStatusLis
 			if (oldPrimary !== newPrimary) {
 				this._onDidChangePrimaryStatus.fire(newPrimary);
 			}
-		} else {
-			this._statuses.set(status.id, status);
-			// It maybe the case that status hasn't changed, there isn't a good way to check this based on
-			// `ITerminalStatus`, so just fire the event anyway.
-			this._onDidAddStatus.fire(status);
 		}
 	}
 

src/vs/workbench/contrib/terminal/test/browser/terminalStatusList.test.ts

@@ -130,6 +130,19 @@ suite('Workbench - TerminalStatusList', () => {
 		strictEqual(list.statuses[1].icon!.id, Codicon.zap.id, 'zap~spin should have animation removed only');
 	});
 
+	test('add should fire onDidRemoveStatus if same status id with a different object reference was added', () => {
+		const eventCalls: string[] = [];
+		list.onDidAddStatus(() => eventCalls.push('add'));
+		list.onDidRemoveStatus(() => eventCalls.push('remove'));
+		list.add({ id: 'test', severity: Severity.Info });
+		list.add({ id: 'test', severity: Severity.Info });
+		deepStrictEqual(eventCalls, [
+			'add',
+			'remove',
+			'add'
+		]);
+	});
+
 	test('remove', () => {
 		list.add({ id: 'info', severity: Severity.Info });
 		list.add({ id: 'warning', severity: Severity.Warning });