Divec has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/328583 )
Change subject: Refactor ve.dm.TransactionBuilder#newFromWrap ...................................................................... Refactor ve.dm.TransactionBuilder#newFromWrap Change-Id: I81c4548cfd596eceba30fc84bc45f81adbd240c0 --- M src/dm/ve.dm.TransactionBuilder.js 1 file changed, 79 insertions(+), 63 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/VisualEditor/VisualEditor refs/changes/83/328583/1 diff --git a/src/dm/ve.dm.TransactionBuilder.js b/src/dm/ve.dm.TransactionBuilder.js index 6c54999..6f63056 100644 --- a/src/dm/ve.dm.TransactionBuilder.js +++ b/src/dm/ve.dm.TransactionBuilder.js @@ -614,7 +614,7 @@ * @return {ve.dm.Transaction} */ ve.dm.TransactionBuilder.static.newFromWrap = function ( doc, range, unwrapOuter, wrapOuter, unwrapEach, wrapEach ) { - var i, j, unwrapOuterData, startOffset, unwrapEachData, closingWrapEach, + var i, startOffset, closingUnwrapEach, closingWrapEach, endOffset, ptr, txBuilder = new ve.dm.TransactionBuilder(), depth = 0; @@ -628,83 +628,99 @@ } return closings; } - /* closingUnwrapEach = */ closingArray( unwrapEach ); + closingUnwrapEach = closingArray( unwrapEach ); closingWrapEach = closingArray( wrapEach ); // TODO: check for and fix nesting validity like fixupInsertion does - if ( range.start > unwrapOuter.length ) { - // Retain up to the first thing we're unwrapping - // The outer unwrapping takes place *outside* - // the range, so compensate for that - txBuilder.pushRetain( range.start - unwrapOuter.length ); - } else if ( range.start < unwrapOuter.length ) { - throw new Error( 'unwrapOuter is longer than the data preceding the range' ); - } - // Replace the opening elements for the outer unwrap&wrap - if ( wrapOuter.length > 0 || unwrapOuter.length > 0 ) { - // Verify that wrapOuter matches the data at this position - unwrapOuterData = doc.data.slice( range.start - unwrapOuter.length, range.start ); - for ( i = 0; i < unwrapOuterData.length; i++ ) { - if ( unwrapOuterData[ i ].type !== unwrapOuter[ i ].type ) { - throw new Error( 'Element in unwrapOuter does not match: expected ' + - unwrapOuter[ i ].type + ' but found ' + unwrapOuterData[ i ].type ); + /** + * Match items before/after an offset, skipping over MetaItems + * + * @param {string} direction Direction of match, backwards|forwards + * @param {number} offset First boundary for the matching items + * @param {Object[]} matchList List of objects with .type properties to compare + * @param {string} matchName Description of the match, for error messages + * @return {number} Other slice boundary for the matching items + * @throws {Error} Unmatched item foo in matchName [(found bar)] + */ + function match( direction, offset, matchList, matchName ) { + var start, stop, step, i, item; + if ( direction === 'forwards' ) { + start = 0; + stop = matchList.length; + step = 1; + // Inclusive 'from' slice boundary + offset--; + } else { + start = matchList.length - 1; + stop = -1; + step = -1; + } + for ( i = start; i !== stop; i += step ) { + offset += step; + item = doc.data.data[ offset ]; + if ( item.type !== matchList[ i ].type ) { + throw new Error( 'Unmatched item ' + matchList[ i ].type + ' in ' + + matchName + ' (found ' + item.type + ')' ); } } - // Instead of putting in unwrapOuter as given, put it in the - // way it appears in the model so we pick up any attributes - txBuilder.pushReplace( doc, range.start - unwrapOuter.length, unwrapOuter.length, ve.copy( wrapOuter ) ); + if ( direction === 'forwards' ) { + // Exclusive 'to' slice boundary + offset++; + } + return offset; } + // Verify the data before range.start matches unwrapOuter, and find where to retain up to + ptr = match( 'backwards', range.start, unwrapOuter, 'unwrapOuter' ); + txBuilder.pushRetain( ptr ); + // Replace wrapper + txBuilder.pushReplace( doc, ptr, range.start - ptr, ve.copy( wrapOuter ) ); - if ( wrapEach.length > 0 || unwrapEach.length > 0 ) { + if ( wrapEach.length === 0 && unwrapEach.length === 0 ) { + // There is no wrapEach/unwrapEach to be done, just retain + // up to the end of the range + txBuilder.pushRetain( range.end - range.start ); + } else { // Visit each top-level child and wrap/unwrap it // TODO figure out if we should use the tree/node functions here // rather than iterating over offsets, it may or may not be faster for ( i = range.start; i < range.end; i++ ) { - if ( doc.data.isElementData( i ) ) { - // This is a structural offset - if ( !doc.data.isCloseElementData( i ) ) { - // This is an opening element - if ( depth === 0 ) { - // We are at the start of a top-level element - // Replace the opening elements - - // Verify that unwrapEach matches the data at this position - unwrapEachData = doc.data.slice( i, i + unwrapEach.length ); - for ( j = 0; j < unwrapEachData.length; j++ ) { - if ( unwrapEachData[ j ].type !== unwrapEach[ j ].type ) { - throw new Error( 'Element in unwrapEach does not match: expected ' + - unwrapEach[ j ].type + ' but found ' + - unwrapEachData[ j ].type ); - } - } - // Instead of putting in unwrapEach as given, put it in the - // way it appears in the model, so we pick up any attributes - txBuilder.pushReplace( doc, i, unwrapEach.length, ve.copy( wrapEach ) ); - - // Store this offset for later - startOffset = i + unwrapEach.length; - } - depth++; - } else { - // This is a closing element - depth--; - if ( depth === 0 ) { - // We are at the end of a top-level element - // Advance past the element, then back up past the unwrapEach - j = ( i + 1 ) - unwrapEach.length; - // Retain the contents of what we're wrapping - txBuilder.pushRetain( j - startOffset ); - // Replace the closing elements - txBuilder.pushReplace( doc, j, unwrapEach.length, ve.copy( closingWrapEach ) ); - } + if ( !doc.data.isElementData( i ) ) { + continue; + } + if ( doc.data.isOpenElementData( i ) ) { + depth++; + if ( depth !== 1 ) { + continue; } + // This is the start of a top-level element + ptr = match( 'forwards', i, unwrapEach, 'unwrapEach' ); + // Replace wrapper + txBuilder.pushReplace( doc, i, ptr - i, ve.copy( wrapEach ) ); + startOffset = ptr; + // Goto end of wrapper + if ( ptr > i ) { + i = ptr - 1; + } + continue; + } + // Else this is a closing element + depth--; + if ( depth !== 0 ) { + continue; + } + // This is the end of a top-level element + ptr = match( 'forwards', i, closingUnwrapEach, 'closingUnwrapEach' ); + endOffset = ( ptr > i ) ? i : i + 1; + // Retain the contents that we're wrapping + txBuilder.pushRetain( endOffset - startOffset ); + // Replace the closing elements + txBuilder.pushReplace( doc, endOffset, ptr - i, ve.copy( closingWrapEach ) ); + // Goto end of wrapper + if ( ptr > i ) { + i = ptr - 1; } } - } else { - // There is no wrapEach/unwrapEach to be done, just retain - // up to the end of the range - txBuilder.pushRetain( range.end - range.start ); } // this is a no-op if unwrapOuter.length===0 and wrapOuter.length===0 -- To view, visit https://gerrit.wikimedia.org/r/328583 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I81c4548cfd596eceba30fc84bc45f81adbd240c0 Gerrit-PatchSet: 1 Gerrit-Project: VisualEditor/VisualEditor Gerrit-Branch: master Gerrit-Owner: Divec <da...@troi.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits