Fix URI Aware Command Handler

Signed-off-by: Colin Grant <[email protected]>

packages/core/src/browser/common-frontend-contribution.ts

@@ -565,9 +565,9 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
     }
 
     registerCommands(commandRegistry: CommandRegistry): void {
-        commandRegistry.registerCommand(CommonCommands.OPEN, new UriAwareCommandHandler<URI[]>(this.selectionService, {
+        commandRegistry.registerCommand(CommonCommands.OPEN, UriAwareCommandHandler.MultiSelect(this.selectionService, {
             execute: uris => uris.map(uri => open(this.openerService, uri)),
-        }, { multi: true }));
+        }));
         commandRegistry.registerCommand(CommonCommands.CUT, {
             execute: () => {
                 if (supportCut) {
@@ -595,7 +595,7 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
                 }
             }
         });
-        commandRegistry.registerCommand(CommonCommands.COPY_PATH, new UriAwareCommandHandler<URI[]>(this.selectionService, {
+        commandRegistry.registerCommand(CommonCommands.COPY_PATH, UriAwareCommandHandler.MultiSelect(this.selectionService, {
             execute: async uris => {
                 if (uris.length) {
                     const lineDelimiter = isWindows ? '\r\n' : '\n';
@@ -605,7 +605,7 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
                     await this.messageService.info('Open a file first to copy its path');
                 }
             }
-        }, { multi: true }));
+        }));
 
         commandRegistry.registerCommand(CommonCommands.UNDO, {
             execute: () => document.execCommand('undo')

packages/core/src/common/uri-command-handler.spec.ts

@@ -0,0 +1,90 @@
+/********************************************************************************
+ * Copyright (C) 2020 Ericsson and others.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * This Source Code may also be made available under the following Secondary
+ * Licenses when the conditions for such availability set forth in the Eclipse
+ * Public License v. 2.0 are satisfied: GNU General Public License, version 2
+ * with the GNU Classpath Exception which is available at
+ * https://www.gnu.org/software/classpath/license.html.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
+ ********************************************************************************/
+
+import * as chai from 'chai';
+import { SelectionService } from '.';
+import { MaybeArray } from './types';
+import URI from './uri';
+import { UriAwareCommandHandler, UriCommandHandler } from './uri-command-handler';
+
+const expect = chai.expect;
+
+interface CommandHandlerMock extends UriCommandHandler<MaybeArray<URI>> {
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    lastCall: any[];
+}
+
+const mockHandler: CommandHandlerMock = {
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    execute(...args: any[]): void { this.lastCall = args; },
+    lastCall: []
+};
+const selectedURIs = [
+    new URI('/foo'),
+    new URI('/bar'),
+];
+const mockSelectionService = {
+    selection: selectedURIs.map(uri => ({ uri }))
+} as unknown as SelectionService;
+
+describe('URI-Aware Command Handlers', () => {
+    afterEach(() => {
+        mockHandler.lastCall = [];
+    });
+
+    describe('UriAwareCommandHandler', () => {
+        it('getUri returns the first argument if it is a URI (single)', () => {
+            const args = [new URI('/passed/in'), 'some', 'other', 'args'];
+            const output = UriAwareCommandHandler.MonoSelect(mockSelectionService, mockHandler)['getUri'](...args);
+            expect(output).equals(args[0]);
+        });
+        it('getUri returns the first argument if it is a URI (multi)', () => {
+            const args = [[new URI('/passed/in')], 'some', 'other', 'args'];
+            const output = UriAwareCommandHandler.MultiSelect(mockSelectionService, mockHandler)['getUri'](...args);
+            expect(output).equals(args[0]);
+        });
+        it('getUri returns an argument from the service if no URI is provided (single)', () => {
+            const args = ['some', 'other', 'args'];
+            const output = UriAwareCommandHandler.MonoSelect(mockSelectionService, mockHandler)['getUri'](...args);
+            expect(output).equals(selectedURIs[0]);
+        });
+        it('getUri returns an argument from the service if no URI is provided (multi)', () => {
+            const args = ['some', 'other', 'args'];
+            const output = UriAwareCommandHandler.MultiSelect(mockSelectionService, mockHandler)['getUri'](...args);
+            expect(output).deep.equals(selectedURIs);
+        });
+        it('calls the handler with the same args if the first argument if it is a URI (single)', () => {
+            const args = [new URI('/passed/in'), 'some', 'other', 'args'];
+            UriAwareCommandHandler.MonoSelect(mockSelectionService, mockHandler)['execute'](...args);
+            expect(mockHandler.lastCall).deep.equals(args);
+        });
+        it('calls the handler with the same args if the first argument if it is a URI (multi)', () => {
+            const args = [[new URI('/passed/in')], 'some', 'other', 'args'];
+            UriAwareCommandHandler.MultiSelect(mockSelectionService, mockHandler)['execute'](...args);
+            expect(mockHandler.lastCall).deep.equals(args);
+        });
+        it('calls the handler with an argument from the service if no URI is provided (single)', () => {
+            const args = ['some', 'other', 'args'];
+            UriAwareCommandHandler.MonoSelect(mockSelectionService, mockHandler)['execute'](...args);
+            expect(mockHandler.lastCall).deep.equals([selectedURIs[0], ...args]);
+        });
+        it('calls the handler with an argument from the service if no URI is provided (multi)', () => {
+            const args = ['some', 'other', 'args'];
+            UriAwareCommandHandler.MultiSelect(mockSelectionService, mockHandler)['execute'](...args);
+            expect(mockHandler.lastCall).deep.equals([selectedURIs, ...args]);
+        });
+    });
+});

packages/core/src/common/uri-command-handler.ts

@@ -14,21 +14,20 @@
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
 
+/* eslint-disable @typescript-eslint/no-explicit-any */
+
 import { SelectionService } from '../common/selection-service';
 import { UriSelection } from '../common/selection';
 import { CommandHandler } from './command';
 import { MaybeArray } from '.';
 import URI from './uri';
 
-export interface UriCommandHandler<T extends MaybeArray<URI>> {
+export interface UriCommandHandler<T extends MaybeArray<URI>> extends CommandHandler {
 
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
     execute(uri: T, ...args: any[]): any;
 
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
     isEnabled?(uri: T, ...args: any[]): boolean;
 
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
     isVisible?(uri: T, ...args: any[]): boolean;
 
 }
@@ -47,74 +46,65 @@ export interface MultiUriCommandHandler extends UriCommandHandler<URI[]> {
 
 }
 
-export namespace UriAwareCommandHandler {
-
+export class UriAwareCommandHandler<T extends MaybeArray<URI>> implements UriCommandHandler<T> {
     /**
-     * Further options for the URI aware command handler instantiation.
+     * @deprecated since 1.6.0. Please use `UriAwareCommandHandler.MonoSelect` or `UriAwareCommandHandler.MultiSelect`.
      */
-    export interface Options {
-
-        /**
-         * `true` if the handler supports multiple selection. Otherwise, `false`. Defaults to `false`.
-         */
-        readonly multi?: boolean,
-
-    }
-
-}
-/**
- * @todo Create different classes for single and multi-uris. State can be
- * corrupt if the developer does something like:
- * ```ts
- * new UriAwareCommandHandler<URI[]>(selectionService, handler, { multi: false })
- * ```
- */
-export class UriAwareCommandHandler<T extends MaybeArray<URI>> implements CommandHandler {
-
     constructor(
         protected readonly selectionService: SelectionService,
         protected readonly handler: UriCommandHandler<T>,
         protected readonly options?: UriAwareCommandHandler.Options
     ) { }
 
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
     protected getUri(...args: any[]): T | undefined {
-        if (args && args[0] instanceof URI) {
-            // @ts-ignore we want to always return URIs
-            return this.isMulti() ? [args[0]] : args[0];
+        const [maybeUriArray] = args;
+        const firstArgIsOK = this.isMulti()
+            ? Array.isArray(maybeUriArray) && maybeUriArray.every(uri => uri instanceof URI)
+            : maybeUriArray instanceof URI;
+
+        if (firstArgIsOK) {
+            return maybeUriArray;
         }
+
         const { selection } = this.selectionService;
-        if (!this.isMulti()) {
-            return UriSelection.getUri(selection) as T;
+
+        const uriOrUris = this.isMulti()
+            ? UriSelection.getUris(selection)
+            : UriSelection.getUri(selection);
+
+        return uriOrUris as T;
+    }
+
+    protected getArgsWithUri(...args: any[]): [T | undefined, ...any[]] {
+        const uri = this.getUri(...args);
+        const [maybeUri, ...others] = args;
+        if (uri === maybeUri) {
+            return [maybeUri, ...others];
         }
-        const uris = UriSelection.getUris(selection);
-        return uris as T;
+        return [uri, ...args];
     }
 
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
     execute(...args: any[]): object | undefined {
-        const uri = this.getUri(...args);
-        return uri ? this.handler.execute(uri, ...args) : undefined;
+        const [uri, ...others] = this.getArgsWithUri(...args);
+        return uri ? this.handler.execute(uri, ...others) : undefined;
     }
 
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
     isVisible(...args: any[]): boolean {
-        const uri = this.getUri(...args);
+        const [uri, ...others] = this.getArgsWithUri(...args);
         if (uri) {
             if (this.handler.isVisible) {
-                return this.handler.isVisible(uri as T, ...args);
+                return this.handler.isVisible(uri, ...others);
             }
             return true;
         }
         return false;
     }
 
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
     isEnabled(...args: any[]): boolean {
-        const uri = this.getUri(...args);
+        const [uri, ...others] = this.getArgsWithUri(...args);
         if (uri) {
             if (this.handler.isEnabled) {
-                return this.handler.isEnabled(uri as T, ...args);
+                return this.handler.isEnabled(uri, ...others);
             }
             return true;
         }
@@ -124,5 +114,35 @@ export class UriAwareCommandHandler<T extends MaybeArray<URI>> implements Comman
     protected isMulti(): boolean | undefined {
         return this.options && !!this.options.multi;
     }
+}
+
+export namespace UriAwareCommandHandler {
+    /**
+     * Further options for the URI aware command handler instantiation.
+     */
+    export interface Options {
+
+        /**
+         * `true` if the handler supports multiple selection. Otherwise, `false`. Defaults to `false`.
+         */
+        readonly multi?: boolean,
+
+    }
+
+    /**
+     * @returns a command handler for mono-select contexts that expects a `URI` as the first parameter of its methods.
+     */
+    export function MonoSelect(selectionService: SelectionService, handler: UriCommandHandler<URI>): UriAwareCommandHandler<URI> {
+        /* eslint-disable-next-line deprecation/deprecation*/ // Safe to use when the generic and the options agree.
+        return new UriAwareCommandHandler<URI>(selectionService, handler, { multi: false });
+    }
 
+    /**
+     * @returns a command handler for multi-select contexts that expects a `URI[]` as the first parameter of its methods.
+     */
+    export function MultiSelect(selectionService: SelectionService, handler: UriCommandHandler<URI[]>): UriAwareCommandHandler<URI[]> {
+        /* eslint-disable-next-line deprecation/deprecation*/ // Safe to use when the generic and the options agree.
+        return new UriAwareCommandHandler<URI[]>(selectionService, handler, { multi: true });
+    }
 }
+

packages/filesystem/src/browser/download/file-download-command-contribution.ts

@@ -35,19 +35,19 @@ export class FileDownloadCommandContribution implements CommandContribution {
     registerCommands(registry: CommandRegistry): void {
         registry.registerCommand(
             FileDownloadCommands.DOWNLOAD,
-            new UriAwareCommandHandler<URI[]>(this.selectionService, {
+            UriAwareCommandHandler.MultiSelect(this.selectionService, {
                 execute: uris => this.executeDownload(uris),
                 isEnabled: uris => this.isDownloadEnabled(uris),
                 isVisible: uris => this.isDownloadVisible(uris),
-            }, { multi: true })
+            })
         );
         registry.registerCommand(
             FileDownloadCommands.COPY_DOWNLOAD_LINK,
-            new UriAwareCommandHandler<URI[]>(this.selectionService, {
+            UriAwareCommandHandler.MultiSelect(this.selectionService, {
                 execute: uris => this.executeDownload(uris, { copyLink: true }),
                 isEnabled: uris => isChrome && this.isDownloadEnabled(uris),
                 isVisible: uris => isChrome && this.isDownloadVisible(uris),
-            }, { multi: true })
+            })
         );
     }
 

packages/navigator/src/browser/navigator-contribution.ts

@@ -333,7 +333,7 @@ export class FileNavigatorContribution extends AbstractViewContribution<FileNavi
             isEnabled: () => this.navigatorDiff.isFirstFileSelected,
             isVisible: () => this.navigatorDiff.isFirstFileSelected
         });
-        registry.registerCommand(FileNavigatorCommands.COPY_RELATIVE_FILE_PATH, new UriAwareCommandHandler<URI[]>(this.selectionService, {
+        registry.registerCommand(FileNavigatorCommands.COPY_RELATIVE_FILE_PATH, UriAwareCommandHandler.MultiSelect(this.selectionService, {
             isEnabled: uris => !!uris.length,
             isVisible: uris => !!uris.length,
             execute: async uris => {
@@ -346,7 +346,7 @@ export class FileNavigatorContribution extends AbstractViewContribution<FileNavi
                 }).join(lineDelimiter);
                 await this.clipboardService.writeText(text);
             }
-        }, { multi: true }));
+        }));
         registry.registerCommand(FileNavigatorCommands.OPEN, {
             isEnabled: () => this.getSelectedFileNodes().length > 0,
             isVisible: () => this.getSelectedFileNodes().length > 0,

packages/plugin-ext/src/main/browser/selection-provider-command.ts

@@ -41,6 +41,6 @@ export class SelectionProviderCommandContribution implements CommandContribution
     }
 
     protected newMultiUriAwareCommandHandler(handler: UriCommandHandler<URI[]>): UriAwareCommandHandler<URI[]> {
-        return new UriAwareCommandHandler(this.selectionService, handler, { multi: true });
+        return UriAwareCommandHandler.MultiSelect(this.selectionService, handler);
     }
 }

packages/scm-extra/src/browser/history/scm-history-contribution.ts

@@ -101,7 +101,7 @@ export class ScmHistoryContribution extends AbstractViewContribution<ScmHistoryW
     }
 
     protected newUriAwareCommandHandler(handler: UriCommandHandler<URI>): UriAwareCommandHandler<URI> {
-        return new UriAwareCommandHandler(this.selectionService, handler);
+        return UriAwareCommandHandler.MonoSelect(this.selectionService, handler);
     }
 
 }

packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts

@@ -239,10 +239,10 @@ export class SearchInWorkspaceFrontendContribution extends AbstractViewContribut
     }
 
     protected newUriAwareCommandHandler(handler: UriCommandHandler<URI>): UriAwareCommandHandler<URI> {
-        return new UriAwareCommandHandler(this.selectionService, handler);
+        return UriAwareCommandHandler.MonoSelect(this.selectionService, handler);
     }
 
     protected newMultiUriAwareCommandHandler(handler: UriCommandHandler<URI[]>): UriAwareCommandHandler<URI[]> {
-        return new UriAwareCommandHandler(this.selectionService, handler, { multi: true });
+        return UriAwareCommandHandler.MultiSelect(this.selectionService, handler);
     }
 }

packages/terminal/src/browser/terminal-frontend-contribution.ts

@@ -305,7 +305,7 @@ export class TerminalFrontendContribution implements TerminalService, CommandCon
             execute: () => (this.shell.activeWidget as TerminalWidget).clearOutput()
         });
 
-        commands.registerCommand(TerminalCommands.TERMINAL_CONTEXT, new UriAwareCommandHandler<URI>(this.selectionService, {
+        commands.registerCommand(TerminalCommands.TERMINAL_CONTEXT, UriAwareCommandHandler.MonoSelect(this.selectionService, {
             execute: uri => this.openInTerminal(uri)
         }));
 

packages/workspace/src/browser/workspace-commands.ts

@@ -343,11 +343,11 @@ export class WorkspaceCommandContribution implements CommandContribution {
     }
 
     protected newUriAwareCommandHandler(handler: UriCommandHandler<URI>): UriAwareCommandHandler<URI> {
-        return new UriAwareCommandHandler(this.selectionService, handler);
+        return UriAwareCommandHandler.MonoSelect(this.selectionService, handler);
     }
 
     protected newMultiUriAwareCommandHandler(handler: UriCommandHandler<URI[]>): UriAwareCommandHandler<URI[]> {
-        return new UriAwareCommandHandler(this.selectionService, handler, { multi: true });
+        return UriAwareCommandHandler.MultiSelect(this.selectionService, handler);
     }
 
     protected newWorkspaceRootUriAwareCommandHandler(handler: UriCommandHandler<URI>): WorkspaceRootUriAwareCommandHandler {

packages/workspace/src/browser/workspace-frontend-contribution.ts

@@ -150,7 +150,7 @@ export class WorkspaceFrontendContribution implements CommandContribution, Keybi
             execute: () => this.saveWorkspaceAs()
         });
         commands.registerCommand(WorkspaceCommands.SAVE_AS,
-            new UriAwareCommandHandler(this.selectionService, {
+            UriAwareCommandHandler.MonoSelect(this.selectionService, {
                 execute: (uri: URI) => this.saveAs(uri),
             }));
     }