-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 for LSP notebooks experiment #19087
Conversation
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.
Do you want to add a news item for this change? Something like "Experiment support for notebooks when using Pylance"? If not, do add the 'skip news' label to this PR.
Also, there will be merge conflicts depending on which PR gets merged in first between this one and #19076. |
I just thought of something. This whole thing is going to break de-duplication that we do for completions we get back from the jupyter kernel. Right now, since the jupyter extension creates its own pylance, we can get the completions from pylance and remove those from jupyter that are the same thing. After this change, the pylance server won't be started by jupyter and we won't be able to ask it directly for completions. We need some way to find the specific completion provider (or the language client). So I think we'd need yet another API to pass the language client through to jupyter. We could do that after this though. This is the line where we ask pylance directly for its completions: We'd need some way to get to this language client from jupyter still (well unless VS code has an API to return a specific completion provider but I don't think it does). |
I entered this issue here for the dedupe problem: |
…c instead of middleware
…y for supported case
…idingMiddleware logic
…art off undefined
Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com>
Contributes to https://github.com/microsoft/pyrx/issues/2343
When Pylance's LSP notebooks experiment is enabled, the Python extension will be responsible for starting Pylance for notebooks and therefore also needs to be able to provide the notebook's python.exe path to Pylance. Prior to this change, this was always done by Jupyter. This PR exposes a new Python API called
registerJupyterPythonPathFunction
which enables Jupyter to hook into Pylance's config setting requests and provide the python.exe path for a notebook's kernel.The default behavior of notebooks is unchanged. The new behavior is only used when the experiment is enabled via the
python.pylanceLspNotebooksEnabled
setting and the installed versions of both Pylance and Jupyter are new enough to support the experiment.A quick overview of the changes:
LspNotebooksExperiment
class determines if the experiment is enabled. It also deals with issues around transitioning into the experiment if Jupyter is installed after Python activates. For example, in this scenario it needs to restart any existing Pylance instances.LanguageClientMiddleware
, so the code related to the experiment would be separated from the Jedi-related logic.NodeLanguageClientMiddleware
does not install theHidingMiddlewareAddon
when the experiment is enabled.NodeLanguageClientMiddleware
also overrides the newgetPythonPathOverride
method to call the Jupyter code provided viaregisterJupyterPythonPathFunction
and get the kernel's python path.The related Pylance changes are in this PR: https://github.com/microsoft/pyrx/pull/2382
The related Jupyter changes are in this PR: microsoft/vscode-jupyter#9842