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: one-field struct with tag on one line should be unchanged #22674

Closed
willfaught opened this issue Nov 12, 2017 · 5 comments
Closed

cmd/gofmt: one-field struct with tag on one line should be unchanged #22674

willfaught opened this issue Nov 12, 2017 · 5 comments
Assignees
Milestone

Comments

@willfaught
Copy link
Contributor

@willfaught willfaught commented Nov 12, 2017

A one-field struct without a tag on one line:

var v struct{ F bool }

...will be unchanged by formatting:

var v struct{ F bool }

However, if you add a tag:

var v struct{ F bool `t` }

...then the formatter breaks it into multiple lines:

var v struct {
    F bool `t`
}

I think it should remain on one line if already that way:

var v struct{ F bool `t` }

A similar request for one-method interfaces was approved: #21952

What did you do?

https://play.golang.org/p/2DxTyl5uji

Wrote:

var v struct{ F bool `t` }

Then formatted.

What did you expect to see?

var v struct{ F bool `t` }

What did you see instead?

var v struct {
    F bool `t`
}

System details

go version go1.9.2 darwin/amd64
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/willfaught/Developer/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.9.2/libexec"
GOTOOLDIR="/usr/local/Cellar/go/1.9.2/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/_1/ggvd2t1x7hz_185crsb36zlr0000gp/T/go-build603510145=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOROOT/bin/go version: go version go1.9.2 darwin/amd64
GOROOT/bin/go tool compile -V: compile version go1.9.2
uname -v: Darwin Kernel Version 17.0.0: Thu Aug 24 21:48:19 PDT 2017; root:xnu-4570.1.46~2/RELEASE_X86_64
ProductName:	Mac OS X
ProductVersion:	10.13
BuildVersion:	17A365
lldb --version: lldb-900.0.57
  Swift-4.0
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Nov 12, 2017

Sorry, the gofmt format is pretty frozen, and personally I think once you had a field tag, there's enough information density there that it should be on its own line, as it already is.

I'm inclined to close this, but I'll let @griesemer decide.

@bradfitz bradfitz added this to the Unplanned milestone Nov 12, 2017
@willfaught
Copy link
Contributor Author

@willfaught willfaught commented Nov 12, 2017

I think the issue is that it’s a personal call, as you said. If an author chooses to put it on one line, then they’ve judged that it’s more readable that way. The formatter doesn’t reformat long lines presumably for this reason.

As @robpike has written (regarding variable names, at least), concision is an important aspect of readability, and one-liners like this are more concise. One-line function declarations are permitted, and useful for sort.Interface implementations, for example. Several of these one-liner structs (with a tag) in a row would similarly be more concise.

The formatter seems to break up lines mostly to eliminate semicolons. If that’s right (please correct me if I’m wrong), then these structs don’t fit that strategy.

@dsnet
Copy link
Member

@dsnet dsnet commented Nov 12, 2017

I should note that it is possible for struct tags to be multiline:

type s struct{ f int `my
multiline
tag` }

Thus, "break up lines mostly to eliminate semicolons" is not the only heuristic the formater follows.

I agree with Brad that there's enough information density to be its own line.

@griesemer
Copy link
Contributor

@griesemer griesemer commented Nov 13, 2017

@willfaught Note that #21952 allowed

interface{ m() }

to be on one line, but not

interface { m() /* foo */ }

the latter continues to be formatted as:

interface {
	m() /* foo */
}

If there's a tag on a struct field or a comment on a struct field or interface method, we want to continue highlighting it by making sure the field or method are on a separate line.

I'm going to close this as working consistently and as expected.

@griesemer griesemer closed this Nov 13, 2017
@willfaught
Copy link
Contributor Author

@willfaught willfaught commented Nov 13, 2017

@dsnet I'd never seen a multi-line tag before. That's interesting.


@griesemer Fair enough. I can see how that's consistent with comments.

For what it's worth, I've never seen a general comment in a struct or interface type literal before, so it's unclear to me how useful that could be. However, one-liners like the following are useful in many ways:

var b, err = json.Marshal(struct{ F bool `json:"f"` }{F: true})

Currently, it would be formatted as:

var b, err = json.Marshal(struct {
    F bool `json:"f"`
}{F: true})

The first is clearer to me.

Thanks, all, for your consideration.

@golang golang locked and limited conversation to collaborators Nov 13, 2018
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
5 participants
You can’t perform that action at this time.