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

Support diagnostics panel in Notebook v7+ #981

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

krassowski
Copy link
Member

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

@github-actions
Copy link

Binder 👈 Launch a binder notebook on branch krassowski/jupyterlab-lsp/diagnostics-panel-notebook

@krassowski krassowski changed the title Support diagnostics panel in notebook Support diagnostics panel in Notebook v7+ Sep 15, 2023
@krassowski krassowski added the help wanted Extra attention is needed label Oct 8, 2023
Comment on lines +106 to +110
app.shell.add(panelWidget, 'right');
await app.commands.execute('application:toggle-panel', {
id: panelWidget.id,
side: 'right'
});
Copy link
Member Author

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)?

Copy link
Collaborator

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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants