Skip to content

Commit fe882ab

Browse files
sandy081rzhao271
sandy081
and
rzhao271
authored
api feedback (microsoft#164470)
* api feedback - remove critical log level - move log level off to be 0 * handle off log level * fix disabling log level in tests Co-authored-by: Raymond Zhao <7199958+rzhao271@users.noreply.github.com>
1 parent b0da2ea commit fe882ab

File tree

18 files changed

+64
-119
lines changed

18 files changed

+64
-119
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ export function delay(ms: number) {
6969
export function withLogDisabled(runnable: () => Promise<any>): () => Promise<void> {
7070
return async (): Promise<void> => {
7171
const logLevel = await vscode.commands.executeCommand('_extensionTests.getLogLevel');
72-
await vscode.commands.executeCommand('_extensionTests.setLogLevel', 6 /* critical */);
72+
await vscode.commands.executeCommand('_extensionTests.setLogLevel', 'off');
7373

7474
try {
7575
await runnable();

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ function logLevelToString(level: LogLevel): string {
1717
case LogLevel.Info: return 'info';
1818
case LogLevel.Warning: return 'warn';
1919
case LogLevel.Error: return 'error';
20-
case LogLevel.Critical: return 'error';
2120
}
2221
return 'info';
2322
}

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ function getLogFunction(logger: ILogger, level: LogLevel): Function {
1717
case LogLevel.Info: return logger.info;
1818
case LogLevel.Warning: return logger.warn;
1919
case LogLevel.Error: return logger.error;
20-
case LogLevel.Critical: return logger.critical;
2120
default: throw new Error('Invalid log level');
2221
}
2322
}
@@ -76,10 +75,6 @@ export class BufferLogService extends AbstractLogger implements ILogService {
7675
this._log(LogLevel.Error, message, ...args);
7776
}
7877

79-
critical(message: string | Error, ...args: any[]): void {
80-
this._log(LogLevel.Critical, message, ...args);
81-
}
82-
8378
override dispose(): void {
8479
this._logger?.dispose();
8580
}

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

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,6 @@ export class FileLogger extends AbstractLogger implements ILogger {
7070
}
7171
}
7272

73-
critical(): void {
74-
if (this.getLevel() <= LogLevel.Critical) {
75-
this._log(LogLevel.Critical, format(arguments));
76-
}
77-
}
78-
7973
flush(): void {
8074
}
8175

@@ -129,7 +123,6 @@ export class FileLogger extends AbstractLogger implements ILogger {
129123

130124
private stringifyLogLevel(level: LogLevel): string {
131125
switch (level) {
132-
case LogLevel.Critical: return 'critical';
133126
case LogLevel.Debug: return 'debug';
134127
case LogLevel.Error: return 'error';
135128
case LogLevel.Info: return 'info';

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

Lines changed: 14 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,12 @@ function now(): string {
2020
}
2121

2222
export enum LogLevel {
23+
Off,
2324
Trace,
2425
Debug,
2526
Info,
2627
Warning,
27-
Error,
28-
Critical,
29-
Off
28+
Error
3029
}
3130

3231
export const DEFAULT_LOG_LEVEL: LogLevel = LogLevel.Info;
@@ -41,7 +40,6 @@ export interface ILogger extends IDisposable {
4140
info(message: string, ...args: any[]): void;
4241
warn(message: string, ...args: any[]): void;
4342
error(message: string | Error, ...args: any[]): void;
44-
critical(message: string | Error, ...args: any[]): void;
4543

4644
/**
4745
* An operation to flush the contents. Can be synchronous.
@@ -56,7 +54,6 @@ export function log(logger: ILogger, level: LogLevel, message: string): void {
5654
case LogLevel.Info: logger.info(message); break;
5755
case LogLevel.Warning: logger.warn(message); break;
5856
case LogLevel.Error: logger.error(message); break;
59-
case LogLevel.Critical: logger.critical(message); break;
6057
default: throw new Error('Invalid log level');
6158
}
6259
}
@@ -198,12 +195,6 @@ export abstract class AbstractMessageLogger extends AbstractLogger implements IL
198195
}
199196
}
200197

201-
critical(message: string | Error, ...args: any[]): void {
202-
if (this.checkLogLevel(LogLevel.Critical)) {
203-
this.log(LogLevel.Critical, format([message, ...args]));
204-
}
205-
}
206-
207198
flush(): void { }
208199
}
209200

@@ -268,16 +259,6 @@ export class ConsoleMainLogger extends AbstractLogger implements ILogger {
268259
}
269260
}
270261

271-
critical(message: string, ...args: any[]): void {
272-
if (this.getLevel() <= LogLevel.Critical) {
273-
if (this.useColors) {
274-
console.error(`\x1b[90m[main ${now()}]\x1b[0m`, message, ...args);
275-
} else {
276-
console.error(`[main ${now()}]`, message, ...args);
277-
}
278-
}
279-
}
280-
281262
override dispose(): void {
282263
// noop
283264
}
@@ -325,12 +306,6 @@ export class ConsoleLogger extends AbstractLogger implements ILogger {
325306
}
326307
}
327308

328-
critical(message: string, ...args: any[]): void {
329-
if (this.getLevel() <= LogLevel.Critical) {
330-
console.log('%cCRITI', 'background: #f33; color: white', message, ...args);
331-
}
332-
}
333-
334309
override dispose(): void {
335310
// noop
336311
}
@@ -377,12 +352,6 @@ export class AdapterLogger extends AbstractLogger implements ILogger {
377352
}
378353
}
379354

380-
critical(message: string | Error, ...args: any[]): void {
381-
if (this.getLevel() <= LogLevel.Critical) {
382-
this.adapter.log(LogLevel.Critical, [this.extractMessage(message), ...args]);
383-
}
384-
}
385-
386355
private extractMessage(msg: string | Error): string {
387356
if (typeof msg === 'string') {
388357
return msg;
@@ -447,12 +416,6 @@ export class MultiplexLogService extends AbstractLogger implements ILogService {
447416
}
448417
}
449418

450-
critical(message: string | Error, ...args: any[]): void {
451-
for (const logService of this.logServices) {
452-
logService.critical(message, ...args);
453-
}
454-
}
455-
456419
flush(): void {
457420
for (const logService of this.logServices) {
458421
logService.flush();
@@ -506,10 +469,6 @@ export class LogService extends Disposable implements ILogService {
506469
this.logger.error(message, ...args);
507470
}
508471

509-
critical(message: string | Error, ...args: any[]): void {
510-
this.logger.critical(message, ...args);
511-
}
512-
513472
flush(): void {
514473
this.logger.flush();
515474
}
@@ -629,6 +588,17 @@ export function getLogLevel(environmentService: IEnvironmentService): LogLevel {
629588
return DEFAULT_LOG_LEVEL;
630589
}
631590

591+
export function LogLevelToString(logLevel: LogLevel): string {
592+
switch (logLevel) {
593+
case LogLevel.Trace: return 'trace';
594+
case LogLevel.Debug: return 'debug';
595+
case LogLevel.Info: return 'info';
596+
case LogLevel.Warning: return 'warn';
597+
case LogLevel.Error: return 'error';
598+
case LogLevel.Off: return 'off';
599+
}
600+
}
601+
632602
export function parseLogLevel(logLevel: string): LogLevel | undefined {
633603
switch (logLevel) {
634604
case 'trace':
@@ -642,21 +612,9 @@ export function parseLogLevel(logLevel: string): LogLevel | undefined {
642612
case 'error':
643613
return LogLevel.Error;
644614
case 'critical':
645-
return LogLevel.Critical;
615+
return LogLevel.Error;
646616
case 'off':
647617
return LogLevel.Off;
648618
}
649619
return undefined;
650620
}
651-
652-
export function LogLevelToString(logLevel: LogLevel): string {
653-
switch (logLevel) {
654-
case LogLevel.Trace: return 'trace';
655-
case LogLevel.Debug: return 'debug';
656-
case LogLevel.Info: return 'info';
657-
case LogLevel.Warning: return 'warn';
658-
case LogLevel.Error: return 'error';
659-
case LogLevel.Critical: return 'critical';
660-
case LogLevel.Off: return 'off';
661-
}
662-
}

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

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,16 @@ import * as spdlog from 'spdlog';
77
import { ByteSize } from 'vs/platform/files/common/files';
88
import { AbstractMessageLogger, ILogger, LogLevel } from 'vs/platform/log/common/log';
99

10+
enum SpdLogLevel {
11+
Trace,
12+
Debug,
13+
Info,
14+
Warning,
15+
Error,
16+
Critical,
17+
Off
18+
}
19+
1020
async function createSpdLogLogger(name: string, logfilePath: string, filesize: number, filecount: number, donotUseFormatters: boolean): Promise<spdlog.Logger | null> {
1121
// Do not crash if spdlog cannot be loaded
1222
try {
@@ -37,7 +47,18 @@ function log(logger: spdlog.Logger, level: LogLevel, message: string): void {
3747
case LogLevel.Info: logger.info(message); break;
3848
case LogLevel.Warning: logger.warn(message); break;
3949
case LogLevel.Error: logger.error(message); break;
40-
case LogLevel.Critical: logger.critical(message); break;
50+
default: throw new Error('Invalid log level');
51+
}
52+
}
53+
54+
function setLogLevel(logger: spdlog.Logger, level: LogLevel): void {
55+
switch (level) {
56+
case LogLevel.Trace: logger.setLevel(SpdLogLevel.Trace); break;
57+
case LogLevel.Debug: logger.setLevel(SpdLogLevel.Debug); break;
58+
case LogLevel.Info: logger.setLevel(SpdLogLevel.Info); break;
59+
case LogLevel.Warning: logger.setLevel(SpdLogLevel.Warning); break;
60+
case LogLevel.Error: logger.setLevel(SpdLogLevel.Error); break;
61+
case LogLevel.Off: logger.setLevel(SpdLogLevel.Off); break;
4162
default: throw new Error('Invalid log level');
4263
}
4364
}
@@ -59,7 +80,9 @@ export class SpdLogLogger extends AbstractMessageLogger implements ILogger {
5980
this.setLevel(level);
6081
this._loggerCreationPromise = this._createSpdLogLogger(name, filepath, rotating, donotUseFormatters);
6182
this._register(this.onDidChangeLogLevel(level => {
62-
this._logger?.setLevel(level);
83+
if (this._logger) {
84+
setLogLevel(this._logger, level);
85+
}
6386
}));
6487
}
6588

@@ -69,7 +92,7 @@ export class SpdLogLogger extends AbstractMessageLogger implements ILogger {
6992
const logger = await createSpdLogLogger(name, filepath, filesize, filecount, donotUseFormatters);
7093
if (logger) {
7194
this._logger = logger;
72-
this._logger.setLevel(this.getLevel());
95+
setLogLevel(this._logger, this.getLevel());
7396
for (const { level, message } of this.buffer) {
7497
log(this._logger, level, message);
7598
}

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,6 @@ class TestTelemetryLogger extends AbstractLogger implements ILogger {
4848
}
4949
}
5050

51-
critical(message: string, ...args: any[]): void {
52-
if (this.getLevel() <= LogLevel.Critical) {
53-
this.logs.push(message);
54-
}
55-
}
56-
5751
override dispose(): void { }
5852
flush(): void { }
5953
}

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,6 @@ export class UserDataSyncLogService extends AbstractLogger implements IUserDataS
4040
this.logger.error(message, ...args);
4141
}
4242

43-
critical(message: string | Error, ...args: any[]): void {
44-
this.logger.critical(message, ...args);
45-
}
46-
4743
flush(): void {
4844
this.logger.flush();
4945
}

‎src/vs/server/node/serverServices.ts‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -315,16 +315,6 @@ class ServerLogService extends AbstractLogger implements ILogService {
315315
}
316316
}
317317

318-
critical(message: string, ...args: any[]): void {
319-
if (this.getLevel() <= LogLevel.Critical) {
320-
if (this.useColors) {
321-
console.error(`\x1b[90m[${now()}]\x1b[0m`, message, ...args);
322-
} else {
323-
console.error(`[${now()}]`, message, ...args);
324-
}
325-
}
326-
}
327-
328318
override dispose(): void {
329319
// noop
330320
}

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { extHostNamedCustomer, IExtHostContext } from 'vs/workbench/services/extensions/common/extHostCustomers';
7-
import { ILoggerOptions, ILoggerService, ILogService, log, LogLevel } from 'vs/platform/log/common/log';
7+
import { ILoggerOptions, ILoggerService, ILogService, log, LogLevel, LogLevelToString, parseLogLevel } from 'vs/platform/log/common/log';
88
import { DisposableStore } from 'vs/base/common/lifecycle';
99
import { ExtHostContext, MainThreadLoggerShape, MainContext } from 'vs/workbench/api/common/extHost.protocol';
1010
import { UriComponents, URI } from 'vs/base/common/uri';
@@ -59,17 +59,20 @@ export class MainThreadLoggerService implements MainThreadLoggerShape {
5959

6060
// --- Internal commands to improve extension test runs
6161

62-
CommandsRegistry.registerCommand('_extensionTests.setLogLevel', function (accessor: ServicesAccessor, level: number) {
62+
CommandsRegistry.registerCommand('_extensionTests.setLogLevel', function (accessor: ServicesAccessor, level: string) {
6363
const logService = accessor.get(ILogService);
6464
const environmentService = accessor.get(IEnvironmentService);
6565

6666
if (environmentService.isExtensionDevelopment && !!environmentService.extensionTestsLocationURI) {
67-
logService.setLevel(level);
67+
const logLevel = parseLogLevel(level);
68+
if (logLevel !== undefined) {
69+
logService.setLevel(logLevel);
70+
}
6871
}
6972
});
7073

7174
CommandsRegistry.registerCommand('_extensionTests.getLogLevel', function (accessor: ServicesAccessor) {
7275
const logService = accessor.get(ILogService);
7376

74-
return logService.getLevel();
77+
return LogLevelToString(logService.getLevel());
7578
});

0 commit comments

Comments
 (0)