PTVS icon indicating copy to clipboard operation
PTVS copied to clipboard

An error message "Invalid path mode '\' in: No newline at end of file" pops up when for formatting document.

Open ttSpace opened this issue 3 years ago • 6 comments

Describe the bug

image

Steps to Reproduce

  1. Check Formatter (Tools/Options/Text Editor/Python/Formatting) is black (default)
  2. Type
######this is a comment
import os,sys;
spam( ham [ 1 ], { eggs : 2 } )
def foo    ():pass
x=1;y     =2;
y = 2
#comment no newline
  1. Go to Edit > Advanced >Format Document

Expected behavior

The package is successfully installed and the code is automatically formatted.

Additional context and screenshots

An error message pops up. This issue reproduced when set Tools/Options/Text Editor/Python/Formatting to black and autopep8, do not reproduce when set yapf.

image

ttSpace avatar Oct 12 '21 08:10 ttSpace

I'm on VS 17.0 Preview 6.0.

I installed all three formatters in my global python environment, and I can confirm that this issue repros as reported. Both black and autopep8 throw an error, while yapf does not. Yapf properly formats the file.

I tried adding a newline to the end of the file, and both black and autopep8 now formatted correctly. So this is an easy workaround. This is not our bug to fix, this is a bug in those formatters.

I've opened bugs in the repos for both of those formatters.

  • https://github.com/hhatto/autopep8/issues/617
  • https://github.com/psf/black/issues/2542

AdamYoblick avatar Oct 15 '21 23:10 AdamYoblick

This might be an issue with the way we call these formatters. Leaving this open until I know for sure after speaking with the owners of the tools.

AdamYoblick avatar Oct 16 '21 00:10 AdamYoblick

The reason why this happens is because we call the formatters with a switch to produce a .diff patch, which we then apply to the document (IIRC this is so that it can format without saving). Now if you run Black on a file like that, here's the output you get:

>py -3.9 -m black --diff input.py
--- input.py    2021-10-18 08:22:14.637461 +0000
+++ input.py    2021-10-18 08:23:56.950593 +0000
@@ -1 +1 @@
-adasasd
\ No newline at end of file
+adasasd                                                                                                                                                                                                                       would reformat input.py
All done! ✨ � ✨
1 file would be reformatted. 

and our diff parser doesn't understand how to handle it. The reason why that's there is because of this:

https://www.gnu.org/software/diffutils/manual/html_node/Incomplete-Lines.html#Incomplete-Lines

and this:

https://bugs.python.org/issue2142

So we just need to parse that as spec'd.

int19h avatar Oct 18 '21 08:10 int19h

Ah ok, so you're saying this IS ours to fix?

Jake mentioned that the core extension just sends the contents with a newline appended on if there isn't one. I wanted to test it out in vscode and see what they do.

I was thinking we have a couple options here:

  1. Pop an error message saying that a specific formatter doesn't support files that dont end in a newline (similar to how we pop an error for those that don't support formatting a selection)
  2. "Fix" the file (add a newline) for the purposes of formatting, then remove the newline at the end if there wasn't one before the format.

@int19h You're saying the other option is for us to handle the "no newline at end of file" message more elegantly?

AdamYoblick avatar Oct 18 '21 18:10 AdamYoblick

I don't think we can do the second option. The whole reason why this line occurs in the diff is because Black etc are trying to add a newline there, because that's part of the style guidelines they enforce. So if we strip the newline at the end of the file, we're effectively reverting what Black did (and what the user expected to happen).

What VSCode does sounds more reasonable to me, if they don't strip the newline out after. It does mean that formatters that don't add a newline at EOF will have one added for them, but I doubt anyone would complain about that (it doesn't break anything; not having a newline at the end can actually break some things on Unix).

But, yes, we could also just handle \ in the diff. We don't actually have to parse the message - from what I can tell, it behaves like a comment. It also seems like we could just ignore it for the original lines in the diff, and assume that it never appears in replacement lines (it would only happen if the formatter removes the last newline, which I don't think any of them do).

int19h avatar Oct 18 '21 19:10 int19h

I can repro in today's build 17.1.0 Preview 3.0 [32030.14.main].

ttSpace avatar Dec 31 '21 08:12 ttSpace

same bug in latest (1.70.2)

mikeseven avatar Aug 31 '22 01:08 mikeseven

I've fixed this particular issue, but it looks like we have other issues with the formatting code. I'm investigating.

AdamYoblick avatar Sep 12 '22 20:09 AdamYoblick

black and yapf are both broken in PTVS currently. I'm digging into this.

autopep8 formats as expected:

autopep8

AdamYoblick avatar Sep 12 '22 20:09 AdamYoblick