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

Removed adding a space into empty docstrings. #2249

Merged
merged 3 commits into from May 25, 2021
Merged

Removed adding a space into empty docstrings. #2249

merged 3 commits into from May 25, 2021

Conversation

MarkCBell
Copy link
Contributor

@MarkCBell MarkCBell commented May 22, 2021

Resolves #2168 by disabling the insertion of a " " when the docstring is entirely empty.

Note that this PR is focussed only on the case of empty docstrings. In particular this does not make any changes to the behaviour that a " " is inserted if a non-empty docstring begins with the quoting character. That is, black still prefers:

""" "something" """

to:

""""something" """

and that:

""""Something""""

is not a legal docstring.

@JelleZijlstra
Copy link
Collaborator

Thanks, I agree we should do this. Can we still make it output a single space if the original docspace is nonempty?

@ichard26 ichard26 added the F: strings Related to our handling of strings label May 22, 2021
Copy link
Collaborator

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Looks good to me. With the change to preserve docstrings that already contain a space, this shouldn't cause thrash for people who applied the formatting from the current release, while preserving completely empty docstrings for those who need them.

Copy link
Collaborator

@ichard26 ichard26 left a comment

Choose a reason for hiding this comment

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

I don't see anything wrong with this and this is a good pragmatic change. Thanks!

@johnthagen
Copy link
Contributor

Can we still make it output a single space if the original docspace is nonempty?

Does this mean if I reformatted the code with the previous black I need to hunt through and find the docstrings """ """ and fix them manually again?

@ichard26
Copy link
Collaborator

Does this mean if I reformatted the code with the previous black I need to hunt through and find the docstrings """ """ and fix them manually again?

Yes. It's not great for docstrings that were erroneously (like for sphinx docs) changed to """ """ but the main reason why is reduce churn by not effectively flip-flopping the original rule. Why the original rule? IIRC it was to indicate to users about the (probably) wrong empty or whitespace only docstring (it turns out an empty docstring isn't always wrong, unfortunately we didn't know this until after we got a report).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: strings Related to our handling of strings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to include empty docstring
4 participants