Indicate that a final newline was added in --diff

[TBBle]

Fixes: #1662

This works around bpo2142 from 2008.

[TBBle]

Now that I know it's an existing Python bug (I didn't think to look until after I posted the PR), I could reword this as a workaround for that bug, if that's valuable.

[ichard26]

The changes themselves look great and testing locally they do work so +1 from me!

Besides my review comment, please add an entry in CHANGES.md so the person verifying and updating the changelog before a new release has less work to do :) And I guess while you're at it, why not add yourself to the authors list in the README?

Also while this isn't a blocker, I can't reproduce the bug where Black doesn't add a newline using the VS Code integration on my machine interestingly 😕 Maybe the fact I'm using Visual Studio Code version 1.52.1 or version v2020.12.424452561 of the Python extension is why? I honestly don't care that much since theoretically this should fix the bug, but from experience even seemingly well tested PRs can have undetected serious holes ...

Thanks for looking into this and submitting a patch anyway! You rock!

[TBBle]

Would it be useful to mention CHANGES.md in CONTRIBUTING.md? I wasn't aware I was supposed to add to the former, based only on reading the latter. (And based on the PR I modelled from, #1328.)

[TBBle]

@ichard26 Should CHANGES.md link to the PR or the issue?

[ichard26]

Would it be useful to mention CHANGES.md in CONTRIBUTING.md? I wasn't aware I was supposed to add to the former, based only on reading the latter. (And based on the PR I modelled from, #1328.)

Yeah it would definitely help! I have other ideas to deal this issue (hi towncrier), but having a reminder in the docs can't hurt!

Should CHANGES.md link to the PR or the issue

Honestly I'm not sure of what the actual recommendation is, but I would go with linking to the issue personally even though the status quo is to link to the PR. For non-bugfix PRs though, linking to PR would probably be better(?)

[TBBle]

@ichard26 On the repro in VS Code, it's possible VS Code's general editor setting for "add missing newline" is set, which will be operating after black --diff has been applied. That's the workaround I am using until this fix flows through to a release.

[TBBle]

Sorry for the wild batch of force-pushes. I got excited and overlooked a couple of requested changes. -_-

It should be all-good now, the only changes since the last review were textual

[ichard26]

On the repro in VS Code, it's possible VS Code's general editor setting for "add missing newline" is set

I checked if I had that option set and I'm pretty sure I didn't. When I purposely installed an immediately crashing version of Black, no newline was added still. But it's probably some misconfiguration or misunderstanding on my part, anyway 95% of time I use Vim. Does this patch for you work on your Visual Studio Code setup? If yes, I'll just go with that then :-)

edit:

I got excited and overlooked a couple of requested changes. -_-

eh, not even close in badness to me unintentionally making it impossible to properly setup a development environment for Black development ... whoops .... So yeah, don't worry about it!

[TBBle]

I can reproduce the problem with released Black and Python extension ms-python.python-2020.12.424452561, under VSCode 1.52.1, after I remove "files.insertFinalNewline": true from the [python] section of settings.json (which is not visible in the settings UI editor).

And using the black.exe built from this PR, the problem is fixed.

[ichard26]

Looking at this again, somethings are sticking out to me, I'mma do another review hopefully soon.

[ichard26]

Hey this almost looks good to go, but on a second pass, something doesn't seem right. Not a big deal, just something odd I noticed.

[ichard26]

/cc @cooperlees @JelleZijlstra this should be ready to go, a final review and eventual merge would be appreciated! Thanks in advance!

[TBBle]

Master build failed on Documentation build but I can't see how it relates to my changes.

Warning, treated as error:
autodoc: failed to import function 'should_split_body_explode' from module 'black'; the following exception was raised:
Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.9.1/x64/lib/python3.9/site-packages/sphinx/util/inspect.py", line 325, in safe_getattr
    return getattr(obj, name, *defargs)
AttributeError: module 'black' has no attribute 'should_split_body_explode'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.9.1/x64/lib/python3.9/site-packages/sphinx/ext/autodoc/importer.py", line 106, in import_object
    obj = attrgetter(obj, mangled_name)
  File "/opt/hostedtoolcache/Python/3.9.1/x64/lib/python3.9/site-packages/sphinx/ext/autodoc/__init__.py", line 289, in get_attr
    return autodoc_attrgetter(self.env.app, obj, name, *defargs)
  File "/opt/hostedtoolcache/Python/3.9.1/x64/lib/python3.9/site-packages/sphinx/ext/autodoc/__init__.py", line 2186, in autodoc_attrgetter
    return safe_getattr(obj, name, *defargs)
  File "/opt/hostedtoolcache/Python/3.9.1/x64/lib/python3.9/site-packages/sphinx/util/inspect.py", line 341, in safe_getattr
    raise AttributeError(name) from exc
AttributeError: should_split_body_explode

should_split_body_explode was renamed to should_split_line in #1963, and I guess /docs/reference/reference_functions.rst wasn't updated.

[jayaddison]

@TBBle Out of interest, did you spot the newline handling at

black/src/black/__init__.py

Lines 1116 to 1117 in cd4295d

	if not src_txt.endswith("\n"):
	src_txt += "\n"

while preparing your changes here? I don't think they're directly related, but wondered if we might be able to remove that special case at some point.

[TBBle]

That code looks more like it's related to the parser driver requiring newline-terminated text, which I think is probably part of the API contract.