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

Be more careful with docstrings ending with quotes + whitespace #1455

Closed
wants to merge 2 commits into from

Conversation

alexmv
Copy link
Contributor

@alexmv alexmv commented May 22, 2020

The PEP 257 algorithm used in #1053 results in trimming trailing
whitespace in docstrings -- see #1415.

Adjust the algorithm to preserve leading and trailing whitespace; this
reverts the changes in #1417. This diverges from PEP 257, but better
retains the contents of the docstring.

Fixes #1452 because we no longer can end up with four trailing quotes.

@alexmv alexmv force-pushed the quotes-in-docstrings branch 3 times, most recently from 45a7168 to 6351989 Compare May 22, 2020 22:14
@JelleZijlstra
Copy link
Collaborator

Thanks for your contribution!

I would personally prefer we leave the space in instead of backslash-escaping the quote. That's more in line with Black's general principle of avoiding backslashes and it matches the original formatting of the real-world examples in #1452. Would that be easy to implement instead?

@alexmv
Copy link
Contributor Author

alexmv commented May 22, 2020

I can take a gander; your #1417 seemed to move in the direction of formalizing the whitespace change, however, so I figured this was merely a bugfix on that behaviour.

@alexmv
Copy link
Contributor Author

alexmv commented May 23, 2020

Here's an alternate commit which moves toward preserving the contents more unchanged. Happy to push this to the branch if this is preferred.

From eb0ab3674c98c742ed7eb4c8fd4d59c550255354 Mon Sep 17 00:00:00 2001
From: Alex Vandiver <[email protected]>
Date: Fri, 22 May 2020 13:38:38 -0700
Subject: [PATCH 1/2] Stop trimming whitespace in docstrings

The PEP 257 algorithm used in #1053 results in trimming trailing
whitespace in docstrings -- see #1415.

Adjust the algorithm to preserve leading and trailing whitespace; this
reverts the changes in #1417.  This diverges from PEP 257, but better
retains the contents of the docstring.

Fixes #1452 because we no longer can end up with four trailing quotes.
---
 src/black/__init__.py   | 22 +++++++++----
 tests/data/docstring.py | 72 +++++++++++++++++++++++++++++++++++++++--
 2 files changed, 84 insertions(+), 10 deletions(-)

diff --git src/black/__init__.py src/black/__init__.py
index a7e89cc..49b277b 100644
--- src/black/__init__.py
+++ src/black/__init__.py
@@ -6008,14 +6008,13 @@ def _stringify_ast(

         else:
             # Constant strings may be indented across newlines, if they are
-            # docstrings; fold spaces after newlines when comparing. Similarly,
-            # trailing and leading space may be removed.
+            # docstrings; fold spaces after newlines when comparing
             if (
                 isinstance(node, ast.Constant)
                 and field == "value"
                 and isinstance(value, str)
             ):
-                normalized = re.sub(r" *\n[ \t]+", "\n ", value).strip()
+                normalized = re.sub(r"\n([ \t]+|(?=\n))", "\n ", value)
             else:
                 normalized = value
             yield f"{'  ' * (depth+2)}{normalized!r},  # {value.__class__.__name__}"
@@ -6423,13 +6422,22 @@ def fix_docstring(docstring: str, prefix: str) -> str:
         if stripped:
             indent = min(indent, len(line) - len(stripped))
     # Remove indentation (first line is special):
-    trimmed = [lines[0].strip()]
-    if indent < sys.maxsize:
+    trimmed = [lines[0]]
+    if indent == sys.maxsize:
+        # We got no indent information from the later lines; they were
+        # all just whitespace.  Preserve the number of newlines.
+        for _ in lines[1:-1]:
+            trimmed.append("")
+        if len(lines) > 1:
+            trimmed.append(prefix)
+    else:
         last_line_idx = len(lines) - 2
         for i, line in enumerate(lines[1:]):
             stripped_line = line[indent:].rstrip()
-            if stripped_line or i == last_line_idx:
-                trimmed.append(prefix + stripped_line)
+            if stripped_line:
+                trimmed.append(prefix + line[indent:])
+            elif i == last_line_idx:
+                trimmed.append(prefix)
             else:
                 trimmed.append("")
     # Return a single string:
diff --git tests/data/docstring.py tests/data/docstring.py
index f5adeb7..b4cb4f0 100644
--- tests/data/docstring.py
+++ tests/data/docstring.py
@@ -73,10 +73,42 @@ def single_line():
     """
     pass

+
+def containing_quotes():
+    """No quotes here
+
+    "quotes here" """
+    pass
+
+
+def containing_unbalanced_quotes():
+    """No quotes here
+
+    quote here" """
+    pass
+
+
+def entirely_space():
+    """
+
+    """
+    pass
+
+
+def just_quote():
+    """ " """
+    pass
+
+
+def escaped_already():
+    """
+    foo\""""
+    pass
+
 # output

 class MyClass:
-    """Multiline
+    """ Multiline
     class docstring
     """

@@ -88,7 +120,7 @@ class MyClass:


 def foo():
-    """This is a docstring with
+    """This is a docstring with
     some lines of text here
     """
     return
@@ -146,5 +178,39 @@ def over_indent():


 def single_line():
-    """But with a newline after it!"""
+    """But with a newline after it!
+
+    """
+    pass
+
+
+def containing_quotes():
+    """No quotes here
+
+    "quotes here" """
+    pass
+
+
+def containing_unbalanced_quotes():
+    """No quotes here
+
+    quote here" """
+    pass
+
+
+def entirely_space():
+    """
+
+    """
+    pass
+
+
+def just_quote():
+    """ " """
+    pass
+
+
+def escaped_already():
+    """
+    foo\""""
     pass
--
2.26.2

src/black/__init__.py Outdated Show resolved Hide resolved
The PEP 257 algorithm used in psf#1053 results in trimming trailing
whitespace in docstrings -- see psf#1415.

Adjust the algorithm to preserve leading and trailing whitespace; this
reverts the changes in psf#1417.  This diverges from PEP 257, but better
retains the contents of the docstring.

Fixes psf#1452 because we no longer can end up with four trailing quotes.
`visit_STRING` finishes by calling `visit_default`, which already
deals with calling `normalize_string_quotes` -- and correctly checks
for `self.normalize_strings` first, which this callsite does not.
@alexmv
Copy link
Contributor Author

alexmv commented Jul 31, 2020

I can't replicate the Primer failures. On MacOS with Python 3.7.8, black-primer runs to completion just fine:

ilwrath ~/src/black/src/black_primer (quotes-in-docstrings>) $ head -n1 `which black`
#!/usr/local/opt/[email protected]/bin/python3.7

ilwrath ~/src/black/src/black_primer (quotes-in-docstrings>) $ /usr/local/opt/[email protected]/bin/python3.7 --version
Python 3.7.8

ilwrath ~/src/black/src/black_primer (quotes-in-docstrings>) $ black-primer -c ./primer.json
[2020-07-30 22:29:49,034] INFO: 16 projects to run Black over (lib.py:311)
[2020-07-30 22:29:50,597] INFO: Finished aioexabgp (lib.py:284)
[2020-07-30 22:29:52,636] INFO: Finished attrs (lib.py:284)
[2020-07-30 22:29:52,805] INFO: Finished bandersnatch (lib.py:284)
[2020-07-30 22:29:52,805] INFO: Skipping django as it's disabled via config (lib.py:254)
[2020-07-30 22:29:54,492] INFO: Finished channels (lib.py:284)
[2020-07-30 22:29:54,543] INFO: Finished flake8-bugbear (lib.py:284)
[2020-07-30 22:29:54,543] INFO: Skipping pandas as it's disabled via config (lib.py:254)
[2020-07-30 22:30:06,216] INFO: Finished hypothesis (lib.py:284)
[2020-07-30 22:30:06,594] INFO: Finished poetry (lib.py:284)
[2020-07-30 22:30:07,738] INFO: Finished ptr (lib.py:284)
[2020-07-30 22:30:07,738] INFO: Skipping pytest as it's disabled via config (lib.py:254)
[2020-07-30 22:30:17,779] INFO: Finished pyramid (lib.py:284)
[2020-07-30 22:30:27,021] INFO: Finished tox (lib.py:284)
[2020-07-30 22:30:33,677] INFO: Finished virtualenv (lib.py:284)
[2020-07-30 22:31:04,426] INFO: Finished warehouse (lib.py:284)
[2020-07-30 22:31:18,728] INFO: Finished sqlalchemy (lib.py:284)
[2020-07-30 22:31:18,729] INFO: Analyzing results (lib.py:327)
-- primer results 📊 --

13 / 16 succeeded (81.25%) ✅
0 / 16 FAILED (0.0%) 💩
 - 3 projects disabled by config
 - 0 projects skipped due to Python version
 - 0 skipped due to long checkout

@ichard26
Copy link
Collaborator

ichard26 commented Aug 8, 2020

Primer is still a bit flaky, it's fine in this case as you've have done a local run that has passed.

@ichard26
Copy link
Collaborator

Sorry @alexmv but this was superseded by #1623.

@ichard26 ichard26 closed this Aug 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Internal error when docstring end follows double quote
4 participants