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

Create JupyterFrontEnd class. #5845

Merged
merged 62 commits into from Jan 29, 2019
Merged

Create JupyterFrontEnd class. #5845

merged 62 commits into from Jan 29, 2019

Conversation

@afshin
Copy link
Member

@afshin afshin commented Jan 8, 2019

This creates a new JupyterFrontEnd application class that any Jupyter front-end that wants the benefit of interacting with the plugin ecosystem can inherit from. The JupyterLab class inherits from JupyterFrontEnd. The additional functionality of JupyterLab is entirely moved into standalone plugins.

In addition, this PR represents an audit of the core JupyterLab plugins, now each known as a JupyterFrontEndPlugin (the change in nomenclature is to reflect that plugins are not specifically tied to JupyterLab any more). This audit is not exhaustive and there may be further relaxations of the hard dependencies a plugin has.

Some candidates for requirements that may be made optional are: setting registry, main menu, command palette, layout restoration, main menu, and launcher functionality, all of which might not be "mission-critical" to a particular plugin.

NB: This is not a prescriptive list, one plugin may require the setting registry while another might simply use it as an enhancement. This PR is merely a starting point for slimming down the hard requirements of plugins.

One prominent example of this audit is the INotebookTracker provider plugin, which used to look like this:

{
  id: '@jupyterlab/notebook-extension:tracker',
  provides: INotebookTracker,
  requires: [
    IMainMenu,
    ICommandPalette,
    NotebookPanel.IContentFactory,
    IEditorServices,
    ILayoutRestorer,
    IRenderMimeRegistry,
    ISettingRegistry
  ],
  optional: [IFileBrowserFactory, ILauncher],
  activate: activateNotebookHandler,
  autoStart: true
}

And now the INotebookTracker provider plugin looks like this:

{
  id: '@jupyterlab/notebook-extension:tracker',
  provides: INotebookTracker,
  requires: [
    NotebookPanel.IContentFactory,
    IEditorServices,
    IRenderMimeRegistry
  ],
  optional: [
    ICommandPalette,
    IFileBrowserFactory,
    ILauncher,
    ILayoutRestorer,
    IMainMenu,
    ISettingRegistry
  ],
  activate: activateNotebookHandler,
  autoStart: true
}
@yuvipanda
Copy link
Contributor

@yuvipanda yuvipanda commented Jan 8, 2019

<3 awesome!

#3974 is also probably relevant here. Should / Can INotebookTracker be here?

@afshin
Copy link
Member Author

@afshin afshin commented Jan 8, 2019

The plugin that created the notebook tracker also imports a ton of other plugins:

const trackerPlugin: JupyterLabPlugin<INotebookTracker> = {
  id: '@jupyterlab/notebook-extension:tracker',
  provides: INotebookTracker,
  requires: [
    IMainMenu,
    ICommandPalette,
    NotebookPanel.IContentFactory,
    IEditorServices,
    ILayoutRestorer,
    IRenderMimeRegistry,
    ISettingRegistry
  ],
  optional: [IFileBrowserFactory, ILauncher],
  activate: activateNotebookHandler,
  autoStart: true
};

So it might be the case that it's doing too many things. This PR gives us a tool to make some plugins be more broadly usable, but they might require separating out lab-specific functions into some exports and client-agnostic plugins into other exports.

@afshin afshin force-pushed the jupyter-client branch 2 times, most recently from 57974fc to f348fc8 Jan 29, 2019
@afshin afshin force-pushed the jupyter-client branch from f348fc8 to 750eda9 Jan 29, 2019
@afshin afshin changed the title [WIP] Create JupyterFrontEnd class. Create JupyterFrontEnd class. Jan 29, 2019
@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented Jan 29, 2019

Thanks for the updates @afshin, This looks ready from my perspective!

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Jan 29, 2019

@afshin - I'm still working with Andrew on #5795, and can help him rebase that on top of this. If this is ready to go in, I think we should merge it and not wait on #5795.

@yuvipanda
Copy link
Contributor

@yuvipanda yuvipanda commented Jan 29, 2019

<3 \o/ <3

@afshin
Copy link
Member Author

@afshin afshin commented Jan 29, 2019

Thanks! Yes, this is ready to go.

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Jan 29, 2019

@ian-r-rose - sounds like you're the one to press the green button, since you reviewed.

@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented Jan 29, 2019

Okay, cool. I am going to take one last look+test-drive. (I am also hopping on a call soon, so it may be after that).

@afshin
Copy link
Member Author

@afshin afshin commented Jan 29, 2019

@jasongrout just a note on conventions when updating #5795. In this PR, when the generic JupyterFrontEnd.IShell is used, it is always either app.shell or a variable named shell. When the JupyterLab-specific functionality of our shell is needed by importing the ILabShelll token, that argument is always called labShell: ILabShell in order to visually indicate that this is not the standard JupyterFrontEnd shell.

For most plugins, it was sufficient to simply use app.shell.add(...) and the ILabShell really wasn't necessary. The plugins that need ILabShell import it because they need layoutModified or saveLayout/restoreLayout.

CommandIDs.closeAllFiles,
CommandIDs.closeOtherTabs,
CommandIDs.closeRightTabs,
CommandIDs.toggleAutosave
Copy link
Member

@ian-r-rose ian-r-rose Jan 29, 2019

toggleAutosave and close are duplicated in the above palette additions and here.

Copy link
Member Author

@afshin afshin Jan 29, 2019

Oh good catch. I can't fix that right this second. If you feel comfortable doing so, please do. If not, I'll be able to later this evening.

Copy link
Member

@ian-r-rose ian-r-rose Jan 29, 2019

Sure, I can fix it.

Copy link
Member Author

@afshin afshin Jan 29, 2019

Cheers!

@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented Jan 29, 2019

Other than that, this looks good to me.

@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented Jan 29, 2019

Here we go, thanks @afshin!

@ian-r-rose ian-r-rose merged commit 0c68ae2 into jupyterlab:master Jan 29, 2019
1 of 3 checks passed
@yuvipanda
Copy link
Contributor

@yuvipanda yuvipanda commented Jan 29, 2019

This is awesome! thank you all very much for pushing this through!

@lock lock bot locked as resolved and limited conversation to collaborators Aug 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.