davidhalter / jedi-vim

Using the jedi autocompletion library for VIM.
MIT License
5.27k stars 370 forks source link

The buffer-local variable `b:jedi_environment_path` #1059

Closed jamescherti closed 3 years ago

jamescherti commented 3 years ago

The buffer-local variable b:jedi_environment_path was added to jedi-vim to allow the user to override the global variable g:jedi#environment_path in the current buffer (useful to set a different virtualenv or Python version in the current buffer).

bew commented 3 years ago

Is it possible to name it b:jedi#environment_path for consistency with the global?

davidhalter commented 3 years ago

@bew Good catch! I personally didn't see that. I agree that would probably make sense. IMO it's still a mistake that the hash is in there, but it's in there everywhere. We should probably try to be consistent.

jamescherti commented 3 years ago

@bew @davidhalter I tried to name the buffer-local variable b:jedi#environment_path to be consistent with the global variables, but I received the following error message from Vim:

:let b:jedi#environment_path="/home/user/.venv"
E461: Illegal variable name: b:jedi#environment_path

-- James Cherti

jamescherti commented 3 years ago

@davidhalter @bew According to the Vim documentation https://vimhelp.org/eval.txt.html#internal-variables : "An internal variable name can be made up of letters, digits and '_'. But it cannot start with a digit.". Internal variables include buffer and global variables.

The documentation does not seem to mention that characters like hashes can be used in variable names. It may explain the Vim error message: "E461: Illegal variable name: b:jedi#environment_path".

I've also noticed that the developers of Vim plugins, like Ale, that allow users to use buffer-local variables to override global variables chose to use underscores instead of hashes in their naming convention. I wonder if it is because they had the same issue?

Here are the solutions I suggest: Solution 1 - Replace "g:jedi#" variables with "g:jedi" (and keep backward compatibility with "g:jedi#"). Solution 2 - Use a different naming convention for buffer-local variables: "b:jedi" (not consistent with global variables, but easier to implement).

What do you think?

bew commented 3 years ago

I'd say solution 2 for simplicity and to keep this pr on point. If @davidhalter wants to change all option names (with fallback) to use underscores it can be done in a later pr.

davidhalter commented 3 years ago

@jamescherti I agree with @bew, leave it as it is for now. I did not know that it was not possible, sorry for that.


I wanted to merge, but there was an issue with the tests, I think it's related. Right?

jamescherti commented 3 years ago

@davidhalter @bew The exception jedi_vim.VimError was fixed in the last commit:

jedi_vim.VimError: Vim(let):E121: Undefined variable: b:jedi_environment_path; created by 'b:jedi_environment_path'

You should be able to merge now.

davidhalter commented 3 years ago

Thanks a lot!

jamescherti commented 3 years ago

Thank you @davidhalter and @bew for the quick approval and merge!