Fix lifecycle issues in DecorationAddon

DecorationAddon owns the decorations, not the markers so it's not its
job to dispose them. This change also disposes the DecorationAddon inside
XtermTerminal when the setting is switched off and recreates when it goes
back on, this needed some changes around restoring DecorationAddon
properly for existing terminals as well.

Fixes microsoft#145212

Author: Tyriar

src/vs/platform/terminal/common/capabilities/capabilities.ts

@@ -87,6 +87,7 @@ export interface ICommandDetectionCapability {
 	readonly commands: readonly ITerminalCommand[];
 	/** The command currently being executed, otherwise undefined. */
 	readonly executingCommand: string | undefined;
+	readonly executingCommandObject: ITerminalCommand | undefined;
 	/** The current cwd at the cursor's position. */
 	readonly cwd: string | undefined;
 	readonly onCommandStarted: Event<ITerminalCommand>;

src/vs/platform/terminal/common/capabilities/commandDetectionCapability.ts

@@ -56,6 +56,13 @@ export class CommandDetectionCapability implements ICommandDetectionCapability {
 
 	get commands(): readonly ITerminalCommand[] { return this._commands; }
 	get executingCommand(): string | undefined { return this._currentCommand.command; }
+	// TODO: as is unsafe here and it duplicates behavor of executingCommand
+	get executingCommandObject(): ITerminalCommand | undefined {
+		if (this._currentCommand.commandStartMarker) {
+			return { marker: this._currentCommand.commandStartMarker } as ITerminalCommand;
+		}
+		return undefined;
+	}
 	get cwd(): string | undefined { return this._cwd; }
 
 	private readonly _onCommandStarted = new Emitter<ITerminalCommand>();

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

@@ -495,10 +495,6 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
 				e.affectsConfiguration(TerminalSettingId.TerminalDescription)) {
 				this._labelComputer?.refreshLabel();
 			}
-			if ((e.affectsConfiguration(TerminalSettingId.ShellIntegrationDecorationsEnabled) && !this._configurationService.getValue(TerminalSettingId.ShellIntegrationDecorationsEnabled)) ||
-				(e.affectsConfiguration(TerminalSettingId.ShellIntegrationEnabled) && !this._configurationService.getValue(TerminalSettingId.ShellIntegrationEnabled))) {
-				this.xterm?.clearDecorations();
-			}
 		}));
 		this._workspaceContextService.onDidChangeWorkspaceFolders(() => this._labelComputer?.refreshLabel());
 

src/vs/workbench/contrib/terminal/browser/xterm/decorationAddon.ts

@@ -3,7 +3,7 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { Disposable, IDisposable, dispose } from 'vs/base/common/lifecycle';
+import { Disposable, IDisposable, dispose, toDisposable } from 'vs/base/common/lifecycle';
 import { ITerminalCommand } from 'vs/workbench/contrib/terminal/common/terminal';
 import { IDecoration, ITerminalAddon, Terminal } from 'xterm';
 import * as dom from 'vs/base/browser/dom';
@@ -61,7 +61,7 @@ export class DecorationAddon extends Disposable implements ITerminalAddon {
 		@IThemeService private readonly _themeService: IThemeService
 	) {
 		super();
-		this._attachToCommandCapability();
+		this._register(toDisposable(() => this.clearDecorations(true)));
 		this._register(this._contextMenuService.onDidShowContextMenu(() => this._contextMenuVisible = true));
 		this._register(this._contextMenuService.onDidHideContextMenu(() => this._contextMenuVisible = false));
 		this._hoverDelayer = this._register(new Delayer(this._configurationService.getValue('workbench.hover.delay')));
@@ -116,7 +116,6 @@ export class DecorationAddon extends Disposable implements ITerminalAddon {
 		this._placeholderDecoration?.marker.dispose();
 		for (const value of this._decorations.values()) {
 			value.decoration.dispose();
-			value.decoration.marker.dispose();
 			dispose(value.disposables);
 		}
 		this._decorations.clear();
@@ -126,11 +125,12 @@ export class DecorationAddon extends Disposable implements ITerminalAddon {
 	private _attachToCommandCapability(): void {
 		if (this._capabilities.has(TerminalCapability.CommandDetection)) {
 			this._addCommandFinishedListener();
+			this._addCommandStartedListener();
 		} else {
 			this._register(this._capabilities.onDidAddCapability(c => {
 				if (c === TerminalCapability.CommandDetection) {
-					this._addCommandStartedListener();
 					this._addCommandFinishedListener();
+					this._addCommandStartedListener();
 				}
 			}));
 		}
@@ -150,11 +150,8 @@ export class DecorationAddon extends Disposable implements ITerminalAddon {
 		if (!capability) {
 			return;
 		}
-		if (capability.commands.length > 0) {
-			const lastCommand = capability.commands[capability.commands.length - 1];
-			if (lastCommand.marker && !lastCommand.endMarker) {
-				this.registerCommandDecoration(lastCommand, true);
-			}
+		if (capability.executingCommandObject?.marker) {
+			this.registerCommandDecoration(capability.executingCommandObject, true);
 		}
 		this._commandStartedListener = capability.onCommandStarted(command => this.registerCommandDecoration(command, true));
 	}
@@ -182,6 +179,7 @@ export class DecorationAddon extends Disposable implements ITerminalAddon {
 
 	activate(terminal: Terminal): void {
 		this._terminal = terminal;
+		this._attachToCommandCapability();
 	}
 
 	registerCommandDecoration(command: ITerminalCommand, beforeCommandExecution?: boolean): IDecoration | undefined {

src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts

@@ -90,7 +90,7 @@ export class XtermTerminal extends DisposableStore implements IXtermTerminal {
 		cols: number,
 		rows: number,
 		location: TerminalLocation,
-		capabilities: ITerminalCapabilityStore,
+		private readonly _capabilities: ITerminalCapabilityStore,
 		@IConfigurationService private readonly _configurationService: IConfigurationService,
 		@IInstantiationService private readonly _instantiationService: IInstantiationService,
 		@ILogService private readonly _logService: ILogService,
@@ -145,6 +145,10 @@ export class XtermTerminal extends DisposableStore implements IXtermTerminal {
 			if (e.affectsConfiguration(TerminalSettingId.UnicodeVersion)) {
 				this._updateUnicodeVersion();
 			}
+			if (e.affectsConfiguration(TerminalSettingId.ShellIntegrationDecorationsEnabled) ||
+				e.affectsConfiguration(TerminalSettingId.ShellIntegrationEnabled)) {
+				this._updateDecorationAddon();
+			}
 		}));
 
 		this.add(this._themeService.onDidColorThemeChange(theme => this._updateTheme(theme)));
@@ -157,16 +161,14 @@ export class XtermTerminal extends DisposableStore implements IXtermTerminal {
 
 		// Load addons
 		this._updateUnicodeVersion();
-		this._commandNavigationAddon = this._instantiationService.createInstance(CommandNavigationAddon, capabilities);
+		this._commandNavigationAddon = this._instantiationService.createInstance(CommandNavigationAddon, _capabilities);
 		this.raw.loadAddon(this._commandNavigationAddon);
 		this._shellIntegrationAddon = this._instantiationService.createInstance(ShellIntegrationAddon);
 		this.raw.loadAddon(this._shellIntegrationAddon);
-		if (this._configurationService.getValue(TerminalSettingId.ShellIntegrationEnabled) && this._configurationService.getValue(TerminalSettingId.ShellIntegrationDecorationsEnabled)) {
-			this._createDecorationAddon(capabilities);
-		}
+		this._updateDecorationAddon();
 	}
-	private _createDecorationAddon(capabilities: ITerminalCapabilityStore): void {
-		this._decorationAddon = this._instantiationService.createInstance(DecorationAddon, capabilities);
+	private _createDecorationAddon(): void {
+		this._decorationAddon = this._instantiationService.createInstance(DecorationAddon, this._capabilities);
 		this._decorationAddon.onDidRequestRunCommand(command => this._onDidRequestRunCommand.fire(command));
 		this.raw.loadAddon(this._decorationAddon);
 	}
@@ -570,4 +572,17 @@ export class XtermTerminal extends DisposableStore implements IXtermTerminal {
 			this.raw.unicode.activeVersion = this._configHelper.config.unicodeVersion;
 		}
 	}
+
+	private _updateDecorationAddon(): void {
+		if (this._configHelper.config.shellIntegration?.enabled && this._configHelper.config.shellIntegration.decorationsEnabled) {
+			if (!this._decorationAddon) {
+				this._createDecorationAddon();
+			}
+			return;
+		}
+		if (this._decorationAddon) {
+			this._decorationAddon.dispose();
+			this._decorationAddon = undefined;
+		}
+	}
 }

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

@@ -292,6 +292,7 @@ export interface ITerminalConfiguration {
 	autoReplies: { [key: string]: string };
 	shellIntegration?: {
 		enabled: boolean;
+		decorationsEnabled: boolean;
 	};
 }