From ddd88c92695241f767a7d737ef0568612a344f12 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Sat, 23 Apr 2022 17:39:05 -0700 Subject: [PATCH] Use string formatting for slice of bytes 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 --- cmp/cmpopts/example_test.go | 2 +- cmp/compare_test.go | 18 ++++++++++++++++++ cmp/example_test.go | 2 +- cmp/report_compare.go | 5 ++++- cmp/report_reflect.go | 2 +- cmp/testdata/diffs | 30 +++++++++++++++++++++++++++--- 6 files changed, 52 insertions(+), 7 deletions(-) diff --git a/cmp/cmpopts/example_test.go b/cmp/cmpopts/example_test.go index 0cf2513..4b9a8ab 100644 --- a/cmp/cmpopts/example_test.go +++ b/cmp/cmpopts/example_test.go @@ -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", ...}, diff --git a/cmp/compare_test.go b/cmp/compare_test.go index 9ad9456..bf36a42 100644 --- a/cmp/compare_test.go +++ b/cmp/compare_test.go @@ -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", }} } diff --git a/cmp/example_test.go b/cmp/example_test.go index e1f4338..9968149 100644 --- a/cmp/example_test.go +++ b/cmp/example_test.go @@ -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"}, diff --git a/cmp/report_compare.go b/cmp/report_compare.go index 104bb30..1ef65ac 100644 --- a/cmp/report_compare.go +++ b/cmp/report_compare.go @@ -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. diff --git a/cmp/report_reflect.go b/cmp/report_reflect.go index 76c04fd..654090d 100644 --- a/cmp/report_reflect.go +++ b/cmp/report_reflect.go @@ -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) } } diff --git a/cmp/testdata/diffs b/cmp/testdata/diffs index d207803..6264bc4 100644 --- a/cmp/testdata/diffs +++ b/cmp/testdata/diffs @@ -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, @@ -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{