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: incorrect comment placement for a comment following a function #4381

Closed
griesemer opened this issue Nov 13, 2012 · 10 comments
Closed

cmd/gofmt: incorrect comment placement for a comment following a function #4381

griesemer opened this issue Nov 13, 2012 · 10 comments
Assignees
Milestone

Comments

@griesemer
Copy link
Contributor

@griesemer griesemer commented Nov 13, 2012

gofmt of:

package p

// foo
func f() {
}
// bar

produces

package p

// foo
func f() {
}

// bar

The // bar comment should probably remain adjacent to the function if it was so in the
beginning. Need to investigate.

(This is an issue with go present: when using // START and // STOP markers, after gofmt,
there is an extra line between a function and and a // STOP marker. The result is an
extra line in the code box when running present.
@rsc
Copy link
Contributor

@rsc rsc commented Dec 9, 2012

Comment 1:

Labels changed: added priority-later, removed priority-triage.

@speter
Copy link

@speter speter commented Dec 9, 2012

Comment 2:

IIUC, "// foo" remains attached to the function just because is treated as a godoc
comment. The convention is that comments precede the function definition, so I would not
call the current behavior "incorrect".
Since in this use case it is not really a godoc comment, I'd suggest considering a
different convention for go present that does not interfere with the pre-existing godoc
one. (E.g. "// START" and "// STOP" surrounded by blank lines.) Alternatively, if godoc
is never to be used with go present source files, having different gofmt modes might be
an option as well (distinguished via a command line option or some marker in the source).
If the gofmt behavior is to be changed universally, it should be designed such that both
of these are handled correctly, which does not seem straightforward to me:
// START
func f() {
}
// END
// START
func g() {
}
// END
...
// Function f blahblahblah...
// blahblahblah...
func f() {
}
// Function g blahblahblah...
// blahblahblah...
func g() {
}
...
Just my two cents.
@rsc
Copy link
Contributor

@rsc rsc commented Dec 9, 2012

Comment 3:

I don't think it's worth bothering with 
// start
func f() {}
// end
either, but if we do we would certainly have to disable the logic for
// start
func f() {}
// end
func g() {}
Thanks for the reminder.
@rsc
Copy link
Contributor

@rsc rsc commented Dec 10, 2012

Comment 4:

Labels changed: added size-m.

@rsc
Copy link
Contributor

@rsc rsc commented Mar 11, 2013

Comment 5:

Go 1.1 is too close. Postponing.

Labels changed: added go1.2, removed go1.1.

@rsc
Copy link
Contributor

@rsc rsc commented Sep 9, 2013

Comment 6:

Labels changed: added go1.3, removed go1.2.

@rsc
Copy link
Contributor

@rsc rsc commented Dec 4, 2013

Comment 7:

Labels changed: added release-go1.3.

@rsc
Copy link
Contributor

@rsc rsc commented Dec 4, 2013

Comment 8:

Labels changed: removed go1.3.

@rsc
Copy link
Contributor

@rsc rsc commented Dec 4, 2013

Comment 9:

Labels changed: added repo-main.

@griesemer
Copy link
Contributor Author

@griesemer griesemer commented Feb 25, 2014

Comment 10:

Changing the behavior for comments following a declaration will indeed require
significantly more logic to do the "right thing". Leaving this one alone.

Status changed to WorkingAsIntended.

@griesemer griesemer self-assigned this Feb 25, 2014
@rsc rsc added this to the Go1.3 milestone Apr 14, 2015
@rsc rsc removed the release-go1.3 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
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.