Repository: google/yapf

Issue:
yapf returns an exit code of 2 on unformatted but valid input.
Hello there and thanks for developing this much appreciated python formatter.

I’m opening this issue as I’m not sure to understand the logic behind yapf's exit code return when applying it on unformatted and valid input. Indeed, yapf returns an exit code of 2 in such case (which implies something went wrong), yet a valid formatted output is returned. I would consider the operation successful, hence expect an exit code of 0.

Consequently, this behavior tends to break scripts where flow execution rely on the format's success.

I saw a related issue: https://github.com/google/yapf/issues/186 but I don't see the need to inform output has been modified compared to input (i.e. formatted) by returning a status code other than 0 as formatting is the purpose of the tool. Otherwise, _--diff_ option seems the right fit if this information is needed (not to mention UNIX tools can also answer this).

Am I missing something or is this behavior indeed odd? Thanks!

Discussion:

Patch:
diff --git a/CHANGELOG b/CHANGELOG
index d331f8d..fd92065 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -5,8 +5,11 @@
 ## [0.11.1] UNRELEASED
 ### Fixed
 - Enforce splitting each element in a dictionary if comma terminated.
-- It's okay to split in the middle of a dotted name if the whole expression is
-  going to go over the column limit.
+
+### Changed
+- Issue #228: Return exit code 0 on success, regardless of whether files were
+  changed.  (Previously, 0 meant success with no files
+  modified, and 2 meant success with at least one file modified.)
 
 ## [0.11.0] 2016-07-17
 ### Added
diff --git a/yapf/__init__.py b/yapf/__init__.py
index dfbb8ee..145d8f4 100644
--- a/yapf/__init__.py
+++ b/yapf/__init__.py
@@ -161,7 +161,7 @@ def main(argv):
         lines=lines,
         verify=args.verify)
     sys.stdout.write(reformatted_source)
-    return 2 if changed else 0
+    return 0
 
   files = file_resources.GetCommandLineFiles(args.files, args.recursive,
                                              args.exclude)
@@ -175,7 +175,7 @@ def main(argv):
       in_place=args.in_place,
       print_diff=args.diff,
       verify=args.verify)
-  return 2 if changed else 0
+  return 0
 
 
 def FormatFiles(filenames,
diff --git a/yapf/yapflib/split_penalty.py b/yapf/yapflib/split_penalty.py
index 50796de..4b802c0 100644
--- a/yapf/yapflib/split_penalty.py
+++ b/yapf/yapflib/split_penalty.py
@@ -185,8 +185,8 @@ class _TreePenaltyAssigner(pytree_visitor.PyTreeVisitor):
             #   atom tr1() tr2
             # It may be necessary (though undesirable) to split up a previous
             # function call's parentheses to the next line.
-            self._SetStronglyConnected(prev_trailer.children[-1])
-          self._SetStronglyConnected(cur_trailer.children[0])
+            self._SetUnbreakable(prev_trailer.children[-1])
+          self._SetUnbreakable(cur_trailer.children[0])
           prev_trailer_idx = cur_trailer_idx
         else:
           break


Test patch:
diff --git a/yapftests/main_test.py b/yapftests/main_test.py
index e5a5a74..42e6f9b 100644
--- a/yapftests/main_test.py
+++ b/yapftests/main_test.py
@@ -91,7 +91,7 @@ class MainTest(unittest.TestCase):
     with patched_input(code):
       with captured_output() as (out, err):
         ret = yapf.main(['-', '--style=chromium'])
-        self.assertEqual(ret, 2)
+        self.assertEqual(ret, 0)
         self.assertEqual(out.getvalue(), chromium_code)
 
   def testEchoBadInput(self):
@@ -116,3 +116,4 @@ class MainTest(unittest.TestCase):
       self.assertEqual(ret, 0)
       version = 'yapf {}\n'.format(yapf.__version__)
       self.assertEqual(version, out.getvalue())
+
diff --git a/yapftests/reformatter_test.py b/yapftests/reformatter_test.py
index cb2e6d5..3092344 100644
--- a/yapftests/reformatter_test.py
+++ b/yapftests/reformatter_test.py
@@ -1750,20 +1750,6 @@ class BuganizerFixes(ReformatterTest):
   def setUpClass(cls):
     style.SetGlobalStyle(style.CreateChromiumStyle())
 
-  def testB30442148(self):
-    unformatted_code = textwrap.dedent("""\
-        def lulz():
-          return (some_long_module_name.SomeLongClassName.
-                  some_long_attribute_name.some_long_method_name())
-        """)
-    expected_formatted_code = textwrap.dedent("""\
-        def lulz():
-          return (some_long_module_name.SomeLongClassName.some_long_attribute_name.
-                  some_long_method_name())
-        """)
-    uwlines = _ParseAndUnwrap(unformatted_code)
-    self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
-
   def testB26868213(self):
     unformatted_code = textwrap.dedent("""\
       def _():
@@ -1843,10 +1829,9 @@ class BuganizerFixes(ReformatterTest):
         """)
     expected_formatted_code = textwrap.dedent("""\
         def _():
-          _xxxxxxxxxxxxxxx(
-              aaaaaaaa,
-              bbbbbbbbbbbbbb.cccccccccc[dddddddddddddddddddddddddddd.
-                                        eeeeeeeeeeeeeeeeeeeeee.fffffffffffffffffffff])
+          _xxxxxxxxxxxxxxx(aaaaaaaa, bbbbbbbbbbbbbb.cccccccccc[
+              dddddddddddddddddddddddddddd.eeeeeeeeeeeeeeeeeeeeee.
+              fffffffffffffffffffff])
         """)
     uwlines = _ParseAndUnwrap(unformatted_code)
     self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
diff --git a/yapftests/split_penalty_test.py b/yapftests/split_penalty_test.py
index 51f8f3e..b21d227 100644
--- a/yapftests/split_penalty_test.py
+++ b/yapftests/split_penalty_test.py
@@ -221,10 +221,10 @@ class SplitPenaltyTest(unittest.TestCase):
     self._CheckPenalties(tree, [
         ('foo', None),
         ('.', UNBREAKABLE),
-        ('bar', DOTTED_NAME),
-        ('.', STRONGLY_CONNECTED),
-        ('baz', DOTTED_NAME),
-        ('(', STRONGLY_CONNECTED),
+        ('bar', UNBREAKABLE),
+        ('.', UNBREAKABLE),
+        ('baz', UNBREAKABLE),
+        ('(', UNBREAKABLE),
         ('1', CONTIGUOUS_LIST),
         (',', CONTIGUOUS_LIST),
         ('2', CONTIGUOUS_LIST),