Apply more granular changes by using # fmt: skip

[wkentaro]

Close #388

@akaihola What do you think of this solution? (NOTE: this won't work with nonstandard indentations)

Before

After

Limitations

[akaihola]

@wkentaro indeed an interesting approach!

In some test cases, the end result seems to be different, see build #1419. Do you think these are in fact improvements to old behavior instead of erroneous results due to a bug?

What happens if the decorated lines become longer than 88 characters (or whatever is the configured Black margin)? I suppose that's ok because the comment instructs Black to ignore those lines and pass them through as such.

Handling of empty lines seems to be asymmetrical in your implementation – decorating them is probably supposed to be special cased as well?

What happens inside multi-line strings?

What happens if I modify only some lines in a multi-line expression which doesn't already conform to Black formatting rules? For example if this is the old revision:

call_func(first_arg=1,
    second_arg = 2,
    third_arg="three"
)

and this is my new revision:

call_func(first_arg=1,
    second_arg = 2,
    third_arg=3
)

(so only the third_arg= line changed)

Would the whole function call still be reformatted by Black, or only the line I modified? According to your "After" example, the latter seems true. Which one would be desirable in your opinion?

Could you approach replace the current difflib-chunk-applying approach completely?

Could we avoid the # fmt:skip variants limitation by appending something unique as another comment? Would Black still recognize its own skip directive?

[wkentaro]

What happens if the decorated lines become longer than 88 characters (or whatever is the configured Black margin)? I suppose that's ok because the comment instructs Black to ignore those lines and pass them through as such.

Yes, that's going to be ok.

---

Handling of empty lines seems to be asymmetrical in your implementation – decorating them is probably supposed to be special cased as well?

When there is a line --# fmt: skip (with 2 spaces), Black reformats it to # fmt: skip (no spaces), that's why we need to have an asymmetric de-decoration. Somehow we need to fix along with removing difflib as described below.

---

What happens inside multi-line strings?

In this case, Black ignores # fmt:skip decoration, and the decoration # fmt:skip is treated as a comment so will affect the result.

If we apply black with 2 times, it gives an expected result.

UPDATE: if we don't change the line of multi-line string, Black raises error, so this needs to be fixed.

---

Would the whole function call still be reformatted by Black, or only the line I modified? According to your "After" example, the latter seems true. Which one would be desirable in your opinion?

In this particular case, the Black won't reformat anything as there is going to be ) # fmt: skip, and it cancels the whole reformat of the function call. This is probably the big drawback of this approach.

In my particular use case (a codebase of several people changing), I prefer the more conservative result since I can still reformat lines if I need it by black --code "<lines extracted by editor>" (I use vim and its visual selection). Reformatting again is easier than reverting a reformat.

---

Could you approach replace the current difflib-chunk-applying approach completely?

I just found an edge case by trying this. Without difflib, it creates an indented line like below:

this is because # fmt:skip will be formatted as --# fmt:skip and the trailing 2 spaces cannot be removed with the current algorithm.

---

Could we avoid the # fmt:skip variants limitation by appending something unique as another comment? Would Black still recognize its own skip directive?

Yes, it seems possible by changing both FMT_SKIP and FMT_PASS.

[akaihola]

What happens inside multi-line strings?

In this case, Black ignores # fmt:skip decoration,

Your example had a concatenation of single-line strings split on multiple lines.

What I meant was a triple-quoted multi-line string like e.g.

"""first line
second line
third line"""

[akaihola]

I prefer the more conservative result [...] Reformatting again is easier than reverting a reformat.

This seems to be a matter of preference. For me it's the opposite – I want the code base to be marching steadily towards complete Blackification, and if this sometimes means that a couple of neighboring unmodified statements are reformatted as well, that's a reasonable trade-off.

This is what I attempted to solve in #221. I hoped to be able to extract Black's reformatted output in chunks corresponding to expressions which are parenthetically balanced and cover a sequence of complete lines of code. If it could also tell which original lines were reformatted, that set of information could replace the use of difflib altogether. Unfortunately this approach didn't really work out (see discussion).

[akaihola]

I'm postponing to consider this for 1.8.0. There are other features lined up for 1.7.0 and I prefer to not delay it further.