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

Allow NBClassic to work with NB7 and ServerApp.base_url #165

Merged
merged 2 commits into from
Oct 18, 2022

Conversation

mcrutch
Copy link
Contributor

@mcrutch mcrutch commented Oct 17, 2022

To test, upgrade to Notebook v7 and Lab v4, install this NBClassic, set ServerApp.base_url, run Jupyter, visit nbclassic

Also resolves an issue where New->New Notebook did not add the "/nbclassic/" in the url when Notebook v7 was installed

Personally not a fan of the var still being called base_url_prefix but I didn't want to change that unilaterally.

Closes #164

…s installed

Also resolves an issue where New->New Notebook did not respect the "/nbclassic/" in the url if it was set
Personally not a fan of the var still being called base_url_prefix but wans't willing to make that big of a change
Closes jupyter#164
@echarles
Copy link
Member

Thx a lot for this. Let's see what CI says.

Personally not a fan of the var still being called base_url_prefix

I am not either.

I didn't want to change that unilaterally.

Please do with a nicer name 👍

@mcrutch
Copy link
Contributor Author

mcrutch commented Oct 17, 2022

The concern with going down the path of changing base_url_prefix/document.baseUrlPrefix is that there were already places it was being used in nbclassic to detect running in "nbclassic" versus "Notebook v6" (see the about dialog) so could others have already keyed on that.

otherwise, nbclassic_path and 'document.nbclassicPathare ideas I had for a rename. if it is not running with NBClassic, the path is just '' otherwise it is '/nbclassic' Other option would be just carrying forward the name of the Python functionurl_prefix_notebook`

@echarles echarles added the bug Something isn't working label Oct 17, 2022
@echarles
Copy link
Member

CI seems to be OK

I like nbclassic_path - If you go for it, it would be great to align the Python name also.

Rename base_url_prefix to nbclassic_path
rename baseUrlPrefix to nbclassicPath
rename url_prefix_notebook() to nbclassic_path()
@mcrutch
Copy link
Contributor Author

mcrutch commented Oct 17, 2022

base_url_prefix, baseUrlPrefix and url_prefix_notebook() all updated to be nbclassic_path or nbclassicPath as appropriate

Copy link
Member

@echarles echarles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM Thx @mcrutch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

nbclassic does not work with Notebook v7 if Jupyter server is not running at /
2 participants