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

Ensure notebook document event registration #14242

Merged
merged 1 commit into from
Oct 8, 2024
Merged

Conversation

msujew
Copy link
Member

@msujew msujew commented Oct 2, 2024

What it does

Ok, this one is a bit difficult to describe, but I will try my best:

The Jupyter plugin uses a context key called jupyter.ispythonnotebook to determine whether to show the jupyter.openVariableView button.

This context key is set inside of the jupyter plugin, after encountering the metadata.kernelSpec property. This metadata property is set immediately when the notebook opens. This is done by performing a notebook metadata edit. This edit is performed correctly in the frontend notebook model, but since the necessary events aren't setup yet, the metadata edit isn't propagated back to the plugin host.

This change adjusts the order in which the service calls are happening to ensure that all events are properly setup before the notebook opening event is sent to the plugin host. This ensures that any edit operation on the frontend side is correctly propagated towards the plugin host.

How to test

Open a notebook editor for a .ipynb file and select a kernel. Reload the app and ensure that the Variables button is available in the notebook toolbar.

Review checklist

Reminder for reviewers

@msujew msujew added the notebook issues related to notebooks label Oct 2, 2024
@msujew msujew requested a review from jonah-iden October 2, 2024 14:00
Copy link
Contributor

@jonah-iden jonah-iden left a comment

Choose a reason for hiding this comment

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

Seems to work fine. Just weird that vscode does it the other way around

@msujew
Copy link
Member Author

msujew commented Oct 8, 2024

Just weird that vscode does it the other way around

VS Code isn't blocking the browser thread by calling await on the RPC call like we do. However, it's better to just move the RPC call to the back anyway.

@msujew msujew merged commit 0f15ab6 into master Oct 8, 2024
11 checks passed
@msujew msujew deleted the msujew/jupyter-button branch October 8, 2024 08:41
@github-actions github-actions bot added this to the 1.55.0 milestone Oct 8, 2024
@jonah-iden
Copy link
Contributor

ah ok, thanks for the explanation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notebook issues related to notebooks
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants