Merge pull request #2222 from github/robertbrignull/use_app_commands_2

Convert call sites to use typed commands (part 2): QueryHistoryManager

Author: robertbrignull

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

@@ -38,6 +38,7 @@ export type BuiltInVsCodeCommands = {
   // The codeQLDatabases.focus command is provided by VS Code because we've registered the custom view
   "codeQLDatabases.focus": () => Promise<void>;
   "markdown.showPreviewToSide": (uri: Uri) => Promise<void>;
+  revealFileInOS: (uri: Uri) => Promise<void>;
   setContext: (
     key: `${"codeql" | "codeQL"}${string}`,
     value: unknown,

extensions/ql-vscode/src/extension.ts

@@ -777,6 +777,7 @@ async function activateWithInstalledDistribution(
   };
 
   const qhm = new QueryHistoryManager(
+    app,
     qs,
     dbm,
     localQueryResultsView,

extensions/ql-vscode/src/query-history/query-history-manager.ts

@@ -1,6 +1,5 @@
 import { join, dirname } from "path";
 import {
-  commands,
   Disposable,
   env,
   EventEmitter,
@@ -26,12 +25,7 @@ import { extLogger } from "../common";
 import { URLSearchParams } from "url";
 import { DisposableObject } from "../pure/disposable-object";
 import { ONE_HOUR_IN_MS, TWO_HOURS_IN_MS } from "../pure/time";
-import {
-  asError,
-  assertNever,
-  getErrorMessage,
-  getErrorStack,
-} from "../pure/helpers-pure";
+import { asError, assertNever, getErrorMessage } from "../pure/helpers-pure";
 import { CompletedLocalQueryInfo, LocalQueryInfo } from "../query-results";
 import {
   getActionsWorkflowRunUrl,
@@ -66,6 +60,8 @@ import { HistoryTreeDataProvider } from "./history-tree-data-provider";
 import { redactableError } from "../pure/errors";
 import { QueryHistoryDirs } from "./query-history-dirs";
 import { QueryHistoryCommands } from "../common/commands";
+import { App } from "../common/app";
+import { tryOpenExternalFile } from "../vscode-utils/external-files";
 
 /**
  * query-history-manager.ts
@@ -135,6 +131,7 @@ export class QueryHistoryManager extends DisposableObject {
   readonly onDidCompleteQuery = this._onDidCompleteQuery.event;
 
   constructor(
+    private readonly app: App,
     private readonly qs: QueryRunner,
     private readonly dbm: DatabaseManager,
     private readonly localQueriesResultsView: ResultsView,
@@ -683,7 +680,10 @@ export class QueryHistoryManager extends DisposableObject {
     }
 
     if (singleItem.completedQuery.logFileLocation) {
-      await this.tryOpenExternalFile(singleItem.completedQuery.logFileLocation);
+      await tryOpenExternalFile(
+        this.app.commands,
+        singleItem.completedQuery.logFileLocation,
+      );
     } else {
       void showAndLogWarningMessage("No log file available");
     }
@@ -751,7 +751,7 @@ export class QueryHistoryManager extends DisposableObject {
         }
       }
       try {
-        await commands.executeCommand(
+        await this.app.commands.execute(
           "revealFileInOS",
           Uri.file(externalFilePath),
         );
@@ -800,7 +800,10 @@ export class QueryHistoryManager extends DisposableObject {
     }
 
     if (finalSingleItem.evalLogLocation) {
-      await this.tryOpenExternalFile(finalSingleItem.evalLogLocation);
+      await tryOpenExternalFile(
+        this.app.commands,
+        finalSingleItem.evalLogLocation,
+      );
     } else {
       this.warnNoEvalLogs();
     }
@@ -825,7 +828,10 @@ export class QueryHistoryManager extends DisposableObject {
     }
 
     if (finalSingleItem.evalLogSummaryLocation) {
-      await this.tryOpenExternalFile(finalSingleItem.evalLogSummaryLocation);
+      await tryOpenExternalFile(
+        this.app.commands,
+        finalSingleItem.evalLogSummaryLocation,
+      );
       return;
     }
 
@@ -968,7 +974,10 @@ export class QueryHistoryManager extends DisposableObject {
     const query = finalSingleItem.completedQuery.query;
     const hasInterpretedResults = query.canHaveInterpretedResults();
     if (hasInterpretedResults) {
-      await this.tryOpenExternalFile(query.resultsPaths.interpretedResultsPath);
+      await tryOpenExternalFile(
+        this.app.commands,
+        query.resultsPaths.interpretedResultsPath,
+      );
     } else {
       const label = this.labelProvider.getLabel(finalSingleItem);
       void showAndLogInformationMessage(
@@ -997,11 +1006,11 @@ export class QueryHistoryManager extends DisposableObject {
     }
     const query = finalSingleItem.completedQuery.query;
     if (await query.hasCsv()) {
-      void this.tryOpenExternalFile(query.csvPath);
+      void tryOpenExternalFile(this.app.commands, query.csvPath);
       return;
     }
     if (await query.exportCsvResults(this.qs.cliServer, query.csvPath)) {
-      void this.tryOpenExternalFile(query.csvPath);
+      void tryOpenExternalFile(this.app.commands, query.csvPath);
     }
   }
 
@@ -1024,7 +1033,8 @@ export class QueryHistoryManager extends DisposableObject {
       return;
     }
 
-    await this.tryOpenExternalFile(
+    await tryOpenExternalFile(
+      this.app.commands,
       await finalSingleItem.completedQuery.query.ensureCsvAlerts(
         this.qs.cliServer,
         this.dbm,
@@ -1051,7 +1061,8 @@ export class QueryHistoryManager extends DisposableObject {
       return;
     }
 
-    await this.tryOpenExternalFile(
+    await tryOpenExternalFile(
+      this.app.commands,
       await finalSingleItem.completedQuery.query.ensureDilPath(
         this.qs.cliServer,
       ),
@@ -1077,7 +1088,7 @@ export class QueryHistoryManager extends DisposableObject {
 
     const actionsWorkflowRunUrl = getActionsWorkflowRunUrl(finalSingleItem);
 
-    await commands.executeCommand(
+    await this.app.commands.execute(
       "vscode.open",
       Uri.parse(actionsWorkflowRunUrl),
     );
@@ -1101,7 +1112,7 @@ export class QueryHistoryManager extends DisposableObject {
       return;
     }
 
-    await commands.executeCommand(
+    await this.app.commands.execute(
       "codeQL.copyVariantAnalysisRepoList",
       finalSingleItem.variantAnalysis.id,
     );
@@ -1171,47 +1182,6 @@ export class QueryHistoryManager extends DisposableObject {
     }
   }
 
-  private async tryOpenExternalFile(fileLocation: string) {
-    const uri = Uri.file(fileLocation);
-    try {
-      await window.showTextDocument(uri, { preview: false });
-    } catch (e) {
-      const msg = getErrorMessage(e);
-      if (
-        msg.includes(
-          "Files above 50MB cannot be synchronized with extensions",
-        ) ||
-        msg.includes("too large to open")
-      ) {
-        const res = await showBinaryChoiceDialog(
-          `VS Code does not allow extensions to open files >50MB. This file
-exceeds that limit. Do you want to open it outside of VS Code?
-
-You can also try manually opening it inside VS Code by selecting
-the file in the file explorer and dragging it into the workspace.`,
-        );
-        if (res) {
-          try {
-            await commands.executeCommand("revealFileInOS", uri);
-          } catch (e) {
-            void showAndLogExceptionWithTelemetry(
-              redactableError(
-                asError(e),
-              )`Failed to reveal file in OS: ${getErrorMessage(e)}`,
-            );
-          }
-        }
-      } else {
-        void showAndLogExceptionWithTelemetry(
-          redactableError(asError(e))`Could not open file ${fileLocation}`,
-          {
-            fullMessage: `${getErrorMessage(e)}\n${getErrorStack(e)}`,
-          },
-        );
-      }
-    }
-  }
-
   private async findOtherQueryToCompare(
     singleItem: QueryHistoryInfo,
     multiSelect: QueryHistoryInfo[],

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

@@ -0,0 +1,50 @@
+import { Uri, window } from "vscode";
+import { AppCommandManager } from "../common/commands";
+import {
+  showAndLogExceptionWithTelemetry,
+  showBinaryChoiceDialog,
+} from "../helpers";
+import { redactableError } from "../pure/errors";
+import { asError, getErrorMessage, getErrorStack } from "../pure/helpers-pure";
+
+export async function tryOpenExternalFile(
+  commandManager: AppCommandManager,
+  fileLocation: string,
+) {
+  const uri = Uri.file(fileLocation);
+  try {
+    await window.showTextDocument(uri, { preview: false });
+  } catch (e) {
+    const msg = getErrorMessage(e);
+    if (
+      msg.includes("Files above 50MB cannot be synchronized with extensions") ||
+      msg.includes("too large to open")
+    ) {
+      const res = await showBinaryChoiceDialog(
+        `VS Code does not allow extensions to open files >50MB. This file
+exceeds that limit. Do you want to open it outside of VS Code?
+
+You can also try manually opening it inside VS Code by selecting
+the file in the file explorer and dragging it into the workspace.`,
+      );
+      if (res) {
+        try {
+          await commandManager.execute("revealFileInOS", uri);
+        } catch (e) {
+          void showAndLogExceptionWithTelemetry(
+            redactableError(
+              asError(e),
+            )`Failed to reveal file in OS: ${getErrorMessage(e)}`,
+          );
+        }
+      }
+    } else {
+      void showAndLogExceptionWithTelemetry(
+        redactableError(asError(e))`Could not open file ${fileLocation}`,
+        {
+          fullMessage: `${getErrorMessage(e)}\n${getErrorStack(e)}`,
+        },
+      );
+    }
+  }
+}

extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/history-tree-data-provider.test.ts

@@ -27,6 +27,7 @@ import {
 } from "../../../../src/query-history/history-tree-data-provider";
 import { QueryHistoryManager } from "../../../../src/query-history/query-history-manager";
 import { createMockQueryHistoryDirs } from "../../../factories/query-history/query-history-dirs";
+import { createMockApp } from "../../../__mocks__/appMock";
 
 describe("HistoryTreeDataProvider", () => {
   const mockExtensionLocation = join(tmpDir.name, "mock-extension-location");
@@ -421,6 +422,7 @@ describe("HistoryTreeDataProvider", () => {
 
   async function createMockQueryHistory(allHistory: QueryHistoryInfo[]) {
     const qhm = new QueryHistoryManager(
+      createMockApp({}),
       {} as QueryRunner,
       {} as DatabaseManager,
       localQueriesResultsViewStub,