Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
ttSpace opened this issue Oct 12, 2021 · 9 comments · Fixed by #7168
Assignees
Labels
fixed in next version A fix has been implemented an will appear in an upcoming version
Milestone

Comments

@ttSpace
Copy link

ttSpace commented Oct 12, 2021

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

@AdamYoblick
Copy link
Member

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.

@AdamYoblick
Copy link
Member

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.

@int19h
Copy link
Contributor

int19h commented Oct 18, 2021

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.

@AdamYoblick
Copy link
Member

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?

@int19h
Copy link
Contributor

int19h commented Oct 18, 2021

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).

@judej judej added this to the Dev 17.2 milestone Oct 20, 2021
@AdamYoblick AdamYoblick removed their assignment Nov 9, 2021
@ttSpace
Copy link
Author

ttSpace commented Dec 31, 2021

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

@mikeseven
Copy link

same bug in latest (1.70.2)

@AdamYoblick AdamYoblick self-assigned this Aug 31, 2022
@AdamYoblick
Copy link
Member

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

@AdamYoblick
Copy link
Member

AdamYoblick commented Sep 12, 2022

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

autopep8 formats as expected:

autopep8

@AdamYoblick AdamYoblick added the fixed in next version A fix has been implemented an will appear in an upcoming version label Sep 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed in next version A fix has been implemented an will appear in an upcoming version
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants