Address review comments #2

- Ensure LogOutputview is validated before logging or appending content - Simplify processing of error objects - Simplify formatting of arguments in log messages - Update Changelog Contributed on behalf of STMicroelectronics. Signed-off-by: Nina Doschek <ndoschek@eclipsesource.com>
eclipse-theia · Apr 21, 2023 · c930230 · c930230
1 parent 6ce5618
commit c930230
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 54 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,12 +6,13 @@
 
 ## v1.37.0 -
 
-- [plugin] implemented the VS Code `LogOutputChannel` API [#12017](https://github.com/eclipse-theia/theia/pull/12429)
+- [plugin] implemented the VS Code `LogOutputChannel` API [#12017](https://github.com/eclipse-theia/theia/pull/12429) - Contributed on behalf of STMicroelectronics
 
 <a name="breaking_changes_1.37.0">[Breaking Changes:](#breaking_changes_1.37.0)</a>
 - [core] Inject core preference into `DockPanelRenderer` constructor [12360](https://github.com/eclipse-theia/theia/pull/12360)
 - [core] Introduced `ScrollableTabBar.updateTabs()` to fully render tabs [12360](https://github.com/eclipse-theia/theia/pull/12360)
-- [plugin] removed enum `LogLevel` and namespace `env` from `plugin/src/theia-proposed.d.ts` [#12017](https://github.com/eclipse-theia/theia/pull/12429)
+- [plugin] `plugin/src/theia-proposed.d.ts`: removed enum `LogLevel` and namespace `env` [#12017](https://github.com/eclipse-theia/theia/pull/12429)
+- [plugin-ext] `output-channel-item.ts`: changed visibility from `private` to `protected` for member `proxy` and function `validate()` [#12017](https://github.com/eclipse-theia/theia/pull/12429)
 
 ## v1.36.0 0 - 03/30/2023
 

diff --git a/packages/plugin-ext/src/plugin/output-channel-registry.ts b/packages/plugin-ext/src/plugin/output-channel-registry.ts
@@ -23,7 +23,7 @@ import { OutputChannelImpl } from './output-channel/output-channel-item';
 
 export class OutputChannelRegistryExtImpl implements OutputChannelRegistryExt {
 
-    private proxy: OutputChannelRegistryMain;
+    proxy: OutputChannelRegistryMain;
 
     constructor(rpc: RPCProtocol) {
         this.proxy = rpc.getProxy(Ext.OUTPUT_CHANNEL_REGISTRY_MAIN);

diff --git a/packages/plugin-ext/src/plugin/output-channel/log-output-channel.ts b/packages/plugin-ext/src/plugin/output-channel/log-output-channel.ts
@@ -21,6 +21,7 @@ import * as theia from '@theia/plugin';
 import { OutputChannelRegistryMain, PluginInfo } from '../../common/plugin-api-rpc';
 import { OutputChannelImpl } from './output-channel-item';
 import { LogLevel } from '../types-impl';
+import { isArray, isObject } from '@theia/core';
 
 export class LogOutputChannelImpl extends OutputChannelImpl implements theia.LogOutputChannel {
 
@@ -45,88 +46,63 @@ export class LogOutputChannelImpl extends OutputChannelImpl implements theia.Log
     }
 
     override append(value: string): void {
+        super.validate();
         this.info(value);
     }
 
     override appendLine(value: string): void {
+        super.validate();
         this.append(value + '\n');
     }
 
-    protected log(level: theia.LogLevel, message: string): void {
-        const now = new Date();
-        const eol = message.endsWith('\n') ? '' : '\n';
-        const logMessage = `${now.toISOString()} [${LogLevel[level]}] ${message}${eol}`;
-        this.proxy.$append(this.name, logMessage, this.pluginInfo);
-    }
-
     override dispose(): void {
         super.dispose();
         this.onDidChangeLogLevelEmitter.dispose();
     }
 
-    // begin
-    // copied from vscode: https://github.com/Microsoft/vscode/blob/main/src/vs/platform/log/common/log.ts
-    /*---------------------------------------------------------------------------------------------
-     *  Copyright (c) Microsoft Corporation. All rights reserved.
-     *  Licensed under the MIT License. See License.txt in the project root for license information.
-     *--------------------------------------------------------------------------------------------*/
+    protected log(level: theia.LogLevel, message: string): void {
+        super.validate();
+        if (this.checkLogLevel(level)) {
+            const now = new Date();
+            const eol = message.endsWith('\n') ? '' : '\n';
+            const logMessage = `${now.toISOString()} [${LogLevel[level]}] ${message}${eol}`;
+            this.proxy.$append(this.name, logMessage, this.pluginInfo);
+        }
+    }
+
     private checkLogLevel(level: theia.LogLevel): boolean {
         return this.logLevel <= level;
     }
 
     trace(message: string, ...args: any[]): void {
-        if (this.checkLogLevel(LogLevel.Trace)) {
-            this.log(LogLevel.Trace, this.format([message, ...args]));
-        }
+        this.log(LogLevel.Trace, this.format(message, args));
     }
 
     debug(message: string, ...args: any[]): void {
-        if (this.checkLogLevel(LogLevel.Debug)) {
-            this.log(LogLevel.Debug, this.format([message, ...args]));
-        }
+        this.log(LogLevel.Debug, this.format(message, args));
     }
 
     info(message: string, ...args: any[]): void {
-        if (this.checkLogLevel(LogLevel.Info)) {
-            this.log(LogLevel.Info, this.format([message, ...args]));
-        }
+        this.log(LogLevel.Info, this.format(message, args));
     }
 
     warn(message: string, ...args: any[]): void {
-        if (this.checkLogLevel(LogLevel.Warning)) {
-            this.log(LogLevel.Warning, this.format([message, ...args]));
-        }
+        this.log(LogLevel.Warning, this.format(message, args));
     }
 
-    error(message: string | Error, ...args: any[]): void {
-        if (this.checkLogLevel(LogLevel.Error)) {
-            if (message instanceof Error) {
-                const array = Array.prototype.slice.call(arguments) as unknown[];
-                array[0] = message.stack;
-                this.log(LogLevel.Error, this.format(array));
-            } else {
-                this.log(LogLevel.Error, this.format([message, ...args]));
-            }
+    error(errorMsg: string | Error, ...args: any[]): void {
+        if (errorMsg instanceof Error) {
+            this.log(LogLevel.Error, this.format(errorMsg.stack || errorMsg.message, args));
+        } else {
+            this.log(LogLevel.Error, this.format(errorMsg, args));
         }
     }
 
-    private format(args: any): string {
-        let result = '';
-
-        for (let i = 0; i < args.length; i++) {
-            let a = args[i];
-
-            if (typeof a === 'object') {
-                try {
-                    a = JSON.stringify(a);
-                } catch (e) { }
-            }
-
-            result += (i > 0 ? ' ' : '') + a;
+    private format(message: string, args: any[]): string {
+        if (args.length > 0) {
+            return `${message} ${args.map((arg: any) => isObject(arg) || isArray(arg) ? JSON.stringify(arg) : arg).join(' ')}`;
         }
-
-        return result;
+        return message;
     }
-    // end
 
 }
diff --git a/packages/plugin-ext/src/plugin/output-channel/output-channel-item.ts b/packages/plugin-ext/src/plugin/output-channel/output-channel-item.ts
@@ -65,7 +65,7 @@ export class OutputChannelImpl implements theia.OutputChannel {
         this.proxy.$close(this.name);
     }
 
-    private validate(): void {
+    protected validate(): void {
         if (this.disposed) {
             throw new Error('Channel has been closed');
         }