[MediaWiki-commits] [Gerrit] VisualEditor/VisualEditor[master]: VisualDiff: Don't diff close elements
jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/374340 )
Change subject: VisualDiff: Don't diff close elements
..
VisualDiff: Don't diff close elements
Force the content diff to be balanced by removing
close elements prior to diffing, then re-inserting
afterwards.
Bug: T171862
Change-Id: I84aa157897ed5774b170e0dc4a3f7d264663c24d
---
M src/dm/ve.dm.VisualDiff.js
M src/ve.DiffMatchPatch.js
M tests/ui/ve.ui.DiffElement.test.js
3 files changed, 64 insertions(+), 7 deletions(-)
Approvals:
Esanders: Looks good to me, approved
jenkins-bot: Verified
diff --git a/src/dm/ve.dm.VisualDiff.js b/src/dm/ve.dm.VisualDiff.js
index 524cd89..022ee44 100644
--- a/src/dm/ve.dm.VisualDiff.js
+++ b/src/dm/ve.dm.VisualDiff.js
@@ -562,8 +562,7 @@
// Diff internal list items
diff = this.computeDiff(
oldDocInternalListItems.toDiff,
- newDocInternalListItems.toDiff,
- this.internalListDiff
+ newDocInternalListItems.toDiff
);
// Check there actually are any changes
diff --git a/src/ve.DiffMatchPatch.js b/src/ve.DiffMatchPatch.js
index 3d20936..b0f19a7 100644
--- a/src/ve.DiffMatchPatch.js
+++ b/src/ve.DiffMatchPatch.js
@@ -70,14 +70,39 @@
return [];
};
-ve.DiffMatchPatch.prototype.getCleanDiff = function () {
- var diffs = this.diff_main.apply( this, arguments ),
+ve.DiffMatchPatch.prototype.getCleanDiff = function ( oldData, newData,
options ) {
+ var cleanDiff, i, ilen, j,
store = this.store,
DIFF_DELETE = this.constructor.static.DIFF_DELETE,
DIFF_INSERT = this.constructor.static.DIFF_INSERT,
DIFF_EQUAL = this.constructor.static.DIFF_EQUAL,
DIFF_CHANGE_DELETE = this.constructor.static.DIFF_CHANGE_DELETE,
DIFF_CHANGE_INSERT = this.constructor.static.DIFF_CHANGE_INSERT;
+
+ /**
+* Remove the close elements from the linear data, to force a balanced
diff.
+*
+* Otherwise, for example, removing the second of two consecutive
comment
+* nodes could result in a diff where [{type: '/comment'}, {type:
'comment'}]
+* is removed, which is unbalanced.
+*
+* Warning: this step assumes that, within a content branch node, an
element
+* is always immediately followed by its close element.
+*
+* @param {Array} data Linear data
+* @return {Array} Linear data without close elements
+*/
+ function removeCloseElements( data ) {
+ var i, ilen;
+ for ( i = 0, ilen = data.length; i < ilen; i++ ) {
+ if ( data[ i ].type && data[ i ].type[ 0 ] === '/' ) {
+ data.splice( i, 1 );
+ ilen--;
+ i--;
+ }
+ }
+ return data;
+ }
/**
* Get the index of the the first or last wordbreak in a data array
@@ -285,8 +310,8 @@
cleanDiff.push( [ DIFF_INSERT, insert ] );
}
- // Finally, go over any consecutive remove-inserts (also
insert-removes?)
- // and if they have the same character data, or are modified
content nodes,
+ // Now go over any consecutive remove-inserts (also
insert-removes?) and
+ // if they have the same character data, or are modified
content nodes,
// make them changes instead
for ( i = 0, ilen = cleanDiff.length - 1; i < ilen; i++ ) {
aItem = cleanDiff[ i ];
@@ -345,5 +370,24 @@
return cleanDiff;
}
- return getCleanDiff( diffs );
+ // Remove the close elements
+ oldData = removeCloseElements( oldData );
+ newData = removeCloseElements( newData );
+
+ // Get the diff
+ cleanDiff = getCleanDiff( this.diff_main( oldData, newData, options ) );
+
+ // Re-insert the close elements
+ for ( i = 0, ilen = cleanDiff.length; i < ilen; i++ ) {
+ for ( j = 0; j < cleanDiff[ i ][ 1 ].length; j++ ) {
+ if ( cleanDiff[ i ][ 1 ][ j ].type ) {
+ cleanDiff[ i ][ 1 ].splice( j + 1, 0, {
+ type: '/' + cleanDiff[ i ][ 1 ][ j
].type
+ } );
+ j++;
+ }
+ }
+ }
+
+ return cleanDiff;
};
diff --git a/tests/ui/ve.ui.DiffElement.test.js
b/tests/ui/ve.ui.DiffElement.test.js
index d3e66dc..665e136 100644
---
[MediaWiki-commits] [Gerrit] VisualEditor/VisualEditor[master]: VisualDiff: Don't diff close elements
Tchanders has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/374340 )
Change subject: VisualDiff: Don't diff close elements
..
VisualDiff: Don't diff close elements
Force the content diff to be balanced by removing
close elements prior to diffing, then re-inserting
afterwards.
Bug: T171862
Change-Id: I84aa157897ed5774b170e0dc4a3f7d264663c24d
---
M src/dm/ve.dm.VisualDiff.js
M src/ve.DiffMatchPatch.js
2 files changed, 42 insertions(+), 7 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/VisualEditor/VisualEditor
refs/changes/40/374340/1
diff --git a/src/dm/ve.dm.VisualDiff.js b/src/dm/ve.dm.VisualDiff.js
index 524cd89..022ee44 100644
--- a/src/dm/ve.dm.VisualDiff.js
+++ b/src/dm/ve.dm.VisualDiff.js
@@ -562,8 +562,7 @@
// Diff internal list items
diff = this.computeDiff(
oldDocInternalListItems.toDiff,
- newDocInternalListItems.toDiff,
- this.internalListDiff
+ newDocInternalListItems.toDiff
);
// Check there actually are any changes
diff --git a/src/ve.DiffMatchPatch.js b/src/ve.DiffMatchPatch.js
index 3d20936..21f12b1 100644
--- a/src/ve.DiffMatchPatch.js
+++ b/src/ve.DiffMatchPatch.js
@@ -70,14 +70,31 @@
return [];
};
-ve.DiffMatchPatch.prototype.getCleanDiff = function () {
- var diffs = this.diff_main.apply( this, arguments ),
+ve.DiffMatchPatch.prototype.getCleanDiff = function ( oldData, newData,
options ) {
+ var cleanDiff, i, ilen, j,
store = this.store,
DIFF_DELETE = this.constructor.static.DIFF_DELETE,
DIFF_INSERT = this.constructor.static.DIFF_INSERT,
DIFF_EQUAL = this.constructor.static.DIFF_EQUAL,
DIFF_CHANGE_DELETE = this.constructor.static.DIFF_CHANGE_DELETE,
DIFF_CHANGE_INSERT = this.constructor.static.DIFF_CHANGE_INSERT;
+
+ /**
+* Remove the close elements from the linear data, to force a balanced
diff
+*
+* @param {Array} data Linear data
+* @return {Array} Linear data without close elements
+*/
+ function removeCloseElements( data ) {
+ var i;
+ for ( i = 0; i < data.length; i++ ) {
+ if ( data[ i ].type && data[ i ].type[ 0 ] === '/' ) {
+ data.splice( i, 1 );
+ i--;
+ }
+ }
+ return data;
+ }
/**
* Get the index of the the first or last wordbreak in a data array
@@ -285,8 +302,8 @@
cleanDiff.push( [ DIFF_INSERT, insert ] );
}
- // Finally, go over any consecutive remove-inserts (also
insert-removes?)
- // and if they have the same character data, or are modified
content nodes,
+ // Now go over any consecutive remove-inserts (also
insert-removes?) and
+ // if they have the same character data, or are modified
content nodes,
// make them changes instead
for ( i = 0, ilen = cleanDiff.length - 1; i < ilen; i++ ) {
aItem = cleanDiff[ i ];
@@ -345,5 +362,24 @@
return cleanDiff;
}
- return getCleanDiff( diffs );
+ // Remove the close elements
+ oldData = removeCloseElements( oldData );
+ newData = removeCloseElements( newData );
+
+ // Get the diff
+ cleanDiff = getCleanDiff( this.diff_main( oldData, newData, options ) );
+
+ // Re-insert the close elements
+ for ( i = 0, ilen = cleanDiff.length; i < ilen; i++ ) {
+ for ( j = 0; j < cleanDiff[ i ][ 1 ].length; j++ ) {
+ if ( cleanDiff[ i ][ 1 ][ j ].type ) {
+ cleanDiff[ i ][ 1 ].splice( j + 1, 0, {
+ type: '/' + cleanDiff[ i ][ 1 ][ j
].type
+ } );
+ j++;
+ }
+ }
+ }
+
+ return cleanDiff;
};
--
To view, visit https://gerrit.wikimedia.org/r/374340
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I84aa157897ed5774b170e0dc4a3f7d264663c24d
Gerrit-PatchSet: 1
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Tchanders
___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
