code review 4589056: go/printer, gofmt: print "select {}" on one line

go/printer, gofmt: print "select {}" on one line

No impact on existing sources under src, misc.

Fixes issue 1959.

[gri, 2011-06-15 18:17:14]

Hello r@golang.org (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[rsc, 2011-06-15 20:39:40]

http://codereview.appspot.com/4589056/diff/4001/src/pkg/go/printer/testdata/s...
File src/pkg/go/printer/testdata/statements.golden (right):

http://codereview.appspot.com/4589056/diff/4001/src/pkg/go/printer/testdata/s...
src/pkg/go/printer/testdata/statements.golden:119: select {	/* comment */
what's going on here?  why is this comment so much farther out than the one on
the previous select?  they seem like identical code.

[r2, 2011-06-15 21:30:52]

LGTM

[gri, 2011-06-15 21:31:45]

On Wed, Jun 15, 2011 at 1:39 PM,  <rsc@golang.org> wrote:
>
>
http://codereview.appspot.com/4589056/diff/4001/src/pkg/go/printer/testdata/s...
> File src/pkg/go/printer/testdata/statements.golden (right):
>
>
http://codereview.appspot.com/4589056/diff/4001/src/pkg/go/printer/testdata/s...
> src/pkg/go/printer/testdata/statements.golden:119: select {     /* comment
> */
> what's going on here?  why is this comment so much farther out than the
> one on the previous select?  they seem like identical code.

investigating. the gofmt output looks correct (but there's a
post-processing phase).

>
> http://codereview.appspot.com/4589056/
>

[gri, 2011-06-15 22:48:18]

On Wed, Jun 15, 2011 at 1:39 PM,  <rsc@golang.org> wrote:
>
http://codereview.appspot.com/4589056/diff/4001/src/pkg/go/printer/testdata/s...
> src/pkg/go/printer/testdata/statements.golden:119: select {     /* comment
> */
> what's going on here?  why is this comment so much farther out than the
> one on the previous select?  they seem like identical code.
>
> http://codereview.appspot.com/4589056/

When turning on debugging output for tabwriter, |'s indicate tabs. In
this case, the output looks like:

// Formatting of selected select statements.
func _() {
---
	|select {}
	|select { /* comment */
---
	|}
---
	|select {	|/* comment */
---
	|}
---
	|select {
---
	|case <-c:
---
	|}
---
}

(The --- indicate the end of a sequence of lines with synchronized columns).

The 3rd select has a tab before the comment while the 2nd one doesn't.
The reason is the code in printer.go:312 ff:

		// make sure there is at least one separator
		if !hasSep {
			if pos.Line == next.Line {
				// next item is on the same line as the comment
				// (which must be a /*-style comment): separate
				// with a blank instead of a tab
				p.write([]byte{' '})
			} else {
				p.write(htab)
			}
		}

The difference between the 2nd and the 3rd select statement is that in
the 2nd, the closing } is on the same line, while in the 3rd, the
closing } is on the next line in the _input_ file. Usually, there is a
tab before a comment (for alignment), this is the case in the 3rd
select. If the comment is a /*-style comment with more stuff on the
same line (at least in the source), the comment is not aligned - thus
no tab. This explains the difference in the output in this case. When
running through gofmt, the difference visible here is gone because
gofmt uses spaces for alignment and so it works out. If the space on
printer.go:318 is replaced with a tab, both cases look the same (but
then other code doesn't look right).

Perhaps the code in printer should make the tab vs non-tab decision
based on the output rather than the input, but that makes things more
complicated (requires look-ahead in the whitespace buffer used by
printer) and in practice (for gofmt) it doesn't matter.

So what we see is expected according to the code, but not obvious.
I'll add a comment to the input.

-gri

[gri, 2011-06-15 22:53:52]

Hello r@golang.org, rsc@golang.org, r@google.com (cc:
golang-dev@googlegroups.com),

Please take another look.

[r, 2011-06-16 01:15:24]

LGTM

[gri, 2011-06-16 04:47:33]

*** Submitted as http://code.google.com/p/go/source/detail?r=c1b037834a2d ***

go/printer, gofmt: print "select {}" on one line

No impact on existing sources under src, misc.

Fixes issue 1959.

R=r, rsc, r
CC=golang-dev
http://codereview.appspot.com/4589056