From: Lawrence Chan <llchan@users.noreply.github.com>
Date: Mon, 28 Oct 2019 19:51:45 +0000 (-0500)
Subject: Fix fmt on/off when multiple exist in leaf prefix (#1086)
X-Git-Url: https://git.madduck.net/etc/vim.git/commitdiff_plain/23fec8b0f73a142d1796c8ca958230ad52d83c24?ds=sidebyside

Fix fmt on/off when multiple exist in leaf prefix (#1086)

The old behavior would detect the existence of a `# fmt: on` in a leaf
node's comment prefix and immediately mark the node as formatting-on,
even if a subsequent `# fmt: off` in the same comment prefix would turn
it back off. This change modifies that logic to track the state through
the entire prefix and take the final state.

Note that this does not fully solve on/off behavior, since any _comment_
lines between the off/on are still formatted. We may need to add
virtual leaf nodes to truly solve that. I will leave that for a separate
commit/PR.

Fixes #1005
---

diff --git a/black.py b/black.py
index 3231503..a48f647 100644
--- a/black.py
+++ b/black.py
@@ -3056,9 +3056,14 @@ def generate_ignored_nodes(leaf: Leaf) -> Iterator[LN]:
     """
     container: Optional[LN] = container_of(leaf)
     while container is not None and container.type != token.ENDMARKER:
+        is_fmt_on = False
         for comment in list_comments(container.prefix, is_endmarker=False):
             if comment.value in FMT_ON:
-                return
+                is_fmt_on = True
+            elif comment.value in FMT_OFF:
+                is_fmt_on = False
+        if is_fmt_on:
+            return
 
         yield container
 
diff --git a/tests/data/fmtonoff.py b/tests/data/fmtonoff.py
index fff751e..5a50eb1 100644
--- a/tests/data/fmtonoff.py
+++ b/tests/data/fmtonoff.py
@@ -108,9 +108,13 @@ def on_and_off_broken():
     # fmt: on
     # fmt: off
     this=should.not_be.formatted()
-    but=it  is  formatted
+    and_=indeed . it  is  not  formatted
     because . the . handling . inside . generate_ignored_nodes()
-    doesnt . consider . ordering . within . one . prefix
+    now . considers . multiple . fmt . directives . within . one . prefix
+    # fmt: on
+    # fmt: off
+        # ...but comments still get reformatted even though they should not be
+    # fmt: on
 def long_lines():
     if True:
         typedargslist.extend(
@@ -318,10 +322,14 @@ def on_and_off_broken():
     """Another known limitation."""
     # fmt: on
     # fmt: off
-    this = should.not_be.formatted()
-    but = it is formatted
-    because.the.handling.inside.generate_ignored_nodes()
-    doesnt.consider.ordering.within.one.prefix
+    this=should.not_be.formatted()
+    and_=indeed . it  is  not  formatted
+    because . the . handling . inside . generate_ignored_nodes()
+    now . considers . multiple . fmt . directives . within . one . prefix
+    # fmt: on
+    # fmt: off
+    # ...but comments still get reformatted even though they should not be
+    # fmt: on
 
 
 def long_lines():
diff --git a/tests/data/fmtonoff3.py b/tests/data/fmtonoff3.py
new file mode 100644
index 0000000..4e3b024
--- /dev/null
+++ b/tests/data/fmtonoff3.py
@@ -0,0 +1,35 @@
+# fmt: off
+x = [
+    1, 2,
+    3, 4,
+]
+# fmt: on
+
+# fmt: off
+x = [
+    1, 2,
+    3, 4,
+]
+# fmt: on
+
+x = [
+    1, 2, 3, 4
+]
+
+# output
+
+# fmt: off
+x = [
+    1, 2,
+    3, 4,
+]
+# fmt: on
+
+# fmt: off
+x = [
+    1, 2,
+    3, 4,
+]
+# fmt: on
+
+x = [1, 2, 3, 4]
diff --git a/tests/test_black.py b/tests/test_black.py
index d4c88d5..93f853b 100644
--- a/tests/test_black.py
+++ b/tests/test_black.py
@@ -614,6 +614,14 @@ class BlackTestCase(unittest.TestCase):
         black.assert_equivalent(source, actual)
         black.assert_stable(source, actual, black.FileMode())
 
+    @patch("black.dump_to_file", dump_to_stderr)
+    def test_fmtonoff3(self) -> None:
+        source, expected = read_data("fmtonoff3")
+        actual = fs(source)
+        self.assertFormatEqual(expected, actual)
+        black.assert_equivalent(source, actual)
+        black.assert_stable(source, actual, black.FileMode())
+
     @patch("black.dump_to_file", dump_to_stderr)
     def test_remove_empty_parentheses_after_class(self) -> None:
         source, expected = read_data("class_blank_parentheses")