Esanders has uploaded a new change for review.
https://gerrit.wikimedia.org/r/86663
Change subject: WIP Add fixUpInsertion to newFromDocumentReplace
......................................................................
WIP Add fixUpInsertion to newFromDocumentReplace
Change-Id: I16d575b61b9796e7e889f2c27cfe02b4a40b7639
---
M modules/ve-mw/test/dm/ve.dm.Transaction.test.js
M modules/ve/dm/ve.dm.Transaction.js
M modules/ve/test/dm/ve.dm.Transaction.test.js
3 files changed, 241 insertions(+), 131 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor
refs/changes/63/86663/1
diff --git a/modules/ve-mw/test/dm/ve.dm.Transaction.test.js
b/modules/ve-mw/test/dm/ve.dm.Transaction.test.js
index c38b7c1..d61becd 100644
--- a/modules/ve-mw/test/dm/ve.dm.Transaction.test.js
+++ b/modules/ve-mw/test/dm/ve.dm.Transaction.test.js
@@ -9,7 +9,8 @@
// FIXME duplicates test runner; should be using a data provider
QUnit.test( 'newFromDocumentReplace with references', function ( assert ) {
- var i, j, doc2, tx, actualStoreItems, expectedStoreItems,
+ var i, j, doc2, txs, actualStoreItems, expectedStoreItems,
+ ops = [],
doc = ve.dm.example.createExampleDocument( 'internalData' ),
complexDoc = ve.dm.mwExample.createExampleDocument(
'complexInternalData' ),
comment = { 'type': 'alienMeta', 'attributes': { 'domElements':
$( '<!-- hello -->' ).get() } },
@@ -48,24 +49,41 @@
) );
},
'expectedOps': [
- {
- 'type': 'replace',
- 'remove': complexDoc.getData(
new ve.Range( 0, 7 ) ),
- 'insert': complexDoc.getData(
new ve.Range( 0, 7 ) ),
- 'removeMetadata':
complexDoc.getMetadata( new ve.Range( 0, 7 ) ),
- 'insertMetadata':
complexDoc.getMetadata( new ve.Range( 0, 5 ) )
- .concat( [ [ comment ]
] )
- .concat(
complexDoc.getMetadata( new ve.Range( 6, 8 ) ) )
- },
- { 'type': 'retain', 'length': 1 },
- {
- 'type': 'replace',
- 'remove': complexDoc.getData(
new ve.Range( 8, 32 ) ),
- 'insert': complexDoc.getData(
new ve.Range( 8, 32 ) ),
- 'removeMetadata':
complexDoc.getMetadata( new ve.Range( 8, 32 ) ),
- 'insertMetadata':
complexDoc.getMetadata( new ve.Range( 8, 32 ) )
- },
- { 'type': 'retain', 'length': 1 }
+ [
+ {
+ 'type': 'replace',
+ 'remove':
complexDoc.getData( new ve.Range( 0, 7 ) ),
+ 'insert': [],
+ 'removeMetadata':
complexDoc.getMetadata( new ve.Range( 0, 7 ) ),
+ 'insertMetadata': [],
+ },
+ {
+ 'type':
'replaceMetadata',
+ 'remove': [],
+ 'insert':
complexDoc.getMetadata( new ve.Range( 7, 8 ) )[0]
+ },
+ { 'type': 'retain', 'length':
26 },
+ ],
+ [
+ {
+ 'type': 'replace',
+ 'remove': [],
+ 'insert':
complexDoc.getData( new ve.Range( 0, 7 ) ),
+ 'removeMetadata': [],
+ 'insertMetadata':
complexDoc.getMetadata( new ve.Range( 0, 5 ) )
+ .concat( [ [
comment ] ] )
+ .concat(
complexDoc.getMetadata( new ve.Range( 6, 8 ) ) )
+ },
+ { 'type': 'retain', 'length': 1
},
+ {
+ 'type': 'replace',
+ 'remove':
complexDoc.getData( new ve.Range( 1, 25 ) ),
+ 'insert':
complexDoc.getData( new ve.Range( 8, 32 ) ),
+ 'removeMetadata':
complexDoc.getMetadata( new ve.Range( 1, 25 ) ),
+ 'insertMetadata':
complexDoc.getMetadata( new ve.Range( 8, 32 ) )
+ },
+ { 'type': 'retain', 'length': 8
}
+ ]
]
},
{
@@ -78,17 +96,19 @@
) );
},
'expectedOps': [
- { 'type': 'retain', 'length': 8 },
- {
- 'type': 'replace',
- 'remove': complexDoc.getData(
new ve.Range( 8, 32 ) ),
- 'insert': complexDoc.getData(
new ve.Range( 8, 32 ) ),
- 'removeMetadata':
complexDoc.getMetadata( new ve.Range( 8, 32 ) ),
- 'insertMetadata':
complexDoc.getMetadata( new ve.Range( 8, 30 ) )
- .concat( [ [] ] )
- .concat(
complexDoc.getMetadata( new ve.Range( 31, 32 ) ) )
- },
- { 'type': 'retain', 'length': 1 }
+ [
+ { 'type': 'retain', 'length': 8
},
+ {
+ 'type': 'replace',
+ 'remove':
complexDoc.getData( new ve.Range( 8, 32 ) ),
+ 'insert':
complexDoc.getData( new ve.Range( 8, 32 ) ),
+ 'removeMetadata':
complexDoc.getMetadata( new ve.Range( 8, 32 ) ),
+ 'insertMetadata':
complexDoc.getMetadata( new ve.Range( 8, 30 ) )
+ .concat( [ [] ]
)
+ .concat(
complexDoc.getMetadata( new ve.Range( 31, 32 ) ) )
+ },
+ { 'type': 'retain', 'length': 1
}
+ ]
]
},
{
@@ -97,49 +117,57 @@
'range': new ve.Range( 7, 7 ),
'newDocData': withReference,
'expectedOps': [
- { 'type': 'retain', 'length': 7 },
- {
- 'type': 'replace',
- 'remove': [],
- 'insert': withReference.slice(
0, 4 )
- // Renumber listIndex
from 0 to 2
- .concat( [
ve.extendObject( true, {}, withReference[4],
- { 'attributes':
{ 'listIndex': 2 } } ) ] )
- .concat(
withReference.slice( 5, 7 ) )
- },
- { 'type': 'retain', 'length': 1 },
- {
- 'type': 'replace',
- 'remove': complexDoc.getData(
new ve.Range( 8, 32 ) ),
- 'insert': complexDoc.getData(
new ve.Range( 8, 32 ) )
- .concat(
withReference.slice( 8, 15 ) ),
- 'removeMetadata':
complexDoc.getMetadata( new ve.Range( 8, 32 ) ),
- 'insertMetadata':
complexDoc.getMetadata( new ve.Range( 8, 32 ) )
- .concat( new Array( 7 )
)
- },
- { 'type': 'retain', 'length': 1 }
+ [
+ { 'type': 'retain', 'length': 7
},
+ {
+ 'type': 'replace',
+ 'remove': [],
+ 'insert':
withReference.slice( 0, 4 )
+ // Renumber
listIndex from 0 to 2
+ .concat( [
ve.extendObject( true, {}, withReference[4],
+ {
'attributes': { 'listIndex': 2 } } ) ] )
+ .concat(
withReference.slice( 5, 7 ) )
+ },
+ { 'type': 'retain', 'length': 1
},
+ {
+ 'type': 'replace',
+ 'remove':
complexDoc.getData( new ve.Range( 8, 32 ) ),
+ 'insert':
complexDoc.getData( new ve.Range( 8, 32 ) )
+ .concat(
withReference.slice( 8, 15 ) ),
+ 'removeMetadata':
complexDoc.getMetadata( new ve.Range( 8, 32 ) ),
+ 'insertMetadata':
complexDoc.getMetadata( new ve.Range( 8, 32 ) )
+ .concat( new
Array( 7 ) )
+ },
+ { 'type': 'retain', 'length': 1
}
+ ]
]
}
];
- QUnit.expect( 2 * cases.length );
- for ( i = 0; i < cases.length; i++ ) {
- doc = cases[i].doc; // TODO deep copy?
- if ( cases[i].newDocData ) {
- doc2 = new ve.dm.Document( cases[i].newDocData
);
- } else {
- doc2 = doc.getDocumentSlice( cases[i].range );
- cases[i].modify( doc2 );
- }
- tx = ve.dm.Transaction.newFromDocumentReplace( doc,
cases[i].range, doc2 );
- assert.deepEqualWithDomElements( tx.getOperations(),
cases[i].expectedOps, cases[i].msg + ': transaction' );
- actualStoreItems = [];
- expectedStoreItems = cases[i].expectedStoreItems || [];
- for ( j = 0; j < expectedStoreItems.length; j++ ) {
- actualStoreItems[j] = doc.store.value(
doc.store.indexOfHash(
- ve.getHash( expectedStoreItems[j] )
- ) );
- }
- assert.deepEqual( actualStoreItems, expectedStoreItems,
cases[i].msg + ': store items' );
+ QUnit.expect( cases.length * 2 );
+
+ for ( i = 0; i < cases.length; i++ ) {
+ doc = cases[i].doc; // TODO deep copy?
+ if ( cases[i].newDocData ) {
+ doc2 = new ve.dm.Document( cases[i].newDocData );
+ } else {
+ doc2 = doc.getDocumentSlice( cases[i].range );
+ cases[i].modify( doc2 );
}
+ txs = ve.dm.Transaction.newFromDocumentReplace( doc,
cases[i].range, doc2 );
+ ops = [];
+ for ( j = 0; j < cases[i].expectedOps.length; j++ ) {
+ ops.push( txs[j].getOperations() );
+ }
+ assert.deepEqualWithDomElements( ops, cases[i].expectedOps,
cases[i].msg + ': transactions' );
+
+ actualStoreItems = [];
+ expectedStoreItems = cases[i].expectedStoreItems || [];
+ for ( j = 0; j < expectedStoreItems.length; j++ ) {
+ actualStoreItems[j] = doc.store.value(
doc.store.indexOfHash(
+ ve.getHash( expectedStoreItems[j] )
+ ) );
+ }
+ assert.deepEqual( actualStoreItems, expectedStoreItems,
cases[i].msg + ': store items' );
+ }
} );
\ No newline at end of file
diff --git a/modules/ve/dm/ve.dm.Transaction.js
b/modules/ve/dm/ve.dm.Transaction.js
index cab2b24..d80cb39 100644
--- a/modules/ve/dm/ve.dm.Transaction.js
+++ b/modules/ve/dm/ve.dm.Transaction.js
@@ -162,17 +162,17 @@
* @param {ve.dm.Document} doc Main document
* @param {ve.Range|ve.dm.Node} removeNodeOrRange Node or range to remove
* @param {ve.dm.Document} newDoc Document to insert
- * @returns {ve.dm.Transaction} Transaction that replaces the node and updates
the internal list
+ * @returns {ve.dm.Transaction[]} Transaction that replaces the node and
updates the internal list
* @throws {Error} removeNodeOrRange must be a ve.dm.Node or a ve.Range
*/
ve.dm.Transaction.newFromDocumentReplace = function ( doc, removeNodeOrRange,
newDoc ) {
- var i, len, range, merge, data, metadata, listData, listMetadata,
oldEndOffset, newEndOffset,
+ var i, len, range, merge, data, metadata, listData, listMetadata,
oldEndOffset, newEndOffset, tx, insertion,
listNode = doc.internalList.getListNode(),
listNodeRange = listNode.getRange(),
newListNode = newDoc.internalList.getListNode(),
newListNodeRange = newListNode.getRange(),
newListNodeOuterRange = newListNode.getOuterRange(),
- tx = new ve.dm.Transaction();
+ txs = [];
if ( removeNodeOrRange instanceof ve.dm.Node ) {
range = removeNodeOrRange.getRange();
@@ -238,6 +238,28 @@
if ( range.end <= listNodeRange.start ) {
// range is entirely before listNodeRange
// First replace the node, then the internal list
+ if ( !range.isCollapsed() ) {
+ tx = new ve.dm.Transaction();
+ tx.pushRetain( range.start );
+ tx.pushReplace( doc, range.start, range.end -
range.start, [] );
+ tx.pushFinalRetain( doc, range.end );
+ txs.push( tx );
+ // Adjust the listNodeRange now that range has been
removed
+ listNodeRange = tx.translateRange( listNodeRange );
+ }
+
+ tx = new ve.dm.Transaction();
+
+ // Fix up the node insertion
+ insertion = doc.fixupInsertion( data.data, range.start );
+ tx.pushRetain( insertion.offset );
+ tx.pushReplace( doc, insertion.offset, insertion.remove,
insertion.data, metadata.data );
+ tx.pushRetain( listNodeRange.start - ( insertion.offset +
insertion.remove ) );
+ tx.pushReplace( doc, listNodeRange.start, listNodeRange.end -
listNodeRange.start,
+ listData, listMetadata
+ );
+ tx.pushFinalRetain( doc, listNodeRange.end );
+/*
tx.pushRetain( range.start );
tx.pushReplace( doc, range.start, range.end - range.start,
data.data, metadata.data );
tx.pushRetain( listNodeRange.start - range.end );
@@ -245,9 +267,31 @@
listData, listMetadata
);
tx.pushRetain( doc.data.getLength() - listNodeRange.end );
+*/
+ txs.push( tx );
} else if ( listNodeRange.end <= range.start ) {
// range is entirely after listNodeRange
// First replace the internal list, then the node
+ if ( !range.isCollapsed() ) {
+ tx = new ve.dm.Transaction();
+ tx.pushRetain( range.start );
+ tx.pushReplace( doc, range.start, range.end -
range.start, [] );
+ tx.pushFinalRetain( doc, range.end );
+ txs.push( tx );
+ }
+ tx = new ve.dm.Transaction();
+ tx.pushRetain( listNodeRange.start );
+ tx.pushReplace( doc, listNodeRange.start, listNodeRange.end -
listNodeRange.start,
+ listData, listMetadata
+ );
+
+ // Fix up the node insertion
+ insertion = doc.fixupInsertion( data.data, range.start );
+ tx.pushRetain( insertion.offset - listNodeRange.end );
+ tx.pushReplace( doc, insertion.offset, insertion.remove,
insertion.data, metadata.data );
+ tx.pushFinalRetain( doc, insertion.offset + insertion.remove );
+
+/*
tx.pushRetain( listNodeRange.start );
tx.pushReplace( doc, listNodeRange.start, listNodeRange.end -
listNodeRange.start,
listData, listMetadata
@@ -255,6 +299,8 @@
tx.pushRetain( range.start - listNodeRange.end );
tx.pushReplace( doc, range.start, range.end - range.start,
data.data, metadata.data );
tx.pushRetain( doc.data.getLength() - range.end );
+*/
+ txs.push( tx );
} else if ( range.start >= listNodeRange.start && range.end <=
listNodeRange.end ) {
// range is entirely within listNodeRange
// Merge data into listData, then only replace the internal list
@@ -262,13 +308,15 @@
range.end - range.start, data.data );
ve.batchSplice( listMetadata, range.start - listNodeRange.start,
range.end - range.start + 1, metadata.data );
+ tx = new ve.dm.Transaction();
tx.pushRetain( listNodeRange.start );
tx.pushReplace( doc, listNodeRange.start, listNodeRange.end -
listNodeRange.start,
listData, listMetadata
);
- tx.pushRetain( doc.data.getLength() - listNodeRange.end );
+ tx.pushFinalRetain( doc, listNodeRange.end );
+ txs.push( tx );
}
- return tx;
+ return txs;
};
/**
diff --git a/modules/ve/test/dm/ve.dm.Transaction.test.js
b/modules/ve/test/dm/ve.dm.Transaction.test.js
index 463b646..6c2623d 100644
--- a/modules/ve/test/dm/ve.dm.Transaction.test.js
+++ b/modules/ve/test/dm/ve.dm.Transaction.test.js
@@ -669,7 +669,8 @@
} );
QUnit.test( 'newFromDocumentReplace', function ( assert ) {
- var i, j, doc2, tx, actualStoreItems, expectedStoreItems,
+ var i, j, doc2, txs, actualStoreItems, expectedStoreItems,
+ ops = [],
doc = ve.dm.example.createExampleDocument( 'internalData' ),
nextIndex = doc.store.valueStore.length,
whee = [ { 'type': 'paragraph' }, 'W', 'h', 'e', 'e', { 'type':
'/paragraph' } ],
@@ -686,15 +687,17 @@
) );
},
'expectedOps': [
- { 'type': 'retain', 'length': 6 },
- {
- 'type': 'replace',
- 'remove': doc.getData( new
ve.Range( 6, 20 ) ),
- 'insert': doc.getData( new
ve.Range( 6, 10 ) )
- .concat( [ 'z', 'a',
'a' ] )
- .concat( doc.getData(
new ve.Range( 10, 20 ) ) )
- },
- { 'type': 'retain', 'length': 7 }
+ [
+ { 'type': 'retain', 'length': 6
},
+ {
+ 'type': 'replace',
+ 'remove': doc.getData(
new ve.Range( 6, 20 ) ),
+ 'insert': doc.getData(
new ve.Range( 6, 10 ) )
+ .concat( [ 'z',
'a', 'a' ] )
+ .concat(
doc.getData( new ve.Range( 10, 20 ) ) )
+ },
+ { 'type': 'retain', 'length': 7
}
+ ]
]
},
{
@@ -708,16 +711,18 @@
) );
},
'expectedOps': [
- { 'type': 'retain', 'length': 6 },
- {
- 'type': 'replace',
- 'remove': doc.getData( new
ve.Range( 6, 20 ) ),
- 'insert': doc.getData( new
ve.Range( 6, 15 ) )
- .concat( [ [
doc.data.getData( 15 ), [ nextIndex ] ] ] )
- .concat( [ [
doc.data.getData( 16 ), [ nextIndex ] ] ] )
- .concat( doc.getData(
new ve.Range( 17, 20 ) ) )
- },
- { 'type': 'retain', 'length': 7 }
+ [
+ { 'type': 'retain', 'length': 6
},
+ {
+ 'type': 'replace',
+ 'remove': doc.getData(
new ve.Range( 6, 20 ) ),
+ 'insert': doc.getData(
new ve.Range( 6, 15 ) )
+ .concat( [ [
doc.data.getData( 15 ), [ nextIndex ] ] ] )
+ .concat( [ [
doc.data.getData( 16 ), [ nextIndex ] ] ] )
+ .concat(
doc.getData( new ve.Range( 17, 20 ) ) )
+ },
+ { 'type': 'retain', 'length': 7
}
+ ]
],
'expectedStoreItems': [
ve.dm.example.bold
@@ -732,18 +737,29 @@
newDoc.commit( insertion.transaction );
},
'expectedOps': [
- { 'type': 'retain', 'length': 6 },
- {
- 'type': 'replace',
- 'remove': doc.getData( new
ve.Range( 6, 20 ) ),
- 'insert': doc.getData( new
ve.Range( 6, 20 ) ).concat( wheeItem )
- },
- { 'type': 'retain', 'length': 1 },
- {
- 'type': 'replace',
- 'remove': doc.getData( new
ve.Range( 21, 27 ) ),
- 'insert': doc.getData( new
ve.Range( 21, 27 ) )
- }
+ [
+ { 'type': 'retain', 'length':
21 },
+ {
+ 'type': 'replace',
+ 'remove': doc.getData(
new ve.Range( 21, 27 ) ),
+ 'insert': []
+ }
+ ],
+ [
+ { 'type': 'retain', 'length': 6
},
+ {
+ 'type': 'replace',
+ 'remove': doc.getData(
new ve.Range( 6, 20 ) ),
+ 'insert': doc.getData(
new ve.Range( 6, 20 ) ).concat( wheeItem )
+ },
+ { 'type': 'retain', 'length': 1
},
+ {
+ 'type': 'replace',
+ 'remove': [],
+ 'insert': doc.getData(
new ve.Range( 21, 27 ) )
+ },
+ { 'type': 'retain', 'length': 6
}
+ ]
]
},
{
@@ -756,20 +772,31 @@
) );
},
'expectedOps': [
- { 'type': 'retain', 'length': 6 },
- {
- 'type': 'replace',
- 'remove': doc.getData( new
ve.Range( 6, 20 ) ),
- 'insert': doc.getData( new
ve.Range( 6, 11 ) )
- .concat( [ '!', '!',
'!' ] )
- .concat( doc.getData(
new ve.Range( 11, 20 ) ) )
- },
- { 'type': 'retain', 'length': 1 },
- {
- 'type': 'replace',
- 'remove': doc.getData( new
ve.Range( 21, 27 ) ),
- 'insert': doc.getData( new
ve.Range( 21, 27 ) )
- }
+ [
+ { 'type': 'retain', 'length':
21 },
+ {
+ 'type': 'replace',
+ 'remove': doc.getData(
new ve.Range( 21, 27 ) ),
+ 'insert': []
+ }
+ ],
+ [
+ { 'type': 'retain', 'length': 6
},
+ {
+ 'type': 'replace',
+ 'remove': doc.getData(
new ve.Range( 6, 20 ) ),
+ 'insert': doc.getData(
new ve.Range( 6, 11 ) )
+ .concat( [ '!',
'!', '!' ] )
+ .concat(
doc.getData( new ve.Range( 11, 20 ) ) )
+ },
+ { 'type': 'retain', 'length': 1
},
+ {
+ 'type': 'replace',
+ 'remove': [],
+ 'insert': doc.getData(
new ve.Range( 21, 27 ) )
+ },
+ { 'type': 'retain', 'length': 6
}
+ ]
]
},
{
@@ -781,17 +808,20 @@
newDoc.commit( insertion.transaction );
},
'expectedOps': [
- { 'type': 'retain', 'length': 6 },
- {
- 'type': 'replace',
- 'remove': doc.getData( new
ve.Range( 6, 20 ) ),
- 'insert': doc.getData( new
ve.Range( 6, 20 ) ).concat( wheeItem )
- },
- { 'type': 'retain', 'length': 7 }
+ [
+ { 'type': 'retain', 'length': 6
},
+ {
+ 'type': 'replace',
+ 'remove': doc.getData(
new ve.Range( 6, 20 ) ),
+ 'insert': doc.getData(
new ve.Range( 6, 20 ) ).concat( wheeItem )
+ },
+ { 'type': 'retain', 'length': 7
}
+ ]
]
}
];
- QUnit.expect( 2 * cases.length );
+
+ QUnit.expect( cases.length * 2);
for ( i = 0; i < cases.length; i++ ) {
doc = ve.dm.example.createExampleDocument( cases[i].doc );
if ( cases[i].newDocData ) {
@@ -800,8 +830,12 @@
doc2 = doc.getDocumentSlice( cases[i].range );
cases[i].modify( doc2 );
}
- tx = ve.dm.Transaction.newFromDocumentReplace( doc,
cases[i].range, doc2 );
- assert.deepEqualWithDomElements( tx.getOperations(),
cases[i].expectedOps, cases[i].msg + ': transaction' );
+ txs = ve.dm.Transaction.newFromDocumentReplace( doc,
cases[i].range, doc2 );
+ ops = [];
+ for ( j = 0; j < cases[i].expectedOps.length; j++ ) {
+ ops.push( txs[j].getOperations() );
+ }
+ assert.deepEqualWithDomElements( ops, cases[i].expectedOps,
cases[i].msg + ': transactions' );
actualStoreItems = [];
expectedStoreItems = cases[i].expectedStoreItems || [];
--
To view, visit https://gerrit.wikimedia.org/r/86663
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I16d575b61b9796e7e889f2c27cfe02b4a40b7639
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Esanders <esand...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
