From 24801a90fda701465f06c5a780ca381de80ef7ac Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Wed, 25 Oct 2023 21:19:02 +0300 Subject: [PATCH 01/27] Remove redundant check --- src/black/comments.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/black/comments.py b/src/black/comments.py index 862fc7607cc..eb02541969c 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -156,7 +156,7 @@ def convert_one_fmt_off_pair(node: Node, mode: Mode) -> bool: # We only want standalone comments. If there's no previous leaf or # the previous leaf is indentation, it's a standalone comment in # disguise. - if should_pass_fmt and comment.type != STANDALONE_COMMENT: + if comment.type != STANDALONE_COMMENT: prev = preceding_leaf(leaf) if prev: if comment.value in FMT_OFF and prev.type not in WHITESPACE: From 422433661868b23e40cf5673d35a7daedfa3100e Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Wed, 25 Oct 2023 21:25:02 +0300 Subject: [PATCH 02/27] Small refactor --- src/black/comments.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/black/comments.py b/src/black/comments.py index eb02541969c..2d0d1bb6fc3 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -147,12 +147,14 @@ def convert_one_fmt_off_pair(node: Node, mode: Mode) -> bool: for leaf in node.leaves(): previous_consumed = 0 for comment in list_comments(leaf.prefix, is_endmarker=False): - should_pass_fmt = comment.value in FMT_OFF or _contains_fmt_skip_comment( - comment.value, mode - ) - if not should_pass_fmt: + found_fmt_off = comment.value in FMT_OFF + found_fmt_skip = _contains_fmt_skip_comment(comment.value, mode) + should_skip_formatting = found_fmt_off or found_fmt_skip + + if not should_skip_formatting: previous_consumed = comment.consumed continue + # We only want standalone comments. If there's no previous leaf or # the previous leaf is indentation, it's a standalone comment in # disguise. From b435b3a370e13cf9ff1cde7fffb831068ec088d2 Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Wed, 25 Oct 2023 21:26:49 +0300 Subject: [PATCH 03/27] Small refactor --- src/black/comments.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/black/comments.py b/src/black/comments.py index 2d0d1bb6fc3..a0f387ee561 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -161,12 +161,9 @@ def convert_one_fmt_off_pair(node: Node, mode: Mode) -> bool: if comment.type != STANDALONE_COMMENT: prev = preceding_leaf(leaf) if prev: - if comment.value in FMT_OFF and prev.type not in WHITESPACE: + if found_fmt_off and prev.type not in WHITESPACE: continue - if ( - _contains_fmt_skip_comment(comment.value, mode) - and prev.type in WHITESPACE - ): + if found_fmt_skip and prev.type in WHITESPACE: continue ignored_nodes = list(generate_ignored_nodes(leaf, comment, mode)) From c212649723ff2537dcdf6322f10f187f6868b8a2 Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Wed, 25 Oct 2023 21:47:25 +0300 Subject: [PATCH 04/27] Small refactor --- src/black/comments.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/black/comments.py b/src/black/comments.py index a0f387ee561..a3038b8c232 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -166,7 +166,13 @@ def convert_one_fmt_off_pair(node: Node, mode: Mode) -> bool: if found_fmt_skip and prev.type in WHITESPACE: continue - ignored_nodes = list(generate_ignored_nodes(leaf, comment, mode)) + if found_fmt_off: + ignored_nodes = list(_generate_ignored_nodes_from_fmt_off(leaf)) + elif found_fmt_skip: + ignored_nodes = list( + _generate_ignored_nodes_from_fmt_skip(leaf, comment) + ) + if not ignored_nodes: continue @@ -212,17 +218,11 @@ def convert_one_fmt_off_pair(node: Node, mode: Mode) -> bool: return False -def generate_ignored_nodes( - leaf: Leaf, comment: ProtoComment, mode: Mode -) -> Iterator[LN]: +def _generate_ignored_nodes_from_fmt_off(leaf: Leaf) -> Iterator[LN]: """Starting from the container of `leaf`, generate all leaves until `# fmt: on`. - If comment is skip, returns leaf only. Stops at the end of the block. """ - if _contains_fmt_skip_comment(comment.value, mode): - yield from _generate_ignored_nodes_from_fmt_skip(leaf, comment) - return container: Optional[LN] = container_of(leaf) while container is not None and container.type != token.ENDMARKER: if is_fmt_on(container): From c574b9752a03910e555c2aa00ba671cc1442fe26 Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Wed, 25 Oct 2023 21:50:38 +0300 Subject: [PATCH 05/27] Refactor --- src/black/comments.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/black/comments.py b/src/black/comments.py index a3038b8c232..59358acb8eb 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -179,15 +179,16 @@ def convert_one_fmt_off_pair(node: Node, mode: Mode) -> bool: first = ignored_nodes[0] # Can be a container node with the `leaf`. parent = first.parent prefix = first.prefix - if comment.value in FMT_OFF: + + if found_fmt_off: first.prefix = prefix[comment.consumed :] - if _contains_fmt_skip_comment(comment.value, mode): - first.prefix = "" - standalone_comment_prefix = prefix - else: standalone_comment_prefix = ( prefix[:previous_consumed] + "\n" * comment.newlines ) + elif found_fmt_skip: + first.prefix = "" + standalone_comment_prefix = prefix + hidden_value = "".join(str(n) for n in ignored_nodes) if comment.value in FMT_OFF: hidden_value = comment.value + "\n" + hidden_value From c1c4d63366107f31c60bc8d2ca8f68a1c35b9a0e Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Wed, 25 Oct 2023 21:53:12 +0300 Subject: [PATCH 06/27] Refactor --- src/black/comments.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/black/comments.py b/src/black/comments.py index 59358acb8eb..69e6ebd07b7 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -190,21 +190,26 @@ def convert_one_fmt_off_pair(node: Node, mode: Mode) -> bool: standalone_comment_prefix = prefix hidden_value = "".join(str(n) for n in ignored_nodes) - if comment.value in FMT_OFF: + + if found_fmt_off: hidden_value = comment.value + "\n" + hidden_value - if _contains_fmt_skip_comment(comment.value, mode): + elif found_fmt_skip: hidden_value += " " + comment.value + if hidden_value.endswith("\n"): - # That happens when one of the `ignored_nodes` ended with a NEWLINE + # This happens when one of the `ignored_nodes` ended with a NEWLINE # leaf (possibly followed by a DEDENT). hidden_value = hidden_value[:-1] + first_idx: Optional[int] = None for ignored in ignored_nodes: index = ignored.remove() if first_idx is None: first_idx = index + assert parent is not None, "INTERNAL ERROR: fmt: on/off handling (1)" assert first_idx is not None, "INTERNAL ERROR: fmt: on/off handling (2)" + parent.insert_child( first_idx, Leaf( From e78b18f4814dd85e01ade72274a3a1cd556a814e Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Wed, 25 Oct 2023 21:58:05 +0300 Subject: [PATCH 07/27] Refactor + rewrite docstrings --- src/black/__init__.py | 4 ++-- src/black/comments.py | 20 ++++++++++++-------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 7cf93b89e42..b8f9f29be78 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -35,7 +35,7 @@ from _black_version import version as __version__ from black.cache import Cache -from black.comments import normalize_fmt_off +from black.comments import normalize_format_skipping from black.const import ( DEFAULT_EXCLUDES, DEFAULT_INCLUDES, @@ -1099,7 +1099,7 @@ def _format_str_once(src_contents: str, *, mode: Mode) -> str: for feature in {Feature.PARENTHESIZED_CONTEXT_MANAGERS} if supports_feature(versions, feature) } - normalize_fmt_off(src_node, mode) + normalize_format_skipping(src_node, mode) lines = LineGenerator(mode=mode, features=context_manager_features) elt = EmptyLineTracker(mode=mode) split_line_features = { diff --git a/src/black/comments.py b/src/black/comments.py index 69e6ebd07b7..6b6587a1137 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -132,17 +132,21 @@ def make_comment(content: str) -> str: return "#" + content -def normalize_fmt_off(node: Node, mode: Mode) -> None: - """Convert content between `# fmt: off`/`# fmt: on` into standalone comments.""" - try_again = True - while try_again: - try_again = convert_one_fmt_off_pair(node, mode) +def normalize_format_skipping(node: Node, mode: Mode) -> None: + """Convert content between `# fmt: off`/`# fmt: on` or on a line with `# fmt:skip` + into a STANDALONE_COMMENT leaf containing the content to skip formatting for. + """ + more_to_process = True + while more_to_process: + more_to_process = _convert_one_fmt_off_or_skip(node, mode) -def convert_one_fmt_off_pair(node: Node, mode: Mode) -> bool: - """Convert content of a single `# fmt: off`/`# fmt: on` into a standalone comment. +def _convert_one_fmt_off_or_skip(node: Node, mode: Mode) -> bool: + """Convert one `# fmt: off`/`# fmt: on` pair or single `# fmt:skip` into a + STANDALONE_COMMENT leaf. This removes the leaf range from the tree and inserts + the unformatted content as the STANDALONE_COMMENT leaf's value. - Returns True if a pair was converted. + Returns True if a format skip was processed. """ for leaf in node.leaves(): previous_consumed = 0 From 2e669606cd0a9600cc5f4520222c184a909ce8b7 Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Wed, 25 Oct 2023 21:59:51 +0300 Subject: [PATCH 08/27] Simplify --- src/black/comments.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/black/comments.py b/src/black/comments.py index 6b6587a1137..bd0a90d47cd 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -136,9 +136,8 @@ def normalize_format_skipping(node: Node, mode: Mode) -> None: """Convert content between `# fmt: off`/`# fmt: on` or on a line with `# fmt:skip` into a STANDALONE_COMMENT leaf containing the content to skip formatting for. """ - more_to_process = True - while more_to_process: - more_to_process = _convert_one_fmt_off_or_skip(node, mode) + while _convert_one_fmt_off_or_skip(node, mode): + pass def _convert_one_fmt_off_or_skip(node: Node, mode: Mode) -> bool: From 4dfdc0f94f00eacf7f247d54e59c8900edc2aa80 Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Thu, 26 Oct 2023 10:05:03 +0300 Subject: [PATCH 09/27] Move if closer to where it can happen --- src/black/comments.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/black/comments.py b/src/black/comments.py index bd0a90d47cd..041902c4d4c 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -196,14 +196,13 @@ def _convert_one_fmt_off_or_skip(node: Node, mode: Mode) -> bool: if found_fmt_off: hidden_value = comment.value + "\n" + hidden_value + if hidden_value.endswith("\n"): + # This happens when one of the `ignored_nodes` ended with a NEWLINE + # leaf (possibly followed by a DEDENT). + hidden_value = hidden_value[:-1] elif found_fmt_skip: hidden_value += " " + comment.value - if hidden_value.endswith("\n"): - # This happens when one of the `ignored_nodes` ended with a NEWLINE - # leaf (possibly followed by a DEDENT). - hidden_value = hidden_value[:-1] - first_idx: Optional[int] = None for ignored in ignored_nodes: index = ignored.remove() From fe9a810e9e928c05f05dd5ca3774ace8df0756bd Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Thu, 26 Oct 2023 10:05:12 +0300 Subject: [PATCH 10/27] Wording --- src/black/comments.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/black/comments.py b/src/black/comments.py index 041902c4d4c..7cb4b49ff5b 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -209,8 +209,8 @@ def _convert_one_fmt_off_or_skip(node: Node, mode: Mode) -> bool: if first_idx is None: first_idx = index - assert parent is not None, "INTERNAL ERROR: fmt: on/off handling (1)" - assert first_idx is not None, "INTERNAL ERROR: fmt: on/off handling (2)" + assert parent is not None, "INTERNAL ERROR: format skipping handling (1)" + assert first_idx is not None, "INTERNAL ERROR: format skipping handling (2)" parent.insert_child( first_idx, From 92bece5a9bedf59bba5fee9f496331c64376f25e Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Thu, 26 Oct 2023 10:22:45 +0300 Subject: [PATCH 11/27] Remove unnecessary check (is handled by checking empty ignored_nodes) --- src/black/comments.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/black/comments.py b/src/black/comments.py index 7cb4b49ff5b..016b52012af 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -166,8 +166,6 @@ def _convert_one_fmt_off_or_skip(node: Node, mode: Mode) -> bool: if prev: if found_fmt_off and prev.type not in WHITESPACE: continue - if found_fmt_skip and prev.type in WHITESPACE: - continue if found_fmt_off: ignored_nodes = list(_generate_ignored_nodes_from_fmt_off(leaf)) From 20a18aa3e9b2a228a5066692bc6f469353f1d57a Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Thu, 26 Oct 2023 10:27:20 +0300 Subject: [PATCH 12/27] Move logical block closer to relevant code --- src/black/comments.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/black/comments.py b/src/black/comments.py index 016b52012af..aff322d234e 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -158,16 +158,15 @@ def _convert_one_fmt_off_or_skip(node: Node, mode: Mode) -> bool: previous_consumed = comment.consumed continue - # We only want standalone comments. If there's no previous leaf or - # the previous leaf is indentation, it's a standalone comment in - # disguise. - if comment.type != STANDALONE_COMMENT: - prev = preceding_leaf(leaf) - if prev: - if found_fmt_off and prev.type not in WHITESPACE: + if found_fmt_off: + # We only want standalone comments. If there's no previous leaf or + # the previous leaf is indentation, it's a standalone comment in + # disguise. + if comment.type != STANDALONE_COMMENT: + prev = preceding_leaf(leaf) + if prev and prev.type not in WHITESPACE: continue - if found_fmt_off: ignored_nodes = list(_generate_ignored_nodes_from_fmt_off(leaf)) elif found_fmt_skip: ignored_nodes = list( From f096ae7bf3adc648171afc253b4e469b066f2084 Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Thu, 26 Oct 2023 11:15:54 +0300 Subject: [PATCH 13/27] Split fmt_off and fmt_skip logic to separate functions --- src/black/comments.py | 93 ++++++++++++++++++++++++++----------------- 1 file changed, 56 insertions(+), 37 deletions(-) diff --git a/src/black/comments.py b/src/black/comments.py index aff322d234e..e4e53bd3b00 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -1,7 +1,7 @@ import re from dataclasses import dataclass from functools import lru_cache -from typing import Final, Iterator, List, Optional, Union +from typing import Final, Iterator, List, Optional, Tuple, Union from black.mode import Mode, Preview from black.nodes import ( @@ -149,6 +149,7 @@ def _convert_one_fmt_off_or_skip(node: Node, mode: Mode) -> bool: """ for leaf in node.leaves(): previous_consumed = 0 + for comment in list_comments(leaf.prefix, is_endmarker=False): found_fmt_off = comment.value in FMT_OFF found_fmt_skip = _contains_fmt_skip_comment(comment.value, mode) @@ -158,47 +159,17 @@ def _convert_one_fmt_off_or_skip(node: Node, mode: Mode) -> bool: previous_consumed = comment.consumed continue - if found_fmt_off: - # We only want standalone comments. If there's no previous leaf or - # the previous leaf is indentation, it's a standalone comment in - # disguise. - if comment.type != STANDALONE_COMMENT: - prev = preceding_leaf(leaf) - if prev and prev.type not in WHITESPACE: - continue - - ignored_nodes = list(_generate_ignored_nodes_from_fmt_off(leaf)) - elif found_fmt_skip: - ignored_nodes = list( - _generate_ignored_nodes_from_fmt_skip(leaf, comment) - ) + (ignored_nodes, hidden_value, standalone_comment_prefix) = ( + _gather_fmt_off_data(leaf, comment, previous_consumed) + if found_fmt_off + else _gather_fmt_skip_data(leaf, comment) + ) if not ignored_nodes: continue - first = ignored_nodes[0] # Can be a container node with the `leaf`. + first = ignored_nodes[0] parent = first.parent - prefix = first.prefix - - if found_fmt_off: - first.prefix = prefix[comment.consumed :] - standalone_comment_prefix = ( - prefix[:previous_consumed] + "\n" * comment.newlines - ) - elif found_fmt_skip: - first.prefix = "" - standalone_comment_prefix = prefix - - hidden_value = "".join(str(n) for n in ignored_nodes) - - if found_fmt_off: - hidden_value = comment.value + "\n" + hidden_value - if hidden_value.endswith("\n"): - # This happens when one of the `ignored_nodes` ended with a NEWLINE - # leaf (possibly followed by a DEDENT). - hidden_value = hidden_value[:-1] - elif found_fmt_skip: - hidden_value += " " + comment.value first_idx: Optional[int] = None for ignored in ignored_nodes: @@ -223,6 +194,54 @@ def _convert_one_fmt_off_or_skip(node: Node, mode: Mode) -> bool: return False +def _gather_fmt_off_data( + leaf: Leaf, comment: ProtoComment, previous_consumed: int +) -> Tuple[List[LN], str, str]: + # We only want standalone comments. If there's no previous leaf or + # the previous leaf is indentation, it's a standalone comment in + # disguise. + if comment.type != STANDALONE_COMMENT: + prev = preceding_leaf(leaf) + if prev and prev.type not in WHITESPACE: + return ([], "", "") + + ignored_nodes = list(_generate_ignored_nodes_from_fmt_off(leaf)) + + if not ignored_nodes: + return ([], "", "") + + first = ignored_nodes[0] # Can be a container node with the `leaf`. + prefix = first.prefix + + first.prefix = prefix[comment.consumed :] + standalone_comment_prefix = prefix[:previous_consumed] + "\n" * comment.newlines + + hidden_value = comment.value + "\n" + "".join(str(n) for n in ignored_nodes) + if hidden_value.endswith("\n"): + # This happens when one of the `ignored_nodes` ended with a NEWLINE + # leaf (possibly followed by a DEDENT). + hidden_value = hidden_value[:-1] + + return (ignored_nodes, hidden_value, standalone_comment_prefix) + + +def _gather_fmt_skip_data( + leaf: Leaf, comment: ProtoComment +) -> Tuple[List[LN], str, str]: + ignored_nodes = list(_generate_ignored_nodes_from_fmt_skip(leaf, comment)) + + if not ignored_nodes: + return ([], "", "") + + first = ignored_nodes[0] # Can be a container node with the `leaf`. + standalone_comment_prefix = first.prefix + first.prefix = "" + + hidden_value = "".join(str(n) for n in ignored_nodes) + " " + comment.value + + return (ignored_nodes, hidden_value, standalone_comment_prefix) + + def _generate_ignored_nodes_from_fmt_off(leaf: Leaf) -> Iterator[LN]: """Starting from the container of `leaf`, generate all leaves until `# fmt: on`. From 31b68e1ee3990408b700b4039d2e099141ede698 Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Thu, 26 Oct 2023 11:29:05 +0300 Subject: [PATCH 14/27] Add new test case --- tests/data/cases/fmtskip5.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/data/cases/fmtskip5.py b/tests/data/cases/fmtskip5.py index d7b15e0ff41..b00fa490d02 100644 --- a/tests/data/cases/fmtskip5.py +++ b/tests/data/cases/fmtskip5.py @@ -8,6 +8,15 @@ else: print("I'm bad") +if ( + a == 3 # fmt: skip + and b != 9 # fmt: skip + and c is not None +): + print("I'm good!") +else: + print("I'm bad") + # output @@ -20,3 +29,12 @@ print("I'm good!") else: print("I'm bad") + +if ( + a == 3 # fmt: skip + and b != 9 # fmt: skip + and c is not None +): + print("I'm good!") +else: + print("I'm bad") From 0d43b9eeecbd49e91847b8b5ca3f3c015cdf1b47 Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Thu, 26 Oct 2023 11:30:30 +0300 Subject: [PATCH 15/27] Rename --- src/black/comments.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/black/comments.py b/src/black/comments.py index e4e53bd3b00..3e0795dc9fa 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -160,9 +160,9 @@ def _convert_one_fmt_off_or_skip(node: Node, mode: Mode) -> bool: continue (ignored_nodes, hidden_value, standalone_comment_prefix) = ( - _gather_fmt_off_data(leaf, comment, previous_consumed) + _gather_data_for_fmt_off(leaf, comment, previous_consumed) if found_fmt_off - else _gather_fmt_skip_data(leaf, comment) + else _gather_data_for_fmt_skip(leaf, comment) ) if not ignored_nodes: @@ -194,7 +194,7 @@ def _convert_one_fmt_off_or_skip(node: Node, mode: Mode) -> bool: return False -def _gather_fmt_off_data( +def _gather_data_for_fmt_off( leaf: Leaf, comment: ProtoComment, previous_consumed: int ) -> Tuple[List[LN], str, str]: # We only want standalone comments. If there's no previous leaf or @@ -225,7 +225,7 @@ def _gather_fmt_off_data( return (ignored_nodes, hidden_value, standalone_comment_prefix) -def _gather_fmt_skip_data( +def _gather_data_for_fmt_skip( leaf: Leaf, comment: ProtoComment ) -> Tuple[List[LN], str, str]: ignored_nodes = list(_generate_ignored_nodes_from_fmt_skip(leaf, comment)) From 035704d2c91de6e54ebcc89e39f88a1c33c592ee Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Thu, 26 Oct 2023 12:18:28 +0300 Subject: [PATCH 16/27] Fix falsy test --- .../preview_single_line_format_skip_with_multiple_comments.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/data/cases/preview_single_line_format_skip_with_multiple_comments.py b/tests/data/cases/preview_single_line_format_skip_with_multiple_comments.py index efde662baa8..ba00d71aee7 100644 --- a/tests/data/cases/preview_single_line_format_skip_with_multiple_comments.py +++ b/tests/data/cases/preview_single_line_format_skip_with_multiple_comments.py @@ -12,7 +12,7 @@ foo = 123 # fmt: skip # noqa: E501 # pylint bar = ( - 123 , + 123, ( 1 + 5 ) # pylint # fmt:skip ) baz = "a" + "b" # pylint; fmt: skip; noqa: E501 From 9799ec8ee2aa387356ee753b83110ede5d95709c Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Thu, 26 Oct 2023 12:19:52 +0300 Subject: [PATCH 17/27] Add test case --- tests/data/cases/fmtskip5.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/data/cases/fmtskip5.py b/tests/data/cases/fmtskip5.py index b00fa490d02..cdb0056efae 100644 --- a/tests/data/cases/fmtskip5.py +++ b/tests/data/cases/fmtskip5.py @@ -17,6 +17,11 @@ else: print("I'm bad") +x = [ + 1 , # fmt: skip + 2 , + 3 , 4 # fmt: skip +] # output @@ -38,3 +43,9 @@ print("I'm good!") else: print("I'm bad") + +x = [ + 1 , # fmt: skip + 2, + 3 , 4 # fmt: skip +] From 813cef5b50ad5c5fce3ed33f4651071a6c45a176 Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Thu, 26 Oct 2023 13:05:17 +0300 Subject: [PATCH 18/27] Add heuristic to use correct sibling/preceding leaf In cases where 'fmt:skip' belonged to a closing bracket, prev_sibling would give us the opening bracket instead of the previous element inside the brackets. This is desirable if the whole thing is on the same line (as that is what fmt:skip targets), but in other cases we actually want to start processing the preceding leaf instead. --- src/black/comments.py | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/src/black/comments.py b/src/black/comments.py index 3e0795dc9fa..8ef186c0243 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -287,19 +287,37 @@ def _generate_ignored_nodes_from_fmt_skip( leaf: Leaf, comment: ProtoComment ) -> Iterator[LN]: """Generate all leaves that should be ignored by the `# fmt: skip` from `leaf`.""" - prev_sibling = leaf.prev_sibling + + # If leaf.prev_sibling is part of a bigger node that would start on a different + # line than the preceding leaf, use preceding leaf instead + preceding = preceding_leaf(leaf) + prev = leaf.prev_sibling + prev_sibling = ( + preceding + if ( + prev is not None + and preceding is not None + and prev.get_lineno() != preceding.get_lineno() + ) + else prev + ) + parent = leaf.parent + # Need to properly format the leaf prefix to compare it to comment.value, # which is also formatted comments = list_comments(leaf.prefix, is_endmarker=False) if not comments or comment.value != comments[0].value: return + if prev_sibling is not None: leaf.prefix = "" - siblings = [prev_sibling] - while "\n" not in prev_sibling.prefix and prev_sibling.prev_sibling is not None: - prev_sibling = prev_sibling.prev_sibling + siblings: List[LN] = [] + lineno = prev_sibling.get_lineno() + # We only want siblings from the same line, as fmt: skip targets a line + while prev_sibling is not None and prev_sibling.get_lineno() == lineno: siblings.insert(0, prev_sibling) + prev_sibling = prev_sibling.prev_sibling yield from siblings elif ( parent is not None and parent.type == syms.suite and leaf.type == token.NEWLINE From 397aa1c64410acc04c5aaf2074bfaadb10d4a907 Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Thu, 26 Oct 2023 13:10:21 +0300 Subject: [PATCH 19/27] Changelog entry --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index c96186c93cc..a97db3716bc 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -8,7 +8,7 @@ ### Stable style - +- Fix bug where multiple fmt:skip pragmas inside single block fails (#XXXX) ### Preview style From 5ac05c04baa9d8ec86365ad36259b8005251f8db Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Thu, 26 Oct 2023 13:16:57 +0300 Subject: [PATCH 20/27] Update PR number --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index a97db3716bc..3712fef032c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -8,7 +8,7 @@ ### Stable style -- Fix bug where multiple fmt:skip pragmas inside single block fails (#XXXX) +- Fix bug where multiple fmt:skip pragmas inside single block fails (#3978) ### Preview style From be040e6bc8131e0b2209d8ef72d2db9f8c6bd634 Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Thu, 26 Oct 2023 13:23:41 +0300 Subject: [PATCH 21/27] Update docs --- docs/contributing/reference/reference_functions.rst | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/docs/contributing/reference/reference_functions.rst b/docs/contributing/reference/reference_functions.rst index dd92e37a7d4..d6726458f4a 100644 --- a/docs/contributing/reference/reference_functions.rst +++ b/docs/contributing/reference/reference_functions.rst @@ -111,8 +111,6 @@ Utilities .. autofunction:: black.nodes.container_of -.. autofunction:: black.comments.convert_one_fmt_off_pair - .. autofunction:: black.diff .. autofunction:: black.linegen.dont_increase_indentation @@ -125,8 +123,6 @@ Utilities .. autofunction:: black.comments.generate_comments -.. autofunction:: black.comments.generate_ignored_nodes - .. autofunction:: black.comments.is_fmt_on .. autofunction:: black.comments.children_contains_fmt_on @@ -145,7 +141,7 @@ Utilities .. autofunction:: black.brackets.max_delimiter_priority_in_atom -.. autofunction:: black.normalize_fmt_off +.. autofunction:: black.normalize_format_skipping .. autofunction:: black.numerics.normalize_numeric_literal From 71ca016ca662deb670145305b547e5558798952f Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Thu, 26 Oct 2023 17:55:20 +0300 Subject: [PATCH 22/27] Add more test cases --- tests/data/cases/fmtskip7.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/data/cases/fmtskip7.py b/tests/data/cases/fmtskip7.py index 15ac0ad7080..f88cb2cbbfa 100644 --- a/tests/data/cases/fmtskip7.py +++ b/tests/data/cases/fmtskip7.py @@ -3,9 +3,15 @@ c = 9 #fmt: skip d = "thisisasuperlongstringthisisasuperlongstringthisisasuperlongstringthisisasuperlongstring" #fmt:skip +if True: print("yay") # fmt: skip +class Foo: ... # fmt: skip + # output a = "this is some code" b = 5 # fmt:skip c = 9 # fmt: skip d = "thisisasuperlongstringthisisasuperlongstringthisisasuperlongstringthisisasuperlongstring" # fmt:skip + +if True: print("yay") # fmt: skip +class Foo: ... # fmt: skip From d232f7bec458268cdc403503717c471d0dee66f4 Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Thu, 26 Oct 2023 17:58:04 +0300 Subject: [PATCH 23/27] Fix another corner case (#3682) Here we enable the backtracking to reach preceding leafs even outside the same parent. As long as they are on the same line, it should be fine, as is supposed to target the whole line. --- src/black/comments.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/black/comments.py b/src/black/comments.py index 8ef186c0243..a5f907bcca1 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -315,9 +315,16 @@ def _generate_ignored_nodes_from_fmt_skip( siblings: List[LN] = [] lineno = prev_sibling.get_lineno() # We only want siblings from the same line, as fmt: skip targets a line - while prev_sibling is not None and prev_sibling.get_lineno() == lineno: + while ( + prev_sibling is not None + and prev_sibling.type not in WHITESPACE + and prev_sibling.get_lineno() == lineno + ): siblings.insert(0, prev_sibling) - prev_sibling = prev_sibling.prev_sibling + preceding = preceding_leaf(prev_sibling) + prev = prev_sibling.prev_sibling + prev_sibling = preceding if prev is None else prev + yield from siblings elif ( parent is not None and parent.type == syms.suite and leaf.type == token.NEWLINE From beefa045e2928c3eccb5716330a4bf103746f660 Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Fri, 27 Oct 2023 11:12:38 +0300 Subject: [PATCH 24/27] Remove check that can never happen --- src/black/comments.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/black/comments.py b/src/black/comments.py index a5f907bcca1..27e9aba7e4e 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -304,12 +304,6 @@ def _generate_ignored_nodes_from_fmt_skip( parent = leaf.parent - # Need to properly format the leaf prefix to compare it to comment.value, - # which is also formatted - comments = list_comments(leaf.prefix, is_endmarker=False) - if not comments or comment.value != comments[0].value: - return - if prev_sibling is not None: leaf.prefix = "" siblings: List[LN] = [] From 422178d8c9ff3b14c8b6b82dace2c3636937e68f Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Fri, 27 Oct 2023 11:13:18 +0300 Subject: [PATCH 25/27] Expand test case --- tests/data/cases/fmtpass_imports.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/data/cases/fmtpass_imports.py b/tests/data/cases/fmtpass_imports.py index 8b3c0bc662a..81d1d3a7e6e 100644 --- a/tests/data/cases/fmtpass_imports.py +++ b/tests/data/cases/fmtpass_imports.py @@ -17,3 +17,6 @@ import tempfile import zoneinfo + +from foo import bar +from foo import bar # fmt: skip From a8e63730edfafce3d321edb6b9fe80ba8a1c8070 Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Fri, 27 Oct 2023 11:45:37 +0300 Subject: [PATCH 26/27] Get rid of special cases and combine to one branch --- src/black/comments.py | 81 ++++++++++++++++--------------------------- 1 file changed, 30 insertions(+), 51 deletions(-) diff --git a/src/black/comments.py b/src/black/comments.py index 27e9aba7e4e..22109b6ff12 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -11,7 +11,6 @@ container_of, first_leaf_of, preceding_leaf, - syms, ) from blib2to3.pgen2 import token from blib2to3.pytree import Leaf, Node @@ -287,61 +286,41 @@ def _generate_ignored_nodes_from_fmt_skip( leaf: Leaf, comment: ProtoComment ) -> Iterator[LN]: """Generate all leaves that should be ignored by the `# fmt: skip` from `leaf`.""" + prev = _get_previous_node_to_ignore(leaf) - # If leaf.prev_sibling is part of a bigger node that would start on a different - # line than the preceding leaf, use preceding leaf instead - preceding = preceding_leaf(leaf) - prev = leaf.prev_sibling - prev_sibling = ( - preceding - if ( - prev is not None - and preceding is not None - and prev.get_lineno() != preceding.get_lineno() - ) - else prev - ) - - parent = leaf.parent - - if prev_sibling is not None: + siblings: List[LN] = [] + if prev is not None and (leaf.get_lineno() or 1) - (prev.get_lineno() or -1) <= 1: leaf.prefix = "" - siblings: List[LN] = [] - lineno = prev_sibling.get_lineno() - # We only want siblings from the same line, as fmt: skip targets a line + lineno = prev.get_lineno() while ( - prev_sibling is not None - and prev_sibling.type not in WHITESPACE - and prev_sibling.get_lineno() == lineno + prev is not None + and prev.type not in WHITESPACE + and prev.get_lineno() == lineno ): - siblings.insert(0, prev_sibling) - preceding = preceding_leaf(prev_sibling) - prev = prev_sibling.prev_sibling - prev_sibling = preceding if prev is None else prev - - yield from siblings - elif ( - parent is not None and parent.type == syms.suite and leaf.type == token.NEWLINE - ): - # The `# fmt: skip` is on the colon line of the if/while/def/class/... - # statements. The ignored nodes should be previous siblings of the - # parent suite node. - leaf.prefix = "" - ignored_nodes: List[LN] = [] - parent_sibling = parent.prev_sibling - while parent_sibling is not None and parent_sibling.type != syms.suite: - ignored_nodes.insert(0, parent_sibling) - parent_sibling = parent_sibling.prev_sibling - # Special case for `async_stmt` where the ASYNC token is on the - # grandparent node. - grandparent = parent.parent + siblings.insert(0, prev) + prev = _get_previous_node_to_ignore(prev) + + yield from siblings + + +def _get_previous_node_to_ignore(node: LN) -> Optional[LN]: + """ + Return previous sibling if it is on the same line as preceding leaf, otherwise + return the preceding leaf. + """ + preceding = preceding_leaf(node) + previous = node.prev_sibling + return ( + preceding if ( - grandparent is not None - and grandparent.prev_sibling is not None - and grandparent.prev_sibling.type == token.ASYNC - ): - ignored_nodes.insert(0, grandparent.prev_sibling) - yield from iter(ignored_nodes) + previous is None + or ( + preceding is not None + and previous.get_lineno() != preceding.get_lineno() + ) + ) + else previous + ) def is_fmt_on(container: LN) -> bool: From 58e011bc1886010528582ba8b0e52a82e0f4dfe9 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Mon, 6 Nov 2023 18:40:49 -0800 Subject: [PATCH 27/27] Update CHANGES.md --- CHANGES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES.md b/CHANGES.md index 3fd374e6c3c..b17862ebfdb 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -12,6 +12,7 @@ ### Stable style - Fix bug where multiple fmt:skip pragmas inside single block fails (#3978) +- Fix crash on formatting bytes strings that look like docstrings (#4003) - Fix crash when whitespace followed a backslash before newline in a docstring (#4008) - Fix crash on formatting code like `await (a ** b)` (#3994) - No longer treat leading f-strings as docstrings. This matches Python's behaviour and