Convert usage of pyls to pylsp.

[douglasdavis]

Palantir's python-language-server has been forked and a new maintained project has started at python-lsp-server (relevant issue: python-lsp/python-lsp-server#4, the contributors from the open source Python community were no longer able to commit to the Palantir project, leaving it unmaintained). This PR transitions pyls usage to pylsp. (I have copyright assignment paperwork done for GNU Emacs)

[joaotavora]

@douglasdavis , an excellent idea. I once tried to contribute to pyls and was turned off by some paperwork I didn't quite agreed with. So if pylsp is the preferred and maintained alternative, I'll gladly switch Eglot's defaults.

While I also much sympathize with the idea of cleaning up the trailing white space, I'd rather we make that a separate commit. So do you think you could appease this poor maintainer's OCD and provide this without the unsolicited whitespace changes in README.md, or put them in a separate PR? I'll then reword the commit message according to GNU Changelog style.

[douglasdavis]

João Távora ***@***.***> writes:

While I also much sympathize with the idea of cleaning up the trailing white space, I'd rather we make that a separate commit. So do you think you could appease this poor maintainer's OCD and provide this without the unsolicited whitespace changes in README.md, or put them in a separate PR? I'll then reword the commit message according to GNU Changelog style.

Happy to! I think I took care of all of my overzealous before-save-hook whitespace-cleanup changes. Also thanks for adjusting the commit message

[nemethf]

I know the structure of `eglot-server-programs' is already quite complicated, but I wonder whether it would be worth the complexity to support both pyls and the new pylsp. For example, if the executable of pylsp is in the path, then eglot should select pylsp, otherwise it should continue with pyls.

[joaotavora]

I know the structure of `eglot-server-programs' is already quite complicated, but I wonder whether it would be worth the complexity to support both pyls and the new pylsp. For example, if the executable of pylsp is in the path, then eglot should select pylsp, otherwise it should continue with pyls.

🙀
🙀
🙀
💀

[douglasdavis]

I must admit I'm not exactly sure if that's a yes or no!

I'm happy to implement something like this (taking inspiration from eglot--eclipse-jdt-contact):

(defun eglot--find-default-py-server (interactive)
  (cond ((executable-find "pylsp") '("pylsp"))
        ((executable-find "pyls") '("pyls"))
        (t '("pylsp"))))

with the entry in eglot-server-programs being (python-mode . eglot--find-default-py-server).

OTOH, I think it would help guide users to transitioning to pylsp if they see Eglot is looking for it (which is what happens if pylsp is undetected, with it being printed in the minibuffer)

[joaotavora]

@douglasdavis sorry, it was a "no, not yet" to @nemethf, some sillyness that wasn't clear to anybody but me probably. But the idea is: let's not overcomplicate eglot-server-programs. If as you say pyls is now unmaintained, and adding it back is a simple one liner for whomever likes it, I think changing to pylsp makes perfect sense. I just need a little time to properly review this.

We can think about that fallback behaviour later on in a new issue.

[joaotavora]

And yes @douglasdavis , your idea is probably the best way to go about implementing @nemethf 's idea. It's a little more verbose but it gets the presumably not-so-common job done.

[joaotavora]

My only concern right now is that I don't know if this kind of patch counts as a "trivial", non-copyright-paperwork-requiring change. I think it does, but I'm not sure. Maybe @monnier or someone else can help?

[douglasdavis]

I actually have copyright assignment paperwork filed for GNU Emacs

[joaotavora]

Douglas Raymond Davis? yup I see it. Great.

[monnier]

My only concern right now is that I don't know if this kind of patch counts as a "trivial", non-copyright-paperwork-requiring change. I think it does, but I'm not sure.

I think it does, but it's irrelevant since he signed the paperwork anyway ;-)

[joaotavora]

@douglasdavis , I was just playing with some code in #537 and @nemethf 's idea and I figured maybe we can keep supporting pyls and pylsp, i.e. support both out of the box. Can you try this patch and tell me if it finds pylsp for you?

diff --git a/eglot.el b/eglot.el
index db468d8..0da5275 100644
--- a/eglot.el
+++ b/eglot.el
@@ -95,8 +95,28 @@
   :prefix "eglot-"
   :group 'applications)
 
-(defvar eglot-server-programs '((rust-mode . (eglot-rls "rls"))
-                                (python-mode . ("pyls"))
+(defun eglot-alternatives (alternatives)
+  "Compute server-picking function suitable for `eglot-server-programs'.
+Each element of ALTERNATIVES is a string PROGRAM or a list of
+strings (PROGRAM ARGS...) where program names an LSP server
+program to start with ARGS."
+  (lambda (&optional interactive)
+    (let* ((listified (cl-loop for a in alternatives
+                               collect (if (listp a) a (list a))))
+           (available (cl-remove-if-not #'executable-find listified :key #'car)))
+      (cond ((and interactive (cdr available))
+             (let ((chosen (completing-read
+                            "[eglot] More than one server executable available:"
+                            (mapcar #'car available)
+                            nil t nil nil (car (car available)))))
+               (assoc chosen available #'equal)))
+            ((car available))
+            (t
+             (car listified))))))
+
+(defvar eglot-server-programs `((rust-mode . (eglot-rls "rls"))
+                                (python-mode
+                                 . ,(eglot-alternatives '("pyls" "pylsp")))
                                 ((js-mode typescript-mode)
                                  . ("typescript-language-server" "--stdio"))
                                 (sh-mode . ("bash-language-server" "start"))

[monnier]

+(defun eglot-alternatives (alternatives)

[...]

+ (let ((chosen (completing-read + "[eglot] More than one server executable available:"

[...]

+(defvar eglot-server-programs `((rust-mode . (eglot-rls "rls")) + (python-mode + . ,(eglot-alternatives '("pyls" "pylsp")))

The act of loading a file should not affect Emacs behavior, so the above is problematic: just loading `eglot.el` will prompt the user regardless if the user intends to use Eglot and even if the user has no intention to edit Python code.

[joaotavora]

That's inside a lambda, Stefan, are you sure you're reading this right?

[monnier]

That's inside a lambda, Stefan, are you sure you're reading this right?

<blush>Well, of course, I saw that. I was just ...hmm... testing you!</blush>

[joaotavora]

😆

[douglasdavis]

Hi @joaotavora, M-x eglot indeed finds pylsp with your patch 👍

[joaotavora]

Hi @joaotavora, M-x eglot indeed finds pylsp with your patch +1

In that case, if you don't mind, I'd rather apply that, while also mentioning pylsp in the README.md.