Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Implement text wrapping for most diagnostic messages #446

Merged
merged 5 commits into from
Feb 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ error: unterminated string literal
|
1 | '\'
| ^^^ expected to be terminated by `'`
= note: this string appears to end in an escaped quote; replace `\'` with `\\''`
= note: this string appears to end in an escaped quote; replace `\'` with
`\\''`

encountered 1 error
12 changes: 6 additions & 6 deletions experimental/report/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,24 +87,24 @@ func unifiedDiff(span Span, edits []Edit) (Span, []hunk) {

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

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

prev = next
prev = i
return true
})

var out []hunk
var prevHunk int
parts(func(_ int, edits []Edit) bool {
parts(func(edits []Edit) bool {
// First, figure out the start and end of the modified region.
start, end := edits[0].Start, edits[0].End
for _, edit := range edits[1:] {
Expand Down
68 changes: 35 additions & 33 deletions experimental/report/renderer.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@ import (
"bytes"
"fmt"
"io"
"math"
"math/bits"
"slices"
"strconv"
"strings"
"unicode"

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

fmt.Fprint(r, r.ss.BoldForLevel(d.level), level, ": ", d.message, r.ss.reset)
fmt.Fprint(r, r.ss.BoldForLevel(d.level), level, ": ")
r.WriteWrapped(d.message, MaxMessageWidth)

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

// Render all the diagnostic windows.
parts := slicesx.Partition(d.snippets, func(a, b *snippet) bool {
if len(a.edits) > 0 || len(b.edits) > 0 {
parts := slicesx.PartitionKey(d.snippets, func(snip snippet) any {
if len(snip.edits) > 0 {
// Suggestions are always rendered in their own windows.
return true
// Return a fresh pointer, since that will always compare as
// distinct.
return new(int)
}

return a.Path() != b.Path()
return snip.Path()
})

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

// Render the footers. For simplicity we collect them into an array first.
footers := make([][3]string, 0, len(d.notes)+len(d.help)+len(d.debug))
for _, note := range d.notes {
footers = append(footers, [3]string{r.ss.bRemark, "note", note})
}
for _, help := range d.help {
footers = append(footers, [3]string{r.ss.bRemark, "help", help})
type footer struct {
color, label, text string
}
if r.ShowDebug {
for _, debug := range d.debug {
footers = append(footers, [3]string{r.ss.bError, "debug", debug})
footers := iterx.Chain(
slicesx.Map(d.notes, func(s string) footer { return footer{r.ss.bRemark, "note", s} }),
slicesx.Map(d.help, func(s string) footer { return footer{r.ss.bRemark, "help", s} }),
slicesx.Map(d.debug, func(s string) footer { return footer{r.ss.bError, "debug", s} }),
)

footers(func(f footer) bool {
isDebug := f.label == "debug"
if isDebug && !r.ShowDebug {
return true
}
}
for _, footer := range footers {

r.WriteString("\n")
r.WriteString(r.ss.nAccent)
r.WriteSpaces(r.margin)
r.WriteString(" = ")
fmt.Fprint(r, footer[0], footer[1], ": ", r.ss.reset)
for i, line := range strings.Split(footer[2], "\n") {
if i > 0 {
r.WriteString("\n")
margin := r.margin + 3 + len(footer[1]) + 2
r.WriteSpaces(margin)
}
r.WriteString(line)
fmt.Fprintf(r, "%s = %s%s: %s", r.ss.nAccent, f.color, f.label, r.ss.reset)

if isDebug {
r.WriteWrapped(f.text, math.MaxInt)
doriable marked this conversation as resolved.
Show resolved Hide resolved
} else {
r.WriteWrapped(f.text, MaxMessageWidth)
}
}

return true
})

r.WriteString(r.ss.reset)
r.WriteString("\n\n")
Expand Down Expand Up @@ -482,7 +485,7 @@ func (r *renderer) window(w *window) {

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

// Now, convert the buffer into a proper string.
var out strings.Builder
parts := slicesx.Partition(buf, func(a, b *byte) bool { return *a != *b })
parts(func(_ int, line []byte) bool {
slicesx.Partition(buf)(func(_ int, line []byte) bool {
level := Level(line[0])
if line[0] == 0 {
out.WriteString(r.ss.reset)
Expand Down Expand Up @@ -906,7 +908,7 @@ func (r *renderer) suggestion(snip snippet) {
r.WriteString(r.ss.nAccent)
r.WriteSpaces(r.margin)
r.WriteString("help: ")
r.WriteString(snip.message)
r.WriteWrapped(snip.message, MaxMessageWidth)

// Add a blank line after the file. This gives the diagnostic window some
// visual breathing room.
Expand Down
3 changes: 0 additions & 3 deletions experimental/report/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ import (
"github.com/bufbuild/protocompile/internal/iter"
)

// TabstopWidth is the size we render all tabstops as.
const TabstopWidth int = 4

// Spanner is any type with a [Span].
type Spanner interface {
// Should return the zero [Span] to indicate that it does not contribute
Expand Down
4 changes: 2 additions & 2 deletions experimental/report/testdata/i18n.yaml.color.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
⟨b.red⟩error: emoji, CJK, bidi⟨reset⟩
⟨b.red⟩error: emoji, CJK, bidi
⟨blu⟩ --> foo.proto:5:9
|
⟨blu⟩ 5 | ⟨reset⟩message 🐈<U+200D>⬛ {
Expand All @@ -10,7 +10,7 @@
⟨blu⟩ 1 | ⟨reset⟩import "חתול שחור.proto";
⟨blu⟩ | ⟨reset⟩ ⟨b.blu⟩---------------⟨reset⟩ ⟨b.blu⟩bidi works if it's quoted, at least⟨reset⟩

⟨b.red⟩error: bidi (Arabic, Hebrew, Farsi, etc) is broken in some contexts⟨reset⟩
⟨b.red⟩error: bidi (Arabic, Hebrew, Farsi, etc) is broken in some contexts
⟨blu⟩ --> foo.proto:7:10
|
⟨blu⟩ 7 | ⟨reset⟩ string القطة السوداء = 2;
Expand Down
4 changes: 2 additions & 2 deletions experimental/report/testdata/multi-file.yaml.color.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
⟨b.red⟩error: two files⟨reset⟩
⟨b.red⟩error: two files
⟨blu⟩ --> foo.proto:3:9
|
⟨blu⟩ 3 | ⟨reset⟩package abc.xyz;
Expand All @@ -11,7 +11,7 @@
⟨blu⟩ 3 | ⟨reset⟩package abc.xyz2;
⟨blu⟩ | ⟨reset⟩ ⟨b.blu⟩-------⟨reset⟩ ⟨b.blu⟩baz⟨reset⟩

⟨b.red⟩error: three files⟨reset⟩
⟨b.red⟩error: three files
⟨blu⟩ --> foo.proto:3:9
|
⟨blu⟩ 3 | ⟨reset⟩package abc.xyz;
Expand Down
4 changes: 2 additions & 2 deletions experimental/report/testdata/multi-underline.yaml.color.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
⟨b.red⟩error: `size_t` is not a built-in Protobuf type⟨reset⟩
⟨b.red⟩error: `size_t` is not a built-in Protobuf type
⟨blu⟩ --> foo.proto:6:12
|
⟨blu⟩ 1 | ⟨reset⟩syntax = "proto4"
Expand All @@ -8,7 +8,7 @@
⟨blu⟩ 6 | ⟨reset⟩ required size_t x = 0;
⟨blu⟩ | ⟨reset⟩ ⟨b.red⟩^^^^^⟨reset⟩ ⟨b.red⟩⟨reset⟩

⟨b.ylw⟩warning: these are pretty bad names⟨reset⟩
⟨b.ylw⟩warning: these are pretty bad names
⟨blu⟩ --> foo.proto:3:9
|
⟨blu⟩ 3 | ⟨reset⟩package abc.xyz;
Expand Down
22 changes: 11 additions & 11 deletions experimental/report/testdata/multiline.yaml.color.txt
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
⟨b.ylw⟩warning: whole block⟨reset⟩
⟨b.ylw⟩warning: whole block
⟨blu⟩ --> foo.proto:5:1
|
⟨blu⟩ 5 | ⟨b.ylw⟩/ ⟨reset⟩message Blah {
⟨blu⟩... ⟨b.ylw⟩|
⟨blu⟩12 | ⟨b.ylw⟩| ⟨reset⟩}
⟨blu⟩ | ⟨b.ylw⟩\_^ this block⟨reset⟩

⟨b.ylw⟩warning: nested blocks⟨reset⟩
⟨b.ylw⟩warning: nested blocks
⟨blu⟩ --> foo.proto:5:1
|
⟨blu⟩ 5 | ⟨b.ylw⟩/ ⟨reset⟩message Blah {
Expand All @@ -18,7 +18,7 @@
⟨blu⟩12 | ⟨b.ylw⟩| ⟨reset⟩}
⟨blu⟩ | ⟨b.ylw⟩\___^ this block⟨reset⟩

⟨b.ylw⟩warning: parallel blocks⟨reset⟩
⟨b.ylw⟩warning: parallel blocks
⟨blu⟩ --> foo.proto:5:1
|
⟨blu⟩ 5 | ⟨b.ylw⟩/ ⟨reset⟩message Blah {
Expand All @@ -31,7 +31,7 @@
⟨blu⟩12 | ⟨b.blu⟩/ ⟨reset⟩}
⟨blu⟩ | ⟨b.blu⟩\_- and this block⟨reset⟩

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

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

⟨b.ylw⟩warning: nested overlap⟨reset⟩
⟨b.ylw⟩warning: nested overlap
⟨blu⟩ --> foo.proto:5:1
|
⟨blu⟩ 5 | ⟨b.ylw⟩/ ⟨reset⟩message Blah {
Expand All @@ -64,7 +64,7 @@
⟨blu⟩12 | ⟨b.blu⟩| ⟨reset⟩}
⟨blu⟩ | ⟨b.blu⟩\_- and this block⟨reset⟩

⟨b.ylw⟩warning: nesting just the braces⟨reset⟩
⟨b.ylw⟩warning: nesting just the braces
⟨blu⟩ --> foo.proto:5:15
|
⟨blu⟩ 5 | ⟨b.ylw⟩ ⟨reset⟩message Blah {
Expand All @@ -78,7 +78,7 @@
⟨blu⟩12 | ⟨b.ylw⟩| ⟨reset⟩}
⟨blu⟩ | ⟨b.ylw⟩\___^ this block⟨reset⟩

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

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

⟨b.ylw⟩warning: braces nesting overlap⟨reset⟩
⟨b.ylw⟩warning: braces nesting overlap
⟨blu⟩ --> foo.proto:5:15
|
⟨blu⟩ 5 | ⟨b.ylw⟩ ⟨reset⟩message Blah {
Expand All @@ -116,7 +116,7 @@
⟨blu⟩12 | ⟨b.blu⟩| ⟨reset⟩}
⟨blu⟩ | ⟨b.blu⟩\_- and this block⟨reset⟩

⟨b.ylw⟩warning: braces nesting overlap (2)⟨reset⟩
⟨b.ylw⟩warning: braces nesting overlap (2)
⟨blu⟩ --> foo.proto:7:17
|
⟨blu⟩ 5 | ⟨b.blu⟩ ⟨reset⟩message Blah {
Expand Down
15 changes: 15 additions & 0 deletions experimental/report/testdata/no-snippets.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ diagnostics:
- message: "system not supported"
level: LEVEL_ERROR

- message: "this diagnostic message is comically long to illustrate message wrapping; real diagnostics should probably avoid doing this"
level: LEVEL_ERROR

- message: 'could not open file "foo.proto": os error 2: no such file or directory'
level: LEVEL_ERROR
in_file: foo.proto
Expand All @@ -28,3 +31,15 @@ diagnostics:
notes: ["that means that the file is screaming"]
help: ["you should delete it to put it out of its misery"]
debug: ["0xaaaaaaaaaaaaaaaa"]

- message: "very long footers"
level: LEVEL_REMARK
in_file: foo.proto
notes:
- "this footer is very very very very very very very very very very very very very very very very very very long"
- "this one is also long, and it's also supercalifragilistcexpialidocious, leading to a very early break"
help:
- "this help is very long (and triggers the same word-wrapping code path)"
- "this one contains a newline\nwhich overrides the default word wrap behavior (but this line is wrapped naturally)"
debug:
- "debug lines are never wrapped, no matter how crazy long they are, since they can contain stack traces"
30 changes: 23 additions & 7 deletions experimental/report/testdata/no-snippets.yaml.color.txt
Original file line number Diff line number Diff line change
@@ -1,13 +1,29 @@
⟨b.red⟩error: system not supported⟨reset⟩⟨reset⟩
⟨b.red⟩error: system not supported⟨reset⟩

⟨b.red⟩error: could not open file "foo.proto": os error 2: no such file or directory⟨reset⟩
⟨b.red⟩error: this diagnostic message is comically long to illustrate message wrapping;
real diagnostics should probably avoid doing this⟨reset⟩

⟨b.red⟩error: could not open file "foo.proto": os error 2: no such file or directory
⟨blu⟩ --> foo.proto⟨reset⟩

⟨b.ylw⟩warning: file consists only of the byte `0xaa`⟨reset⟩
⟨b.ylw⟩warning: file consists only of the byte `0xaa`
⟨blu⟩ --> foo.proto
⟨blu⟩ = ⟨b.cyn⟩note: ⟨reset⟩that means that the file is screaming
⟨blu⟩ = ⟨b.cyn⟩help: ⟨reset⟩you should delete it to put it out of its misery
⟨blu⟩ = ⟨b.red⟩debug: ⟨reset⟩0xaaaaaaaaaaaaaaaa⟨reset⟩

⟨b.cyn⟩remark: very long footers
⟨blu⟩ --> foo.proto
⟨blu⟩ = ⟨b.cyn⟩note: ⟨reset⟩that means that the file is screaming
⟨blu⟩ = ⟨b.cyn⟩help: ⟨reset⟩you should delete it to put it out of its misery
⟨blu⟩ = ⟨b.red⟩debug: ⟨reset⟩0xaaaaaaaaaaaaaaaa⟨reset⟩
⟨blu⟩ = ⟨b.cyn⟩note: ⟨reset⟩this footer is very very very very very very very very very very very
very very very very very very very long
⟨blu⟩ = ⟨b.cyn⟩note: ⟨reset⟩this one is also long, and it's also
supercalifragilistcexpialidocious, leading to a very early break
⟨blu⟩ = ⟨b.cyn⟩help: ⟨reset⟩this help is very long (and triggers the same word-wrapping code
path)
⟨blu⟩ = ⟨b.cyn⟩help: ⟨reset⟩this one contains a newline
which overrides the default word wrap behavior (but this line is
wrapped naturally)
⟨blu⟩ = ⟨b.red⟩debug: ⟨reset⟩debug lines are never wrapped, no matter how crazy long they are, since they can contain stack traces⟨reset⟩

⟨b.red⟩encountered 2 errors and 1 warning
⟨b.red⟩encountered 3 errors and 1 warning
⟨reset⟩
Loading
Loading