go/printer: consider empty lines in table layout computation
In previous versions of Go including 1.10, an empty line would break the
alignment of elements within an expression list.
golang.org/cl/104755 changed the heuristic, with the side effect that
empty lines no longer broke the table alignment.
A prior fix (https://go-review.googlesource.com/c/go/+/125260, reverted)
introduced another regression (#26930) which this change doesn't produce.
Added test cases for both #26352 and #26930.
Fixes #26352.
Updates #26930.
Change-Id: I371f48e6f3620ebbab53f2128ec5e58bcd4a62f1
Reviewed-on: https://go-review.googlesource.com/129256
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martà <mvdan@mvdan.cc>
Reviewed-by: Alan Donovan <adonovan@google.com>
diff --git a/src/go/printer/nodes.go b/src/go/printer/nodes.go
index 18f2371..1de7cd8 100644
--- a/src/go/printer/nodes.go
+++ b/src/go/printer/nodes.go
@@ -32,8 +32,10 @@
// Print as many newlines as necessary (but at least min newlines) to get to
// the current line. ws is printed before the first line break. If newSection
-// is set, the first line break is printed as formfeed. Returns true if any
-// line break was printed; returns false otherwise.
+// is set, the first line break is printed as formfeed. Returns 0 if no line
+// breaks were printed, returns 1 if there was exactly one newline printed,
+// and returns a value > 1 if there was a formfeed or more than one newline
+// printed.
//
// TODO(gri): linebreak may add too many lines if the next statement at "line"
// is preceded by comments because the computation of n assumes
@@ -43,7 +45,7 @@
// linebreaks. At the moment there is no easy way to know about
// future (not yet interspersed) comments in this function.
//
-func (p *printer) linebreak(line, min int, ws whiteSpace, newSection bool) (printedBreak bool) {
+func (p *printer) linebreak(line, min int, ws whiteSpace, newSection bool) (nbreaks int) {
n := nlimit(line - p.pos.Line)
if n < min {
n = min
@@ -53,11 +55,12 @@
if newSection {
p.print(formfeed)
n--
+ nbreaks = 2
}
+ nbreaks += n
for ; n > 0; n-- {
p.print(newline)
}
- printedBreak = true
}
return
}
@@ -173,7 +176,7 @@
// The first linebreak is always a formfeed since this section must not
// depend on any previous formatting.
prevBreak := -1 // index of last expression that was followed by a linebreak
- if prev.IsValid() && prev.Line < line && p.linebreak(line, 0, ws, true) {
+ if prev.IsValid() && prev.Line < line && p.linebreak(line, 0, ws, true) > 0 {
ws = ignore
prevBreak = 0
}
@@ -234,14 +237,6 @@
useFF = r*ratio <= 1 || r <= ratio
}
}
- if useFF {
- lnsum = 0
- count = 0
- }
- if size > 0 {
- lnsum += math.Log(float64(size))
- count++
- }
needsLinebreak := 0 < prevLine && prevLine < line
if i > 0 {
@@ -257,11 +252,20 @@
// Lines are broken using newlines so comments remain aligned
// unless useFF is set or there are multiple expressions on
// the same line in which case formfeed is used.
- if p.linebreak(line, 0, ws, useFF || prevBreak+1 < i) {
+ nbreaks := p.linebreak(line, 0, ws, useFF || prevBreak+1 < i)
+ if nbreaks > 0 {
ws = ignore
prevBreak = i
needsBlank = false // we got a line break instead
}
+ // If there was a new section or more than one new line
+ // (which means that the tabwriter will implicitly break
+ // the section), reset the geomean variables since we are
+ // starting a new group of elements with the next element.
+ if nbreaks > 1 {
+ lnsum = 0
+ count = 0
+ }
}
if needsBlank {
p.print(blank)
@@ -281,6 +285,11 @@
p.expr0(x, depth)
}
+ if size > 0 {
+ lnsum += math.Log(float64(size))
+ count++
+ }
+
prevLine = line
}
@@ -338,7 +347,7 @@
p.print(token.COMMA)
}
// separator if needed (linebreak or blank)
- if needsLinebreak && p.linebreak(parLineBeg, 0, ws, true) {
+ if needsLinebreak && p.linebreak(parLineBeg, 0, ws, true) > 0 {
// break line if the opening "(" or previous parameter ended on a different line
ws = ignore
} else if i > 0 {
@@ -709,7 +718,7 @@
if xline != yline && xline > 0 && yline > 0 {
// at least one line break, but respect an extra empty line
// in the source
- if p.linebreak(yline, 1, ws, true) {
+ if p.linebreak(yline, 1, ws, true) > 0 {
ws = ignore
printBlank = false // no blank after line break
}
diff --git a/src/go/printer/testdata/alignment.golden b/src/go/printer/testdata/alignment.golden
index c65defe..96086ed 100644
--- a/src/go/printer/testdata/alignment.golden
+++ b/src/go/printer/testdata/alignment.golden
@@ -128,3 +128,45 @@
abcdefghijklmnopqrstuvwxyz: "foo",
}
}
+
+// ----------------------------------------------------------------------------
+// Examples from issue #26352.
+
+var _ = map[int]string{
+ 1: "",
+
+ 12345678901234567890123456789: "",
+ 12345678901234567890123456789012345678: "",
+}
+
+func f() {
+ _ = map[int]string{
+ 1: "",
+
+ 12345678901234567: "",
+ 12345678901234567890123456789012345678901: "",
+ }
+}
+
+// ----------------------------------------------------------------------------
+// Examples from issue #26930.
+
+var _ = S{
+ F1: []string{},
+ F2____: []string{},
+}
+
+var _ = S{
+ F1: []string{},
+ F2____: []string{},
+}
+
+var _ = S{
+ F1____: []string{},
+ F2: []string{},
+}
+
+var _ = S{
+ F1____: []string{},
+ F2: []string{},
+}
diff --git a/src/go/printer/testdata/alignment.input b/src/go/printer/testdata/alignment.input
index 9b0aae6..323d268 100644
--- a/src/go/printer/testdata/alignment.input
+++ b/src/go/printer/testdata/alignment.input
@@ -128,3 +128,52 @@
abcdefghijklmnopqrstuvwxyz: "foo",
}
}
+
+// ----------------------------------------------------------------------------
+// Examples from issue #26352.
+
+var _ = map[int]string{
+ 1: "",
+
+ 12345678901234567890123456789: "",
+ 12345678901234567890123456789012345678: "",
+}
+
+func f() {
+ _ = map[int]string{
+ 1: "",
+
+ 12345678901234567: "",
+ 12345678901234567890123456789012345678901: "",
+ }
+}
+
+// ----------------------------------------------------------------------------
+// Examples from issue #26930.
+
+var _ = S{
+ F1: []string{
+ },
+ F2____: []string{},
+}
+
+var _ = S{
+ F1: []string{
+
+
+ },
+ F2____: []string{},
+}
+
+var _ = S{
+ F1____: []string{
+ },
+ F2: []string{},
+}
+
+var _ = S{
+ F1____: []string{
+
+ },
+ F2: []string{},
+}
