-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] Allow Black to use a different Python version than Vim #4245
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
return True | ||
|
||
# TODO: Is this check about the Vim plugin, or about Black? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's about Black primarily; Black itself requires 3.8+ (since Python 3.7 is no longer supported). By default I would want the same version requirement for the Vim plugin, but if there's some reason to keep supporting older versions in the plugin code, I'm open to it.
@@ -120,7 +135,12 @@ def _initialize_black_env(upgrade=False): | |||
return True | |||
|
|||
if _initialize_black_env(): | |||
import black | |||
# TODO: Better handling of the case when import succeeds but Black doesn't |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this comment; if Black is uninstalled, surely the import would fail?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, not always. This can be triggered by Vim built with Py3.12 importing black from env/lib/python3.11/site-packages
, which this PR makes possible.
If black is uninstalled after it was used once in such a setup, then pip uninstall
leaves site-packages/black/__pycache__/
behind and that causes import black
to succeed:
$ env/bin/python3 -c "import black; print(dir(black)); print(black.__file__)"
['__doc__', '__file__', '__loader__', '__name__', '__package__', '__path__', '__spec__']
None
It's a corner case of a corner case, but it's a pain to troubleshoot when it bites.
An alternative approach is to detect the Python version in the virtualenv, and if it doesn't match what Vim was built with, then go through these steps:
This approach wouldn't enable mixing different Python versions and would use a bit of extra space for a new ~/.vim/black virtualenv, but should still leave the user with a working Black and no chance of confusion. |
After giving this some more thought, and further discussion in pypa/pip#11835, I'm now convinced that my approach isn't ideal. Mixing Python versions can have undesired side effects. |
Description
This branch should fix #2547
It is sometimes the case that Vim was built with a different Python version than the one used by Black.
One such scenario is when the black virtualenv was created by an older version of Vim and then the system was upgraded in place.
Another scenario is when
g:black_use_virtualenv
is disabled and the manually created environment ing:black_virtualenv
uses a different Python version. Right now Ubuntu 24.04 builds Vim with Python 3.12 but the system uses Python 3.11 as default, making this scenario more likely to happen:Currently,
:BlackVersion
reports Vim's Python version instead of the one found in the virtualenv. This PR fixes that inconsistency as well.Finally, I added a few TODO comments for issues that I encountered while testing various scenarios but didn't have the time to fix in this PR.
Checklist - did you ...
CHANGES.md
if necessary?