Address review comments

- remove checks for files against workspace URI - update variable name to a more representative one - Simplify algorithm for single selected URI case - update signatures to indicate an array of files or folders can be opened rather than a single element
eclipse-theia · Sep 26, 2023 · 8f60558 · 8f60558
1 parent 7e8f500
commit 8f60558
Showing 1 changed file with 41 additions and 50 deletions.
diff --git a/packages/workspace/src/browser/workspace-frontend-contribution.ts b/packages/workspace/src/browser/workspace-frontend-contribution.ts
@@ -262,85 +262,76 @@ export class WorkspaceFrontendContribution implements CommandContribution, Keybi
      * This is the generic `Open` method. Opens files and directories too. Resolves to the opened URI.
      * Except when you are on either Windows or Linux `AND` running in electron. If so, it opens a file.
      */
-    protected async doOpen(): Promise<MaybeArray<URI> | undefined> {
+    protected async doOpen(): Promise<URI[] | undefined> {
         if (!isOSX && this.isElectron()) {
             return this.doOpenFile();
         }
         const [rootStat] = await this.workspaceService.roots;
-        const destinationUri = await this.fileDialogService.showOpenDialog({
+        let selectedUris = await this.fileDialogService.showOpenDialog({
             title: WorkspaceCommands.OPEN.dialogLabel,
             canSelectFolders: true,
             canSelectFiles: true,
             canSelectMany: true
         }, rootStat);
-        if (destinationUri) {
-            if (Array.isArray(destinationUri)) {
-                // only open files then open all folders in a new workspace, as done with Electron see doOpenFolder
-                const folders: MaybeArray<URI> = [];
-                for (const uri of destinationUri) {
+        if (selectedUris) {
+            if (!Array.isArray(selectedUris)) {
+                selectedUris = [selectedUris];
+            }
+            const folders: URI[] = [];
+            //  Only open files then open all folders in a new workspace, as done with Electron see doOpenFolder.
+            for (const uri of selectedUris) {
+                const destination = await this.fileService.resolve(uri);
+                if (destination.isDirectory) {
                     if (this.getCurrentWorkspaceUri()?.toString() !== uri.toString()) {
-                        const destination = await this.fileService.resolve(uri);
-                        if (destination.isDirectory) {
-                            folders.push(uri);
-                        } else {
-                            await open(this.openerService, uri);
-                        }
-                    }
-                }
-                if (folders.length > 0) {
-                    const openableURI = await this.getOpenableWorkspaceUri(folders);
-                    if (openableURI && (!this.workspaceService.workspace || !openableURI.isEqual(this.workspaceService.workspace.resource))) {
-                        this.workspaceService.open(openableURI);
+                        folders.push(uri);
                     }
+                } else {
+                    await open(this.openerService, uri);
                 }
-
-                return destinationUri;
-            } else {
-                return this.doOpenFileOrFolder(destinationUri, true);
             }
-        }
-        return undefined;
-    }
-
-    protected async doOpenFileOrFolder(uri: URI, openFolders: boolean): Promise<URI> {
-        if (this.getCurrentWorkspaceUri()?.toString() !== uri.toString()) {
-            const destination = await this.fileService.resolve(uri);
-            if (destination.isDirectory) {
-                if (openFolders) {
-                    this.workspaceService.open(uri);
+            if (folders.length > 0) {
+                const openableURI = await this.getOpenableWorkspaceUri(folders);
+                if (openableURI && (!this.workspaceService.workspace || !openableURI.isEqual(this.workspaceService.workspace.resource))) {
+                    this.workspaceService.open(openableURI);
                 }
-            } else {
-                await open(this.openerService, uri);
             }
+
+            return selectedUris;
         }
-        return uri;
+        return undefined;
     }
 
     /**
-     * Opens a file after prompting the `Open File` dialog. Resolves to `undefined`, if
+     * Opens a set of files after prompting the `Open File` dialog. Resolves to `undefined`, if
      *  - the workspace root is not set,
      *  - the file to open does not exist, or
      *  - it was not a file, but a directory.
      *
-     * Otherwise, resolves to the URI of the file.
+     * Otherwise, resolves to the set of URIs of the files.
      */
-    protected async doOpenFile(): Promise<MaybeArray<URI> | undefined> {
+    protected async doOpenFile(): Promise<URI[] | undefined> {
         const props: OpenFileDialogProps = {
             title: WorkspaceCommands.OPEN_FILE.dialogLabel,
             canSelectFolders: false,
             canSelectFiles: true,
             canSelectMany: true
         };
         const [rootStat] = await this.workspaceService.roots;
-        const destinationFileUri: MaybeArray<URI> | undefined = await this.fileDialogService.showOpenDialog(props, rootStat);
-        if (Array.isArray(destinationFileUri)) {
+        let selectedFilesUris: MaybeArray<URI> | undefined = await this.fileDialogService.showOpenDialog(props, rootStat);
+        if (selectedFilesUris) {
+            if (!Array.isArray(selectedFilesUris)) {
+                selectedFilesUris = [selectedFilesUris];
+            }
+
             const result = [];
-            for (const uri of destinationFileUri) {
-                result.push(await this.doOpenFileOrFolder(uri, false));
+            for (const uri of selectedFilesUris) {
+                const destination = await this.fileService.resolve(uri);
+                if (destination.isFile) {
+                    await open(this.openerService, uri);
+                    result.push(uri);
+                }
             }
             return result;
-        } else if (destinationFileUri) {
-            return this.doOpenFileOrFolder(destinationFileUri, false);
         }
         return undefined;
     }
@@ -364,11 +355,11 @@ export class WorkspaceFrontendContribution implements CommandContribution, Keybi
         const [rootStat] = await this.workspaceService.roots;
         const targetFolders = await this.fileDialogService.showOpenDialog(props, rootStat);
         if (targetFolders) {
-            const openableURI = await this.getOpenableWorkspaceUri(targetFolders);
-            if (openableURI) {
-                if (!this.workspaceService.workspace || !openableURI.isEqual(this.workspaceService.workspace.resource)) {
-                    this.workspaceService.open(openableURI);
-                    return openableURI;
+            const openableUri = await this.getOpenableWorkspaceUri(targetFolders);
+            if (openableUri) {
+                if (!this.workspaceService.workspace || !openableUri.isEqual(this.workspaceService.workspace.resource)) {
+                    this.workspaceService.open(openableUri);
+                    return openableUri;
                 }
             };
         }