Go: cmd/gofmt: doesn't honor line endings of Windows \r\n terminated source files

[pbennett]

This is present in all current versions of gofmt.
Issue #2242 referenced it, but I was asked to create a new issue for it by Robert.
Robert, we spoke about this at Gophercon to help jog your memory.

Basically, with a \r\n terminated .go file, gofmt will rewrite the entire file to be \n terminated instead. Windows systems use this by default (so this isn't the place for a one-true EOL discussion please) and gofmt should honor the format of the source file. Changing the line endings has a huge impact in regards to source control systems let alone some native windows tools not handling it well.

To keep it simple, I would suggest just using the line terminator from the first line as the hint for the entire file. Determine the EOL chars from the first line - apply on all future writes.

[pbennett]

@griesemer Mentioning you in comment to bring you in... I'm not sure how to assign to you directly.

[bradfitz]

I am strongly opposed to changing this. The whole point of gofmt is to have ONE format. The choice of "\n" vs "\r\n" is no different from the choice between spaces and tabs. gofmt made the decision already.

[pbennett]

Line endings are a platform issue. If you want to support Windows, you need to accept the fact that it is different. Comparing line-endings to the tabs vs. space issues is incorrect. They aren't the same issue, at all. Some Windows tools don't properly handle LF terminated files. None care about tabs or spaces.
gofmt should not be the hammer used to drive home a personal 'one true EOL' battle.
Would I like everything to use the same line endings? Sure, but that's not the reality.
This is fundamentally a Windows platform support issue, and one where Go is not being a good citizen.

[dominikh]

huge impact in regards to source control systems

Can you expand on this?

Some Windows tools don't properly handle LF terminated files

Which relevant tools don't? Only notepad.exe comes to mind, which hardly counts as relevant.

[bradfitz]

Breaking notepad.exe is unfortunate, but I think it's the lesser evil compared to having gofmt not have consistent output.

The only thing that's changed in the 5 years since we last had this conversation in #2242 and on the mailing list is that more Windows editors now support NL-only files.

[pbennett]

This is also an issue with source control systems. Line endings will be checked out based on the local system and expected to be in the local system format - possibly converting to a common format internally. Variations abound in how this is handled by Git, Perforce, etc. So having a file checked in as CR/LF and gofmt changing it to LF _is_ a problem. It is changing the file in a way that can cause checkin problems.
Again, the standard Windows EOL terminator is \r\n - and gofmt should honor that when present.

[griesemer]

I think @bradfitz makes a strong point - we really want to just have one format. The CR/LF vs LF only differences caused a lot of pain in the early days. The language spec was even adjusted to accommodate for that (e.g. CRs are stripped from raw strings https://golang.org/ref/spec#String_literals).

We also know that a huge number of Go developers work on Windows (the majority?), so I'd like to hear what other Windows users have to say here. Specifically, is the fact that gofmt strips CRs from .go files a problem when interacting with whatever version control system is used?

My inclination is that the right approach here might be for an organization that works exclusively in a CR/LF environment to make their own customized gofmt. It's trivial to to that (the cmd/gofmt command essentially uses the go/format package which produces a string containing the formatted source. It's a simple loop to introduce CR\LFs as necessary.)

[alexbrainman]

I don't think I can be considered as average Windows developer. But I never had any problems with Go source files having LF instead of CR/LF.

All standard Go repositories use .gitattributes file (see https://github.com/golang/go/issues/9281#issuecomment-66839988 for details) to make git treat all source files as binaries. This helps with "LF instead of CR/LF" problem. Is that correct approach? Is it suitable for others? I am not sure. But works fine for Go Team.

I suppose we need to understand what the real problem here is. Then we can decide what the solution is.

Alex

[gopherbot]

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please reopen if this is a mistake or you have the requested information.)

[dlwyatt]

IMO, the tools should support two things:

• Normalizing the file contents before comparison, so it doesn't treat EOL-only differences as something "wrong"
• Writing the file contents back to disk with CRLF line endings _if those were detected in the original source file_.

This should make the tooling friendly to all dev environments, regardless of the source control tools / settings used (such as git's autocrlf.)

[dlwyatt]

I opened a PR to do add both of those features to goimports (https://go-review.googlesource.com/c/tools/+/83535) , but was directed here for "policy discussion".

[bradfitz]

@dlwyatt, what breaks for you on Windows if you keep the files with only LF line endings?

[dlwyatt]

Is that relevant? The argument isn't whether anyone should have CRLF endings, it's whether go's tooling should work well when people do. The compiler doesn't care about the CRLF, why should gofmt / goimports?

[bradfitz]

I believe it is relevant. You've said there's a problem and are proposing a solution, so we want to understand what the problem is first, before evaluating the solution.

[griesemer]

@dlwyatt The compiler only consumes a text file; it doesn't produce one. But gofmt is in the business of standardizing textual output. So this comparison doesn't quite hold up.

The question is relevant because it's not obvious what to do with line endings if there are multiple options. What should be done if a file mixes CRLF and LF endings? What about CRLF's in comments? Note also that the compiler discards CRs in raw string literals (per the spec https://golang.org/ref/spec#String_literals).

It gets messy pretty quickly. If LF works, why make it more complicated?

[dlwyatt]

If you're going to force LF-only via tooling, you may as well make the compiler puke if it comes across CRLF.

[dlwyatt]

Excluding the argument about producing versus consuming for a moment: if you run them with the -l switch, they'll report files as wrong even if the line endings are the only difference. That's a consume-only bit of functionality that still doesn't work on a file that the compiler treats as legal.

[griesemer]

@dlwyatt We certainly don't want to complain if the compiler sees a CRLF; that would put an unnecessary burden on people to ensure their editors behave in just the one way.

But I hear your comment regarding the -l flag. Perhaps CRLF's and LF's should be considered equal for that purpose. Would that alleviate your concerns?

[dlwyatt]

Mostly. At least then if a file were to be modified, it would have a valid content change (in addition to having its line endings wiped out and replaced.) It's still a bit annoying to have the git line ending warnings show up, but not the end of the world.

[bradfitz]

It's still a bit annoying to have the git line ending warnings show up, but not the end of the world.

Where was that mentioned?

Again, please discuss problems. We're much more receptive to discussing problems than discussing solutions to unstated problems.

[griesemer]

@dlwyatt Since this is a closed issue, file a new issue with the concrete problem ("-l considers CRLF and LF line endings as different")?

[dlwyatt]

Sorry, I thought I had posted a bit of console output, but that was over in ptimports (https://github.com/palantir/checks/pull/59)

Looks like yes, goimports also inserts plain newlines:

C:\Users\dwyatt\Documents\Git\go\src\github.com\palantir\checks\ptimports [windows-support ≡]
λ  goimports -w .\ptimports.go
C:\Users\dwyatt\Documents\Git\go\src\github.com\palantir\checks\ptimports [windows-support ≡ +0 ~1 -0 !]
λ  git diff
warning: LF will be replaced by CRLF in ptimports/ptimports.go.
The file will have its original line endings in your working directory.
C:\Users\dwyatt\Documents\Git\go\src\github.com\palantir\checks\ptimports [windows-support ≡ +0 ~1 -0 !]

Never noticed that before because I use VSCode, and apparently it takes care of that already when it runs goimports.

[grokify]

notepad.exe now handles LF as desired:

https://blogs.msdn.microsoft.com/commandline/2018/05/08/extended-eol-in-notepad/

[MyYellowBlanket]

if someone runs 'goreturns -d somefile.go' on windows (in a git-bash since no 'diff' command installed). He probably see the whole file has changed if goreturns doesn't keep the original line ending.