Skip to content

Commit 797d668

Browse files
authored
Implement text wrapping for most diagnostic messages (#446)
This PR adds basic text wrapping to diagnostic messages (except those attached to snippets) to avoid needing to wrap notes/helps by hand when writing diagnostics.
1 parent 95835d8 commit 797d668

24 files changed

+387
-184
lines changed

experimental/parser/testdata/lexer/strings/unclosed2.proto.stderr.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ error: unterminated string literal
33
|
44
1 | '\'
55
| ^^^ expected to be terminated by `'`
6-
= note: this string appears to end in an escaped quote; replace `\'` with `\\''`
6+
= note: this string appears to end in an escaped quote; replace `\'` with
7+
`\\''`
78

89
encountered 1 error

experimental/report/diff.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -87,24 +87,24 @@ func unifiedDiff(span Span, edits []Edit) (Span, []hunk) {
8787

8888
// Partition offsets into overlapping lines. That is, this connects together
8989
// all edit spans whose end and start are not separated by a newline.
90-
prev := &edits[0]
91-
parts := slicesx.Partition(edits, func(_, next *Edit) bool {
92-
if next == prev {
90+
prev := 0
91+
parts := slicesx.SplitFunc(edits, func(i int, next Edit) bool {
92+
if i == prev {
9393
return false
9494
}
9595

96-
chunk := src[prev.End:next.Start]
96+
chunk := src[edits[i-1].End:next.Start]
9797
if !strings.Contains(chunk, "\n") {
9898
return false
9999
}
100100

101-
prev = next
101+
prev = i
102102
return true
103103
})
104104

105105
var out []hunk
106106
var prevHunk int
107-
parts(func(_ int, edits []Edit) bool {
107+
parts(func(edits []Edit) bool {
108108
// First, figure out the start and end of the modified region.
109109
start, end := edits[0].Start, edits[0].End
110110
for _, edit := range edits[1:] {

experimental/report/renderer.go

Lines changed: 35 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,14 @@ import (
1818
"bytes"
1919
"fmt"
2020
"io"
21+
"math"
2122
"math/bits"
2223
"slices"
2324
"strconv"
2425
"strings"
2526
"unicode"
2627

28+
"github.com/bufbuild/protocompile/internal/ext/iterx"
2729
"github.com/bufbuild/protocompile/internal/ext/slicesx"
2830
"github.com/bufbuild/protocompile/internal/ext/stringsx"
2931
)
@@ -186,7 +188,8 @@ func (r *renderer) diagnostic(report *Report, d Diagnostic) {
186188
// For the other styles, we imitate the Rust compiler. See
187189
// https://github.com/rust-lang/rustc-dev-guide/blob/master/src/diagnostics.md
188190

189-
fmt.Fprint(r, r.ss.BoldForLevel(d.level), level, ": ", d.message, r.ss.reset)
191+
fmt.Fprint(r, r.ss.BoldForLevel(d.level), level, ": ")
192+
r.WriteWrapped(d.message, MaxMessageWidth)
190193

191194
locations := make([][2]Location, len(d.snippets))
192195
for i, snip := range d.snippets {
@@ -210,13 +213,14 @@ func (r *renderer) diagnostic(report *Report, d Diagnostic) {
210213
r.margin = max(2, len(strconv.Itoa(greatestLine))) // Easier than messing with math.Log10()
211214

212215
// Render all the diagnostic windows.
213-
parts := slicesx.Partition(d.snippets, func(a, b *snippet) bool {
214-
if len(a.edits) > 0 || len(b.edits) > 0 {
216+
parts := slicesx.PartitionKey(d.snippets, func(snip snippet) any {
217+
if len(snip.edits) > 0 {
215218
// Suggestions are always rendered in their own windows.
216-
return true
219+
// Return a fresh pointer, since that will always compare as
220+
// distinct.
221+
return new(int)
217222
}
218-
219-
return a.Path() != b.Path()
223+
return snip.Path()
220224
})
221225

222226
parts(func(i int, snippets []snippet) bool {
@@ -261,34 +265,33 @@ func (r *renderer) diagnostic(report *Report, d Diagnostic) {
261265
fmt.Fprintf(r, "--> %s", d.inFile)
262266
}
263267

264-
// Render the footers. For simplicity we collect them into an array first.
265-
footers := make([][3]string, 0, len(d.notes)+len(d.help)+len(d.debug))
266-
for _, note := range d.notes {
267-
footers = append(footers, [3]string{r.ss.bRemark, "note", note})
268-
}
269-
for _, help := range d.help {
270-
footers = append(footers, [3]string{r.ss.bRemark, "help", help})
268+
type footer struct {
269+
color, label, text string
271270
}
272-
if r.ShowDebug {
273-
for _, debug := range d.debug {
274-
footers = append(footers, [3]string{r.ss.bError, "debug", debug})
271+
footers := iterx.Chain(
272+
slicesx.Map(d.notes, func(s string) footer { return footer{r.ss.bRemark, "note", s} }),
273+
slicesx.Map(d.help, func(s string) footer { return footer{r.ss.bRemark, "help", s} }),
274+
slicesx.Map(d.debug, func(s string) footer { return footer{r.ss.bError, "debug", s} }),
275+
)
276+
277+
footers(func(f footer) bool {
278+
isDebug := f.label == "debug"
279+
if isDebug && !r.ShowDebug {
280+
return true
275281
}
276-
}
277-
for _, footer := range footers {
282+
278283
r.WriteString("\n")
279-
r.WriteString(r.ss.nAccent)
280284
r.WriteSpaces(r.margin)
281-
r.WriteString(" = ")
282-
fmt.Fprint(r, footer[0], footer[1], ": ", r.ss.reset)
283-
for i, line := range strings.Split(footer[2], "\n") {
284-
if i > 0 {
285-
r.WriteString("\n")
286-
margin := r.margin + 3 + len(footer[1]) + 2
287-
r.WriteSpaces(margin)
288-
}
289-
r.WriteString(line)
285+
fmt.Fprintf(r, "%s = %s%s: %s", r.ss.nAccent, f.color, f.label, r.ss.reset)
286+
287+
if isDebug {
288+
r.WriteWrapped(f.text, math.MaxInt)
289+
} else {
290+
r.WriteWrapped(f.text, MaxMessageWidth)
290291
}
291-
}
292+
293+
return true
294+
})
292295

293296
r.WriteString(r.ss.reset)
294297
r.WriteString("\n\n")
@@ -482,7 +485,7 @@ func (r *renderer) window(w *window) {
482485

483486
// Next, we can render the underline parts. This aggregates all underlines
484487
// for the same line into rendered chunks
485-
parts := slicesx.Partition(w.underlines, func(a, b *underline) bool { return a.line != b.line })
488+
parts := slicesx.PartitionKey(w.underlines, func(u underline) int { return u.line })
486489
parts(func(_ int, part []underline) bool {
487490
cur := &info[part[0].line-w.start]
488491
cur.shouldEmit = true
@@ -517,8 +520,7 @@ func (r *renderer) window(w *window) {
517520

518521
// Now, convert the buffer into a proper string.
519522
var out strings.Builder
520-
parts := slicesx.Partition(buf, func(a, b *byte) bool { return *a != *b })
521-
parts(func(_ int, line []byte) bool {
523+
slicesx.Partition(buf)(func(_ int, line []byte) bool {
522524
level := Level(line[0])
523525
if line[0] == 0 {
524526
out.WriteString(r.ss.reset)
@@ -906,7 +908,7 @@ func (r *renderer) suggestion(snip snippet) {
906908
r.WriteString(r.ss.nAccent)
907909
r.WriteSpaces(r.margin)
908910
r.WriteString("help: ")
909-
r.WriteString(snip.message)
911+
r.WriteWrapped(snip.message, MaxMessageWidth)
910912

911913
// Add a blank line after the file. This gives the diagnostic window some
912914
// visual breathing room.

experimental/report/span.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,6 @@ import (
2727
"github.com/bufbuild/protocompile/internal/iter"
2828
)
2929

30-
// TabstopWidth is the size we render all tabstops as.
31-
const TabstopWidth int = 4
32-
3330
// Spanner is any type with a [Span].
3431
type Spanner interface {
3532
// Should return the zero [Span] to indicate that it does not contribute

experimental/report/testdata/i18n.yaml.color.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
⟨b.red⟩error: emoji, CJK, bidi⟨reset⟩
1+
⟨b.red⟩error: emoji, CJK, bidi
22
⟨blu⟩ --> foo.proto:5:9
33
|
44
⟨blu⟩ 5 | ⟨reset⟩message 🐈<U+200D>⬛ {
@@ -10,7 +10,7 @@
1010
⟨blu⟩ 1 | ⟨reset⟩import "חתול שחור.proto";
1111
⟨blu⟩ | ⟨reset⟩ ⟨b.blu⟩---------------⟨reset⟩ ⟨b.blu⟩bidi works if it's quoted, at least⟨reset⟩
1212

13-
⟨b.red⟩error: bidi (Arabic, Hebrew, Farsi, etc) is broken in some contexts⟨reset⟩
13+
⟨b.red⟩error: bidi (Arabic, Hebrew, Farsi, etc) is broken in some contexts
1414
⟨blu⟩ --> foo.proto:7:10
1515
|
1616
⟨blu⟩ 7 | ⟨reset⟩ string القطة السوداء = 2;

experimental/report/testdata/multi-file.yaml.color.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
⟨b.red⟩error: two files⟨reset⟩
1+
⟨b.red⟩error: two files
22
⟨blu⟩ --> foo.proto:3:9
33
|
44
⟨blu⟩ 3 | ⟨reset⟩package abc.xyz;
@@ -11,7 +11,7 @@
1111
⟨blu⟩ 3 | ⟨reset⟩package abc.xyz2;
1212
⟨blu⟩ | ⟨reset⟩ ⟨b.blu⟩-------⟨reset⟩ ⟨b.blu⟩baz⟨reset⟩
1313

14-
⟨b.red⟩error: three files⟨reset⟩
14+
⟨b.red⟩error: three files
1515
⟨blu⟩ --> foo.proto:3:9
1616
|
1717
⟨blu⟩ 3 | ⟨reset⟩package abc.xyz;

experimental/report/testdata/multi-underline.yaml.color.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
⟨b.red⟩error: `size_t` is not a built-in Protobuf type⟨reset⟩
1+
⟨b.red⟩error: `size_t` is not a built-in Protobuf type
22
⟨blu⟩ --> foo.proto:6:12
33
|
44
⟨blu⟩ 1 | ⟨reset⟩syntax = "proto4"
@@ -8,7 +8,7 @@
88
⟨blu⟩ 6 | ⟨reset⟩ required size_t x = 0;
99
⟨blu⟩ | ⟨reset⟩ ⟨b.red⟩^^^^^⟨reset⟩ ⟨b.red⟩⟨reset⟩
1010

11-
⟨b.ylw⟩warning: these are pretty bad names⟨reset⟩
11+
⟨b.ylw⟩warning: these are pretty bad names
1212
⟨blu⟩ --> foo.proto:3:9
1313
|
1414
⟨blu⟩ 3 | ⟨reset⟩package abc.xyz;

experimental/report/testdata/multiline.yaml.color.txt

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
⟨b.ylw⟩warning: whole block⟨reset⟩
1+
⟨b.ylw⟩warning: whole block
22
⟨blu⟩ --> foo.proto:5:1
33
|
44
⟨blu⟩ 5 | ⟨b.ylw⟩/ ⟨reset⟩message Blah {
55
⟨blu⟩... ⟨b.ylw⟩|
66
⟨blu⟩12 | ⟨b.ylw⟩| ⟨reset⟩}
77
⟨blu⟩ | ⟨b.ylw⟩\_^ this block⟨reset⟩
88

9-
⟨b.ylw⟩warning: nested blocks⟨reset⟩
9+
⟨b.ylw⟩warning: nested blocks
1010
⟨blu⟩ --> foo.proto:5:1
1111
|
1212
⟨blu⟩ 5 | ⟨b.ylw⟩/ ⟨reset⟩message Blah {
@@ -18,7 +18,7 @@
1818
⟨blu⟩12 | ⟨b.ylw⟩| ⟨reset⟩}
1919
⟨blu⟩ | ⟨b.ylw⟩\___^ this block⟨reset⟩
2020

21-
⟨b.ylw⟩warning: parallel blocks⟨reset⟩
21+
⟨b.ylw⟩warning: parallel blocks
2222
⟨blu⟩ --> foo.proto:5:1
2323
|
2424
⟨blu⟩ 5 | ⟨b.ylw⟩/ ⟨reset⟩message Blah {
@@ -31,7 +31,7 @@
3131
⟨blu⟩12 | ⟨b.blu⟩/ ⟨reset⟩}
3232
⟨blu⟩ | ⟨b.blu⟩\_- and this block⟨reset⟩
3333

34-
⟨b.ylw⟩warning: nested blocks same start⟨reset⟩
34+
⟨b.ylw⟩warning: nested blocks same start
3535
⟨blu⟩ --> foo.proto:5:1
3636
|
3737
⟨blu⟩ 5 | ⟨b.ylw⟩/ ⟨b.blu⟩/ ⟨reset⟩message Blah {
@@ -41,7 +41,7 @@
4141
⟨blu⟩12 | ⟨b.ylw⟩| ⟨reset⟩}
4242
⟨blu⟩ | ⟨b.ylw⟩\___^ this block⟨reset⟩
4343

44-
⟨b.ylw⟩warning: nested blocks same end⟨reset⟩
44+
⟨b.ylw⟩warning: nested blocks same end
4545
⟨blu⟩ --> foo.proto:5:1
4646
|
4747
⟨blu⟩ 5 | ⟨b.ylw⟩/ ⟨reset⟩message Blah {
@@ -52,7 +52,7 @@
5252
⟨blu⟩ | ⟨b.ylw⟩\___^ this block
5353
⟨blu⟩ | ⟨b.ylw⟩ ⟨b.blu⟩\_- and this block⟨reset⟩
5454

55-
⟨b.ylw⟩warning: nested overlap⟨reset⟩
55+
⟨b.ylw⟩warning: nested overlap
5656
⟨blu⟩ --> foo.proto:5:1
5757
|
5858
⟨blu⟩ 5 | ⟨b.ylw⟩/ ⟨reset⟩message Blah {
@@ -64,7 +64,7 @@
6464
⟨blu⟩12 | ⟨b.blu⟩| ⟨reset⟩}
6565
⟨blu⟩ | ⟨b.blu⟩\_- and this block⟨reset⟩
6666

67-
⟨b.ylw⟩warning: nesting just the braces⟨reset⟩
67+
⟨b.ylw⟩warning: nesting just the braces
6868
⟨blu⟩ --> foo.proto:5:15
6969
|
7070
⟨blu⟩ 5 | ⟨b.ylw⟩ ⟨reset⟩message Blah {
@@ -78,7 +78,7 @@
7878
⟨blu⟩12 | ⟨b.ylw⟩| ⟨reset⟩}
7979
⟨blu⟩ | ⟨b.ylw⟩\___^ this block⟨reset⟩
8080

81-
⟨b.ylw⟩warning: nesting just the braces same start⟨reset⟩
81+
⟨b.ylw⟩warning: nesting just the braces same start
8282
⟨blu⟩ --> foo.proto:5:15
8383
|
8484
⟨blu⟩ 5 | ⟨b.ylw⟩ ⟨b.blu⟩ ⟨reset⟩message Blah {
@@ -90,7 +90,7 @@
9090
⟨blu⟩12 | ⟨b.ylw⟩| ⟨reset⟩}
9191
⟨blu⟩ | ⟨b.ylw⟩\___^ this block⟨reset⟩
9292

93-
⟨b.ylw⟩warning: nesting just the braces same start (2)⟨reset⟩
93+
⟨b.ylw⟩warning: nesting just the braces same start (2)
9494
⟨blu⟩ --> foo.proto:5:15
9595
|
9696
⟨blu⟩ 5 | ⟨b.blu⟩ ⟨b.ylw⟩ ⟨reset⟩message Blah {
@@ -102,7 +102,7 @@
102102
⟨blu⟩12 | ⟨b.blu⟩| ⟨reset⟩}
103103
⟨blu⟩ | ⟨b.blu⟩\___- this block⟨reset⟩
104104

105-
⟨b.ylw⟩warning: braces nesting overlap⟨reset⟩
105+
⟨b.ylw⟩warning: braces nesting overlap
106106
⟨blu⟩ --> foo.proto:5:15
107107
|
108108
⟨blu⟩ 5 | ⟨b.ylw⟩ ⟨reset⟩message Blah {
@@ -116,7 +116,7 @@
116116
⟨blu⟩12 | ⟨b.blu⟩| ⟨reset⟩}
117117
⟨blu⟩ | ⟨b.blu⟩\_- and this block⟨reset⟩
118118

119-
⟨b.ylw⟩warning: braces nesting overlap (2)⟨reset⟩
119+
⟨b.ylw⟩warning: braces nesting overlap (2)
120120
⟨blu⟩ --> foo.proto:7:17
121121
|
122122
⟨blu⟩ 5 | ⟨b.blu⟩ ⟨reset⟩message Blah {

experimental/report/testdata/no-snippets.yaml

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ diagnostics:
1818
- message: "system not supported"
1919
level: LEVEL_ERROR
2020

21+
- message: "this diagnostic message is comically long to illustrate message wrapping; real diagnostics should probably avoid doing this"
22+
level: LEVEL_ERROR
23+
2124
- message: 'could not open file "foo.proto": os error 2: no such file or directory'
2225
level: LEVEL_ERROR
2326
in_file: foo.proto
@@ -28,3 +31,15 @@ diagnostics:
2831
notes: ["that means that the file is screaming"]
2932
help: ["you should delete it to put it out of its misery"]
3033
debug: ["0xaaaaaaaaaaaaaaaa"]
34+
35+
- message: "very long footers"
36+
level: LEVEL_REMARK
37+
in_file: foo.proto
38+
notes:
39+
- "this footer is very very very very very very very very very very very very very very very very very very long"
40+
- "this one is also long, and it's also supercalifragilistcexpialidocious, leading to a very early break"
41+
help:
42+
- "this help is very long (and triggers the same word-wrapping code path)"
43+
- "this one contains a newline\nwhich overrides the default word wrap behavior (but this line is wrapped naturally)"
44+
debug:
45+
- "debug lines are never wrapped, no matter how crazy long they are, since they can contain stack traces"
Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,29 @@
1-
⟨b.red⟩error: system not supported⟨reset⟩⟨reset⟩
1+
⟨b.red⟩error: system not supported⟨reset⟩
22

3-
⟨b.red⟩error: could not open file "foo.proto": os error 2: no such file or directory⟨reset⟩
3+
⟨b.red⟩error: this diagnostic message is comically long to illustrate message wrapping;
4+
real diagnostics should probably avoid doing this⟨reset⟩
5+
6+
⟨b.red⟩error: could not open file "foo.proto": os error 2: no such file or directory
47
⟨blu⟩ --> foo.proto⟨reset⟩
58

6-
⟨b.ylw⟩warning: file consists only of the byte `0xaa`⟨reset⟩
9+
⟨b.ylw⟩warning: file consists only of the byte `0xaa`
10+
⟨blu⟩ --> foo.proto
11+
⟨blu⟩ = ⟨b.cyn⟩note: ⟨reset⟩that means that the file is screaming
12+
⟨blu⟩ = ⟨b.cyn⟩help: ⟨reset⟩you should delete it to put it out of its misery
13+
⟨blu⟩ = ⟨b.red⟩debug: ⟨reset⟩0xaaaaaaaaaaaaaaaa⟨reset⟩
14+
15+
⟨b.cyn⟩remark: very long footers
716
⟨blu⟩ --> foo.proto
8-
⟨blu⟩ = ⟨b.cyn⟩note: ⟨reset⟩that means that the file is screaming
9-
⟨blu⟩ = ⟨b.cyn⟩help: ⟨reset⟩you should delete it to put it out of its misery
10-
⟨blu⟩ = ⟨b.red⟩debug: ⟨reset⟩0xaaaaaaaaaaaaaaaa⟨reset⟩
17+
⟨blu⟩ = ⟨b.cyn⟩note: ⟨reset⟩this footer is very very very very very very very very very very very
18+
very very very very very very very long
19+
⟨blu⟩ = ⟨b.cyn⟩note: ⟨reset⟩this one is also long, and it's also
20+
supercalifragilistcexpialidocious, leading to a very early break
21+
⟨blu⟩ = ⟨b.cyn⟩help: ⟨reset⟩this help is very long (and triggers the same word-wrapping code
22+
path)
23+
⟨blu⟩ = ⟨b.cyn⟩help: ⟨reset⟩this one contains a newline
24+
which overrides the default word wrap behavior (but this line is
25+
wrapped naturally)
26+
⟨blu⟩ = ⟨b.red⟩debug: ⟨reset⟩debug lines are never wrapped, no matter how crazy long they are, since they can contain stack traces⟨reset⟩
1127

12-
⟨b.red⟩encountered 2 errors and 1 warning
28+
⟨b.red⟩encountered 3 errors and 1 warning
1329
⟨reset⟩

0 commit comments

Comments
 (0)