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: remove leading/trailing blank lines from function bodies #6996

Open
josharian opened this issue Dec 21, 2013 · 15 comments
Open

cmd/gofmt: remove leading/trailing blank lines from function bodies #6996

josharian opened this issue Dec 21, 2013 · 15 comments
Assignees
Labels
Milestone

Comments

@josharian
Copy link
Contributor

@josharian josharian commented Dec 21, 2013

What steps will reproduce the problem?

1. http://play.golang.org/p/Np4vGQFcqA
2. Hit Format.


What is the expected output? What do you see instead?

Want: Lines 6 and 13 removed.
Have: No change.


Please use labels and text to provide additional information.

Discussed with the community in
https://groups.google.com/forum/#!topic/golang-nuts/Z_s_tLTazJU

Based on that feedback, I propose that gofmt:

* remove trailing blank lines from function bodies
* remove leading blank lines when the function declaration is a single line

If this (or some variant thereof) gets an official blessing, I'd be happy to implement.
@calmh
Copy link
Contributor

@calmh calmh commented Dec 22, 2013

Comment 1:

I sometimes write block comments for functions like http://play.golang.org/p/Nsovaj5c7j.
I find the leading blank line helps separate it from the code, much like I would have a
leading blank line before a block comment when not at the head of a function. It also
makes it easier to use my editors wrap paragraph functionality. I would be slightly to
somewhat annoyed if gofmt removed the line.

Loading

@josharian
Copy link
Contributor Author

@josharian josharian commented Dec 27, 2013

Comment 2:

How about just removing trailing blank lines? No one yet on golang-nuts or here seems to
be opposed to the idea. (And they drive me crazy.)

Loading

@eaigner
Copy link
Contributor

@eaigner eaigner commented Jan 8, 2014

Comment 3:

#1 the doc is supposed to go above the function declaration anyway

Loading

@griesemer
Copy link
Contributor

@griesemer griesemer commented Jan 17, 2014

Comment 4:

At the moment, gofmt doesn't distinguish between a function body and a block inside a
function (for the purpose of these empty lines) - though it could. At least for inner
blocks, sometimes it is desired to have extra empty lines as in:
if ... {
   a lot of code here
   a lot of code here
   a lot of code here
   a lot of code here
} else {
   a lot of code here
   a lot of code here
   a lot of code here
   a lot of code here
}
to highlight the structure of the if.

Labels changed: added release-none, repo-main.

Owner changed to @griesemer.

Status changed to Thinking.

Loading

@mmwtsn
Copy link

@mmwtsn mmwtsn commented Jul 20, 2018

Thoughts on this one now that some time has passed @griesemer?

I am still surprised when gofmt leaves leading/trailing newlines in function bodies.

It's not a huge deal but removing them seems consistent with other gofmt newline behavior.

Loading

@mvdan
Copy link
Member

@mvdan mvdan commented Jul 20, 2018

@calmh this reply is almost four years late, but have you considered doing:

func Foo() int {
        //
	// This function frobbles integers into other integers by means of
	// a quux filter. The filter is five legged.

I have seen a similar technique used in the Go tree, to have function godocs that leave an empty trailing line, yet are still attached to the function:

// Foo does amazing stuff, and this godoc is very long.
//
func Foo() int {

Although it does seem safer to start by just removing trailing empty lines. A separate issue could be opened about leading empty lines.

Loading

@calmh
Copy link
Contributor

@calmh calmh commented Jul 20, 2018

Looking at my code I don't appear to write comments like that any more. This could be an effect of my editor becoming smarter at paragraph-wrapping just the comment and not the adjacent code, or my taste just evolved. Regardless, I no longer care. :)

Loading

@PSUdaemon
Copy link

@PSUdaemon PSUdaemon commented Jul 24, 2018

It would be great to make sure there is one new line between things like imports and const and top level var and functions definitions too.

Loading

@griesemer
Copy link
Contributor

@griesemer griesemer commented Aug 6, 2018

@mmwtsn Sorry, this has not been a priority. I agree it would be nice to fix this.

Loading

@griesemer
Copy link
Contributor

@griesemer griesemer commented Nov 9, 2018

Marked for 1.13 so this gets some attention.

Loading

@AlekSi
Copy link
Contributor

@AlekSi AlekSi commented Apr 30, 2019

Cross-reference to @mvdan's https://github.com/mvdan/gofumpt that does that (among other nice things).

Loading

@griesemer griesemer removed this from the Go1.13 milestone May 6, 2019
@griesemer griesemer added this to the Go1.14 milestone May 6, 2019
@Eun
Copy link

@Eun Eun commented Jun 27, 2019

Shameless plug here for my tool I have written in the past:

https://github.com/Eun/goremovelines

Loading

@gabroo
Copy link

@gabroo gabroo commented Oct 12, 2021

Any chance of this being implemented soon?

Loading

@griesemer
Copy link
Contributor

@griesemer griesemer commented Oct 12, 2021

@mvdan for input. The Go team is very much busy with the 1.18 generics release and unlikely to have time for this in this round.

Loading

@mvdan
Copy link
Member

@mvdan mvdan commented Oct 12, 2021

If anyone wants this feature today, I'd suggest to look at third party tools that build on top of gofmt, like gofumpt. It's very unlikely that we'll add features to gofmt this cycle, like Robert says.

As for adding this feature to gofmt in the future - I'd certainly be in favor. The heuristic we've got in gofumpt has been tweaked over the past couple of years, and we're still finding bugs in it today. I'd be happy to contribute an adapted version of that code to upstream Go if we agree that its logic is sane. It's essentially:

  1. Trailing empty lines are always removed (like Josh said)
  2. Leading lines are removed, unless they help readability in the case below:
func multilineResults() (p1 string,
	p2 string) {

	println("body")
}

That is, if the function declaration ends on the same indentation level as the body, preserve one existing empty line as separation.

In my opinion it's better to instead write it with an unindented line, but I think forcing the user to do that is unnecessary:

func multilineResults() (p1 string,
	p2 string,
) {
	println("body")
}

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet