Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a string merging/split issue caused by standalone comments. #3227

Merged
merged 6 commits into from Aug 23, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.md
Expand Up @@ -26,6 +26,8 @@
normalized as expected (#3168)
- When using `--skip-magic-trailing-comma` or `-C`, trailing commas are stripped from
subscript expressions with more than 1 element (#3209)
- Fix a string merging/split issue when a comment is present in the middle of implicitly
concatenated strings on its own line (#3227)

### _Blackd_

Expand Down
18 changes: 17 additions & 1 deletion src/black/trans.py
Expand Up @@ -553,6 +553,9 @@ def make_naked(string: str, string_prefix: str) -> str:

next_str_idx += 1

# Take a note on the index of the non-STRING leaf.
non_string_idx = next_str_idx

S_leaf = Leaf(token.STRING, S)
if self.normalize_strings:
S_leaf.value = normalize_string_quotes(S_leaf.value)
Expand All @@ -572,7 +575,20 @@ def make_naked(string: str, string_prefix: str) -> str:
string_leaf = Leaf(token.STRING, S_leaf.value.replace(BREAK_MARK, ""))

if atom_node is not None:
replace_child(atom_node, string_leaf)
# If not all children of the atom node are merged (this can happen
# when there is a standalone comment in the middle) ...
if non_string_idx - string_idx < len(atom_node.children):
# We need to replace the old STRING leaves with the new string leaf.
first_child_idx: Optional[int] = None
for idx in range(string_idx, non_string_idx):
child_idx = LL[idx].remove()
if first_child_idx is None:
first_child_idx = child_idx
if first_child_idx is not None:
atom_node.insert_child(first_child_idx, string_leaf)
yilei marked this conversation as resolved.
Show resolved Hide resolved
else:
# Else replace the atom node with the new string leaf.
replace_child(atom_node, string_leaf)

# Build the final line ('new_line') that this method will later return.
new_line = line.clone()
Expand Down
35 changes: 35 additions & 0 deletions tests/data/preview/long_strings.py
Expand Up @@ -72,6 +72,25 @@
zzz,
)

inline_comments_func1(
"if there are inline "
"comments in the middle "
# Here is the standard alone comment.
"of the implicitly concatenated "
"string, we should handle "
"them correctly",
xxx,
)

inline_comments_func2(
"what if the string is very very very very very very very very very very long and this part does "
"not fit into a single line? "
# Here is the standard alone comment.
"then the string should still be properly handled by merging and splitting "
"it into parts that fit in line length.",
xxx,
)

raw_string = r"This is a long raw string. When re-formatting this string, black needs to make sure it prepends the 'r' onto the new string."

fmt_string1 = "We also need to be sure to preserve any and all {} which may or may not be attached to the string in question.".format("method calls")
Expand Down Expand Up @@ -395,6 +414,22 @@ def foo():
zzz,
)

inline_comments_func1(
"if there are inline comments in the middle "
# Here is the standard alone comment.
"of the implicitly concatenated string, we should handle them correctly",
xxx,
)

inline_comments_func2(
"what if the string is very very very very very very very very very very long and"
" this part does not fit into a single line? "
# Here is the standard alone comment.
"then the string should still be properly handled by merging and splitting "
"it into parts that fit in line length.",
xxx,
)

raw_string = (
r"This is a long raw string. When re-formatting this string, black needs to make"
r" sure it prepends the 'r' onto the new string."
Expand Down