Add support for Eglot (and maybe Eshell)

[zyxir]

Thank you for creating this package! It is great and works with most Python packages I use. However I encountered some problems while extending pet's support to Eglot and Eshell. I want to discuss them here (I have already made some efforts).

My workflow

I appreciate that pet works with so many virtual environment managers. Currently I only use Python's built-in venv, and have a .venv directory in all of my Python projects. I want packages like python-pytest and flycheck automatically pick executables and packages from the .venv directory, which is handled pretty well by pet.

Problem 1: Eglot integration

Eglot is a light-weight implementation of LSP (language server protocol) inside Emacs, and is built-in since Emacs 29. I use Eglot to communicate with an external language server (pylsp in my case), and the language server can do all sort of things for me, like providing completion via rope, formatting my buffer via black, linting my buffer via mypy and ruff, etc..

By default, eglot just find the pylsp executable via executable-find, and launch it as the language server. However, this by default launches the global pylsp executable, instead of the local one inside the virtual environment. Thus, the global language server cannot recognize the local packages inside the virtual environment (because they are not installed globally), and cannot do its works like linting and providing completions properly.

Can pet solve this? definitely! I solved the problem with the following steps:

1. Add eglot-ensure to python-mode-hook (or python-ts-mode-hook if tree-sitter is enabled) with a depth of 100, so that eglot is always enabled after pet.
2. Advice the eglot--executable-find function with the pet counterpart, like this.

Now Eglot can use the local pylsp as expected, only with one drawback: it does not work on TRAMP, because pet-executable-find does not work on TRAMP. Luckily, #20 solves this. Hope it be merged soon.

My problem is: should my workaround be pet's default behavior? Since Eglot is very popular (maybe more popular than those other packages that pet supports, as it is built-in now), adding its support may not be a bad idea. If you think so, I can create a PR on this.

Problem 2: Eshell or Shell integration

When working on my Python projects, sometimes I have to interact with a shell, for example when I have to create the virtual environemnt with python -m venv .venv, or when I have to install new packages into the virtual environment with pip install, or even when I try to test my package's CLI with python -m my_awesome_module. I don't know if this is only my own unique demand, but I need a shell quite a lot.

First I try to use the eshell, but eshell cannot activate a virtual environment via activate .venv/bin/activate, as eshell does not execute Bash commands.

After that, I tried shell. In shell, I can activate the virtual environment manually. But, oh god, why do I have to do things manually as an Emacser? So I wrote these code to auto-activate the Python virtual environments, if there is one in the default-directory (It is also worth noting that VS Code does this by default).

It seems fine, but:

1. My workaround is my own magical hack. I didn't use pet (which I did want to, but I didn't know how, because I'm not sure if every virtual environment manager offer an "activate" script).
2. My workaround is ugly. I love eshell, it is very Lispy, and provides consistent behavior on every platform I use. I shoudn't be forced to use shell on such basic need, and shell even cannot work properly with PowerShell on Windows. There must be some other workaround for eshell.

The final solution to this must be integrating pet into eshell. virtualenvwrapper is already able to make shell, eshell and any other subprocesses aware of the Python virtual environment, despite it support less virtual environment managers than pet.

I do want to make pet perfect (so that you don't have to install a crazy amount of virtual environment packages), so I expect an opportunity to thouroughly discuss these problems. If a preferred method is found, I would be more than willing to implement it and create a pull request.

[egelja]

Here's how I did it with eglot

(use-package pet
  :ensure-system-package (dasel sqlite3)
  :config
  (add-hook 'python-mode-hook
            #'(lambda ()
                ;; Python interpreter
                (setq-local python-shell-interpreter (pet-executable-find "python")
                            python-shell-virtualenv-root (pet-virtualenv-root))
                ;; Pytest (for python-pytest)
                ;;(setq-local python-pytest-executable (pet-executable-find "pytest"))
                ;; Eglot
                (require 'eglot)
                (setq-local eglot-server-programs
                            (cons `((python-mode python-ts-mode)
                                    . (,(pet-executable-find "pylsp")))
                                  eglot-server-programs))                              
                ))
  :custom
  (pet-toml-to-json-program-arguments '("-f" "-" "-r" "toml" "-w" "json"))
  (pet-yaml-to-json-program-arguments '("-f" "-" "-r" "yaml" "-w" "json")))

[zyxir]

Here's how I did it with eglot

@egelja Impressive! I have no doubt your config works well for most people. But if Pet should add official support for Eglot, these things should be taken into consideration:

1. The user may not be an Eglot user, or the user prefers launching Eglot manually via M-x eglot, so better wrap the Eglot config into a with-eval-after-load block.
2. The user may use another language server than pylsp, for example pyright or jedi-language-server, so the language server name should not be hard-wired. I think advising eglot--executable-find may be a better option.

Anyway, adding Eglot support should not be a hard thing, I just want to hear the author's acknowledgement. What concerns me most is how to implement the Eshell integration.

[egelja]

The user may not be an Eglot user, or the user prefers launching Eglot manually via M-x eglot, so better wrap the Eglot config into a with-eval-after-load block.

Yeah, I was just being a bit lazy here. Good point.

The user may use another language server than pylsp, for example pyright or jedi-language-server, so the language server name should not be hard-wired. I think advising eglot--executable-find may be a better option.

The only problem is that you cannot override functions in a buffer-local way, and we don't want to touch exec-path, so maybe having a custom variable with the preferred language server would allow us to do this in a customizable way.

[zyxir]

The only problem is that you cannot override functions in a buffer-local way, and we don't want to touch exec-path, so maybe having a custom variable with the preferred language server would allow us to do this in a customizable way.

I looked at eglot-server-programs, the Python part is:

((python-mode python-ts-mode)
 . ,(eglot-alternatives
     '("pylsp" "pyls" ("pyright-langserver" "--stdio") "jedi-language-server")))

So maybe we can just do

(setq-local eglot-server-programs
            (cons
             `((python-mode python-ts-mode)
               . ,(eglot-alternatives
                   (mapcar
                    (lambda (server)
                      (if (string-suffix-p "pyright-langserver" server)
                          `(,server "--stdio")
                        server))
                    (mapcar (lambda (server)
                              (pet-executable-find server))
                            '("pylsp"
                              "pyls"
                              "pyright"
                              "jedi-language-server")))))
             eglot-server-programs))

This supports every language server that Eglot support, and does it the Eglot way: When there are several language server available at the same time, prompt the user to choose one.

[wyuenho]

Problem 1:

@zyxir that snippet looks like a sensible solution.

Problem 2:

I don't use Eshell, haven't touched Eshell for probably over a decade and have no idea why people still use it. What do I need to support it?

[wyuenho]

@zyxir I just took a look at your problem 2 description, eshell and your init.el file. I think there's some misconception here. The point of PET is to eliminate the need to manipulate the environment, which includes any manipulation of exec-path, the PATH environment variable of the Emacs process, and by extension, the need to activate any virtual environment at all just so you can get to your tools.

For the occasional need where you have to pip install something into the right virtualenv, either M-! pip install package RET, or use pippel and set pippel-python-command to (pet-executable-find "python").

[zyxir]

have no idea why people still use it

@wyuenho There is also a tons of articles recommending Eshell on the internet, and I think whether Eshell is powerful is not the key point here.

that snippet looks like a sensible solution.

And should this solution be made a part of PET?

[wyuenho]

Sure! I don't use eglot so feel free to send over a PR and I'll take a look

[egelja]

This doesn't work for me...

eglot-alternatives calls executable-find on the return value of pet-executable-find, which returns nil (as pet-executable-find returns the full path already).

Then, eglot states I have no server programs found.

[egelja]

Maybe we could do something like this:

(defcustom pet-prefered-eglot-server nil)

(when pet-prefered-eglot-server
  (setq-local eglot-server-programs
              (cons `((python-mode python-ts-mode)
                      . ,(cons 
                           (pet-executable-find (car pet-prefered-eglot-server))
                           (cdr pet-prefered-eglot-server))
                    eglot-server-programs)))  

And then the variable is set like this

(setq pet-prefered-eglot-server '("server" "arg1" "arg2" "..."))

[zyxir]

This doesn't work for me...

@egelja What is your Emacs and Eglot version? I am using Emacs 29 (built from source) where Eglot is built-in. In my version, executable-find works on full paths, like the code shown below:

;; The following expression is evaluated in the *scratch* buffer.
(executable-find "/home/zyxir/.venv/bin/pylsp")
"/home/zyxir/.venv/bin/pylsp"

If executable-find returns nil on a full path, that may be a BUG of executable-find itself, which was fixed at some version of Emacs, so that it behaves differently on Emacs 29, the version I am currently using.

Anyway, I think introducing more local variables may create more mess, and maybe advising executable-find is a better way.

[egelja]

It may have to do with calling executable-find on a TRAMP path. Anyways, making the multiple executable-find calls over TRAMP is quite slow (takes ~30sec to open a file), and this reduces it to one.

[zyxir]

It may have to do with calling executable-find on a TRAMP path. Anyways, making the multiple executable-find calls over TRAMP is quite slow (takes ~30sec to open a file), and this reduces it to one.

@egelja I totally understand the inconvenience, but Eglot calls executable-find multiple times by default because there are indeed multiple possible language servers. I think PET should interfere less with the default behavior of Eglot and your workaround should better be in personal configuration. However I seldom use TRAMP myself so my suggestion may be impractical.

[wyuenho]

I'm going to remove support for eglot as there's no reasonable way to support supplying :initializationOptions to the server arguments. The author has a penchant to offer lazy APIs in all of his packages by way of not offering customizable variables. The only reasonable way to use eglot is to completely redefine eglot-server-programs using the hundreds of LSP servers defined in lsp-mode and their default customizations as examples.

For now, the best way to do use pet-mode to help you find the LSP server for eglot is something like this:

(add-hook 'pet-mode-hook
          (lambda ()
            (with-eval-after-load 'eglot
              (setq-local 'eglot-server-programs
                          (append
                           `(((python-mode python-ts-mode) . (,(pet-executable-find "pyright-langserver") "--stdio")))
                           (copy-alist (default-value 'eglot-server-programs)))))))

I'll document this in the README.

[zyxir]

That is unfortunate. Maybe it's good time for me to start using Lsp-mode as well.

[wyuenho]

I just spent a weekend to come up with an eglot setup that works with pet, but alas, it can't be put into pet without doing some inscrutable parsing into eglot-server-programs to insert the initializationOptions. I might be able to, but it's going to be nasty and I really don't want to. Let me know if something like this work for you both, if so, I'll think about if I can put this into pet without making a mess.

(require 'eglot)

(defun pet-eglot--executable-find-advice (fn &rest args)
  "Advice `eglot--executable-find' to use `pet-executable-find' to
look up Python languager servers."
  (pcase-let ((`(,command . ,_) args))
    (if (member command '("pylsp" "pyls" "pyright-langserver" "jedi-language-server"))
        (pet-executable-find command)
      (apply fn args))))

(defun pet-eglot--uri-to-path-advice (fn &rest args)
  "This is used to fix an Eglot bug.

Bug description:

Pyright needs to be configured after initialization. On a
workspace/configuration request from the server, Eglot will
receive a `scopeUri' that is a directory but does not end in a /.

The `scopeUri' is passed to `eglot--uri-to-path', which does not
canonicalize the path. The output is then given to
`eglot--workspace-configuration-plist', which assumes the path is
always a file instead of a directory, so `file-name-directory'
simply returns the parent of the project directory instead of the
project directory, which in turns sets the `default-directory'
incorrectly for looking up the directory local variables."
  (let ((result (apply fn args)))
    (if (file-directory-p result)
        (file-name-as-directory result)
      result)))

(defun pet-eglot-workspace-configuration (server)
  (when (string-match-p
         "pyright-langserver"
         (car (process-command (jsonrpc--process server))))
    `(:python
      (:pythonPath
       ,(pet-executable-find "python")
       :venvPath
       ,(pet-virtualenv-root)))))

(defun pet-eglot--workspace-configuration-plist-advice (fn &rest args)
  "`eglot-workspace-configuration' is a buffer local variable that
can accept a function value, but it's invoked in a temp buffer,
so this is pretty much the only way to set it to a function
without mucking around with the dir-locals functions."
  (let ((eglot-workspace-configuration 'pet-eglot-workspace-configuration))
    (apply fn args)))

(defun pet-eglot-setup ()
  (advice-add 'eglot--executable-find :around 'pet-eglot--executable-find-advice)
  (advice-add 'eglot--uri-to-path :around 'pet-eglot--uri-to-path-advice)
  (advice-add 'eglot--workspace-configuration-plist :around 'pet-eglot--workspace-configuration-plist-advice)

  (let ((server-programs (assoc-delete-all '(python-mode python-ts-mode)
                                           (copy-tree (default-value 'eglot-server-programs))))
        (python-server-programs `((python-mode python-ts-mode) .
                                  ,(eglot-alternatives
                                    `(("pylsp"
                                       :initializationOptions
                                       (:pylsp
                                        (:plugins
                                         (:jedi
                                          (:environment
                                           ,(pet-virtualenv-root))))))
                                      ("pyls"
                                       :initializationOptions
                                       (:pyls
                                        (:plugins
                                         (:jedi
                                          (:environment
                                           ,(pet-virtualenv-root))))))
                                      ("pyright-langserver" "--stdio")
                                      "jedi-language-server")))))
    (setq-local eglot-server-programs (cons python-server-programs server-programs)))

  (eglot-ensure))

(add-hook 'python-base-mode-hook 'pet-mode -10)
(add-hook 'python-base-mode-hook 'pet-eglot-setup)

[zyxir]

The latest commit already worked for me. After adding your snippet into my configuration it worked as well.

It seems that you make pylsp virtualenv-aware by making jedi virtualenv-aware, so that completion, hover, references, etc. are project-related. My solution was using pylsp inside the project virtualenv, as different toolset and configuration might be needed for different projects. I often manage pylsp and needed accesories with dev-dependencies in pyproject.toml, and configured tools integrated into pylsp, like ruff, black, and mypy, in a per-project manner.

I think a mature solution would be using the global pylsp by default, while providing project-specific completions (empowered by your snippet), and when there is a local pylsp, use that instead. Though such solution is a bit nasty to implement, it is a solution.

By the way, I often tweak my LSP configuration via eglot-workspace-configuration which is very easy to set up and can become buffer-local. I wonder if it can be an alternative to :initializationOptions.

[wyuenho]

@zyxir I'm curious, how do you usually use eglot-workspace-configuration'? Do you do what the manual suggests and just drop a.dir-locals.el` file into the directory?

[zyxir]

@wyuenho Actually I don't configure eglot-workspace-configuration locally at this point, but I will when it is necessary. Currently I configure my pylsp to make sure some third-party extensions like mypy, black, and ruff are actually used, like this, because I want them available in every use scenario, including writting simple scripts.