search

emacs-lsp / lsp-python-ms

lsp-mode :heart: Microsoft's python language server
https://emacs-lsp.github.io/lsp-python-ms
BSD 3-Clause "New" or "Revised" License
190 stars 41 forks source link

lsp-python-ms-locate-python locates the wrong python instance #122

Closed randomphrase closed 4 years ago

randomphrase commented 4 years ago

I am using Conda but I believe this problem can be exhibited with other virtual envs. Here is my (simplified) directory heirarchy:

Basically the problem is that when loading my_file.py the wrong python interpreter is located. This can be verified by calling (lsp-python-ms-locate-python "/path/to/venv/zzz_env/src/my_file.py"), which results in /path/to/venv/aaa_env/bin/python. I would expect it to locate the python in the containing environment, namely /path/to/venv/zzz_env/bin/python.

I believe the problem is caused by the lsp-python-ms--dominating-venv-python function, namely:

(defun lsp-python-ms--dominating-venv-python (&optional dir)
  "Look for directories that look like venvs"
  (let* ((path (or dir default-directory))
         (dominating-venv (locate-dominating-file path #'lsp-python-ms--venv-dir)))
    (when dominating-venv
      (lsp-python-ms--venv-python (lsp-python-ms--venv-dir dominating-venv)))))

Here, the dominating-venv symbol is set to the directory containing the virtual envs, which in my case is ~/venv. The subsequent call to lsp-python-ms--venv-dir picks the first virtual environment in this directory, namely ~/venv/aaa_env.

I believe it should be using the lsp-python-ms--venv-python predicate to locate-dominating-file instead.

Please let me know if my description is not clear or if I can provide any further details. Thanks.

seagle0128 commented 4 years ago

Would you mind to create a PR?

randomphrase commented 4 years ago

Thanks!

andrew-christianson commented 4 years ago

FYI I think the fix to this broke lsp-python-ms--dominating-venv-python for cases like 

project_dir/
     pkg/
         src.py <- locate from a buffer here
     venv/ <- misses this venv
         ....

I'll open a PR when I get a chance (need to disentangle branches on my fork)

edit: I see also there's an issue complaining of the same (#126)

randomphrase commented 4 years ago

Nice one, can confirm that this still works for my directory structure.