clojure-lsp / lsp4clj

LSP base support for any LSP that is implemented in Clojure
MIT License
51 stars 5 forks source link

lsp4clj expects to be socket server #39

Closed mainej closed 1 year ago

mainej commented 1 year ago

Context

Problem

This is not how VS Code is designed. It expects to open the socket itself. Then it tells the server what port to connect to. That is, VS Code expects to be the socket server, and it expects the LSP server to be the socket client.

I haven't checked, but I would guess that other clients do the same for compatibility.

So, in practice, if lsp4clj.socket-server/server also tries to open the port it will fail, because the client has already opened it.

mainej commented 1 year ago

It's already quite easy for an lsp4clj server to connect to an open socket. This is the entire implementation:

(defn server [port]
  (let [addr (java.net.InetAddress/getByName nil)
        sock (java.net.Socket. ^java.net.InetAddress addr ^int port)]
    (lsp4clj.io-server/server {:in sock
                               :out sock})))

Candidate Solution 1

Document this in the README, and deprecate lsp4clj.socket-server.

It might also be good to document that though the LSP spec suggests that servers should accept a command-line arg --port, VS Code actually adds --socket=${port}. Of course, it wouldn't be hard on the language server side to accept either --port or --socket.

Candidate Solution 2

I don't think we really benefit much from hiding the Java bits from users, but I thought I'd propose this anyway.

Candidate Solution 3

Some combination of 1 and 2, but without deprecating lsp4clj.socket-server. Perhaps @SevereOverfl0w you could add an opinion. Did you ever end up using lsp4clj.socket-server?

SevereOverfl0w commented 1 year ago

I haven't gotten around to playing with lsp4clj yet, but I'm happy with #1 personally. That's down to my personal aesthetic preference of offering as little as possible as a library.

ericdallo commented 1 year ago

Thanks for noticing that. Just curious if this is a problem right now? is @SevereOverfl0w using the lib and missing that functionality?

I'm ok deprecating the current way, improving readme as mentioned in 1., but would be nice to align with LSP spec if we should support --socket or or --port, that's probably something to be fixed there

mainej commented 1 year ago

OK thanks. It sounds like we don't know of any existing users of lsp4clj.socket-server. It could probably just be deleted, but I'll plan to deprecate it and adjust the README.

would be nice to align with LSP spec if we should support --socket or or --port

lsp4clj won't directly support either—that'll be a decision for the language server. If and when we add socket communication to clojure-lsp, we should remember to support both --socket (for vscode-languageclient with TransportKind.socket) as well as --port (to align with the LSP spec).

ericdallo commented 1 year ago

Ok, makes sense, so I think it's ok to deprecate and adjust README 👍🏻