Preview style unnecessarily moves comment

[JelleZijlstra]

The preview style moves a comment on one line of an implicitly concatenated string to a different line.

$ python -m black --version
python -m black, 23.9.1 (compiled: yes)
Python (CPython) 3.9.14
$ python -m black --diff --preview movecomment.py 
--- movecomment.py      2023-10-06 01:40:10.153647+00:00
+++ movecomment.py      2023-10-06 01:40:41.077230+00:00
@@ -1,7 +1,7 @@
 xxxxxxx.xxfx(
-    f"xxxx_xxxxxxxx: "  # xxxxxxx
+    "xxxx_xxxxxxxx: "
     f"xxxxx={xxxx_xxxxxxxx.xxxxx}, "
     f"_xxx_xxxxxx={xxxx_xxxxxxxx._xxx_xxxxxx}, "
     f"xxx_xxxxxx={xxxx_xxxxxxxx.xxx_xxxxxx}, "
-    f"xxx_xxxxx={xxxx_xxxxxxxx.xxx_xxxxx}"
+    f"xxx_xxxxx={xxxx_xxxxxxxx.xxx_xxxxx}"  # xxxxxxx
 )
would reformat movecomment.py

All done! ✨ 🍰 ✨
1 file would be reformatted.
$ cat movecomment.py 
xxxxxxx.xxfx(
    f"xxxx_xxxxxxxx: "  # xxxxxxx
    f"xxxxx={xxxx_xxxxxxxx.xxxxx}, "
    f"_xxx_xxxxxx={xxxx_xxxxxxxx._xxx_xxxxxx}, "
    f"xxx_xxxxxx={xxxx_xxxxxxxx.xxx_xxxxxx}, "
    f"xxx_xxxxx={xxxx_xxxxxxxx.xxx_xxxxx}"
)

[henriholopainen]

This seems to be happening also with the stable style. Should it still be considered as a bug?

% black --version
black, 23.9.1 (compiled: yes)
Python (CPython) 3.11.3
% black --diff movecomment.py
--- movecomment.py	2023-10-14 08:29:58.442254+00:00
+++ movecomment.py	2023-10-14 08:29:59.965634+00:00
@@ -1,7 +1,7 @@
 xxxxxxx.xxfx(
-    f"xxxx_xxxxxxxx: "  # xxxxxxx
+    "xxxx_xxxxxxxx: "
     f"xxxxx={xxxx_xxxxxxxx.xxxxx}, "
     f"_xxx_xxxxxx={xxxx_xxxxxxxx._xxx_xxxxxx}, "
     f"xxx_xxxxxx={xxxx_xxxxxxxx.xxx_xxxxxx}, "
-    f"xxx_xxxxx={xxxx_xxxxxxxx.xxx_xxxxx}"
+    f"xxx_xxxxx={xxxx_xxxxxxxx.xxx_xxxxx}"  # xxxxxxx
 )
would reformat movecomment.py

All done! ✨ 🍰 ✨
1 file would be reformatted.

[JelleZijlstra]

It doesn't happen in the stable style. You may have a configuration file that sets preview to true.

[henriholopainen]

Hmm, couldn't find the configuration, but a clean container install proves you are correct. I can try and take a look at this issue.

[henriholopainen]

Hmm, couldn't find the configuration, but a clean container install proves you are correct. I can try and take a look at this issue.

[JelleZijlstra]

black -v should print where it gets its config from.

[henriholopainen]

This is proving to be a bit tricky. Touching implicitly concatenated strings with comments on any line could change the semantics of the comment. The safest approach would be to not transform such strings at all and hold onto the integrity of the comment. Currently the logic goes like this:

foo(
  "a"  # c1
  "b"  # c2
  "c"  # c3
)
-->
foo("abc")  # c1  # c2  # c3

I think we should either accept this, or then having a comment on any line "a" – "c" should skip processing for the whole string. Any thoughts?

[JelleZijlstra]

We probably should just keep your example code unchanged. If the user put a comment next to part of the string, we should keep it there.

Though note that we currently merge the comments if the different lines hold non-strings instead:

$ black -c 'foo(
> a, # 1
> b, # 2
> c  # 3
> )'
foo(a, b, c)  # 1  # 2  # 3

That's a behavior I'd be open to changing.

[henriholopainen]

A fix proposal in this PR, let me know what you think! I can take a look at the non-string case in another PR if you think it is worth the time.

[JelleZijlstra]

Thanks for the fix!