Repository: google/yapf
Pull request #690 (2019-03-14 14:23:36)

Fix removing of extra lines on the edge of enabled lines boundaries.
This was broken by https://github.com/google/yapf/commit/dd9f72b9ec74267b84d5b0c0f70a12595f920166#diff-1020ef5a942231c7da662fa974a70a3bR145 which added a hacky fix that prevented removing newlines.

I haven't fixed the test that was added in that commit because I haven't yet work out how to tell if a range is affected by a disable comment. The correct solution will be to handle that comment instead of disallowing removal of lines.

Help welcome.

kevincox: cc @gwelymernans You introduced this issue.
gwelymernans: Could you also update the `CHANGELOG`.
kevincox: > Could you also update the CHANGELOG.

Done.
gwelymernans: Great! Once you're ready, go ahead and send the PR.
kevincox: I'm having trouble figuring out when lines are disabled via a comment in the code. It doesn't seem like they are added or removed from lines so there must be some other mechanism in play.
gwelymernans: After the program is parsed and the unwrapped lines are created, it goes through to determine if there are any disable comments. If so it marks the affected lines as disabled.
kevincox: Ok, I have fixed the disable check. It is a little hacky but better then before.
What is does is if the current line and the previous like are disabled it removes the lines from the set of editable lines.

A better approach would be to remove disabled lines from the set of editable lines for the whole reformat but that would require more refactoring and would best be done in a different fix.

Patch:
diff --git a/CHANGELOG b/CHANGELOG
index 1df010a..b352191 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -14,6 +14,7 @@
 - Avoid splitting after opening parens if SPLIT_BEFORE_FIRST_ARGUMENT is set
   to False.
 - Adjust default SPLIT_PENALTY_AFTER_OPENING_BRACKET.
+- Re-enable removal of extra lines on the boundaries of formatted regions.
 
 ## [0.26.0] 2019-02-08
 ### Added
diff --git a/yapf/yapflib/reformatter.py b/yapf/yapflib/reformatter.py
index 48858a3..3492ffc 100644
--- a/yapf/yapflib/reformatter.py
+++ b/yapf/yapflib/reformatter.py
@@ -114,12 +114,21 @@ def _RetainHorizontalSpacing(uwline):
 
 
 def _RetainRequiredVerticalSpacing(cur_uwline, prev_uwline, lines):
+  if cur_uwline.disable and (not prev_uwline or prev_uwline.disable):
+    # If both lines are disabled we aren't allowed to reformat anything.
+    lines = set()
+
   prev_tok = None
   if prev_uwline is not None:
     prev_tok = prev_uwline.last
   for cur_tok in cur_uwline.tokens:
     _RetainRequiredVerticalSpacingBetweenTokens(cur_tok, prev_tok, lines)
+
     prev_tok = cur_tok
+    if cur_uwline.disable:
+      # After the first token we are acting on a single line. So if it is
+      # disabled we must not reformat.
+      lines = set()
 
 
 def _RetainRequiredVerticalSpacingBetweenTokens(cur_tok, prev_tok, lines):
@@ -151,8 +160,6 @@ def _RetainRequiredVerticalSpacingBetweenTokens(cur_tok, prev_tok, lines):
     pass
   elif lines and (cur_lineno in lines or prev_lineno in lines):
     desired_newlines = cur_tok.whitespace_prefix.count('\n')
-    if desired_newlines < required_newlines:
-      desired_newlines = required_newlines
     whitespace_lines = range(prev_lineno + 1, cur_lineno)
     deletable_lines = len(lines.intersection(whitespace_lines))
     required_newlines = max(required_newlines - deletable_lines,


Test patch:
diff --git a/yapftests/blank_line_calculator_test.py b/yapftests/blank_line_calculator_test.py
index 2a27a2f..afc65bd 100644
--- a/yapftests/blank_line_calculator_test.py
+++ b/yapftests/blank_line_calculator_test.py
@@ -350,6 +350,72 @@ class BasicBlankLineCalculatorTest(yapf_test_helper.YAPFTest):
     self.assertCodeEqual(expected_formatted_code, code)
     self.assertFalse(changed)
 
+  def testLinesRangeRemove(self):
+    unformatted_code = textwrap.dedent(u"""\
+        def A():
+          pass
+
+
+
+        def B():  # 6
+          pass  # 7
+
+
+
+
+        def C():
+          pass
+        """)
+    expected_formatted_code = textwrap.dedent(u"""\
+        def A():
+          pass
+
+
+        def B():  # 6
+          pass  # 7
+
+
+        def C():
+          pass
+        """)
+    code, changed = yapf_api.FormatCode(unformatted_code, lines=[(5, 9)])
+    self.assertCodeEqual(expected_formatted_code, code)
+
+  def testLinesRangeRemoveSome(self):
+    unformatted_code = textwrap.dedent(u"""\
+        def A():
+          pass
+
+
+
+
+        def B():  # 7
+          pass  # 8
+
+
+
+
+        def C():
+          pass
+        """)
+    expected_formatted_code = textwrap.dedent(u"""\
+        def A():
+          pass
+
+
+
+        def B():  # 7
+          pass  # 8
+
+
+
+        def C():
+          pass
+        """)
+    code, changed = yapf_api.FormatCode(unformatted_code, lines=[(6, 9)])
+    self.assertCodeEqual(expected_formatted_code, code)
+    self.assertTrue(changed)
+
 
 if __name__ == '__main__':
   unittest.main()