emacs-lsp / lsp-pyright

lsp-mode :heart: pyright
https://emacs-lsp.github.io/lsp-pyright
GNU General Public License v3.0
291 stars 25 forks source link

`lsp-pyright-venv-path` vs. `lsp-pyright-venv-directory`? #49

Open joostkremers opened 3 years ago

joostkremers commented 3 years ago

Yesterday's commit 3fd23f17ddff8d22115f7b0b9d3f4ed8fb90add3 added the option lsp-pyright-venv-directory, but there was already an option lsp-pyright-venv-path, which, judging by the description, has the same function.

BTW, both options have an incomplete :type spec. Since they can also be nil, the type should be something like this:

(defcustom lsp-pyright-venv-path nil
  "Path to folder with subdirectories that contain virtual environments.
Virtual Envs specified in pyrightconfig.json will be looked up in this path."
  :type '(choice (string :tag "Venv path")
                 (const :tag "No venv path" nil))
  :group 'lsp-pyright)

That way, they are displayed correctly in the Customize buffer and can be properly type-checked.

seagle0128 commented 3 years ago

lsp-pyright-venv-path is absolute path, while lsp-pyright-venv-directory is the folder name.

See:

(defun lsp-pyright-locate-venv ()
  "Look for virtual environments local to the workspace."
  (or lsp-pyright-venv-path
      (and lsp-pyright-venv-directory
           (-when-let (venv-base-directory (locate-dominating-file default-directory lsp-pyright-venv-directory))
             (concat venv-base-directory lsp-pyright-venv-directory)))
      (-when-let (venv-base-directory (locate-dominating-file default-directory "venv/"))
        (concat venv-base-directory "venv"))
      (-when-let (venv-base-directory (locate-dominating-file default-directory ".venv/"))
        (concat venv-base-directory ".venv"))))
joostkremers commented 3 years ago

Ah, thanks for the clarification. Admittedly, I didn't look at the code, just at the doc strings, which didn't make the difference clear to me. If you would like, I could prepare a PR to suggest an update to the doc strings.

seagle0128 commented 3 years ago

PR is welcome