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: add missing space if any in beginning of comment #30540

Closed
santhosh-tekuri opened this issue Mar 3, 2019 · 4 comments
Closed

cmd/gofmt: add missing space if any in beginning of comment #30540

santhosh-tekuri opened this issue Mar 3, 2019 · 4 comments

Comments

@santhosh-tekuri
Copy link
Contributor

@santhosh-tekuri santhosh-tekuri commented Mar 3, 2019

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

$ go version
go version go1.10.3 windows/amd64

Does this issue reproduce with the latest release?

yes

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

go env Output
$ go env
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\rajag\AppData\Local\go-build
set GOEXE=.exe
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=C:\Users\rajag\go
set GORACE=
set GOROOT=C:\Installations\Go
set GOTMPDIR=
set GOTOOLDIR=C:\Installations\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\rajag\AppData\Local\Temp\go-build212850038=/tmp/go-bu
ild -gno-record-gcc-switches

What did you do?

https://play.golang.org/p/N4WdI_Bjdpq

What did you expect to see?

space in beginning of comment

type employee struct {
	name  string // name
	age   int    // age
	email string // email
}

What did you see instead?

type employee struct {
	name  string //name
	age   int    //age
	email string // email
}
@agnivade
Copy link
Contributor

@agnivade agnivade commented Mar 3, 2019

A similar proposal here #27949 which has been rejected. On the same vein, since the language spec does not say anything about what goes inside comments, I don't think gofmt should mess with it.

There are also valid comments like //go:noescape, //go:noinline, //go:linkname which do not have a preceding space.

/cc @griesemer for decision.

@agnivade agnivade changed the title gotfmt: add missing space if any in beginning of comment cmd/gofmt: add missing space if any in beginning of comment Mar 3, 2019
@griesemer
Copy link
Contributor

@griesemer griesemer commented Mar 5, 2019

What @agnivade said. Also, what about comments such as ////////// (some people like that) or //-------- etc.? Where do we draw the line with extra rules?

I agree that it would be nice if gofmt would "clean up" messy comments. In fact it already does try to align stars in comments such as

/* bla bla bla
 * ...
 * ...
 */

but it doesn't always work and it's not clear it was a good idea in the first place. I'm hesitant to interfere more with the contents of comments at this point.

Thanks for the suggestion, but I'd rather not add another somewhat ad-hoc heuristic. There will be just too many exceptions to the rule.

@griesemer griesemer closed this Mar 5, 2019
@santhosh-tekuri
Copy link
Contributor Author

@santhosh-tekuri santhosh-tekuri commented Mar 6, 2019

I think //go:noescape, //go:noinline, //go:linkname inside method and struct fields
are not treated as compiler pragmas.
in such comments we can add missing space

@agnivade
Copy link
Contributor

@agnivade agnivade commented Mar 6, 2019

As @griesemer has already said, there are too many ways this can go wrong. Different people have different preferences for comments. And adding special handling inside method and struct fields would unnecessarily add more complexity.

@golang golang locked and limited conversation to collaborators Mar 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.