psf / black

The uncompromising Python code formatter
https://black.readthedocs.io/en/stable/
MIT License
38.76k stars 2.43k forks source link

`# fmt: skip` ignored in `if` conditionals #3682

Open metov opened 1 year ago

metov commented 1 year ago

Describe the bug

# fmt: skip does not work in if conditionals.

To Reproduce I expect the following code to not be reformatted, due to the skip directive. It is.

if True: print("yay") # fmt: skip

https://black.vercel.app/?version=stable&state=_Td6WFoAAATm1rRGAgAhARYAAAB0L-Wj4AC8AG9dAD2IimZxl1N_Wlws4TBexXdsrAK05TP_h68mLf6AjhWJHCRgXa5Rwd9gFE21oq9pVC8qdR56y-a_GL_VJamRio8oas6Eub07I8wP5I3sv2WdLkei-6Q30-uDRVSurPlC7LiqqMoMjKtW0_uwtGuaAAAAiskgpy3XxxYAAYsBvQEAAAblmGSxxGf7AgAAAAAEWVo%3D

Expected behavior

Since the line contains # fmt: skip, black should leave it alone.

Environment

Problem happens on Black Playground, clearly it is not env specific.

metov commented 1 year ago

This may be related to https://github.com/psf/black/issues/3364. I'm not really familiar with how exactly black works, but I assume the skip directive applies to a single "syntactic element", and the condition is split off from the statement into a separate element before the skip is applied. Clearly it should be the other way around.

This is only a hunch though. Please do not close either of these as duplicate until there is actual evidence that these are indeed the same bug.

treykeown commented 1 year ago

I believe the underlying cause for my issue is related. #fmt: skip doesn't work for single-line class definitions like this:

class Foo: ...  # fmt: skip

It's always reformatted for me to

class Foo:
    ...  # fmt: skip
tim-timman commented 7 months ago

Just want to add another situation this bugs out, but I've verified that #3978 fixes this (so please get that merged).

For example, take this code:

# {} () [] / % & | and or ... - + .  etc as part of the "else" clause presents the bug
(
    ""
    if True else  # fmt: skip
    []
    # NOTE: this entire comment is completely GONE!
    # Doesn't matter how many rows
)

Run in Online Formatter

Incorrectly returns:

(
    ""
    if True
    else []  # fmt: skip
)

Note: The comment is gone and the if True else line was broken.

Whereas in here it causes the formatting to fail completely.

(
    ""
    if True else  # fmt: skip
    [""]  # any single element [x], {x} (not parens), or ""[x] etc. breaks given that this comment (together with "else ..." format) goes beyond the allowed line length (default: 88)
)

Run in Online Formatter

The resulting error is:

Unable to match a closing bracket to the following opening bracket: ]

sizhky commented 6 days ago

https://black.vercel.app/?version=stable&state=_Td6WFoAAATm1rRGAgAhARYAAAB0L-Wj4AEdAJtdAD2IimZxl1N_Wlws4TBexXdsrAK05TP_h68mLf6AjVQdGFkTtfD6qMBt_Tm8ESlGKFFnqOt0PnkLfYp7ALPnHIRICw47qE26nQ5B_5gDrdsCVX6ITnWUI7gt6krUlEc1vTJ5DppRKZ7gm8CjXe-wH7oWz1ERA1RIdOgcLiJJDy6LJuxRj_ZprzNIrWxQCHxwSkpX-GESJcU47CUAAAC55fD1-pQswwABtwGeAgAAWZzhp7HEZ_sCAAAAAARZWg==

When can this be fixed? It's a major blocker for a lot of devs IMO, who prefer one-liners for short contexts