Merge pull request #6561 from tslaton/open-dir-from-path

feat: "Open From Path..." works on directories

packages/application-extension/src/index.tsx

@@ -276,7 +276,7 @@ const tree: JupyterFrontEndPlugin<void> = {
         router.navigate(url);
 
         try {
-          await commands.execute('filebrowser:navigate', { path });
+          await commands.execute('filebrowser:open-path', { path });
         } catch (error) {
           console.warn('Tree routing failed.', error);
         }

packages/apputils/src/dialog.ts

@@ -50,7 +50,7 @@ export function showErrorMessage(
 
   // Cache promises to prevent multiple copies of identical dialogs showing
   // to the user.
-  let body = error.message || title;
+  let body = typeof error === 'string' ? error : error.message;
   let key = title + '----' + body;
   let promise = Private.errorMessagePromiseCache.get(key);
   if (promise) {
@@ -456,7 +456,7 @@ export namespace Dialog {
     host: HTMLElement;
 
     /**
-     * The to buttons to display. Defaults to cancel and accept buttons.
+     * The buttons to display. Defaults to cancel and accept buttons.
      */
     buttons: ReadonlyArray<IButton>;
 

packages/apputils/src/inputdialog.ts

@@ -36,6 +36,16 @@ export namespace InputDialog {
      * default renderer.
      */
     renderer?: Dialog.IRenderer;
+
+    /**
+     * Label for ok button.
+     */
+    okLabel?: string;
+
+    /**
+     * Label for cancel button.
+     */
+    cancelLabel?: string;
   }
 
   /**
@@ -61,7 +71,10 @@ export namespace InputDialog {
     return showDialog({
       ...options,
       body: new InputBooleanDialog(options),
-      buttons: [Dialog.cancelButton(), Dialog.okButton()],
+      buttons: [
+        Dialog.cancelButton({ label: options.cancelLabel }),
+        Dialog.okButton({ label: options.okLabel })
+      ],
       focusNodeSelector: 'input'
     });
   }
@@ -89,7 +102,10 @@ export namespace InputDialog {
     return showDialog({
       ...options,
       body: new InputNumberDialog(options),
-      buttons: [Dialog.cancelButton(), Dialog.okButton()],
+      buttons: [
+        Dialog.cancelButton({ label: options.cancelLabel }),
+        Dialog.okButton({ label: options.okLabel })
+      ],
       focusNodeSelector: 'input'
     });
   }
@@ -132,7 +148,10 @@ export namespace InputDialog {
     return showDialog({
       ...options,
       body: new InputItemsDialog(options),
-      buttons: [Dialog.cancelButton(), Dialog.okButton()],
+      buttons: [
+        Dialog.cancelButton({ label: options.cancelLabel }),
+        Dialog.okButton({ label: options.okLabel })
+      ],
       focusNodeSelector: options.editable ? 'input' : 'select'
     });
   }
@@ -164,7 +183,10 @@ export namespace InputDialog {
     return showDialog({
       ...options,
       body: new InputTextDialog(options),
-      buttons: [Dialog.cancelButton(), Dialog.okButton()],
+      buttons: [
+        Dialog.cancelButton({ label: options.cancelLabel }),
+        Dialog.okButton({ label: options.okLabel })
+      ],
       focusNodeSelector: 'input'
     });
   }

packages/docmanager-extension/src/index.ts

@@ -23,7 +23,6 @@ import { IChangedArgs, ISettingRegistry, Time } from '@jupyterlab/coreutils';
 
 import {
   renameDialog,
-  getOpenPath,
   DocumentManager,
   IDocumentManager,
   PathStatus,
@@ -54,8 +53,6 @@ namespace CommandIDs {
 
   export const openBrowserTab = 'docmanager:open-browser-tab';
 
-  export const openDirect = 'docmanager:open-direct';
-
   export const reload = 'docmanager:reload';
 
   export const rename = 'docmanager:rename';
@@ -368,34 +365,6 @@ function addCommands(
     label: () => 'Open in New Browser Tab'
   });
 
-  commands.addCommand(CommandIDs.openDirect, {
-    label: () => 'Open From Path...',
-    caption: 'Open from path',
-    isEnabled: () => true,
-    execute: () => {
-      return getOpenPath(docManager.services.contents).then(path => {
-        if (!path) {
-          return;
-        }
-        docManager.services.contents.get(path, { content: false }).then(
-          args => {
-            // exists
-            return commands.execute(CommandIDs.open, { path: path });
-          },
-          () => {
-            // does not exist
-            return showDialog({
-              title: 'Cannot open',
-              body: 'File not found',
-              buttons: [Dialog.okButton()]
-            });
-          }
-        );
-        return;
-      });
-    }
-  });
-
   commands.addCommand(CommandIDs.reload, {
     label: () =>
       `Reload ${fileType(shell.currentWidget, docManager)} from Disk`,
@@ -549,7 +518,6 @@ function addCommands(
 
   if (palette) {
     [
-      CommandIDs.openDirect,
       CommandIDs.reload,
       CommandIDs.restoreCheckpoint,
       CommandIDs.save,
@@ -636,7 +604,7 @@ function addLabCommands(
 
       // 'activate' is needed if this command is selected in the "open tabs" sidebar
       await commands.execute('filebrowser:activate', { path: context.path });
-      await commands.execute('filebrowser:navigate', { path: context.path });
+      await commands.execute('filebrowser:go-to-path', { path: context.path });
     }
   });
 

packages/docmanager/src/dialogs.ts

@@ -145,49 +145,6 @@ class RenameHandler extends Widget {
   }
 }
 
-/*
- * A widget used to open a file directly.
- */
-class OpenDirectWidget extends Widget {
-  /**
-   * Construct a new open file widget.
-   */
-  constructor() {
-    super({ node: Private.createOpenNode() });
-  }
-
-  /**
-   * Get the value of the widget.
-   */
-  getValue(): string {
-    return this.inputNode.value;
-  }
-
-  /**
-   * Get the input text node.
-   */
-  get inputNode(): HTMLInputElement {
-    return this.node.getElementsByTagName('input')[0] as HTMLInputElement;
-  }
-}
-
-/**
- * Create the node for the open handler.
- */
-export function getOpenPath(contentsManager: any): Promise<string | undefined> {
-  return showDialog({
-    title: 'Open File',
-    body: new OpenDirectWidget(),
-    buttons: [Dialog.cancelButton(), Dialog.okButton({ label: 'Open' })],
-    focusNodeSelector: 'input'
-  }).then((result: any) => {
-    if (result.button.label === 'Open') {
-      return result.value;
-    }
-    return;
-  });
-}
-
 /**
  * A namespace for private data.
  */
@@ -213,21 +170,4 @@ namespace Private {
     body.appendChild(name);
     return body;
   }
-
-  /**
-   * Create the node for a open widget.
-   */
-  export function createOpenNode(): HTMLElement {
-    let body = document.createElement('div');
-    let existingLabel = document.createElement('label');
-    existingLabel.textContent = 'File Path:';
-
-    let input = document.createElement('input');
-    input.value = '';
-    input.placeholder = '/path/to/file';
-
-    body.appendChild(existingLabel);
-    body.appendChild(input);
-    return body;
-  }
 }

packages/docmanager/src/manager.ts

@@ -412,8 +412,11 @@ export class DocumentManager implements IDocumentManager {
     path: string,
     factoryName: string
   ): Private.IContext | undefined {
+    const normalizedPath = this.services.contents.normalize(path);
     return find(this._contexts, context => {
-      return context.path === path && context.factoryName === factoryName;
+      return (
+        context.path === normalizedPath && context.factoryName === factoryName
+      );
     });
   }
 
@@ -426,7 +429,8 @@ export class DocumentManager implements IDocumentManager {
    * notebook model factory and a text model factory).
    */
   private _contextsForPath(path: string): Private.IContext[] {
-    return this._contexts.filter(context => context.path === path);
+    const normalizedPath = this.services.contents.normalize(path);
+    return this._contexts.filter(context => context.path === normalizedPath);
   }
 
   /**

packages/docregistry/src/context.ts

@@ -47,7 +47,7 @@ export class Context<T extends DocumentRegistry.IModel>
     let manager = (this._manager = options.manager);
     this._factory = options.factory;
     this._opener = options.opener || Private.noOp;
-    this._path = options.path;
+    this._path = this._manager.contents.normalize(options.path);
     const localPath = this._manager.contents.localPath(this._path);
     let lang = this._factory.preferredLanguage(PathExt.basename(localPath));
 

packages/filebrowser-extension/src/index.ts

@@ -13,7 +13,9 @@ import {
   MainAreaWidget,
   ToolbarButton,
   WidgetTracker,
-  ICommandPalette
+  ICommandPalette,
+  InputDialog,
+  showErrorMessage
 } from '@jupyterlab/apputils';
 
 import {
@@ -26,8 +28,6 @@ import {
 
 import { IDocumentManager } from '@jupyterlab/docmanager';
 
-import { IMainMenu } from '@jupyterlab/mainmenu';
-
 import {
   FileBrowserModel,
   FileBrowser,
@@ -37,6 +37,8 @@ import {
 
 import { Launcher } from '@jupyterlab/launcher';
 
+import { IMainMenu } from '@jupyterlab/mainmenu';
+
 import { Contents } from '@jupyterlab/services';
 
 import { IStatusBar } from '@jupyterlab/statusbar';
@@ -71,7 +73,9 @@ namespace CommandIDs {
   // For main browser only.
   export const hideBrowser = 'filebrowser:hide-main';
 
-  export const navigate = 'filebrowser:navigate';
+  export const goToPath = 'filebrowser:go-to-path';
+
+  export const openPath = 'filebrowser:open-path';
 
   export const open = 'filebrowser:open';
 
@@ -311,22 +315,21 @@ function activateBrowser(
       });
 
     // Whether to automatically navigate to a document's current directory
-    labShell.currentChanged.connect((_, change) => {
+    labShell.currentChanged.connect(async (_, change) => {
       if (navigateToCurrentDirectory && change.newValue) {
         const { newValue } = change;
         const context = docManager.contextForWidget(newValue);
         if (context) {
           const { path } = context;
-          Private.navigateToPath(path, factory)
-            .then(() => {
-              labShell.currentWidget.activate();
-            })
-            .catch((reason: any) => {
-              console.warn(
-                `${CommandIDs.navigate} failed to open: ${path}`,
-                reason
-              );
-            });
+          try {
+            await Private.navigateToPath(path, factory);
+            labShell.currentWidget.activate();
+          } catch (reason) {
+            console.warn(
+              `${CommandIDs.goToPath} failed to open: ${path}`,
+              reason
+            );
+          }
         }
       }
     });
@@ -371,10 +374,8 @@ function addCommands(
   commandPalette: ICommandPalette | null,
   mainMenu: IMainMenu | null
 ): void {
-  const registry = app.docRegistry;
-  const { commands } = app;
-  const { defaultBrowser: browser } = factory;
-  const { tracker } = factory;
+  const { docRegistry: registry, commands } = app;
+  const { defaultBrowser: browser, tracker } = factory;
 
   commands.addCommand(CommandIDs.del, {
     execute: () => {
@@ -447,21 +448,75 @@ function addCommands(
     }
   });
 
-  commands.addCommand(CommandIDs.navigate, {
-    execute: args => {
+  commands.addCommand(CommandIDs.goToPath, {
+    execute: async args => {
       const path = (args.path as string) || '';
-      return Private.navigateToPath(path, factory)
-        .then(() => {
-          return commands.execute('docmanager:open', { path });
-        })
-        .catch((reason: any) => {
-          console.warn(
-            `${CommandIDs.navigate} failed to open: ${path}`,
-            reason
-          );
+      try {
+        await Private.navigateToPath(path, factory);
+      } catch (reason) {
+        console.warn(`${CommandIDs.goToPath} failed to go to: ${path}`, reason);
+      }
+      const browserForPath = Private.getBrowserForPath(path, factory);
+      browserForPath.clearSelectedItems();
+      const parts = path.split('/');
+      const name = parts[parts.length - 1];
+      if (name) {
+        await browserForPath.selectItemByName(name);
+      }
+
+      return commands.execute(CommandIDs.showBrowser, { path });
+    }
+  });
+
+  commands.addCommand(CommandIDs.openPath, {
+    label: args => (args.path ? `Open ${args.path}` : 'Open from Path…'),
+    caption: args => (args.path ? `Open ${args.path}` : 'Open from path'),
+    execute: async ({ path }: { path?: string }) => {
+      if (!path) {
+        path = (await InputDialog.getText({
+          label: 'Path',
+          placeholder: '/path/relative/to/jlab/root',
+          title: 'Open Path',
+          okLabel: 'Open'
+        })).value;
+      }
+      if (!path) {
+        return;
+      }
+      try {
+        let trailingSlash = path !== '/' && path.endsWith('/');
+        if (trailingSlash) {
+          // The normal contents service errors on paths ending in slash
+          path = path.slice(0, path.length - 1);
+        }
+        const browserForPath = Private.getBrowserForPath(path, factory);
+        const { services } = browserForPath.model.manager;
+        const item = await services.contents.get(path, {
+          content: false
         });
+        if (trailingSlash && item.type !== 'directory') {
+          throw new Error(`Path ${path}/ is not a directory`);
+        }
+        await commands.execute(CommandIDs.goToPath, { path });
+        if (item.type === 'directory') {
+          return;
+        }
+        return commands.execute('docmanager:open', { path });
+      } catch (reason) {
+        if (reason.response && reason.response.status === 404) {
+          reason.message = `Could not find path: ${path}`;
+        }
+        return showErrorMessage('Cannot open', reason);
+      }
     }
   });
+  // Add the openPath command to the command palette
+  if (commandPalette) {
+    commandPalette.addItem({
+      command: CommandIDs.openPath,
+      category: 'File Operations'
+    });
+  }
 
   commands.addCommand(CommandIDs.open, {
     execute: args => {
@@ -897,7 +952,7 @@ namespace Private {
       if (!browserForPath) {
         // warn that no filebrowser could be found for this driveName
         console.warn(
-          `${CommandIDs.navigate} failed to find filebrowser for path: ${path}`
+          `${CommandIDs.goToPath} failed to find filebrowser for path: ${path}`
         );
         return;
       }
@@ -910,27 +965,25 @@ namespace Private {
   }
 
   /**
-   * Navigate to a path.
+   * Navigate to a path or the path containing a file.
    */
-  export function navigateToPath(
+  export async function navigateToPath(
     path: string,
     factory: IFileBrowserFactory
-  ): Promise<any> {
+  ): Promise<Contents.IModel> {
     const browserForPath = Private.getBrowserForPath(path, factory);
     const { services } = browserForPath.model.manager;
     const localPath = services.contents.localPath(path);
 
-    return services.ready
-      .then(() => services.contents.get(path, { content: false }))
-      .then(value => {
-        const { model } = browserForPath;
-        const { restored } = model;
-
-        if (value.type === 'directory') {
-          return restored.then(() => model.cd(`/${localPath}`));
-        }
-
-        return restored.then(() => model.cd(`/${PathExt.dirname(localPath)}`));
-      });
+    await services.ready;
+    let item = await services.contents.get(path, { content: false });
+    const { model } = browserForPath;
+    await model.restored;
+    if (item.type === 'directory') {
+      await model.cd(`/${localPath}`);
+    } else {
+      await model.cd(`/${PathExt.dirname(localPath)}`);
+    }
+    return item;
   }
 }

packages/filebrowser/src/browser.ts

@@ -122,6 +122,19 @@ export class FileBrowser extends Widget {
     return this._listing.selectedItems();
   }
 
+  /**
+   * Select an item by name.
+   *
+   * @param name - The name of the item to select.
+   */
+  async selectItemByName(name: string) {
+    await this._listing.selectItemByName(name);
+  }
+
+  clearSelectedItems() {
+    this._listing.clearSelectedItems();
+  }
+
   /**
    * Rename the first currently selected item.
    *

packages/filebrowser/src/listing.ts

@@ -558,28 +558,35 @@ export class DirListing extends Widget {
     return undefined;
   }
 
+  /**
+   * Clear the selected items.
+   */
+  clearSelectedItems() {
+    this._selection = Object.create(null);
+  }
+
   /**
    * Select an item by name.
    *
-   * @parem name - The name of the item to select.
+   * @param name - The name of the item to select.
    *
    * @returns A promise that resolves when the name is selected.
    */
-  selectItemByName(name: string): Promise<void> {
+  async selectItemByName(name: string): Promise<void> {
     // Make sure the file is available.
-    return this.model.refresh().then(() => {
-      if (this.isDisposed) {
-        throw new Error('File browser is disposed.');
-      }
-      let items = this._sortedItems;
-      let index = ArrayExt.findFirstIndex(items, value => value.name === name);
-      if (index === -1) {
-        throw new Error('Item does not exist.');
-      }
-      this._selectItem(index, false);
-      MessageLoop.sendMessage(this, Widget.Msg.UpdateRequest);
-      ElementExt.scrollIntoViewIfNeeded(this.contentNode, this._items[index]);
-    });
+    await this.model.refresh();
+
+    if (this.isDisposed) {
+      throw new Error('File browser is disposed.');
+    }
+    let items = this._sortedItems;
+    let index = ArrayExt.findFirstIndex(items, value => value.name === name);
+    if (index === -1) {
+      throw new Error('Item does not exist.');
+    }
+    this._selectItem(index, false);
+    MessageLoop.sendMessage(this, Widget.Msg.UpdateRequest);
+    ElementExt.scrollIntoViewIfNeeded(this.contentNode, this._items[index]);
   }
 
   /**
@@ -872,7 +879,7 @@ export class DirListing extends Widget {
       let altered = event.metaKey || event.shiftKey || event.ctrlKey;
       // See if we need to clear the other selection.
       if (!altered && event.button === 0) {
-        this._selection = Object.create(null);
+        this.clearSelectedItems();
         this._selection[this._softSelection] = true;
         this.update();
       }
@@ -1267,7 +1274,7 @@ export class DirListing extends Widget {
       // Default to selecting the only the item.
     } else {
       // Select only the given item.
-      this._selection = Object.create(null);
+      this.clearSelectedItems();
       this._selection[name] = true;
     }
     this.update();
@@ -1411,7 +1418,7 @@ export class DirListing extends Widget {
     // Selected the given row(s)
     let items = this._sortedItems;
     if (!keepExisting) {
-      this._selection = Object.create(null);
+      this.clearSelectedItems();
     }
     let name = items[index].name;
     this._selection[name] = true;
@@ -1424,7 +1431,7 @@ export class DirListing extends Widget {
   private _onModelRefreshed(): void {
     // Update the selection.
     let existing = Object.keys(this._selection);
-    this._selection = Object.create(null);
+    this.clearSelectedItems();
     each(this._model.items(), item => {
       let name = item.name;
       if (existing.indexOf(name) !== -1) {
@@ -1444,7 +1451,7 @@ export class DirListing extends Widget {
    */
   private _onPathChanged(): void {
     // Reset the selection.
-    this._selection = Object.create(null);
+    this.clearSelectedItems();
     // Update the sorted items.
     this.sort(this.sortState);
   }

packages/mainmenu-extension/src/index.ts

@@ -409,11 +409,12 @@ export function createFileMenu(
     { command: 'filebrowser:create-main-launcher' }
   ];
 
+  const openGroup = [{ command: 'filebrowser:open-path' }];
+
   const newViewGroup = [
     { command: 'docmanager:clone' },
     { command: CommandIDs.createConsole },
-    inspector ? { command: 'inspector:open' } : null,
-    { command: 'docmanager:open-direct' }
+    inspector ? { command: 'inspector:open' } : null
   ].filter(item => !!item);
 
   // Add the close group
@@ -451,10 +452,11 @@ export function createFileMenu(
   const printGroup = [{ command: 'apputils:print' }];
 
   menu.addGroup(newGroup, 0);
-  menu.addGroup(newViewGroup, 1);
-  menu.addGroup(closeGroup, 2);
-  menu.addGroup(saveGroup, 3);
-  menu.addGroup(reGroup, 4);
+  menu.addGroup(openGroup, 1);
+  menu.addGroup(newViewGroup, 2);
+  menu.addGroup(closeGroup, 3);
+  menu.addGroup(saveGroup, 4);
+  menu.addGroup(reGroup, 5);
   menu.addGroup(printGroup, 98);
   if (menu.quitEntry) {
     menu.addGroup(quitGroup, 99);

packages/services/src/contents/index.ts

@@ -231,6 +231,17 @@ export namespace Contents {
      */
     localPath(path: string): string;
 
+    /**
+     * Normalize a global path. Reduces '..' and '.' parts, and removes
+     * leading slashes from the local part of the path, while retaining
+     * the drive name if it exists.
+     *
+     * @param path: the path.
+     *
+     * @returns The normalized path.
+     */
+    normalize(path: string): string;
+
     /**
      * Given a path of the form `drive:local/portion/of/it.txt`
      * get the name of the drive. If the path is missing
@@ -597,6 +608,23 @@ export class ContentsManager implements Contents.IManager {
     return PathExt.join(firstParts.slice(1).join(':'), ...parts.slice(1));
   }
 
+  /**
+   * Normalize a global path. Reduces '..' and '.' parts, and removes
+   * leading slashes from the local part of the path, while retaining
+   * the drive name if it exists.
+   *
+   * @param path: the path.
+   *
+   * @returns The normalized path.
+   */
+  normalize(path: string): string {
+    const parts = path.split(':');
+    if (parts.length === 1) {
+      return PathExt.normalize(path);
+    }
+    return `${parts[0]}:${PathExt.normalize(parts.slice(1).join(':'))}`;
+  }
+
   /**
    * Given a path of the form `drive:local/portion/of/it.txt`
    * get the name of the drive. If the path is missing
@@ -678,7 +706,7 @@ export class ContentsManager implements Contents.IManager {
    */
   newUntitled(options: Contents.ICreateOptions = {}): Promise<Contents.IModel> {
     if (options.path) {
-      let globalPath = Private.normalize(options.path);
+      let globalPath = this.normalize(options.path);
       let [drive, localPath] = this._driveForPath(globalPath);
       return drive
         .newUntitled({ ...options, path: localPath })
@@ -746,7 +774,7 @@ export class ContentsManager implements Contents.IManager {
     path: string,
     options: Partial<Contents.IModel> = {}
   ): Promise<Contents.IModel> {
-    const globalPath = Private.normalize(path);
+    const globalPath = this.normalize(path);
     const [drive, localPath] = this._driveForPath(path);
     return drive
       .save(localPath, { ...options, path: localPath })
@@ -1404,17 +1432,4 @@ namespace Private {
     }
     return extension;
   }
-
-  /**
-   * Normalize a global path. Reduces '..' and '.' parts, and removes
-   * leading slashes from the local part of the path, while retaining
-   * the drive name if it exists.
-   */
-  export function normalize(path: string): string {
-    const parts = path.split(':');
-    if (parts.length === 1) {
-      return PathExt.normalize(path);
-    }
-    return `${parts[0]}:${PathExt.normalize(parts.slice(1).join(':'))}`;
-  }
 }