Darker ignores blank lines re-inserted by Black where user had only deleted lines

[jedie]

• Add a really minimal test case in the test suite
• Create a fix

---

It seems to me that changes in newlines will not fix always.

e.g.: A wrong formatted code part:

#!/usr/bin/env python3
"""
    simple demo
    ~~~~~~~~~~~
"""
from creole import creole2html, html2creole, html2rest, html2textile
source_creole = """\
== simple demo

You can convert from:

* **creole2html**, **html2creole**, **html2rest**, //html2textile//

=== a table:

|=headline 1 |= headline 2 |
| 1.1. cell  | 1.2. cell   |
| 2.1. cell  | 2.2. cell   |

----

More info on our [[http://code.google.com/p/python-creole/|Homepage]]."""
if __name__ == "__main__":
    print("_" * 79 + "\n*** Convert creole into html: ***\n\n")
    html = creole2html(source_creole)
    print(html)





    print("\n\n" + "_" * 79 + "\n*** Convert html back into creole: ***\n\n")
    creole = html2creole(html)
    print(creole)

git diff looks like:

diff --git a/demo.py b/demo.py
index 93cd349..d7d33b5 100644
--- a/demo.py
+++ b/demo.py
@@ -1,15 +1,9 @@
 #!/usr/bin/env python3
-
-
 """
     simple demo
     ~~~~~~~~~~~
 """
-
-
 from creole import creole2html, html2creole, html2rest, html2textile
-
-
 source_creole = """\
 == simple demo
 
@@ -26,13 +20,15 @@ You can convert from:
 ----
 
 More info on our [[http://code.google.com/p/python-creole/|Homepage]]."""
-
-
 if __name__ == "__main__":
     print("_" * 79 + "\n*** Convert creole into html: ***\n\n")
     html = creole2html(source_creole)
     print(html)
 
+
+
+
+
     print("\n\n" + "_" * 79 + "\n*** Convert html back into creole: ***\n\n")
     creole = html2creole(html)
     print(creole)

After darker run only one fix was applied:

#!/usr/bin/env python3
"""
    simple demo
    ~~~~~~~~~~~
"""
from creole import creole2html, html2creole, html2rest, html2textile
source_creole = """\
== simple demo

You can convert from:

* **creole2html**, **html2creole**, **html2rest**, //html2textile//

=== a table:

|=headline 1 |= headline 2 |
| 1.1. cell  | 1.2. cell   |
| 2.1. cell  | 2.2. cell   |

----

More info on our [[http://code.google.com/p/python-creole/|Homepage]]."""
if __name__ == "__main__":
    print("_" * 79 + "\n*** Convert creole into html: ***\n\n")
    html = creole2html(source_creole)
    print(html)

    print("\n\n" + "_" * 79 + "\n*** Convert html back into creole: ***\n\n")
    creole = html2creole(html)
    print(creole)

Running black, results in:

#!/usr/bin/env python3
"""
    simple demo
    ~~~~~~~~~~~
"""
from creole import creole2html, html2creole, html2rest, html2textile

source_creole = """\
== simple demo

You can convert from:

* **creole2html**, **html2creole**, **html2rest**, //html2textile//

=== a table:

|=headline 1 |= headline 2 |
| 1.1. cell  | 1.2. cell   |
| 2.1. cell  | 2.2. cell   |

----

More info on our [[http://code.google.com/p/python-creole/|Homepage]]."""
if __name__ == "__main__":
    print("_" * 79 + "\n*** Convert creole into html: ***\n\n")
    html = creole2html(source_creole)
    print(html)

    print("\n\n" + "_" * 79 + "\n*** Convert html back into creole: ***\n\n")
    creole = html2creole(html)
    print(creole)

Black is also not perfect... Think it should look like:

#!/usr/bin/env python3

"""
    simple demo
    ~~~~~~~~~~~
"""

from creole import creole2html, html2creole, html2rest, html2textile


source_creole = """\
== simple demo

You can convert from:

* **creole2html**, **html2creole**, **html2rest**, //html2textile//

=== a table:

|=headline 1 |= headline 2 |
| 1.1. cell  | 1.2. cell   |
| 2.1. cell  | 2.2. cell   |

----

More info on our [[http://code.google.com/p/python-creole/|Homepage]]."""


if __name__ == "__main__":
    print("_" * 79 + "\n*** Convert creole into html: ***\n\n")
    html = creole2html(source_creole)
    print(html)

    print("\n\n" + "_" * 79 + "\n*** Convert html back into creole: ***\n\n")
    creole = html2creole(html)
    print(creole)

black issues are full with "blank lines" stuff, see: https://github.com/psf/black/issues?q=is%3Aissue+is%3Aopen+blank+lines So maybe it's fixed sometimes...

The darker bug is that it seems not all blank line changes will be pass to black, isn't it?

[akaihola]

not all blank line changes will be pass to black

That's actually not how Darker works. It simply runs Black on the entire new version of the file, and only applies those Black reformattings which fall onto lines which were modified according to git diff.

But sure enough, I can reproduce this. Debug output from Darker is:

$ darker -vv --diff demo.py
[...]
DEBUG:darker.git:[/tmp/drkdbg]$ git rev-parse --is-inside-work-tree
DEBUG:darker.git:[/tmp/drkdbg]$ git diff --name-only --relative HEAD -- demo.py
DEBUG:darker.git:[/tmp/drkdbg]$ git ls-files --others --exclude-standard -- demo.py
DEBUG:darker.__main__:Read 34 lines from edited file /tmp/drkdbg/demo.py
DEBUG:darker.__main__:Black reformat resulted in 31 lines
DEBUG:darker.diff:Diff between edited and reformatted has 5 opcodes
DEBUG:darker.git:[/tmp/drkdbg]$ git show HEAD:./demo.py
DEBUG:darker.git:[/tmp/drkdbg]$ git log -1 --format=%ct HEAD -- demo.py
DEBUG:darker.diff:Diff between edited and reformatted has 7 opcodes
DEBUG:darker.chooser:Found no edits on lines 1-6
DEBUG:darker.chooser:Using 6 original lines at line 1
DEBUG:darker.chooser:Found no edits on line 7
DEBUG:darker.chooser:Using 0 original lines at line 7
DEBUG:darker.chooser:Found no edits on lines 7-27
DEBUG:darker.chooser:Using 21 original lines at line 7
DEBUG:darker.chooser:Found no edits on lines 28-31
DEBUG:darker.chooser:Using 4 original lines at line 28
DEBUG:darker.chooser:Found no edits on lines 32-34
DEBUG:darker.chooser:Using 3 original lines at line 32
DEBUG:darker.__main__:Verifying that the 34 original edited lines and 34 reformatted lines parse into an identical abstract syntax tree

So the problem is that since the lines were removed, in Darker's world there are really no "changed lines" in the new version of demo.py.

Indeed this is an edge case Darker doesn't handle at all. If Black inserts new lines in place of lines the user has removed, Darker will miss the reformatting.

The reformatting-applying algorithm needs to support this special case e.g. by

• detecting when there's a gap between two consecutive diff chunks with the "equal" label
• checking whether the reformatted version has lines inserted there
• applying them

I'm not sure it's that straightforward though before looking into the details.

[jedie]

Hm. What's about to expand the change code block and include all white spaces?

[akaihola]

Can you give a concrete example of that idea? I don't quite follow yet.

[jedie]

Sorry for bad english ;) I'll try again:

It simply runs Black on the entire new version of the file, and only applies those Black reformattings which fall onto lines which were modified according to git diff.

I have named this lines which were modified according to git diff as a code block, that may be reformattet.
These "diff code blocks", must be include all new lines before and after every "block" to fix this problem, here.
So https://github.com/akaihola/darker/blob/master/src/darker/chooser.py is the file that must be change, isn't it?

[akaihola]

But, you see, there is no "diff code block" that may be reformatted.

If I have this file (with line numbers):

import os  # first line in original file


"Fourth line in original file"

and I delete the empty lines:

import os  # first line in original file
"Fourth line in original file"

Now black --diff will indeed give me

@@ -1,2 +1,3 @@
 import os  # first line in original file
+
 "Fourth line in original file"

If I run darker --diff with a debugger, I see this function call:

choose_lines(
    black_chunks=[
        (
            1,
            ('import os  # first line in original file',),
            ('import os  # first line in original file',)
        ),
        (
            2, 
            (), 
            ('',)
        ),
        (
            2, 
            ('"Fourth line in original file"',), 
            ('"Fourth line in original file"',)
        )
    ],
    edit_linenums=[],
)

So edit_linenums is empty, because

import os  # first line in original file      # <-- line 1 was not edited
"Fourth line in original file"                # <-- line 2 was not edited

in the newer version of the file.

I hope you see that with Darker's current diff logic, it's not quite that simple to implement this change.

We would somehow need to keep track not only of the line numbers in the new file which have been modified, but also "half-line-numbers", or consecutive line number pairs, between which lines have been removed. In the example above, choose_lines() would accept a third argument e.g.

choose_lines(
    black_chunks=...,
    edit_linenums=[],
    deletions_between_linenums=[(1, 2)],
)

So this is of course possible, but requires a bit of a larger design effort.

Could you help by

• making this modification to choose_lines() and implementing the correct behavior,
• fixing the unit tests in test_chooser.py by passing the extra deletions_between_linenums=[],
• converting the example above into a test case in test_chooser.py, and
• possibly adding further test cases like your original one?

I could then make sure the rest of Darker finds out the correct value for deletions_between_linenums= and passes it to choose_lines().

[akaihola]

Oh, and what I tend to do when I get stuck with English is let deepl.com do a translation suggestion for me. It is unbelievably good.