Merge changes I899d2b3c,I0272fe38,I7ad24e7f

* changes:
  Rewrite MetaList.onTransact
  Allow replace operations to replace metadata as well
  Fix processing of double metadata replacements
This commit is contained in:
jenkins-bot 2013-09-17 21:01:41 +00:00 committed by Gerrit Code Review
commit 973d9f999d
6 changed files with 204 additions and 131 deletions

View file

@ -128,39 +128,6 @@ ve.dm.MetaItem.prototype.setIndex = function ( index ) {
this.index = index;
};
/**
* Queue up a change to the item's offset and index.
* @param {number} offset New offset
* @param {number} index New index
*/
ve.dm.MetaItem.prototype.setMove = function ( offset, index ) {
this.move = {
'offset': offset,
'index': index
};
};
/**
* Whether or not a move is pending.
* @returns {boolean} A move is pending
*/
ve.dm.MetaItem.prototype.isMovePending = function () {
return this.move !== null;
};
/**
* Apply the pending move and clear it.
* @throws No move pending
*/
ve.dm.MetaItem.prototype.applyMove = function () {
if ( this.move === null ) {
throw new Error( 'No move pending' );
}
this.setOffset( this.move.offset );
this.setIndex( this.move.index );
this.move = null;
};
/**
* Attach this item to a MetaList.
* @param {ve.dm.MetaList} list Parent list to attach to
@ -187,3 +154,11 @@ ve.dm.MetaItem.prototype.detach = function ( list ) {
this.index = null;
}
};
/**
* Check whether this item is attached to a MetaList.
* @returns {boolean} Whether item is attached
*/
ve.dm.MetaItem.prototype.isAttached = function () {
return this.list !== null;
};

View file

@ -80,122 +80,183 @@ ve.mixinClass( ve.dm.MetaList, ve.EventEmitter );
* @emits remove
*/
ve.dm.MetaList.prototype.onTransact = function ( tx, reversed ) {
var i, j, k, ilen, jlen, klen, ins, rm, itemIndex, item,
offset = 0, newOffset = 0, index = 0, ops = tx.getOperations(),
insertedItems = [], removedItems = [];
// Look for replaceMetadata operations in the transaction and insert/remove items as appropriate
// This requires we also inspect retain, replace and replaceMetadata operations in order to
// track the offset and index. We track the pre-transaction offset, we need to do that in
// order to remove items correctly. This also means inserted items are initially at the wrong
// offset, but we translate it later.
var i, ilen, j, jlen, k, klen, item, ins, rm, insMeta, rmMeta,
numItems = this.items.length,
itemIndex = 0, // Current index into this.items
offset = 0, // Current pre-transaction offset
newOffset = 0, // Current post-transaction offset
index = 0, // Current pre-transaction index
newIndex = 0, // Current post-transaction index
// Array of items that should appear in this.items after we're done. This includes newly
// inserted items as well as existing items that aren't being removed.
// [ { item: ve.dm.MetaItem, offset: offset to move to, index: index to move to } ]
newItems = [],
removedItems = [], // Array of items that should be removed from this.items
events = [], // Array of events that we should emit when we're done
ops = tx.getOperations();
// Go through the transaction operations and plan out where to add, remove and move items. We
// don't actually touch this.items yet, otherwise we 1) get it out of order which breaks
// findItem() and 2) lose information about what the pre-transaction state of this.items was.
for ( i = 0, ilen = ops.length; i < ilen; i++ ) {
switch ( ops[i].type ) {
case 'retain':
// Advance itemIndex through the retain and update items we encounter along the way
for ( ;
itemIndex < numItems && this.items[itemIndex].offset < offset + ops[i].length;
itemIndex++
) {
// Plan to move this item to the post-transaction offset and index
newItems.push( {
'item': this.items[itemIndex],
'offset': this.items[itemIndex].offset + newOffset - offset,
'index': this.items[itemIndex].offset === offset ?
// Adjust index for insertions or removals that happened at this offset
newIndex - index + this.items[itemIndex].index :
// Offset is retained over completely, don't adjust index
this.items[itemIndex].index } );
}
offset += ops[i].length;
newOffset += ops[i].length;
index = 0;
newIndex = 0;
break;
case 'retainMetadata':
// Advance itemIndex through the retain and update items we encounter along the way
for ( ;
itemIndex < numItems && this.items[itemIndex].offset === offset &&
this.items[itemIndex].index < index + ops[i].length;
itemIndex++
) {
newItems.push( {
'item': this.items[itemIndex],
'offset': newOffset,
'index': this.items[itemIndex].index + newIndex - index
} );
}
index += ops[i].length;
newIndex += ops[i].length;
break;
case 'replace':
// if we have metadata replace info we can calculate the new
// offset and index directly
ins = reversed ? ops[i].removeMetadata : ops[i].insertMetadata;
rm = reversed ? ops[i].insertMetadata : ops[i].removeMetadata;
if ( rm === undefined ) {
// no impact on metadata.
/* jshint noempty: false */
} else if ( ins.length === 0 ) {
for ( j = 0, jlen = rm.length; j < jlen; j++ ) {
if ( rm[j] !== undefined ) {
for ( k = 0, klen = rm[j].length; k < klen; k++ ) {
item = this.deleteRemovedItem( offset + j, k );
removedItems.push( { 'item': item, 'offset': offset + j, 'index': k } );
}
}
ins = reversed ? ops[i].remove : ops[i].insert;
rm = reversed ? ops[i].insert : ops[i].remove;
if ( ops[i].removeMetadata !== undefined ) {
insMeta = reversed ? ops[i].removeMetadata : ops[i].insertMetadata;
rmMeta = reversed ? ops[i].insertMetadata : ops[i].removeMetadata;
// Process removed metadata
for ( ;
itemIndex < numItems &&
this.items[itemIndex].offset < offset + rmMeta.length;
itemIndex++
) {
removedItems.push( this.items[itemIndex] );
}
} else if ( rm.length === 0 ) {
itemIndex = -Math.pow(2, 53); // INT_MIN
for ( j = 0, jlen = ins.length; j < jlen; j++ ) {
if ( ins[j] !== undefined ) {
for ( k = 0, klen = ins[j].length; k < klen; k++ ) {
item = ve.dm.metaItemFactory.createFromElement( ins[j][k] );
// offset is pre-transaction, but we'll fix it up w/ setMove
this.addInsertedItem( offset, itemIndex++, item );
item.setMove( newOffset + j, k );
insertedItems.push( { 'item': item } );
// Process inserted metadata
for ( j = 0, jlen = insMeta.length; j < jlen; j++ ) {
if ( insMeta[j] ) {
for ( k = 0, klen = insMeta[j].length; k < klen; k++ ) {
item = ve.dm.metaItemFactory.createFromElement( insMeta[j][k] );
newItems.push( {
'item': item,
'offset': newOffset + j,
'index': k
} );
}
}
}
} else {
// find the first itemIndex - the rest should be in order after it
for ( j = 0, jlen = rm.length; j < jlen; j++ ) {
if ( rm[j] !== undefined ) {
itemIndex = this.findItem( offset + j, 0 );
break;
}
}
// iterate through all the inserted metaItems
for ( j = 0, jlen = ins.length; j < jlen; j++ ) {
item = ins[j];
if ( item !== undefined ) {
for ( k = 0, klen = item.length; k < klen; k++ ) {
// Queue up the move for later so we don't break the metaItem ordering
this.items[itemIndex].setMove( newOffset + j, k );
itemIndex++;
}
}
// No metadata handling specified, which means we just have to deal with offset
// adjustments, same as a retain
for ( ;
itemIndex < numItems &&
this.items[itemIndex].offset < offset + rm.length;
itemIndex++
) {
newItems.push( {
'item': this.items[itemIndex],
'offset': this.items[itemIndex].offset + newOffset - offset,
'index': this.items[itemIndex].index
} );
}
}
offset += reversed ? ops[i].insert.length : ops[i].remove.length;
newOffset += reversed ? ops[i].remove.length : ops[i].insert.length;
index = 0;
break;
case 'retainMetadata':
index += ops[i].length;
offset += rm.length;
newOffset += ins.length;
break;
case 'replaceMetadata':
ins = reversed ? ops[i].remove : ops[i].insert;
rm = reversed ? ops[i].insert : ops[i].remove;
for ( j = 0, jlen = rm.length; j < jlen; j++ ) {
item = this.deleteRemovedItem( offset, index + j );
removedItems.push( { 'item': item, 'offset': offset, 'index': index } );
insMeta = reversed ? ops[i].remove : ops[i].insert;
rmMeta = reversed ? ops[i].insert : ops[i].remove;
// Process removed items
for ( ;
itemIndex < numItems && this.items[itemIndex].offset === offset &&
this.items[itemIndex].index < index + rmMeta.length;
itemIndex++
) {
removedItems.push( this.items[itemIndex] );
}
for ( j = 0, jlen = ins.length; j < jlen; j++ ) {
item = ve.dm.metaItemFactory.createFromElement( ins[j] );
// offset and index are pre-transaction, but we'll fix them later
this.addInsertedItem( offset, index + j, item );
insertedItems.push( { 'item': item } );
// Process inserted items
for ( j = 0, jlen = insMeta.length; j < jlen; j++ ) {
item = ve.dm.metaItemFactory.createFromElement( insMeta[j] );
newItems.push( { 'item': item, 'offset': newOffset, 'index': newIndex + j } );
}
index += rm.length;
index += rmMeta.length;
newIndex += insMeta.length;
break;
}
}
// Translate the offsets of all items, and reindex them too
// Reindexing is simple because the above ensures the items are already in the right order
offset = -1;
index = 0;
for ( i = 0, ilen = this.items.length; i < ilen; i++ ) {
if ( this.items[i].isMovePending() ) {
// move was calculated from metadata replace info, just apply it
this.items[i].applyMove();
} else {
// otherwise calculate the new offset from the transaction
this.items[i].setOffset( tx.translateOffset( this.items[i].getOffset(), reversed ) );
if ( this.items[i].getOffset() === offset ) {
index++;
} else {
index = 0;
}
this.items[i].setIndex( index );
}
offset = this.items[i].getOffset();
// Update the remaining items that the transaction didn't touch or retain over
for ( ; itemIndex < numItems; itemIndex++ ) {
newItems.push( {
'item': this.items[itemIndex],
'offset': this.items[itemIndex].offset + newOffset - offset,
'index': this.items[itemIndex].offset === offset ?
newIndex - index + this.items[itemIndex].index :
this.items[itemIndex].index
} );
}
for ( i = 0, ilen = insertedItems.length; i < ilen; i++ ) {
this.emit( 'insert', insertedItems[i].item );
}
// Process the changes, and queue up events. We emit the events at the end when the MetaList
// is back in a consistent state
// Remove removed items
for ( i = 0, ilen = removedItems.length; i < ilen; i++ ) {
this.emit( 'remove', removedItems[i].item, removedItems[i].offset, removedItems[i].index );
this.deleteRemovedItem( removedItems[i].offset, removedItems[i].index );
events.push( [
'remove', removedItems[i].item, removedItems[i].offset, removedItems[i].index
] );
}
// Move moved items (these appear as inserted items that are already attached)
for ( i = 0, ilen = newItems.length; i < ilen; i++ ) {
if ( newItems[i].item.isAttached() ) {
if ( newItems[i].offset !== newItems[i].item.offset || newItems[i].index !== newItems[i].item.index ) {
this.deleteRemovedItem( newItems[i].item.offset, newItems[i].item.index );
this.addInsertedItem( newItems[i].offset, newItems[i].index, newItems[i].item );
}
}
}
// Insert new items
for ( i = 0, ilen = newItems.length; i < ilen; i++ ) {
if ( !newItems[i].item.isAttached() ) {
this.addInsertedItem( newItems[i].offset, newItems[i].index, newItems[i].item );
events.push( [ 'insert', newItems[i].item ] );
}
}
// Emit events
for ( i = 0, ilen = events.length; i < ilen; i++ ) {
this.emit.apply( this, events[i] );
}
};

View file

@ -908,22 +908,24 @@ ve.dm.Transaction.prototype.addSafeRemoveOps = function ( doc, removeStart, remo
* @param {number} offset Offset to start at
* @param {number} removeLength Number of data items to remove
* @param {Array} insert Data to insert
* @param {Array} [insertMetadata] Overwrite the metadata with this data, rather than collapsing it
*/
ve.dm.Transaction.prototype.pushReplace = function ( doc, offset, removeLength, insert ) {
ve.dm.Transaction.prototype.pushReplace = function ( doc, offset, removeLength, insert, insertMetadata ) {
if ( removeLength === 0 && insert.length === 0 ) {
// Don't push no-ops
return;
}
var op, end = this.operations.length - 1,
var op, extraMetadata, end = this.operations.length - 1,
lastOp = end >= 0 ? this.operations[end] : null,
penultOp = end >= 1 ? this.operations[ end - 1 ] : null,
range = new ve.Range( offset, offset + removeLength ),
remove = doc.getData( range ),
removeMetadata = doc.getMetadata( range ),
insertMetadata, extraMetadata;
isRemoveEmpty = ve.compare( removeMetadata, new Array( removeMetadata.length ) ),
isInsertEmpty = insertMetadata && ve.compare( insertMetadata, new Array( insertMetadata.length ) );
if ( !ve.compare( removeMetadata, new Array( removeMetadata.length ) ) ) {
if ( !insertMetadata && !isRemoveEmpty ) {
// if we are removing a range which includes metadata, we need to
// collapse it. If there's nothing to insert, we also need to add
// an extra `replaceMetadata` operation later in order to insert the
@ -936,6 +938,9 @@ ve.dm.Transaction.prototype.pushReplace = function ( doc, offset, removeLength,
// pad out at end so insert metadata is the same length as insert data
ve.batchSplice( insertMetadata, 1, 0, new Array( insert.length - 1 ) );
}
} else if ( isInsertEmpty && isRemoveEmpty ) {
// No metadata changes, don't pollute the transaction with [undefined, undefined, ...]
insertMetadata = undefined;
}
// simple replaces can be combined

View file

@ -497,4 +497,5 @@ ve.dm.TransactionProcessor.processors.replaceMetadata = function ( op ) {
insert = this.reversed ? op.remove : op.insert;
this.document.spliceMetadata( this.cursor, this.metadataCursor, remove.length, insert );
this.metadataCursor += insert.length;
};

View file

@ -55,6 +55,22 @@ QUnit.test( 'onTransact', function ( assert ) {
],
'msg': 'Transaction inserting, replacing and removing text'
},
{
'calls': [
[ 'pushRetain', 1 ],
[
'pushReplace', doc, 1, 9,
[ 'f', 'O', 'O', 'b', 'A', 'R', 'b', 'A', 'Z' ],
[
undefined,
[ ve.dm.example.withMetaMetaData[9][0], ve.dm.example.withMetaMetaData[7][0] ],
undefined, undefined, undefined, [ ve.dm.example.withMetaMetaData[4][0] ],
undefined, undefined, undefined
]
]
],
'msg': 'Transaction replacing text and metadata at the same time'
},
{
// delta: 0
'calls': [
@ -115,7 +131,7 @@ QUnit.test( 'onTransact', function ( assert ) {
];
// HACK: This works because most transactions above don't change the document length, and the
// ones that do change it cancel out
QUnit.expect( cases.length*( 4*doc.metadata.getTotalDataLength() + 2 ) );
QUnit.expect( cases.length*( 8*doc.metadata.getTotalDataLength() + 2 ) );
for ( i = 0; i < cases.length; i++ ) {
tx = new ve.dm.Transaction();
@ -127,9 +143,9 @@ QUnit.test( 'onTransact', function ( assert ) {
list = new ve.dm.MetaList( surface );
// Test both the transaction-via-surface and transaction-via-document flows
surface.change( tx );
assertItemsMatchMetadata( assert, doc.metadata, list, cases[i].msg, false );
assertItemsMatchMetadata( assert, doc.metadata, list, cases[i].msg, true );
doc.rollback( tx );
assertItemsMatchMetadata( assert, doc.metadata, list, cases[i].msg + ' (rollback)', false );
assertItemsMatchMetadata( assert, doc.metadata, list, cases[i].msg + ' (rollback)', true );
}
} );

View file

@ -305,6 +305,21 @@ QUnit.test( 'commit/rollback', function ( assert ) {
data.splice( 27, 2, metaElementInsert, metaElementInsertClose );
}
},
'replacing metadata twice at the same offset': {
'data': ve.dm.example.withMeta,
'calls': [
[ 'pushRetain', 11 ],
[ 'pushRetainMetadata', 1 ],
[ 'pushReplaceMetadata', [ ve.dm.example.withMetaMetaData[11][1] ], [ metaElementInsert ] ],
[ 'pushRetainMetadata', 1 ],
[ 'pushReplaceMetadata', [ ve.dm.example.withMetaMetaData[11][3] ], [ metaElementInsert ] ],
[ 'pushRetain', 1 ]
],
'expected': function ( data ) {
data.splice( 23, 2, metaElementInsert, metaElementInsertClose );
data.splice( 27, 2, metaElementInsert, metaElementInsertClose );
}
},
'removing data from between metadata merges metadata': {
'data': ve.dm.example.withMeta,
'calls': [