Make the remote experience easier

[toshokan]

As the docs mention, if we want to use lsp-mode over TRAMP, we have to register a new client with :remote t.
From crawling other people's dotfiles, it seems like the way people do this is:

1. Find the client you normally use locally in the lsp-mode client sources
2. Copy it into your emacs config
3. add remote: t
4. change lsp-stdio-connection to lsp-tramp-connection in :new-connection
5. append -remote to the server id

If #2531 is merged, lsp-tramp-connection will be very similar to lsp-stdio-connection, since the logic used to determine whether the process is started locally or remotely is encapsulated in make-processalready via the :file-handler t arg.
These functions may then be candidates for merging into a single function.

Currently, in my config, I have

(defun tkn/make-remote-lsp-client (original-server-id new-server-id new-connection)
  (let* ((client (copy-lsp--client (gethash original-server-id lsp-clients))))
    (setf (lsp--client-new-connection client) new-connection)
    (setf (lsp--client-remote? client) t)
    (setf (lsp--client-server-id client) new-server-id)
    (lsp-register-client client)))

Then for servers I want to use remotely, say rust-analyzer, I can do

(tkn/make-remote-lsp-client
 'rust-analyzer
 'rust-analyzer-remote
 (lsp-tramp-connection (lambda () lsp-rust-analyzer-server-command)))

The new-server-id can easily be auto-generated from the non-remote server id according to a pattern.
If, once #2531 is merged, we merge lsp-stdio-connection and lsp-tramp-connection into a single function, an even nicer interface can be provided out of the box.

lsp-mode could provide an interface where users simply list the server-ids they also want to use remotely (and maybe any lsp--client config overrides they want to apply to the remote version) and the remote clients could be autogenerated.

This would reduce duplication from people copying clients from the lsp-mode sources and not updating them, allow fixes to local clients to be automatically integrated into remote clients, and make using lsp-mode over TRAMP easier out of the box since now, in the best case, only configuration is required rather than writing or copying elisp to get it to work.

Another option, if lsp-[stdio/tramp]-connection become a single function, is to just drop the :remote slot on lsp--client entirely, allow all clients to be used remotely by default, and let the :file-handler/default-directory determine which should be started. This might lead to an even better user experience, but might be cumbersome if different configurations are desired for remote and local clients.

I can put together a PR if this sounds interesting to the project members, but I wanted to get some feedback on which option is preferable

[ericdallo]

That makes sense to me, actually, @yyoncho suggested something like that last week

[yyoncho]

I like your proposal, this is something that I was planning to implement. I want to alter the signature, to allow calling the function like this:

(tkn/make-remote-lsp-client 'rust-analyzer
 'rust-analyzer-remote
 (lsp-tramp-connection (lambda () lsp-rust-analyzer-server-command))
 :foo-bar-client-property new-value)

We can even make it a bit more generic:

(lsp-register-remote-client 
 'clangd
 :server-id 'cland-remote
 :new-connection ...)

Another option, if lsp-[stdio/tramp]-connection become a single function, is to just drop the :remote slot on lsp--client entirely, allow all clients to be used remotely by default, and let the :file-handler/default-directory determine which should be started. This might lead to an even better user experience, but might be cumbersome if different configurations are desired for remote and local clients.

I didn't go in that direction initially due to the fact that some clients need a specific configuration when they are run remotely. E. g. lsp-java has several paths that are local and they will crash the server if they do not exist on the remote server.

[toshokan]

@yyoncho I think that signature is nice. What do you think of an implementation like

(defun lsp-register-remote-client (based-on &rest args)
  ;; Create a copy of based-on client
  (let ((client (copy-lsp--client (gethash based-on lsp-clients))))
    ;; Set a default server id and set remote
    (setf (lsp--client-server-id client) (make-symbol (format "%s-remote" based-on)))
    (setf (lsp--client-remote? client) t)
    (cl-loop for (prop val) on args by 'cddr do
             (let* ((slot-sym (intern (substring (symbol-name prop) 1)))
                    (slot (ignore-error 'cl-struct-unknown-slot
                            (cl-struct-slot-value 'lsp--client slot-sym client))))
               (when slot
                 (setf slot val))))
    (lsp-register-client client)))

This would let you do (lsp-register-remote-client 'ruby-ls) to define 'ruby-ls-remote identically to the local 'ruby-ls, and

(lsp-register-remote-client 'rust-analyzer
  :server-id 'rust-analyzer-custom-remote-name
  :new-command (lsp-tramp-connection (lambda () lsp-rust-analyzer-server-command)))

to define 'rust-analyzer-custom-remote-name, with :new-command overridden.

Replicating the constructor's cl-style keyword args is a little awkward.

[ericdallo]

@toshokan looks good to me, maybe you can open a PR and we can discuss there, WDYT?