black icon indicating copy to clipboard operation
black copied to clipboard

style regression: too long single line docstrings have their quotes moved to a new line.

Open onerandomusername opened this issue 1 year ago • 5 comments

Describe the bug

Single-line docstrings that are longer than the line length have the final three " moved to a new line.

To Reproduce

For example, take this code:

from typing import Optional

# leaves this the same
def large_image_url(self) -> Optional[str]:
    """Optional[:class:`str`]: Returns a URL pointing to the large image asset of this activity, if applicable.
    """
    pass


# leaves this the same too
def large_image_url(self) -> Optional[str]:
    """
    Optional[:class:`str`]: Returns a URL pointing to the large image asset of this activity, if applicable.
    """
    pass


# depending on the length of the docstring, this one *may* be left as-is
def large_image_url(self) -> Optional[str]:
    """
    Optional[:class:`str`]: Returns a URL pointing to the large image asset of this activity, if applicable."""
    pass


# but this one is modified
def large_image_url(self) -> Optional[str]:
    """Optional[:class:`str`]: Returns a URL pointing to the large image asset of this activity, if applicable."""
    pass

And run it with these arguments:

$ black file.py --preview

We get this format:

from typing import Optional

# leaves this the same
def large_image_url(self) -> Optional[str]:
    """Optional[:class:`str`]: Returns a URL pointing to the large image asset of this activity, if applicable.
    """
    pass


# leaves this the same too
def large_image_url(self) -> Optional[str]:
    """
    Optional[:class:`str`]: Returns a URL pointing to the large image asset of this activity, if applicable.
    """
    pass


# this one is also left as-is
def large_image_url(self) -> Optional[str]:
    """
    Optional[:class:`str`]: Returns a URL pointing to the large image asset of this activity, if applicable.
    """
    pass


# but this one is modified
def large_image_url(self) -> Optional[str]:
    """Optional[:class:`str`]: Returns a URL pointing to the large image asset of this activity, if applicable.
    """
    pass

Expected behavior

Single line docstrings are left alone. I understand what this change is supposed to do, and on already multiline docstrings this wouldn't be an issue (although it would be nice for the docstring formatter to be entirely consistent among all docstrings instead of having different behavior for the line length but i digress).

Environment

  • Black's version: 22.10.0
  • OS and Python version: Python 3.11.0rc2, on linux, but i can recreate with python 3.8 too

Additional Comments

This seems like a regression from #3044

onerandomusername avatar Oct 10 '22 05:10 onerandomusername

Hey! I'd like to contribute to fixing this bug as part of Hacktoberfest. Hope that's ok!

itxasos23 avatar Oct 12 '22 14:10 itxasos23

Hi! Looking at the changes again, it seems intended.

@itxasos23 Thanks for offering! However, #2885 deals with docstrings and could clash with this change. So I think we'll at least have to discuss a bit. If you'd be willing to try something else, have a look at the ones in https://github.com/psf/black/labels/good%20first%20issue!

felix-hilden avatar Oct 15 '22 19:10 felix-hilden

Hi! Looking at the changes again, it seems intended.

The biggest problem is this change for single line docstrings is a violation of PEP 257.

  • The closing quotes are on the same line as the opening quotes. This looks better for one-liners.

onerandomusername avatar Oct 17 '22 00:10 onerandomusername

@felix-hilden Amazing! I see you got that covered on that PR :smile:

itxasos23 avatar Oct 17 '22 19:10 itxasos23

However, #2885 deals with docstrings and could clash with this change.

This pr is in agreement with 2885, as the author of 2885 states:

At the moment, any docstring with only one line of content will be converted to the one-line form.

This is in line with pep 257, and should satisfy this issue as well.

onerandomusername avatar Oct 17 '22 19:10 onerandomusername

I propose a fix in #3430. cc @idorrington92 as the author of #3044 which introduced this behavior.

JelleZijlstra avatar Dec 11 '22 23:12 JelleZijlstra