From 87c6a294c436343ce95530bb1cd28cb1ca2562b8 Mon Sep 17 00:00:00 2001 From: Jesse Clemens Date: Fri, 25 Aug 2023 12:58:49 -0400 Subject: [PATCH 1/4] Add a new knob to allow logical operators to split This comes from the issue https://github.com/google/yapf/issues/844 where logical operators are bin-packed if the line is too long instead of wrapping nicely one to each line --- yapf/yapflib/format_decision_state.py | 51 +++++++++++++++++++++++++++ yapf/yapflib/logical_line.py | 49 +++++++++++++++++-------- yapf/yapflib/style.py | 8 +++++ 3 files changed, 93 insertions(+), 15 deletions(-) diff --git a/yapf/yapflib/format_decision_state.py b/yapf/yapflib/format_decision_state.py index 06f3455d9..c015428fe 100644 --- a/yapf/yapflib/format_decision_state.py +++ b/yapf/yapflib/format_decision_state.py @@ -555,6 +555,37 @@ def SurroundedByParens(token): if not self._FitsOnLine(previous, previous.matching_bracket): return True + ########################################################################### + # Logical Operator Splitting + if style.Get('SPLIT_ALL_LOGICAL_OPERATORS_IF_ANY_SPLIT'): + split_before = style.Get("SPLIT_BEFORE_LOGICAL_OPERATOR") + check_token = current if split_before else current.previous_token + if (check_token and check_token.name == "NAME" and check_token.value in logical_line._LOGICAL_OPERATORS): + opening = _GetOpeningBracket(check_token) + if opening: + ending = opening.matching_bracket + length = ending.total_length - opening.total_length + length += self.stack[-1].indent + # If we're keeping it on a single line, but the next token is also + # a logical operator then we have to consider that as part of the + # length because we might wrap after it + next_token = ending.next_token + prev_token = opening.previous_token + if split_before: + if prev_token: + clause_start = _PrevLogicalClause(prev_token) + length += opening.total_length - clause_start.total_length + else: + if next_token: + clause_end = _NextLogicalClause(next_token) + length += clause_end.total_length - ending.total_length + else: + end_token = _LastTokenInLine(check_token) + length = end_token.total_length + self.stack[-1].indent + + if length >= self.column_limit: + return True + ########################################################################### # Original Formatting Splitting # These checks rely upon the original formatting. This is in order to @@ -1181,6 +1212,26 @@ def _LastTokenInLine(current): return current +def _NextLogicalClause(token): + """ Get the start of the next logical clause or the last token in the line""" + while token: + if token in logical_line._LOGICAL_OPERATORS: + return token + if not token.next_token: + return token + token = token.next_token + + +def _PrevLogicalClause(token): + """ Get the start of the previous logical clause or the first token""" + while token: + if token.value in logical_line._LOGICAL_OPERATORS: + return token + if not token.previous_token: + return token + token = token.previous_token + + def _IsFunctionDefinition(current): prev = current.previous_token return current.value == '(' and prev and subtypes.FUNC_DEF in prev.subtypes diff --git a/yapf/yapflib/logical_line.py b/yapf/yapflib/logical_line.py index 4433276f7..50c113d29 100644 --- a/yapf/yapflib/logical_line.py +++ b/yapf/yapflib/logical_line.py @@ -610,21 +610,40 @@ def _SplitPenalty(prev_token, cur_token): if pval == 'not': return split_penalty.UNBREAKABLE - if cur_token.node_split_penalty > 0: - return cur_token.node_split_penalty - - if style.Get('SPLIT_BEFORE_LOGICAL_OPERATOR'): - # Prefer to split before 'and' and 'or'. - if pval in _LOGICAL_OPERATORS: - return style.Get('SPLIT_PENALTY_LOGICAL_OPERATOR') - if cval in _LOGICAL_OPERATORS: - return 0 - else: - # Prefer to split after 'and' and 'or'. - if pval in _LOGICAL_OPERATORS: - return 0 - if cval in _LOGICAL_OPERATORS: - return style.Get('SPLIT_PENALTY_LOGICAL_OPERATOR') + node_split_penalty = cur_token.node_split_penalty + + logical_splitting = style.Get('SPLIT_ALL_LOGICAL_OPERATORS_IF_ANY_SPLIT') + + if logical_splitting: + if style.Get('SPLIT_BEFORE_LOGICAL_OPERATOR'): + # Prefer to split before 'and' and 'or'. + if pval in _LOGICAL_OPERATORS: + return max(node_split_penalty, style.Get('SPLIT_PENALTY_LOGICAL_OPERATOR')) + if cval in _LOGICAL_OPERATORS: + return max(node_split_penalty, 0) + else: + # Prefer to split after 'and' and 'or'. + if pval in _LOGICAL_OPERATORS: + return max(node_split_penalty, 0) + if cval in _LOGICAL_OPERATORS: + return max(node_split_penalty, style.Get('SPLIT_PENALTY_LOGICAL_OPERATOR')) + + if node_split_penalty > 0: + return node_split_penalty + + if not logical_splitting: + if style.Get('SPLIT_BEFORE_LOGICAL_OPERATOR'): + # Prefer to split before 'and' and 'or'. + if pval in _LOGICAL_OPERATORS: + return style.Get('SPLIT_PENALTY_LOGICAL_OPERATOR') + if cval in _LOGICAL_OPERATORS: + return 0 + else: + # Prefer to split after 'and' and 'or'. + if pval in _LOGICAL_OPERATORS: + return 0 + if cval in _LOGICAL_OPERATORS: + return style.Get('SPLIT_PENALTY_LOGICAL_OPERATOR') if style.Get('SPLIT_BEFORE_BITWISE_OPERATOR'): # Prefer to split before '&', '|', and '^'. diff --git a/yapf/yapflib/style.py b/yapf/yapflib/style.py index 7642c01f4..19bbc0eb1 100644 --- a/yapf/yapflib/style.py +++ b/yapf/yapflib/style.py @@ -355,6 +355,12 @@ def method(): SPLIT_ALL_COMMA_SEPARATED_VALUES=textwrap.dedent("""\ Split before arguments. """), + SPLIT_ALL_LOGICAL_OPERATORS_IF_ANY_SPLIT=textwrap.dedent("""\ + If a line that contains logical operators needs to be split, split on all + the logical operators in that line. This will treat logical operators + in sub-clauses defined by parentheses as a discrete element that will + ignore wrapping unless the entire clause is longer than the column width. + """), SPLIT_ALL_TOP_LEVEL_COMMA_SEPARATED_VALUES=textwrap.dedent("""\ Split before arguments, but do not split all subexpressions recursively (unless needed). @@ -514,6 +520,7 @@ def CreatePEP8Style(): SPACES_AROUND_TUPLE_DELIMITERS=False, SPACES_BEFORE_COMMENT=2, SPLIT_ALL_COMMA_SEPARATED_VALUES=False, + SPLIT_ALL_LOGICAL_OPERATORS_IF_ANY_SPLIT=False, SPLIT_ALL_TOP_LEVEL_COMMA_SEPARATED_VALUES=False, SPLIT_ARGUMENTS_WHEN_COMMA_TERMINATED=False, SPLIT_BEFORE_ARITHMETIC_OPERATOR=False, @@ -703,6 +710,7 @@ def _IntOrIntListConverter(s): SPACES_AROUND_TUPLE_DELIMITERS=_BoolConverter, SPACES_BEFORE_COMMENT=_IntOrIntListConverter, SPLIT_ALL_COMMA_SEPARATED_VALUES=_BoolConverter, + SPLIT_ALL_LOGICAL_OPERATORS_IF_ANY_SPLIT=_BoolConverter, SPLIT_ALL_TOP_LEVEL_COMMA_SEPARATED_VALUES=_BoolConverter, SPLIT_ARGUMENTS_WHEN_COMMA_TERMINATED=_BoolConverter, SPLIT_BEFORE_ARITHMETIC_OPERATOR=_BoolConverter, From d4a70e4badd97b2ea0fc07b8d69117bb7aa9e262 Mon Sep 17 00:00:00 2001 From: Jesse Clemens Date: Fri, 25 Aug 2023 13:01:36 -0400 Subject: [PATCH 2/4] Apply proper formatting to the changes --- yapf/yapflib/format_decision_state.py | 9 +++++---- yapf/yapflib/logical_line.py | 14 ++++++++------ 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/yapf/yapflib/format_decision_state.py b/yapf/yapflib/format_decision_state.py index c015428fe..2f1563583 100644 --- a/yapf/yapflib/format_decision_state.py +++ b/yapf/yapflib/format_decision_state.py @@ -560,7 +560,8 @@ def SurroundedByParens(token): if style.Get('SPLIT_ALL_LOGICAL_OPERATORS_IF_ANY_SPLIT'): split_before = style.Get("SPLIT_BEFORE_LOGICAL_OPERATOR") check_token = current if split_before else current.previous_token - if (check_token and check_token.name == "NAME" and check_token.value in logical_line._LOGICAL_OPERATORS): + if (check_token and check_token.name == "NAME" and + check_token.value in logical_line._LOGICAL_OPERATORS): opening = _GetOpeningBracket(check_token) if opening: ending = opening.matching_bracket @@ -572,9 +573,9 @@ def SurroundedByParens(token): next_token = ending.next_token prev_token = opening.previous_token if split_before: - if prev_token: - clause_start = _PrevLogicalClause(prev_token) - length += opening.total_length - clause_start.total_length + if prev_token: + clause_start = _PrevLogicalClause(prev_token) + length += opening.total_length - clause_start.total_length else: if next_token: clause_end = _NextLogicalClause(next_token) diff --git a/yapf/yapflib/logical_line.py b/yapf/yapflib/logical_line.py index 50c113d29..7f25c648c 100644 --- a/yapf/yapflib/logical_line.py +++ b/yapf/yapflib/logical_line.py @@ -616,17 +616,19 @@ def _SplitPenalty(prev_token, cur_token): if logical_splitting: if style.Get('SPLIT_BEFORE_LOGICAL_OPERATOR'): - # Prefer to split before 'and' and 'or'. - if pval in _LOGICAL_OPERATORS: - return max(node_split_penalty, style.Get('SPLIT_PENALTY_LOGICAL_OPERATOR')) - if cval in _LOGICAL_OPERATORS: - return max(node_split_penalty, 0) + # Prefer to split before 'and' and 'or'. + if pval in _LOGICAL_OPERATORS: + return max(node_split_penalty, + style.Get('SPLIT_PENALTY_LOGICAL_OPERATOR')) + if cval in _LOGICAL_OPERATORS: + return max(node_split_penalty, 0) else: # Prefer to split after 'and' and 'or'. if pval in _LOGICAL_OPERATORS: return max(node_split_penalty, 0) if cval in _LOGICAL_OPERATORS: - return max(node_split_penalty, style.Get('SPLIT_PENALTY_LOGICAL_OPERATOR')) + return max(node_split_penalty, + style.Get('SPLIT_PENALTY_LOGICAL_OPERATOR')) if node_split_penalty > 0: return node_split_penalty From 48c98b2e16d610a96cc6bf0096a208ee87e9c50c Mon Sep 17 00:00:00 2001 From: Jesse Clemens Date: Fri, 25 Aug 2023 15:29:48 -0400 Subject: [PATCH 3/4] Simplify the conditional --- yapf/yapflib/format_decision_state.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/yapf/yapflib/format_decision_state.py b/yapf/yapflib/format_decision_state.py index 2f1563583..1a6daddcb 100644 --- a/yapf/yapflib/format_decision_state.py +++ b/yapf/yapflib/format_decision_state.py @@ -572,14 +572,12 @@ def SurroundedByParens(token): # length because we might wrap after it next_token = ending.next_token prev_token = opening.previous_token - if split_before: - if prev_token: - clause_start = _PrevLogicalClause(prev_token) - length += opening.total_length - clause_start.total_length - else: - if next_token: - clause_end = _NextLogicalClause(next_token) - length += clause_end.total_length - ending.total_length + if split_before and prev_token: + clause_start = _PrevLogicalClause(prev_token) + length += opening.total_length - clause_start.total_length + elif not split_before and next_token + clause_end = _NextLogicalClause(next_token) + length += clause_end.total_length - ending.total_length else: end_token = _LastTokenInLine(check_token) length = end_token.total_length + self.stack[-1].indent From 7ae8aaa0912c17fa826915757f060b5e6b9de9d8 Mon Sep 17 00:00:00 2001 From: Jesse Clemens Date: Mon, 28 Aug 2023 10:58:52 -0400 Subject: [PATCH 4/4] Add a missing colon that got misplaced during code cleanup --- yapf/yapflib/format_decision_state.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yapf/yapflib/format_decision_state.py b/yapf/yapflib/format_decision_state.py index 1a6daddcb..308fd75b9 100644 --- a/yapf/yapflib/format_decision_state.py +++ b/yapf/yapflib/format_decision_state.py @@ -575,7 +575,7 @@ def SurroundedByParens(token): if split_before and prev_token: clause_start = _PrevLogicalClause(prev_token) length += opening.total_length - clause_start.total_length - elif not split_before and next_token + elif not split_before and next_token: clause_end = _NextLogicalClause(next_token) length += clause_end.total_length - ending.total_length else: