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

Jump to bottom

cmd/gofmt: inconsistent formatting of struct with comments #29784

Closed
DylanMeeus opened this issue Jan 17, 2019 · 7 comments
Closed

cmd/gofmt: inconsistent formatting of struct with comments #29784

DylanMeeus opened this issue Jan 17, 2019 · 7 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Go1.13

Comments

@DylanMeeus
Copy link

DylanMeeus commented Jan 17, 2019 •
edited

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

$ go version 
go1.11.4 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\dmeeus1\AppData\Local\go-build                                                                                                                                                                                                                             
set GOEXE=.exe                                                                                                                                                                                                                                                                  
set GOFLAGS=                                                                                                                                                                                                                                                                    
set GOHOSTARCH=amd64                                                                                                                                                                                                                                                            
set GOHOSTOS=windows                                                                                                                                                                                                                                                            
set GOOS=windows                                                                                                                                                                                                                                                                
set GOPATH=C:\Development\Go\src                                                                                                                                                                                                                                                
set GOPROXY=                                                                                                                                                                                                                                                                    
set GORACE=                                                                                                                                                                                                                                                                     
set GOROOT=C:\Go                                                                                                                                                                                                                                                                
set GOTMPDIR=                                                                                                                                                                                                                                                                   
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64                                                                                                                                                                                                                                      
set GCCGO=gccgo                                                                                                                                                                                                                                                                 
set CC=gcc                                                                                                                                                                                                                                                                      
set CXX=g++                                                                                                                                                                                                                                                                     
set CGO_ENABLED=1                                                                                                                                                                                                                                                               
set GOMOD=C:\Development\Go\moapr\golang\nexuz\moapr\go.mod                                                                                                                                                                                                                     
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 -fmessage-length=0 -fdebug-prefix-map=C:\Users\dmeeus1\AppData\Local\Temp\go-build162758338=/tmp/go-build -gno-record-gcc-switches

What did you do?

I created a struct that has some comments in it. For example:

type T struct {
   a string
   aMuchLongerName string
   // a comment.
   c int
   delta int

Next I hit format. And it formats each comment section as if they are of different blocks.

Just click on format here:
https://play.golang.org/p/wFOqUymxaBY

What did you expect to see?

That all the types would be aligned throughout the entire struct.

What did you see instead?

Alignment in blocks divided by comments.

PS: Maybe this is intentionally so. I couldn't find a related issue with a quick search.
Thanks for taking a look!

~Dylan
@mvdan
Copy link
Member

mvdan commented Jan 17, 2019

This is indeed working as intended, although gofmt's behavior is not documented in detail. See #26352 (comment), for example. Perhaps the documentation somewhere could be improved, such as https://golang.org/cmd/gofmt/.

/cc @griesemer

@DylanMeeus
Copy link
Author

Thank you :)

Just to give my humble opinion.. it makes the code less readable. See for example: https://play.golang.org/p/T9lxMNkITmz

But I can see the reasoning behind breaking it in "logical blocks" and then formatting those. Hence why I thought it might have been by design ;-)

@mvdan
Copy link
Member

mvdan commented Jan 17, 2019

Indeed, not being able to break down the formatting into blocks would be much worse than what we currently have. One could argue that only empty lines should break alignment, and not comments alone, but I wonder if that could be changed at this point. I'll leave that to Robert.

@agnivade
Copy link
Contributor

Sometimes in my code, when I have large structs, I comment in a separate line to group fields, which creates a new "logical block" of fields.

@DylanMeeus - If you just remove the empty //, I think it is pretty readable and clear.

type T struct {
	a               string
	aMuchLongerName string

	// comment
	c     int
	delta int

	// and another!
	b           int32
	theseAre    string
	LongerAgain string
}

@agnivade agnivade changed the title gofmt inconsistent formatting of struct with comments cmd/gofmt: inconsistent formatting of struct with comments Jan 17, 2019
@agnivade agnivade added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jan 17, 2019
@mvdan
Copy link
Member

mvdan commented Jan 17, 2019

@agnivade I think what we could potentially do, which is what I tried to explain above, is keep the alignment in the scenario where the lines in between fields aren't truly empty:

type Foo struct {
        // F1 is ...
        F1   string
        // FF2 is ...
        FF2  string
        // FFF3 is ...
        FFF3 string
}

Not saying it's definitely a good idea, but it doesn't look unreasonable to me.

@DylanMeeus
Copy link
Author

For what it's worth, I think that's a good idea @mvdan.

How I stumbled on this bug was that I (temporarily) wanted to remove something from a struct, though all fields did logically belong together. But then it grouped the imports because of the commented-out field :-)

Sometimes you'd want to add comments without declaring a new 'logical block', so it makes sense to keep them together. And other times you'd want to split them up.

What @agnivade suggested is a good work-around and keeps the readability of the struct. Though it'd be nice to also have the option to keep all fields as one block, even with comments.

@bradfitz bradfitz added this to the Go1.13 milestone Jan 18, 2019
@DylanMeeus DylanMeeus reopened this Jan 18, 2019
@rsc
Copy link
Contributor

rsc commented May 1, 2019

And it formats each comment section as if they are of different blocks.

You've correctly diagnosed what gofmt does, but that's the intended design. Whole-line // comments introduce new blocks. At this point it is better not to make changes to the algorithm. It is what it is.

@rsc rsc closed this as completed May 1, 2019
@golang golang locked and limited conversation to collaborators Apr 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Milestone
Go1.13
Development

No branches or pull requests
6 participants
@bradfitz @rsc @agnivade @DylanMeeus @mvdan @gopherbot