From 08c91388992890976d935f980aa59606ba4bb0f1 Mon Sep 17 00:00:00 2001 From: Charlie Date: Thu, 27 Oct 2022 21:11:45 +0100 Subject: [PATCH 01/19] Add comma split before delimiter split This fixes #3350. --- src/black/linegen.py | 80 ++++++++++++++++++- .../simple_cases/type_hint_after_comment.py | 5 ++ 2 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 tests/data/simple_cases/type_hint_after_comment.py diff --git a/src/black/linegen.py b/src/black/linegen.py index a2e41bf5912..a6dca8a482a 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -538,7 +538,12 @@ def _rhs( ] else: if line.inside_brackets: - transformers = [delimiter_split, standalone_comment_split, rhs] + transformers = [ + comma_split, + delimiter_split, + standalone_comment_split, + rhs, + ] else: transformers = [rhs] # It's always safe to attempt hugging of power operations and pretty much every line @@ -786,6 +791,79 @@ def split_wrapper(line: Line, features: Collection[Feature] = ()) -> Iterator[Li return split_wrapper +@dont_increase_indentation +def comma_split(line: Line, features: Collection[Feature] = ()) -> Iterator[Line]: + """Split on commas + + It works the same as delimiter_split(), but it only splits on commas. + """ + try: + last_leaf = line.leaves[-1] + except IndexError: + raise CannotSplit("Line empty") from None + + bt = line.bracket_tracker + try: + max_delimiter_priority = bt.max_delimiter_priority( + exclude={id(last_leaf)} + ) + trailing_comma_safe = max_delimiter_priority == COMMA_PRIORITY + except ValueError: + trailing_comma_safe = False + + current_line = Line( + mode=line.mode, depth=line.depth, inside_brackets=line.inside_brackets + ) + lowest_depth = sys.maxsize + + def append_to_line(leaf: Leaf) -> Iterator[Line]: + """Append `leaf` to current line or to new line if appending impossible.""" + nonlocal current_line + try: + current_line.append_safe(leaf, preformatted=True) + except ValueError: + yield current_line + + current_line = Line( + mode=line.mode, depth=line.depth, inside_brackets=line.inside_brackets + ) + current_line.append(leaf) + + for leaf in line.leaves: + yield from append_to_line(leaf) + + for comment_after in line.comments_after(leaf): + yield from append_to_line(comment_after) + + lowest_depth = min(lowest_depth, leaf.bracket_depth) + if leaf.bracket_depth == lowest_depth: + if is_vararg(leaf, within={syms.typedargslist}): + trailing_comma_safe = ( + trailing_comma_safe and Feature.TRAILING_COMMA_IN_DEF in features + ) + elif is_vararg(leaf, within={syms.arglist, syms.argument}): + trailing_comma_safe = ( + trailing_comma_safe and Feature.TRAILING_COMMA_IN_CALL in features + ) + + leaf_priority = bt.delimiters.get(id(leaf)) + if leaf_priority == COMMA_PRIORITY: + yield current_line + + current_line = Line( + mode=line.mode, depth=line.depth, inside_brackets=line.inside_brackets + ) + if current_line: + if ( + trailing_comma_safe + and current_line.leaves[-1].type != token.COMMA + and current_line.leaves[-1].type != STANDALONE_COMMENT + ): + new_comma = Leaf(token.COMMA, ",") + current_line.append(new_comma) + yield current_line + + @dont_increase_indentation def delimiter_split(line: Line, features: Collection[Feature] = ()) -> Iterator[Line]: """Split according to delimiters of the highest priority. diff --git a/tests/data/simple_cases/type_hint_after_comment.py b/tests/data/simple_cases/type_hint_after_comment.py new file mode 100644 index 00000000000..85f305d94a9 --- /dev/null +++ b/tests/data/simple_cases/type_hint_after_comment.py @@ -0,0 +1,5 @@ +def get_requires_for_build_sdist( + # pylint: disable-next=unused-argument + config_settings: dict[str, str | list[str]] | None = None, +) -> list[str]: + return ["pathspec", "pyproject_metadata"] From 42f359581a522dd0ead5bf274cced746cf6c364a Mon Sep 17 00:00:00 2001 From: Charlie Date: Fri, 28 Oct 2022 09:53:17 +0100 Subject: [PATCH 02/19] Refactor comma_split() and delimiter_split() --- src/black/linegen.py | 89 +++++++++++++++----------------------------- 1 file changed, 31 insertions(+), 58 deletions(-) diff --git a/src/black/linegen.py b/src/black/linegen.py index a6dca8a482a..213dfcf9e04 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -795,7 +795,8 @@ def split_wrapper(line: Line, features: Collection[Feature] = ()) -> Iterator[Li def comma_split(line: Line, features: Collection[Feature] = ()) -> Iterator[Line]: """Split on commas - It works the same as delimiter_split(), but it only splits on commas. + If the appropriate Features are given, the split will add trailing commas + also in function signatures and calls that contain `*` and `**`. """ try: last_leaf = line.leaves[-1] @@ -804,64 +805,13 @@ def comma_split(line: Line, features: Collection[Feature] = ()) -> Iterator[Line bt = line.bracket_tracker try: - max_delimiter_priority = bt.max_delimiter_priority( - exclude={id(last_leaf)} - ) - trailing_comma_safe = max_delimiter_priority == COMMA_PRIORITY + max_delimiter_priority = bt.max_delimiter_priority(exclude={id(last_leaf)}) except ValueError: - trailing_comma_safe = False - - current_line = Line( - mode=line.mode, depth=line.depth, inside_brackets=line.inside_brackets - ) - lowest_depth = sys.maxsize - - def append_to_line(leaf: Leaf) -> Iterator[Line]: - """Append `leaf` to current line or to new line if appending impossible.""" - nonlocal current_line - try: - current_line.append_safe(leaf, preformatted=True) - except ValueError: - yield current_line - - current_line = Line( - mode=line.mode, depth=line.depth, inside_brackets=line.inside_brackets - ) - current_line.append(leaf) - - for leaf in line.leaves: - yield from append_to_line(leaf) - - for comment_after in line.comments_after(leaf): - yield from append_to_line(comment_after) - - lowest_depth = min(lowest_depth, leaf.bracket_depth) - if leaf.bracket_depth == lowest_depth: - if is_vararg(leaf, within={syms.typedargslist}): - trailing_comma_safe = ( - trailing_comma_safe and Feature.TRAILING_COMMA_IN_DEF in features - ) - elif is_vararg(leaf, within={syms.arglist, syms.argument}): - trailing_comma_safe = ( - trailing_comma_safe and Feature.TRAILING_COMMA_IN_CALL in features - ) + raise CannotSplit("No delimiters found") from None - leaf_priority = bt.delimiters.get(id(leaf)) - if leaf_priority == COMMA_PRIORITY: - yield current_line + add_trailing_commas = max_delimiter_priority == COMMA_PRIORITY - current_line = Line( - mode=line.mode, depth=line.depth, inside_brackets=line.inside_brackets - ) - if current_line: - if ( - trailing_comma_safe - and current_line.leaves[-1].type != token.COMMA - and current_line.leaves[-1].type != STANDALONE_COMMENT - ): - new_comma = Leaf(token.COMMA, ",") - current_line.append(new_comma) - yield current_line + yield from _delimiter_split(line, COMMA_PRIORITY, add_trailing_commas, features) @dont_increase_indentation @@ -878,10 +828,33 @@ def delimiter_split(line: Line, features: Collection[Feature] = ()) -> Iterator[ bt = line.bracket_tracker try: - delimiter_priority = bt.max_delimiter_priority(exclude={id(last_leaf)}) + max_delimiter_priority = bt.max_delimiter_priority(exclude={id(last_leaf)}) except ValueError: raise CannotSplit("No delimiters found") from None + add_trailing_commas = max_delimiter_priority == COMMA_PRIORITY + + yield from _delimiter_split( + line, + max_delimiter_priority, + add_trailing_commas, + features + ) + + +def _delimiter_split( + line: Line, + delimiter_priority: int, + add_trailing_commas: bool, + features: Collection[Feature] = (), +) -> Iterator[Line]: + """Split according to delimiters of the highest priority, or a custom priority. + + If the appropriate Features are given, the split will add trailing commas + also in function signatures and calls that contain `*` and `**`. + """ + bt = line.bracket_tracker + if delimiter_priority == DOT_PRIORITY: if bt.delimiter_count_with_priority(delimiter_priority) == 1: raise CannotSplit("Splitting a single attribute from its owner looks wrong") @@ -932,7 +905,7 @@ def append_to_line(leaf: Leaf) -> Iterator[Line]: if current_line: if ( trailing_comma_safe - and delimiter_priority == COMMA_PRIORITY + and add_trailing_commas and current_line.leaves[-1].type != token.COMMA and current_line.leaves[-1].type != STANDALONE_COMMENT ): From c2eb395e41cf5a7944230a7e0769e3da1ae933cf Mon Sep 17 00:00:00 2001 From: Charlie Date: Fri, 28 Oct 2022 10:31:36 +0100 Subject: [PATCH 03/19] Add to CHANGES.md --- CHANGES.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index ba9f4c06f28..31f5db9bbe4 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,6 +10,8 @@ +- Stop type hints after comments from wrapping unnecessarily. (#3362) + ### Preview style From 2ec9eda2d3f7d1e9e23d62170a039ed659b8f9ad Mon Sep 17 00:00:00 2001 From: Charlie Date: Fri, 28 Oct 2022 13:06:48 +0100 Subject: [PATCH 04/19] Add comma_split() to developer reference --- docs/contributing/reference/reference_functions.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/contributing/reference/reference_functions.rst b/docs/contributing/reference/reference_functions.rst index 3bda5de1774..16aba078a56 100644 --- a/docs/contributing/reference/reference_functions.rst +++ b/docs/contributing/reference/reference_functions.rst @@ -83,6 +83,8 @@ Split functions .. autofunction:: black.linegen.delimiter_split +.. autofunction:: black.linegen.comma_split + .. autofunction:: black.linegen.left_hand_split .. autofunction:: black.linegen.right_hand_split From 8ae3b7c05f3b1ff7ea8fe3058c58b86979ea39d1 Mon Sep 17 00:00:00 2001 From: Charlie Date: Fri, 28 Oct 2022 14:23:32 +0100 Subject: [PATCH 05/19] Reformat linegen.py --- src/black/linegen.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/black/linegen.py b/src/black/linegen.py index 213dfcf9e04..b9d2a7f81c1 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -835,18 +835,15 @@ def delimiter_split(line: Line, features: Collection[Feature] = ()) -> Iterator[ add_trailing_commas = max_delimiter_priority == COMMA_PRIORITY yield from _delimiter_split( - line, - max_delimiter_priority, - add_trailing_commas, - features + line, max_delimiter_priority, add_trailing_commas, features ) def _delimiter_split( - line: Line, - delimiter_priority: int, - add_trailing_commas: bool, - features: Collection[Feature] = (), + line: Line, + delimiter_priority: int, + add_trailing_commas: bool, + features: Collection[Feature] = (), ) -> Iterator[Line]: """Split according to delimiters of the highest priority, or a custom priority. From 54b511a8204aa38ffd6d3a28ba846bc72013eb2e Mon Sep 17 00:00:00 2001 From: Charlie Date: Sun, 30 Oct 2022 15:34:57 +0000 Subject: [PATCH 06/19] Handle other cases of comments in brackets This commit makes Black handle cases where there is a comment in the middle of a statement in brackets. It also removes uses of comma_split() --- src/black/linegen.py | 16 ++++++++++------ ....py => wrapping_with_comments_in_brackets.py} | 14 ++++++++++++++ 2 files changed, 24 insertions(+), 6 deletions(-) rename tests/data/simple_cases/{type_hint_after_comment.py => wrapping_with_comments_in_brackets.py} (67%) diff --git a/src/black/linegen.py b/src/black/linegen.py index b9d2a7f81c1..97286bcb5af 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -538,12 +538,16 @@ def _rhs( ] else: if line.inside_brackets: - transformers = [ - comma_split, - delimiter_split, - standalone_comment_split, - rhs, - ] + if ( + not line.contains_standalone_comments() + or len(line.comments) > 0 + or any( + leaf.type == STANDALONE_COMMENT for leaf in line.leaves[1:-1] + ) + ): + transformers = [delimiter_split, standalone_comment_split, rhs] + else: + transformers = [standalone_comment_split, delimiter_split, rhs] else: transformers = [rhs] # It's always safe to attempt hugging of power operations and pretty much every line diff --git a/tests/data/simple_cases/type_hint_after_comment.py b/tests/data/simple_cases/wrapping_with_comments_in_brackets.py similarity index 67% rename from tests/data/simple_cases/type_hint_after_comment.py rename to tests/data/simple_cases/wrapping_with_comments_in_brackets.py index 85f305d94a9..8531296e646 100644 --- a/tests/data/simple_cases/type_hint_after_comment.py +++ b/tests/data/simple_cases/wrapping_with_comments_in_brackets.py @@ -3,3 +3,17 @@ def get_requires_for_build_sdist( config_settings: dict[str, str | list[str]] | None = None, ) -> list[str]: return ["pathspec", "pyproject_metadata"] + + +( + a + and b + # comment + and c + and d +) + +( + # comment + a and b and c and d +) \ No newline at end of file From a0c658d49d4dac89e9ccb86f511c70d740d76cef Mon Sep 17 00:00:00 2001 From: Charlie Date: Sun, 30 Oct 2022 16:41:16 +0000 Subject: [PATCH 07/19] Revert "Add comma_split() to developer reference" This reverts commit 2ec9eda2d3f7d1e9e23d62170a039ed659b8f9ad. --- docs/contributing/reference/reference_functions.rst | 2 -- 1 file changed, 2 deletions(-) diff --git a/docs/contributing/reference/reference_functions.rst b/docs/contributing/reference/reference_functions.rst index 16aba078a56..3bda5de1774 100644 --- a/docs/contributing/reference/reference_functions.rst +++ b/docs/contributing/reference/reference_functions.rst @@ -83,8 +83,6 @@ Split functions .. autofunction:: black.linegen.delimiter_split -.. autofunction:: black.linegen.comma_split - .. autofunction:: black.linegen.left_hand_split .. autofunction:: black.linegen.right_hand_split From 57315e3628fdd580c33ccae80f92a6de95894fbd Mon Sep 17 00:00:00 2001 From: Charlie Date: Sun, 30 Oct 2022 16:41:22 +0000 Subject: [PATCH 08/19] Revert "Add to CHANGES.md" This reverts commit c2eb395e41cf5a7944230a7e0769e3da1ae933cf. --- CHANGES.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 31f5db9bbe4..ba9f4c06f28 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,8 +10,6 @@ -- Stop type hints after comments from wrapping unnecessarily. (#3362) - ### Preview style From 37421815f4a89bd30cec177eaef44630c039a6f1 Mon Sep 17 00:00:00 2001 From: Charlie Date: Sun, 30 Oct 2022 16:43:16 +0000 Subject: [PATCH 09/19] Revert "Refactor comma_split() and delimiter_split()" This reverts commit 42f359581a522dd0ead5bf274cced746cf6c364a. --- src/black/linegen.py | 86 +++++++++++++++++++++++++++++--------------- 1 file changed, 58 insertions(+), 28 deletions(-) diff --git a/src/black/linegen.py b/src/black/linegen.py index 97286bcb5af..987e2361b7c 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -799,8 +799,7 @@ def split_wrapper(line: Line, features: Collection[Feature] = ()) -> Iterator[Li def comma_split(line: Line, features: Collection[Feature] = ()) -> Iterator[Line]: """Split on commas - If the appropriate Features are given, the split will add trailing commas - also in function signatures and calls that contain `*` and `**`. + It works the same as delimiter_split(), but it only splits on commas. """ try: last_leaf = line.leaves[-1] @@ -809,13 +808,64 @@ def comma_split(line: Line, features: Collection[Feature] = ()) -> Iterator[Line bt = line.bracket_tracker try: - max_delimiter_priority = bt.max_delimiter_priority(exclude={id(last_leaf)}) + max_delimiter_priority = bt.max_delimiter_priority( + exclude={id(last_leaf)} + ) + trailing_comma_safe = max_delimiter_priority == COMMA_PRIORITY except ValueError: - raise CannotSplit("No delimiters found") from None + trailing_comma_safe = False + + current_line = Line( + mode=line.mode, depth=line.depth, inside_brackets=line.inside_brackets + ) + lowest_depth = sys.maxsize + + def append_to_line(leaf: Leaf) -> Iterator[Line]: + """Append `leaf` to current line or to new line if appending impossible.""" + nonlocal current_line + try: + current_line.append_safe(leaf, preformatted=True) + except ValueError: + yield current_line - add_trailing_commas = max_delimiter_priority == COMMA_PRIORITY + current_line = Line( + mode=line.mode, depth=line.depth, inside_brackets=line.inside_brackets + ) + current_line.append(leaf) - yield from _delimiter_split(line, COMMA_PRIORITY, add_trailing_commas, features) + for leaf in line.leaves: + yield from append_to_line(leaf) + + for comment_after in line.comments_after(leaf): + yield from append_to_line(comment_after) + + lowest_depth = min(lowest_depth, leaf.bracket_depth) + if leaf.bracket_depth == lowest_depth: + if is_vararg(leaf, within={syms.typedargslist}): + trailing_comma_safe = ( + trailing_comma_safe and Feature.TRAILING_COMMA_IN_DEF in features + ) + elif is_vararg(leaf, within={syms.arglist, syms.argument}): + trailing_comma_safe = ( + trailing_comma_safe and Feature.TRAILING_COMMA_IN_CALL in features + ) + + leaf_priority = bt.delimiters.get(id(leaf)) + if leaf_priority == COMMA_PRIORITY: + yield current_line + + current_line = Line( + mode=line.mode, depth=line.depth, inside_brackets=line.inside_brackets + ) + if current_line: + if ( + trailing_comma_safe + and current_line.leaves[-1].type != token.COMMA + and current_line.leaves[-1].type != STANDALONE_COMMENT + ): + new_comma = Leaf(token.COMMA, ",") + current_line.append(new_comma) + yield current_line @dont_increase_indentation @@ -832,30 +882,10 @@ def delimiter_split(line: Line, features: Collection[Feature] = ()) -> Iterator[ bt = line.bracket_tracker try: - max_delimiter_priority = bt.max_delimiter_priority(exclude={id(last_leaf)}) + delimiter_priority = bt.max_delimiter_priority(exclude={id(last_leaf)}) except ValueError: raise CannotSplit("No delimiters found") from None - add_trailing_commas = max_delimiter_priority == COMMA_PRIORITY - - yield from _delimiter_split( - line, max_delimiter_priority, add_trailing_commas, features - ) - - -def _delimiter_split( - line: Line, - delimiter_priority: int, - add_trailing_commas: bool, - features: Collection[Feature] = (), -) -> Iterator[Line]: - """Split according to delimiters of the highest priority, or a custom priority. - - If the appropriate Features are given, the split will add trailing commas - also in function signatures and calls that contain `*` and `**`. - """ - bt = line.bracket_tracker - if delimiter_priority == DOT_PRIORITY: if bt.delimiter_count_with_priority(delimiter_priority) == 1: raise CannotSplit("Splitting a single attribute from its owner looks wrong") @@ -906,7 +936,7 @@ def append_to_line(leaf: Leaf) -> Iterator[Line]: if current_line: if ( trailing_comma_safe - and add_trailing_commas + and delimiter_priority == COMMA_PRIORITY and current_line.leaves[-1].type != token.COMMA and current_line.leaves[-1].type != STANDALONE_COMMENT ): From 63dc4424358e189ab1f0abd03c30c9f47c38142a Mon Sep 17 00:00:00 2001 From: Charlie Date: Sun, 30 Oct 2022 16:46:11 +0000 Subject: [PATCH 10/19] Revert "Add comma split before delimiter split" This reverts commit 08c91388992890976d935f980aa59606ba4bb0f1. --- src/black/linegen.py | 73 -------------------------------------------- 1 file changed, 73 deletions(-) diff --git a/src/black/linegen.py b/src/black/linegen.py index 987e2361b7c..2f2d84e68d0 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -795,79 +795,6 @@ def split_wrapper(line: Line, features: Collection[Feature] = ()) -> Iterator[Li return split_wrapper -@dont_increase_indentation -def comma_split(line: Line, features: Collection[Feature] = ()) -> Iterator[Line]: - """Split on commas - - It works the same as delimiter_split(), but it only splits on commas. - """ - try: - last_leaf = line.leaves[-1] - except IndexError: - raise CannotSplit("Line empty") from None - - bt = line.bracket_tracker - try: - max_delimiter_priority = bt.max_delimiter_priority( - exclude={id(last_leaf)} - ) - trailing_comma_safe = max_delimiter_priority == COMMA_PRIORITY - except ValueError: - trailing_comma_safe = False - - current_line = Line( - mode=line.mode, depth=line.depth, inside_brackets=line.inside_brackets - ) - lowest_depth = sys.maxsize - - def append_to_line(leaf: Leaf) -> Iterator[Line]: - """Append `leaf` to current line or to new line if appending impossible.""" - nonlocal current_line - try: - current_line.append_safe(leaf, preformatted=True) - except ValueError: - yield current_line - - current_line = Line( - mode=line.mode, depth=line.depth, inside_brackets=line.inside_brackets - ) - current_line.append(leaf) - - for leaf in line.leaves: - yield from append_to_line(leaf) - - for comment_after in line.comments_after(leaf): - yield from append_to_line(comment_after) - - lowest_depth = min(lowest_depth, leaf.bracket_depth) - if leaf.bracket_depth == lowest_depth: - if is_vararg(leaf, within={syms.typedargslist}): - trailing_comma_safe = ( - trailing_comma_safe and Feature.TRAILING_COMMA_IN_DEF in features - ) - elif is_vararg(leaf, within={syms.arglist, syms.argument}): - trailing_comma_safe = ( - trailing_comma_safe and Feature.TRAILING_COMMA_IN_CALL in features - ) - - leaf_priority = bt.delimiters.get(id(leaf)) - if leaf_priority == COMMA_PRIORITY: - yield current_line - - current_line = Line( - mode=line.mode, depth=line.depth, inside_brackets=line.inside_brackets - ) - if current_line: - if ( - trailing_comma_safe - and current_line.leaves[-1].type != token.COMMA - and current_line.leaves[-1].type != STANDALONE_COMMENT - ): - new_comma = Leaf(token.COMMA, ",") - current_line.append(new_comma) - yield current_line - - @dont_increase_indentation def delimiter_split(line: Line, features: Collection[Feature] = ()) -> Iterator[Line]: """Split according to delimiters of the highest priority. From 2dd9a7e85ef795d762ae3b8ca6919c2ac9701dfe Mon Sep 17 00:00:00 2001 From: Charlie Date: Tue, 1 Nov 2022 13:40:12 +0000 Subject: [PATCH 11/19] Move changes to preview --- src/black/linegen.py | 22 ++++++++++--------- src/black/mode.py | 1 + .../wrapping_with_comments_in_brackets.py | 0 3 files changed, 13 insertions(+), 10 deletions(-) rename tests/data/{simple_cases => preview}/wrapping_with_comments_in_brackets.py (100%) diff --git a/src/black/linegen.py b/src/black/linegen.py index 2f2d84e68d0..e56ba57c8a3 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -528,6 +528,17 @@ def _rhs( string_paren_wrap, rhs, ] + + if ( + Preview.stop_some_unnecessary_wrapping_inside_brackets in mode + and line.contains_standalone_comments() + and len(line.comments) == 0 + and all( + leaf.type != STANDALONE_COMMENT for leaf in line.leaves[1:-1] + ) + ): + transformers.remove(standalone_comment_split) + transformers.insert(0, standalone_comment_split) else: transformers = [ string_merge, @@ -538,16 +549,7 @@ def _rhs( ] else: if line.inside_brackets: - if ( - not line.contains_standalone_comments() - or len(line.comments) > 0 - or any( - leaf.type == STANDALONE_COMMENT for leaf in line.leaves[1:-1] - ) - ): - transformers = [delimiter_split, standalone_comment_split, rhs] - else: - transformers = [standalone_comment_split, delimiter_split, rhs] + transformers = [delimiter_split, standalone_comment_split, rhs] else: transformers = [rhs] # It's always safe to attempt hugging of power operations and pretty much every line diff --git a/src/black/mode.py b/src/black/mode.py index e3c36450ed1..99c77b8e18f 100644 --- a/src/black/mode.py +++ b/src/black/mode.py @@ -157,6 +157,7 @@ class Preview(Enum): remove_redundant_parens = auto() string_processing = auto() skip_magic_trailing_comma_in_subscript = auto() + stop_some_unnecessary_wrapping_inside_brackets = auto() class Deprecated(UserWarning): diff --git a/tests/data/simple_cases/wrapping_with_comments_in_brackets.py b/tests/data/preview/wrapping_with_comments_in_brackets.py similarity index 100% rename from tests/data/simple_cases/wrapping_with_comments_in_brackets.py rename to tests/data/preview/wrapping_with_comments_in_brackets.py From ed21fcf5bf625fff2dc2ed9e669e67de7f915c8d Mon Sep 17 00:00:00 2001 From: Charlie Date: Tue, 1 Nov 2022 13:40:39 +0000 Subject: [PATCH 12/19] Add to CHANGES.md --- CHANGES.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index ba9f4c06f28..baf44a79b3a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -14,6 +14,8 @@ +- Stop wrapping unnecessarily when there are comments inside brackets (#3362). + ### Configuration From 82d05f0cf0e8bd527cd91783e0701553abb9d82a Mon Sep 17 00:00:00 2001 From: Charlie Bareham Date: Sat, 1 Jul 2023 13:43:46 +0100 Subject: [PATCH 13/19] Edit test to make sure a delimiter split is performed This test makes sure a trailing comma is added when there is a comment in the brackets. However, a delimiter split is no longer performed in these cases, so there is no need to add a trailing comma. This commit makes the lines longer so a delimiter split is performed, and the behaviour with the trailing comma can be tested. --- tests/data/preview/trailing_comma.py | 48 ++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/tests/data/preview/trailing_comma.py b/tests/data/preview/trailing_comma.py index 5b09c664606..0155de6919f 100644 --- a/tests/data/preview/trailing_comma.py +++ b/tests/data/preview/trailing_comma.py @@ -7,19 +7,19 @@ f = [ arg1, arg2, - arg3, arg4 + arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10, arg11, arg12, arg13, arg14, arg15, arg16, arg17, arg18 # comment ] g = ( arg1, arg2, - arg3, arg4 + arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10, arg11, arg12, arg13, arg14, arg15, arg16, arg17, arg18 # comment ) h = { arg1, arg2, - arg3, arg4 + arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10, arg11, arg12, arg13, arg14, arg15, arg16, arg17, arg18 # comment } @@ -37,6 +37,20 @@ arg2, arg3, arg4, + arg5, + arg6, + arg7, + arg8, + arg9, + arg10, + arg11, + arg12, + arg13, + arg14, + arg15, + arg16, + arg17, + arg18, # comment ] g = ( @@ -44,6 +58,20 @@ arg2, arg3, arg4, + arg5, + arg6, + arg7, + arg8, + arg9, + arg10, + arg11, + arg12, + arg13, + arg14, + arg15, + arg16, + arg17, + arg18, # comment ) h = { @@ -51,5 +79,19 @@ arg2, arg3, arg4, + arg5, + arg6, + arg7, + arg8, + arg9, + arg10, + arg11, + arg12, + arg13, + arg14, + arg15, + arg16, + arg17, + arg18, # comment } From 618eb16a6b19677c80eb0dd31a682ae97c2e357c Mon Sep 17 00:00:00 2001 From: Charlie Bareham Date: Sat, 1 Jul 2023 13:56:14 +0100 Subject: [PATCH 14/19] Add comment --- src/black/linegen.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/black/linegen.py b/src/black/linegen.py index de6e217c103..72ec423f78a 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -591,6 +591,12 @@ def _rhs( rhs, ] + # If there are comments at the beginning or end of the line, call + # standalone_comment_split before delimiter_split. Else, call + # delimiter_split first. This means that when there is a comment in the + # middle of an expression or list, it will perform a delimiter split. + # But when the comment is at the top or bottom, it won't perform a + # delimiter split (if it doesn't have to). if ( Preview.stop_some_unnecessary_wrapping_inside_brackets in mode and line.contains_standalone_comments() From 5658c395bee4f5dd38789785e4b98c6415387858 Mon Sep 17 00:00:00 2001 From: Charlie Bareham Date: Sun, 2 Jul 2023 18:04:43 +0100 Subject: [PATCH 15/19] Handle magic trailing commas with comments --- src/black/linegen.py | 2 ++ src/black/lines.py | 13 ++++++++++++- tests/data/preview/trailing_comma_with_comment.py | 6 ++++++ 3 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 tests/data/preview/trailing_comma_with_comment.py diff --git a/src/black/linegen.py b/src/black/linegen.py index 72ec423f78a..89a7d10d9e8 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -604,6 +604,7 @@ def _rhs( and all( leaf.type != STANDALONE_COMMENT for leaf in line.leaves[1:-1] ) + and line.magic_trailing_comma_token is None ): transformers.remove(standalone_comment_split) transformers.insert(0, standalone_comment_split) @@ -754,6 +755,7 @@ def _first_right_hand_split( body = bracket_split_build_line( body_leaves, line, opening_bracket, component=_BracketSplitComponent.body ) + body.magic_trailing_comma_token = line.magic_trailing_comma_token tail = bracket_split_build_line( tail_leaves, line, opening_bracket, component=_BracketSplitComponent.tail ) diff --git a/src/black/lines.py b/src/black/lines.py index ea8fe520756..5ae830cab8f 100644 --- a/src/black/lines.py +++ b/src/black/lines.py @@ -58,6 +58,7 @@ class Line: inside_brackets: bool = False should_split_rhs: bool = False magic_trailing_comma: Optional[Leaf] = None + magic_trailing_comma_token: Optional[Leaf] = None def append( self, leaf: Leaf, preformatted: bool = False, track_bracket: bool = False @@ -88,6 +89,7 @@ def append( if self.mode.magic_trailing_comma: if self.has_magic_trailing_comma(leaf): self.magic_trailing_comma = leaf + self.magic_trailing_comma_token = self.get_last_non_comment_leaf() elif self.has_magic_trailing_comma(leaf, ensure_removable=True): self.remove_trailing_comma() if not self.append_comment(leaf): @@ -297,6 +299,13 @@ def contains_unsplittable_type_ignore(self) -> bool: def contains_multiline_strings(self) -> bool: return any(is_multiline_string(leaf) for leaf in self.leaves) + def get_last_non_comment_leaf(self) -> Optional[Leaf]: + for leaf in reversed(self.leaves): + if leaf.type != STANDALONE_COMMENT: + return leaf + + return None + def has_magic_trailing_comma( self, closing: Leaf, ensure_removable: bool = False ) -> bool: @@ -308,10 +317,12 @@ def has_magic_trailing_comma( - it's not from square bracket indexing (specifically, single-element square bracket indexing) """ + last_non_comment_leaf = self.get_last_non_comment_leaf() if not ( closing.type in CLOSING_BRACKETS and self.leaves - and self.leaves[-1].type == token.COMMA + and last_non_comment_leaf is not None + and last_non_comment_leaf.type == token.COMMA ): return False diff --git a/tests/data/preview/trailing_comma_with_comment.py b/tests/data/preview/trailing_comma_with_comment.py new file mode 100644 index 00000000000..cac789b6396 --- /dev/null +++ b/tests/data/preview/trailing_comma_with_comment.py @@ -0,0 +1,6 @@ +li = [ + 1, + 2, + 3, + # comment +] \ No newline at end of file From de4a5179a85b71f735bb6b97d470b803dae79347 Mon Sep 17 00:00:00 2001 From: Charlie Bareham Date: Wed, 5 Jul 2023 19:19:58 +0100 Subject: [PATCH 16/19] Remove magic trailing comma correctly --- src/black/lines.py | 8 +++++++- tests/data/miscellaneous/skip_magic_trailing_comma.py | 5 +++++ tests/test_format.py | 11 +++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 tests/data/miscellaneous/skip_magic_trailing_comma.py diff --git a/src/black/lines.py b/src/black/lines.py index 5ae830cab8f..79a55088d70 100644 --- a/src/black/lines.py +++ b/src/black/lines.py @@ -413,7 +413,13 @@ def comments_after(self, leaf: Leaf) -> List[Leaf]: def remove_trailing_comma(self) -> None: """Remove the trailing comma and moves the comments attached to it.""" - trailing_comma = self.leaves.pop() + # There might be comments after the magic trailing comma, so we must search + # backwards to find it. + for i in range(len(self.leaves) - 1, -1, -1): + if self.leaves[i].type == token.COMMA: + trailing_comma = self.leaves.pop(i) + break + trailing_comma_comments = self.comments.pop(id(trailing_comma), []) self.comments.setdefault(id(self.leaves[-1]), []).extend( trailing_comma_comments diff --git a/tests/data/miscellaneous/skip_magic_trailing_comma.py b/tests/data/miscellaneous/skip_magic_trailing_comma.py new file mode 100644 index 00000000000..9b50adb70da --- /dev/null +++ b/tests/data/miscellaneous/skip_magic_trailing_comma.py @@ -0,0 +1,5 @@ +def func( + x, + # this comment shouldn't be deleted +): + pass diff --git a/tests/test_format.py b/tests/test_format.py index 8e0ada99cba..62439e07483 100644 --- a/tests/test_format.py +++ b/tests/test_format.py @@ -210,3 +210,14 @@ def test_type_comment_syntax_error() -> None: source, expected = read_data("type_comments", "type_comment_syntax_error") assert_format(source, expected) black.assert_equivalent(source, expected) + + +def test_skip_magic_trailing_comma() -> None: + """ + Test a case where the --skip-magic-trailing-comma option used + """ + source, expected = read_data( + "miscellaneous", "skip_magic_trailing_comma" + ) + mode = replace(DEFAULT_MODE, magic_trailing_comma=False, preview=True) + assert_format(source, expected, mode) From 2ccc70f8f9fc2d59612061479348a8130db206ec Mon Sep 17 00:00:00 2001 From: Charlie Bareham Date: Wed, 5 Jul 2023 19:28:26 +0100 Subject: [PATCH 17/19] Rename magic trailing comma attributes Magic_trailing_comma now points to the actual comma token, and bracket_after_magic_trailing_comma now points to the following close bracket. --- src/black/linegen.py | 10 +++++----- src/black/lines.py | 7 ++++--- src/black/trans.py | 1 + 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/black/linegen.py b/src/black/linegen.py index 89a7d10d9e8..b1e47794a65 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -532,7 +532,7 @@ def transform_line( if ( not line.contains_uncollapsable_type_comments() and not line.should_split_rhs - and not line.magic_trailing_comma + and not line.bracket_after_magic_trailing_comma and ( is_line_short_enough(line, mode=mode, line_str=line_str) or line.contains_unsplittable_type_ignore() @@ -604,7 +604,7 @@ def _rhs( and all( leaf.type != STANDALONE_COMMENT for leaf in line.leaves[1:-1] ) - and line.magic_trailing_comma_token is None + and line.magic_trailing_comma is None ): transformers.remove(standalone_comment_split) transformers.insert(0, standalone_comment_split) @@ -755,7 +755,7 @@ def _first_right_hand_split( body = bracket_split_build_line( body_leaves, line, opening_bracket, component=_BracketSplitComponent.body ) - body.magic_trailing_comma_token = line.magic_trailing_comma_token + body.magic_trailing_comma = line.magic_trailing_comma tail = bracket_split_build_line( tail_leaves, line, opening_bracket, component=_BracketSplitComponent.tail ) @@ -804,7 +804,7 @@ def _maybe_split_omitting_optional_parens( ) # the left side of assignment won't explode further because of magic # trailing comma - and rhs.head.magic_trailing_comma is None + and rhs.head.bracket_after_magic_trailing_comma is None # the split by omitting optional parens isn't preferred by some other # reason and not _prefer_split_rhs_oop(rhs_oop, mode) @@ -1481,7 +1481,7 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf """ omit: Set[LeafID] = set() - if not line.magic_trailing_comma: + if not line.bracket_after_magic_trailing_comma: yield omit length = 4 * line.depth diff --git a/src/black/lines.py b/src/black/lines.py index 79a55088d70..d61070ae5c1 100644 --- a/src/black/lines.py +++ b/src/black/lines.py @@ -57,8 +57,8 @@ class Line: bracket_tracker: BracketTracker = field(default_factory=BracketTracker) inside_brackets: bool = False should_split_rhs: bool = False + bracket_after_magic_trailing_comma: Optional[Leaf] = None magic_trailing_comma: Optional[Leaf] = None - magic_trailing_comma_token: Optional[Leaf] = None def append( self, leaf: Leaf, preformatted: bool = False, track_bracket: bool = False @@ -88,8 +88,8 @@ def append( self.bracket_tracker.mark(leaf) if self.mode.magic_trailing_comma: if self.has_magic_trailing_comma(leaf): - self.magic_trailing_comma = leaf - self.magic_trailing_comma_token = self.get_last_non_comment_leaf() + self.bracket_after_magic_trailing_comma = leaf + self.magic_trailing_comma = self.get_last_non_comment_leaf() elif self.has_magic_trailing_comma(leaf, ensure_removable=True): self.remove_trailing_comma() if not self.append_comment(leaf): @@ -471,6 +471,7 @@ def clone(self) -> "Line": inside_brackets=self.inside_brackets, should_split_rhs=self.should_split_rhs, magic_trailing_comma=self.magic_trailing_comma, + bracket_after_magic_trailing_comma=self.bracket_after_magic_trailing_comma, ) def __str__(self) -> str: diff --git a/src/black/trans.py b/src/black/trans.py index 4d40cb4bdf6..93a30bc9133 100644 --- a/src/black/trans.py +++ b/src/black/trans.py @@ -2134,6 +2134,7 @@ def do_transform( inside_brackets=True, should_split_rhs=line.should_split_rhs, magic_trailing_comma=line.magic_trailing_comma, + bracket_after_magic_trailing_comma=line.bracket_after_magic_trailing_comma, ) string_leaf = Leaf(token.STRING, string_value) insert_str_child(string_leaf) From 2d326163ac7afafd6d8d1f567e71c48595a27822 Mon Sep 17 00:00:00 2001 From: Charlie Bareham Date: Fri, 7 Jul 2023 21:06:55 +0100 Subject: [PATCH 18/19] Gate changes to preview --- src/black/lines.py | 27 ++++++++++++------- .../trailing_comma_and_comment.pyi | 5 ++++ tests/test_format.py | 14 ++++++++++ 3 files changed, 37 insertions(+), 9 deletions(-) create mode 100644 tests/data/miscellaneous/trailing_comma_and_comment.pyi diff --git a/src/black/lines.py b/src/black/lines.py index d61070ae5c1..978c51fd59d 100644 --- a/src/black/lines.py +++ b/src/black/lines.py @@ -317,12 +317,18 @@ def has_magic_trailing_comma( - it's not from square bracket indexing (specifically, single-element square bracket indexing) """ - last_non_comment_leaf = self.get_last_non_comment_leaf() + if Preview.stop_some_unnecessary_wrapping_inside_brackets in self.mode: + leaf_to_check = self.get_last_non_comment_leaf() + elif len(self.leaves) > 0: + leaf_to_check = self.leaves[-1] + else: + return False + if not ( closing.type in CLOSING_BRACKETS and self.leaves - and last_non_comment_leaf is not None - and last_non_comment_leaf.type == token.COMMA + and leaf_to_check is not None + and leaf_to_check.type == token.COMMA ): return False @@ -413,12 +419,15 @@ def comments_after(self, leaf: Leaf) -> List[Leaf]: def remove_trailing_comma(self) -> None: """Remove the trailing comma and moves the comments attached to it.""" - # There might be comments after the magic trailing comma, so we must search - # backwards to find it. - for i in range(len(self.leaves) - 1, -1, -1): - if self.leaves[i].type == token.COMMA: - trailing_comma = self.leaves.pop(i) - break + if Preview.stop_some_unnecessary_wrapping_inside_brackets in self.mode: + # There might be comments after the magic trailing comma, so we must search + # backwards to find it. + for i in range(len(self.leaves) - 1, -1, -1): + if self.leaves[i].type == token.COMMA: + trailing_comma = self.leaves.pop(i) + break + else: + trailing_comma = self.leaves.pop() trailing_comma_comments = self.comments.pop(id(trailing_comma), []) self.comments.setdefault(id(self.leaves[-1]), []).extend( diff --git a/tests/data/miscellaneous/trailing_comma_and_comment.pyi b/tests/data/miscellaneous/trailing_comma_and_comment.pyi new file mode 100644 index 00000000000..1b9eef1ae7a --- /dev/null +++ b/tests/data/miscellaneous/trailing_comma_and_comment.pyi @@ -0,0 +1,5 @@ +def func( + x, + y, + # comment +): ... diff --git a/tests/test_format.py b/tests/test_format.py index 62439e07483..4fefaf4e1f2 100644 --- a/tests/test_format.py +++ b/tests/test_format.py @@ -221,3 +221,17 @@ def test_skip_magic_trailing_comma() -> None: ) mode = replace(DEFAULT_MODE, magic_trailing_comma=False, preview=True) assert_format(source, expected, mode) + + +def test_trailing_comma_and_comment_in_pyi() -> None: + source, expected = read_data( + "miscellaneous", "trailing_comma_and_comment.pyi" + ) + mode = replace( + DEFAULT_MODE, + magic_trailing_comma=False, + preview=False, + line_length=130, + is_pyi=True, + ) + assert_format(source, expected, mode) From cbce27153b5425ffe053f89d898508543cd1838a Mon Sep 17 00:00:00 2001 From: Charlie Bareham Date: Sat, 8 Jul 2023 13:52:33 +0100 Subject: [PATCH 19/19] Reformat --- tests/test_format.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/test_format.py b/tests/test_format.py index 32ffeb9ff51..847e37ae82f 100644 --- a/tests/test_format.py +++ b/tests/test_format.py @@ -217,17 +217,13 @@ def test_skip_magic_trailing_comma() -> None: """ Test a case where the --skip-magic-trailing-comma option used """ - source, expected = read_data( - "miscellaneous", "skip_magic_trailing_comma" - ) + source, expected = read_data("miscellaneous", "skip_magic_trailing_comma") mode = replace(DEFAULT_MODE, magic_trailing_comma=False, preview=True) assert_format(source, expected, mode) def test_trailing_comma_and_comment_in_pyi() -> None: - source, expected = read_data( - "miscellaneous", "trailing_comma_and_comment.pyi" - ) + source, expected = read_data("miscellaneous", "trailing_comma_and_comment.pyi") mode = replace( DEFAULT_MODE, magic_trailing_comma=False,