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

Should Insert Final Newline be on by default? #1551

Open
DavisVaughan opened this issue Oct 11, 2023 · 5 comments
Open

Should Insert Final Newline be on by default? #1551

DavisVaughan opened this issue Oct 11, 2023 · 5 comments
Labels
area: core Issues related to Core category.

Comments

@DavisVaughan
Copy link
Contributor

DavisVaughan commented Oct 11, 2023

Screenshot 2023-10-11 at 9 26 56 AM

RStudio has this on by default and I think it is very helpful. i.e. I was just editing a _pkgdown.yml file and forgot the final new line and saw this when running pkgdown::build_site():

Warning message:
In readLines(con, warn = readLines.warn) :
  incomplete final line found on './_pkgdown.yml'

There is also this secondary option, which I think I like and will opt into, but I don't think we need to turn it on by default

Screenshot 2023-10-11 at 9 29 19 AM
@petetronic
Copy link
Collaborator

While we may want this behavior for our team, it does seem like a decision customer teams will need to make themselves... and probably best left to the existing default from vscode.

@DavisVaughan
Copy link
Contributor Author

DavisVaughan commented Oct 12, 2023

I am realizing that I actually asked for this already (ill close that one)
#726

Interesting that we use this in Positron and amalthea

"files.insertFinalNewline": true,

I do wonder if this has the possibility to pop up for a lot of R developers, since readLines() (a common way to read a file in R) seems like it is going to consistently throw a warning on this.

GitHub will also tell you that a final newline is missing with a red symbol, so I think this is a common convention
image

I also think git will complain

Lots of good reasons for it here too https://stackoverflow.com/questions/5813311/whats-the-significance-of-the-no-newline-at-end-of-file-log

It is also apparently a POSIX standard to end a file with a new line
microsoft/vscode#141169

@lionel-
Copy link
Contributor

lionel- commented Oct 12, 2023

Personally I'm all for changing VS Code defaults for this sort of settings that help developer communities converge towards shared conventions, especially when the convention is so well established.

It looks like pylint also warns about final newlines missing: http://pylint-messages.wikidot.com/messages:c0304

@nealrichardson
Copy link

Maybe this isn't the right way to think about it, but I expect adding a trailing newline to be part of the "format on save" action, that is, not a separate VSCode setting but something controlled by a language module or formatter that is triggered by format-on-save. In VSCode, if you have format-on-save enabled, black will insert a trailing newline in python files, styler will add it in R files, clang-format will for C/C++, and so on. This is all without "Insert Final Newline" enabled.

So if anything, I'd wonder if the right question is if format-on-save should be on by default (I'd advocate for it), and making sure we include the right formatters for Python and R and any other languages we want great out-of-the-box support for.

@kevinushey
Copy link
Contributor

The POSIX standard requires source files to end with a newline. We wouldn't want to encourage users to violate the POSIX standard, would we? 😉

Also, fun fact: if your ~/.Rprofile doesn't have a trailing newline, then the last line / statement will be silently ignored. I think we should protect users from these sorts of insidious issues.

@DavisVaughan DavisVaughan added this to the Release Candidate milestone Feb 22, 2024
@wesm wesm added the area: core Issues related to Core category. label Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: core Issues related to Core category.
Projects
None yet
Development

No branches or pull requests

6 participants