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 all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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