Conversation
|
\cc @rogpeppe, this PR will fix the sub-optimal printing of the diffs. |
rogpeppe
left a comment
rogpeppe
left a comment
There was a problem hiding this comment.
LGTM - this looks like it'll be a huge improvement, thanks!
Note: I haven't looked at the detail of the implementation properly I'm afraid, but I'm very happy to see this change.
Even with an optimal diffing algoritm, coalescing adjacent edit groups
may cause the corresponding pair of strings for an edit group to have
leading or trailing spans of equal elements.
While this is technically a correct representation of a diff,
it is a suboptimal outcome. As such, scan through all unequal groups and
move leading/trailing equal spans to the preceding/succeeding equal group.
Before this change:
strings.Join({
"org-4747474747474747,bucket-4242424242424242:m,tag1=a,tag2=aa",
- ",#=_value _value=2 ",
+ " _value=2 ",
`11 org-4747474747474747,bucket-4242424242424242:m,tag1=a,tag2=bb`,
- ",#=_value _value=2 2",
+ " _value=2 2",
`1 org-4747474747474747,bucket-4242424242424242:m,tag1=b,tag2=cc`,
- ",#=_value",
` _value=1 21 org-4747474747474747,bucket-4242424242424242:m,tag1`,
"=a,tag2",
- "=dd,#=_value",
+ "=dd",
` _value=3 31 org-4747474747474747,bucket-4242424242424242:m,tag1`,
- "=c,#=_value",
+ "=c",
` _value=4 41 `,
}, "")
After this change:
strings.Join({
"org-4747474747474747,bucket-4242424242424242:m,tag1=a,tag2=aa",
- ",#=_value",
` _value=2 11 org-4747474747474747,bucket-4242424242424242:m,tag1`,
"=a,tag2=bb",
- ",#=_value",
` _value=2 21 org-4747474747474747,bucket-4242424242424242:m,tag1`,
"=b,tag2=cc",
- ",#=_value",
` _value=1 21 org-4747474747474747,bucket-4242424242424242:m,tag1`,
"=a,tag2=dd",
- ",#=_value",
` _value=3 31 org-4747474747474747,bucket-4242424242424242:m,tag1`,
"=c",
- ",#=_value",
` _value=4 41 `,
}, "")
AnomalRoil
left a comment
AnomalRoil
left a comment
There was a problem hiding this comment.
I know this is already merged, but since I'm reviewing our dependencies before updating, I'm just letting you know my thoughts ;)
Thanks for the comments, it helped a lot with the review process.
| nx := ds.NumIdentical + ds.NumRemoved + ds.NumModified | ||
| ny := ds.NumIdentical + ds.NumInserted + ds.NumModified | ||
| var numLeadingIdentical, numTrailingIdentical int | ||
| for i := 0; i < nx && i < ny && eq(ix+i, iy+i); i++ { |
There was a problem hiding this comment.
The shadowing of i is not helping with readability.
| for i := 0; i < nx && i < ny && eq(ix+i, iy+i); i++ { | ||
| numLeadingIdentical++ | ||
| } | ||
| for i := 0; i < nx && i < ny && eq(ix+nx-1-i, iy+ny-1-i); i++ { |
|
@AnomalRoil, I commend you for reviewing changes to your dependencies! This is something I wish people would do more often. Thank you. |
|
Yup, and that's especially true when dealing with crypto & passwords, since these are quite sensitive. |
4 participants
Even with an optimal diffing algoritm, coalescing adjacent edit groups
may cause the corresponding pair of strings for an edit group to have
leading or trailing spans of equal elements.
While this is technically a correct representation of a diff,
it is a suboptimal outcome. As such, scan through all unequal groups and
move leading/trailing equal spans to the preceding/succeeding equal group.
Before this change:
strings.Join({ "org-4747474747474747,bucket-4242424242424242:m,tag1=a,tag2=aa", - ",#=_value _value=2 ", + " _value=2 ", `11 org-4747474747474747,bucket-4242424242424242:m,tag1=a,tag2=bb`, - ",#=_value _value=2 2", + " _value=2 2", `1 org-4747474747474747,bucket-4242424242424242:m,tag1=b,tag2=cc`, - ",#=_value", ` _value=1 21 org-4747474747474747,bucket-4242424242424242:m,tag1`, "=a,tag2", - "=dd,#=_value", + "=dd", ` _value=3 31 org-4747474747474747,bucket-4242424242424242:m,tag1`, - "=c,#=_value", + "=c", ` _value=4 41 `, }, "")After this change:
strings.Join({ "org-4747474747474747,bucket-4242424242424242:m,tag1=a,tag2=aa", - ",#=_value", ` _value=2 11 org-4747474747474747,bucket-4242424242424242:m,tag1`, "=a,tag2=bb", - ",#=_value", ` _value=2 21 org-4747474747474747,bucket-4242424242424242:m,tag1`, "=b,tag2=cc", - ",#=_value", ` _value=1 21 org-4747474747474747,bucket-4242424242424242:m,tag1`, "=a,tag2=dd", - ",#=_value", ` _value=3 31 org-4747474747474747,bucket-4242424242424242:m,tag1`, "=c", - ",#=_value", ` _value=4 41 `, }, "")