Closed Stebalien closed 7 months ago
See also #2. I'd certainly consider a PR (ideally using cl-defmethod
).
How is this custom class being used? Can you show some of the details? I actually think a custom class might be the solution, but obviously if you have one already...
I don't think there's any way to do this with custom classes.
I've defined the following methods:
;; Define a macro expansion method for servers.
(cl-defmethod eglot-macro-expansion (_server &key &allow-other-keys)
(eglot--message "Server does not support macro expansion"))
(defun eglot-expand-macro ()
"Expands the macro call at point recursively."
(interactive)
(eglot-macro-expansion (eglot--current-server-or-lose)))
Then, I define a custom class for rust-analyzer
:
(defclass eglot-rust-analyzer (eglot-lsp-server) () :documentation "Rust Analyzer.")
Replace the existing rust-analyzer
with my class:
(let* ((key '(rust-mode rust-ts-mode)))
(setq eglot-server-programs
(--remove (-intersection (ensure-list (car it)) key) eglot-server-programs))
(push (cons key '(eglot-rust-analyzer "rust-analyzer"))
eglot-server-programs))
And define the generic eglot-macro-expansion
function for the class in question:
(cl-defmethod eglot-macro-expansion ((server eglot-rust-analyzer))
...)
That way, I can call eglot-expand-macro
macro in any eglot-enabled buffer to expand the macro at point, if supported by the current server. The idea is that, if some other server adds support for macro expansion, I could just define the method for that new server.
I don't think there's any way to do this with custom classes.
Unless there's a way to override/wrap the "contact" with the class. But the method I'm currently using is:
(defconst eglot-booster--boost-prefix
'("emacs-lsp-booster" "--json-false-value" ":json-false" "--"))
(defun eglot-booster--boost (cmd)
(if (file-remote-p default-directory)
cmd
(cons (append eglot-booster--boost-prefix (car cmd)) (cdr cmd))))
(define-advice eglot-booster--wrap (:override (&optional unwrap) advise-cmd)
(if unwrap
(advice-remove 'eglot--cmd #'eglot-booster--boost)
(advice-add 'eglot--cmd :filter-args #'eglot-booster--boost '((depth . 50)))))
(i.e., advising eglot--cmd
).
Yeah, that looks possible, but it i) does not handle remote or network servers (which should be ignored as lap-booster cannot talk to them) and ii) may be brittle in future changes to eglot (as patching eglot-server-programs
directly has proven to be; see #4).
It seems you are pretty familiar with the class based design here; do you think there's a way to hook into the jsonrpc
class heirarchy, i.e. at a level higher than eglot-lsp-server
?
Unfortunately that doesn't let you override the server program. Technically the class stores the process-initialization function, but you'd have to re-implement most of eglot--connect
if you wanted to mess with that.
does not handle remote or network servers (which should be ignored as lap-booster cannot talk to them
It should. That's why eglot-booster--boost
checks to see if default-directory
is remote and avoids prepending the booster command to it in that case.
That's why eglot-booster--boost checks to see if default-directory is remote and avoids prepending the booster command to it in that case.
That's not the relevant check, since you can in principle run lsp servers which communicate over network ports locally. Here's the relevant doc:
* A list (HOST PORT [TCP-ARGS...]) where HOST is a string and
PORT is a positive integer for connecting to a server via TCP.
Remaining ARGS are passed to `open-network-stream' for
upgrading the connection with encryption or other capabilities.
* A list (PROGRAM [ARGS...] :autoport [MOREARGS...]), whereupon a
combination of previous options is used. First, an attempt is
made to find an available server port, then PROGRAM is launched
with ARGS; the `:autoport' keyword substituted for that number;
and MOREARGS. Eglot then attempts to establish a TCP
connection to that port number on the localhost.
Ah, well, that's less "remote" and more "connect to an existing language server". The above advice will work (i.e., it the booster will be disabled) in that case as well as it will only affect language servers started by eglot.
If you wanted to support that case (not just disable the booster), you'd need to:
eglot--connect
to the booster.eglot--connect
itself to replace contact
with a command that always starts the booster.In my understanding, this will not work, because lsp-booster only knows how to read from stdin and write to stdout, but not how to read/write from TCP network sockets. I.e. eglot can start servers which only communicate via network ports, and then proceed to communicate with them via those ports. This functionality could be added to lsp-booster of course (read from server on one port, write to emacs on another), but at present it's not there, so there's no way to add it here. Rather, you just need to identify such cases, and avoid boosting them.
In any case, I've switched to advising eglot-connect
. Can you please give that a test with your custom class config?
because lsp-booster only knows how to read from stdin and write to stdout, but not how to read/write from TCP network sockets
Yes, it would require changes to the booster. But so would any other solution.
In any case, I've switched to advising eglot-connect. Can you please give that a test with your custom class config?
That works locally, but it's broken with TRAMP (remote connections):
eglot--cmd
gets a crack at it.I really recommend that you advise eglot--cmd
as described above. eglot--cmd
exists for wrapping/modifying the command.
You seem to be focused on a particular solution instead of clearly describing the problem. Starting a remote process across TRAMP does not differ from starting it locally, so I don't understand why this is relevant. I don't want to advise eglot--cmd
because it is a small internal helper function which could easily disappear going forward, for example if the bug it references is fixed upstream. That risk also exists for eglot--connect
, but it is much smaller IMO.
Please show explicitly how this breaks tramp with an example of what the contact
args would be in the situation you are describing.
Please show explicitly how this breaks tramp with an example of what the contact args would be in the situation you are describing.
It prepends the eglot-booster
command to the contact even when the default-directory
is remote (i.e., using TRAMP). Which means it's going to try to run eglot-booster
on the remote machine.
If one were to install eglot-booster
on the remote machine, it still wouldn't work because eglot--cmd
wraps the contact in in a shell command when the default-directory
is remote (which will break the booster detection in eglot-booster--init
).
Additionally, by advising eglot--connect
instead of eglot--cmd
:
eglot--connect
to determine if the "contact" is actually a command, or if it's something else (brittle to changes in eglot).eglot--connect
is just as internal as eglot--cmd
.I'm focusing on my proposed solution because it works and we're going back and forth on a lot of solutions either won't work or won't work as well.
Please show explicitly how this breaks tramp with an example of what the contact args would be in the situation you are describing.
It prepends the
eglot-booster
command to the contact even when thedefault-directory
is remote (i.e., using TRAMP). Which means it's going to try to runeglot-booster
on the remote machine.
Yes, and this is exactly what most users would expect I think. I now understand that you're trying to run emacs-lsp-booster
locally, but the LSP server it communicates with remotely? That's indeed an unusual setup, and I'm not sure how it would work, since the local emacs-lsp-booster
program would need to read from TRAMP, then write back to emacs.
I'd personally recommend installing emacs-lsp-booster
remotely, since TRAMP will benefit from the reduced traffic volume too, a double-win.
If one were to install
eglot-booster
on the remote machine, it still wouldn't work becauseeglot--cmd
wraps the contact in in a shell command when thedefault-directory
is remote (which will break the booster detection ineglot-booster--init
).
It does? How? That just checks if "emacs-lsp-booster" is anywhere in the command. Have you tried this? If you try this and it doesn't work, please open another issue.
Additionally, by advising
eglot--connect
instead ofeglot--cmd
:
- You need to re-implement parts of
eglot--connect
to determine if the "contact" is actually a command, or if it's something else (brittle to changes in eglot).
I already had to do this when updating server-programs, so it's nothing new.
- You're not really gaining anything by not using an "internal" function because
eglot--connect
is just as internal aseglot--cmd
.
I disagree. eglot--cmd
says:
;; TODO: this seems like a bug, although it’s everywhere. For
;; some reason, for remote connections only, over a pipe, we
;; need to turn off line buffering on the tty.
which is a strong indication that this is an unwanted workaround function, not a point for hanging additional customization. If this is fixed in emacs, this function would disappear. For your situation, it sounds like eglot--cmd
may be a better target.
I'm focusing on my proposed solution because it works and we're going back and forth on a lot of solutions either won't work or won't work as well.
For your case, that may indeed be the case. If you get this alternative style working well (local booster, remote server via TRAMP), would you please document it in the Wiki for others? I'd suggest including an example of the "effective command" that enables this.
That's indeed an unusual setup, and I'm not sure how it would work, since the local emacs-lsp-booster program would need to read from TRAMP, then write back to emacs.
In my setup, I don't use lsp-booster when I use TRAMP because, while I can easily install language servers on remote machines, installing lsp-booster on remote machines is not always easy and rarely worth it (I'm already willing to put up with a loss of performance at that point).
That and directly executing bytecode sent from a remote machine is... not something I want to do.
It does? How? That just checks if "emacs-lsp-booster" is anywhere in the command. Have you tried this? If you try this and it doesn't work, please open another issue.
It checks if it's contained within the car of the command:
(string-search "emacs-lsp-booster" (car-safe com))
In this case, the car of the command will be the user's shell-file-name
.
I disagree. eglot--cmd says:
Any of these functions can change at a moments notice. In the case of eglot--connect
. The right way to do this is to ask upstream for a stable point to wrap.
But really, it doesn't matter. If you want to advise eglot--connect
, that's up to you.
In this case, the car of the command will be the user's shell-file-name.
See:
(defun eglot--cmd (contact)
"Helper for `eglot--connect'."
(if (file-remote-p default-directory)
;; TODO: this seems like a bug, although it’s everywhere. For
;; some reason, for remote connections only, over a pipe, we
;; need to turn off line buffering on the tty.
;;
;; Not only does this seem like there should be a better way,
;; but it almost certainly doesn’t work on non-unix systems.
(list shell-file-name "-c" ;; <<<<<<<<<<<<<<<<<< HERE
(string-join (cons "stty raw > /dev/null;"
(mapcar #'shell-quote-argument contact))
" "))
contact))
In this case, the car of the command will be the user's
shell-file-name
.
Good point, I added a fix to search all elements of the command list.
I still don't understand how you can have a remote LSP server
-> TRAMP -> emacs-lsp-booster
-> eglot
setup. If you get this working, please add information to the Wiki for others who may want that setup.
In terms of a stable upstream connection, I agree; please see #2 and make any suggestions you have there.
I still don't understand how you can have a remote LSP server -> TRAMP -> emacs-lsp-booster -> eglot setup. If you get this working, please add information to the Wiki for others who may want that setup.
I don't. I just disable emacs-lsp-booster
over TRAMP.
My issue with the your previous approach is that it tried to run emacs-lsp-booster
over TRAMP, but then failed to detect it.
In terms of a stable upstream connection, I agree; please see https://github.com/jdtsmith/eglot-booster/issues/2 and make any suggestions you have there.
I don't really have anything to add that joaotavora hasn't already said. The only thing left is to upstream some patches to both eglot and jsonrpc.
For now, given the brittle nature of this integration, I've decided that it's just easier to maintain a slimmed-down version in my own config (so I can, e.g., fix it when it breaks). IMO, fixing #2 is really the highest priority. After that, everything should get simpler.
(defconst eglot-booster--boost-prefix
'("emacs-lsp-booster" "--json-false-value" ":json-false" "--"))
(defvar-local eglot-booster-boosted nil)
(define-advice jsonrpc--json-read (:around (orig-func) boost)
"Read JSON or bytecode, wrapping the ORIG-FUNC JSON reader."
(if eglot-booster-boosted ; local to process-buffer
(or (and (= (following-char) ?#)
(let ((bytecode (read (current-buffer))))
(when (byte-code-function-p bytecode)
(funcall bytecode))))
(funcall orig-func))
;; Not in a boosted process, fallback
(funcall orig-func)))
(define-advice eglot--cmd (:filter-args (cmd) boost 50)
(if (file-remote-p default-directory)
cmd
(cons (append eglot-booster--boost-prefix (car cmd)) (cdr cmd))))
(defun eglot-booster--init (server)
"Register eglot SERVER as boosted if it is."
(when-let* ((proc (-some-> server jsonrpc--process))
(cmd (-some-> proc process-command car-safe))
(buf (process-buffer proc))
((string-equal "emacs-lsp-booster" cmd)))
(setf (buffer-local-value 'eglot-booster-boosted buf) t)))
(add-hook 'eglot-server-initialized-hook #'eglot-booster--init)
I just disable emacs-lsp-booster over TRAMP. My issue with the your previous approach is that it tried to run emacs-lsp-booster over TRAMP, but then failed to detect it.
I see. I've added a custom var eglot-booster-no-remote-boost
which you can try if you like (it's nil
by default).
The only thing left is to upstream some patches to both eglot and jsonrpc.
I agree. Would you like to contribute to that effort?
I agree. Would you like to contribute to that effort?
I uh... have a few too many projects going on right now so I probably shouldn't volunteer for it. I will if I get a free weekend, but don't hold your breath.
When an eglot server is defined with a custom class
(custom-class . init-args)
, eglot-booster blindly prepends the "emacs-lsp-booster" command to the entry ineglot-server-programs
. Have you considered advisingeglot--cmd
instead of modifying this list? That should "just work" for just about everything.