Commit c882f4b6 authored by Robert Griesemer's avatar Robert Griesemer

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: default avatarDaniel Martí <mvdan@mvdan.cc>
Reviewed-by: default avatarAlan Donovan <adonovan@google.com>
parent 469ada6e
...@@ -32,8 +32,10 @@ import ( ...@@ -32,8 +32,10 @@ import (
// Print as many newlines as necessary (but at least min newlines) to get to // 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 // 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 // is set, the first line break is printed as formfeed. Returns 0 if no line
// line break was printed; returns false otherwise. // 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" // TODO(gri): linebreak may add too many lines if the next statement at "line"
// is preceded by comments because the computation of n assumes // is preceded by comments because the computation of n assumes
...@@ -43,7 +45,7 @@ import ( ...@@ -43,7 +45,7 @@ import (
// linebreaks. At the moment there is no easy way to know about // linebreaks. At the moment there is no easy way to know about
// future (not yet interspersed) comments in this function. // 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) n := nlimit(line - p.pos.Line)
if n < min { if n < min {
n = min n = min
...@@ -53,11 +55,12 @@ func (p *printer) linebreak(line, min int, ws whiteSpace, newSection bool) (prin ...@@ -53,11 +55,12 @@ func (p *printer) linebreak(line, min int, ws whiteSpace, newSection bool) (prin
if newSection { if newSection {
p.print(formfeed) p.print(formfeed)
n-- n--
nbreaks = 2
} }
nbreaks += n
for ; n > 0; n-- { for ; n > 0; n-- {
p.print(newline) p.print(newline)
} }
printedBreak = true
} }
return return
} }
...@@ -173,7 +176,7 @@ func (p *printer) exprList(prev0 token.Pos, list []ast.Expr, depth int, mode exp ...@@ -173,7 +176,7 @@ func (p *printer) exprList(prev0 token.Pos, list []ast.Expr, depth int, mode exp
// The first linebreak is always a formfeed since this section must not // The first linebreak is always a formfeed since this section must not
// depend on any previous formatting. // depend on any previous formatting.
prevBreak := -1 // index of last expression that was followed by a linebreak 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 ws = ignore
prevBreak = 0 prevBreak = 0
} }
...@@ -234,14 +237,6 @@ func (p *printer) exprList(prev0 token.Pos, list []ast.Expr, depth int, mode exp ...@@ -234,14 +237,6 @@ func (p *printer) exprList(prev0 token.Pos, list []ast.Expr, depth int, mode exp
useFF = r*ratio <= 1 || r <= ratio 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 needsLinebreak := 0 < prevLine && prevLine < line
if i > 0 { if i > 0 {
...@@ -257,11 +252,20 @@ func (p *printer) exprList(prev0 token.Pos, list []ast.Expr, depth int, mode exp ...@@ -257,11 +252,20 @@ func (p *printer) exprList(prev0 token.Pos, list []ast.Expr, depth int, mode exp
// Lines are broken using newlines so comments remain aligned // Lines are broken using newlines so comments remain aligned
// unless useFF is set or there are multiple expressions on // unless useFF is set or there are multiple expressions on
// the same line in which case formfeed is used. // 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 ws = ignore
prevBreak = i prevBreak = i
needsBlank = false // we got a line break instead 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 { if needsBlank {
p.print(blank) p.print(blank)
...@@ -281,6 +285,11 @@ func (p *printer) exprList(prev0 token.Pos, list []ast.Expr, depth int, mode exp ...@@ -281,6 +285,11 @@ func (p *printer) exprList(prev0 token.Pos, list []ast.Expr, depth int, mode exp
p.expr0(x, depth) p.expr0(x, depth)
} }
if size > 0 {
lnsum += math.Log(float64(size))
count++
}
prevLine = line prevLine = line
} }
...@@ -338,7 +347,7 @@ func (p *printer) parameters(fields *ast.FieldList) { ...@@ -338,7 +347,7 @@ func (p *printer) parameters(fields *ast.FieldList) {
p.print(token.COMMA) p.print(token.COMMA)
} }
// separator if needed (linebreak or blank) // 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 // break line if the opening "(" or previous parameter ended on a different line
ws = ignore ws = ignore
} else if i > 0 { } else if i > 0 {
...@@ -709,7 +718,7 @@ func (p *printer) binaryExpr(x *ast.BinaryExpr, prec1, cutoff, depth int) { ...@@ -709,7 +718,7 @@ func (p *printer) binaryExpr(x *ast.BinaryExpr, prec1, cutoff, depth int) {
if xline != yline && xline > 0 && yline > 0 { if xline != yline && xline > 0 && yline > 0 {
// at least one line break, but respect an extra empty line // at least one line break, but respect an extra empty line
// in the source // in the source
if p.linebreak(yline, 1, ws, true) { if p.linebreak(yline, 1, ws, true) > 0 {
ws = ignore ws = ignore
printBlank = false // no blank after line break printBlank = false // no blank after line break
} }
......
...@@ -128,3 +128,45 @@ func main() { ...@@ -128,3 +128,45 @@ func main() {
abcdefghijklmnopqrstuvwxyz: "foo", 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{},
}
...@@ -128,3 +128,52 @@ func main() { ...@@ -128,3 +128,52 @@ func main() {
abcdefghijklmnopqrstuvwxyz: "foo", 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{},
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment