black icon indicating copy to clipboard operation
black copied to clipboard

Reconsider the vim plugin provided in this repository

Open ichard26 opened this issue 3 years ago • 8 comments

Summary

Get rid of the Vim plugin (and associated documentation) provided this repository and point users to a third-party Vim plugin or plugins.

Why

  • We don't have enough resources to support the vim plugin. For one, the active maintainers (Jelle, Cooper, and I) don't use the plugin and aren't qualified to review vim plugin PRs. And patches and improvements are pretty much solely dependent on the wider Black contributor base.
    • Generally, it eats up even more of our limited resources for not much benefits.
  • Our plugin ain't great. Bugs, lack of features, and performance (+ other QOL issues) are all issues currently present.

Points for discussion

  • Should we do this?
  • What third-party plugin(s) should we recommend or at least list?
  • How should we should do this?
    • Are there features that exist in the official plugin that should be added to third-party plugins (to reduce impact)?

ichard26 avatar Mar 08 '21 00:03 ichard26

I second this. I’m lost in vscode these days, with a vim plugin, ironic.

I call for anyone who maintains a vim plugin for black or who’d like to port the official to a repo and maintain it to voice their proposals and plans and we can just make it so.

I could potentially maybe get another psf repository spun up dedicated for this too.

cooperlees avatar Mar 08 '21 01:03 cooperlees

If someone steps up to work on maintenance for the plugin, we could also give that person commit rights to this repo and keep the plugin here. I don't think it's a bad idea in principle to maintain editor plugins in this repo, it's just that they're hard to maintain when none of the maintainers use or understand them.

JelleZijlstra avatar Mar 08 '21 02:03 JelleZijlstra

[...] we could also give that person commit rights to this repo [...]

That makes me a bit uncomfortable since they might not be qualified to handle the usual Black core team duties yet can do them (poorly). I guess it's just a matter of trust though.

I don't think it's a bad idea in principle to maintain editor plugins in this repo, it's just that they're hard to maintain when none of the maintainers use or understand them.

TBH, I've been kicking around an idea of having a black-dev organization or something similar so we could host stuff like this while keeping the boundaries pretty clear. Now, obviously that ain't happening (far too much admin overhead). I'm split on whether editor plugins should be maintained in this repo. On the one hand, there's the "officiality" status and simplification of Black connected development. But there's the potential boundaries' issue which is scaring me.

In the end, this repo would a bit of work to accommodate side-by-side development of Black and its integrations, but it is possible. We also do host GitHub Actions integration and pre-commit already, so if I had to choose a side, +0.5 for "keep it here and onboard a new committer for it".

ichard26 avatar Mar 08 '21 04:03 ichard26

If I may chime in too, I'm for stricter separation. Not only is the current team not versed in Vim plugins (yeah me neither), but a dedicated repo could be well off without the distractions of development here in general. Officiality could be given by simply mentioning them in our documentation, and if a black-dev org is created, surely the repository can then be invited there!

So for what it's worth, I'm +0.7 on moving it 😄 An active contributor that is willing and able to work on the plugin would change things for sure. But I feel the separation of concerns is important too, especially if there would be other plugins on their way.

felix-hilden avatar Jun 10 '21 12:06 felix-hilden

Probably way less cool than the plugin, and probably have a lot of caveats, but I found a way to replace the plugin by adding this to my .vimrc:

set autoread
command! Black
            \   execute 'silent !black ' . expand('%')
            \ | execute 'redraw!'
autocmd BufWritePost *.py execute ':Black'

So whenever I run :Black or save it applies the black command from the path. It has the advantage that it is able to use any venv black you have in your path.

daisukixci avatar Dec 03 '21 18:12 daisukixci

Wanted to tag onto this since I've had some issues with this myself. The Vim integration here isn't great (as admitted) and actually even has some bugs in the formatting. Example:

#!/usr/bin/env python3

import math

x = 2
y = 3

r = math.sqrt(x**2 + y ** 3)

Notice above that the spacing between ** is different for x and y. With the official vim extension it formats this as:

#!/usr/bin/env python3

import math

x = 2
y = 3

r = math.sqrt(x ** 2 + y ** 3)

whereas if you were to call black on the file itself it would provide the following:

#!/usr/bin/env python3

import math

x = 2
y = 3

r = math.sqrt(x**2 + y**3)

This is obviously a small example and a small bug, but definitely something noticeable that means I can't fully trust the plugin. VSCode / others behave similarly to just calling black on the file itself, so clearly the vim plugin is in the wrong here.

To replace it, I've gone ahead and found averms/black-nvim. I'm already using neovim so I can get away with this, but for those using regular vim this won't be a good substitution. Plus, the instructions for install aren't abundantly clear either and I had some extra install steps that are not listed.

ALE has their own black formatter, but I found this to be a little heavy for my use. I don't really want to track / use all of ALE so I went without. Non-neovim users might find more success there.

On another note:

Probably way less cool than the plugin, and probably have a lot of caveats, but I found a way to replace the plugin by adding this to my .vimrc:

set autoread
command! Black
            \   execute 'silent !black ' . expand('%')
            \ | execute 'redraw!'
autocmd BufWritePost *.py execute ':Black'

So whenever I run :Black or save it applies the black command from the path. It has the advantage that it is able to use any venv black you have in your path.

This is a workable substitute but enforcing that set autoread is on won't work for everyone. Likewise, this really needs to be changed to a BufWritePre so that the file is formatted prior to writing (and that writing fails if formatting fails). I'm not sure how to do that in the moment, but BufWritePost / FileWritePost have weird semantics with regards to reloading / rewriting the file.

ThatGeoGuy avatar Nov 07 '22 20:11 ThatGeoGuy

@ThatGeoGuy you are probably running a different version of Black within your vim plugin.

JelleZijlstra avatar Nov 07 '22 20:11 JelleZijlstra

@ThatGeoGuy you are probably running a different version of Black within your vim plugin.

You're probably right on that, but I think that's kind of part of the issue too? I want to be sure if I'm using e.g. Pyenv in a directory that I'm using the version of black that is in the requirements.txt for that given project, not whatever virtualenv was constructed by the plugin.

There's a lot of complexity and nuance here that doesn't seem to be an issue in other editors / integrations so I think my migration away from the vim plugin stands. I should hope that others might notice this sort of disparity as well. I'm not trying to dunk on the developers here either - in fact I'm mostly just confirming that this is an existing issue and is one reason that leaving this up to the rest of the community (and there are examples that I linked of other parts of the community supporting this) is a fine idea. Whether it's to take burden off the devs or to just have a more canonical set of tooling that fits with how other editors handle this I think it'll be a huge boon compared to draining resources away from other work 😄

ThatGeoGuy avatar Nov 08 '22 00:11 ThatGeoGuy