8
0
Просмотр исходного кода

Merge pull request #1147 from ckeditor/t/1146

Internal: Make setting "undo mode" in transformations more explicit. Closes #1146.
Piotrek Koszuliński 8 лет назад
Родитель
Сommit
dcf3c286f0

+ 11 - 11
packages/ckeditor5-engine/src/model/delta/basic-transformations.js

@@ -56,7 +56,7 @@ addTransformationCase( AttributeDelta, SplitDelta, ( a, b, context ) => {
 		return defaultTransform( a, b, context );
 	}
 
-	const undoMode = context.aWasUndone || context.bWasUndone;
+	const undoMode = context.undoMode;
 	const splitPosition = new Position( b.position.root, b.position.path.slice( 0, -1 ) );
 
 	const deltas = defaultTransform( a, b, context );
@@ -102,7 +102,7 @@ addTransformationCase( InsertDelta, MergeDelta, ( a, b, context ) => {
 		return defaultTransform( a, b, context );
 	}
 
-	const undoMode = context.aWasUndone || context.bWasUndone;
+	const undoMode = context.undoMode;
 
 	// If insert is applied at the same position where merge happened, we reverse the merge (we treat it like it
 	// didn't happen) and then apply the original insert operation. This is "mirrored" in MergeDelta x InsertDelta
@@ -141,7 +141,7 @@ addTransformationCase( MarkerDelta, RenameDelta, transformMarkerDelta );
 
 // Add special case for MoveDelta x MergeDelta transformation.
 addTransformationCase( MoveDelta, MergeDelta, ( a, b, context ) => {
-	const undoMode = context.aWasUndone || context.bWasUndone;
+	const undoMode = context.undoMode;
 
 	// Do not apply special transformation case in undo mode or if `MergeDelta` has `NoOperation` as the second operation.
 	if ( undoMode || !b.position ) {
@@ -174,7 +174,7 @@ addTransformationCase( MergeDelta, InsertDelta, ( a, b, context ) => {
 		return defaultTransform( a, b, context );
 	}
 
-	const undoMode = context.aWasUndone || context.bWasUndone;
+	const undoMode = context.undoMode;
 
 	// If merge is applied at the same position where we inserted a range of nodes we cancel the merge as it's results
 	// may be unexpected and very weird. Even if we do some "magic" we don't know what really are users' expectations.
@@ -187,7 +187,7 @@ addTransformationCase( MergeDelta, InsertDelta, ( a, b, context ) => {
 
 // Add special case for MergeDelta x MoveDelta transformation.
 addTransformationCase( MergeDelta, MoveDelta, ( a, b, context ) => {
-	const undoMode = context.aWasUndone || context.bWasUndone;
+	const undoMode = context.undoMode;
 
 	// Do not apply special transformation case in undo mode or if `MergeDelta` has `NoOperation` as the second operation.
 	if ( undoMode || !a.position ) {
@@ -210,7 +210,7 @@ addTransformationCase( MergeDelta, MoveDelta, ( a, b, context ) => {
 } );
 
 addTransformationCase( SplitDelta, SplitDelta, ( a, b, context ) => {
-	const undoMode = context.aWasUndone || context.bWasUndone;
+	const undoMode = context.undoMode;
 
 	// Do not apply special transformation case if transformation is in undo mode.
 	if ( undoMode ) {
@@ -356,7 +356,7 @@ addTransformationCase( SplitDelta, AttributeDelta, ( a, b, context ) => {
 
 	a = a.clone();
 
-	const undoMode = context.aWasUndone || context.bWasUndone;
+	const undoMode = context.undoMode;
 	const splitPosition = new Position( a.position.root, a.position.path.slice( 0, -1 ) );
 
 	// Special case applies only if undo is not a context and only if `SplitDelta` has `InsertOperation` (not `ReinsertOperation`).
@@ -450,7 +450,7 @@ addTransformationCase( WrapDelta, SplitDelta, ( a, b, context ) => {
 
 // Add special case for RenameDelta x SplitDelta transformation.
 addTransformationCase( RenameDelta, SplitDelta, ( a, b, context ) => {
-	const undoMode = context.aWasUndone || context.bWasUndone;
+	const undoMode = context.undoMode;
 	const deltas = defaultTransform( a, b, context );
 
 	// Special case applies only if undo is not a context and only if `SplitDelta` has `InsertOperation` (not `ReinsertOperation`).
@@ -476,7 +476,7 @@ addTransformationCase( RenameDelta, SplitDelta, ( a, b, context ) => {
 addTransformationCase( SplitDelta, RenameDelta, ( a, b, context ) => {
 	a = a.clone();
 
-	const undoMode = context.aWasUndone || context.bWasUndone;
+	const undoMode = context.undoMode;
 
 	// Special case applies only if undo is not a context and only if `SplitDelta` has `InsertOperation` (not `ReinsertOperation`).
 	if ( undoMode || !( a._cloneOperation instanceof InsertOperation ) ) {
@@ -508,7 +508,7 @@ addTransformationCase( RemoveDelta, SplitDelta, ( a, b, context ) => {
 		return defaultTransform( a, b, context );
 	}
 
-	const undoMode = context.aWasUndone || context.bWasUndone;
+	const undoMode = context.undoMode;
 
 	// Special case applies only if undo is not a context.
 	if ( undoMode ) {
@@ -533,7 +533,7 @@ addTransformationCase( RemoveDelta, SplitDelta, ( a, b, context ) => {
 
 // Add special case for SplitDelta x RemoveDelta transformation.
 addTransformationCase( SplitDelta, RemoveDelta, ( a, b, context ) => {
-	const undoMode = context.aWasUndone || context.bWasUndone;
+	const undoMode = context.undoMode;
 
 	// Special case applies only if undo is not a context.
 	if ( undoMode ) {

+ 10 - 21
packages/ckeditor5-engine/src/model/delta/transform.js

@@ -217,7 +217,7 @@ const transform = {
 	 * deltas are considered more important (than `deltasB`) when resolving conflicts.
 	 * @param {Array.<module:engine/model/delta/delta~Delta>} deltasB Array with the second set of deltas to transform. These
 	 * deltas are considered less important (than `deltasA`) when resolving conflicts.
-	 * @param {module:engine/model/document~Document} [document=null] If set, deltas will be transformed in "context mode"
+	 * @param {module:engine/model/document~Document} [document=null] If set, deltas will be transformed in "undo mode"
 	 * and given `document` will be used to determine relations between deltas. If not set (default), deltas will be
 	 * transforming without additional context information.
 	 * @returns {Object}
@@ -240,6 +240,7 @@ const transform = {
 			contextAB.wasAffected = new Map();
 			contextAB.originalDelta = new Map();
 			contextAB.document = document;
+			contextAB.undoMode = true;
 
 			for ( const delta of transformedDeltasB ) {
 				contextAB.originalDelta.set( delta, delta );
@@ -263,8 +264,7 @@ const transform = {
 							forceNotSticky: contextAB.forceNotSticky,
 							isStrong: contextAB.isStrong,
 							forceWeakRemove: contextAB.forceWeakRemove,
-							aWasUndone: false,
-							bWasUndone: contextAB.bWasUndone
+							undoMode: contextAB.undoMode
 						} );
 
 						const resultBA = transform.transform( deltaB[ l ], deltaA[ k ], {
@@ -272,8 +272,7 @@ const transform = {
 							forceNotSticky: contextAB.forceNotSticky,
 							isStrong: !contextAB.isStrong,
 							forceWeakRemove: contextAB.forceWeakRemove,
-							aWasUndone: contextAB.bWasUndone,
-							bWasUndone: false
+							undoMode: contextAB.undoMode
 						} );
 
 						if ( useAdditionalContext ) {
@@ -354,21 +353,10 @@ function padWithNoOps( deltas, howMany ) {
 // Using data given in `context` object, sets `context.insertBefore` and `context.forceNotSticky` flags.
 // Also updates `context.wasAffected`.
 function _setContext( a, b, context ) {
-	_setBWasUndone( b, context );
 	_setWasAffected( a, b, context );
 	_setInsertBeforeContext( a, b, context );
 	_setForceWeakRemove( b, context );
-	_setForceNotSticky( context );
-}
-
-// Sets `context.bWasUndone` basing on `context.document` history for `b` delta.
-//
-// `context.bWasUndone` is set to `true` if the (originally transformed) `b` delta was undone or was undoing delta.
-function _setBWasUndone( b, context ) {
-	const originalDelta = context.originalDelta.get( b );
-	const history = context.document.history;
-
-	context.bWasUndone = history.isUndoneDelta( originalDelta ) || history.isUndoingDelta( originalDelta );
+	_setForceNotSticky( b, context );
 }
 
 // Sets `context.insertBefore` basing on `context.document` history for `a` by `b` transformation.
@@ -432,10 +420,11 @@ function _setInsertBeforeContext( a, b, context ) {
 // if delta `b` was already undone or if delta `b` is an undoing delta.
 //
 // This affects `MoveOperation` (and its derivatives).
-function _setForceNotSticky( context ) {
-	if ( context.bWasUndone ) {
-		context.forceNotSticky = true;
-	}
+function _setForceNotSticky( b, context ) {
+	const originalDelta = context.originalDelta.get( b );
+	const history = context.document.history;
+
+	context.forceNotSticky = history.isUndoneDelta( originalDelta ) || history.isUndoingDelta( originalDelta );
 }
 
 // Sets `context.forceWeakRemove` basing on `context.document` history for transformation by `b` delta.

+ 1 - 1
packages/ckeditor5-engine/tests/model/delta/transform/attributedelta.js

@@ -285,7 +285,7 @@ describe( 'transform', () => {
 				attributeDelta.addOperation( new AttributeOperation( range, 'key', 'oldValue', 'newValue', baseVersion ) );
 
 				// The split delta was undone.
-				context.bWasUndone = true;
+				context.undoMode = true;
 
 				const transformed = transform( attributeDelta, splitDelta, context );
 

+ 1 - 1
packages/ckeditor5-engine/tests/model/delta/transform/insertdelta.js

@@ -137,7 +137,7 @@ describe( 'transform', () => {
 				// In undo mode, default transformation algorithm should be used.
 				const mergeDelta = getMergeDelta( insertPosition, 4, 12, baseVersion );
 
-				context.bWasUndone = true;
+				context.undoMode = true;
 				const transformed = transform( insertDelta, mergeDelta, context );
 
 				baseVersion = mergeDelta.operations.length;

+ 1 - 1
packages/ckeditor5-engine/tests/model/delta/transform/mergedelta.js

@@ -92,7 +92,7 @@ describe( 'transform', () => {
 				// In undo mode, default transformation algorithm should be used.
 				const insertDelta = getInsertDelta( mergePosition, [ nodeA, nodeB ], baseVersion );
 
-				context.bWasUndone = true;
+				context.undoMode = true;
 				const transformed = transform( mergeDelta, insertDelta, context );
 
 				baseVersion = insertDelta.operations.length;

+ 1 - 1
packages/ckeditor5-engine/tests/model/delta/transform/removedelta.js

@@ -193,7 +193,7 @@ describe( 'transform', () => {
 				const nodeCopy = new Element( 'x' );
 				const splitDelta = getSplitDelta( splitPosition, nodeCopy, 2, baseVersion );
 
-				context.bWasUndone = true;
+				context.undoMode = true;
 
 				const transformed = transform( removeDelta, splitDelta, context );
 

+ 1 - 1
packages/ckeditor5-engine/tests/model/delta/transform/renamedelta.js

@@ -90,7 +90,7 @@ describe( 'transform', () => {
 				const splitPosition = new Position( root, [ 3, 3, 3 ] );
 				const splitDelta = getSplitDelta( splitPosition, new Element( 'p' ), 9, baseVersion );
 
-				context.bWasUndone = true;
+				context.undoMode = true;
 
 				const transformed = transform( renameDelta, splitDelta, context );
 

+ 3 - 3
packages/ckeditor5-engine/tests/model/delta/transform/splitdelta.js

@@ -135,7 +135,7 @@ describe( 'transform', () => {
 				const transformed = transform( splitDelta, splitDeltaB, {
 					isStrong: false,
 					insertBefore: true,
-					bWasUndone: true // If `insertBefore` was set it means that delta `b` had to be undone.
+					undoMode: true // If `insertBefore` was set it means that delta `b` had to be undone.
 				} );
 
 				baseVersion = splitDeltaB.operations.length;
@@ -887,7 +887,7 @@ describe( 'transform', () => {
 					new Position( root, [ 3, 3, 3 ] ), 'p', 'li', baseVersion
 				) );
 
-				context.aWasUndone = true;
+				context.undoMode = true;
 				const transformed = transform( splitDelta, renameDelta, context );
 
 				baseVersion = renameDelta.operations.length;
@@ -1031,7 +1031,7 @@ describe( 'transform', () => {
 				const removePosition = new Position( root, [ 3, 3, 3 ] );
 				const removeDelta = getRemoveDelta( removePosition, 1, baseVersion );
 
-				context.bWasUndone = true;
+				context.undoMode = true;
 
 				const transformed = transform( splitDelta, removeDelta, context );
 

+ 17 - 0
packages/ckeditor5-engine/tests/model/delta/transform/transform.js

@@ -545,6 +545,23 @@ describe( 'transform', () => {
 					]
 				} );
 			} );
+
+			it( 'should set `context.undoMode` in undo mode (when document is passed)', () => {
+				const deltaA = new Delta();
+				deltaA.addOperation( new NoOperation( 0 ) );
+				const deltaB = new Delta();
+				deltaB.addOperation( new NoOperation( 0 ) );
+
+				sinon.spy( deltaTransform, 'transform' );
+
+				transformDeltaSets( [ deltaA ], [ deltaB ], doc );
+
+				const contextAB = deltaTransform.transform.args[ 0 ][ 2 ];
+				const contextBA = deltaTransform.transform.args[ 1 ][ 2 ];
+
+				expect( contextAB.undoMode ).to.be.true;
+				expect( contextBA.undoMode ).to.be.true;
+			} );
 		} );
 	} );
 } );