From 89395f258edf8079eae61778d7b563f491dfc25d Mon Sep 17 00:00:00 2001
From: Chad Norvell <chadnorvell@google.com>
Date: Thu, 19 Dec 2024 09:21:27 -0800
Subject: [PATCH] pw_ide: Decouple VS Code logger from pure logic
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We can't import `vscode` into places where it's unavailable, even
transitively via the logger (since the log drain is the VS Code output
window). So we need an interface in between that lets code run outside
of VS Code. In particular, unit tests do not have access to `vscode`.

Change-Id: Ia528b22affc74641ef70442bb0a91b6c490dd884
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/255712
Lint: Lint 🤖 <android-build-ayeaye@system.gserviceaccount.com>
Reviewed-by: Alexei Frolov <frolv@google.com>
Commit-Queue: Chad Norvell <chadnorvell@google.com>
Docs-Not-Needed: Chad Norvell <chadnorvell@google.com>
Presubmit-Verified: CQ Bot Account <pigweed-scoped@luci-project-accounts.iam.gserviceaccount.com>
---
 pw_ide/ts/pigweed-vscode/src/extension.ts     |  2 +-
 pw_ide/ts/pigweed-vscode/src/logging.ts       |  6 ++++++
 .../ts/pigweed-vscode/src/refreshManager.ts   | 21 +++++++++++++++----
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/pw_ide/ts/pigweed-vscode/src/extension.ts b/pw_ide/ts/pigweed-vscode/src/extension.ts
index 0c4df2563a..24ab11d66d 100644
--- a/pw_ide/ts/pigweed-vscode/src/extension.ts
+++ b/pw_ide/ts/pigweed-vscode/src/extension.ts
@@ -169,7 +169,7 @@ async function initAsBazelProject(context: vscode.ExtensionContext) {
   }
 
   // Marshall all of our components and dependencies.
-  const refreshManager = disposer.add(RefreshManager.create());
+  const refreshManager = disposer.add(RefreshManager.create({ logger }));
   linkRefreshManagerToEvents(refreshManager);
 
   const { clangdActiveFilesCache, compileCommandsWatcher } = disposer.addMany({
diff --git a/pw_ide/ts/pigweed-vscode/src/logging.ts b/pw_ide/ts/pigweed-vscode/src/logging.ts
index cbf04ff2f4..ee679a7bd4 100644
--- a/pw_ide/ts/pigweed-vscode/src/logging.ts
+++ b/pw_ide/ts/pigweed-vscode/src/logging.ts
@@ -14,6 +14,12 @@
 
 import * as vscode from 'vscode';
 
+export interface Logger {
+  info: (msg: string) => void;
+  warn: (msg: string) => void;
+  error: (msg: string) => void;
+}
+
 export const output = vscode.window.createOutputChannel('Pigweed', {
   log: true,
 });
diff --git a/pw_ide/ts/pigweed-vscode/src/refreshManager.ts b/pw_ide/ts/pigweed-vscode/src/refreshManager.ts
index fadfe8e3fd..5627c71eb9 100644
--- a/pw_ide/ts/pigweed-vscode/src/refreshManager.ts
+++ b/pw_ide/ts/pigweed-vscode/src/refreshManager.ts
@@ -40,7 +40,7 @@
  * in-progress refresh process.
  */
 
-import logger from './logging';
+import type { Logger } from './logging';
 
 /** Refresh statuses that broadly represent a refresh in progress. */
 export type RefreshStatusInProgress =
@@ -84,6 +84,9 @@ type NextState<State extends RefreshStatus> =
 
 /** Options for constructing a refresh manager. */
 interface RefreshManagerOptions {
+  /** A concrete logger instance to log to. */
+  logger?: Logger;
+
   /** The timeout period (in ms) between handling the reset signal. */
   refreshSignalHandlerTimeout: number;
 
@@ -101,6 +104,12 @@ const defaultRefreshManagerOptions: RefreshManagerOptions = {
  * be called at each status transition, and provide triggers for status change.
  */
 export class RefreshManager<State extends RefreshStatus> {
+  private _logger: Logger = {
+    info: (msg: string) => console.log(msg),
+    warn: (msg: string) => console.warn(msg),
+    error: (msg: string) => console.error(msg),
+  };
+
   /** The current refresh status. */
   private _state: RefreshStatus;
 
@@ -129,6 +138,10 @@ export class RefreshManager<State extends RefreshStatus> {
   constructor(state: RefreshStatus, options: RefreshManagerOptions) {
     this._state = state;
 
+    if (options.logger) {
+      this._logger = options.logger;
+    }
+
     if (options.useRefreshSignalHandler) {
       this.refreshSignalHandlerInterval = setInterval(
         () => this.handleRefreshSignal(),
@@ -268,14 +281,14 @@ export class RefreshManager<State extends RefreshStatus> {
         // hoisted to here by `throw`ing them, we will also catch unexpected or
         // not-well-behaved errors as exceptions (`{ message: '...' }`-style).
         if (typeof err === 'string') {
-          logger.error(err);
+          this._logger.error(err);
         } else {
           const { message } = err as { message: string | undefined };
 
           if (message) {
-            logger.error(message);
+            this._logger.error(message);
           } else {
-            logger.error('Unknown error occurred');
+            this._logger.error('Unknown error occurred');
           }
         }