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 main app plugin to work without palette #7385

Merged
merged 5 commits into from
Jan 14, 2020

Conversation

vidartf
Copy link
Member

@vidartf vidartf commented Oct 17, 2019

Code changes

Changes the dependency of various plugins on the Command palette from required to optional.

User-facing changes

Users can now disable the command palette without causing other core plugins to fail to activate.

Backwards-incompatible changes

Some of the plugins' activate function changes signature. This should probably be considered backwards incompatible, but could be considered non-breaking as well if we insist on the signature being dynamically defined by the required/optional keys.

@jupyterlab-dev-mode
Copy link

Thanks for making a pull request to JupyterLab!

To try out this branch on binder, follow this link: Binder

@jasongrout
Copy link
Contributor

This should probably be considered backwards incompatible

So 2.0?

@jasongrout jasongrout added this to the 2.0 milestone Oct 17, 2019
@vidartf
Copy link
Member Author

vidartf commented Oct 18, 2019

So 2.0?

I wrote it that way because I wanted to have a discussion about this 😃

@afshin
Copy link
Member

afshin commented Oct 24, 2019

I think maybe the if (palette) {...} statements should all be consolidated into one.

We don't need to block on this, but this PR does need a rebase.

@blink1073
Copy link
Member

@declanvk, look, auto-labels!

Copy link
Member

@afshin afshin left a comment

Choose a reason for hiding this comment

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

Thank you!

@afshin
Copy link
Member

afshin commented Jan 14, 2020

The CI integrity check failure is legit. You just need to run jlpm run lint and check in the changes. The console-extension and hub-extension index files need to be linted:

diff --git a/packages/console-extension/src/index.ts b/packages/console-extension/src/index.ts
index 22c14004b..655f4f8a9 100644
--- a/packages/console-extension/src/index.ts
+++ b/packages/console-extension/src/index.ts
@@ -104,7 +104,13 @@ const tracker: JupyterFrontEndPlugin<IConsoleTracker> = {
     IRenderMimeRegistry,
     ISettingRegistry
   ],
-  optional: [IMainMenu, ICommandPalette, ILauncher, ILabStatus, ISessionContextDialogs],
+  optional: [
+    IMainMenu,
+    ICommandPalette,
+    ILauncher,
+    ILabStatus,
+    ISessionContextDialogs
+  ],
   activate: activateConsole,
   autoStart: true
 };
@@ -650,7 +656,7 @@ async function activateConsole(
     // Add kernel information to the application help menu.
     mainMenu.helpMenu.kernelUsers.add({
       tracker,
-    getKernel: current => current.sessionContext.session?.kernel
+      getKernel: current => current.sessionContext.session?.kernel
     } as IHelpMenu.IKernelUser<ConsolePanel>);
   }

diff --git a/packages/hub-extension/src/index.ts b/packages/hub-extension/src/index.ts
index 864071c73..5c01e9944 100644
--- a/packages/hub-extension/src/index.ts
+++ b/packages/hub-extension/src/index.ts
@@ -36,7 +36,7 @@ function activateHubExtension(
   app: JupyterFrontEnd,
   paths: JupyterFrontEnd.IPaths,
   palette: ICommandPalette | null,
-  mainMenu: IMainMenu | null,
+  mainMenu: IMainMenu | null
 ): void {
   const hubHost = paths.urls.hubHost || '';
   const hubPrefix = paths.urls.hubPrefix || '';

@afshin
Copy link
Member

afshin commented Jan 14, 2020

Er, I just pushed those changed, never mind 😄

@vidartf
Copy link
Member Author

vidartf commented Jan 14, 2020

Er, I just pushed those changed, never mind 😄

Sounds reasonable since you stole my commit hook lints back ;)

@afshin afshin merged commit bb8276c into jupyterlab:master Jan 14, 2020
@vidartf vidartf deleted the main-no-pallette branch January 15, 2020 01:05
@lock lock bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Feb 14, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Feb 14, 2020
@jasongrout jasongrout removed this from the 2.1 milestone Feb 24, 2020
@jasongrout jasongrout added this to the 2.0 milestone Feb 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pkg:application pkg:console pkg:hub pkg:launcher pkg:logconsole pkg:mainmenu status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants