diff --git a/CHANGELOG b/CHANGELOG index 5450b8f27..3f8a6fe4b 100644 --- a/CHANGELOG +++ b/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 diff --git a/yapf/yapflib/format_decision_state.py b/yapf/yapflib/format_decision_state.py index 2e84f61d4..8bbe8f7a0 100644 --- a/yapf/yapflib/format_decision_state.py +++ b/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. diff --git a/yapf/yapflib/reformatter.py b/yapf/yapflib/reformatter.py index 9b791badb..fd8f3178a 100644 --- a/yapf/yapflib/reformatter.py +++ b/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 diff --git a/yapf/yapflib/split_penalty.py b/yapf/yapflib/split_penalty.py index 8a9a64552..a18eb02ff 100644 --- a/yapf/yapflib/split_penalty.py +++ b/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. diff --git a/yapf/yapflib/subtype_assigner.py b/yapf/yapflib/subtype_assigner.py index 3e27fbfe5..646cdc8a4 100644 --- a/yapf/yapflib/subtype_assigner.py +++ b/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. diff --git a/yapf/yapflib/unwrapped_line.py b/yapf/yapflib/unwrapped_line.py index 8e0d3d138..77b2f1e2e 100644 --- a/yapf/yapflib/unwrapped_line.py +++ b/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 # diff --git a/yapf/yapflib/yapf_api.py b/yapf/yapflib/yapf_api.py index 3a5031998..dd91849ea 100644 --- a/yapf/yapflib/yapf_api.py +++ b/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'): diff --git a/yapftests/file_resources_test.py b/yapftests/file_resources_test.py index 67e373b4b..adfdae769 100644 --- a/yapftests/file_resources_test.py +++ b/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'), ])) diff --git a/yapftests/reformatter_basic_test.py b/yapftests/reformatter_basic_test.py index c6182cce3..c530ffc09 100644 --- a/yapftests/reformatter_basic_test.py +++ b/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): diff --git a/yapftests/subtype_assigner_test.py b/yapftests/subtype_assigner_test.py index 72cb6a278..db0e17d10 100644 --- a/yapftests/subtype_assigner_test.py +++ b/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 diff --git a/yapftests/yapf_test.py b/yapftests/yapf_test.py index ed94810c8..fdd4fb98a 100644 --- a/yapftests/yapf_test.py +++ b/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,