Skip to content

Commit

Permalink
Add knobs to split before an arithmetic operator.
Browse files Browse the repository at this point in the history
Closes #658
  • Loading branch information
bwendling committed Feb 10, 2019
1 parent e2ce893 commit 27fe4ad
Show file tree
Hide file tree
Showing 9 changed files with 73 additions and 32 deletions.
6 changes: 5 additions & 1 deletion CHANGELOG
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@
# All notable changes to this project will be documented in this file.
# This project adheres to [Semantic Versioning](http://semver.org/).

## [0.26.1] UNRELEASED
## [0.27.0] UNRELEASED
### Added
- `SPLIT_BEFORE_ARITHMETIC_OPERATOR` splits before an arithmetic operator when
set. `SPLIT_PENALTY_ARITHMETIC_OPERATOR` allows you to set the split penalty
around arithmetic operators.
### Fixed
- Parse integer lists correctly, removing quotes if the list is within a
string.
Expand Down
13 changes: 10 additions & 3 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -564,9 +564,12 @@ Knobs
If a comma separated list (dict, list, tuple, or function def) is on a
line that is too long, split such that all elements are on a single line.

``SPLIT_BEFORE_BITWISE_OPERATOR``
Set to ``True`` to prefer splitting before ``&``, ``|`` or ``^`` rather
than after.
``SPLIT_BEFORE__OPERATOR``
Set to True to prefer splitting before '&', '|' or '^' rather than after.
``SPLIT_BEFORE_ARITHMETIC_OPERATOR``
Set to True to prefer splitting before '+', '-', '*', '/', '//', or '@'
rather than after.

``SPLIT_BEFORE_CLOSING_BRACKET``
Split before the closing bracket if a list or dict literal doesn't fit on
Expand Down Expand Up @@ -639,6 +642,10 @@ Knobs
``SPLIT_PENALTY_AFTER_UNARY_OPERATOR``
The penalty for splitting the line after a unary operator.

``SPLIT_PENALTY_ARITHMETIC_OPERATOR``
The penalty of splitting the line around the ``+``, ``-``, ``*``, ``/``,
``//``, ``%``, and ``@`` operators.

``SPLIT_PENALTY_BEFORE_IF_EXPR``
The penalty for splitting right before an ``if`` expression.

Expand Down
4 changes: 2 additions & 2 deletions yapf/yapflib/comment_splicer.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,8 @@ def _VisitNodeRec(node):
rindex = (0 if '\n' not in comment_prefix.rstrip() else
comment_prefix.rstrip().rindex('\n') + 1)
comment_column = (
len(comment_prefix[rindex:]) - len(
comment_prefix[rindex:].lstrip()))
len(comment_prefix[rindex:]) -
len(comment_prefix[rindex:].lstrip()))
comments = _CreateCommentsFromPrefix(
comment_prefix,
comment_lineno,
Expand Down
12 changes: 6 additions & 6 deletions yapf/yapflib/format_token.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,8 @@ def AddWhitespacePrefix(self, newlines_before, spaces=0, indent_level=0):
else:
indent_before = '\t' * indent_level + ' ' * spaces
else:
indent_before = (
' ' * indent_level * style.Get('INDENT_WIDTH') + ' ' * spaces)
indent_before = (' ' * indent_level * style.Get('INDENT_WIDTH') +
' ' * spaces)

if self.is_comment:
comment_lines = [s.lstrip() for s in self.value.splitlines()]
Expand All @@ -180,15 +180,15 @@ def AddWhitespacePrefix(self, newlines_before, spaces=0, indent_level=0):
self.value = self.node.value

if not self.whitespace_prefix:
self.whitespace_prefix = (
'\n' * (self.newlines or newlines_before) + indent_before)
self.whitespace_prefix = ('\n' * (self.newlines or newlines_before) +
indent_before)
else:
self.whitespace_prefix += indent_before

def AdjustNewlinesBefore(self, newlines_before):
"""Change the number of newlines before this token."""
self.whitespace_prefix = (
'\n' * newlines_before + self.whitespace_prefix.lstrip('\n'))
self.whitespace_prefix = ('\n' * newlines_before +
self.whitespace_prefix.lstrip('\n'))

def RetainHorizontalSpacing(self, first_column, depth):
"""Retains a token's horizontal spacing."""
Expand Down
12 changes: 8 additions & 4 deletions yapf/yapflib/split_penalty.py
Original file line number Diff line number Diff line change
Expand Up @@ -550,10 +550,14 @@ def _SetBitwiseOperandPenalty(node, op):

def _SetExpressionOperandPenalty(node, ops, penalty):
for index in py3compat.range(1, len(node.children) - 1):
child = node.children[index]
if pytree_utils.NodeName(child) in ops:
next_node = pytree_utils.FirstLeafNode(node.children[index + 1])
_SetSplitPenalty(next_node, penalty)
child = node.children[index]
if pytree_utils.NodeName(child) in ops:
if style.Get('SPLIT_BEFORE_ARITHMETIC_OPERATOR'):
_SetSplitPenalty(child, style.Get('SPLIT_PENALTY_ARITHMETIC_OPERATOR'))
else:
_SetSplitPenalty(
pytree_utils.FirstLeafNode(node.children[index + 1]),
style.Get('SPLIT_PENALTY_ARITHMETIC_OPERATOR'))


def _IncreasePenalty(node, amt):
Expand Down
10 changes: 10 additions & 0 deletions yapf/yapflib/style.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,9 @@ def method():
comma."""),
SPLIT_ALL_COMMA_SEPARATED_VALUES=textwrap.dedent("""\
Split before arguments"""),
SPLIT_BEFORE_ARITHMETIC_OPERATOR=textwrap.dedent("""\
Set to True to prefer splitting before '+', '-', '*', '/', '//', or '@'
rather than after."""),
SPLIT_BEFORE_BITWISE_OPERATOR=textwrap.dedent("""\
Set to True to prefer splitting before '&', '|' or '^' rather than
after."""),
Expand Down Expand Up @@ -297,6 +300,9 @@ def method():
The penalty for splitting right after the opening bracket."""),
SPLIT_PENALTY_AFTER_UNARY_OPERATOR=textwrap.dedent("""\
The penalty for splitting the line after a unary operator."""),
SPLIT_PENALTY_ARITHMETIC_OPERATOR=textwrap.dedent("""\
The penalty of splitting the line around the '+', '-', '*', '/', '//',
``%``, and '@' operators."""),
SPLIT_PENALTY_BEFORE_IF_EXPR=textwrap.dedent("""\
The penalty for splitting right before an if expression."""),
SPLIT_PENALTY_BITWISE_OPERATOR=textwrap.dedent("""\
Expand Down Expand Up @@ -363,6 +369,7 @@ def CreatePEP8Style():
SPACES_BEFORE_COMMENT=2,
SPLIT_ARGUMENTS_WHEN_COMMA_TERMINATED=False,
SPLIT_ALL_COMMA_SEPARATED_VALUES=False,
SPLIT_BEFORE_ARITHMETIC_OPERATOR=False,
SPLIT_BEFORE_BITWISE_OPERATOR=True,
SPLIT_BEFORE_CLOSING_BRACKET=True,
SPLIT_BEFORE_DICT_SET_GENERATOR=True,
Expand All @@ -374,6 +381,7 @@ def CreatePEP8Style():
SPLIT_COMPLEX_COMPREHENSION=False,
SPLIT_PENALTY_AFTER_OPENING_BRACKET=30,
SPLIT_PENALTY_AFTER_UNARY_OPERATOR=10000,
SPLIT_PENALTY_ARITHMETIC_OPERATOR=300,
SPLIT_PENALTY_BEFORE_IF_EXPR=0,
SPLIT_PENALTY_BITWISE_OPERATOR=300,
SPLIT_PENALTY_COMPREHENSION=80,
Expand Down Expand Up @@ -536,6 +544,7 @@ def _IntOrIntListConverter(s):
SPACES_BEFORE_COMMENT=_IntOrIntListConverter,
SPLIT_ARGUMENTS_WHEN_COMMA_TERMINATED=_BoolConverter,
SPLIT_ALL_COMMA_SEPARATED_VALUES=_BoolConverter,
SPLIT_BEFORE_ARITHMETIC_OPERATOR=_BoolConverter,
SPLIT_BEFORE_BITWISE_OPERATOR=_BoolConverter,
SPLIT_BEFORE_CLOSING_BRACKET=_BoolConverter,
SPLIT_BEFORE_DICT_SET_GENERATOR=_BoolConverter,
Expand All @@ -547,6 +556,7 @@ def _IntOrIntListConverter(s):
SPLIT_COMPLEX_COMPREHENSION=_BoolConverter,
SPLIT_PENALTY_AFTER_OPENING_BRACKET=int,
SPLIT_PENALTY_AFTER_UNARY_OPERATOR=int,
SPLIT_PENALTY_ARITHMETIC_OPERATOR=int,
SPLIT_PENALTY_BEFORE_IF_EXPR=int,
SPLIT_PENALTY_BITWISE_OPERATOR=int,
SPLIT_PENALTY_COMPREHENSION=int,
Expand Down
15 changes: 5 additions & 10 deletions yapf/yapflib/unwrapped_line.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ def _IsUnaryOperator(tok):


def _HasPrecedence(tok):
"""Whether a binary operation has presedence within its context."""
"""Whether a binary operation has precedence within its context."""
node = tok.node

# We let ancestor be the statement surrounding the operation that tok is the
Expand All @@ -241,20 +241,20 @@ def _HasPrecedence(tok):
predecessor_type = pytree_utils.NodeName(ancestor)
if predecessor_type in ['arith_expr', 'term']:
# An ancestor "arith_expr" or "term" means we have found an operator
# with lower presedence than our tok.
# with lower precedence than our tok.
return True
if predecessor_type != 'atom':
# We understand the context to look for precedence within as an
# arbitrary nesting of "arith_expr", "term", and "atom" nodes. If we
# leave this context we have not found a lower presedence operator.
# leave this context we have not found a lower precedence operator.
return False
# Under normal usage we expect a complete parse tree to be available and
# we will return before we get an AttributeError from the root.
ancestor = ancestor.parent


def _PriorityIndicatingNoSpace(tok):
"""Whether to remove spaces around an operator due to presedence."""
"""Whether to remove spaces around an operator due to precedence."""
if not tok.is_arithmetic_op or not tok.is_simple_expr:
# Limit space removal to highest priority arithmetic operators
return False
Expand Down Expand Up @@ -519,7 +519,7 @@ def IsSurroundedByBrackets(tok):

_LOGICAL_OPERATORS = frozenset({'and', 'or'})
_BITWISE_OPERATORS = frozenset({'&', '|', '^'})
_TERM_OPERATORS = frozenset({'*', '/', '%', '//', '@'})
_ARITHMETIC_OPERATORS = frozenset({'+', '-', '*', '/', '%', '//', '@'})


def _SplitPenalty(prev_token, cur_token):
Expand Down Expand Up @@ -568,9 +568,6 @@ def _SplitPenalty(prev_token, cur_token):
if pval == ',':
# Breaking after a comma is fine, if need be.
return 0
if prev_token.is_binary_op:
# We would rather not split after an equality operator.
return split_penalty.CONNECTED
if pval == '**' or cval == '**':
return split_penalty.STRONGLY_CONNECTED
if (format_token.Subtype.VARARGS_STAR in prev_token.subtypes or
Expand All @@ -596,6 +593,4 @@ def _SplitPenalty(prev_token, cur_token):
if cur_token.ClosesScope():
# Give a slight penalty for splitting before the closing scope.
return 100
if pval in _TERM_OPERATORS or cval in _TERM_OPERATORS:
return 50
return 0
8 changes: 4 additions & 4 deletions yapftests/reformatter_buganizer_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -840,8 +840,8 @@ def _():
expected_formatted_code = textwrap.dedent("""\
class _():
def _():
hints.append(('hg tag -f -l -r %s %s # %s' % (short(
ctx.node()), candidatetag, firstline))[:78])
hints.append(('hg tag -f -l -r %s %s # %s' %
(short(ctx.node()), candidatetag, firstline))[:78])
""")
uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code)
self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
Expand Down Expand Up @@ -1457,8 +1457,8 @@ def foo():
expected_formatted_code = textwrap.dedent("""\
def foo():
if True:
return (
struct.pack('aaaa', bbbbbbbbbb, ccccccccccccccc, dddddddd) + eeeeeee)
return (struct.pack('aaaa', bbbbbbbbbb, ccccccccccccccc, dddddddd) +
eeeeeee)
""")
uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code)
self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
Expand Down
25 changes: 23 additions & 2 deletions yapftests/reformatter_pep8_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,8 @@ def testIndentSizeChanging(self):
""")
expected_formatted_code = textwrap.dedent("""\
if True:
runtime_mins = (
program_end_time - program_start_time).total_seconds() / 60.0
runtime_mins = (program_end_time -
program_start_time).total_seconds() / 60.0
""")
uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code)
self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
Expand Down Expand Up @@ -454,6 +454,27 @@ def _():
uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code)
self.assertEqual(expected_code, reformatter.Reformat(uwlines))

def testSplitBeforeArithmeticOperators(self):
try:
style.SetGlobalStyle(
style.CreateStyleFromConfig(
'{based_on_style: pep8, split_before_arithmetic_operator: true}'))

unformatted_code = """\
def _():
raise ValueError('This is a long message that ends with an argument: ' + str(42))
"""
expected_formatted_code = """\
def _():
raise ValueError('This is a long message that ends with an argument: '
+ str(42))
"""
uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code)
self.assertCodeEqual(expected_formatted_code,
reformatter.Reformat(uwlines))
finally:
style.SetGlobalStyle(style.CreatePEP8Style())


if __name__ == '__main__':
unittest.main()

0 comments on commit 27fe4ad

Please sign in to comment.