PR feedback

Author: marcosnav

extensions/vscode/src/utils/window.test.ts

@@ -17,6 +17,10 @@ const terminalMock = {
   },
 };
 
+const cancelTokenMock = {
+  onCancellationRequested: vi.fn(),
+};
+
 vi.mock("vscode", () => {
   // mock Disposable
   const disposableMock = vi.fn();
@@ -28,7 +32,7 @@ vi.mock("vscode", () => {
     showWarningMessage: vi.fn(),
     showInformationMessage: vi.fn(),
     withProgress: vi.fn().mockImplementation((_, progressCallback) => {
-      progressCallback();
+      progressCallback(_, cancelTokenMock);
     }),
     createTerminal: vi.fn().mockImplementation(() => {
       terminalMock.sendText = vi.fn();
@@ -92,21 +96,44 @@ describe("Consumers of vscode window", () => {
     );
   });
 
-  test("taskWithProgressMsg", () => {
-    const taskMock = vi.fn();
-    taskWithProgressMsg(
-      "Running a task with a progress notification",
-      taskMock,
-    );
-    expect(window.withProgress).toHaveBeenCalledWith(
-      {
-        location: 15,
-        title: "Running a task with a progress notification",
-        cancellable: false,
-      },
-      expect.any(Function),
-    );
-    expect(taskMock).toHaveBeenCalled();
+  describe("taskWithProgressMsg", () => {
+    test("not cancellable", () => {
+      const taskMock = vi.fn();
+      taskWithProgressMsg(
+        "Running a task with a progress notification",
+        taskMock,
+      );
+      expect(window.withProgress).toHaveBeenCalledWith(
+        {
+          location: 15,
+          title: "Running a task with a progress notification",
+          cancellable: false,
+        },
+        expect.any(Function),
+      );
+      expect(taskMock).toHaveBeenCalled();
+      expect(cancelTokenMock.onCancellationRequested).not.toHaveBeenCalled();
+    });
+
+    test("cancellable", () => {
+      const taskMock = vi.fn();
+      taskWithProgressMsg(
+        "Running a task with a progress notification",
+        taskMock,
+        () => {},
+      );
+      expect(window.withProgress).toHaveBeenCalledWith(
+        {
+          location: 15,
+          title: "Running a task with a progress notification",
+          cancellable: true,
+        },
+        expect.any(Function),
+      );
+      // Cancel listener is registered
+      expect(taskMock).toHaveBeenCalled();
+      expect(cancelTokenMock.onCancellationRequested).toHaveBeenCalled();
+    });
   });
 
   describe("runTerminalCommand", () => {

extensions/vscode/src/utils/window.ts

@@ -21,13 +21,9 @@ export function showInformationMsg(msg: string, ...items: string[]) {
 
 type taskFunc = <T>(p: Progress<T>, t: CancellationToken) => Promise<void>;
 const progressCallbackHandlerFactory =
-  (
-    task: taskFunc,
-    cancellable: boolean = false,
-    onCancel?: () => void,
-  ): taskFunc =>
+  (task: taskFunc, onCancel?: () => void): taskFunc =>
   (progress, token) => {
-    if (cancellable && onCancel) {
+    if (onCancel) {
       token.onCancellationRequested(() => {
         onCancel();
       });
@@ -38,16 +34,15 @@ const progressCallbackHandlerFactory =
 export function taskWithProgressMsg(
   msg: string,
   task: taskFunc,
-  cancellable: boolean = false,
   onCancel?: () => void,
 ) {
   return window.withProgress(
     {
       location: ProgressLocation.Notification,
       title: msg,
-      cancellable,
+      cancellable: onCancel !== undefined,
     },
-    progressCallbackHandlerFactory(task, cancellable, onCancel),
+    progressCallbackHandlerFactory(task, onCancel),
   );
 }
 

internal/inspect/dependencies/renv/manifest_packages.go

@@ -195,7 +195,7 @@ func (m *defaultPackageMapper) GetManifestPackages(
 	lockfile, err := ReadLockfile(lockfilePath)
 	if err != nil {
 		if errors.Is(err, os.ErrNotExist) {
-			return nil, m.renvEnvironmentCheck(base, log)
+			return nil, m.renvEnvironmentCheck(log)
 		}
 		return nil, err
 	}
@@ -260,7 +260,6 @@ func (m *defaultPackageMapper) GetManifestPackages(
 }
 
 func (m *defaultPackageMapper) renvEnvironmentCheck(
-	base util.AbsolutePath,
 	log logging.Logger,
 ) *types.AgentError {
 	rInterpreter, err := m.rInterpreterFactory()