Don't squish all the args to the RHS

It looks bad when they're all aligned over to the right when there's all
this whitespace being wasted.

CHANGELOG

@@ -16,6 +16,8 @@
 - Ensure splitting of arguments if there's a named assign present.
 - Prefer to coalesce opening brackets if it's not at the beginning of a
   function call.
+- Prefer not to squish all of the elements in a function call over to the
+  right-hand side. Split the arguments instead.
 
 ## [0.16.0] 2017-02-05
 ### Added

yapf/yapflib/format_decision_state.py

@@ -80,6 +80,7 @@ def __init__(self, line, first_indent):
     self.column_limit = style.Get('COLUMN_LIMIT')
 
   def Clone(self):
+    """Clones a FormatDecisionState object."""
     new = FormatDecisionState(self.line, self.first_indent)
     new.next_token = self.next_token
     new.column = self.column
@@ -102,9 +103,9 @@ def __eq__(self, other):
             self.column == other.column and
             self.paren_level == other.paren_level and
             self.start_of_line_level == other.start_of_line_level and
-            self.lowest_level_on_line == other.lowest_level_on_line and (
-                self.ignore_stack_for_comparison or
-                other.ignore_stack_for_comparison or self.stack == other.stack))
+            self.lowest_level_on_line == other.lowest_level_on_line and
+            (self.ignore_stack_for_comparison or
+             other.ignore_stack_for_comparison or self.stack == other.stack))
 
   def __ne__(self, other):
     return not self == other
@@ -323,6 +324,10 @@ def MustSplit(self):
           # line and it's not a function call.
           if self._FitsOnLine(previous, previous.matching_bracket):
             return False
+        elif not self._FitsOnLine(previous, previous.matching_bracket):
+          if (self.column_limit - self.column) / float(self.column_limit) < 0.3:
+            # Try not to squish all of the arguments off to the right.
+            return current.next_token != previous.matching_bracket
       else:
         # Split after the opening of a container if it doesn't fit on the
         # current line or if it has a comment.

yapf/yapflib/reformatter.py

@@ -22,7 +22,6 @@
 
 from __future__ import unicode_literals
 import collections
-import copy
 import heapq
 import re
 
@@ -273,8 +272,8 @@ def __init__(self, state, newline, previous):
     self.previous = previous
 
   def __repr__(self):  # pragma: no cover
-    return 'StateNode(state=[\n{0}\n], newline={1})'.format(self.state,
-                                                            self.newline)
+    return 'StateNode(state=[\n{0}\n], newline={1})'.format(
+        self.state, self.newline)
 
 
 # A tuple of (penalty, count) that is used to prioritize the BFS. In case of

yapf/yapflib/split_penalty.py

@@ -374,12 +374,9 @@ def Visit_atom(self, node):  # pylint: disable=invalid-name
           pytree_utils.SetNodeAnnotation(node.children[-1],
                                          pytree_utils.Annotation.SPLIT_PENALTY,
                                          COMPARISON_EXPRESSION)
-    elif node.children[0].value in '[{':
+    elif node.children[0].value in '[{' and len(node.children) == 2:
       # Keep empty containers together if we can.
-      lbracket = node.children[0]
-      rbracket = node.children[-1]
-      if len(node.children) == 2:
-        _SetUnbreakable(node.children[-1])
+      _SetUnbreakable(node.children[-1])
 
   ############################################################################
   # Helper methods that set the annotations.

yapf/yapflib/subtype_assigner.py

@@ -91,8 +91,10 @@ def Visit_dictsetmaker(self, node):  # pylint: disable=invalid-name
             else:
               _AppendFirstLeafTokenSubtype(
                   child, format_token.Subtype.DICTIONARY_VALUE)
-          elif (child is not None and (isinstance(child, pytree.Node) or (
-              not child.value.startswith('#') and child.value not in '{:,'))):
+          elif (
+              child is not None and
+              (isinstance(child, pytree.Node) or
+               (not child.value.startswith('#') and child.value not in '{:,'))):
             # Mark the first leaf of a key entry as a DICTIONARY_KEY. We
             # normally want to split before them if the dictionary cannot exist
             # on a single line.

yapf/yapflib/unwrapped_line.py

@@ -145,8 +145,8 @@ def __str__(self):  # pragma: no cover
   def __repr__(self):  # pragma: no cover
     tokens_repr = ','.join(
         ['{0}({1!r})'.format(tok.name, tok.value) for tok in self._tokens])
-    return 'UnwrappedLine(depth={0}, tokens=[{1}])'.format(self.depth,
-                                                           tokens_repr)
+    return 'UnwrappedLine(depth={0}, tokens=[{1}])'.format(
+        self.depth, tokens_repr)
 
   ############################################################################
   # Properties                                                               #

yapf/yapflib/yapf_api.py

@@ -260,17 +260,15 @@ def _MarkLinesToFormat(uwlines, lines):
 
 
 def _DisableYAPF(line):
-  return (re.search(DISABLE_PATTERN, line.split('\n')[0].strip(),
-                    re.IGNORECASE) or re.search(DISABLE_PATTERN,
-                                                line.split('\n')[-1].strip(),
-                                                re.IGNORECASE))
+  return (
+      re.search(DISABLE_PATTERN, line.split('\n')[0].strip(), re.IGNORECASE) or
+      re.search(DISABLE_PATTERN, line.split('\n')[-1].strip(), re.IGNORECASE))
 
 
 def _EnableYAPF(line):
-  return (re.search(ENABLE_PATTERN, line.split('\n')[0].strip(),
-                    re.IGNORECASE) or re.search(ENABLE_PATTERN,
-                                                line.split('\n')[-1].strip(),
-                                                re.IGNORECASE))
+  return (
+      re.search(ENABLE_PATTERN, line.split('\n')[0].strip(), re.IGNORECASE) or
+      re.search(ENABLE_PATTERN, line.split('\n')[-1].strip(), re.IGNORECASE))
 
 
 def _GetUnifiedDiff(before, after, filename='code'):

yapftests/file_resources_test.py

@@ -105,7 +105,8 @@ def test_recursive_find_in_dir(self):
     tdir3 = self._make_test_dir('test3/foo/bar/bas/xxx')
     files = [
         os.path.join(tdir1, 'testfile1.py'),
-        os.path.join(tdir2, 'testfile2.py'), os.path.join(tdir3, 'testfile3.py')
+        os.path.join(tdir2, 'testfile2.py'),
+        os.path.join(tdir3, 'testfile3.py'),
     ]
     _touch_files(files)
 
@@ -121,7 +122,8 @@ def test_recursive_find_in_dir_with_exclude(self):
     tdir3 = self._make_test_dir('test3/foo/bar/bas/xxx')
     files = [
         os.path.join(tdir1, 'testfile1.py'),
-        os.path.join(tdir2, 'testfile2.py'), os.path.join(tdir3, 'testfile3.py')
+        os.path.join(tdir2, 'testfile2.py'),
+        os.path.join(tdir3, 'testfile3.py'),
     ]
     _touch_files(files)
 
@@ -131,7 +133,7 @@ def test_recursive_find_in_dir_with_exclude(self):
                 [self.test_tmpdir], recursive=True, exclude=['*test*3.py'])),
         sorted([
             os.path.join(tdir1, 'testfile1.py'),
-            os.path.join(tdir2, 'testfile2.py')
+            os.path.join(tdir2, 'testfile2.py'),
         ]))
 
 

yapftests/format_decision_state_test.py

@@ -13,7 +13,6 @@
 # limitations under the License.
 """Tests for yapf.format_decision_state."""
 
-import copy
 import textwrap
 import unittest
 
@@ -82,7 +81,7 @@ def f(a, b):
     self.assertFalse(state.CanSplit(False))
     self.assertFalse(state.MustSplit())
 
-    clone = copy.copy(state)
+    clone = state.Clone()
     self.assertEqual(repr(state), repr(clone))
 
   def testSimpleFunctionDefWithSplitting(self):
@@ -130,7 +129,7 @@ def f(a, b):
     self.assertEqual(':', state.next_token.value)
     self.assertFalse(state.CanSplit(False))
 
-    clone = copy.copy(state)
+    clone = state.Clone()
     self.assertEqual(repr(state), repr(clone))
 
 

yapftests/reformatter_basic_test.py

@@ -1573,6 +1573,33 @@ def method(self):
     finally:
       style.SetGlobalStyle(style.CreateChromiumStyle())
 
+  def testStableInlinedDictionaryFormatting(self):
+    try:
+      style.SetGlobalStyle(style.CreatePEP8Style())
+      unformatted_code = textwrap.dedent("""\
+          def _():
+              url = "http://{0}/axis-cgi/admin/param.cgi?{1}".format(
+                  value, urllib.urlencode({'action': 'update', 'parameter': value}))
+          """)
+      expected_formatted_code = textwrap.dedent("""\
+          def _():
+              url = "http://{0}/axis-cgi/admin/param.cgi?{1}".format(
+                  value, urllib.urlencode({
+                      'action': 'update',
+                      'parameter': value
+                  }))
+          """)
+
+      uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code)
+      reformatted_code = reformatter.Reformat(uwlines)
+      self.assertCodeEqual(expected_formatted_code, reformatted_code)
+
+      uwlines = yapf_test_helper.ParseAndUnwrap(reformatted_code)
+      reformatted_code = reformatter.Reformat(uwlines)
+      self.assertCodeEqual(expected_formatted_code, reformatted_code)
+    finally:
+      style.SetGlobalStyle(style.CreateChromiumStyle())
+
   def testDontSplitKeywordValueArguments(self):
     unformatted_code = textwrap.dedent("""\
         def mark_game_scored(gid):

yapftests/subtype_assigner_test.py

@@ -158,7 +158,7 @@ def testBitwiseOperators(self):
          ('3', [format_token.Subtype.NONE]),
          (')', [format_token.Subtype.NONE]),
          ('>>', {format_token.Subtype.BINARY_OPERATOR}),
-         ('1', [format_token.Subtype.NONE])],
+         ('1', [format_token.Subtype.NONE]),],
     ])  # yapf: disable
 
   def testSubscriptColon(self):
@@ -167,14 +167,16 @@ def testSubscriptColon(self):
         """)
     uwlines = yapf_test_helper.ParseAndUnwrap(code)
     self._CheckFormatTokenSubtypes(uwlines, [
-        [('x', [format_token.Subtype.NONE]),
-         ('[', {format_token.Subtype.SUBSCRIPT_BRACKET}),
-         ('0', [format_token.Subtype.NONE]),
-         (':', {format_token.Subtype.SUBSCRIPT_COLON}),
-         ('42', [format_token.Subtype.NONE]),
-         (':', {format_token.Subtype.SUBSCRIPT_COLON}),
-         ('1', [format_token.Subtype.NONE]),
-         (']', {format_token.Subtype.SUBSCRIPT_BRACKET})],
+        [
+            ('x', [format_token.Subtype.NONE]),
+            ('[', {format_token.Subtype.SUBSCRIPT_BRACKET}),
+            ('0', [format_token.Subtype.NONE]),
+            (':', {format_token.Subtype.SUBSCRIPT_COLON}),
+            ('42', [format_token.Subtype.NONE]),
+            (':', {format_token.Subtype.SUBSCRIPT_COLON}),
+            ('1', [format_token.Subtype.NONE]),
+            (']', {format_token.Subtype.SUBSCRIPT_BRACKET}),
+        ],
     ])
 
   def testFunctionCallWithStarExpression(self):
@@ -188,7 +190,7 @@ def testFunctionCallWithStarExpression(self):
          (',', [format_token.Subtype.NONE]),
          ('*', {format_token.Subtype.UNARY_OPERATOR}),
          ('b', [format_token.Subtype.NONE]),
-         (']', [format_token.Subtype.NONE])]
+         (']', [format_token.Subtype.NONE]),],
     ])  # yapf: disable
 
 

yapftests/yapf_test.py

@@ -1137,8 +1137,8 @@ def foo_function():
             pass
         """)
 
-    with utils.NamedTempFile(
-        dirname=self.test_tmpdir) as (stylefile, stylepath):
+    with utils.NamedTempFile(dirname=self.test_tmpdir) as (stylefile,
+                                                           stylepath):
       p = subprocess.Popen(
           YAPF_BINARY + ['--style-help'],
           stdout=stylefile,