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

Language server protocol (LSP) #72

Merged
merged 3 commits into from
Sep 23, 2021
Merged

Language server protocol (LSP) #72

merged 3 commits into from
Sep 23, 2021

Conversation

krassowski
Copy link
Member

@krassowski krassowski commented Jun 27, 2021

As discussed in the pre-proposal and in this JEP we would like to propose adoption of the Language Server Protocol to enhance coding assistance in Jupyter notebooks (and other editors used by Jupyter frontends). In addition to rather broad JEP (we do not propose a specific implementation as a final standard) we would also like to convert the current repository where prototyping and exploration of this topic happens into a GitHub organization following Jupyter governance, contribution guidelines and branding to foster collaboration in this area (as discussed in the pre-proposal).

Closes #67 (pre-proposal) and closes #26 (supersedes).

@blink1073 is acting as the shepherd for the proposal.

Pre-requisites:

  1. Create a GitHub issue in the Jupyter Enhancement Proposals repo that
    1. Briefly outlines the proposal
    2. Suggests a review team (optional)
    3. Declares why it should be a JEP (See the “JEP / Not-a-JEP Rubric” below.)
  2. Identify a Shepherd to see the process through. (Shepherds are assigned
    on a round-robin basis from a set of interested engaged volunteers).
  3. Decide if it's a JEP according to the criteria listed above. The Shepherd decides if the JEP criteria have been met.
  4. Used the new JEP markdown template

The pre-proposal received several (what I assume are) positive reactions, but no comments in the 6 months since it was opened.

@krassowski
Copy link
Member Author

What should I do to move this forward?

@blink1073
Copy link
Contributor

blink1073 commented Aug 3, 2021

Opening the RFC phase, pinging @lresende @williamstein @hbcarlos @jtpio @ellisonbg @rgbkrk @Zsailer @TylerLeonhardt @MSeal @blois @choldgraf @captainsafia @westurner who have all engaged on this topic.

@blink1073
Copy link
Contributor

Eight folks have given a thumbs up on the PR text, let's move this into the Final Comment Period (FCP) until 26 August.

@blink1073
Copy link
Contributor

@jupyter/steeringcouncil I've added voting options to the top level comment

on the integration stalled a few years ago. Differently to that previous proposal we **do not**
propose to adopt any specific implementation, yet we bring a working implementation for CodeMirror 5
editor, which is already in use by two of the official front-ends for Jupyter (Jupyter Notebook and
JupyterLab). While the nearly-feature-complete CodeMirror 6 has specifically declared LSP
Copy link
Contributor

@choldgraf choldgraf Aug 27, 2021

Choose a reason for hiding this comment

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

Could you quickly speak to the ramifications of this? It is a bit unclear to me if adopting LSP at a core level will mean that CM6 cannot be used, or if this just means that CM6 won't itself have core support for this and will require extra hacking to get it working? Presumably that hacking would also be required for CM5 anyway?

Copy link
Member Author

@krassowski krassowski Aug 27, 2021

Choose a reason for hiding this comment

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

CM6 won't itself have core support for this

This one.

and will require extra hacking to get it working

I would not call it extra "hacking". CM6 exposes programmatic API to implement advanced features just like CM5 did; they just declared support that LSP as out of scope (as they never supported it), but it does not mean that one cannot implement it on top of their public API. Moreover JupyterLab completer (as an example) is already a separate component (independent of the editor) so we don't even rely on CodeMirror completer API. Also, there are reports of successful implementation of LSP with CodeMirror6: https://discuss.codemirror.net/t/codemirror-6-and-typescript-lsp/3398/3 so I would say there is absolutely nothing to worry about here.

Choose a reason for hiding this comment

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

Among the things I am interested in CM6, it further formalizes the editor marks (gutters, underlines, etc). right now in CM5, it's pretty wild west, and even if jl-lsp wanted to, we might not be able to do much with things that the debugger has "claimed" for DAP. In CM6, there is a concept of "bundles" which can co-exist happily, with separate owners. Extensions will benefit from those being managed in a more model-like form, and users will have a more consistent experience as they add/remove extensions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for both of these helpful explanations - I consider my question "resolved" - but will not click that button in the github UI so that others can stumble upon these thoughts as well

Copy link
Member Author

@krassowski krassowski Aug 27, 2021

Choose a reason for hiding this comment

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

Yes, clearing diagnostics is a headache in CM5 and should no longer be a problem in CM6. This is not to say that the journey of porting to CM6 will be easy for Jupyter - it won't, it will be a huge change and CM6 has naturally fewer extensions and modes available as it is younger. But I am digressing.

a working implementation, the proposal is not tied to it (beyond a proposal for migration of the
repository to a Jupyter-managed GitHub organization) but rather aimed to guide the process of
formalizing and evolving the way of integrating Jupyter with LSP in general.

Copy link
Member

Choose a reason for hiding this comment

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

but rather aimed to guide the process of formalizing and evolving the way of integrating Jupyter with LSP in general.

This is not really a technical question, but rather a strategic question to the community. Expanding on this comment are we comfortable moving JupyterLab to be more and more dependent on LSP.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree that this is a strategic and important question. Personally I do not see this as a way of making Jupyter dependent on LSP (I would say that Jupyter(Lab) needs to be independent of LSP), but as a way of enabling seamless integration and unifying the interfaces for interoperability.

Choose a reason for hiding this comment

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

Right: much like with DAP, we're trying to avoid re-implementing the 120 LSP messages on top of the existing 32 JK messages. For most folk, the experience is just fine without them, especially where they overlap.

Conversely, LSP doesn't have an effective way to do anything like custom comms or execution, which power the more outrageous features of the Jupyter experience, and basically can't do things like binary buffers without going out-of-band because of JSON-RPC, to which that spec is likely married forever. And indeed: i'd like to see comms become the model for managing the lifecycle of one or more language servers.

Choose a reason for hiding this comment

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

How will LSP interact with - is it called / issue # ? - multikernel notebooks with multiple languages? Apologies if this is already elsewhere addressed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this is mentioned in the JEP under "polyglot" term. One discussion of this issue is in https://github.com/krassowski/jupyterlab-lsp/issues/282

Choose a reason for hiding this comment

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

How to implement LSP for a multi-language kernel (SoS)? https://github.com/krassowski/jupyterlab-lsp/issues/282

@SylvainCorlay
Copy link
Member

I am very much in favor of Jupyter embracing LSP !

I like that the proposal is not definitive about implementation details. There are lots of things to address like how we can deal with remote execution environments fo example, which break the assumption that files are accessible from the server. Including LSP in the kernel protocol would make this work with remote execution environment, but it would not resolve the issue for normal files that are not associated with a running kernel, as you explain well in the proposal.

I am approving the PR.

Copy link
Member

@fperez fperez left a comment

Choose a reason for hiding this comment

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

Thanks so much @krassowski for this detailed document and your LSP work!

I'm very enthusiastic about this - I recall discussions with MS folks early on when LSP was just getting started, about how well its ideas tied into the Jupyter model, but we hadn't come to closer integration. I fully support having this, and I think once it lands, it's the kind of foundation that can lead to a lot of interesting exploration, new tooling, etc.

Big 👍 from me, thx again!

Copy link
Member

@mpacer mpacer left a comment

Choose a reason for hiding this comment

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

I appreciate the summary and the deep dives in the comments. Interoperability with well-established standards like LSP seems worthwhile and exciting. Thanks all for pushing this through!

Copy link
Contributor

@choldgraf choldgraf left a comment

Choose a reason for hiding this comment

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

I agree w/ what @mpacer says :-) I think it's a good idea!

Copy link
Member

@jasongrout jasongrout left a comment

Choose a reason for hiding this comment

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

This looks great. Thank you for all of your hard work on this project and JEP!

@blink1073
Copy link
Contributor

Merging since it has been 4 weeks and all votes have been in favor. Thanks @krassowski!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pre-proposal: incorporation of jupyter(lab)-lsp