#132183 clean up

- Use FileOutputChannelModel for OutputChannelBackedByFile
- Rename replaceAll to replace
- simplify exthost output service and channel impl
- enhance update api

src/vs/workbench/api/browser/mainThreadOutputService.ts

@@ -4,7 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 import { Registry } from 'vs/platform/registry/common/platform';
-import { IOutputService, IOutputChannel, OUTPUT_VIEW_ID } from 'vs/workbench/contrib/output/common/output';
+import { IOutputService, IOutputChannel, OUTPUT_VIEW_ID, OutputChannelUpdateMode } from 'vs/workbench/contrib/output/common/output';
 import { Extensions, IOutputChannelRegistry } from 'vs/workbench/services/output/common/output';
 import { MainThreadOutputServiceShape, MainContext, IExtHostContext, ExtHostOutputServiceShape, ExtHostContext } from '../common/extHost.protocol';
 import { extHostNamedCustomer } from 'vs/workbench/api/common/extHostCustomers';
@@ -12,6 +12,7 @@ import { UriComponents, URI } from 'vs/base/common/uri';
 import { Disposable, toDisposable } from 'vs/base/common/lifecycle';
 import { Event } from 'vs/base/common/event';
 import { IViewsService } from 'vs/workbench/common/views';
+import { isNumber } from 'vs/base/common/types';
 
 @extHostNamedCustomer(MainContext.MainThreadOutputService)
 export class MainThreadOutputService extends Disposable implements MainThreadOutputServiceShape {
@@ -65,26 +66,30 @@ export class MainThreadOutputService extends Disposable implements MainThreadOut
 		return undefined;
 	}
 
-	public $update(channelId: string): Promise<void> | undefined {
+	public $update(channelId: string, mode: OutputChannelUpdateMode, till?: number): Promise<void> | undefined {
 		const channel = this._getChannel(channelId);
 		if (channel) {
-			channel.update();
+			if (mode === OutputChannelUpdateMode.Append) {
+				channel.update(mode);
+			} else if (isNumber(till)) {
+				channel.update(mode, till);
+			}
 		}
 		return undefined;
 	}
 
-	public $clear(channelId: string, till: number): Promise<void> | undefined {
+	public $clear(channelId: string): Promise<void> | undefined {
 		const channel = this._getChannel(channelId);
 		if (channel) {
-			channel.clear(till);
+			channel.clear();
 		}
 		return undefined;
 	}
 
-	public $replaceAll(channelId: string, till: number, value: string): Promise<void> | undefined {
+	public $replace(channelId: string, value: string): Promise<void> | undefined {
 		const channel = this._getChannel(channelId);
 		if (channel) {
-			channel.replaceAll(till, value);
+			channel.replace(value);
 		}
 		return undefined;
 	}

src/vs/workbench/api/common/extHost.protocol.ts

@@ -56,6 +56,7 @@ import { IAdapterDescriptor, IConfig, IDebugSessionReplMode } from 'vs/workbench
 import * as notebookCommon from 'vs/workbench/contrib/notebook/common/notebookCommon';
 import { CellExecutionUpdateType, ICellExecutionComplete, ICellExecutionStateUpdate } from 'vs/workbench/contrib/notebook/common/notebookExecutionService';
 import { ICellRange } from 'vs/workbench/contrib/notebook/common/notebookRange';
+import { OutputChannelUpdateMode } from 'vs/workbench/contrib/output/common/output';
 import { InputValidationType } from 'vs/workbench/contrib/scm/common/scm';
 import { ITextQueryBuilderOptions } from 'vs/workbench/contrib/search/common/queryBuilder';
 import { ISerializableEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariable';
@@ -445,9 +446,10 @@ export interface MainThreadMessageServiceShape extends IDisposable {
 export interface MainThreadOutputServiceShape extends IDisposable {
 	$register(label: string, log: boolean, file?: UriComponents, extensionId?: string): Promise<string>;
 	$append(channelId: string, value: string): Promise<void> | undefined;
-	$update(channelId: string): Promise<void> | undefined;
-	$clear(channelId: string, till: number): Promise<void> | undefined;
-	$replaceAll(channelId: string, till: number, value?: string): Promise<void> | undefined;
+	$clear(channelId: string): Promise<void> | undefined;
+	$replace(channelId: string, value: string): Promise<void> | undefined;
+	$update(channelId: string, mode: OutputChannelUpdateMode.Append): Promise<void> | undefined;
+	$update(channelId: string, mode: OutputChannelUpdateMode, till: number): Promise<void> | undefined;
 	$reveal(channelId: string, preserveFocus: boolean): Promise<void> | undefined;
 	$close(channelId: string): Promise<void> | undefined;
 	$dispose(channelId: string): Promise<void> | undefined;

src/vs/workbench/api/common/extHostOutput.ts

@@ -7,7 +7,6 @@ import { MainContext, MainThreadOutputServiceShape, ExtHostOutputServiceShape }
 import type * as vscode from 'vscode';
 import { URI } from 'vs/base/common/uri';
 import { Disposable } from 'vs/base/common/lifecycle';
-import { VSBuffer } from 'vs/base/common/buffer';
 import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
 import { IExtHostRpcService } from 'vs/workbench/api/common/extHostRpcService';
 import { IExtensionDescription } from 'vs/platform/extensions/common/extensions';
@@ -18,73 +17,37 @@ export abstract class AbstractExtHostOutputChannel extends Disposable implements
 	readonly _id: Promise<string>;
 	private readonly _name: string;
 	protected readonly _proxy: MainThreadOutputServiceShape;
+
 	private _disposed: boolean;
-	private _offset: number;
-	private readonly _extension: IExtensionDescription;
+	get disposed(): boolean { return this._disposed; }
 
 	public visible: boolean = false;
 
-	constructor(name: string, log: boolean, file: URI | undefined, extension: IExtensionDescription, proxy: MainThreadOutputServiceShape) {
+	constructor(name: string, log: boolean, file: URI | undefined, extensionId: string, proxy: MainThreadOutputServiceShape) {
 		super();
 
 		this._name = name;
 		this._proxy = proxy;
-		this._id = proxy.$register(this.name, log, file, extension.identifier.value);
+		this._id = proxy.$register(this.name, log, file, extensionId);
 		this._disposed = false;
-		this._offset = 0;
-		this._extension = extension;
 	}
 
 	get name(): string {
 		return this._name;
 	}
 
-	append(value: string): void {
-		this.validate();
-		this.incrementOffset(value);
-	}
-
 	appendLine(value: string): void {
-		this.validate();
 		this.append(value + '\n');
 	}
 
-	clear(): void {
-		this.validate();
-		const till = this._offset;
-		this._id.then(id => this._proxy.$clear(id, till));
-	}
-
-	replaceAll(value: string, donotSendValue?: boolean): void {
-		this.validate(true);
-		const till = this._offset;
-		this.incrementOffset(value);
-		this._id.then(id => this._proxy.$replaceAll(id, till, donotSendValue ? undefined : value));
-	}
-
 	show(columnOrPreserveFocus?: vscode.ViewColumn | boolean, preserveFocus?: boolean): void {
-		this.validate();
 		this._id.then(id => this._proxy.$reveal(id, !!(typeof columnOrPreserveFocus === 'boolean' ? columnOrPreserveFocus : preserveFocus)));
 	}
 
 	hide(): void {
-		this.validate();
 		this._id.then(id => this._proxy.$close(id));
 	}
 
-	protected validate(checkProposedApi?: boolean): void {
-		if (checkProposedApi) {
-			checkProposedApiEnabled(this._extension);
-		}
-		if (this._disposed) {
-			throw new Error('Channel has been closed');
-		}
-	}
-
-	private incrementOffset(value: string) {
-		this._offset += value ? VSBuffer.fromString(value).byteLength : 0;
-	}
-
 	override dispose(): void {
 		super.dispose();
 
@@ -94,50 +57,30 @@ export abstract class AbstractExtHostOutputChannel extends Disposable implements
 				.then(() => this._disposed = true);
 		}
 	}
+
+	abstract append(value: string): void;
+	abstract clear(): void;
+	abstract replace(value: string): void;
 }
 
 export class ExtHostPushOutputChannel extends AbstractExtHostOutputChannel {
 
-	constructor(name: string, extension: IExtensionDescription, proxy: MainThreadOutputServiceShape) {
-		super(name, false, undefined, extension, proxy);
+	constructor(name: string, extensionId: string, proxy: MainThreadOutputServiceShape) {
+		super(name, false, undefined, extensionId, proxy);
 	}
 
-	override append(value: string): void {
-		super.append(value);
+	append(value: string): void {
 		this._id.then(id => this._proxy.$append(id, value));
 	}
 
-
-}
-
-export class LazyOutputChannel implements vscode.OutputChannel {
-
-	constructor(
-		readonly name: string,
-		private readonly _channel: Promise<AbstractExtHostOutputChannel>
-	) { }
-
-	append(value: string): void {
-		this._channel.then(channel => channel.append(value));
-	}
-	appendLine(value: string): void {
-		this._channel.then(channel => channel.appendLine(value));
-	}
 	clear(): void {
-		this._channel.then(channel => channel.clear());
+		this._id.then(id => this._proxy.$clear(id));
 	}
-	replaceAll(value: string): void {
-		this._channel.then(channel => channel.replaceAll(value));
-	}
-	show(columnOrPreserveFocus?: vscode.ViewColumn | boolean, preserveFocus?: boolean): void {
-		this._channel.then(channel => channel.show(columnOrPreserveFocus, preserveFocus));
-	}
-	hide(): void {
-		this._channel.then(channel => channel.hide());
-	}
-	dispose(): void {
-		this._channel.then(channel => channel.dispose());
+
+	replace(value: string): void {
+		this._id.then(id => this._proxy.$replace(id, value));
 	}
+
 }
 
 export class ExtHostOutputService implements ExtHostOutputServiceShape {
@@ -145,12 +88,18 @@ export class ExtHostOutputService implements ExtHostOutputServiceShape {
 	readonly _serviceBrand: undefined;
 
 	protected readonly _proxy: MainThreadOutputServiceShape;
+	private readonly _channels: Map<string, AbstractExtHostOutputChannel> = new Map<string, AbstractExtHostOutputChannel>();
+	private visibleChannelId: string | null = null;
 
 	constructor(@IExtHostRpcService extHostRpc: IExtHostRpcService) {
 		this._proxy = extHostRpc.getProxy(MainContext.MainThreadOutputService);
 	}
 
-	$setVisibleChannel(channelId: string): void {
+	$setVisibleChannel(visibleChannelId: string | null): void {
+		this.visibleChannelId = visibleChannelId;
+		for (const [id, channel] of this._channels) {
+			channel.visible = id === this.visibleChannelId;
+		}
 	}
 
 	createOutputChannel(name: string, extension: IExtensionDescription): vscode.OutputChannel {
@@ -158,9 +107,73 @@ export class ExtHostOutputService implements ExtHostOutputServiceShape {
 		if (!name) {
 			throw new Error('illegal argument `name`. must not be falsy');
 		}
-		return new ExtHostPushOutputChannel(name, extension, this._proxy);
+		const extHostOutputChannel = this.doCreateOutChannel(name, extension);
+		extHostOutputChannel.then(channel => channel._id.then(id => {
+			this._channels.set(id, channel);
+			channel.visible = id === this.visibleChannelId;
+		}));
+		return this.createExtHostOutputChannel(name, extHostOutputChannel, extension);
 	}
 
+	protected async doCreateOutChannel(name: string, extension: IExtensionDescription): Promise<AbstractExtHostOutputChannel> {
+		return new ExtHostPushOutputChannel(name, extension.identifier.value, this._proxy);
+	}
+
+	private createExtHostOutputChannel(name: string, channelPromise: Promise<AbstractExtHostOutputChannel>, extensionDescription: IExtensionDescription): vscode.OutputChannel {
+		const validate = (channel: AbstractExtHostOutputChannel, checkProposedApi?: boolean) => {
+			if (checkProposedApi) {
+				checkProposedApiEnabled(extensionDescription);
+			}
+			if (channel.disposed) {
+				throw new Error('Channel has been closed');
+			}
+		};
+		return {
+			get name(): string { return name; },
+			append(value: string): void {
+				channelPromise.then(channel => {
+					validate(channel);
+					channel.append(value);
+				});
+			},
+			appendLine(value: string): void {
+				channelPromise.then(channel => {
+					validate(channel);
+					channel.appendLine(value);
+				});
+			},
+			clear(): void {
+				channelPromise.then(channel => {
+					validate(channel);
+					channel.clear();
+				});
+			},
+			replace(value: string): void {
+				channelPromise.then(channel => {
+					validate(channel, true);
+					channel.replace(value);
+				});
+			},
+			show(columnOrPreserveFocus?: vscode.ViewColumn | boolean, preserveFocus?: boolean): void {
+				channelPromise.then(channel => {
+					validate(channel);
+					channel.show(columnOrPreserveFocus, preserveFocus);
+				});
+			},
+			hide(): void {
+				channelPromise.then(channel => {
+					validate(channel);
+					channel.hide();
+				});
+			},
+			dispose(): void {
+				channelPromise.then(channel => {
+					validate(channel);
+					channel.dispose();
+				});
+			}
+		};
+	}
 }
 
 export interface IExtHostOutputService extends ExtHostOutputService { }

src/vs/workbench/api/node/extHost.node.services.ts

@@ -4,7 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 import { registerSingleton } from 'vs/platform/instantiation/common/extensions';
-import { ExtHostOutputService2 } from 'vs/workbench/api/node/extHostOutputService';
+import { ExtHostOutputService } from 'vs/workbench/api/node/extHostOutputService';
 import { ExtHostTerminalService } from 'vs/workbench/api/node/extHostTerminalService';
 import { ExtHostTask } from 'vs/workbench/api/node/extHostTask';
 import { ExtHostDebugService } from 'vs/workbench/api/node/extHostDebugService';
@@ -34,7 +34,7 @@ registerSingleton(ILogService, ExtHostLogService);
 registerSingleton(IExtensionStoragePaths, ExtensionStoragePaths);
 
 registerSingleton(IExtHostDebugService, ExtHostDebugService);
-registerSingleton(IExtHostOutputService, ExtHostOutputService2);
+registerSingleton(IExtHostOutputService, ExtHostOutputService);
 registerSingleton(IExtHostSearch, NativeExtHostSearch);
 registerSingleton(IExtHostTask, ExtHostTask);
 registerSingleton(IExtHostTerminalService, ExtHostTerminalService);

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

@@ -9,7 +9,7 @@ import { URI } from 'vs/base/common/uri';
 import { join } from 'vs/base/common/path';
 import { toLocalISOString } from 'vs/base/common/date';
 import { Promises, SymlinkSupport } from 'vs/base/node/pfs';
-import { AbstractExtHostOutputChannel, ExtHostPushOutputChannel, ExtHostOutputService, LazyOutputChannel } from 'vs/workbench/api/common/extHostOutput';
+import { AbstractExtHostOutputChannel, ExtHostOutputService as BaseExtHostOutputService } from 'vs/workbench/api/common/extHostOutput';
 import { IExtHostInitDataService } from 'vs/workbench/api/common/extHostInitDataService';
 import { IExtHostRpcService } from 'vs/workbench/api/common/extHostRpcService';
 import { ILogService } from 'vs/platform/log/common/log';
@@ -17,6 +17,8 @@ import { createRotatingLogger } from 'vs/platform/log/node/spdlogLog';
 import { Logger } from 'spdlog';
 import { ByteSize } from 'vs/platform/files/common/files';
 import { IExtensionDescription } from 'vs/platform/extensions/common/extensions';
+import { OutputChannelUpdateMode } from 'vs/workbench/contrib/output/common/output';
+import { VSBuffer } from 'vs/base/common/buffer';
 
 class OutputAppender {
 
@@ -41,23 +43,34 @@ class OutputAppender {
 
 class ExtHostOutputChannelBackedByFile extends AbstractExtHostOutputChannel {
 
-	private _appender: OutputAppender;
+	private _offset: number;
+	private readonly _appender: OutputAppender;
 
-	constructor(name: string, appender: OutputAppender, extension: IExtensionDescription, proxy: MainThreadOutputServiceShape) {
-		super(name, false, URI.file(appender.file), extension, proxy);
+	constructor(name: string, appender: OutputAppender, extensionId: string, proxy: MainThreadOutputServiceShape) {
+		super(name, false, URI.file(appender.file), extensionId, proxy);
+		this._offset = 0;
 		this._appender = appender;
 	}
 
-	override append(value: string): void {
-		super.append(value);
+	append(value: string): void {
+		this.incrementOffset(value);
 		this._appender.append(value);
 		if (this.visible) {
-			this.update();
+			this._appender.flush();
+			this._id.then(id => this._proxy.$update(id, OutputChannelUpdateMode.Append));
 		}
 	}
 
-	override replaceAll(value: string): void {
-		super.replaceAll(value, true);
+	clear(): void {
+		const till = this._offset;
+		this._appender.flush();
+		this._id.then(id => this._proxy.$update(id, OutputChannelUpdateMode.Clear, till));
+	}
+
+	replace(value: string): void {
+		const till = this._offset;
+		this.incrementOffset(value);
+		this._id.then(id => this._proxy.$update(id, OutputChannelUpdateMode.Replace, till));
 		this._appender.append(value);
 		if (this.visible) {
 			this._appender.flush();
@@ -69,24 +82,16 @@ class ExtHostOutputChannelBackedByFile extends AbstractExtHostOutputChannel {
 		super.show(columnOrPreserveFocus, preserveFocus);
 	}
 
-	override clear(): void {
-		this._appender.flush();
-		super.clear();
+	private incrementOffset(value: string) {
+		this._offset += VSBuffer.fromString(value).byteLength;
 	}
 
-	private update(): void {
-		this._appender.flush();
-		this._id.then(id => this._proxy.$update(id));
-	}
 }
 
-export class ExtHostOutputService2 extends ExtHostOutputService {
+export class ExtHostOutputService extends BaseExtHostOutputService {
 
 	private _logsLocation: URI;
 	private _namePool: number = 1;
-	private readonly _channels: Map<string, AbstractExtHostOutputChannel> = new Map<string, AbstractExtHostOutputChannel>();
-
-	private visibleChannelId: string | null = null;
 
 	constructor(
 		@IExtHostRpcService extHostRpc: IExtHostRpcService,
@@ -97,27 +102,7 @@ export class ExtHostOutputService2 extends ExtHostOutputService {
 		this._logsLocation = initData.logsLocation;
 	}
 
-	override $setVisibleChannel(visibleChannelId: string | null): void {
-		this.visibleChannelId = visibleChannelId;
-		for (const [id, channel] of this._channels) {
-			channel.visible = id === this.visibleChannelId;
-		}
-	}
-
-	override createOutputChannel(name: string, extension: IExtensionDescription): vscode.OutputChannel {
-		name = name.trim();
-		if (!name) {
-			throw new Error('illegal argument `name`. must not be falsy');
-		}
-		const extHostOutputChannel = this._doCreateOutChannel(name, extension);
-		extHostOutputChannel.then(channel => channel._id.then(id => {
-			this._channels.set(id, channel);
-			channel.visible = id === this.visibleChannelId;
-		}));
-		return new LazyOutputChannel(name, extHostOutputChannel);
-	}
-
-	private async _doCreateOutChannel(name: string, extension: IExtensionDescription): Promise<AbstractExtHostOutputChannel> {
+	protected override async doCreateOutChannel(name: string, extension: IExtensionDescription): Promise<AbstractExtHostOutputChannel> {
 		try {
 			const outputDirPath = join(this._logsLocation.fsPath, `output_logging_${toLocalISOString(new Date()).replace(/-|:|\.\d+Z$/g, '')}`);
 			const exists = await SymlinkSupport.existsDirectory(outputDirPath);
@@ -127,11 +112,11 @@ export class ExtHostOutputService2 extends ExtHostOutputService {
 			const fileName = `${this._namePool++}-${name.replace(/[\\/:\*\?"<>\|]/g, '')}`;
 			const file = URI.file(join(outputDirPath, `${fileName}.log`));
 			const appender = await OutputAppender.create(fileName, file.fsPath);
-			return new ExtHostOutputChannelBackedByFile(name, appender, extension, this._proxy);
+			return new ExtHostOutputChannelBackedByFile(name, appender, extension.identifier.value, this._proxy);
 		} catch (error) {
 			// Do not crash if logger cannot be created
 			this.logService.error(error);
-			return new ExtHostPushOutputChannel(name, extension, this._proxy);
 		}
+		return super.doCreateOutChannel(name, extension);
 	}
 }