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 metadata for launcher items #7654
Conversation
Thanks for making a pull request to JupyterLab! To try out this branch on binder, follow this link: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 , pending rebase
@@ -190,7 +191,8 @@ async function activateConsole( | |||
args: { isLauncher: true, kernelPreference: { name } }, | |||
category: 'Console', | |||
rank, | |||
kernelIconUrl | |||
kernelIconUrl, | |||
metadata: spec.metadata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels like we should namespace the kernel spec metadata so items can have other metadata too. For example, metadata: {kernel: spec.metadata}
or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm. That's a good point. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fair, I'll make the change.
* Metadata about the item. This can be used by the launcher to | ||
* affect how the item is displayed. | ||
*/ | ||
metadata?: ReadonlyJSONObject; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps the default implementation should display this metadata somewhere? It feels a bit weird to include metadata without using it anywhere. Presumably you have an overridden launcher that actually is using the metadata?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this change was meant to allow experimentation without needing to override core extensions like notebook
and console
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, okay.
02955c2
to
c26c8cc
Compare
Looks like the failures are legit |
References
Fixes #7652.
Code changes
Allows optional metadata to be passed to launcher items, and passes in the kernel metadata for the items created by notebooks and consoles.
User-facing changes
None, for the current implementation of the launcher.
Backwards-incompatible changes
None