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

cmd/gofmt: Minor spacing difference in cmd/gofmt formatting between 1.5 and 1.6. #15294

Closed
dmitshur opened this issue Apr 14, 2016 · 7 comments
Closed

Comments

@dmitshur
Copy link
Contributor

As a preface, I am completely okay if this issue is marked as "Unfortunate", I don't think it negatively affects me or anyone, and the chances of other people running into it are very low (and if they do, working around it is trivial). But given how unexpected and rare the event is (a difference in cmd/gofmt behavior between point releases of Go), it may be interesting/useful to investigate the root cause and ensure such regressions don't happen in future releases. I'm reporting it for that reason only.

So far, I have not spent any time looking into why the behavior changed (again, because I didn't think it was worth the time, and there are higher priority Go issues for me to get to first).

1. What version of Go are you using (go version)?

go version go1.6.1 darwin/amd64

2. What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/Dmitri/Dropbox/Work/2013/GoLanding:/Users/Dmitri/Dropbox/Work/2013/GoLand"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GO15VENDOREXPERIMENT="1"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fno-common"
CXX="clang++"
CGO_ENABLED="1"

3. What did you do?

Take the following .go file and gofmt it using cmd/gofmt of Go 1.6 (either 1.6 or 1.6.1). I've done it on OS X, but I think it affects all OSes.

package main

import (
    "fmt"
    "runtime"

    // Some comment.
)

func main() {
    fmt.Println(runtime.Version())
}

(https://play.golang.org/p/iYq3-js1Em)

4. What did you expect to see?

According to Go 1.5 (specifically tested version right now is 1.5.3), there would be no difference, that file is considered "gofmt"ed as is.

$ ~/Downloads/go1.5.3/bin/gofmt -d fmt.go
$ 

5. What did you see instead?

The gofmt of Go 1.6 removes the blank line between the import spec and the line with comment.

$ gofmt -d fmt.go 
diff fmt.go gofmt/fmt.go
--- /var/folders/tw/kgz4v2kn4n7d7ryg5k_z3dk40000gn/T/gofmt204683414 2016-04-13 23:54:20.000000000 -0700
+++ /var/folders/tw/kgz4v2kn4n7d7ryg5k_z3dk40000gn/T/gofmt140905213 2016-04-13 23:54:20.000000000 -0700
@@ -3,7 +3,6 @@
 import (
    "fmt"
    "runtime"
-
    // Some comment.
 )

$ 

Resulting in:

package main

import (
    "fmt"
    "runtime"
    // Some comment.
)

func main() {
    fmt.Println(runtime.Version())
}

Workarounds are to accept the new gofmt behavior and not have the blank line, or simply remove that comment. I ran into this by accident (/cc @neelance), the comment was going to be removed soon thereafter anyway.

@dmitshur dmitshur changed the title cmd/gofmt: Extremely minor but non-zero difference in cmd/gofmt behavior between 1.5 and 1.6. cmd/gofmt: Minor spacing difference in cmd/gofmt formatting behavior between 1.5 and 1.6. Apr 14, 2016
@dmitshur dmitshur changed the title cmd/gofmt: Minor spacing difference in cmd/gofmt formatting behavior between 1.5 and 1.6. cmd/gofmt: Minor spacing difference in cmd/gofmt formatting between 1.5 and 1.6. Apr 14, 2016
@0xmohit
Copy link
Contributor

0xmohit commented Apr 14, 2016

Seems to be a side-effect of https://go-review.googlesource.com/#/c/17440/

@bradfitz bradfitz added this to the Unplanned milestone Apr 14, 2016
@bradfitz
Copy link
Contributor

Over to @griesemer to triage.

@griesemer
Copy link
Contributor

@0xmohit Thanks for investigating this. I probably won't try to address this at the moment - there's more important work to be finished for 1.7. That said, down the road I am hoping to allocate some time to address this and many other open gofmt issues.

@lzl124631x
Copy link

Facing the same issue.

My gofmt on localbox format the code in this way

		log.StdoutHandler,                                     // Log to terminal
		log.Must.FileHandler(logFileName, log.LogfmtFormat()), // Log to file

But the gofmt in Travis gave me this result

-		log.StdoutHandler,                                     // Log to terminal
+		log.StdoutHandler, // Log to terminal
 		log.Must.FileHandler(logFileName, log.LogfmtFormat()), // Log to file

@agnivade
Copy link
Contributor

@lzl124631x - Your issue is slightly different I believe. It was already raised and marked as unfortunate. See #25161.

@lzl124631x
Copy link

@agnivade Thanks. Interesting. Unfortunate 🤣

@seankhliao
Copy link
Member

Seems reasonable to close now that the behaviour has been standard for ~7 years

@golang golang locked and limited conversation to collaborators Jan 28, 2024
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

8 participants