Skip to content

Commit

Permalink
Fix a string merging/split issue caused by standalone comments.
Browse files Browse the repository at this point in the history
Fixes psf#2734: a standalone comment causes strings to be merged into one far too long (and requiring two passes to do so).
  • Loading branch information
yilei committed Aug 16, 2022
1 parent 4ebf14d commit 7f81852
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 1 deletion.
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
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
17 changes: 16 additions & 1 deletion src/black/trans.py
Original file line number Diff line number Diff line change
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,19 @@ 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...
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 = 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)
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
Original file line number Diff line number Diff line change
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

0 comments on commit 7f81852

Please sign in to comment.