Skip to content

Commit

Permalink
Reduce minimum length for specialize string diffing (#275)
Browse files Browse the repository at this point in the history
The original threshold of 64 was chosen without much thought.
Lower it to 32 now that we have some concrete examples
that it is aesthetically better.

Co-authored-by: Damien Neil <neild@users.noreply.github.com>
  • Loading branch information
dsnet and neild authored Oct 12, 2021
1 parent f1773ad commit 6faefd0
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 1 deletion.
13 changes: 13 additions & 0 deletions cmp/compare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1338,6 +1338,19 @@ using the AllowUnexported option.`, "\n"),
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",
}, {
label: label + "/ShortJSON",
x: `{
"id": 1,
"foo": true,
"bar": true,
}`,
y: `{
"id": 1434180,
"foo": true,
"bar": true,
}`,
reason: "short multiline JSON should prefer triple-quoted string diff as it is more readable",
}}
}

Expand Down
2 changes: 1 addition & 1 deletion cmp/report_slices.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func (opts formatOptions) CanFormatDiffSlice(v *valueNode) bool {
}

// Use specialized string diffing for longer slices or strings.
const minLength = 64
const minLength = 32
return vx.Len() >= minLength && vy.Len() >= minLength
}

Expand Down
12 changes: 12 additions & 0 deletions cmp/testdata/diffs
Original file line number Diff line number Diff line change
Expand Up @@ -1122,6 +1122,18 @@
+ X: nil,
}
>>> TestDiff/Reporter/NonStringifiedNamedBytes
<<< TestDiff/Reporter/ShortJSON
(
"""
{
- "id": 1,
+ "id": 1434180,
"foo": true,
"bar": true,
}
"""
)
>>> TestDiff/Reporter/ShortJSON
<<< TestDiff/EmbeddedStruct/ParentStructA/Inequal
teststructs.ParentStructA{
privateStruct: teststructs.privateStruct{
Expand Down

0 comments on commit 6faefd0

Please sign in to comment.