Allow :initializationOptions with all forms of contact in eglot-server-programs

[astoff]

Currently, passing :initialiationOptions to a server in eglot-server-programs is not supported when the connection is over TCP or the contact is a class.

This PR lifts this restriction this by making the :initializationOptions feature orthogonal to the details of how to launch a server program.

(The initial submission of this PR phrased it differently, so note that the ensuing discussion doesn't address the description above directly.)

See als Emacs bug#57725.

Closes #940

[joaotavora]

I'd be more comfortable with a more surgical change such as:

diff --git a/eglot.el b/eglot.el
index ff94d5ca5f..3b1b1a9536 100644
--- a/eglot.el
+++ b/eglot.el
@@ -796,6 +796,8 @@ treated as in `eglot-dbind'."
   :documentation
   "Represents a server. Wraps a process for LSP communication.")

+(cl-defmethod initialize-instance :before ((_server eglot-lsp-server) &optional args)
+  (cl-remf args :initializationOptions))

 ;;; Process management
 (defvar eglot--servers-by-project (make-hash-table :test #'equal)

This seems to do the right thing in my simple tests.

[astoff]

With this patch and setting

(setf (alist-get 'python-mode eglot-server-programs)
        '("jedi-language-server"
          "-v"
          :initializationOptions
          (:workspace (:symbols (:maxSymbols 200)))))

I get the following error

Error in post-command-hook (#[0 "\303\304\300\242\305#\210\306\301!\205\0r\301q\210
?\205\0\307\310\311 \")\207" [(#0) #<buffer x.py> eglot--managed-mode remove-hook post-command-hook nil buffer-live-p apply eglot--connect eglot--guess-contact] 4]): (wrong-type-argument stringp :initializationOptions)

This happens because, as you know

ELISP> (let ((data '(even :odd 1))) (cl-remf data :odd) data)
(even :odd 1)

So these horrible manipulations in the PR

            (when-let ((probe (cl-position :initializationOptions contact)))
                     (prog1 (nth (1+ probe) contact)
                       (setq contact (append (cl-subseq contact 0 probe)
                                             (cl-subseq contact (+ 2 probe)))))))

seem necessary.

[astoff]

Oops, the above reason for the error is wrong. It's rather something in eglot--guess-contact.

[astoff]

By the way, my PR is longer than your suggestion because it makes :initializationOptions work in combination with :autoport or any of the other forms an entry of eglot-server-programs can have (testing pending).

[joaotavora]

Oops, the above reason for the error is wrong. It's rather something in eglot--guess-contact.

Yes, I see the error now, thanks for reporting it.

See my new commit, which simplifies eglot--guess-contact to not worry about providing an interactive prompt when keywords are found in the contact. It's just not worth it. So, barring any more errors, I think we can fix this by actually removing complexity.

[astoff]

So, barring any more errors, I think we can fix this by actually removing complexity.

I hope this means my version, since it prunes down the possible forms of CONTACT in eglot-server-programs from 6 to 5, while at the same time adding flexibility :-).

[joaotavora]

No, I'm sorry. It's too much code for and complicates the structure of the eglot-server-programs docstring. I'd rather something more surgical. Actually I think your very first suggestion over at the Emacs bug tracker also worked fine (except for the eglot--guess-contact thing).

prunes down the possible forms of CONTACT in eglot-server-programs from 6 to 5,

It just moves a section down to the end. I actually think that complicates the docstring.

Also I don't think :autoport is a widely used feature. And the docstring doesn't promise that :autoport works with :initializationOptions. Making it so doesn't seem very complicated (one just needs to prune out keywords in eglot--inferior-bootstrap) But I don't think it's worth it anyway.

[astoff]

Well, it's true that this is not the minimal hack necessary to make :initializationOptions to work at all.

This change is slightly longer because it makes :initializationOptions orthogonal to the rest of the other forms a contact can have in eglot-server-programs. So instead of a table of 5 × 2 possibilities where 4 of them are forbidden, there are 5 × 2 combinations and all work. To me this is simpler, although of course the code is a couple lines longer.

Anyway, I'll rename the PR to reflect what it does, even though I don't expect it will be merged.

[joaotavora]

So instead of a table of 5 × 2 possibilities where 4 of them are forbidden, there are 5 × 2 combinations and all work. To me this is simpler, although of course the code is a couple lines longer.

In my opinion, it is not simpler. I went to some lengths to make that list a flat list of 6 forms and I hope never to have to extend it again.

And it's not a questions of making the code "a couple of lines longer": it's about unmotivated changes. If I don't witness an actual need for making something,I won't. Here my need is to make :initializationOptions work at all: it was totally botched and now it's not (hopefully).

Eglot is about 4 years old. Well-meaning ambitious contributors have come and gone. I myself drift away often: I need the codebase to be as minimal and as stable as possible. See for example all the TRAMP related bugs: I didn't implement TRAMP support and I don't use it regularly. There were some proposals for adding this to Eglot and I took some time to choose the simplest one. Even that seems to cause intermittent problems I just don't have time to debug.

[astoff]

See for example all the TRAMP related bugs: I didn't implement TRAMP support and I don't use it regularly.

The T in Tramp is for "transparent", so Eglot shouldn't do anything at all — except for translating some magic file names to the local name in the remote machine, of course. I completely agree with your approach there.

I guess I should close this after all, so thanks for your patience :-).