Skip to content
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

Vim plugin fix string normalization option #1869

Merged
merged 5 commits into from Jun 12, 2021

Commits on Dec 15, 2020

  1. Copy the full SHA
    72bfc37 View commit details
    Browse the repository at this point in the history
  2. Copy the full SHA
    b00ac1d View commit details
    Browse the repository at this point in the history

Commits on Jun 12, 2021

  1. Copy the full SHA
    90efa66 View commit details
    Browse the repository at this point in the history
  2. 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 psf#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 psf#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 <mwozniski@bloomberg.net>
    ichard26 and godlygeek committed Jun 12, 2021
    Copy the full SHA
    962f64b View commit details
    Browse the repository at this point in the history
  3. Fix plugin/black.vim (need to up my vim game)

    Co-authored-by: Matt Wozniski <godlygeek@gmail.com>
    ichard26 and godlygeek committed Jun 12, 2021
    Copy the full SHA
    50522cf View commit details
    Browse the repository at this point in the history