Document experimental string processing and docstring indentation

[felix-hilden]

From #53: this PR adds short descriptions of #1053 and #1132 to the Black code style document.

Possible points of review/contention:

• Has the processing changed since? Reading the changelog I don't think it has, but I haven't been around.
• Doc placement: I think the "Strings" section is appropriate for both, unless another section for indentation is created. I appended the text to the section by default, and because "experimental" hints that maybe it shouldn't be the first thing to be introduced to the reader. I'm not sure about the docstring paragraph though. It is quite short, so maybe it ought to be merged with some other paragraph.

I don't think any of the checks apply, and skip news could also be added. Pre-commit did line up the text though.

Happy to make any changes you suggest!

---

Also, while running the install stuff, there was some sort of process pool termination when formatting the repository. Is that something that is expected to happen on Windows?

[felix-hilden]

Self-review!

So I had a look over the issue tracker, and #1740 implies the existence of further docstring processing (i.e. stripping white space), but I couldn't find the original source. I did find the original Docstring opinion issue, and several by Jelle fixing crashes on docstring modifications like removing trailing whitespace. But I got pretty lost pretty quickly.

Should we just add a mention about stripping white space? Alternatively, #1740 (or a follow-up PR) could add all the information about docstring contents.

I think the long-term solution to closing #53 could be to add a section to CONTRIBUTING.md where contributors are instructed to modify the Black code style document when making style changes. It would be nice to enforce it automatically, but I don't know if a CI check about requiring that is even possible. So manual review it is I guess.

[JelleZijlstra]

So I had a look over the issue tracker, and #1740 implies the existence of further docstring processing (i.e. stripping white space), but I couldn't find the original source. I did find the original Docstring opinion issue, and several by Jelle fixing crashes on docstring modifications like removing trailing whitespace. But I got pretty lost pretty quickly.

Did you try looking at the test cases for docstrings? That's probably the best way to find out all the things we change. I believe it's mostly about reindentation and stripping whitespace.

Also, thanks for contributing this! I'll take a more detailed look later.

[ichard26]

Yep, the docstring formatting Black does mostly boils down to reindenting and stripping whitespace, but there is an edge case where Black does move the quotes:

ichard26@acer-ubuntu:~/programming/oss/black$ cat test.py 
def test8():
    """Or like this.
    """
ichard26@acer-ubuntu:~/programming/oss/black$ black test.py --diff -q
--- test.py	2021-04-14 22:59:18.638257 +0000
+++ test.py	2021-04-14 22:59:33.679733 +0000
@@ -1,3 +1,2 @@
 def test8():
-    """Or like this.
-    """
+    """Or like this."""

This behaviour does have a report: #1635 but it's unclear whether this should exist (and therefore be documented) (@JelleZijlstra I'm personally of the opinion we should get consistent about this by either uniformingly moving quotes or not moving them at all)

Alternatively, #1740 (or a follow-up PR) could add all the information about docstring contents.

I am leaning towards letting the proposed PRs add the information on docstring reformatting just in case we reject one (seems unlikely but always a possibility), but I don't feel strongly about this.

I think the long-term solution to closing #53 could be to add a section to CONTRIBUTING.md where contributors are instructed to modify the Black code style document when making style changes. It would be nice to enforce it automatically, but I don't know if a CI check about requiring that is even possible. So manual review it is I guess.

Yeah I agree to add such a section in the contributing documentation (I'll probably end up doing it myself). In terms enforcing it automatically... yeah it would difficult to do that. I do want to write PR review documentation though, so I suppose we could mention checking for this there.

Also, while running the install stuff, there was some sort of process pool termination when formatting the repository. Is that something that is expected to happen on Windows?

One, that certainly shouldn't happen regardless of the platform and sounds like a bug. And two, is it I/O operation on closed file errors? This issue has been reported and fixed already but the fix hasn't been released yet (see GH-1664).

[ichard26]

Other than the docstring docs being incorrect I can't find anything to fault your documentation on.

Has the processing changed since?

Other than a lot of bugfixes, no IIRC the processing hasn't changed.

RE: Doc placement

I agree with your reasoning and I'm happy with where it is as of right now.

Excellent work and thank you for picking this up! I'll leave this for Jelle to review.

[felix-hilden]

Did you try looking at the test cases for docstrings? That's probably the best way ...

Oh you're totally right! What an oversight 😅

One, that certainly shouldn't happen regardless of the platform and sounds like a bug. And two, is it I/O operation on closed file errors?

It occured while Blackening with pre-commit for the first time (but does reproduce with it now too). It's an access denied on semlock.__enter__() leading to a bunch of process terminations. Seems like it's worth another bug report once I narrow it down a bit.

[felix-hilden]

Sorry about the force noise 😅 I got a bit carried away with small details.

I squashed the changes into the two commits, so you can choose if you'd like to separate the commits in master or not, up to you! And the docstring paragraph became a lot longer, so I think it looks nice where it is.

[felix-hilden]

Heya once more! With the closing of #1740, I added its style description to this PR, and added a missing test to validate that completely empty docstrings get the space inserted too.

Ready if you are!

[JelleZijlstra]

Thank you!