Skip to content

Commit

Permalink
Use string formatting for slice of bytes
Browse files Browse the repository at this point in the history
If a slice of bytes is mostly text, format them as text
instead of as []byte literal with hexadecimal digits.

Avoid always printing the type. This is technically invalid Go code,
but is unnecessary in many cases since the type is inferred
from the parent concrete type.

Fixes #272
  • Loading branch information
dsnet committed Apr 24, 2022
1 parent 79433ac commit ddd88c9
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 7 deletions.
2 changes: 1 addition & 1 deletion cmp/cmpopts/example_test.go
Expand Up @@ -39,7 +39,7 @@ func ExampleIgnoreFields_testing() {
// SSID: "CoffeeShopWiFi",
// - IPAddress: s"192.168.0.2",
// + IPAddress: s"192.168.0.1",
// NetMask: {0xff, 0xff, 0x00, 0x00},
// NetMask: s"ffff0000",
// Clients: []cmpopts_test.Client{
// ... // 3 identical elements
// {Hostname: "espresso", ...},
Expand Down
18 changes: 18 additions & 0 deletions cmp/compare_test.go
Expand Up @@ -1351,6 +1351,24 @@ using the AllowUnexported option.`, "\n"),
"bar": true,
}`,
reason: "short multiline JSON should prefer triple-quoted string diff as it is more readable",
}, {
label: label + "/SliceOfBytesText",
x: [][]byte{
[]byte("hello"), []byte("foo"), []byte("barbaz"), []byte("blahdieblah"),
},
y: [][]byte{
[]byte("foo"), []byte("foo"), []byte("barbaz"), []byte("added"), []byte("here"), []byte("hrmph"),
},
reason: "should print text byte slices as strings",
}, {
label: label + "/SliceOfBytesBinary",
x: [][]byte{
[]byte("\xde\xad\xbe\xef"), []byte("\xffoo"), []byte("barbaz"), []byte("blahdieblah"),
},
y: [][]byte{
[]byte("\xffoo"), []byte("foo"), []byte("barbaz"), []byte("added"), []byte("here"), []byte("hrmph\xff"),
},
reason: "should print text byte slices as strings except those with binary",
}}
}

Expand Down
2 changes: 1 addition & 1 deletion cmp/example_test.go
Expand Up @@ -37,7 +37,7 @@ func ExampleDiff_testing() {
// SSID: "CoffeeShopWiFi",
// - IPAddress: s"192.168.0.2",
// + IPAddress: s"192.168.0.1",
// NetMask: {0xff, 0xff, 0x00, 0x00},
// NetMask: s"ffff0000",
// Clients: []cmp_test.Client{
// ... // 2 identical elements
// {Hostname: "macchiato", IPAddress: s"192.168.0.153", LastSeen: s"2009-11-10 23:39:43 +0000 UTC"},
Expand Down
5 changes: 4 additions & 1 deletion cmp/report_compare.go
Expand Up @@ -116,7 +116,10 @@ func (opts formatOptions) FormatDiff(v *valueNode, ptrs *pointerReferences) (out
}

// For leaf nodes, format the value based on the reflect.Values alone.
if v.MaxDepth == 0 {
// As a special case, treat equal []byte as a leaf nodes.
isBytes := v.Type.Kind() == reflect.Slice && v.Type.Elem() == reflect.TypeOf(byte(0))
isEqualBytes := isBytes && v.NumDiff+v.NumIgnored+v.NumTransformed == 0
if v.MaxDepth == 0 || isEqualBytes {
switch opts.DiffMode {
case diffUnknown, diffIdentical:
// Format Equal.
Expand Down
2 changes: 1 addition & 1 deletion cmp/report_reflect.go
Expand Up @@ -211,7 +211,7 @@ func (opts formatOptions) FormatValue(v reflect.Value, parentKind reflect.Kind,
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)
return opts.FormatType(t, out)
}
}

Expand Down
30 changes: 27 additions & 3 deletions cmp/testdata/diffs
Expand Up @@ -241,9 +241,9 @@
"string",
}),
Bytes: []uint8(Inverse(SplitBytes, [][]uint8{
{0x73, 0x6f, 0x6d, 0x65},
{0x6d, 0x75, 0x6c, 0x74, ...},
{0x6c, 0x69, 0x6e, 0x65},
"some",
"multi",
"line",
{
- 0x62,
+ 0x42,
Expand Down Expand Up @@ -1134,6 +1134,30 @@
"""
)
>>> TestDiff/Reporter/ShortJSON
<<< TestDiff/Reporter/SliceOfBytesText
[][]uint8{
- "hello",
"foo",
+ "foo",
"barbaz",
+ "added",
+ "here",
- "blahdieblah",
+ "hrmph",
}
>>> TestDiff/Reporter/SliceOfBytesText
<<< TestDiff/Reporter/SliceOfBytesBinary
[][]uint8{
- {0xde, 0xad, 0xbe, 0xef},
{0xff, 0x6f, 0x6f},
+ "foo",
"barbaz",
+ "added",
+ "here",
- "blahdieblah",
+ {0x68, 0x72, 0x6d, 0x70, 0x68, 0xff},
}
>>> TestDiff/Reporter/SliceOfBytesBinary
<<< TestDiff/EmbeddedStruct/ParentStructA/Inequal
teststructs.ParentStructA{
privateStruct: teststructs.privateStruct{
Expand Down

0 comments on commit ddd88c9

Please sign in to comment.