From cc9647a88e1e31c2f11f17fbd7e20236628e58f0 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Tue, 9 Jun 2020 23:05:13 -0700 Subject: [PATCH] Use custom triple-quote syntax for diffing string literals Using strings.Join to denote differences in a multi-line string is visually noisy due to extensive use of quotes and escape sequences. Add a custom triple-quote syntax that unambiguously shows line differences with less visual noise. If the triple-quote syntax cannot unmabiguously show differences, then the reporter falls back on using the strings.Join format, which is never ambiguous. Fixes #195 --- cmp/compare_test.go | 36 ++++++++++++ cmp/report_slices.go | 51 ++++++++++++++++- cmp/report_text.go | 15 ++--- cmp/testdata/diffs | 129 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 223 insertions(+), 8 deletions(-) diff --git a/cmp/compare_test.go b/cmp/compare_test.go index 4ffa0eb..eb0e634 100644 --- a/cmp/compare_test.go +++ b/cmp/compare_test.go @@ -883,6 +883,42 @@ func reporterTests() []test { y: MyComposite{BytesA: []byte(`{"firstName":"John","lastName":"Smith","isAlive":true,"age":27,"address":{"streetAddress":"21 2nd Street","city":"New York","state":"NY","postalCode":"10021-3100"},"phoneNumbers":[{"type":"home","number":"212 555-1234"},{"type":"office","number":"646 555-4567"},{"type":"mobile","number":"123 456-7890"}],"children":[],"spouse":null}`)}, wantEqual: false, reason: "batched textual diff desired since bytes looks like textual data", + }, { + label: label + "/TripleQuote", + x: "aaa\nbbb\nccc\nddd\neee\nfff\nggg\nhhh\niii\njjj\nkkk\nlll\nmmm\nnnn\nooo\nppp\nqqq\nRRR\nsss\nttt\nuuu\nvvv\nwww\nxxx\nyyy\nzzz\n", + y: "aaa\nbbb\nCCC\nddd\neee\nfff\nggg\nhhh\niii\njjj\nkkk\nlll\nmmm\nnnn\nooo\nppp\nqqq\nrrr\nSSS\nttt\nuuu\nvvv\nwww\nxxx\nyyy\nzzz\n", + wantEqual: false, + reason: "use triple-quote syntax", + }, { + label: label + "/TripleQuoteEndlines", + x: "aaa\nbbb\nccc\nddd\neee\nfff\nggg\r\nhhh\n\riii\njjj\nkkk\nlll\nmmm\nnnn\nooo\nppp\nqqq\nRRR\nsss\nttt\nuuu\nvvv\nwww\nxxx\nyyy\nzzz\n\r", + y: "aaa\nbbb\nCCC\nddd\neee\nfff\nggg\r\nhhh\n\riii\njjj\nkkk\nlll\nmmm\nnnn\nooo\nppp\nqqq\nrrr\nsss\nttt\nuuu\nvvv\nwww\nxxx\nyyy\nzzz", + wantEqual: false, + reason: "use triple-quote syntax", + }, { + label: label + "/AvoidTripleQuoteAmbiguousQuotes", + x: "aaa\nbbb\nccc\nddd\neee\nfff\nggg\nhhh\niii\njjj\nkkk\nlll\nmmm\nnnn\nooo\nppp\nqqq\nRRR\nsss\nttt\nuuu\nvvv\nwww\nxxx\nyyy\nzzz\n", + y: "aaa\nbbb\nCCC\nddd\neee\n\"\"\"\nggg\nhhh\niii\njjj\nkkk\nlll\nmmm\nnnn\nooo\nppp\nqqq\nrrr\nsss\nttt\nuuu\nvvv\nwww\nxxx\nyyy\nzzz\n", + wantEqual: false, + reason: "avoid triple-quote syntax due to presence of ambiguous triple quotes", + }, { + label: label + "/AvoidTripleQuoteAmbiguousEllipsis", + x: "aaa\nbbb\nccc\n...\neee\nfff\nggg\nhhh\niii\njjj\nkkk\nlll\nmmm\nnnn\nooo\nppp\nqqq\nRRR\nsss\nttt\nuuu\nvvv\nwww\nxxx\nyyy\nzzz\n", + y: "aaa\nbbb\nCCC\nddd\neee\nfff\nggg\nhhh\niii\njjj\nkkk\nlll\nmmm\nnnn\nooo\nppp\nqqq\nrrr\nsss\nttt\nuuu\nvvv\nwww\nxxx\nyyy\nzzz\n", + wantEqual: false, + reason: "avoid triple-quote syntax due to presence of ambiguous ellipsis", + }, { + label: label + "/AvoidTripleQuoteNonPrintable", + x: "aaa\nbbb\nccc\nddd\neee\nfff\nggg\nhhh\niii\njjj\nkkk\nlll\nmmm\nnnn\nooo\nppp\nqqq\nRRR\nsss\nttt\nuuu\nvvv\nwww\nxxx\nyyy\nzzz\n", + y: "aaa\nbbb\nCCC\nddd\neee\nfff\nggg\nhhh\niii\njjj\nkkk\nlll\nmmm\nnnn\no\roo\nppp\nqqq\nrrr\nsss\nttt\nuuu\nvvv\nwww\nxxx\nyyy\nzzz\n", + wantEqual: false, + reason: "use triple-quote syntax", + }, { + label: label + "/AvoidTripleQuoteIdenticalWhitespace", + x: "aaa\nbbb\nccc\n ddd\neee\nfff\nggg\nhhh\niii\njjj\nkkk\nlll\nmmm\nnnn\nooo\nppp\nqqq\nRRR\nsss\nttt\nuuu\nvvv\nwww\nxxx\nyyy\nzzz\n", + y: "aaa\nbbb\nccc \nddd\neee\nfff\nggg\nhhh\niii\njjj\nkkk\nlll\nmmm\nnnn\nooo\nppp\nqqq\nrrr\nsss\nttt\nuuu\nvvv\nwww\nxxx\nyyy\nzzz\n", + wantEqual: false, + reason: "avoid triple-quote syntax due to visual equivalence of differences", }, { label: label, x: MyComposite{ diff --git a/cmp/report_slices.go b/cmp/report_slices.go index 344cbac..81dedfd 100644 --- a/cmp/report_slices.go +++ b/cmp/report_slices.go @@ -8,6 +8,7 @@ import ( "bytes" "fmt" "reflect" + "strconv" "strings" "unicode" "unicode/utf8" @@ -84,7 +85,7 @@ func (opts formatOptions) FormatDiffSlice(v *valueNode) textNode { } if isText || isBinary { var numLines, lastLineIdx, maxLineLen int - isBinary = false + isBinary = !utf8.ValidString(sx) || !utf8.ValidString(sy) for i, r := range sx + sy { if !(unicode.IsPrint(r) || unicode.IsSpace(r)) || r == utf8.RuneError { isBinary = true @@ -119,6 +120,54 @@ func (opts formatOptions) FormatDiffSlice(v *valueNode) textNode { }, ) delim = "\n" + + // If possible, use a custom triple-quote (""") syntax for printing + // differences in a string literal. This format is more readable, + // but has edge-cases where differences are visually indistinguishable. + // This format is avoided under the following conditions: + // • A line starts with `"""` + // • A line starts with "..." + // • A line contains non-printable characters + // • Adjacent different lines differ only by whitespace + isTripleQuoted := true + prevDiffLines := map[string]bool{} + var list2 textList + list2 = append(list2, textRecord{Value: textLine(`"""`), ElideComma: true}) + for _, r := range list { + if !r.Value.Equal(textEllipsis) { + line, _ := strconv.Unquote(string(r.Value.(textLine))) + line = strings.TrimPrefix(strings.TrimSuffix(line, "\r"), "\r") // trim leading/trailing carriage returns for legacy Windows endline support + normLine := strings.Map(func(r rune) rune { + if unicode.IsSpace(r) { + return -1 // drop spaces to avoid visually indistinguishable output due to whitespace + } + return r + }, line) + isTripleQuoted = isTripleQuoted && + !strings.HasPrefix(line, `"""`) && + !strings.HasPrefix(line, "...") && + strings.TrimFunc(line, func(r rune) bool { return unicode.IsPrint(r) || r == '\t' }) == "" && + (r.Diff == 0 || !prevDiffLines[normLine]) + if !isTripleQuoted { + break + } + r.Value = textLine(line) + r.ElideComma = true + prevDiffLines[normLine] = true + } + if r.Diff == 0 { + prevDiffLines = map[string]bool{} // start a new non-adjacent difference group + } + list2 = append(list2, r) + } + if r := list2[len(list2)-1]; r.Diff == 0 && len(r.Value.(textLine)) == 0 { + list2 = list2[:len(list2)-1] // elide single empty line at the end + } + list2 = append(list2, textRecord{Value: textLine(`"""`), ElideComma: true}) + if isTripleQuoted { + return textWrap{"(", list2, ")"} + } + // If the text appears to be single-lined text, // then perform differencing in approximately fixed-sized chunks. // The output is printed as quoted strings. diff --git a/cmp/report_text.go b/cmp/report_text.go index 8b8fcab..37d6f6c 100644 --- a/cmp/report_text.go +++ b/cmp/report_text.go @@ -136,10 +136,11 @@ func (s textWrap) formatExpandedTo(b []byte, d diffMode, n indentMode) []byte { // of the textList.formatCompactTo method. type textList []textRecord type textRecord struct { - Diff diffMode // e.g., 0 or '-' or '+' - Key string // e.g., "MyField" - Value textNode // textWrap | textLine - Comment fmt.Stringer // e.g., "6 identical fields" + Diff diffMode // e.g., 0 or '-' or '+' + Key string // e.g., "MyField" + Value textNode // textWrap | textLine + ElideComma bool // avoid trailing comma + Comment fmt.Stringer // e.g., "6 identical fields" } // AppendEllipsis appends a new ellipsis node to the list if none already @@ -149,9 +150,9 @@ func (s *textList) AppendEllipsis(ds diffStats) { hasStats := ds != diffStats{} if len(*s) == 0 || !(*s)[len(*s)-1].Value.Equal(textEllipsis) { if hasStats { - *s = append(*s, textRecord{Value: textEllipsis, Comment: ds}) + *s = append(*s, textRecord{Value: textEllipsis, ElideComma: true, Comment: ds}) } else { - *s = append(*s, textRecord{Value: textEllipsis}) + *s = append(*s, textRecord{Value: textEllipsis, ElideComma: true}) } return } @@ -256,7 +257,7 @@ func (s textList) formatExpandedTo(b []byte, d diffMode, n indentMode) []byte { b = alignKeyLens[i].appendChar(b, ' ') b = r.Value.formatExpandedTo(b, d|r.Diff, n) - if !r.Value.Equal(textEllipsis) { + if !r.ElideComma { b = append(b, ',') } b = alignValueLens[i].appendChar(b, ' ') diff --git a/cmp/testdata/diffs b/cmp/testdata/diffs index 56a9a32..2c6316c 100644 --- a/cmp/testdata/diffs +++ b/cmp/testdata/diffs @@ -413,6 +413,135 @@ ... // 9 identical fields } >>> TestDiff/Reporter#05 +<<< TestDiff/Reporter/TripleQuote + ( + """ + aaa + bbb +- ccc ++ CCC + ddd + eee + ... // 10 identical lines + ppp + qqq +- RRR +- sss ++ rrr ++ SSS + ttt + uuu + ... // 6 identical lines + """ + ) +>>> TestDiff/Reporter/TripleQuote +<<< TestDiff/Reporter/TripleQuoteEndlines + ( + """ + aaa + bbb +- ccc ++ CCC + ddd + eee + ... // 10 identical lines + ppp + qqq +- RRR ++ rrr + sss + ttt + ... // 4 identical lines + yyy + zzz +- + """ + ) +>>> TestDiff/Reporter/TripleQuoteEndlines +<<< TestDiff/Reporter/AvoidTripleQuoteAmbiguousQuotes + strings.Join({ + "aaa", + "bbb", +- "ccc", ++ "CCC", + "ddd", + "eee", +- "fff", ++ `"""`, + "ggg", + "hhh", + ... // 7 identical lines + "ppp", + "qqq", +- "RRR", ++ "rrr", + "sss", + "ttt", + ... // 7 identical lines + }, "\n") +>>> TestDiff/Reporter/AvoidTripleQuoteAmbiguousQuotes +<<< TestDiff/Reporter/AvoidTripleQuoteAmbiguousEllipsis + strings.Join({ + "aaa", + "bbb", +- "ccc", +- "...", ++ "CCC", ++ "ddd", + "eee", + "fff", + ... // 9 identical lines + "ppp", + "qqq", +- "RRR", ++ "rrr", + "sss", + "ttt", + ... // 7 identical lines + }, "\n") +>>> TestDiff/Reporter/AvoidTripleQuoteAmbiguousEllipsis +<<< TestDiff/Reporter/AvoidTripleQuoteNonPrintable + strings.Join({ + "aaa", + "bbb", +- "ccc", ++ "CCC", + "ddd", + "eee", + ... // 7 identical lines + "mmm", + "nnn", +- "ooo", ++ "o\roo", + "ppp", + "qqq", +- "RRR", ++ "rrr", + "sss", + "ttt", + ... // 7 identical lines + }, "\n") +>>> TestDiff/Reporter/AvoidTripleQuoteNonPrintable +<<< TestDiff/Reporter/AvoidTripleQuoteIdenticalWhitespace + strings.Join({ + "aaa", + "bbb", +- "ccc", +- " ddd", ++ "ccc ", ++ "ddd", + "eee", + "fff", + ... // 9 identical lines + "ppp", + "qqq", +- "RRR", ++ "rrr", + "sss", + "ttt", + ... // 7 identical lines + }, "\n") +>>> TestDiff/Reporter/AvoidTripleQuoteIdenticalWhitespace <<< TestDiff/Reporter#06 cmp_test.MyComposite{ StringA: strings.Join({