Change showAndLog functions to take NotificationLogger

Author: koesie10

extensions/ql-vscode/src/codeql-cli/distribution.ts

@@ -160,6 +160,7 @@ export class DistributionManager implements DistributionProvider {
     if (this.config.customCodeQlPath) {
       if (!(await pathExists(this.config.customCodeQlPath))) {
         void showAndLogErrorMessage(
+          extLogger,
           `The CodeQL executable path is specified as "${this.config.customCodeQlPath}" ` +
             "by a configuration setting, but a CodeQL executable could not be found at that path. Please check " +
             "that a CodeQL executable exists at the specified path or remove the setting.",
@@ -852,6 +853,7 @@ export async function getExecutableFromDirectory(
 
 function warnDeprecatedLauncher() {
   void showAndLogWarningMessage(
+    extLogger,
     `The "${deprecatedCodeQlLauncherName()!}" launcher has been deprecated and will be removed in a future version. ` +
       `Please use "${codeQlLauncherName()}" instead. It is recommended to update to the latest CodeQL binaries.`,
   );

extensions/ql-vscode/src/codeql-cli/query-language.ts

@@ -59,6 +59,7 @@ export async function askForLanguage(
       throw new UserCancellationException("Cancelled.");
     } else {
       void showAndLogErrorMessage(
+        extLogger,
         "Language not found. Language must be specified manually.",
       );
     }
@@ -67,6 +68,7 @@ export async function askForLanguage(
 
   if (!isQueryLanguage(language)) {
     void showAndLogErrorMessage(
+      extLogger,
       `Language '${language}' is not supported. Only languages ${Object.values(
         QueryLanguage,
       ).join(", ")} are supported.`,

extensions/ql-vscode/src/common/logging/index.ts

@@ -1,4 +1,5 @@
 export * from "./logger";
+export * from "./notification-logger";
 export * from "./tee-logger";
 export * from "./vscode/loggers";
 export * from "./vscode/output-channel-logger";

extensions/ql-vscode/src/common/logging/notification-logger.ts

@@ -0,0 +1,7 @@
+import { Logger } from "./logger";
+
+export interface NotificationLogger extends Logger {
+  showErrorMessage(message: string): Promise<void>;
+  showWarningMessage(message: string): Promise<void>;
+  showInformationMessage(message: string): Promise<void>;
+}

extensions/ql-vscode/src/common/logging/vscode/output-channel-logger.ts

@@ -1,11 +1,15 @@
 import { window as Window, OutputChannel, Progress } from "vscode";
 import { Logger, LogOptions } from "../logger";
 import { DisposableObject } from "../../../pure/disposable-object";
+import { NotificationLogger } from "../notification-logger";
 
 /**
  * A logger that writes messages to an output channel in the VS Code Output tab.
  */
-export class OutputChannelLogger extends DisposableObject implements Logger {
+export class OutputChannelLogger
+  extends DisposableObject
+  implements Logger, NotificationLogger
+{
   public readonly outputChannel: OutputChannel;
   isCustomLogDirectory: boolean;
 
@@ -42,6 +46,30 @@ export class OutputChannelLogger extends DisposableObject implements Logger {
   show(preserveFocus?: boolean): void {
     this.outputChannel.show(preserveFocus);
   }
+
+  async showErrorMessage(message: string): Promise<void> {
+    await this.showMessage(message, Window.showErrorMessage);
+  }
+
+  async showInformationMessage(message: string): Promise<void> {
+    await this.showMessage(message, Window.showInformationMessage);
+  }
+
+  async showWarningMessage(message: string): Promise<void> {
+    await this.showMessage(message, Window.showWarningMessage);
+  }
+
+  private async showMessage(
+    message: string,
+    show: (message: string, ...items: string[]) => Thenable<string | undefined>,
+  ): Promise<void> {
+    const label = "Show Log";
+    const result = await show(message, label);
+
+    if (result === label) {
+      this.show();
+    }
+  }
 }
 
 export type ProgressReporter = Progress<{ message: string }>;

extensions/ql-vscode/src/common/vscode/commands.ts

@@ -54,18 +54,15 @@ export function registerCommandWithErrorHandling(
         if (e.silent) {
           void outputLogger.log(errorMessage.fullMessage);
         } else {
-          void showAndLogWarningMessage(errorMessage.fullMessage, {
-            outputLogger,
-          });
+          void showAndLogWarningMessage(outputLogger, errorMessage.fullMessage);
         }
       } else {
         // Include the full stack in the error log only.
         const errorStack = getErrorStack(e);
         const fullMessage = errorStack
           ? `${errorMessage.fullMessage}\n${errorStack}`
           : errorMessage.fullMessage;
-        void showAndLogExceptionWithTelemetry(errorMessage, {
-          outputLogger,
+        void showAndLogExceptionWithTelemetry(outputLogger, errorMessage, {
           fullMessage,
           extraTelemetryProperties: {
             command: commandId,

extensions/ql-vscode/src/common/vscode/external-files.ts

@@ -8,6 +8,7 @@ import {
   getErrorStack,
 } from "../../pure/helpers-pure";
 import { showAndLogExceptionWithTelemetry } from "./log";
+import { extLogger } from "../logging";
 
 export async function tryOpenExternalFile(
   commandManager: AppCommandManager,
@@ -34,6 +35,7 @@ the file in the file explorer and dragging it into the workspace.`,
           await commandManager.execute("revealFileInOS", uri);
         } catch (e) {
           void showAndLogExceptionWithTelemetry(
+            extLogger,
             redactableError(
               asError(e),
             )`Failed to reveal file in OS: ${getErrorMessage(e)}`,
@@ -42,6 +44,7 @@ the file in the file explorer and dragging it into the workspace.`,
       }
     } else {
       void showAndLogExceptionWithTelemetry(
+        extLogger,
         redactableError(asError(e))`Could not open file ${fileLocation}`,
         {
           fullMessage: `${getErrorMessage(e)}\n${getErrorStack(e)}`,

extensions/ql-vscode/src/common/vscode/log.ts

@@ -1,18 +1,13 @@
-import { window } from "vscode";
 import { RedactableError } from "../../pure/errors";
 import { telemetryListener } from "../../telemetry";
-import { extLogger, OutputChannelLogger } from "../logging";
+import { NotificationLogger } from "../logging";
 
 interface ShowAndLogExceptionOptions extends ShowAndLogOptions {
   /** Custom properties to include in the telemetry report. */
   extraTelemetryProperties?: { [key: string]: string };
 }
 
 interface ShowAndLogOptions {
-  /** The output logger that will receive the message. */
-  outputLogger?: OutputChannelLogger;
-  /** A set of items that will be rendered as actions in the message. */
-  items?: string[];
   /**
    * An alternate message that is added to the log, but not displayed in the popup.
    * This is useful for adding extra detail to the logs that would be too noisy for the popup.
@@ -23,34 +18,39 @@ interface ShowAndLogOptions {
 /**
  * Show an error message, log it to the console, and emit redacted information as telemetry
  *
+ * @param logger The logger that will receive the message.
  * @param error The error to show. Only redacted information will be included in the telemetry.
  * @param options See individual fields on `ShowAndLogExceptionOptions` type.
  *
  * @return A promise that resolves to the selected item or undefined when being dismissed.
  */
 export async function showAndLogExceptionWithTelemetry(
+  logger: NotificationLogger,
   error: RedactableError,
   options: ShowAndLogExceptionOptions = {},
-): Promise<string | undefined> {
+): Promise<void> {
   telemetryListener?.sendError(error, options.extraTelemetryProperties);
-  return showAndLogErrorMessage(error.fullMessage, options);
+  return showAndLogErrorMessage(logger, error.fullMessage, options);
 }
 
 /**
  * Show an error message and log it to the console
  *
+ * @param logger The logger that will receive the message.
  * @param message The message to show.
- * @param options See individual fields on `ShowAndLogOptions` type.
+ * @param options? See individual fields on `ShowAndLogOptions` type.
  *
  * @return A promise that resolves to the selected item or undefined when being dismissed.
  */
 export async function showAndLogErrorMessage(
+  logger: NotificationLogger,
   message: string,
   options?: ShowAndLogOptions,
-): Promise<string | undefined> {
+): Promise<void> {
   return internalShowAndLog(
+    logger,
     dropLinesExceptInitial(message),
-    window.showErrorMessage,
+    logger.showErrorMessage,
     { fullMessage: message, ...options },
   );
 }
@@ -62,48 +62,53 @@ function dropLinesExceptInitial(message: string, n = 2) {
 /**
  * Show a warning message and log it to the console
  *
+ * @param logger The logger that will receive the message.
  * @param message The message to show.
- * @param options See individual fields on `ShowAndLogOptions` type.
+ * @param options? See individual fields on `ShowAndLogOptions` type.
  *
  * @return A promise that resolves to the selected item or undefined when being dismissed.
  */
 export async function showAndLogWarningMessage(
+  logger: NotificationLogger,
   message: string,
   options?: ShowAndLogOptions,
-): Promise<string | undefined> {
-  return internalShowAndLog(message, window.showWarningMessage, options);
+): Promise<void> {
+  return internalShowAndLog(
+    logger,
+    message,
+    logger.showWarningMessage,
+    options,
+  );
 }
 
 /**
  * Show an information message and log it to the console
  *
+ * @param logger The logger that will receive the message.
  * @param message The message to show.
- * @param options See individual fields on `ShowAndLogOptions` type.
+ * @param options? See individual fields on `ShowAndLogOptions` type.
  *
  * @return A promise that resolves to the selected item or undefined when being dismissed.
  */
 export async function showAndLogInformationMessage(
+  logger: NotificationLogger,
   message: string,
   options?: ShowAndLogOptions,
-): Promise<string | undefined> {
-  return internalShowAndLog(message, window.showInformationMessage, options);
+): Promise<void> {
+  return internalShowAndLog(
+    logger,
+    message,
+    logger.showInformationMessage,
+    options,
+  );
 }
 
-type ShowMessageFn = (
-  message: string,
-  ...items: string[]
-) => Thenable<string | undefined>;
-
 async function internalShowAndLog(
+  logger: NotificationLogger,
   message: string,
-  fn: ShowMessageFn,
-  { items = [], outputLogger = extLogger, fullMessage }: ShowAndLogOptions = {},
-): Promise<string | undefined> {
-  const label = "Show Log";
-  void outputLogger.log(fullMessage || message);
-  const result = await fn(message, label, ...items);
-  if (result === label) {
-    outputLogger.show();
-  }
-  return result;
+  fn: (message: string) => Promise<void>,
+  { fullMessage }: ShowAndLogOptions = {},
+): Promise<void> {
+  void logger.log(fullMessage || message);
+  await fn.bind(logger)(message);
 }

extensions/ql-vscode/src/common/vscode/selection-commands.ts

@@ -4,6 +4,7 @@ import {
   TreeViewContextSingleSelectionCommandFunction,
 } from "../commands";
 import { showAndLogErrorMessage } from "./log";
+import { extLogger } from "../logging";
 
 // A hack to match types that are not an array, which is useful to help avoid
 // misusing createSingleSelectionCommand, e.g. where T accidentally gets instantiated
@@ -32,7 +33,10 @@ export function createSingleSelectionCommand<T extends NotArray>(
     if (multiSelect === undefined || multiSelect.length === 1) {
       return f(singleItem);
     } else {
-      void showAndLogErrorMessage(`Please select a single ${itemName}.`);
+      void showAndLogErrorMessage(
+        extLogger,
+        `Please select a single ${itemName}.`,
+      );
       return;
     }
   };

extensions/ql-vscode/src/compare/compare-view.ts

@@ -5,7 +5,7 @@ import {
   ToCompareViewMessage,
   QueryCompareResult,
 } from "../pure/interface-types";
-import { Logger } from "../common";
+import { extLogger, Logger } from "../common";
 import { CodeQLCliServer } from "../codeql-cli/cli";
 import { DatabaseManager } from "../databases/local-databases";
 import { jumpToLocation } from "../databases/local-databases/locations";
@@ -152,6 +152,7 @@ export class CompareView extends AbstractWebview<
 
       case "unhandledError":
         void showAndLogExceptionWithTelemetry(
+          extLogger,
           redactableError(
             msg.error,
           )`Unhandled error in result comparison view: ${msg.error.message}`,