From 71220fc3ca5513eaf3583d10fd18da893bc332d8 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Mon, 25 Apr 2022 09:32:19 -0700 Subject: [PATCH] Use string formatting for slice of bytes (#294) 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 7278485..aafe441 100644 --- a/cmp/compare_test.go +++ b/cmp/compare_test.go @@ -1385,6 +1385,24 @@ using the AllowUnexported option.`, "\n"), x: struct{ X interface{} }{int(0)}, y: struct{ X interface{} }{uint(0)}, reason: "mismatched underlying value within interface", + }, { + 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 d9d7323..287b893 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 23781f7..8bff76f 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, @@ -1158,6 +1158,30 @@ + X: uint(0), } >>> TestDiff/Reporter/StructFieldAny +<<< 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{