Merge pull request #4708 from afshin/tree-handler-revisited

Reconcile tree routing handler  with workspaces.

packages/application-extension/src/index.tsx

@@ -6,7 +6,7 @@ import {
 } from '@jupyterlab/application';
 
 import {
-  Dialog, ICommandPalette, showDialog
+  Dialog, ICommandPalette, IWindowResolver, showDialog, showErrorMessage
 } from '@jupyterlab/apputils';
 
 import {
@@ -54,7 +54,7 @@ namespace CommandIDs {
  */
 namespace Patterns {
   export
-  const tree = /^\/tree\/(.+)/;
+  const tree = /[^?]*(\/tree\/([^?]+))/;
 }
 
 
@@ -63,8 +63,15 @@ namespace Patterns {
  */
 const main: JupyterLabPlugin<void> = {
   id: '@jupyterlab/application-extension:main',
-  requires: [ICommandPalette, IRouter],
-  activate: (app: JupyterLab, palette: ICommandPalette, router: IRouter) => {
+  requires: [ICommandPalette, IRouter, IWindowResolver],
+  activate: (app: JupyterLab, palette: ICommandPalette, router: IRouter, resolver: IWindowResolver) => {
+    // Requiring the window resolver guarantees that the application extension
+    // only loads if there is a viable window name. Otherwise, the application
+    // will short-circuit and ask the user to navigate away.
+    const workspace = resolver.name ? `"${resolver.name}"` : '[default: /lab]';
+
+    console.log(`Starting application in workspace: ${workspace}`);
+
     // If there were errors registering plugins, tell the user.
     if (app.registerPluginErrors.length !== 0) {
       const body = (
@@ -72,13 +79,8 @@ const main: JupyterLabPlugin<void> = {
           {app.registerPluginErrors.map(e => e.message).join('\n')}
         </pre>
       );
-      let options = {
-        title: 'Error Registering Plugins',
-        body,
-        buttons: [Dialog.okButton()],
-        okText: 'DISMISS'
-      };
-      showDialog(options).then(() => { /* no-op */ });
+
+      showErrorMessage('Error Registering Plugins', { message: body });
     }
 
     addCommands(app, palette);
@@ -89,9 +91,8 @@ const main: JupyterLabPlugin<void> = {
       app.commands.notifyCommandChanged();
     });
 
-    let builder = app.serviceManager.builder;
-
-    let doBuild = () => {
+    const builder = app.serviceManager.builder;
+    const build = () => {
       return builder.build().then(() => {
         return showDialog({
           title: 'Build Complete',
@@ -104,37 +105,33 @@ const main: JupyterLabPlugin<void> = {
           router.reload();
         }
       }).catch(err => {
-        showDialog({
-          title: 'Build Failed',
-          body: (<pre>{err.message}</pre>)
-        });
+        showErrorMessage('Build Failed', { message: <pre>{err.message}</pre> });
       });
     };
 
     if (builder.isAvailable && builder.shouldCheck) {
       builder.getStatus().then(response => {
         if (response.status === 'building') {
-          return doBuild();
+          return build();
         }
+
         if (response.status !== 'needed') {
           return;
         }
-        let body = (<div>
+
+        const body = (<div>
           <p>
             JupyterLab build is suggested:
             <br />
             <pre>{response.message}</pre>
           </p>
         </div>);
+
         showDialog({
           title: 'Build Recommended',
           body,
           buttons: [Dialog.cancelButton(), Dialog.okButton({ label: 'BUILD' })]
-        }).then(result => {
-          if (result.button.accept) {
-            return doBuild();
-          }
-        });
+        }).then(result => result.button.accept ? build() : undefined);
       });
     }
 
@@ -191,23 +188,6 @@ const router: JupyterLabPlugin<IRouter> = {
     const base = PageConfig.getOption('pageUrl');
     const router = new Router({ base, commands });
 
-    commands.addCommand(CommandIDs.tree, {
-      execute: (args: IRouter.ILocation) => {
-        const path = decodeURIComponent((args.path.match(Patterns.tree)[1]));
-
-        // File browser navigation waits for the application to be restored.
-        // As a result, this command cannot return a promise because it would
-        // create a circular dependency on the restored promise that would
-        // cause the application to never restore.
-        const opened = commands.execute('filebrowser:navigate-main', { path });
-
-        // Change the URL back to the base application URL without adding the
-        // URL change to the browser history.
-        opened.then(() => { router.navigate('', { silent: true }); });
-      }
-    });
-
-    router.register({ command: CommandIDs.tree, pattern: Patterns.tree });
     app.started.then(() => {
       // Route the very first request on load.
       router.route();
@@ -223,6 +203,37 @@ const router: JupyterLabPlugin<IRouter> = {
 };
 
 
+/**
+ * The tree route handler provider.
+ */
+const tree: JupyterLabPlugin<void> = {
+  id: '@jupyterlab/application-extension:tree',
+  autoStart: true,
+  requires: [IRouter],
+  activate: (app: JupyterLab, router: IRouter) => {
+    const { commands } = app;
+
+    commands.addCommand(CommandIDs.tree, {
+      execute: (args: IRouter.ILocation) => {
+        const { request } = args;
+        const path = decodeURIComponent((args.path.match(Patterns.tree)[2]));
+        const url = request.replace(request.match(Patterns.tree)[1], '');
+        const immediate = true;
+
+        // Silently remove the tree portion of the URL leaving the rest intact.
+        router.navigate(url, { silent: true });
+
+        return commands.execute('filebrowser:navigate-main', { path })
+          .then(() => commands.execute('apputils:save-statedb', { immediate }))
+          .catch(reason => { console.warn(`Tree routing failed:`, reason); });
+      }
+    });
+
+    router.register({ command: CommandIDs.tree, pattern: Patterns.tree });
+  }
+};
+
+
 /**
  * The default URL not found extension.
  */
@@ -231,6 +242,9 @@ const notfound: JupyterLabPlugin<void> = {
   activate: (app: JupyterLab, router: IRouter) => {
     const bad = PageConfig.getOption('notFoundUrl');
     const base = router.base;
+    const message = `
+      The path: ${bad} was not found. JupyterLab redirected to: ${base}
+    `;
 
     if (!bad) {
       return;
@@ -240,11 +254,7 @@ const notfound: JupyterLabPlugin<void> = {
     // URL change to the browser history.
     router.navigate('', { silent: true });
 
-    showDialog({
-      title: 'Path Not Found',
-      body: `The path: ${bad} was not found. JupyterLab redirected to: ${base}`,
-      buttons: [Dialog.okButton()]
-    });
+    showErrorMessage('Path Not Found', { message });
   },
   requires: [IRouter],
   autoStart: true
@@ -375,6 +385,8 @@ function addCommands(app: JupyterLab, palette: ICommandPalette): void {
 /**
  * Export the plugins as default.
  */
-const plugins: JupyterLabPlugin<any>[] = [main, layout, router, notfound, busy];
+const plugins: JupyterLabPlugin<any>[] = [
+  main, layout, router, tree, notfound, busy
+];
 
 export default plugins;

packages/apputils-extension/src/index.ts

@@ -13,7 +13,8 @@ import {
 } from '@jupyterlab/apputils';
 
 import {
-  DataConnector, ISettingRegistry, IStateDB, SettingRegistry, StateDB, URLExt
+  DataConnector, ISettingRegistry, IStateDB, PageConfig, SettingRegistry,
+  StateDB, URLExt
 } from '@jupyterlab/coreutils';
 
 import {
@@ -93,10 +94,10 @@ namespace CommandIDs {
  */
 namespace Patterns {
   export
-  const cloneState = /(\?clone\=|\&clone\=)([^&]+)($|&)/;
+  const cloneState = /[?&]clone([=&]|$)/;
 
   export
-  const loadState = /^\/workspaces\/([^?]+)/;
+  const loadState = /^\/workspaces\/([^?\/]+)/;
 
   export
   const resetOnLoad = /(\?reset|\&reset)($|&)/;
@@ -273,7 +274,11 @@ const resolver: JupyterLabPlugin<IWindowResolver> = {
 
         return Private.redirect(router);
       })
-      .then(() => resolver);
+      .then(() => {
+        PageConfig.setOption('workspace', resolver.name);
+
+        return resolver;
+      });
   }
 };
 
@@ -304,8 +309,8 @@ const state: JupyterLabPlugin<IStateDB> = {
   id: '@jupyterlab/apputils-extension:state',
   autoStart: true,
   provides: IStateDB,
-  requires: [IRouter, IWindowResolver],
-  activate: (app: JupyterLab, router: IRouter, resolver: IWindowResolver) => {
+  requires: [IRouter, IWindowResolver, ISplashScreen],
+  activate: (app: JupyterLab, router: IRouter, resolver: IWindowResolver, splash: ISplashScreen) => {
     let debouncer: number;
     let resolved = false;
 
@@ -447,9 +452,13 @@ const state: JupyterLabPlugin<IStateDB> = {
             delete query['clone'];
 
             const url = path + URLExt.objectToQueryString(query) + hash;
-            const silent = true;
+            const cloned = commands.execute(CommandIDs.saveState, { immediate })
+              .then(() => router.stop);
 
-            router.navigate(url, { silent });
+            // After the state has been cloned, navigate to the URL.
+            cloned.then(() => { router.navigate(url, { silent: true }); });
+
+            return cloned;
           }
 
           // After the state database has finished loading, save it.
@@ -458,12 +467,15 @@ const state: JupyterLabPlugin<IStateDB> = {
       }
     });
     // Both the load state and clone state patterns should trigger the load
-    // state command if the URL matches one of them.
+    // state command if the URL matches one of them, but cloning a workspace
+    // outranks loading it because it is an explicit user action.
     router.register({
-      command: CommandIDs.loadState, pattern: Patterns.loadState
+      command: CommandIDs.loadState, pattern: Patterns.cloneState,
+      rank: 20 // Set loading rank at a higher priority than the default 100.
     });
     router.register({
-      command: CommandIDs.loadState, pattern: Patterns.cloneState
+      command: CommandIDs.loadState, pattern: Patterns.loadState,
+      rank: 30 // Set loading rank at a higher priority than the default 100.
     });
 
     commands.addCommand(CommandIDs.reset, {
@@ -480,11 +492,14 @@ const state: JupyterLabPlugin<IStateDB> = {
         const { hash, path, search } = args;
         const query = URLExt.queryStringToObject(search || '');
         const reset = 'reset' in query;
+        const clone = 'clone' in query;
 
         if (!reset) {
           return;
         }
 
+        const loading = splash.show();
+
         // If the state database has already been resolved, resetting is
         // impossible without reloading.
         if (resolved) {
@@ -498,12 +513,21 @@ const state: JupyterLabPlugin<IStateDB> = {
         // Maintain the query string parameters but remove `reset`.
         delete query['reset'];
 
+        const silent = true;
+        const hard = true;
         const url = path + URLExt.objectToQueryString(query) + hash;
         const cleared = commands.execute(CommandIDs.recoverState)
           .then(() => router.stop); // Stop routing before new route navigation.
 
         // After the state has been reset, navigate to the URL.
-        cleared.then(() => { router.navigate(url, { silent: true }); });
+        if (clone) {
+          cleared.then(() => { router.navigate(url, { silent, hard }); });
+        } else {
+          cleared.then(() => {
+            router.navigate(url, { silent });
+            loading.dispose();
+          });
+        }
 
         return cleared;
       }

packages/apputils/src/dialog.ts

@@ -51,14 +51,13 @@ function showDialog<T>(options: Partial<Dialog.IOptions<T>>={}): Promise<Dialog.
  */
 export
 function showErrorMessage(title: string, error: any): Promise<void> {
-  console.error(error);
-  let options = {
+  console.warn('Showing error:', error);
+
+  return showDialog({
     title: title,
     body: error.message || title,
-    buttons: [Dialog.okButton()],
-    okText: 'DISMISS'
-  };
-  return showDialog(options).then(() => { /* no-op */ });
+    buttons: [Dialog.okButton({ label: 'DISMISS' })]
+  }).then(() => { /* no-op */ });
 }
 
 /**

packages/coreutils/src/pageconfig.ts

@@ -24,6 +24,17 @@ declare var require: any;
  */
 export
 namespace PageConfig {
+  /**
+   * The tree URL construction options.
+   */
+  export
+  interface ITreeOptions {
+    /**
+     * If `true`, the tree URL will include the current workspace, if any.
+     */
+    workspace?: boolean;
+  }
+
   /**
    * Get global configuration data for the Jupyter application.
    *
@@ -121,10 +132,22 @@ namespace PageConfig {
 
   /**
    * Get the tree url for a JupyterLab application.
+   *
+   * @param options - The tree URL construction options.
    */
   export
-  function getTreeUrl(): string {
-    return URLExt.join(getBaseUrl(), getOption('pageUrl'), 'tree');
+  function getTreeUrl(options: ITreeOptions = { }): string {
+    const base = getBaseUrl();
+    const page = getOption('pageUrl');
+    const workspaces = getOption('workspacesUrl');
+    const workspace = getOption('workspace');
+    const includeWorkspace = !!options.workspace;
+
+    if (includeWorkspace && workspace) {
+      return URLExt.join(base, workspaces, workspace, 'tree');
+    } else {
+      return URLExt.join(base, page, 'tree');
+    }
   }
 
   /**

packages/filebrowser-extension/src/index.ts

@@ -400,7 +400,7 @@ function addCommands(app: JupyterLab, tracker: InstanceTracker<FileBrowser>, bro
   commands.addCommand(CommandIDs.share, {
     execute: () => {
       const path = encodeURIComponent(browser.selectedItems().next().path);
-      const tree = PageConfig.getTreeUrl();
+      const tree = PageConfig.getTreeUrl({ workspace: true });
 
       Clipboard.copyToSystem(URLExt.join(tree, path));
     },