CHANGE: @W-18088706@: Improve readability of error and warning events in terminal (#1876)

Author: stephen-carter-at-sf

src/lib/listeners/LogEventListener.ts

@@ -2,6 +2,7 @@ import {CodeAnalyzer, EngineLogEvent, EventType, LogEvent, LogLevel} from '@sale
 import {Display} from '../Display';
 import {LogWriter} from '../writers/LogWriter';
 import {BundleName, getMessage} from "../messages";
+import {indent, makeGrey} from '../utils/StylingUtil';
 
 export interface LogEventListener {
 	listen(codeAnalyzer: CodeAnalyzer): void;
@@ -17,8 +18,8 @@ export class LogEventDisplayer implements LogEventListener {
 
 	public listen(codeAnalyzer: CodeAnalyzer): void {
 		// Set up listeners
-		codeAnalyzer.onEvent(EventType.LogEvent, (e: LogEvent) => this.handleEvent('Core', e));
-		codeAnalyzer.onEvent(EventType.EngineLogEvent, (e: EngineLogEvent) => this.handleEvent(e.engineName, e));
+		codeAnalyzer.onEvent(EventType.LogEvent, (e: LogEvent) => this.handleEvent('Code Analyzer', e));
+		codeAnalyzer.onEvent(EventType.EngineLogEvent, (e: EngineLogEvent) => this.handleEvent(`Engine '${e.engineName}'`, e));
 	}
 
 	public stopListening(): void {
@@ -31,16 +32,25 @@ export class LogEventDisplayer implements LogEventListener {
 		if (event.logLevel > LogLevel.Info) {
 			return;
 		}
-		const formattedMessage = `${source} [${formatTimestamp(event.timestamp)}]: ${event.message}`;
+		const decoratedTimestamp = makeGrey(`[${formatTimestamp(event.timestamp)}]`);
+		const formattedMessage = `${source} ${decoratedTimestamp}:\n${indent(event.message)}`;
 		switch (event.logLevel) {
 			case LogLevel.Error:
 				this.display.displayError(formattedMessage);
+				// Adds a newline outside of the error formatting to make errors easy to read. That is, we do not want
+				// to add a \n inside of the above displayError or use displayError('') here because it always adds in a
+				// red "">" character.
+				this.display.displayInfo('');
 				return;
 			case LogLevel.Warn:
 				this.display.displayWarning(formattedMessage);
+				// Likewise, we want spacing here, but don't want to add in displayWarning('') because it adds back in
+				// the word "Warning". So it's best to just use displayInfo('') to add in a blank line.
+				this.display.displayInfo('');
 				return;
 			case LogLevel.Info:
 				this.display.displayInfo(formattedMessage);
+				this.display.displayInfo('');
 				return;
 		}
 	}
@@ -72,5 +82,5 @@ export class LogEventLogger implements LogEventListener {
 }
 
 function formatTimestamp(timestamp: Date): string {
-	return `${timestamp.getHours()}:${timestamp.getMinutes()}:${timestamp.getSeconds()}`;
+	return `${timestamp.getHours()}:${timestamp.getMinutes()}:${timestamp.getSeconds()}.${timestamp.getMilliseconds()}`;
 }

test/lib/actions/ConfigAction.test.ts

@@ -625,10 +625,9 @@ describe('ConfigAction tests', () => {
 
 		// ==== OUTPUT PROCESSING ====
 		const displayEvents = spyDisplay.getDisplayEvents();
-		if (displayEvents[4].type === DisplayEventType.LOG) {
-			return ansis.strip(displayEvents[4].data);
-		} else if (displayEvents[5].type === DisplayEventType.LOG) {
-			return ansis.strip(displayEvents[5].data);
+		const displayedConfigEventArray = displayEvents.filter(e => e.data.includes("CODE ANALYZER CONFIGURATION"));
+		if (displayedConfigEventArray.length === 1) {
+			return ansis.strip(displayedConfigEventArray[0].data);
 		} else {
 			return 'Could Not Get Specific Output';
 		}

test/lib/listeners/LogEventListener.test.ts

@@ -51,10 +51,11 @@ describe('LogEventListener implementations', () => {
 
 			// ==== ASSERTIONS ====
 			const displayEvents = spyDisplay.getDisplayEvents();
-			expect(displayEvents).toHaveLength(3);
+			expect(displayEvents).toHaveLength(6);
 			for (let i = 0; i < 3; i++) {
-				expect(displayEvents[i].type).toEqual(displayEvent);
-				expect(displayEvents[i].data).toContain(expectedMessages[i]);
+				expect(displayEvents[i*2].type).toEqual(displayEvent);
+				expect(displayEvents[i*2].data).toContain(expectedMessages[i]);
+				expect(displayEvents[i*2+1]).toEqual({type: DisplayEventType.INFO, data: ""}); // We display an empty line after each message
 			}
 		});