Merge pull request #3466 from github/robertbrignull/remove_withInheritedProgress

Delete withInheritedProgress and ProgressContext
2024-03-14 13:44:22 +00:00 · 2024-03-14 13:44:22 +00:00 · d9ac8744b6
--- a/extensions/ql-vscode/src/common/vscode/progress.ts
+++ b/extensions/ql-vscode/src/common/vscode/progress.ts
@ -85,29 +85,6 @@ export function withProgress<R>(
  );
 }

-export interface ProgressContext {
-  progress: ProgressCallback;
-  token: CancellationToken;
-}
-
-/**
- * Like `withProgress()`, except that the caller is not required to provide a progress context. If
- * the caller does provide one, any long-running operations performed by `task` will use the
- * supplied progress context. Otherwise, this function wraps `task` in a new progress context with
- * the supplied options.
- */
-export function withInheritedProgress<R>(
-  parent: ProgressContext | undefined,
-  task: ProgressTask<R>,
-  options: ProgressOptions,
-): Thenable<R> {
-  if (parent !== undefined) {
-    return task(parent.progress, parent.token);
-  } else {
-    return withProgress(task, options);
-  }
-}
-
 /**
 * Displays a progress monitor that indicates how much progess has been made
 * reading from a stream.
--- a/extensions/ql-vscode/src/databases/local-databases-ui.ts
+++ b/extensions/ql-vscode/src/databases/local-databases-ui.ts
@ -16,7 +16,6 @@ import {
  ThemeIcon,
  ThemeColor,
  workspace,
-  ProgressLocation,
 } from "vscode";
 import { pathExists, stat, readdir, remove } from "fs-extra";

@ -25,13 +24,9 @@ import type {
  DatabaseItem,
  DatabaseManager,
 } from "./local-databases";
-import type {
-  ProgressCallback,
-  ProgressContext,
-} from "../common/vscode/progress";
+import type { ProgressCallback } from "../common/vscode/progress";
 import {
  UserCancellationException,
-  withInheritedProgress,
  withProgress,
 } from "../common/vscode/progress";
 import {
@ -332,10 +327,9 @@ export class DatabaseUI extends DisposableObject {

  private async chooseDatabaseFolder(
    progress: ProgressCallback,
-    token: CancellationToken,
  ): Promise<void> {
    try {
-      await this.chooseAndSetDatabase(true, { progress, token });
+      await this.chooseAndSetDatabase(true, progress);
    } catch (e) {
      void showAndLogExceptionWithTelemetry(
        this.app.logger,
@ -349,8 +343,8 @@ export class DatabaseUI extends DisposableObject {

  private async handleChooseDatabaseFolder(): Promise<void> {
    return withProgress(
-      async (progress, token) => {
-        await this.chooseDatabaseFolder(progress, token);
+      async (progress) => {
+        await this.chooseDatabaseFolder(progress);
      },
      {
        title: "Adding database from folder",
@ -360,8 +354,8 @@ export class DatabaseUI extends DisposableObject {

  private async handleChooseDatabaseFolderFromPalette(): Promise<void> {
    return withProgress(
-      async (progress, token) => {
-        await this.chooseDatabaseFolder(progress, token);
+      async (progress) => {
+        await this.chooseDatabaseFolder(progress);
      },
      {
        title: "Choose a Database from a Folder",
@ -502,10 +496,9 @@ export class DatabaseUI extends DisposableObject {

  private async chooseDatabaseArchive(
    progress: ProgressCallback,
-    token: CancellationToken,
  ): Promise<void> {
    try {
-      await this.chooseAndSetDatabase(false, { progress, token });
+      await this.chooseAndSetDatabase(false, progress);
    } catch (e: unknown) {
      void showAndLogExceptionWithTelemetry(
        this.app.logger,
@ -519,8 +512,8 @@ export class DatabaseUI extends DisposableObject {

  private async handleChooseDatabaseArchive(): Promise<void> {
    return withProgress(
-      async (progress, token) => {
-        await this.chooseDatabaseArchive(progress, token);
+      async (progress) => {
+        await this.chooseDatabaseArchive(progress);
      },
      {
        title: "Adding database from archive",
@ -530,8 +523,8 @@ export class DatabaseUI extends DisposableObject {

  private async handleChooseDatabaseArchiveFromPalette(): Promise<void> {
    return withProgress(
-      async (progress, token) => {
-        await this.chooseDatabaseArchive(progress, token);
+      async (progress) => {
+        await this.chooseDatabaseArchive(progress);
      },
      {
        title: "Choose a Database from an Archive",
@ -851,9 +844,8 @@ export class DatabaseUI extends DisposableObject {
   */
  public async getDatabaseItem(
    progress: ProgressCallback,
-    token: CancellationToken,
  ): Promise<DatabaseItem | undefined> {
-    return await this.getDatabaseItemInternal({ progress, token });
+    return await this.getDatabaseItemInternal(progress);
  }

  /**
@ -866,10 +858,10 @@ export class DatabaseUI extends DisposableObject {
   * notification if it tries to perform any long-running operations.
   */
  private async getDatabaseItemInternal(
-    progressContext: ProgressContext | undefined,
+    progress: ProgressCallback | undefined,
  ): Promise<DatabaseItem | undefined> {
    if (this.databaseManager.currentDatabaseItem === undefined) {
-      progressContext?.progress({
+      progress?.({
        maxStep: 2,
        step: 1,
        message: "Choosing database",
@ -996,41 +988,31 @@ export class DatabaseUI extends DisposableObject {
   */
  private async chooseAndSetDatabase(
    byFolder: boolean,
-    progress: ProgressContext | undefined,
+    progress: ProgressCallback,
  ): Promise<DatabaseItem | undefined> {
    const uri = await chooseDatabaseDir(byFolder);
    if (!uri) {
      return undefined;
    }

-    return await withInheritedProgress(
-      progress,
-      async (progress) => {
-        if (byFolder) {
-          const fixedUri = await this.fixDbUri(uri);
-          // we are selecting a database folder
-          return await this.databaseManager.openDatabase(fixedUri, {
-            type: "folder",
-          });
-        } else {
-          // we are selecting a database archive. Must unzip into a workspace-controlled area
-          // before importing.
-          return await importLocalDatabase(
-            this.app.commands,
-            uri.toString(true),
-            this.databaseManager,
-            this.storagePath,
-            progress,
-            this.queryServer.cliServer,
-          );
-        }
-      },
-      {
-        location: ProgressLocation.Notification,
-        cancellable: true,
-        title: "Opening database",
-      },
-    );
+    if (byFolder) {
+      const fixedUri = await this.fixDbUri(uri);
+      // we are selecting a database folder
+      return await this.databaseManager.openDatabase(fixedUri, {
+        type: "folder",
+      });
+    } else {
+      // we are selecting a database archive. Must unzip into a workspace-controlled area
+      // before importing.
+      return await importLocalDatabase(
+        this.app.commands,
+        uri.toString(true),
+        this.databaseManager,
+        this.storagePath,
+        progress,
+        this.queryServer.cliServer,
+      );
+    }
  }

  /**
--- a/extensions/ql-vscode/src/local-queries/local-queries.ts
+++ b/extensions/ql-vscode/src/local-queries/local-queries.ts
@ -290,14 +290,8 @@ export class LocalQueries extends DisposableObject {

  private async quickQuery(): Promise<void> {
    await withProgress(
-      async (progress, token) =>
-        displayQuickQuery(
-          this.app,
-          this.cliServer,
-          this.databaseUI,
-          progress,
-          token,
-        ),
+      async (progress) =>
+        displayQuickQuery(this.app, this.cliServer, this.databaseUI, progress),
      {
        title: "Run Quick Query",
      },
@ -445,7 +439,7 @@ export class LocalQueries extends DisposableObject {

    // If no databaseItem is specified, use the database currently selected in the Databases UI
    databaseItem =
-      databaseItem ?? (await this.databaseUI.getDatabaseItem(progress, token));
+      databaseItem ?? (await this.databaseUI.getDatabaseItem(progress));
    if (databaseItem === undefined) {
      throw new Error("Can't run query without a selected database");
    }
--- a/extensions/ql-vscode/src/local-queries/quick-query.ts
+++ b/extensions/ql-vscode/src/local-queries/quick-query.ts
@ -1,7 +1,6 @@
 import { ensureDir, writeFile, pathExists, readFile } from "fs-extra";
 import { dump, load } from "js-yaml";
 import { basename, join } from "path";
-import type { CancellationToken } from "vscode";
 import { window as Window, workspace, Uri } from "vscode";
 import { LSPErrorCodes, ResponseError } from "vscode-languageclient";
 import type { CodeQLCliServer } from "../codeql-cli/cli";
@ -56,7 +55,6 @@ export async function displayQuickQuery(
  cliServer: CodeQLCliServer,
  databaseUI: DatabaseUI,
  progress: ProgressCallback,
-  token: CancellationToken,
 ) {
  try {
    // If there is already a quick query open, don't clobber it, just
@ -111,7 +109,7 @@ export async function displayQuickQuery(
    }

    // We're going to infer which qlpack to use from the current database
-    const dbItem = await databaseUI.getDatabaseItem(progress, token);
+    const dbItem = await databaseUI.getDatabaseItem(progress);
    if (dbItem === undefined) {
      throw new Error("Can't start quick query without a selected database");
    }
--- a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/local-databases-ui.test.ts
+++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/local-databases-ui.test.ts
@ -7,7 +7,7 @@ import {
  createFileSync,
  pathExistsSync,
 } from "fs-extra";
-import { CancellationTokenSource, Uri, window } from "vscode";
+import { Uri, window } from "vscode";

 import type {
  DatabaseImportQuickPickItems,
@ -127,7 +127,6 @@ describe("local-databases-ui", () => {

  describe("getDatabaseItem", () => {
    const progress = jest.fn();
-    const token = new CancellationTokenSource().token;
    describe("when there is a current database", () => {
      const databaseUI = new DatabaseUI(
        app,
@ -153,7 +152,7 @@ describe("local-databases-ui", () => {
      );

      it("should return current database", async () => {
-        const databaseItem = await databaseUI.getDatabaseItem(progress, token);
+        const databaseItem = await databaseUI.getDatabaseItem(progress);

        expect(databaseItem).toEqual({ databaseUri: Uri.file(db1) });
      });
@ -211,7 +210,7 @@ describe("local-databases-ui", () => {
          "setCurrentDatabaseItem",
        );

-        await databaseUI.getDatabaseItem(progress, token);
+        await databaseUI.getDatabaseItem(progress);

        expect(showQuickPickSpy).toHaveBeenCalledTimes(2);
        expect(setCurrentDatabaseItemSpy).toHaveBeenCalledWith({
@ -241,7 +240,7 @@ describe("local-databases-ui", () => {
          .spyOn(databaseUI as any, "handleChooseDatabaseGithub")
          .mockResolvedValue(undefined);

-        await databaseUI.getDatabaseItem(progress, token);
+        await databaseUI.getDatabaseItem(progress);

        expect(showQuickPickSpy).toHaveBeenCalledTimes(2);
        expect(handleChooseDatabaseGithubSpy).toHaveBeenCalledTimes(1);
@ -264,7 +263,7 @@ describe("local-databases-ui", () => {
          .spyOn(databaseUI as any, "handleChooseDatabaseGithub")
          .mockResolvedValue(undefined);

-        await databaseUI.getDatabaseItem(progress, token);
+        await databaseUI.getDatabaseItem(progress);

        expect(showQuickPickSpy).toHaveBeenCalledTimes(1);
        expect(handleChooseDatabaseGithubSpy).toHaveBeenCalledTimes(1);