Add knob SPLIT_ALL_TOP_LEVEL_COMMA_SEPARATED_VALUES (#1)
Add knob SPLIT_ALL_TOP_LEVEL_COMMA_SEPARATED_VALUES
The existing knob SPLIT_ALL_COMMA_SEPARATED_VALUES is overly aggresive in that, if an argument list (or, more generally, a container) needs splitting, then all subexpressions will be split as well. For instance, in
```
abcdef(aReallyLongThing, b=(c,d))
```
if the line needs splitting because of `aReallyLongThing`, then it will produce
```
abcdef(
aReallyLongThing,
b=(c,
d))
```
I've seen terrible things like
```
argument: [Int,
Int]
```
in function definitions with many arguments (even if the second `Int` fits in the line).
The new knob checks if a container (argument list, list literal, etc) fits in a line and avoids breaking if so, producing
```
abcdef(
aReallyLongThing,
b=(c, d))
```
which makes more sense (to me at least).
See https://github.com/prodo-ai/plz/pull/248 for an example of a codebase reformatted with the new knob (plus `split_before_first_argument`). Nice, isn't it?diff --git a/yapf/yapflib/format_decision_state.py b/yapf/yapflib/format_decision_state.py
index a322d38..4965d60 100644
--- a/yapf/yapflib/format_decision_state.py
+++ b/yapf/yapflib/format_decision_state.py
@@ -176,6 +176,20 @@
if style.Get('SPLIT_ALL_COMMA_SEPARATED_VALUES') and previous.value == ',':
return True
+ if style.Get(
+ 'SPLIT_ALL_TOP_LEVEL_COMMA_SEPARATED_VALUES') and previous.value == ',':
+ # Avoid breaking in a container that fits in the current line if possible
+ opening = _GetOpeningBracket(current)
+
+ # Can't find opening bracket, behave the same way as
+ # SPLIT_ALL_COMMA_SEPARATED_VALUES
+ if not opening:
+ return True
+
+ matching_bracket = opening.matching_bracket
+ # If the container doesn't fit in the current line, must split
+ return not self._ContainerFitsOnStartLine(opening)
+
if (self.stack[-1].split_before_closing_bracket and
current.value in '}]' and style.Get('SPLIT_BEFORE_CLOSING_BRACKET')):
# Split before the closing bracket if we can.
@@ -370,10 +384,7 @@
opening = _GetOpeningBracket(current)
if opening:
- arglist_length = (
- opening.matching_bracket.total_length - opening.total_length +
- self.stack[-1].indent)
- return arglist_length > self.column_limit
+ return not self._ContainerFitsOnStartLine(opening)
if (current.value not in '{)' and previous.value == '(' and
self._ArgumentListHasDictionaryEntry(current)):
@@ -935,6 +946,15 @@
token = token.next_token
return False
+ def _ContainerFitsOnStartLine(self, opening):
+ """Check if the container can fit on its starting line.
+
+ Arguments:
+ opening: (FormatToken) The unwrapped line we're currently processing.
+ """
+ return (opening.matching_bracket.total_length - opening.total_length +
+ self.stack[-1].indent) <= self.column_limit
+
_COMPOUND_STMTS = frozenset(
{'for', 'while', 'if', 'elif', 'with', 'except', 'def', 'class'})
diff --git a/yapf/yapflib/style.py b/yapf/yapflib/style.py
index 7954c34..48b3984 100644
--- a/yapf/yapflib/style.py
+++ b/yapf/yapflib/style.py
@@ -239,6 +239,9 @@
comma."""),
SPLIT_ALL_COMMA_SEPARATED_VALUES=textwrap.dedent("""\
Split before arguments"""),
+ SPLIT_ALL_TOP_LEVEL_COMMA_SEPARATED_VALUES=textwrap.dedent("""\
+ Split before arguments, but do not split all subexpressions recursively
+ (unless needed)."""),
SPLIT_BEFORE_ARITHMETIC_OPERATOR=textwrap.dedent("""\
Set to True to prefer splitting before '+', '-', '*', '/', '//', or '@'
rather than after."""),
@@ -367,6 +370,7 @@
SPACES_BEFORE_COMMENT=2,
SPLIT_ARGUMENTS_WHEN_COMMA_TERMINATED=False,
SPLIT_ALL_COMMA_SEPARATED_VALUES=False,
+ SPLIT_ALL_TOP_LEVEL_COMMA_SEPARATED_VALUES=False,
SPLIT_BEFORE_ARITHMETIC_OPERATOR=False,
SPLIT_BEFORE_BITWISE_OPERATOR=True,
SPLIT_BEFORE_CLOSING_BRACKET=True,
@@ -543,6 +547,7 @@
SPACES_BEFORE_COMMENT=_IntOrIntListConverter,
SPLIT_ARGUMENTS_WHEN_COMMA_TERMINATED=_BoolConverter,
SPLIT_ALL_COMMA_SEPARATED_VALUES=_BoolConverter,
+ SPLIT_ALL_TOP_LEVEL_COMMA_SEPARATED_VALUES=_BoolConverter,
SPLIT_BEFORE_ARITHMETIC_OPERATOR=_BoolConverter,
SPLIT_BEFORE_BITWISE_OPERATOR=_BoolConverter,
SPLIT_BEFORE_CLOSING_BRACKET=_BoolConverter,
diff --git a/yapftests/reformatter_basic_test.py b/yapftests/reformatter_basic_test.py
index 60d7cb6..a7d5af1 100644
--- a/yapftests/reformatter_basic_test.py
+++ b/yapftests/reformatter_basic_test.py
@@ -43,6 +43,8 @@
"whatever": 120
}
""")
+ uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code)
+ self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
unformatted_code = textwrap.dedent("""\
def foo(long_arg, really_long_arg, really_really_long_arg, cant_keep_all_these_args):
pass
@@ -77,6 +79,104 @@
""")
uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code)
self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
+ # There is a test for split_all_top_level_comma_separated_values, with
+ # different expected value
+ unformatted_code = textwrap.dedent("""\
+ someLongFunction(this_is_a_very_long_parameter,
+ abc=(a, this_will_just_fit_xxxxxxx))
+ """)
+ expected_formatted_code = textwrap.dedent("""\
+ someLongFunction(
+ this_is_a_very_long_parameter,
+ abc=(a,
+ this_will_just_fit_xxxxxxx))
+ """)
+ uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code)
+ self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
+
+ def testSplittingTopLevelAllArgs(self):
+ style.SetGlobalStyle(
+ style.CreateStyleFromConfig(
+ '{split_all_top_level_comma_separated_values: true, column_limit: 40}'
+ ))
+ # Works the same way as split_all_comma_separated_values
+ unformatted_code = textwrap.dedent("""\
+ responseDict = {"timestamp": timestamp, "someValue": value, "whatever": 120}
+ """)
+ expected_formatted_code = textwrap.dedent("""\
+ responseDict = {
+ "timestamp": timestamp,
+ "someValue": value,
+ "whatever": 120
+ }
+ """)
+ uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code)
+ self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
+ # Works the same way as split_all_comma_separated_values
+ unformatted_code = textwrap.dedent("""\
+ def foo(long_arg, really_long_arg, really_really_long_arg, cant_keep_all_these_args):
+ pass
+ """)
+ expected_formatted_code = textwrap.dedent("""\
+ def foo(long_arg,
+ really_long_arg,
+ really_really_long_arg,
+ cant_keep_all_these_args):
+ pass
+ """)
+ uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code)
+ self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
+ # Works the same way as split_all_comma_separated_values
+ unformatted_code = textwrap.dedent("""\
+ foo_tuple = [long_arg, really_long_arg, really_really_long_arg, cant_keep_all_these_args]
+ """)
+ expected_formatted_code = textwrap.dedent("""\
+ foo_tuple = [
+ long_arg,
+ really_long_arg,
+ really_really_long_arg,
+ cant_keep_all_these_args
+ ]
+ """)
+ uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code)
+ self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
+ # Works the same way as split_all_comma_separated_values
+ unformatted_code = textwrap.dedent("""\
+ foo_tuple = [short, arg]
+ """)
+ expected_formatted_code = textwrap.dedent("""\
+ foo_tuple = [short, arg]
+ """)
+ uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code)
+ self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
+ # There is a test for split_all_comma_separated_values, with different
+ # expected value
+ unformatted_code = textwrap.dedent("""\
+ someLongFunction(this_is_a_very_long_parameter,
+ abc=(a, this_will_just_fit_xxxxxxx))
+ """)
+ expected_formatted_code = textwrap.dedent("""\
+ someLongFunction(
+ this_is_a_very_long_parameter,
+ abc=(a, this_will_just_fit_xxxxxxx))
+ """)
+ uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code)
+ actual_formatted_code = reformatter.Reformat(uwlines)
+ self.assertEqual(40, len(actual_formatted_code.splitlines()[-1]))
+ self.assertCodeEqual(expected_formatted_code, actual_formatted_code)
+
+ unformatted_code = textwrap.dedent("""\
+ someLongFunction(this_is_a_very_long_parameter,
+ abc=(a, this_will_not_fit_xxxxxxxxx))
+ """)
+ expected_formatted_code = textwrap.dedent("""\
+ someLongFunction(
+ this_is_a_very_long_parameter,
+ abc=(a,
+ this_will_not_fit_xxxxxxxxx))
+ """)
+ uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code)
+ self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
def testSimpleFunctionsWithTrailingComments(self):
unformatted_code = textwrap.dedent("""\
