Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tree handler #3396

Merged
merged 23 commits into from Dec 23, 2017
Merged

Tree handler #3396

merged 23 commits into from Dec 23, 2017

Conversation

@afshin
Copy link
Member

@afshin afshin commented Dec 16, 2017

The behavior of this PR will be:

When a user goes to a path that begins with /lab/tree that path will be navigated to by the main file browser. This means that if the path is a directory, the file browser opens that directory. If the path is a file, then the file browser navigates to that subfolder and then the file opens and is placed into the active tab panel.

Any state restoration that needs to happen will happen in addition to the path that is in the URL under /lab/tree/...

The application will then change the URL back to /lab but this change will not be added to history.

share

Fixes #1075
Fixes #2532

Depends on jupyterlab/jupyterlab_server#31

@ellisonbg
Copy link
Contributor

@ellisonbg ellisonbg commented Dec 17, 2017

Excited about this! Ping @yuvipanda @choldgraf @minrk @willingc as this will allow Binder URLs to point to individual files within JupyterLab.

Question: for some files types, it would be very nice to be able to specify which file handler to use in opening the file. Maybe a URL parameters for that?

@choldgraf
Copy link
Contributor

@choldgraf choldgraf commented Dec 17, 2017

very cool! LMK when it's released and we can bump the jlab version + create a binder-examples example

@blink1073
Copy link
Member

@blink1073 blink1073 commented Dec 19, 2017

Turns out this is all we need for index.html:

diff --git a/dev_mode/webpack.config.js b/dev_mode/webpack.config.js
index 80d5ce074..cec22178b 100644
--- a/dev_mode/webpack.config.js
+++ b/dev_mode/webpack.config.js
@@ -109,7 +109,7 @@ module.exports = {
   },
   output: {
     path: path.resolve(buildDir),
-    publicPath: 'lab/static/',
+    publicPath: '{{base_url}}lab/static/',
     filename: '[name].[chunkhash].js'
   },
   module: {

@blink1073
Copy link
Member

@blink1073 blink1073 commented Dec 19, 2017

cf #3422

@blink1073 blink1073 mentioned this pull request Dec 19, 2017
@afshin afshin force-pushed the tree-handler branch 3 times, most recently from 8196dc4 to 4b6d838 Dec 19, 2017
@blink1073
Copy link
Member

@blink1073 blink1073 commented Dec 21, 2017

This change allows us to run jupyter lab path/to/foo.csv:

diff --git a/jupyterlab/extension.py b/jupyterlab/extension.py
index 338816d..e2608ec 100644
--- a/jupyterlab/extension.py
+++ b/jupyterlab/extension.py
@@ -30,7 +30,7 @@ def load_jupyter_server_extension(nbapp):
     """
     # Delay imports to speed up jlpmapp
     from jupyterlab_launcher import add_handlers, LabConfig
-    from notebook.utils import url_path_join as ujoin
+    from notebook.utils import url_path_join as ujoin, url_escape
     from tornado.ioloop import IOLoop
     from .build_handler import build_path, Builder, BuildHandler
     from .commands import (
@@ -82,6 +82,12 @@ def load_jupyter_server_extension(nbapp):
     page_config['token'] = nbapp.token
     page_config['devMode'] = dev_mode

+    if nbapp.file_to_run:
+        relpath = os.path.relpath(nbapp.file_to_run, nbapp.notebook_dir)
+        uri = url_escape(ujoin('/lab/tree', *relpath.split(os.sep)))
+        nbapp.default_url = uri
+        nbapp.file_to_run = ''
+
     if core_mode:
         app_dir = HERE
         logger.info(CORE_NOTE.strip())

@ellisonbg
Copy link
Contributor

@ellisonbg ellisonbg commented Dec 22, 2017

Is this in a working state to try out?

@ellisonbg
Copy link
Contributor

@ellisonbg ellisonbg commented Dec 22, 2017

I did a local test:

  • Opening a file (/lab/tree/notebook.ipynb) worked fine.
  • Opening a directory (/lab/tree/dir) failed as it tried to treat it as a file.
  • Doesn't seem to be populating the history, which is the behavior we talked about.

@afshin
Copy link
Member Author

@afshin afshin commented Dec 22, 2017

@ellisonbg Thanks for pointing that out! I need to discover if the thing you're querying is a file or a folder and I wasn't doing that.

When I fix that, the only thing remaining is adding the context menu option to the main file browser.

@afshin afshin changed the title [wip] Tree handler Tree handler Dec 22, 2017
@ellisonbg
Copy link
Contributor

@ellisonbg ellisonbg commented Dec 22, 2017

It will probably be common to have users type URLs - right now if there is a typo, and a Tornado handler doesn't capture it, the notebook server shows a generic 404 page. We should probably add a generic /lab/* handler that is last and that just redirects to /lab`.

Copy link
Contributor

@ellisonbg ellisonbg left a comment

Talks to Darian earlier today and did a local test of everything. Everything we talked about is working - this is amazing - even opening a file from the command line works!

I think this is a solid, but conservation offering for the beta. One thing that Darian and I talked about is to eventually add a global command that copies a sharable link into the clipboard for the active document. The logic is a but subtle as the command first has to ask the default file browser if the file is managed by it.

This would allow us to add a "Copy Sharable Link" to the File menu and the context menu of each documents dock panel tab.

@afshin
Copy link
Member Author

@afshin afshin commented Dec 22, 2017

@ellisonbg re: generic /lab/* handler. We talked about this a little bit and it's something we could consider, but we should do it with some thought about the HTTP response code that comes back. So even if we serve the main index and the application works, we might still want to report a 404 back to the client, etc.

Like all of these URL things, our choices might cast a long shadow and it's good to deliberate about the ramifications.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Dec 23, 2017

💥

@blink1073 blink1073 merged commit 658f0e7 into jupyterlab:master Dec 23, 2017
2 checks passed
@blink1073 blink1073 mentioned this pull request Dec 23, 2017
1 task
@sputnik62
Copy link

@sputnik62 sputnik62 commented Dec 31, 2017

This is fantastic! Exactly the behavior we have been looking for to enable wider use of Jupyter Lab in http://labs.CognitiveClass.ai

@afshin afshin deleted the tree-handler branch Feb 9, 2018
mpkouznetsov pushed a commit to dominodatalab/workspace-configs that referenced this issue Apr 5, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Aug 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

5 participants