Allow to pass the FileMode options in the vim plugin #1319

shaoran · 2020-03-27T00:57:34Z

The vim plugin takes now into consideration the target-version value of
the pyproject.toml file.

You can also specify a default target version with the configuration
variable g:black_target_version.

Vim's :Black command accepts now the following optional arguments:

line_length
string_normalization
black_target_version

Those allow you to override any settings that come either from vim's
configuration or the pyproject.toml file.

It also supports supports a simple tab completion to allow to pass the
arguments quicky.

The vim plugin takes now into consideration the `target-version` value of the `pyproject.toml` file. You can also specify a default target version with the configuration variable `g:black_target_version`. Vim's `:Black` command accepts now the following optional arguments: - `line_length` - `string_normalization` - `black_target_version` Those allow you to override any settings that come either from vim's configuration or the `pyproject.toml` file. It also supports supports a simple tab completion to allow to pass the arguments quicky.

I didn't realize that it was a boolan value.

JelleZijlstra · 2021-04-11T21:08:22Z

Sorry for leaving this waiting for so long! There are a few merge conflicts now. Do we still need this change?

shaoran · 2021-04-12T13:48:10Z

Hi

Sorry for leaving this waiting for so long! There are a few merge conflicts now. Do we still need this change?

I'd say yes, because :Black has no options and if you need to pass some command line option like setting python's target version or the line length, you cannot do it directly from vim, you have to do that from the console. In my eyes, that defeats the purpose of having black inside vim, if I have to open a console anyway.

This PR would allow you to do the following in vim: :Black target_version="py27". And because it's something I need all the time, I have this in my vim config:

Plug 'shaoran/black', { 'branch': 'allow_override_args_19.10b0' }

It's so long since I've created this PR that my changes might now work anymore, I'd have to check them, but I'll have time to do it at the weekend.

felix-hilden · 2021-06-13T19:41:47Z

If I may, I'll provide a review posted on our Discord in these messages: 1, 2 by Godlygeek:

The core issue seems to be whether or not we'd like to be able to pass arguments as comma-separated key=value pairs. Some concerns were that:

The format is unusual: ordinarily the arguments look more like CLI flags, being space-separated
Passing varying configuration is an odd design choice since Black is all about consistent formatting, which can be achieved with file configuration or setting g_* variables in Vim

Without knowing too much about the plugin system, I do share these concerns. Target version is of course one argument whose value is naturally varied. Could you comment a bit on your view on the matter, @shaoran? Much appreciated!

shaoran · 2021-06-26T11:32:09Z

@felix-hilden I'm sorry, I have been very busy this week and was not able to reply. I cannot at the moment, but soon I will answer your questions.

felix-hilden · 2021-09-11T20:41:30Z

Hi @shaoran, any updates on this? It'd be nice to bring this issue closer to the finish!

shaoran · 2021-09-12T00:50:55Z

@felix-hilden I'm so sorry, I've forgotten about this. I'll try to take a look tomorrow or during the next week.

shaoran · 2021-09-14T19:38:36Z

@felix-hilden I tried merging upstream/main in my branch and I got a conflict in README.md. Back in 19.10b0 (from which
I originally branched of) the documentation was directly in the README.md
file. Now it's been moved to https://black.readthedocs.io

Is the documentation source code for readthedocs still in the repository?

shaoran · 2021-09-14T19:45:12Z

It changed a lot, I basically would have to rewrite the whole thing again. OK, I'll try that.

felix-hilden · 2021-09-14T20:54:31Z

Yep, the documentation source is in the docs/ folder, particularly docs/integrations might be good for this.

shaoran · 2021-09-14T21:03:35Z

Ok. this will take me a little bit longer. Since there are so many changes since I created the pull request, all my changes are useless and basically have to write them again. I hope, tomorrow I can push a new version.

felix-hilden · 2021-09-14T21:09:24Z

No worries! And I'd also like to remind you about the review points I left above 😃

shaoran · 2021-09-22T19:41:55Z

@felix-hilden sorry that it took a while, but I've been very busy at work.

I try to address the concerns here

The format is unusual: ordinarily the arguments look more like CLI flags, being space-separated

I made a change so that arguments are space-separated.

Passing varying configuration is an odd design choice since Black is all about consistent formatting, which can be achieved with file configuration or setting g_* variables in Vim

Yes, that is true, that's why I removed passing the other options like fast, linelength, etc. The only option remaining is target_version and that because of convenience.

I maintain a python library that targets multiple python version, some modules only work with Py27, some with Py36, etc. Very often I have those files open at the same time and I'd like to format one with py27 as target and the other with py36 as target without having to set up variables and special rules in my config. So that's why it is for me more convenient to just pass target_version=py27 directly to :Black when I need it. And thanks to tab completion, I don't even have to type it, just press TAB.

Like I said before, I already use this on my environment and has been a huge time saver.

CI seems to be failing because of changelog / Changelog Entry Check (pull_request). Clicking on Details reveals that I should add Please add '(#1319)' change line to CHANGES.md but I'm not sure I should be the one adding this line, besides I don't even know under which release this pull request would be merged (if accepted). So do I have to add that line, or do you do that?

felix-hilden

Thanks for the updates! Having target_version as an argument makes total sense to me as well 👍 I had a look at the code and it looks reasonable, although I don't know vim plugins.

Our changelog is currently at the release, so a new heading needs to be added to the top. So please add a new ## Unreleased heading and place the change under ### Integrations. I left one comment as well. Since we're short on vim wizards, I'm willing to take your word on this working just fine 😄

autoload/black.vim

felix-hilden · 2021-09-23T06:33:31Z

docs/integrations/editors.md

@@ -116,6 +116,8 @@ Wing supports black via the OS Commands tool, as explained in the Wing documenta
 Commands and shortcuts:

 - `:Black` to format the entire file (ranges not supported);
+  - you can optionally pass `target_version=<version>` where `<version>` is one of
+    `[py27, py36, py37, py38, py39]` to set the target version.


Also: we should probably not advertise specific versions here. I mean it's fine, but seems like another place to eventually get stale as we add support and drop support for older versions. Should be fine just to mention that it's the same argument as in the CLI, right? It would be a bit of an indirection though. Thoughts?

Should be fine just to mention that it's the same argument as in the CLI, right?

Yes, that would be better, and because of TAB completion, you get the correct list anyway. I'll change that.

Very nice 👌

in the command line

to make CI happy

if no target verison is found

shaoran · 2021-09-27T15:24:41Z

So, what is the status of this merge request? Is it going to be merged?

felix-hilden · 2021-09-27T15:35:16Z

I'd say yes, but sadly I don't have commit access to the repo. I think if you agree that the conversation is resolved it's only a matter of time 👌

shaoran · 2021-09-28T13:52:03Z

I'd say yes, but sadly I don't have commit access to the repo. I think if you agree that the conversation is resolved it's only a matter of time ok_hand

I hope so. Yesterday I had to merge it again with upstream because it was showing once again conflicts with upstream (luckily on the changelog). I'm afraid, that if it takes again weeks or even months (like when I created the pull request), my changes will be once again completely outdated and I would have to do it from scratch again.

felix-hilden · 2021-09-28T14:42:29Z

I feel you, sadly sometimes we're busy doing other things. But I'll try to push this to get resolved, and since it is pretty much done and big overhauls aren't planned, I think we're good 👍

shaoran added 3 commits March 27, 2020 01:53

chore: also allowing to pass the fast flag

dffcfe1

Fixing how to handle string_normalization argument

d1b4242

I didn't realize that it was a boolan value.

JelleZijlstra assigned JelleZijlstra and unassigned JelleZijlstra May 8, 2020

autophagy added the C: vim label May 8, 2020

ichard26 added the S: awaiting response label Jul 16, 2021

shaoran added 4 commits September 22, 2021 21:14

Merge remote-tracking branch 'upstream/main' into allow_override_args

102eabe

Merge remote-tracking branch 'upstream/main' into allow_override_args

7cd11bb

doc: updating vim integration docs

f9c7f77

chore: running pre-commit run --all-files

7c27c10

felix-hilden requested changes Sep 23, 2021

View reviewed changes

autoload/black.vim Show resolved Hide resolved

felix-hilden reviewed Sep 23, 2021

View reviewed changes

felix-hilden removed the S: awaiting response label Sep 23, 2021

shaoran added 3 commits September 23, 2021 23:08

doc: mentioning that target_version is the same as

9427edc

in the command line

chore: add this pull request to the changelog

d983f74

to make CI happy

chore: catch KeyError and print warning

956d190

if no target verison is found

felix-hilden approved these changes Sep 24, 2021

View reviewed changes

Merge remote-tracking branch 'upstream/main' into allow_override_args

68e63d9

JelleZijlstra merged commit 09915f4 into psf:main Sep 29, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow to pass the FileMode options in the vim plugin #1319

Allow to pass the FileMode options in the vim plugin #1319

shaoran commented Mar 27, 2020

JelleZijlstra commented Apr 11, 2021

shaoran commented Apr 12, 2021 •

edited

Loading

felix-hilden commented Jun 13, 2021

shaoran commented Jun 26, 2021

felix-hilden commented Sep 11, 2021

shaoran commented Sep 12, 2021

shaoran commented Sep 14, 2021

shaoran commented Sep 14, 2021

felix-hilden commented Sep 14, 2021

shaoran commented Sep 14, 2021

felix-hilden commented Sep 14, 2021

shaoran commented Sep 22, 2021

felix-hilden left a comment

felix-hilden Sep 23, 2021

shaoran Sep 23, 2021

felix-hilden Sep 24, 2021

shaoran commented Sep 27, 2021

felix-hilden commented Sep 27, 2021

shaoran commented Sep 28, 2021

felix-hilden commented Sep 28, 2021

Allow to pass the FileMode options in the vim plugin #1319

Allow to pass the FileMode options in the vim plugin #1319

Conversation

shaoran commented Mar 27, 2020

JelleZijlstra commented Apr 11, 2021

shaoran commented Apr 12, 2021 • edited Loading

felix-hilden commented Jun 13, 2021

shaoran commented Jun 26, 2021

felix-hilden commented Sep 11, 2021

shaoran commented Sep 12, 2021

shaoran commented Sep 14, 2021

shaoran commented Sep 14, 2021

felix-hilden commented Sep 14, 2021

shaoran commented Sep 14, 2021

felix-hilden commented Sep 14, 2021

shaoran commented Sep 22, 2021

felix-hilden left a comment

Choose a reason for hiding this comment

felix-hilden Sep 23, 2021

Choose a reason for hiding this comment

shaoran Sep 23, 2021

Choose a reason for hiding this comment

felix-hilden Sep 24, 2021

Choose a reason for hiding this comment

shaoran commented Sep 27, 2021

felix-hilden commented Sep 27, 2021

shaoran commented Sep 28, 2021

felix-hilden commented Sep 28, 2021

shaoran commented Apr 12, 2021 •

edited

Loading