Skip to content

Commit 9a5dd41

Browse files
author
Neil Fraser
committed
Fix empty diff operations in cleanupMerge
Python was creating invalid diffs. Java was fine. Other languages were creating sub-optimal diffs.
1 parent 0ee3cb5 commit 9a5dd41

File tree

18 files changed

+203
-168
lines changed

18 files changed

+203
-168
lines changed

‎csharp/DiffMatchPatch.cs‎

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -676,7 +676,7 @@ protected void diff_charsToLines(ICollection<Diff> diffs,
676676
* @return The number of characters common to the start of each string.
677677
*/
678678
public int diff_commonPrefix(string text1, string text2) {
679-
// Performance analysis: http://neil.fraser.name/news/2007/10/09/
679+
// Performance analysis: https://neil.fraser.name/news/2007/10/09/
680680
int n = Math.Min(text1.Length, text2.Length);
681681
for (int i = 0; i < n; i++) {
682682
if (text1[i] != text2[i]) {
@@ -693,7 +693,7 @@ public int diff_commonPrefix(string text1, string text2) {
693693
* @return The number of characters common to the end of each string.
694694
*/
695695
public int diff_commonSuffix(string text1, string text2) {
696-
// Performance analysis: http://neil.fraser.name/news/2007/10/09/
696+
// Performance analysis: https://neil.fraser.name/news/2007/10/09/
697697
int text1_length = text1.Length;
698698
int text2_length = text2.Length;
699699
int n = Math.Min(text1.Length, text2.Length);
@@ -734,7 +734,7 @@ protected int diff_commonOverlap(string text1, string text2) {
734734

735735
// Start by looking for a single character match
736736
// and increase length until no match is found.
737-
// Performance analysis: http://neil.fraser.name/news/2010/11/04/
737+
// Performance analysis: https://neil.fraser.name/news/2010/11/04/
738738
int best = 0;
739739
int length = 1;
740740
while (true) {
@@ -1220,22 +1220,19 @@ public void diff_cleanupMerge(List<Diff> diffs) {
12201220
}
12211221
}
12221222
// Delete the offending records and add the merged ones.
1223-
if (count_delete == 0) {
1224-
diffs.Splice(pointer - count_insert,
1225-
count_delete + count_insert,
1226-
new Diff(Operation.INSERT, text_insert));
1227-
} else if (count_insert == 0) {
1228-
diffs.Splice(pointer - count_delete,
1229-
count_delete + count_insert,
1223+
pointer -= count_delete + count_insert;
1224+
diffs.Splice(pointer, count_delete + count_insert);
1225+
if (text_delete.Length != 0) {
1226+
diffs.Splice(pointer, 0,
12301227
new Diff(Operation.DELETE, text_delete));
1231-
} else {
1232-
diffs.Splice(pointer - count_delete - count_insert,
1233-
count_delete + count_insert,
1234-
new Diff(Operation.DELETE, text_delete),
1228+
pointer++;
1229+
}
1230+
if (text_insert.Length != 0) {
1231+
diffs.Splice(pointer, 0,
12351232
new Diff(Operation.INSERT, text_insert));
1233+
pointer++;
12361234
}
1237-
pointer = pointer - count_delete - count_insert +
1238-
(count_delete != 0 ? 1 : 0) + (count_insert != 0 ? 1 : 0) + 1;
1235+
pointer++;
12391236
} else if (pointer != 0
12401237
&& diffs[pointer - 1].operation == Operation.EQUAL) {
12411238
// Merge this equality with the previous one.
@@ -1860,7 +1857,7 @@ public List<Patch> patch_make(string text1, List<Diff> diffs) {
18601857
patches.Add(patch);
18611858
patch = new Patch();
18621859
// Unlike Unidiff, our patch lists have a rolling context.
1863-
// http://code.google.com/p/google-diff-match-patch/wiki/Unidiff
1860+
// https://github.com/google/diff-match-patch/wiki/Unidiff
18641861
// Update prepatch text & pos to reflect the application of the
18651862
// just completed patch.
18661863
prepatch_text = postpatch_text;

‎csharp/tests/DiffMatchPatchTest.cs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,14 @@ public void diff_cleanupMergeTest() {
240240
diffs = new List<Diff> {new Diff(Operation.EQUAL, "x"), new Diff(Operation.DELETE, "ca"), new Diff(Operation.EQUAL, "c"), new Diff(Operation.DELETE, "b"), new Diff(Operation.EQUAL, "a")};
241241
this.diff_cleanupMerge(diffs);
242242
assertEquals("diff_cleanupMerge: Slide edit right recursive.", new List<Diff> {new Diff(Operation.EQUAL, "xca"), new Diff(Operation.DELETE, "cba")}, diffs);
243+
244+
diffs = new List<Diff> {new Diff(Operation.DELETE, "b"), new Diff(Operation.INSERT, "ab"), new Diff(Operation.EQUAL, "c")};
245+
this.diff_cleanupMerge(diffs);
246+
assertEquals("diff_cleanupMerge: Empty merge.", new List<Diff> {new Diff(Operation.INSERT, "a"), new Diff(Operation.EQUAL, "bc")}, diffs);
247+
248+
diffs = new List<Diff> {new Diff(Operation.EQUAL, ""), new Diff(Operation.INSERT, "a"), new Diff(Operation.EQUAL, "b")};
249+
this.diff_cleanupMerge(diffs);
250+
assertEquals("diff_cleanupMerge: Empty equality.", new List<Diff> {new Diff(Operation.INSERT, "a"), new Diff(Operation.EQUAL, "b")}, diffs);
243251
}
244252

245253
public void diff_cleanupSemanticLosslessTest() {

‎dart/DMPClass.dart‎

Lines changed: 14 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,7 @@ class DiffMatchPatch {
529529
int diff_commonPrefix(String text1, String text2) {
530530
// TODO: Once Dart's performance stabilizes, determine if linear or binary
531531
// search is better.
532-
// Performance analysis: http://neil.fraser.name/news/2007/10/09/
532+
// Performance analysis: https://neil.fraser.name/news/2007/10/09/
533533
final n = min(text1.length, text2.length);
534534
for (int i = 0; i < n; i++) {
535535
if (text1[i] != text2[i]) {
@@ -548,7 +548,7 @@ class DiffMatchPatch {
548548
int diff_commonSuffix(String text1, String text2) {
549549
// TODO: Once Dart's performance stabilizes, determine if linear or binary
550550
// search is better.
551-
// Performance analysis: http://neil.fraser.name/news/2007/10/09/
551+
// Performance analysis: https://neil.fraser.name/news/2007/10/09/
552552
final text1_length = text1.length;
553553
final text2_length = text2.length;
554554
final n = min(text1_length, text2_length);
@@ -589,7 +589,7 @@ class DiffMatchPatch {
589589

590590
// Start by looking for a single character match
591591
// and increase length until no match is found.
592-
// Performance analysis: http://neil.fraser.name/news/2010/11/04/
592+
// Performance analysis: https://neil.fraser.name/news/2010/11/04/
593593
int best = 0;
594594
int length = 1;
595595
while (true) {
@@ -1094,27 +1094,17 @@ class DiffMatchPatch {
10941094
}
10951095
}
10961096
// Delete the offending records and add the merged ones.
1097-
if (count_delete == 0) {
1098-
diffs.removeRange(pointer - count_insert, pointer);
1099-
diffs.insert(pointer - count_insert,
1100-
new Diff(Operation.insert, text_insert));
1101-
} else if (count_insert == 0) {
1102-
diffs.removeRange(pointer - count_delete, pointer);
1103-
diffs.insert(pointer - count_delete,
1104-
new Diff(Operation.delete, text_delete));
1105-
} else {
1106-
diffs.replaceRange(
1107-
pointer - count_delete - count_insert, pointer, [
1108-
new Diff(Operation.delete, text_delete),
1109-
new Diff(Operation.insert, text_insert)
1110-
]);
1097+
pointer -= count_delete + count_insert;
1098+
diffs.removeRange(pointer, pointer + count_delete + count_insert);
1099+
if (!text_delete.isEmpty) {
1100+
diffs.insert(pointer, new Diff(Operation.delete, text_delete));
1101+
pointer++;
1102+
}
1103+
if (!text_insert.isEmpty) {
1104+
diffs.insert(pointer, new Diff(Operation.insert, text_insert));
1105+
pointer++;
11111106
}
1112-
pointer = pointer -
1113-
count_delete -
1114-
count_insert +
1115-
(count_delete == 0 ? 0 : 1) +
1116-
(count_insert == 0 ? 0 : 1) +
1117-
1;
1107+
pointer++;
11181108
} else if (pointer != 0 &&
11191109
diffs[pointer - 1].operation == Operation.equal) {
11201110
// Merge this equality with the previous one.
@@ -1725,7 +1715,7 @@ class DiffMatchPatch {
17251715
patches.add(patch);
17261716
patch = new Patch();
17271717
// Unlike Unidiff, our patch lists have a rolling context.
1728-
// http://code.google.com/p/google-diff-match-patch/wiki/Unidiff
1718+
// https://github.com/google/diff-match-patch/wiki/Unidiff
17291719
// Update prepatch text & pos to reflect the application of the
17301720
// just completed patch.
17311721
prepatch_text = postpatch_text;

‎dart/tests/DiffMatchPatchTest.dart‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,14 @@ void testDiffCleanupMerge() {
286286
diffs = [new Diff(Operation.equal, 'x'), new Diff(Operation.delete, 'ca'), new Diff(Operation.equal, 'c'), new Diff(Operation.delete, 'b'), new Diff(Operation.equal, 'a')];
287287
dmp.diff_cleanupMerge(diffs);
288288
Expect.listEquals([new Diff(Operation.equal, 'xca'), new Diff(Operation.delete, 'cba')], diffs, 'diff_cleanupMerge: Slide edit right recursive.');
289+
290+
diffs = [new Diff(Operation.delete, 'b'), new Diff(Operation.insert, 'ab'), new Diff(Operation.equal, 'c')];
291+
dmp.diff_cleanupMerge(diffs);
292+
Expect.listEquals([new Diff(Operation.insert, 'a'), new Diff(Operation.equal, 'bc')], diffs, 'diff_cleanupMerge: Empty merge.');
293+
294+
diffs = [new Diff(Operation.equal, ''), new Diff(Operation.insert, 'a'), new Diff(Operation.equal, 'b')];
295+
dmp.diff_cleanupMerge(diffs);
296+
Expect.listEquals([new Diff(Operation.insert, 'a'), new Diff(Operation.equal, 'b')], diffs, 'diff_cleanupMerge: Empty equality.');
289297
}
290298

291299
void testDiffCleanupSemanticLossless() {

‎java/src/name/fraser/neil/plaintext/diff_match_patch.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -591,7 +591,7 @@ protected void diff_charsToLines(List<Diff> diffs,
591591
* @return The number of characters common to the start of each string.
592592
*/
593593
public int diff_commonPrefix(String text1, String text2) {
594-
// Performance analysis: http://neil.fraser.name/news/2007/10/09/
594+
// Performance analysis: https://neil.fraser.name/news/2007/10/09/
595595
int n = Math.min(text1.length(), text2.length());
596596
for (int i = 0; i < n; i++) {
597597
if (text1.charAt(i) != text2.charAt(i)) {
@@ -608,7 +608,7 @@ public int diff_commonPrefix(String text1, String text2) {
608608
* @return The number of characters common to the end of each string.
609609
*/
610610
public int diff_commonSuffix(String text1, String text2) {
611-
// Performance analysis: http://neil.fraser.name/news/2007/10/09/
611+
// Performance analysis: https://neil.fraser.name/news/2007/10/09/
612612
int text1_length = text1.length();
613613
int text2_length = text2.length();
614614
int n = Math.min(text1_length, text2_length);
@@ -649,7 +649,7 @@ protected int diff_commonOverlap(String text1, String text2) {
649649

650650
// Start by looking for a single character match
651651
// and increase length until no match is found.
652-
// Performance analysis: http://neil.fraser.name/news/2010/11/04/
652+
// Performance analysis: https://neil.fraser.name/news/2010/11/04/
653653
int best = 0;
654654
int length = 1;
655655
while (true) {
@@ -1875,7 +1875,7 @@ public LinkedList<Patch> patch_make(String text1, LinkedList<Diff> diffs) {
18751875
patches.add(patch);
18761876
patch = new Patch();
18771877
// Unlike Unidiff, our patch lists have a rolling context.
1878-
// http://code.google.com/p/google-diff-match-patch/wiki/Unidiff
1878+
// https://github.com/google/diff-match-patch/wiki/Unidiff
18791879
// Update prepatch text & pos to reflect the application of the
18801880
// just completed patch.
18811881
prepatch_text = postpatch_text;

‎java/tests/name/fraser/neil/plaintext/diff_match_patch_test.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,14 @@ public static void testDiffCleanupMerge() {
243243
diffs = diffList(new Diff(EQUAL, "x"), new Diff(DELETE, "ca"), new Diff(EQUAL, "c"), new Diff(DELETE, "b"), new Diff(EQUAL, "a"));
244244
dmp.diff_cleanupMerge(diffs);
245245
assertEquals("diff_cleanupMerge: Slide edit right recursive.", diffList(new Diff(EQUAL, "xca"), new Diff(DELETE, "cba")), diffs);
246+
247+
diffs = diffList(new Diff(DELETE, "b"), new Diff(INSERT, "ab"), new Diff(EQUAL, "c"));
248+
dmp.diff_cleanupMerge(diffs);
249+
assertEquals("diff_cleanupMerge: Empty merge.", diffList(new Diff(INSERT, "a"), new Diff(EQUAL, "bc")), diffs);
250+
251+
diffs = diffList(new Diff(EQUAL, ""), new Diff(INSERT, "a"), new Diff(EQUAL, "b"));
252+
dmp.diff_cleanupMerge(diffs);
253+
assertEquals("diff_cleanupMerge: Empty equality.", diffList(new Diff(INSERT, "a"), new Diff(EQUAL, "b")), diffs);
246254
}
247255

248256
public static void testDiffCleanupSemanticLossless() {

‎javascript/diff_match_patch.js‎

Lines changed: 2 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎javascript/diff_match_patch_uncompressed.js‎

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,7 @@ diff_match_patch.prototype.diff_commonPrefix = function(text1, text2) {
530530
return 0;
531531
}
532532
// Binary search.
533-
// Performance analysis: http://neil.fraser.name/news/2007/10/09/
533+
// Performance analysis: https://neil.fraser.name/news/2007/10/09/
534534
var pointermin = 0;
535535
var pointermax = Math.min(text1.length, text2.length);
536536
var pointermid = pointermax;
@@ -562,7 +562,7 @@ diff_match_patch.prototype.diff_commonSuffix = function(text1, text2) {
562562
return 0;
563563
}
564564
// Binary search.
565-
// Performance analysis: http://neil.fraser.name/news/2007/10/09/
565+
// Performance analysis: https://neil.fraser.name/news/2007/10/09/
566566
var pointermin = 0;
567567
var pointermax = Math.min(text1.length, text2.length);
568568
var pointermid = pointermax;
@@ -611,7 +611,7 @@ diff_match_patch.prototype.diff_commonOverlap_ = function(text1, text2) {
611611

612612
// Start by looking for a single character match
613613
// and increase length until no match is found.
614-
// Performance analysis: http://neil.fraser.name/news/2010/11/04/
614+
// Performance analysis: https://neil.fraser.name/news/2010/11/04/
615615
var best = 0;
616616
var length = 1;
617617
while (true) {
@@ -1112,19 +1112,17 @@ diff_match_patch.prototype.diff_cleanupMerge = function(diffs) {
11121112
}
11131113
}
11141114
// Delete the offending records and add the merged ones.
1115-
if (count_delete === 0) {
1116-
diffs.splice(pointer - count_insert,
1117-
count_delete + count_insert, [DIFF_INSERT, text_insert]);
1118-
} else if (count_insert === 0) {
1119-
diffs.splice(pointer - count_delete,
1120-
count_delete + count_insert, [DIFF_DELETE, text_delete]);
1121-
} else {
1122-
diffs.splice(pointer - count_delete - count_insert,
1123-
count_delete + count_insert, [DIFF_DELETE, text_delete],
1124-
[DIFF_INSERT, text_insert]);
1115+
pointer -= count_delete + count_insert;
1116+
diffs.splice(pointer, count_delete + count_insert);
1117+
if (text_delete.length) {
1118+
diffs.splice(pointer, 0, [DIFF_DELETE, text_delete]);
1119+
pointer++;
1120+
}
1121+
if (text_insert.length) {
1122+
diffs.splice(pointer, 0, [DIFF_INSERT, text_insert]);
1123+
pointer++;
11251124
}
1126-
pointer = pointer - count_delete - count_insert +
1127-
(count_delete ? 1 : 0) + (count_insert ? 1 : 0) + 1;
1125+
pointer++;
11281126
} else if (pointer !== 0 && diffs[pointer - 1][0] == DIFF_EQUAL) {
11291127
// Merge this equality with the previous one.
11301128
diffs[pointer - 1][1] += diffs[pointer][1];
@@ -1725,7 +1723,7 @@ diff_match_patch.prototype.patch_make = function(a, opt_b, opt_c) {
17251723
patch = new diff_match_patch.patch_obj();
17261724
patchDiffLength = 0;
17271725
// Unlike Unidiff, our patch lists have a rolling context.
1728-
// http://code.google.com/p/google-diff-match-patch/wiki/Unidiff
1726+
// https://github.com/google/diff-match-patch/wiki/Unidiff
17291727
// Update prepatch text & pos to reflect the application of the
17301728
// just completed patch.
17311729
prepatch_text = postpatch_text;

‎javascript/tests/diff_match_patch_test.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,16 @@ function testDiffCleanupMerge() {
282282
diffs = [[DIFF_EQUAL, 'x'], [DIFF_DELETE, 'ca'], [DIFF_EQUAL, 'c'], [DIFF_DELETE, 'b'], [DIFF_EQUAL, 'a']];
283283
dmp.diff_cleanupMerge(diffs);
284284
assertEquivalent([[DIFF_EQUAL, 'xca'], [DIFF_DELETE, 'cba']], diffs);
285+
286+
// Empty merge.
287+
diffs = [[DIFF_DELETE, 'b'], [DIFF_INSERT, 'ab'], [DIFF_EQUAL, 'c']];
288+
dmp.diff_cleanupMerge(diffs);
289+
assertEquivalent([[DIFF_INSERT, 'a'], [DIFF_EQUAL, 'bc']], diffs);
290+
291+
// Empty equality.
292+
diffs = [[DIFF_EQUAL, ''], [DIFF_INSERT, 'a'], [DIFF_EQUAL, 'b']];
293+
dmp.diff_cleanupMerge(diffs);
294+
assertEquivalent([[DIFF_INSERT, 'a'], [DIFF_EQUAL, 'b']], diffs);
285295
}
286296

287297
function testDiffCleanupSemanticLossless() {

0 commit comments

Comments
 (0)