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: aligns type spec alias = with type spec non-alias rhs expressions #24302

Closed
willfaught opened this issue Mar 7, 2018 · 5 comments
Closed

Comments

@willfaught
Copy link
Contributor

@willfaught willfaught commented Mar 7, 2018

Please answer these questions before submitting your issue. Thanks!

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

go version go1.10 darwin/amd64

Does this issue reproduce with the latest release?

Yes

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

GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/willfaught/Library/Caches/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/willfaught/Developer/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.10/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.10/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
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"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/_1/ggvd2t1x7hz_185crsb36zlr0000gp/T/go-build335444054=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

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

What did you expect to see?

type (
	T1              int
	T222222222222 = int
	T3              int
)

What did you see instead?

type (
	T1            int
	T222222222222 = int
	T3            int
)

I can see the argument for doing it the current way, in that it might seem at a glance (incorrectly) that all types were aliases if done the other way. Maybe. But to my eyes it also just...looks a little weird. I searched the issues and didn't see anything open or closed related to this, so I thought I would bring this up just in case it's a mistake. Sorry if it's not.

@andybons andybons changed the title go/format: aligns type spec alias = with type spec non-alias rhs expressions cmd/gofmt: aligns type spec alias = with type spec non-alias rhs expressions Mar 7, 2018
@andybons
Copy link
Member

@andybons andybons commented Mar 7, 2018

Sorry, gofmt is not soliciting change requests on its format at this time. Gofmt made a set of choices at one point, and everybody agrees to live with them, even if everybody might take issue with different individual choices. (as Rob says: https://www.youtube.com/watch?v=PAAkCSZUG1c&t=8m43s) For Go 2, @griesemer may trawl through the issue tracker for all old gofmt style request bugs.

@andybons andybons closed this Mar 7, 2018
@cespare
Copy link
Contributor

@cespare cespare commented Mar 7, 2018

Gofmt made a set of choices at one point

Yes, but that was many years before type aliases existed :)

@andybons
Copy link
Member

@andybons andybons commented Mar 7, 2018

That’s true but the formatting follows what var groupings does:

var (
	v1            int
	v222222222222 = 42
	v3            int
)

So it’s consistent. Changing one (type grouping) would arguably warrant changing the other (var groupings), which we aren’t planning to do.

@willfaught
Copy link
Contributor Author

@willfaught willfaught commented Mar 7, 2018

Sorry, gofmt is not soliciting change requests on its format at this time. Gofmt made a set of choices at one point, and everybody agrees to live with them, even if everybody might take issue with different individual choices. (as Rob says: https://www.youtube.com/watch?v=PAAkCSZUG1c&t=8m43s) For Go 2, @griesemer may trawl through the issue tracker for all old gofmt style request bugs.

This reads like you're referring to a years-old policy, so please correct me if that's wrong, but the 1.10 release contradicts this position. Not only did it change the gofmt format:

Two minor details of the default formatting of Go source code have changed. First, certain complex three-index slice expressions previously formatted like x[i+1 : j:k] and now format with more consistent spacing: x[i+1 : j : k]. Second, single-method interface literals written on a single line, which are sometimes used in type assertions, are no longer split onto multiple lines.

but the release notes go on to say that gofmt changes are to be expected from time to time:

Note that these kinds of minor updates to gofmt are expected from time to time.

And then discuss the ramifications:

In general, we recommend against building systems that check that source code matches the output of a specific version of gofmt. For example, a continuous integration test that fails if any code already checked into a repository is not “properly formatted” is inherently fragile and not recommended.

Was there a policy change after 1.10 shipped that I missed?


So it’s consistent. Changing one (type grouping) would arguably warrant changing the other (var groupings), which we aren’t planning to do.

Ah, good point! Seems clear it's intentional, then. Makes sense to me to keep this closed.

@andybons
Copy link
Member

@andybons andybons commented Mar 7, 2018

I should have worded it a bit differently.

If a change is small, then the proper mechanism would be to propose a change to gofmt via the proposal process.

But as this would carry over into other aspects separate from type aliases, I feel it’s too large a change to do at the moment for the reasons stated above.

As noted, @griesemer may look through old gofmt bugs for Go 2. When things get started in earnest I encourage you to re-open the issue. It’s just too early at this point.

@golang golang locked and limited conversation to collaborators Mar 7, 2019
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.