golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
122.87k stars 17.52k forks source link

cmd/gofmt: inconsistent spacing on slice indices #11497

Open tgrosinger opened 9 years ago

tgrosinger commented 9 years ago

What version of Go are you using (go version)? go version go1.4.2 linux/amd64 What operating system and processor architecture are you using? Ubuntu 14.04 What did you do? Run gofmt on below code examples What did you expect to see? Consistent spacing of slice indices

These are two lines within the same function. They vary slightly, but the interesting part is the [1:len(key-1] which has inconsistent spacing. Adding spaces to the first one will result in them being removed when gofmt is run, and removing the spaces from the second will only have them added back.

If this is not a bug, please let me know what difference here is causing the spacing to change.

return "?" + key[1:len(key)-1]
key = key[1 : len(key)-1]
adg commented 9 years ago

It's working as intended. There's a good explanation here.

We like regularity too, but we like readability more, and the few breaks from regularity in gofmt are for things that we felt, after over a year of writing Go programs, enhanced readability enough to justify breaking from the simpler rules.

In this specific case, 1<<x + y is much clearer as to its meaning than 1 << x + y, because it emphasizes the precedence.

Your example - "x && rand.Int()%RAND_FACT" - is not the whole story: you lifted a fragment. The real expression would have been something like

x && rand.Int()%RAND_FACT != 0 

since a % cannot produce a bool and && requires one. If the op were & instead of %, like in

x && rand.Int()&RAND_MASK != 0 

then I appreciate the visual reinforcement that it's being parsed the way I intend (i.e. it has a different meaning here than it would in C).

Especially because operator precedence is a notorious source of confusion in C and because Go changed the precedences to simplify things, we think this is an important way to help programmers adjust.

tgrosinger commented 9 years ago

Hmm, I understand that there may be times where one formatting is better than another. But in this case the core piece of the line is exactly the same in both cases. Is there a more specific reasoning that causes this particular difference?

griesemer commented 9 years ago

@tgrosinger You are correct - they should format the same:

1) The spacing changes when going from http://play.golang.org/p/ToSuqHfqWu to http://play.golang.org/p/rVyw-A9om1 - the difference is just the added parentheses around the slice expression. However, the slice expression already introduces a form of parentheses (square brackets), so inside there shouldn't be a change.

2) There are long-standing TODOs for exactly this issue in go/printer:776 (also line 769).

The fix is trivial, but in the interest of not introducing lots of changes over huge amounts of code, we haven't pushed it through, and I suggest leaving it as is for now.

That said, I've been working on a new version of gofmt (long overdue, hopefully after 1.5 I can concentrate on finishing it) which should solve many of the open gofmt issues. This may be a good time to address this one as well.

tgrosinger commented 9 years ago

Good to know, thank you.