Allowing remote LSP connection in vscode API #102

Stevendeo · 2023-10-30T15:27:52Z

This PR adds the required utils for connecting vscode editor to a remote LSP.

nberth · 2023-10-30T15:37:50Z

@Stevendeo we you say "remote", do you mean the LSP has no longer access to the file-system where the project source code is? If that's the case, I think quite a bit of work is needed, and we should schedule for a much later milestone.

In particular, we should first handle some of the workspace-related requests.

Stevendeo · 2023-10-30T16:09:45Z

@Stevendeo we you say "remote", do you mean the LSP has no longer access to the file-system where the project source code is? If that's the case, I think quite a bit of work is needed, and we should schedule for a much later milestone.

In particular, we should first handle some of the workspace-related requests.

We discussed this particular issue with Fabrice the other day, so I was making a few tests. This PR is still in draft and should not have impact on the project anyway.

Stevendeo · 2023-10-30T16:24:59Z

@Stevendeo we you say "remote", do you mean the LSP has no longer access to the file-system where the project source code is? If that's the case, I think quite a bit of work is needed, and we should schedule for a much later milestone.

In particular, we should first handle some of the workspace-related requests.

Oh, and no I am just talking about the lsp being accessible through a specific address & port. I think this step is necessary before having the whole project in remote.

nberth

Ok now I start seeing what development with jsoo looks like ;-)

Very minor comments to address, and we can merge.

src/vscode/superbol-vscode-platform/superbol_instance.ml

src/vscode/vscode-languageclient-js-stubs/vscode_languageclient.ml

Stevendeo · 2023-11-15T09:46:27Z

Ok now I start seeing what development with jsoo looks like ;-)

To be fair, it should and could probably be cleaner. The problem is here that I did not find the interface between js of ocaml and the library used in the project, nor a module for websockets. I'm pretty sure there is one, but for testing I don't need it and I can do dirty things (until it breaks, because it will).

nberth

Ok to merge. BTW do you just test it with nc?

Stevendeo · 2023-11-15T13:14:22Z

I did not test it yet, I did not manage to find the proper combination to make it work, as superbol expects to receive something directly from stdin and not as an argument. Is there a way to call something like superbol-free lsp file.json ?

EDIT: nevermind, by writing this message I found how to.

Stevendeo · 2023-11-15T13:52:24Z

Okay, so it manages to establish a connection, but I think something is wrong when actually sending the data.

Stevendeo · 2023-11-15T14:10:19Z

I added a script to simulate a remote LSP in local. For now it only allows to receive data and give it to the LSP, which seems
to have some trouble to decode the request.

nberth · 2023-11-16T13:05:06Z

REMOTE_TEST.md

I think we could just

mkfifo /tmp/somewhere # possibly in a directory resulting from `mktemp -d` nc -l 8000 < /tmp/somewhere | superbol-free lsp > /tmp/somewhere

(as suggested here)

I definitely can't make nc work properly with the lsp server... I'll try something else I spend too much time on that

Okay, the protocol used is HTTP/1.1 with chunk transfer encoding, which I believe is not handled by the current lsp server...

Do you mean the websocket layer uses that protocol?

I'm not sure, perhaps that is the protocol used by VSCode when communicating through a remote server.

src/vscode/vscode-languageclient-js-stubs/vscode_languageclient.ml

nberth

I think the REMOTE_TEST.md still needs some changes. Also, I think for the moment its contents would better fit in the sphinx docs, maybe in a new file sphinx/remote-lsp.rst.

GitMensch · 2024-08-13T10:48:43Z

So this "just" needs a move + adjustment of the docs + rebase to be merged?

Stevendego · 2024-08-23T11:37:31Z

Doc updated & branch rebased!

nberth

Great to see that revived ;-)

I think there's a point remaining about the documentation for testing that feature.

nberth · 2024-08-23T11:51:11Z

src/vscode/superbol-vscode-platform/superbol_instance.ml

+      LanguageClient.make_stream
+        ~id
+        ~name


Suggested change

LanguageClient.make_stream

~id

~name

LanguageClient.make_stream ~id ~name

nberth · 2024-08-23T11:52:17Z

src/vscode/superbol-vscode-platform/superbol_instance.ml

+          Promise.return (
+              Vscode_languageclient.StreamInfo.create
+                ~writer:njs_stream
+                ~reader:njs_stream
+                ()
+            )
+        )


Suggested change

Promise.return (

Vscode_languageclient.StreamInfo.create

~writer:njs_stream

~reader:njs_stream

()

)

)

Promise.return @@

Vscode_languageclient.StreamInfo.create ()

~writer:njs_stream

~reader:njs_stream)

nberth · 2024-08-23T11:55:46Z

src/vscode/superbol-vscode-platform/superbol_instance.ml

+      LanguageClient.make ()
+        ~id
+        ~name
+        ~serverOptions
+        ~clientOptions


Suggested change

LanguageClient.make ()

~id

~name

~serverOptions

~clientOptions

LanguageClient.make () ~id ~name ~serverOptions ~clientOptions

(We don't go that vertical in this project.)

nberth · 2024-08-23T11:57:47Z

src/vscode/superbol-vscode-platform/superbol_instance.ml

-      ~clientOptions:(Superbol_languageclient.client_options ())
+    let cmd = Executable.command serverOptions in
+    if String.starts_with ~prefix:"ws://" cmd then
+      LanguageClient.make_stream


Am I right if I observe that options given to "usual" servers are ignored?

Also, what is the string that needs to be put in the command for testing? Is it just something like ws://localhost:<some port>?

(This PR is a bit old even for me, I'll try to answer as I remeber)

Am I right if I observe that options given to "usual" servers are ignored?

For now yes, I just added the bare minimum.

what is the string that needs to be put in the command for testing

ws:// is the prefix to add to the VSCode URL in its configuration. For testing with the cmd, just start the lsp and use this command

nberth · 2024-08-23T11:59:34Z

sphinx/remote-lsp.rst

+and call it with `test.nc`.
+
+TODO: this script only receives information, but does not send it back to
+VSCODE; it only writes it in superbol.log. It should answer to the client.


I think that's quite a big TODO. What happens when you try this suggestion?

I think that's quite a big TODO.

The thing was, when I started this PR, the LSP did not have a HTTP server (compatible with VSCode) , hence neither these script actually make it work. This PR was initially only to provide the base before starting the actual server

import/merlin

Stevendego · 2024-08-23T14:32:27Z

I'd say there is a missing dependency somewhere, I don't have any error

EDIT : $ npm install ws?

nberth · 2024-08-23T14:39:27Z

I'd say there is a missing dependency somewhere, I don't have any error

EDIT : $ npm install ws?

Yes I think that should be a dependency to declare in package.json. This file is actually generated from data in src/lsp/superbol_free_lib/vscode_extension.ml. I would add something like "ws", "^8" in the ~dependencies list there.

Edit: What version of ws do you have?

Stevendego · 2024-08-23T14:42:48Z

What version of ws do you have?

[email protected] (probably not the latest version, I did not update anything since a looooong time)

nberth · 2024-08-23T14:44:48Z

Ok thanks. Note, though: you need to make package.json and then amend the last commit with the generated package.json (the CI checks the generated file does not differ from the package.json in the commit).

nberth · 2024-08-23T14:52:08Z

Ouch…

[Error - 4:51:07 PM] SuperBOL Language Server client: couldn't create connection to server.
ReferenceError: joo_global_object is not defined
	at eval (eval at caml_js_eval_string (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:24526:16), <anonymous>:1:1)
	at caml_js_eval_string (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:24526:16)
	at caml_call1 (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:44842:57)
	at k$0 (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:46114:18)
	at caml_call1 (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:38481:57)
	at make_printf$0 (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:40953:20)
	at make_printf (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:41330:33)
	at width (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:40934:20)
	at caml_call_gen (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:23284:20)
	at Object.caml_call_gen (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:23289:18)
	at caml_call3 (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:54533:81)
	at njs_stream_of_string (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:54833:16)
	at caml_call1 (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:57328:57)
	at _t_ (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:57363:32)
	at caml_call1 (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:54527:57)
	at /tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:54881:23
	at caml_call_gen (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:23284:20)
	at /tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:28257:18
	at LanguageClient.createMessageTransports (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:17297:18)
	at LanguageClient.<anonymous> (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:15499:41)
	at Generator.next (<anonymous>)
	at /tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:61:61
	at new Promise (<anonymous>)
	at __async (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:45:10)
	at LanguageClient.createConnection (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:15492:16)
	at LanguageClient.<anonymous> (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:15083:43)
	at Generator.next (<anonymous>)
	at /tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:61:61
	at new Promise (<anonymous>)
	at __async (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:45:10)
	at LanguageClient.start (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:15051:16)
	at start (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:54891:48)
	at caml_call1 (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:57328:57)
	at _r_ (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:57380:21)
	at caml_call1 (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:47696:57)
	at fulfilled_safe (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:47728:18)
	at caml_call_gen (/tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:23284:20)
	at /tmp/vscode-exts/ocamlpro.superbol-0.1.4/_dist/superbol-vscode-platform-bundle.js:25313:21

src/vendor/vscode-ocaml-platform/src-bindings/vscode_languageclient/vscode_languageclient.ml

src/vendor/vscode-ocaml-platform/src-bindings/node/node.ml

GitMensch · 2024-09-01T14:27:31Z

@nberth Is there anything missing apart from the Changelog entry?
Would it be reasonable to pull that in soon?

nberth · 2024-09-02T07:15:05Z

@nberth Is there anything missing apart from the Changelog entry? Would it be reasonable to pull that in soon?

I think a better error reporting might be relevant still. At the moment if the TCP connection fails VSCode reports a lot of errors to the user, none of which seeming to indicate the root cause of the issue. Fixing that may be delayed though, as otherwise remote connection over TCP is operational (a more secure option, e.g with TLS, could be better — but that's definitely for later).

@Stevendeo I think parts of the new code is not in the proper module. I have a commit ready to move it (did it when I tried to fix error reporting … to no avail alas) ; may I push directly on your branch?

Stevendeo · 2024-09-02T12:31:53Z

may I push directly on your branch?

Yes of course; tell me when you're done so I can push a few last updates (Changelog + doc)

nberth · 2024-09-02T13:08:41Z

may I push directly on your branch?

Yes of course; tell me when you're done so I can push a few last updates (Changelog + doc)

Actually I opened a PR on your repository so you can see the changes first.

Some additions for the remote LSP connection

sphinx/remote-lsp.rst

nberth

Great many thanks! We can merge after the forgotten comment has been removed.

Stevendeo · 2024-09-03T12:21:04Z

We can merge after the forgotten comment has been removed.

Done and ready to merge!

Stevendeo force-pushed the remote-lsp branch from 21350d4 to c21f493 Compare October 30, 2023 16:13

Stevendeo marked this pull request as ready for review November 6, 2023 09:59

Stevendeo force-pushed the remote-lsp branch from c21f493 to cd5208f Compare November 14, 2023 09:55

Stevendeo requested review from lefessan and nberth November 14, 2023 09:57

nberth approved these changes Nov 15, 2023

View reviewed changes

src/vscode/superbol-vscode-platform/superbol_instance.ml Outdated Show resolved Hide resolved

src/vscode/vscode-languageclient-js-stubs/vscode_languageclient.ml Outdated Show resolved Hide resolved

nberth approved these changes Nov 15, 2023

View reviewed changes

Stevendeo changed the title ~~Allowing remote LSP for superbol~~ Allowing remote LSP connection in vscode API Nov 15, 2023

nberth reviewed Nov 16, 2023

View reviewed changes

nberth reviewed Feb 6, 2024

View reviewed changes

src/vscode/vscode-languageclient-js-stubs/vscode_languageclient.ml Outdated Show resolved Hide resolved

Stevendeo force-pushed the remote-lsp branch from d9ff64a to 5b6917d Compare March 1, 2024 10:02

nberth self-requested a review March 4, 2024 10:44

nberth requested changes Mar 4, 2024

View reviewed changes

Stevendeo force-pushed the remote-lsp branch from 5b6917d to 09dd251 Compare March 8, 2024 09:08

Stevendeo force-pushed the remote-lsp branch from 09dd251 to 65d8a77 Compare August 23, 2024 08:18

Allowing remote LSP for superbol

60eb348

Stevendeo force-pushed the remote-lsp branch from 65d8a77 to 60eb348 Compare August 23, 2024 10:06

nberth reviewed Aug 23, 2024

View reviewed changes

import/merlin Outdated Show resolved Hide resolved

Poetry

123af76

Stevendeo force-pushed the remote-lsp branch from e13340d to 123af76 Compare August 23, 2024 13:20

WS Dependency

9b711ee

Stevendeo force-pushed the remote-lsp branch from 858d317 to 9b711ee Compare August 23, 2024 14:48

nberth reviewed Aug 23, 2024

View reviewed changes

src/vendor/vscode-ocaml-platform/src-bindings/vscode_languageclient/vscode_languageclient.ml Outdated Show resolved Hide resolved

Draft: connecting to the LSP server via a TCP socket

72fb347

Stevendeo force-pushed the remote-lsp branch from 479d073 to 72fb347 Compare August 26, 2024 12:10

Stevendeo added 2 commits August 26, 2024 14:23

Update with option host & port

b4c5ba4

Version with proper types

d8248be

nberth reviewed Aug 26, 2024

View reviewed changes

src/vendor/vscode-ocaml-platform/src-bindings/node/node.ml Outdated Show resolved Hide resolved

Stevendeo and others added 3 commits August 26, 2024 17:21

Removing extra dependency

ee7e447

Not reinventing hot water

ed9b9a4

Use a cleaner interface for Vscode_languageclient.StreamInfo

f6434e3

nberth added 2 commits September 2, 2024 09:23

Move server access scan (command / tcp) out of Superbol_instance

817872f

Add CHANGELOG entry

18f4a99

Stevendeo added 2 commits September 2, 2024 15:50

Merge pull request #1 from nberth/remote-lsp

422d7a1

Some additions for the remote LSP connection

Update doc

df61d57

nberth reviewed Sep 3, 2024

View reviewed changes

sphinx/remote-lsp.rst Outdated Show resolved Hide resolved

Update doc

99a89d4

nberth approved these changes Sep 3, 2024

View reviewed changes

Forgotten comment

f113244

nberth merged commit ed344b4 into OCamlPro:master Sep 6, 2024
4 checks passed

Allowing remote LSP connection in vscode API #102

Allowing remote LSP connection in vscode API #102

Conversation

Stevendeo commented Oct 30, 2023

nberth commented Oct 30, 2023 • edited Loading

Stevendeo commented Oct 30, 2023

Stevendeo commented Oct 30, 2023

nberth left a comment

Choose a reason for hiding this comment

Stevendeo commented Nov 15, 2023

nberth left a comment

Choose a reason for hiding this comment

Stevendeo commented Nov 15, 2023 • edited Loading

Stevendeo commented Nov 15, 2023

Stevendeo commented Nov 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nberth left a comment

Choose a reason for hiding this comment

GitMensch commented Aug 13, 2024

Stevendego commented Aug 23, 2024

nberth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Stevendego Aug 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Stevendego commented Aug 23, 2024 • edited Loading

nberth commented Aug 23, 2024 • edited Loading

Stevendego commented Aug 23, 2024

nberth commented Aug 23, 2024 • edited Loading

nberth commented Aug 23, 2024

GitMensch commented Sep 1, 2024

nberth commented Sep 2, 2024

Stevendeo commented Sep 2, 2024 • edited Loading

nberth commented Sep 2, 2024

nberth left a comment

Choose a reason for hiding this comment

Stevendeo commented Sep 3, 2024

nberth commented Oct 30, 2023 •

edited

Loading

Stevendeo commented Nov 15, 2023 •

edited

Loading

Stevendego Aug 23, 2024 •

edited

Loading

Stevendego commented Aug 23, 2024 •

edited

Loading

nberth commented Aug 23, 2024 •

edited

Loading

nberth commented Aug 23, 2024 •

edited

Loading

Stevendeo commented Sep 2, 2024 •

edited

Loading