errors/fmt: improve error printing in detail mode
- Add a colon after a non-detailed message if
it has details or of it is followed by another
message.
Follows the design draft.
This is more readable and more consistent
with non-detailed errors.
- Gobble newline after last detail
This results in a better API experience when
printing multiple sections consecutively.
- Allow Detail to be called more than once.
Change-Id: Ic19179cdf1479fded8b657d569cb66bd2e54467e
Reviewed-on: https://go-review.googlesource.com/c/144199
Run-TryBot: Marcel van Lohuizen <mpvl@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
diff --git a/errors/fmt/errors.go b/errors/fmt/errors.go
index da0e3db..5e68180 100644
--- a/errors/fmt/errors.go
+++ b/errors/fmt/errors.go
@@ -5,6 +5,7 @@
package fmt
import (
+ "bytes"
"strings"
"golang.org/x/exp/errors"
@@ -87,8 +88,8 @@
func fmtError(p *pp, verb rune, err error) (handled bool) {
var (
- sep = ": " // separator before next error
- w = p // print buffer where error text is written
+ sep = " " // separator before next error
+ w = p // print buffer where error text is written
)
switch {
// Note that this switch must match the preference order
@@ -156,7 +157,15 @@
if err == nil {
break
}
+ if !w.fmt.inDetail || !p.fmt.plusV {
+ w.buf.WriteByte(':')
+ }
+ // Strip last newline of detail.
+ if bytes.HasSuffix([]byte(w.buf), detailSep) {
+ w.buf = w.buf[:len(w.buf)-len(detailSep)]
+ }
w.buf.WriteString(sep)
+ w.fmt.inDetail = false
}
if w != p {
@@ -165,9 +174,10 @@
return true
}
+var detailSep = []byte("\n ")
+
// errPPState wraps a pp to implement State with indentation. It is used
// for errors implementing fmt.Formatter.
-
type errPPState pp
func (p *errPPState) Width() (wid int, ok bool) { return (*pp)(p).Width() }
@@ -181,7 +191,7 @@
for i, c := range b {
if c == '\n' {
p.buf.Write(b[k:i])
- p.buf.Write([]byte("\n "))
+ p.buf.Write(detailSep)
k = i + 1
}
}
@@ -215,8 +225,11 @@
}
func (p *errPP) Detail() bool {
- p.fmt.indent = p.fmt.plusV
+ inDetail := p.fmt.inDetail
p.fmt.inDetail = true
- (*errPPState)(p).Write([]byte("\n"))
+ p.fmt.indent = p.fmt.plusV
+ if p.fmt.plusV && !inDetail {
+ (*errPPState)(p).Write([]byte(":\n"))
+ }
return p.fmt.plusV
}
diff --git a/errors/fmt/errors_test.go b/errors/fmt/errors_test.go
index 427a527..57b7f2b 100644
--- a/errors/fmt/errors_test.go
+++ b/errors/fmt/errors_test.go
@@ -49,7 +49,7 @@
"chained/somefile.go:xxx"),
}, {
fmt.Errorf("not wrapped: %+v", chained),
- chain("not wrapped: chained somefile.go:123/path.TestErrorf/path.go:xxx"),
+ chain("not wrapped: chained: somefile.go:123/path.TestErrorf/path.go:xxx"),
}}
for i, tc := range testCases {
t.Run(strconv.Itoa(i)+"/"+path.Join(tc.want...), func(t *testing.T) {
@@ -80,7 +80,10 @@
&wrapped{"and another\none", nil}}
fallback = &wrapped{"fallback", os.ErrNotExist}
oldAndNew = &wrapped{"new style", formatError("old style")}
- opaque = &wrapped{"outer",
+ framed = &withFrameAndMore{
+ frame: errors.Caller(0),
+ }
+ opaque = &wrapped{"outer",
errors.Opaque(&wrapped{"mid",
&wrapped{"inner", nil}})}
)
@@ -97,24 +100,29 @@
fmt: "%s",
want: "can't adumbrate elephant: out of peanuts",
}, {
- err: simple,
- fmt: "%+v",
- want: "simple\n somefile.go:123",
+ err: &wrapped{"a", &wrapped{"b", &wrapped{"c", nil}}},
+ fmt: "%s",
+ want: "a: b: c",
+ }, {
+ err: simple,
+ fmt: "%+v",
+ want: "simple:" +
+ "\n somefile.go:123",
}, {
err: elephant,
fmt: "%+v",
- want: "can't adumbrate elephant" +
+ want: "can't adumbrate elephant:" +
"\n somefile.go:123" +
- "\n--- out of peanuts" +
+ "\n--- out of peanuts:" +
"\n the elephant is on strike" +
"\n and the 12 monkeys" +
"\n are laughing",
}, {
err: transition,
fmt: "%+v",
- want: "elephant still on strike" +
+ want: "elephant still on strike:" +
"\n somefile.go:123" +
- "\n--- out of peanuts" +
+ "\n--- out of peanuts:" +
"\n the elephant is on strike" +
"\n and the 12 monkeys" +
"\n are laughing",
@@ -123,6 +131,13 @@
fmt: "%#v",
want: "&fmt_test.wrapped{msg:\"simple\", err:error(nil)}",
}, {
+ err: framed,
+ fmt: "%+v",
+ want: "something:" +
+ "\n golang.org/x/exp/errors/fmt_test.TestErrorFormatter" +
+ "\n /Users/mpvl/dev/go/text/src/golang.org/x/exp/errors/fmt/errors_test.go:84" +
+ "\n something more",
+ }, {
err: fmtTwice("Hello World!"),
fmt: "%#v",
want: "2 times Hello World!",
@@ -131,9 +146,12 @@
fmt: "%s",
want: "fallback: file does not exist",
}, {
- err: fallback,
- fmt: "%+v",
- want: "fallback\n somefile.go:123\n--- file does not exist",
+ err: fallback,
+ fmt: "%+v",
+ // Note: no colon after the last error, as there are no details.
+ want: "fallback:" +
+ "\n somefile.go:123" +
+ "\n--- file does not exist",
}, {
err: opaque,
fmt: "%s",
@@ -141,11 +159,11 @@
}, {
err: opaque,
fmt: "%+v",
- want: "outer" +
+ want: "outer:" +
"\n somefile.go:123" +
- "\n--- mid" +
+ "\n--- mid:" +
"\n somefile.go:123" +
- "\n--- inner" +
+ "\n--- inner:" +
"\n somefile.go:123",
}, {
err: oldAndNew,
@@ -159,16 +177,22 @@
err: oldAndNew,
fmt: "%+v",
// Note the extra indentation.
- want: "new style\n somefile.go:123\n--- old style\n otherfile.go:456",
+ // Colon for old style error is rendered by the fmt.Formatter
+ // implementation of the old-style error.
+ want: "new style:" +
+ "\n somefile.go:123" +
+ "\n--- old style:" +
+ "\n otherfile.go:456",
}, {
err: simple,
fmt: "%-12s",
want: "simple ",
}, {
// Don't use formatting flags for detailed view.
- err: simple,
- fmt: "%+12v",
- want: "simple\n somefile.go:123",
+ err: simple,
+ fmt: "%+12v",
+ want: "simple:" +
+ "\n somefile.go:123",
}, {
err: elephant,
fmt: "%+50s",
@@ -186,9 +210,11 @@
fmt: "% x",
want: "73 69 6d 70 6c 65",
}, {
- err: newline,
- fmt: "%s",
- want: "msg with\nnewline: and another\none",
+ err: newline,
+ fmt: "%s",
+ want: "msg with" +
+ "\nnewline: and another" +
+ "\none",
}, {
err: nil,
fmt: "%+v",
@@ -277,6 +303,21 @@
return nil
}
+type withFrameAndMore struct {
+ frame errors.Frame
+}
+
+func (e *withFrameAndMore) Error() string { return fmt.Sprint(e) }
+
+func (e *withFrameAndMore) Format(p errors.Printer) (next error) {
+ p.Print("something")
+ if p.Detail() {
+ e.frame.Format(p)
+ p.Print("something more")
+ }
+ return nil
+}
+
// formatError is an error implementing Format instead of errors.Formatter.
// The implementation mimics the implementation of github.com/pkg/errors,
// including that
@@ -290,7 +331,7 @@
case 'v':
if s.Flag('+') {
io.WriteString(s, string(e))
- fmt.Fprintf(s, "\n%s", "otherfile.go:456")
+ fmt.Fprintf(s, ":\n%s", "otherfile.go:456")
return
}
fallthrough
@@ -332,7 +373,7 @@
func (panicValue) String() string { panic("panic") }
var rePath = regexp.MustCompile(`( [^ ]*)fmt.*test\.`)
-var reLine = regexp.MustCompile(":[0-9]*$")
+var reLine = regexp.MustCompile(":[0-9]*\n?$")
func cleanPath(s string) string {
s = rePath.ReplaceAllString(s, "/path.")
diff --git a/errors/fmt/format_example_test.go b/errors/fmt/format_example_test.go
index 7609ffc..c549cc8 100644
--- a/errors/fmt/format_example_test.go
+++ b/errors/fmt/format_example_test.go
@@ -28,13 +28,13 @@
// foo: bar(nameserver 139): baz flopped
//
// Detailed error:
- // foo
+ // foo:
// golang.org/x/exp/errors/fmt_test.foo
// golang.org/x/exp/errors/fmt/format_example_test.go:17
- // --- bar(nameserver 139)
+ // --- bar(nameserver 139):
// golang.org/x/exp/errors/fmt_test.bar
// golang.org/x/exp/errors/fmt/format_example_test.go:16
- // --- baz flopped
+ // --- baz flopped:
// golang.org/x/exp/errors/fmt_test.baz
// golang.org/x/exp/errors/fmt/format_example_test.go:15
}
diff --git a/errors/frame.go b/errors/frame.go
index 10d2e20..a5369e5 100644
--- a/errors/frame.go
+++ b/errors/frame.go
@@ -50,7 +50,7 @@
p.Printf("%s\n ", function)
}
if file != "" {
- p.Printf("%s:%d", file, line)
+ p.Printf("%s:%d\n", file, line)
}
}
}
diff --git a/errors/wrap_test.go b/errors/wrap_test.go
index dbf6e31..9481401 100644
--- a/errors/wrap_test.go
+++ b/errors/wrap_test.go
@@ -128,7 +128,7 @@
}
got = fmt.Errorf("foo: %+v", errors.Opaque(errorD{}))
- want = "foo: errorD\n detail"
+ want = "foo: errorD:\n detail"
if got.Error() != want {
t.Errorf("error with Format: got %v; want %v", got, want)
}
