Skip to content
This repository has been archived by the owner on Nov 19, 2021. It is now read-only.

Add an option for --single-quote, but strongly prefer double quote in readme. #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bryanhelmig
Copy link
Member

@bryanhelmig bryanhelmig commented May 29, 2018

Simply adds a --single-quote option, though it appears that upstream is not keen in adding this complexity to trunk. So out of respect to ambv and the fundamental intent of the project, we won't be opening a PR to the core repo (as it will likely just result in useless bike shedding).

However, if Zapier decides to make use of black, we'll likely maintain this internal fork and will regularly rebase it. If upstream ever decides to entertain single quotes, certainly happy to provide this code to the community!

Mostly provided here as an ongoing reference point.

@ambv
Copy link

ambv commented May 29, 2018

You could just add another entry to FileMode which would greatly save you on the size of this patch. That will help you keep up with master development in the future. The modes are flags so are composable.

@bryanhelmig
Copy link
Member Author

That is a good idea @ambv -- wasn't sure if the FileMode was expected to house arbitrary extensions.

@bryanhelmig bryanhelmig force-pushed the single-quote-option branch 2 times, most recently from 96253c3 to 4cdef98 Compare May 30, 2018 18:43
black.py Outdated
@@ -126,6 +126,7 @@ class FileMode(Flag):
AUTO_DETECT = 0
PYTHON36 = 1
PYI = 2
SINGLE_QUOTE = 4
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you decide to keep the fork, note: psf@8ebbd26

(you'll want to pick a higher number than 4; say 64 since we're unlikely to reach it any time soon)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh nice, thanks @ambv -- we'll explore this new flag. Good compromise on --skip-string-normalization. 👍

@bryanhelmig
Copy link
Member Author

Handy rebase commands:

git fetch upstream
git checkout master
git merge upstream/master
git push
git checkout single-quote-option
git rebase -i master

@bryanhelmig bryanhelmig force-pushed the single-quote-option branch 2 times, most recently from 70dd1f2 to 4a44add Compare August 10, 2018 21:49
@charettes
Copy link

Just rebased on top of 18.9b0.

samplantzapier pushed a commit that referenced this pull request Oct 21, 2021
This commit fixes parsing of the skip-string-normalization option in vim
plugin. Originally, the plugin read the string-normalization option,
which does not exist in help (--help) and it's not respected by black
on command line.

Commit history before merge:

* fix string normalization option in vim plugin
* fix string normalization option in vim plugin
* Finish and fix patch (thanks Matt Wozniski!)

FYI: this is totally the work and the comments below of Matt (AKA godlygeek)

This fixes two entirely different problems related to how pyproject.toml
files are handled by the vim plugin.

=== Problem #1 ===

The plugin fails to properly read boolean values from pyproject.toml.
For instance, if you create this pyproject.toml:

```
[tool.black]
quiet = true
```

the Black CLI is happy with it and runs without any messages, but the
:Black command provided by this plugin fails with:

```
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "<string>", line 102, in Black
  File "<string>", line 150, in get_configs
  File "<string>", line 150, in <dictcomp>
  File "/usr/lib/python3.6/distutils/util.py", line 311, in strtobool
    val = val.lower()
AttributeError: 'bool' object has no attribute 'lower'
```

That's because the value returned by the toml.load() is already a
bool, but the vim plugin incorrectly tries to convert it from a str to a bool.

The value returned by toml_config.get() was always being passed to
flag.cast(), which is a function that either converts a string to an
int or a string to a bool, depending on the flag. vim.eval()
returns integers and strings all as str, which is why we need the cast,
but that's the wrong thing to do for values that came from toml.load().
We should be applying the cast only to the return from vim.eval()
(since we know it always gives us a string), rather than casting the
value that toml.load() found - which is already the right type.

=== Problem #2 ===

The vim plugin fails to take the value for skip_string_normalization
from pyproject.toml. That's because it looks for a string_normalization
key instead of a skip_string_normalization key, thanks to this line
saying the name of the flag is string_normalization:

black/autoload/black.vim (line 25 in 05b54b8)
```
 Flag(name="string_normalization", cast=strtobool),
```

and this dictcomp looking up each flag's name in the config dict:

black/autoload/black.vim (lines 148 to 151 in 05b54b8)
```
 return {
   flag.var_name: flag.cast(toml_config.get(flag.name, vim.eval(flag.vim_rc_name)))
   for flag in FLAGS
 }
```

For the second issue, I think I'd do a slightly different patch. I'd
keep the change to invert this flag's meaning and change its name that
this PR proposes, but I'd also change the handling of the
g:black_skip_string_normalization and g:black_string_normalization
variables to make it clear that g:black_skip_string_normalization is
the expected name, and g:black_string_normalization is only checked
when the expected name is unset, for backwards compatibility.

My proposed behavior is to check if g:black_skip_string_normalization
is defined and to define it if not, using the inverse of
g:black_string_normalization if that is set, and otherwise to the
default of 0. The Python code in autoload/black.vim runs later, and
will use the value of g:black_skip_string_normalization (and ignore
g:black_string_normalization; it will only be used to set
g:black_skip_string_normalization if it wasn't already set).

---

Co-authored-by: Matt Wozniski <[email protected]>

* Fix plugin/black.vim (need to up my vim game)

Co-authored-by: Matt Wozniski <[email protected]>

Co-authored-by: Richard Si <[email protected]>
Co-authored-by: Matt Wozniski <[email protected]>
Co-authored-by: Matt Wozniski <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants