Skip to content

Commit

Permalink
Fix textual printing of byte slices
Browse files Browse the repository at this point in the history
There are two bugs being fixed:

1.  The hueristic for whether a slice of byte looks like text
    should check whether a rune IsPrint OR IsSpace, and not both.
    Only a single rune (i.e., U+0020) ever satisfies both conditions.

    Previously, it would print as:
        MyBytes{0x68, 0x65, 0x6c, 0x6c, 0x6f}
    and now it would now print as:
        MyBytes(MyBytes("hello"))

2.  If we're printing as string, then we should set skipType=true
    since we already explicitly format the value with the type.

    Previously, it would print as:
        MyBytes(MyBytes("hello"))
    and now it would now print as:
        MyBytes("hello")
  • Loading branch information
dsnet committed Jul 19, 2021
1 parent 290a6a2 commit d5fcb38
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 1 deletion.
20 changes: 20 additions & 0 deletions cmp/compare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1317,6 +1317,26 @@ using the AllowUnexported option.`, "\n"),
x: "d5c14bdf6bac81c27afc5429500ed750\n25483503b557c606dad4f144d27ae10b\n90bdbcdbb6ea7156068e3dcfb7459244\n978f480a6e3cced51e297fbff9a506b7\n",
y: "Xd5c14bdf6bac81c27afc5429500ed750\nX25483503b557c606dad4f144d27ae10b\nX90bdbcdbb6ea7156068e3dcfb7459244\nX978f480a6e3cced51e297fbff9a506b7\n",
reason: "all lines are different, so diffing based on lines is pointless",
}, {
label: label + "/StringifiedBytes",
x: struct{ X []byte }{[]byte("hello, world!")},
y: struct{ X []byte }{},
reason: "[]byte should be printed as text since it is printable text",
}, {
label: label + "/NonStringifiedBytes",
x: struct{ X []byte }{[]byte("\xde\xad\xbe\xef")},
y: struct{ X []byte }{},
reason: "[]byte should not be printed as text since it is binary data",
}, {
label: label + "/StringifiedNamedBytes",
x: struct{ X MyBytes }{MyBytes("hello, world!")},
y: struct{ X MyBytes }{},
reason: "MyBytes should be printed as text since it is printable text",
}, {
label: label + "/NonStringifiedNamedBytes",
x: struct{ X MyBytes }{MyBytes("\xde\xad\xbe\xef")},
y: struct{ X MyBytes }{},
reason: "MyBytes should not be printed as text since it is binary data",
}}
}

Expand Down
3 changes: 2 additions & 1 deletion cmp/report_reflect.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,10 @@ func (opts formatOptions) FormatValue(v reflect.Value, parentKind reflect.Kind,
// Check whether this is a []byte of text data.
if t.Elem() == reflect.TypeOf(byte(0)) {
b := v.Bytes()
isPrintSpace := func(r rune) bool { return unicode.IsPrint(r) && unicode.IsSpace(r) }
isPrintSpace := func(r rune) bool { return unicode.IsPrint(r) || unicode.IsSpace(r) }
if len(b) > 0 && utf8.Valid(b) && len(bytes.TrimFunc(b, isPrintSpace)) == 0 {
out = opts.formatString("", string(b))
skipType = true
return opts.WithTypeMode(emitType).FormatType(t, out)
}
}
Expand Down
24 changes: 24 additions & 0 deletions cmp/testdata/diffs
Original file line number Diff line number Diff line change
Expand Up @@ -1096,6 +1096,30 @@
"978f480a6e3cced51e297fbff9a506b7\n",
}, "")
>>> TestDiff/Reporter/AllLinesDiffer
<<< TestDiff/Reporter/StringifiedBytes
struct{ X []uint8 }{
- X: []uint8("hello, world!"),
+ X: nil,
}
>>> TestDiff/Reporter/StringifiedBytes
<<< TestDiff/Reporter/NonStringifiedBytes
struct{ X []uint8 }{
- X: []uint8{0xde, 0xad, 0xbe, 0xef},
+ X: nil,
}
>>> TestDiff/Reporter/NonStringifiedBytes
<<< TestDiff/Reporter/StringifiedNamedBytes
struct{ X cmp_test.MyBytes }{
- X: cmp_test.MyBytes("hello, world!"),
+ X: nil,
}
>>> TestDiff/Reporter/StringifiedNamedBytes
<<< TestDiff/Reporter/NonStringifiedNamedBytes
struct{ X cmp_test.MyBytes }{
- X: cmp_test.MyBytes{0xde, 0xad, 0xbe, 0xef},
+ X: nil,
}
>>> TestDiff/Reporter/NonStringifiedNamedBytes
<<< TestDiff/EmbeddedStruct/ParentStructA/Inequal
teststructs.ParentStructA{
privateStruct: teststructs.privateStruct{
Expand Down

0 comments on commit d5fcb38

Please sign in to comment.