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

Darker keeps indenting docstrings #240

Closed
Carreau opened this issue Nov 15, 2021 · 7 comments · Fixed by #300
Closed

Darker keeps indenting docstrings #240

Carreau opened this issue Nov 15, 2021 · 7 comments · Fixed by #300
Assignees
Labels
bug Something isn't working
Milestone

Comments

@Carreau
Copy link
Collaborator

Carreau commented Nov 15, 2021

Sorry this need to be a full repo for reproduction,

But basically darker -r <revision> file does not converge and keep indenting the docstring on each call

see https://github.com/Carreau/bugd

I'll dive into it when I have time (and yes I still need to look into how to integrate pyupgrade and find more time to contribute to darker)

@akaihola
Copy link
Owner

I tried this out, and indeed, Darker insists on increasing the indent on every subsequent call:

$ darker --diff -r 82dcfa0 foo.py
--- foo.py	2021-11-15 20:16:43.478134 +0000
+++ foo.py	2021-11-15 20:16:48.155692 +0000
@@ -1,18 +1,18 @@
 def doctest_tb_plain():
     """
 In [18]: xmode plain
 Exception reporting mode: Plain
 
-    In [19]: run simpleerr.py
-    Traceback (most recent call last):
-        ...line ..., in <module>
-        bar(mode)
-        ...line ..., in bar
-        div0()
-        ...line ..., in div0
-        x/y
-    ZeroDivisionError: ...
+        In [19]: run simpleerr.py
+        Traceback (most recent call last):
+            ...line ..., in <module>
+            bar(mode)
+            ...line ..., in bar
+            div0()
+            ...line ..., in div0
+            x/y
+        ZeroDivisionError: ...
     """
 
 
 empty = 1
$ darker -r 82dcfa0 foo.py
$ darker --diff -r 82dcfa0 foo.py
--- foo.py	2021-11-15 20:17:23.705493 +0000
+++ foo.py	2021-11-15 20:17:53.426252 +0000
@@ -1,18 +1,18 @@
 def doctest_tb_plain():
     """
 In [18]: xmode plain
 Exception reporting mode: Plain
 
-        In [19]: run simpleerr.py
-        Traceback (most recent call last):
-            ...line ..., in <module>
-            bar(mode)
-            ...line ..., in bar
-            div0()
-            ...line ..., in div0
-            x/y
-        ZeroDivisionError: ...
+            In [19]: run simpleerr.py
+            Traceback (most recent call last):
+                ...line ..., in <module>
+                bar(mode)
+                ...line ..., in bar
+                div0()
+                ...line ..., in div0
+                x/y
+            ZeroDivisionError: ...
     """
 
 
 empty = 1
$ darker -r 82dcfa0 foo.py
$ darker --diff -r 82dcfa0 foo.py
--- foo.py	2021-11-15 20:18:05.321865 +0000
+++ foo.py	2021-11-15 20:18:06.706663 +0000
@@ -1,18 +1,18 @@
 def doctest_tb_plain():
     """
 In [18]: xmode plain
 Exception reporting mode: Plain
 
-            In [19]: run simpleerr.py
-            Traceback (most recent call last):
-                ...line ..., in <module>
-                bar(mode)
-                ...line ..., in bar
-                div0()
-                ...line ..., in div0
-                x/y
-            ZeroDivisionError: ...
+                In [19]: run simpleerr.py
+                Traceback (most recent call last):
+                    ...line ..., in <module>
+                    bar(mode)
+                    ...line ..., in bar
+                    div0()
+                    ...line ..., in div0
+                    x/y
+                ZeroDivisionError: ...
     """
 
 
 empty = 1

@akaihola
Copy link
Owner

Curiously, letting Black reformat the same file repeatedly doesn't cause this behavior:

$ black --diff foo.py
--- foo.py	2021-11-15 20:20:29.998158 +0000
+++ foo.py	2021-11-15 20:20:50.559362 +0000
@@ -1,18 +1,18 @@
 def doctest_tb_plain():
     """
-In [18]: xmode plain
-Exception reporting mode: Plain
+    In [18]: xmode plain
+    Exception reporting mode: Plain
 
-    In [19]: run simpleerr.py
-    Traceback (most recent call last):
-        ...line ..., in <module>
-        bar(mode)
-        ...line ..., in bar
-        div0()
-        ...line ..., in div0
-        x/y
-    ZeroDivisionError: ...
+        In [19]: run simpleerr.py
+        Traceback (most recent call last):
+            ...line ..., in <module>
+            bar(mode)
+            ...line ..., in bar
+            div0()
+            ...line ..., in div0
+            x/y
+        ZeroDivisionError: ...
     """
 
 
 empty = 1
would reformat foo.py
All done! ✨ 🍰 ✨
1 file would be reformatted.
$ black foo.py
reformatted foo.py
All done! ✨ 🍰 ✨
1 file reformatted.

$ black --diff foo.py
All done! ✨ 🍰 ✨
1 file would be left unchanged.

@akaihola
Copy link
Owner

If I edit the file reformatted by Black by moving the two first non-empty docstring lines back to column 1:

def doctest_tb_plain():
    """
In [18]: xmode plain
Exception reporting mode: Plain

        In [19]: run simpleerr.py
        Traceback (most recent call last):
            ...line ..., in <module>
            bar(mode)
            ...line ..., in bar
            div0()
            ...line ..., in div0
            x/y
        ZeroDivisionError: ...
    """


empty = 1

and run Black again:

$ black --diff foo.py

sure enough, it will continue indenting:

--- foo.py	2021-11-15 20:23:29.994420 +0000
+++ foo.py	2021-11-15 20:24:43.420740 +0000
@@ -1,18 +1,18 @@
 def doctest_tb_plain():
     """
-In [18]: xmode plain
-Exception reporting mode: Plain
+    In [18]: xmode plain
+    Exception reporting mode: Plain
 
-        In [19]: run simpleerr.py
-        Traceback (most recent call last):
-            ...line ..., in <module>
-            bar(mode)
-            ...line ..., in bar
-            div0()
-            ...line ..., in div0
-            x/y
-        ZeroDivisionError: ...
+            In [19]: run simpleerr.py
+            Traceback (most recent call last):
+                ...line ..., in <module>
+                bar(mode)
+                ...line ..., in bar
+                div0()
+                ...line ..., in div0
+                x/y
+            ZeroDivisionError: ...
     """
 
 
 empty = 1
would reformat foo.py
All done! ✨ 🍰 ✨
1 file would be reformatted.

@akaihola
Copy link
Owner

So clearly the problem lies within the combination of

  • Black's rule to indent a whole docstring so it is indented at least to the opening triple quotes, and
  • Darker splitting the changes by empty lines and leaving unmodified sections of the docstring unindented.

I wonder if the same problem exists outside docstrings with actual source code when non-standard indentation is being corrected by Black.

If so, we're in trouble.

Docstrings we could fix by finding multi-line docstrings using AST parsing and detecting whether there are indentation changes in them. But for code, hmm...

@akaihola
Copy link
Owner

sigh of relief... – AST verification is protecting Darker from garbling code with non-standard indentation. Instead of accepting re-indented modified code and ignoring re-indentation of unmodified code, it will actually extend surrounding extra context lines until the re-indentation succeeds to keep the AST identical.

In other words, non-standard indentation in files will cause Darker to re-indent also unmodified code around modified lines so mixed indentation doesn't change the meaning of the code or render it invalid.

So we only need to find a fix for indentation of multi-line docstrings here. I assume not all multi-line strings are re-indented by Black.

@akaihola
Copy link
Owner

Creating a failing test case should be easy by using the git_repo fixture and adding a file like

def docstring_func():
    """
originally unindented

    originally indented
    """

and committing it, then modifying it like

def docstring_func():
    """
originally unindented

    modified and still indented
    """

and running Darker's main program on it.

@akaihola akaihola self-assigned this Feb 10, 2022
@akaihola akaihola added the bug Something isn't working label Feb 10, 2022
@akaihola akaihola modified the milestones: 1.4.1, 1.4.2 Feb 10, 2022
@akaihola akaihola changed the title Darker keep indenting docstring. Darker keeps indenting docstrings Feb 21, 2022
@akaihola
Copy link
Owner

Moving to version 1.5.0 (1.4.2 was just released).

@akaihola akaihola modified the milestones: 1.4.2, 1.5.0 Mar 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants