Skip to content

Commit

Permalink
pw_ide: Decouple VS Code logger from pure logic
Browse files Browse the repository at this point in the history
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 🤖 <[email protected]>
Reviewed-by: Alexei Frolov <[email protected]>
Commit-Queue: Chad Norvell <[email protected]>
Docs-Not-Needed: Chad Norvell <[email protected]>
Presubmit-Verified: CQ Bot Account <[email protected]>
  • Loading branch information
chadnorvell authored and CQ Bot Account committed Dec 19, 2024
1 parent ef5c858 commit 89395f2
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 5 deletions.
2 changes: 1 addition & 1 deletion pw_ide/ts/pigweed-vscode/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
6 changes: 6 additions & 0 deletions pw_ide/ts/pigweed-vscode/src/logging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
Expand Down
21 changes: 17 additions & 4 deletions pw_ide/ts/pigweed-vscode/src/refreshManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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;

Expand All @@ -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;

Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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');
}
}

Expand Down

0 comments on commit 89395f2

Please sign in to comment.