Refactor LSPConnection, ConnectionManager

[bollwyvl]

References

This has been mentioned a few places (e.g. #76), but doesn't have a concrete issue. Whoops!

Code changes

Changes the design contract to support:

• exactly one ConnectionManager per application (instead of one per Notebook/Editor)
• one LSPConnection per language (instead of one per Editor/Cell(!))
  • therefore one WebSocket
• in at least one feature highlight.ts, instead of waiting for the emit, the results are consumed directly behind a promise (or Thenable, but who's counting)
  • we probably want to use this pattern all over for request/response style messages
  • otherwise, i was getting duplicate "same token" highlights in totally crazy places in unrelated editors
  • we need to generally start testing with multiple documents open, as these might already be issues

In addition, this lazy-loads the connection.ts and all of lsp-ws-connection when the first language server is requested via await import.

User-facing changes

Apart from the initial delay loading the 1.5mb slug of lsp-ws-connection, this generally makes things much snappier, as each additional document/cell adds very little overhead.

Backwards-incompatible changes

No doubt, a lot of things that were expecting to "just work" will need to be more aware of the documentUri... I haven't merged up to master, so perhaps the recent completion fixes will uncover new issues.

Chores

• linted
• tested
• documented
• changelog entry

[bollwyvl]

Probably need to put this down for a bit.

• all of the acceptance tests are passing. I've || echod the jest tests for now on azure so i can get some eyes on other OSes.
• most of the unit tests working, and will be able to figure out the editor is undefined, but haven't dug into what rename is doing, so can't really tell why they might be failing.

[bollwyvl]

Also taking a look on binder to see if the slow connection issues are cleared up:

[bollwyvl]

Ugh. It looks like binder installs r-studio from apt/random shell scripts now: no idea what that's going to do to the r language server install we have.

[bollwyvl]

Hooray CI and binder both working the first time :P

[bollwyvl]

Still haven't merged in docs branch, but am otherwise starting to feel like this is done...

[bollwyvl]

Noticed some more stuff in looking into #190, doing some more work here...

[bollwyvl]

Similar to https://github.com/krassowski/jupyterlab-lsp/pull/165#issuecomment-581177638, i've also moved the kernelChanged handling up to the plugin level.

[bollwyvl]

Of note to the above: it's possible for a file editor to also have a kernel attached to it and provide completion (and even execution). Not going to handle on this PR, but probably also needs some love.

[bollwyvl]

Ah: that change won't work, because the kernel_info never comes back from the No Kernel. This is probably not desirable, will just revert.

[bollwyvl]

Well, the docker test is just being a jerk on osx, but I really can't think of anything else I want to do on this, having done the docs piece directly over on #177. I feel like we should probably merge this and move forward.

[krassowski]

Will do some local testing and merge today!

[bollwyvl]

🛸

…

On Sun, Feb 9, 2020, 01:54 Michał Krassowski ***@***.***> wrote: Will do some local testing and merge today! — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <https://github.com/krassowski/jupyterlab-lsp/pull/165?email_source=notifications&email_token=AAALCRE42G5GG6NLDFFHRXDRB6SDXA5CNFSM4KI2VBHKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELGEE7I#issuecomment-583811709>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAALCRAMA3OQEJZ6PHVBFCLRB6SDXANCNFSM4KI2VBHA> .

[krassowski]

Thank you!