api feedback

extensions/vscode-api-tests/src/utils.ts

@@ -69,7 +69,7 @@ export function delay(ms: number) {
 export function withLogDisabled(runnable: () => Promise<any>): () => Promise<void> {
 	return async (): Promise<void> => {
 		const logLevel = await vscode.commands.executeCommand('_extensionTests.getLogLevel');
-		await vscode.commands.executeCommand('_extensionTests.setLogLevel', 6 /* critical */);
+		await vscode.commands.executeCommand('_extensionTests.setLogLevel', 'off');
 
 		try {
 			await runnable();

src/vs/platform/log/browser/log.ts

@@ -17,7 +17,6 @@ function logLevelToString(level: LogLevel): string {
 		case LogLevel.Info: return 'info';
 		case LogLevel.Warning: return 'warn';
 		case LogLevel.Error: return 'error';
-		case LogLevel.Critical: return 'error';
 	}
 	return 'info';
 }

src/vs/platform/log/common/bufferLog.ts

@@ -17,7 +17,6 @@ function getLogFunction(logger: ILogger, level: LogLevel): Function {
 		case LogLevel.Info: return logger.info;
 		case LogLevel.Warning: return logger.warn;
 		case LogLevel.Error: return logger.error;
-		case LogLevel.Critical: return logger.critical;
 		default: throw new Error('Invalid log level');
 	}
 }
@@ -76,10 +75,6 @@ export class BufferLogService extends AbstractLogger implements ILogService {
 		this._log(LogLevel.Error, message, ...args);
 	}
 
-	critical(message: string | Error, ...args: any[]): void {
-		this._log(LogLevel.Critical, message, ...args);
-	}
-
 	override dispose(): void {
 		this._logger?.dispose();
 	}

src/vs/platform/log/common/fileLog.ts

@@ -70,12 +70,6 @@ export class FileLogger extends AbstractLogger implements ILogger {
 		}
 	}
 
-	critical(): void {
-		if (this.getLevel() <= LogLevel.Critical) {
-			this._log(LogLevel.Critical, format(arguments));
-		}
-	}
-
 	flush(): void {
 	}
 
@@ -129,7 +123,6 @@ export class FileLogger extends AbstractLogger implements ILogger {
 
 	private stringifyLogLevel(level: LogLevel): string {
 		switch (level) {
-			case LogLevel.Critical: return 'critical';
 			case LogLevel.Debug: return 'debug';
 			case LogLevel.Error: return 'error';
 			case LogLevel.Info: return 'info';

src/vs/platform/log/common/log.ts

@@ -20,13 +20,12 @@ function now(): string {
 }
 
 export enum LogLevel {
+	Off,
 	Trace,
 	Debug,
 	Info,
 	Warning,
-	Error,
-	Critical,
-	Off
+	Error
 }
 
 export const DEFAULT_LOG_LEVEL: LogLevel = LogLevel.Info;
@@ -41,7 +40,6 @@ export interface ILogger extends IDisposable {
 	info(message: string, ...args: any[]): void;
 	warn(message: string, ...args: any[]): void;
 	error(message: string | Error, ...args: any[]): void;
-	critical(message: string | Error, ...args: any[]): void;
 
 	/**
 	 * An operation to flush the contents. Can be synchronous.
@@ -56,7 +54,6 @@ export function log(logger: ILogger, level: LogLevel, message: string): void {
 		case LogLevel.Info: logger.info(message); break;
 		case LogLevel.Warning: logger.warn(message); break;
 		case LogLevel.Error: logger.error(message); break;
-		case LogLevel.Critical: logger.critical(message); break;
 		default: throw new Error('Invalid log level');
 	}
 }
@@ -198,12 +195,6 @@ export abstract class AbstractMessageLogger extends AbstractLogger implements IL
 		}
 	}
 
-	critical(message: string | Error, ...args: any[]): void {
-		if (this.checkLogLevel(LogLevel.Critical)) {
-			this.log(LogLevel.Critical, format([message, ...args]));
-		}
-	}
-
 	flush(): void { }
 }
 
@@ -268,16 +259,6 @@ export class ConsoleMainLogger extends AbstractLogger implements ILogger {
 		}
 	}
 
-	critical(message: string, ...args: any[]): void {
-		if (this.getLevel() <= LogLevel.Critical) {
-			if (this.useColors) {
-				console.error(`\x1b[90m[main ${now()}]\x1b[0m`, message, ...args);
-			} else {
-				console.error(`[main ${now()}]`, message, ...args);
-			}
-		}
-	}
-
 	override dispose(): void {
 		// noop
 	}
@@ -325,12 +306,6 @@ export class ConsoleLogger extends AbstractLogger implements ILogger {
 		}
 	}
 
-	critical(message: string, ...args: any[]): void {
-		if (this.getLevel() <= LogLevel.Critical) {
-			console.log('%cCRITI', 'background: #f33; color: white', message, ...args);
-		}
-	}
-
 	override dispose(): void {
 		// noop
 	}
@@ -377,12 +352,6 @@ export class AdapterLogger extends AbstractLogger implements ILogger {
 		}
 	}
 
-	critical(message: string | Error, ...args: any[]): void {
-		if (this.getLevel() <= LogLevel.Critical) {
-			this.adapter.log(LogLevel.Critical, [this.extractMessage(message), ...args]);
-		}
-	}
-
 	private extractMessage(msg: string | Error): string {
 		if (typeof msg === 'string') {
 			return msg;
@@ -447,12 +416,6 @@ export class MultiplexLogService extends AbstractLogger implements ILogService {
 		}
 	}
 
-	critical(message: string | Error, ...args: any[]): void {
-		for (const logService of this.logServices) {
-			logService.critical(message, ...args);
-		}
-	}
-
 	flush(): void {
 		for (const logService of this.logServices) {
 			logService.flush();
@@ -506,10 +469,6 @@ export class LogService extends Disposable implements ILogService {
 		this.logger.error(message, ...args);
 	}
 
-	critical(message: string | Error, ...args: any[]): void {
-		this.logger.critical(message, ...args);
-	}
-
 	flush(): void {
 		this.logger.flush();
 	}
@@ -629,6 +588,17 @@ export function getLogLevel(environmentService: IEnvironmentService): LogLevel {
 	return DEFAULT_LOG_LEVEL;
 }
 
+export function LogLevelToString(logLevel: LogLevel): string {
+	switch (logLevel) {
+		case LogLevel.Trace: return 'trace';
+		case LogLevel.Debug: return 'debug';
+		case LogLevel.Info: return 'info';
+		case LogLevel.Warning: return 'warn';
+		case LogLevel.Error: return 'error';
+		case LogLevel.Off: return 'off';
+	}
+}
+
 export function parseLogLevel(logLevel: string): LogLevel | undefined {
 	switch (logLevel) {
 		case 'trace':
@@ -642,21 +612,9 @@ export function parseLogLevel(logLevel: string): LogLevel | undefined {
 		case 'error':
 			return LogLevel.Error;
 		case 'critical':
-			return LogLevel.Critical;
+			return LogLevel.Error;
 		case 'off':
 			return LogLevel.Off;
 	}
 	return undefined;
 }
-
-export function LogLevelToString(logLevel: LogLevel): string {
-	switch (logLevel) {
-		case LogLevel.Trace: return 'trace';
-		case LogLevel.Debug: return 'debug';
-		case LogLevel.Info: return 'info';
-		case LogLevel.Warning: return 'warn';
-		case LogLevel.Error: return 'error';
-		case LogLevel.Critical: return 'critical';
-		case LogLevel.Off: return 'off';
-	}
-}

src/vs/platform/log/node/spdlogLog.ts

@@ -7,6 +7,16 @@ import * as spdlog from 'spdlog';
 import { ByteSize } from 'vs/platform/files/common/files';
 import { AbstractMessageLogger, ILogger, LogLevel } from 'vs/platform/log/common/log';
 
+enum SpdLogLevel {
+	Trace,
+	Debug,
+	Info,
+	Warning,
+	Error,
+	Critical,
+	Off
+}
+
 async function createSpdLogLogger(name: string, logfilePath: string, filesize: number, filecount: number, donotUseFormatters: boolean): Promise<spdlog.Logger | null> {
 	// Do not crash if spdlog cannot be loaded
 	try {
@@ -37,7 +47,18 @@ function log(logger: spdlog.Logger, level: LogLevel, message: string): void {
 		case LogLevel.Info: logger.info(message); break;
 		case LogLevel.Warning: logger.warn(message); break;
 		case LogLevel.Error: logger.error(message); break;
-		case LogLevel.Critical: logger.critical(message); break;
+		default: throw new Error('Invalid log level');
+	}
+}
+
+function setLogLevel(logger: spdlog.Logger, level: LogLevel): void {
+	switch (level) {
+		case LogLevel.Trace: logger.setLevel(SpdLogLevel.Trace); break;
+		case LogLevel.Debug: logger.setLevel(SpdLogLevel.Debug); break;
+		case LogLevel.Info: logger.setLevel(SpdLogLevel.Info); break;
+		case LogLevel.Warning: logger.setLevel(SpdLogLevel.Warning); break;
+		case LogLevel.Error: logger.setLevel(SpdLogLevel.Error); break;
+		case LogLevel.Off: logger.setLevel(SpdLogLevel.Off); break;
 		default: throw new Error('Invalid log level');
 	}
 }
@@ -59,7 +80,9 @@ export class SpdLogLogger extends AbstractMessageLogger implements ILogger {
 		this.setLevel(level);
 		this._loggerCreationPromise = this._createSpdLogLogger(name, filepath, rotating, donotUseFormatters);
 		this._register(this.onDidChangeLogLevel(level => {
-			this._logger?.setLevel(level);
+			if (this._logger) {
+				setLogLevel(this._logger, level);
+			}
 		}));
 	}
 
@@ -69,7 +92,7 @@ export class SpdLogLogger extends AbstractMessageLogger implements ILogger {
 		const logger = await createSpdLogLogger(name, filepath, filesize, filecount, donotUseFormatters);
 		if (logger) {
 			this._logger = logger;
-			this._logger.setLevel(this.getLevel());
+			setLogLevel(this._logger, this.getLevel());
 			for (const { level, message } of this.buffer) {
 				log(this._logger, level, message);
 			}

src/vs/platform/telemetry/test/common/telemetryLogAppender.test.ts

@@ -48,12 +48,6 @@ class TestTelemetryLogger extends AbstractLogger implements ILogger {
 		}
 	}
 
-	critical(message: string, ...args: any[]): void {
-		if (this.getLevel() <= LogLevel.Critical) {
-			this.logs.push(message);
-		}
-	}
-
 	override dispose(): void { }
 	flush(): void { }
 }

src/vs/platform/userDataSync/common/userDataSyncLog.ts

@@ -40,10 +40,6 @@ export class UserDataSyncLogService extends AbstractLogger implements IUserDataS
 		this.logger.error(message, ...args);
 	}
 
-	critical(message: string | Error, ...args: any[]): void {
-		this.logger.critical(message, ...args);
-	}
-
 	flush(): void {
 		this.logger.flush();
 	}

src/vs/server/node/serverServices.ts

@@ -315,16 +315,6 @@ class ServerLogService extends AbstractLogger implements ILogService {
 		}
 	}
 
-	critical(message: string, ...args: any[]): void {
-		if (this.getLevel() <= LogLevel.Critical) {
-			if (this.useColors) {
-				console.error(`\x1b[90m[${now()}]\x1b[0m`, message, ...args);
-			} else {
-				console.error(`[${now()}]`, message, ...args);
-			}
-		}
-	}
-
 	override dispose(): void {
 		// noop
 	}

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

@@ -4,7 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 import { extHostNamedCustomer, IExtHostContext } from 'vs/workbench/services/extensions/common/extHostCustomers';
-import { ILoggerOptions, ILoggerService, ILogService, log, LogLevel } from 'vs/platform/log/common/log';
+import { ILoggerOptions, ILoggerService, ILogService, log, LogLevel, LogLevelToString, parseLogLevel } from 'vs/platform/log/common/log';
 import { DisposableStore } from 'vs/base/common/lifecycle';
 import { ExtHostContext, MainThreadLoggerShape, MainContext } from 'vs/workbench/api/common/extHost.protocol';
 import { UriComponents, URI } from 'vs/base/common/uri';
@@ -59,17 +59,20 @@ export class MainThreadLoggerService implements MainThreadLoggerShape {
 
 // --- Internal commands to improve extension test runs
 
-CommandsRegistry.registerCommand('_extensionTests.setLogLevel', function (accessor: ServicesAccessor, level: number) {
+CommandsRegistry.registerCommand('_extensionTests.setLogLevel', function (accessor: ServicesAccessor, level: string) {
 	const logService = accessor.get(ILogService);
 	const environmentService = accessor.get(IEnvironmentService);
 
 	if (environmentService.isExtensionDevelopment && !!environmentService.extensionTestsLocationURI) {
-		logService.setLevel(level);
+		const logLevel = parseLogLevel(level);
+		if (logLevel !== undefined) {
+			logService.setLevel(logLevel);
+		}
 	}
 });
 
 CommandsRegistry.registerCommand('_extensionTests.getLogLevel', function (accessor: ServicesAccessor) {
 	const logService = accessor.get(ILogService);
 
-	return logService.getLevel();
+	return LogLevelToString(logService.getLevel());
 });

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

@@ -37,7 +37,7 @@ export function connectProxyResolver(
 				case LogLevel.Info: extHostLogService.info(message, ...args); break;
 				case LogLevel.Warning: extHostLogService.warn(message, ...args); break;
 				case LogLevel.Error: extHostLogService.error(message, ...args); break;
-				case LogLevel.Critical: extHostLogService.critical(message, ...args); break;
+				case LogLevel.Critical: extHostLogService.error(message, ...args); break;
 				case LogLevel.Off: break;
 				default: never(level, message, args); break;
 			}

src/vs/workbench/contrib/bulkEdit/browser/bulkFileEdits.ts

@@ -256,7 +256,7 @@ class DeleteOperation implements IFileOperation {
 				try {
 					fileContent = await this._fileService.readFile(edit.oldUri);
 				} catch (err) {
-					this._logService.critical(err);
+					this._logService.error(err);
 				}
 			}
 			if (fileContent !== undefined) {

src/vs/workbench/contrib/editSessions/common/editSessionsLogService.ts

@@ -40,10 +40,6 @@ export class EditSessionsLogService extends AbstractLogger implements IEditSessi
 		this.logger.error(message, ...args);
 	}
 
-	critical(message: string | Error, ...args: any[]): void {
-		this.logger.critical(message, ...args);
-	}
-
 	flush(): void {
 		this.logger.flush();
 	}

src/vs/workbench/contrib/logs/common/logsActions.ts

@@ -78,7 +78,6 @@ export class SetLogLevelAction extends Action {
 			{ label: this.getLabel(nls.localize('info', "Info"), LogLevel.Info, current), level: LogLevel.Info, description: this.getDescription(LogLevel.Info, defaultLogLevel) },
 			{ label: this.getLabel(nls.localize('warn', "Warning"), LogLevel.Warning, current), level: LogLevel.Warning, description: this.getDescription(LogLevel.Warning, defaultLogLevel) },
 			{ label: this.getLabel(nls.localize('err', "Error"), LogLevel.Error, current), level: LogLevel.Error, description: this.getDescription(LogLevel.Error, defaultLogLevel) },
-			{ label: this.getLabel(nls.localize('critical', "Critical"), LogLevel.Critical, current), level: LogLevel.Critical, description: this.getDescription(LogLevel.Critical, defaultLogLevel) },
 			{ label: this.getLabel(nls.localize('off', "Off"), LogLevel.Off, current), level: LogLevel.Off, description: this.getDescription(LogLevel.Off, defaultLogLevel) },
 		];
 

src/vs/workbench/contrib/notebook/common/notebookEditorModelResolverServiceImpl.ts

@@ -123,7 +123,7 @@ class NotebookModelReferenceCollection extends ReferenceCollection<Promise<IReso
 			this._modelListener.delete(model);
 			model.dispose();
 		}).catch(err => {
-			this._logService.critical('FAILED to destory notebook', err);
+			this._logService.error('FAILED to destory notebook', err);
 		});
 	}
 }