-
Notifications
You must be signed in to change notification settings - Fork 148
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
Support diagnostics panel in Notebook v7+ #981
base: main
Are you sure you want to change the base?
Conversation
app.shell.add(panelWidget, 'right'); | ||
await app.commands.execute('application:toggle-panel', { | ||
id: panelWidget.id, | ||
side: 'right' | ||
}); |
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.
This does not seem to be enough to add a Notebook panel from extension 😿. I am yet to investigate further, but in the meantime, @jtpio do you have thoughts on this one (given our other conversation on lab-notebook shell interfaces divergence)?
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.
Hmmm...
app instanceof NotebookApp
This is probably not going to work, long term, as it means the notebook packages will be bundled along with the already-large (and duplicated) jupyterlab-lsp
assets, and could very well return a false negative if webpack is feeling grumpy.
Making it a sharedPackage
might improve things, but would really need to be in an entirely separate plugin file with a naked (not type
) import
...
Further, this is often not enough information, as one still has to consult the PageConfig.getOption('notebookPage')
, to determine whether a given notebook page should not be able to get a panel (e.g. tree
, terminal
)... it would actually be lovely if this list of documented string constants (not as a TypeScript enum, for the same reason above) were hoisted as a dedicate package.
Alas, we kinda missed the boat on changing the IFrontendShell
interface to provide a dumb, string-capability-based model on the major version revs.
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.
Thus far, this pattern has worked for stuff I've been tinkering with:
References
An attempt to display diagnostics panel in notebook v7
Code changes
Import
NotebookApp
and execute appropriate command.User-facing changes
TBD
Backwards-incompatible changes
None