From 3e57f8979ba0c1fe173f02b9f7b8f7f7e8ae6ce7 Mon Sep 17 00:00:00 2001
From: Daniel Imms <2193314+Tyriar@users.noreply.github.com>
Date: Wed, 10 May 2023 07:04:36 -0700
Subject: [PATCH] Fire onDidRemoveStatus if the status differs

Fixes #181755
---
 .../contrib/terminal/browser/terminalStatusList.ts  | 12 +++++++-----
 .../test/browser/terminalStatusList.test.ts         | 13 +++++++++++++
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/src/vs/workbench/contrib/terminal/browser/terminalStatusList.ts b/src/vs/workbench/contrib/terminal/browser/terminalStatusList.ts
index 68a867eb313e8..3d8e715653b11 100644
--- a/src/vs/workbench/contrib/terminal/browser/terminalStatusList.ts
+++ b/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);
 		}
 	}
 
diff --git a/src/vs/workbench/contrib/terminal/test/browser/terminalStatusList.test.ts b/src/vs/workbench/contrib/terminal/test/browser/terminalStatusList.test.ts
index 0eef780fbaab0..21d76a53d1655 100644
--- a/src/vs/workbench/contrib/terminal/test/browser/terminalStatusList.test.ts
+++ b/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 });