Whitespace before fmt: skip comment is normalised

[m-czernek]

Describe the bug

Apologies if this is a duplicate; I have read:

• Behavior of # fmt: skip in the presence of other comments/pragmas #3330
• https://github.com/psf/black/pull/3959/files
• Can't use other comments after "# fmt: ..." (like "# noqa: ...") #2213

Based on #3959, Black should support e.g. this: # pylint: disable=redefined-outer-name # fmt: skip and yet I cannot get it to work on the newest Black version.

I have the following line:

client = billingdataservice.app.test_client() # pylint: disable=redefined-outer-name

The line is too long for black, so when I run black, I get this:

client = (
    billingdataservice.app.test_client()
)  # pylint: disable=redefined-outer-name

We don't want that, so I tried to use fmt: skip. I tried the following comments:

• client = billingdataservice.app.test_client() # pylint: disable=redefined-outer-name; fmt: skip
• client = billingdataservice.app.test_client() # pylint: disable=redefined-outer-name # fmt: skip
• client = billingdataservice.app.test_client() # fmt: skip # pylint: disable=redefined-outer-name
• client = billingdataservice.app.test_client() # fmt: skip; pylint: disable=redefined-outer-name

as well as mixing number of spaces before/after the hash sign, e.g. ...test_client() # fmt: skip; pylint: ... or ...test_client() # fmt: skip; pylint: ... etc.

Reproducer

file.py:

def client():
    client = billingdataservice.app.test_client() # fmt: skip; pylint: disable=redefined-outer-name

Command:

$ black --preview file.py

(The --preview flag seems not to matter for this issue; behavior is identical with or without it)

Black always modifies the line, e.g.:

def client():
    client = ( 
        billingdataservice.app.test_client()
    )  # fmt: skip; pylint: disable=redefined-outer-name

Which means fmt: skip did not get honored.

Environment

$ black --version
black, 23.12.1 (compiled: yes)
Python (CPython) 3.11.6

• Black's version: 23.12.1
• OS and Python version: OpenSUSE Tumbleweed, Python 3.11

[m-czernek]

In fact, the # fmt: skip comment seems not to be honored at all?

For example, black still modifies client = billingdataservice.app.test_client() # fmt: skip -> client = billingdataservice.app.test_client() # fmt: skip (note added space before the # character)

Is that expected behavior?

[m-czernek]

Simplest reproducer:

$ black -c "test # fmt:skip"
test  # fmt:skip
$ black -c "test # fmt: skip"
test  # fmt: skip
$ black --preview -c "test # fmt: skip"
test  # fmt: skip
$ black --preview -c "test # fmt:skip"
test  # fmt:skip

Despite a line that contains fmt: skip comment, that line is modified by Black (always added a space before the # symbol). Not sure if I'm missing some flag or configuration.

[JelleZijlstra]

I can't reproduce much of what you're reporting. You may be running a different version of Black than you think you're running.

% black -c "1+1 # fmt:skip; some other comment"          
1+1  # fmt:skip; some other comment
% black -c '''def client():
    client = billingdataservice.app.test_client() # fmt: skip; pylint: disable=redefined-outer-name
'''
def client():
    client = billingdataservice.app.test_client()  # fmt: skip; pylint: disable=redefined-outer-name
% black --version                                       
black, 23.12.1 (compiled: yes)
Python (CPython) 3.11.1

[m-czernek]

Hmm, this is strange. The above output is from Black I have installed by using pip install black.

I have reproduced the issue on the container we have in the repo, which I locally built with the black-container name:

podman run --rm black-container:latest black -c "test # fmt:skip"
test  # fmt:skip
$ podman run --rm black-container:latest black -c '''def client():
>     client = billingdataservice.app.test_client() # fmt: skip; pylint: disable=redefined-outer-name
> '''
def client():
    client = (
        billingdataservice.app.test_client()
    )  # fmt: skip; pylint: disable=redefined-outer-name

Similarly, I can do the same on Fedora (a different computer) when installing Black directly from Github:

$ black -c '''def client():
    client = billingdataservice.app.test_client() # fmt: skip; pylint: disable=redefined-outer-name
'''
def client():
    client = (
        billingdataservice.app.test_client()
    )  # fmt: skip; pylint: disable=redefined-outer-name

$ black --version
black, 23.12.2.dev13+ge11eaf2 (compiled: no)
Python (CPython) 3.12.1

I am a bit stumped as to what could be the discrepancy here...

[hauntsaninja]

@m-czernek the fix is only in the preview style (although I think technically it would have met the stability policy).

That is, your examples are fixed if you use --preview. I bet @JelleZijlstra is just working in a codebase where he has preview style in his config.

(We're also at the time of year where we cut preview style over to stable, so follow on at #4042 if you're interested)

[m-czernek]

@hauntsaninja I'm not sure you understand the issue. The examples seems not to be fixed by using --preview flag, for example using the container on your main:

podman run --rm black-container:latest black --preview -c "test # fmt:skip"
test  # fmt:skip

In other words, the # fmt:skip functionality seems to me not to work regardless of the --preview flag.

[hauntsaninja]

Okay, apologies, I was looking too much at #4143 (comment) / mention of #3959

IIUC the only issue remaining in preview is that Black normalises the whitespace before # fmt: skip (it does this on stable as well). This is regardless of whether there are other comments present on the line or not.

Does that sound right to you? (I've renamed the issue, but will rename again if there's something else as well)

[cobaltt7]

See also #2970 (review)