Add "# fmt: skip" directive to black

[saroad2]

Fixes #1162

After being requested for quite some time, implemented the ability to skip formatting only one line using #fmt: skip 😄 .

[cooperlees]

Thanks! Code makes sense to me, and the tests look to have good coverage. This looks good to me.

[ichard26]

The lack of documentation additions scares me :-) I would like to see a quick explanation on how it works and a few examples. Although honestly the documentation needs to be restructured again since it's so messy right now. I don't know about my availability but I'll probably end up being the guy who will handle this refactor. See #1759.

[cooperlees]

Yeah my bad there. Good call @ichard26 - If you make a plan and split up how you'd like things refactored we might be able to get help from others on that so you don't have to do it all!

[saroad2]

Hey guys, you want me to fill the documentation gaps, or you're on it?

[HarrySky]

Great feature! What is the status of this PR? Friendly ping @ichard26 @cooperlees

[mrgnw]

I got this error when I tried, looks like the first pass removes # fmt: skip.

$ black skip_test.py
error: cannot format skip_test.py: INTERNAL ERROR: Black produced different code on the second pass of the formatter.  Please report a bug on https://github.com/psf/black/issues.  This diff might be helpful:

Log:

Mode(target_versions=set(), line_length=88, string_normalization=True, experimental_string_processing=False, is_pyi=False)
--- source
+++ first pass
@@ -1,11 +1,8 @@
-z = 'hello ' + "world"
+z = "hello " + "world"
 
-# fmt: skip
 custom_formatting = [
     0,  1,
     2
 ]
 
-regular_formatting = [
-   0,  1,  2,  'this will be formatted'
-]
+regular_formatting = [0, 1, 2, "this will be formatted"]
--- first pass
+++ second pass
@@ -1,8 +1,5 @@
 z = "hello " + "world"
 
-custom_formatting = [
-    0,  1,
-    2
-]
+custom_formatting = [0, 1, 2]
 
 regular_formatting = [0, 1, 2, "this will be formatted"]

(Python 3.8.0 / macOS 10.15, pip install git+git://github.com/saroad2/black.git@add_skip)

[saroad2]

First of all, nice catch @mrgnw ! I fixed the bug and added a unit test to validate this won't happen again.

However, this is a misuse of the # fmt: skip directive. You should use this directive at the end of the line you wish to skip. For example:

l = ["I", "will", "not", "be", "formatted",]  # fmt: skip

The following will not work:

# fmt: skip
l = ["I", "will", "be", "formatted",] 

[mrgnw]

I see, thank you! I just copy+pasted the example from #1162. Technically I think this is more in line with #790 because it's in-line, but I think this is great and I'm sure many of us would be happy to have it.

[saroad2]

Kindly reminding that this PR is ready to be merged.

[JelleZijlstra]

Could you add tests that put # fmt: skip in the middle of a complicated statement like a multi-line if condition? That's where we got most of our stability bugs lately, so it would be good to make sure this new feature doesn't introduce more.

I'm supportive of merging otherwise.

[saroad2]

Hey @JelleZijlstra , thank you for the response!
Apparently, there was actually a bug there! nice catch!

I fixed the bug and added another unit test to check the new behavior. If you have some other edge cases you think we should address, please let me know.

[admirabilis]

Sorry for the +1, but since this PR is ready, it would come in very handy for me, since I'm in the process of sprinkling my code with debug lines that are to be stripped on code optimization with python -O, like:

if __debug__: log.debug(f"{large_dict=!r:.100s}") # fmt: skip

[saroad2]

Hey @cooperlees , @ichard26 , @JelleZijlstra, and maybe other collaborators of the Black project.

This PR is almost 4 months old. I know you are busy people, and I know that this a voluntary project, but I would be very disappointed if a new version of Black would be released without it.

So please, if something is missing in this PR, let me know and I'll fix/add/update it.
If not, let's merge this thing!

Again, I must emphasize that I don't mean to harm anyone or to be disrespectful. I just want to see my work being used :)

[ichard26]

TBH, I'm a tiny bit hesitant since this may go against of Black's goal of being consistent, but since we have fmt: off/on already and while this will probably mean less consistent code since # fmt: skip is convenient, the benefits seem worth it. The only actual thing delaying/blocking this is that this should have documentation. I've given up on finishing up the reorganization effort before this PR gets merged since it's 99.9999999% not gonna happen due to an aforementioned lack of time. Apologies for not getting back to you sooner. For now, I recommend adding the docs to this section. I can deal with the messiness myself, eventually.

Thanks for your understanding and patience!

[saroad2]

Hey @ichard26 , thanks for the feedback!

I added the missing documentation piece to the README file.
As for the build failure, I have no idea what causes it, but it doesn't seem like my code.

Let me know if something else is missing!

[JelleZijlstra]

Merged. I guess I was hesitant because I'm still worried this will cause new stability bugs. I guess we'll see. Thanks for your contribution to Black!

[admirabilis]

It doesn't work for me. In an empty file with just if True: pass # fmt: skip, a newline gets appended, and if the space is omitted between # and fmt, it generates invalid code.

Tested on Python 3.9.1, black 20.8b2.dev83+g71bbb67

[ptsl]

From the documentation:

lines that ends with # fmt: skip
Does this mean that # fmt: skip cannot be used as a single directive on the line just before the statement (as requested in Issue #1162)?

[ichard26]

Does this mean that # fmt: skip cannot be used as a single directive on the line just before the statement (as requested in Issue #1162)?

I suppose so. /cc @saroad2 was this an intentional choice due to technicalities or something else?

[saroad2]

I guess I have linked the wrong issue to this PR.

The idea of this PR was to ignore lines which ends with # fmt: skip. For example, the following line will not be formatted:

l = ["I have",    "too much space"]  # fmt: skip

However, the following will be formatted:

# fmt: skip
l = ["I have",    "too much space"]

A new PR might include the ability that if # fmt: skip is added to a blank line, the next line should not be formatted. I guess it should be relatively easy to do, and if you wish to, I'll go ahead and make this PR.

Another thing that was mentioned here in the comments is that in order to check which lines to skip, Black compare the comment to specific values. Therefore, if you miss a space as @teresaejunior said, It will not ignore that line. Another PR should use regex comparison instead in order to solve this problem.

[admirabilis]

@saroad2 It could be my specific Python version, but it doesn't work even if the space is included. And instead of just ignoring the #fmt instruction as it should, it produces invalid code. It is always reproducible for me, and I don't have other problems with black using the Python 3.9.1 / black 20.8b2.dev83+g71bbb67 combo.

[ichard26]

@teresaejunior the newline handling being completely unaffected issue has been also reported with the old-style fmt: off/on.

[admirabilis]

@ichard26 @saroad2 Ah, OK, it works for other kinds of formatting! (Sorry, I don't know if I should be pinging you.)

[HarrySky]

Hi, great to see this feature merged, but it seems that it cannot be used with other comments like "noqa"

This will be skipped (as expected):

log.debug("Long debug message")  # fmt: skip

This will be formatted:

log.debug("Long debug message, so we ignore flake8's E501 long line message")  # fmt: skip # noqa: E501

Should I create an issue about it? Or is it how black is supposed to work?

For example bandit and flake8 detect everything properly if I put both # nosec # noqa comments on the same line.

[JelleZijlstra]

Please open a new issue. I feel like it should work as you expect, but others may disagree.

[bersbersbers]

Please open a new issue.

For any one (like me) searching: #2213