Skip to content

Commit

Permalink
Fix a string merging/split issue caused by standalone comments. (#3227)
Browse files Browse the repository at this point in the history
Fixes #2734: a standalone comment causes strings to be merged into one far too long (and requiring two passes to do so).

Co-authored-by: Richard Si <63936253+ichard26@users.noreply.github.com>
  • Loading branch information
yilei and ichard26 authored Aug 23, 2022
1 parent 59acf8a commit 21218b6
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 2 deletions.
1 change: 0 additions & 1 deletion .github/workflows/diff_shades.yml
Original file line number Diff line number Diff line change
Expand Up @@ -155,4 +155,3 @@ jobs:
if: always()
run: >
diff-shades show-failed --check --show-log ${{ matrix.target-analysis }}
--check-allow 'sqlalchemy:test/orm/test_relationship_criteria.py'
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
16 changes: 15 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,18 @@ 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 = LL[string_idx].remove()
for idx in range(string_idx + 1, non_string_idx):
LL[idx].remove()
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 21218b6

Please sign in to comment.