wyuenho / emacs-pet

Tracks down the correct Python tooling executables from your virtualenvs so you can glue the binaries to Emacs and delete code in init.el
GNU General Public License v3.0
121 stars 15 forks source link

Default root to default-directory if not in a project #31

Closed akreisher closed 6 months ago

akreisher commented 10 months ago

Thanks for this package, I am using it in eshell to automatically "activate" venvs for the current eshell dir with the following snippet:

  (defun my:eshell-insert-pet-venv (path)
    "Advice for adding python venv to eshell PATH."
    (if-let ((venv (pet-virtualenv-root)))
        (cons (expand-file-name "bin/" venv) path)
      path))

  (defun my:enable-eshell-pet ()
    "Automatically activate python env in eshell."
    (interactive)
    (advice-add #'eshell-get-path :filter-return #'my:eshell-insert-pet-venv))

  (defun my:disable-eshell-pet ()
    (advice-remove #'eshell-get-path #'my:eshell-insert-pet-venv))

This works well for directories in recognized projects. When visiting a non-project directory with a venv, however, I run into a problem with pet-virtualenv-root returning the cached venv value even when leaving to another directory with no venv. I traced this to pet-virtualenv-root caching the venv location with a root key of nil (since pet-project-root returns nil). Since this root is used for all non-project dirs, pet-virtualenv-root returns an incorrect value for all the non-project dirs visited after the first one with a recognized venv.

For example, to reproduce with the above code, I can run:

cd /tmp
python -m venv venv
which python3  # /tmp/venv/bin/python3, as expected
cd / 
which python3  # still /tmp/venv/bin/python3,  expected system python

To fix, I just added a default of the current default-directory to the value of root used in pet-virtualenv-root, which causes it to correctly associate the found venv with the searched directory, regardless of whether it is in a project or not. Alternatively, we could not cache venvs with a root of nil, but I figured it would be better like this since root is only used by the caching mechanism here anyways.

wyuenho commented 7 months ago

In this case, shouldn't the solution be to make sure no (root . nil) entry is written into the cache in the first place?

wyuenho commented 6 months ago

See if f615a2fd3557a94642d0f95e52cf1f40decc99f5 works for you