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

gofmt and gofix mangle line endings #2242

Closed
gopherbot opened this issue Sep 9, 2011 · 13 comments
Closed

gofmt and gofix mangle line endings #2242

gopherbot opened this issue Sep 9, 2011 · 13 comments
Milestone

Comments

@gopherbot
Copy link
Contributor

by seanerussell:

Bug, or feature request?  I'm not sure.

Up to weekly.2011-06-23 8876 (b42014761d96), both gofix and gofmt mangle line endings. 
I argue that this is undesirable behavior, since it interferes with developers working
on different platforms.

To reproduce: create a Go source file on Linux, and run gofmt on it.  Take the resulting
file to a Windows machine, and run the exact same version of gofmt on it on Windows.

When run on Windows, gofmt will report that every file needs to be fixed (which is
incorrect), and if told to write the changes, will change the line endings of every line
in the file; consequently, version control systems will think every line has changed.

gofmt should be idempotent when the same version of gofmt is run on the same file on
different platforms.  In other words, gofmt should leave line endings alone.  It's
acceptable for gofmt to choose an EOL style when it has to insert or break lines.
Ideally, gofmt should grab the line ending of the first line in the file and use that
when it has to insert line endings, but otherwise leave line endings alone.

gofix does the same thing, changing the line endings on lines which it otherwise doesn't
touch.  This is slightly less onerous than gofmt's behavior, because gofmt is more of a
desirable candidate to be added as a pre-commit hook, or part of the build chain (for
example).
@adg
Copy link
Contributor

adg commented Sep 9, 2011

Comment 1:

A bug, for sure.
Gofmt should either use one specific type of line ending (\n only, for example), or
leave them alone. If it introduces new lines, it should use whatever the closest line
ending does.

Owner changed to @griesemer.

Status changed to Accepted.

@rsc
Copy link
Contributor

rsc commented Sep 9, 2011

Comment 2:

Related to issue #680.
Gofmt makes files canonical.  I don't think the current
behavior is actually a bug.  We've coddled Windows \r\n
for long enough.  Saying that gofmt should figure out
whether this is a \r\n file or a \n file is like saying it
should figure out whether this is a spaces file or a tabs file.
It's been what, thirty years?  Enough is enough.

@gopherbot
Copy link
Contributor Author

Comment 3 by seanerussell:

I'm not saying that it should assert one line ending over another; I'm saying that these
tools shouldn't mangle the line endings.  The current behavior renders gofmt useless for
people who are sharing code with people on different development environments.  Given a
well-formatted go source file from a different OS, gofmt will (a) _incorrectly_ report
that the file needs to be reformatted (-l), and (b) unnecessarily change the entire file.

@rsc
Copy link
Contributor

rsc commented Sep 9, 2011

Comment 4:

Indeed.  I am saying that it might be okay to
decide that files using \r\n are categorically
not standard gofmt style, implying that gofmt
is correct.  We don't support \r-only on Macs either.
If you are sharing code with people on different
development environments your tools are all
dealing with which line endings to use, uselessly.
On the other hand, if everyone ran gofmt on
checkin, then the files would be in a single canonical
format, with nothing to worry about.  This seems
preferable to making every tool in use anywhere
know about \r\n just because some developers
use Windows.

@peterGo
Copy link
Contributor

peterGo commented Sep 10, 2011

Comment 5:

Unable to reproduce the issue.
"To reproduce: create a Go source file on Linux, and run gofmt on it.  Take the
resulting file to a Windows machine, and run the exact same version of gofmt on it on
Windows."
First, on Linux, amd64, hg id 546f21eebee8 tip:
$ cat 2242.go
package main
import "fmt";
func main() { fmt.Println("Hello, World!"); }
$ stat -c '%n %s %y' 2242.go
2242.go 75 2011-09-10 00:29:01.691300331 -0400
$ gofmt -l -w 2242.go
2242.go
$ stat -c '%n %s %y' 2242.go
2242.go 73 2011-09-10 00:29:37.491276882 -0400
$ cat 2242.go
package main
import "fmt"
func main() { fmt.Println("Hello, World!") }
Second, on Windows, 386, hg id 546f21eebee8 tip:
>type 2242.go
package main
import "fmt"
func main() { fmt.Println("Hello, World!") }
>dir 2242.go
09/10/2011  12:29 AM                73 2242.go
>gofmt -l -w 2242.go
>dir 2242.go
09/10/2011  12:29 AM                73 2242.go
"When run on Windows, gofmt will [NOT] report that every file needs to be fixed (which
is [CORRECT not] incorrect), and if told to write the changes, will [NOT] change the
line endings of every line in the file; consequently, version control systems will [NOT]
think every line has changed."

@adg
Copy link
Contributor

adg commented Sep 12, 2011

Comment 6:

That's interesting. Can the issue reporter comment as to why they observe different
behavior?
Russ, making "\n" canonical sounds fine to me. We could consider adding a flag to gofmt
to specify a different line-ending character, the same way we permit customizing indent.

@adg
Copy link
Contributor

adg commented Sep 12, 2011

Comment 7:

Status changed to WaitingForReply.

@rsc
Copy link
Contributor

rsc commented Dec 9, 2011

Comment 9:

Labels changed: added priority-later.

@rsc
Copy link
Contributor

rsc commented Dec 12, 2011

Comment 10:

Labels changed: added priority-go1.

@robpike
Copy link
Contributor

robpike commented Jan 13, 2012

Comment 11:

Owner changed to builder@golang.org.

@rsc
Copy link
Contributor

rsc commented Jan 24, 2012

Comment 12:

I believe these tools are working as intended, as per comments #2 and #4.

Status changed to WorkingAsIntended.

@gopherbot
Copy link
Contributor Author

Comment 13 by hraban@0brg.net:

there is a problem with comments
hraban@xxx ~/tmp
$ gofmt -d ./test.go.txt
diff ./test.go.txt gofmt/./test.go.txt
--- C:\Users\hraban\AppData\Local\Temp\gofmt642705955   Thu Apr 18 19:52:28 2013
+++ C:\Users\hraban\AppData\Local\Temp\gofmt308162854   Thu Apr 18 19:52:28 2013
@@ -1,7 +1,7 @@
-package man
-
+package man
+
 // walks into a
-
-func bar() string {
-       return "ouch"
-}
+
+func bar() string {
+       return "ouch"
+}
first gofmt takes the liberty to convert all line endings from CRLF to LF. That was
pretty confusing (non-standard) and unobvious behavior but okay, I can live with that.
Problem is that comment lines are left CRLF. That is obviously a problem because now all
other tools think hey this is a unix-style file (eg vim starts displaying ^M after every
comment line).

@davecheney
Copy link
Contributor

Comment 14:

This issue is closed. Please retest your issue with tip or one of the go1.1betas
available from the download site and open a new issue if this remains an open issue for
you.

Labels changed: added restrict-addissuecomment-commit.

@golang golang locked and limited conversation to collaborators Dec 8, 2014
@rsc rsc added this to the Go1 milestone Apr 10, 2015
@rsc rsc removed the priority-go1 label Apr 10, 2015
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants