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 3231503e4fb228c1bbbe40973ac73d47015c114e..a48f64765d954e0ef2b13e9ecdb7c4d7f4778cdb 100644 (file)
--- 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:
     """
     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:
         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
 
 
         yield container
 

diff --git a/tests/data/fmtonoff.py b/tests/data/fmtonoff.py
index fff751e37248dcaa131b5c8883538ccc86ce1420..5a50eb12ed32b21b31c836505dc6e94ffea2fd56 100644 (file)
--- 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()
     # 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()
     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(
 def long_lines():
     if True:
         typedargslist.extend(


@@ -318,10 +322,14 @@ def on_and_off_broken():
     """Another known limitation."""
     # fmt: on
     # fmt: off
     """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():
 
 
 def long_lines():

diff --git a/tests/data/fmtonoff3.py b/tests/data/fmtonoff3.py
new file mode 100644 (file)
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 d4c88d514265620714cc6b0615a0f3688545507c..93f853bae4f560f80584c5260192c9cf7ce7d259 100644 (file)
--- 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())
 
         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")
     @patch("black.dump_to_file", dump_to_stderr)
     def test_remove_empty_parentheses_after_class(self) -> None:
         source, expected = read_data("class_blank_parentheses")