Browse Source

Changed Position.clone and LivePosition.clone to Position.createFromPosition and LivePosition.createFromPosition. Changed some constructors to copy passed parameters. Docs LivePosition update.

Szymon Cofalik 10 năm trước cách đây
mục cha
commit
0e021a6834

+ 33 - 15
packages/ckeditor5-engine/src/treemodel/liveposition.js

@@ -45,21 +45,6 @@ CKEDITOR.define( [
 			bindWithDocument.call( this );
 		}
 
-		/**
-		 * Creates and returns a new position which is equal to this LivePosition. Returned object may be
-		 * an instance of Position or LivePosition, depending on passed argument.
-		 *
-		 * @param {Boolean} [makeLivePosition] Flag representing whether new object should be "live". Defaults to false.
-		 * @returns {treeModel.Position|treeModel.LivePosition}
-		 */
-		clone( makeLivePosition ) {
-			if ( makeLivePosition ) {
-				return new LivePosition( this.root, this.path.slice() );
-			} else {
-				return super.clone();
-			}
-		}
-
 		/**
 		 * Unbinds all events previously bound by LivePosition. Use it whenever you don't need LivePosition instance
 		 * anymore (i.e. when leaving scope in which it was declared or before re-assigning variable that was
@@ -68,6 +53,39 @@ CKEDITOR.define( [
 		detach() {
 			this.stopListening();
 		}
+
+		/**
+		 * @static
+		 * @method createAfter
+		 * @see {@link treeModel.Position#createAfter}
+		 * @param {treeModel.Node} node
+		 * @returns {treeModel.LivePosition}
+		 */
+
+		/**
+		 * @static
+		 * @method createBefore
+		 * @see {@link treeModel.Position#createBefore}
+		 * @param {treeModel.Node} node
+		 * @returns {treeModel.LivePosition}
+		 */
+
+		/**
+		 * @static
+		 * @method createFromParentAndOffset
+		 * @see {@link treeModel.Position#createFromParentAndOffset}
+		 * @param {treeModel.Element} parent
+		 * @param {Number} offset
+		 * @returns {treeModel.LivePosition}
+		 */
+
+		/**
+		 * @static
+		 * @method createFromPosition
+		 * @see {@link treeModel.Position#createFromPosition}
+		 * @param {treeModel.Position} position
+		 * @returns {treeModel.LivePosition}
+		 */
 	}
 
 	/**

+ 3 - 2
packages/ckeditor5-engine/src/treemodel/operation/insertoperation.js

@@ -8,9 +8,10 @@
 CKEDITOR.define( [
 	'treemodel/operation/operation',
 	'treemodel/nodelist',
+	'treemodel/position',
 	'treemodel/range',
 	'treemodel/operation/removeoperation'
-], ( Operation, NodeList, Range ) => {
+], ( Operation, NodeList, Position, Range ) => {
 	/**
 	 * Operation to insert list of nodes on the given position in the tree data model.
 	 *
@@ -35,7 +36,7 @@ CKEDITOR.define( [
 			 * @readonly
 			 * @type {treeModel.Position}
 			 */
-			this.position = position;
+			this.position = Position.createFromPosition( position );
 
 			/**
 			 * List of nodes to insert.

+ 6 - 5
packages/ckeditor5-engine/src/treemodel/operation/moveoperation.js

@@ -7,10 +7,11 @@
 
 CKEDITOR.define( [
 	'treemodel/operation/operation',
+	'treemodel/position',
 	'treemodel/range',
 	'ckeditorerror',
 	'utils'
-], ( Operation, Range, CKEditorError, utils ) => {
+], ( Operation, Position, Range, CKEditorError, utils ) => {
 	/**
 	 * Operation to move list of subsequent nodes from one position in the document to another.
 	 *
@@ -34,7 +35,7 @@ CKEDITOR.define( [
 			 *
 			 * @type {treeModel.Position}
 			 */
-			this.sourcePosition = sourcePosition;
+			this.sourcePosition = Position.createFromPosition( sourcePosition );
 
 			/**
 			 * How many nodes to move.
@@ -48,7 +49,7 @@ CKEDITOR.define( [
 			 *
 			 * @type {treeModel.Position}
 			 */
-			this.targetPosition = targetPosition;
+			this.targetPosition = Position.createFromPosition( targetPosition );
 		}
 
 		get type() {
@@ -56,11 +57,11 @@ CKEDITOR.define( [
 		}
 
 		clone() {
-			return new this.constructor( this.sourcePosition.clone(), this.howMany, this.targetPosition.clone(), this.baseVersion );
+			return new this.constructor( this.sourcePosition, this.howMany, this.targetPosition, this.baseVersion );
 		}
 
 		getReversed() {
-			return new this.constructor( this.targetPosition.clone(), this.howMany, this.sourcePosition.clone(), this.baseVersion + 1 );
+			return new this.constructor( this.targetPosition, this.howMany, this.sourcePosition, this.baseVersion + 1 );
 		}
 
 		_execute() {

+ 3 - 2
packages/ckeditor5-engine/src/treemodel/operation/transform.js

@@ -48,9 +48,10 @@ CKEDITOR.define( [
 	'treemodel/operation/attributeoperation',
 	'treemodel/operation/moveoperation',
 	'treemodel/operation/nooperation',
+	'treemodel/position',
 	'treemodel/range',
 	'utils'
-], ( InsertOperation, AttributeOperation, MoveOperation, NoOperation, Range, utils ) => {
+], ( InsertOperation, AttributeOperation, MoveOperation, NoOperation, Position, Range, utils ) => {
 	const ot = {
 		InsertOperation: {
 			// Transforms InsertOperation `a` by InsertOperation `b`. Accepts a flag stating whether `a` is more important
@@ -208,7 +209,7 @@ CKEDITOR.define( [
 					return new MoveOperation(
 						range.start,
 						range.end.offset - range.start.offset,
-						newTargetPosition.clone(),
+						Position.createFromPosition( newTargetPosition ),
 						a.baseVersion
 					);
 				} );

+ 13 - 13
packages/ckeditor5-engine/src/treemodel/position.js

@@ -128,16 +128,6 @@ CKEDITOR.define( [ 'treemodel/rootelement', 'utils', 'ckeditorerror' ], ( RootEl
 			return parent;
 		}
 
-		/**
-		 * Creates and returns a new instance of {@link treeModel.Position}
-		 * which is equal to this {@link treeModel.Position position}.
-		 *
-		 * @returns {treeModel.Position} Cloned {@link treeModel.Position position}.
-		 */
-		clone() {
-			return new Position( this.root, this.path.slice() );
-		}
-
 		/**
 		 * Checks whether this position is before or after given position.
 		 *
@@ -192,7 +182,7 @@ CKEDITOR.define( [ 'treemodel/rootelement', 'utils', 'ckeditorerror' ], ( RootEl
 		 * @returns {treeModel.Position|null} Transformed position or `null`.
 		 */
 		getTransformedByDeletion( deletePosition, howMany ) {
-			let transformed = this.clone();
+			let transformed = Position.createFromPosition( this );
 
 			// This position can't be affected if deletion was in a different root.
 			if ( this.root != deletePosition.root ) {
@@ -241,7 +231,7 @@ CKEDITOR.define( [ 'treemodel/rootelement', 'utils', 'ckeditorerror' ], ( RootEl
 		 * @returns {treeModel.Position} Transformed position.
 		 */
 		getTransformedByInsertion( insertPosition, howMany, insertBefore ) {
-			let transformed = this.clone();
+			let transformed = Position.createFromPosition( this );
 
 			// This position can't be affected if insertion was in a different root.
 			if ( this.root != insertPosition.root ) {
@@ -461,6 +451,16 @@ CKEDITOR.define( [ 'treemodel/rootelement', 'utils', 'ckeditorerror' ], ( RootEl
 			return new this( parent.root, path );
 		}
 
+		/**
+		 * Creates and returns a new instance of Position, which is equal to passed position.
+		 *
+		 * @param {treeModel.Position} position Position to be cloned.
+		 * @returns {treeModel.Position}
+		 */
+		static createFromPosition( position ) {
+			return new this( position.root, position.path.slice() );
+		}
+
 		/**
 		 * Returns a new position that is a combination of this position and given positions. The combined
 		 * position is this position transformed by moving a range starting at `from` to `to` position.
@@ -495,7 +495,7 @@ CKEDITOR.define( [ 'treemodel/rootelement', 'utils', 'ckeditorerror' ], ( RootEl
 			const i = source.path.length - 1;
 
 			// The first part of a path to combined position is a path to the place where nodes were moved.
-			let combined = target.clone();
+			let combined = Position.createFromPosition( target );
 
 			// Then we have to update the rest of the path.
 

+ 3 - 3
packages/ckeditor5-engine/src/treemodel/range.js

@@ -25,14 +25,14 @@ CKEDITOR.define( [ 'treemodel/position', 'treemodel/positioniterator', 'utils' ]
 			 *
 			 * @property {treeModel.Position}
 			 */
-			this.start = start.clone();
+			this.start = Position.createFromPosition( start );
 
 			/**
 			 * End position.
 			 *
 			 * @property {treeModel.Position}
 			 */
-			this.end = end.clone();
+			this.end = Position.createFromPosition( end );
 		}
 
 		/**
@@ -275,7 +275,7 @@ CKEDITOR.define( [ 'treemodel/position', 'treemodel/positioniterator', 'utils' ]
 		 * @returns {treeModel.Range}
 		 */
 		static createFromPositionAndShift( position, shift ) {
-			let endPosition = position.clone();
+			let endPosition = Position.createFromPosition( position );
 			endPosition.offset += shift;
 
 			return new this( position, endPosition );

+ 6 - 19
packages/ckeditor5-engine/tests/treemodel/liveposition.js

@@ -46,25 +46,6 @@ describe( 'LivePosition', () => {
 		expect( live ).to.be.instanceof( Position );
 	} );
 
-	it( 'should return instance of Position when cloned', () => {
-		let live = new LivePosition( root, [ 0 ] );
-		let clone = live.clone();
-
-		expect( clone ).to.be.instanceof( Position );
-
-		live.detach();
-	} );
-
-	it( 'should return instance of LivePosition when cloned with flag set to true', () => {
-		let live = new LivePosition( root, [ 0 ] );
-		let clone = live.clone( true );
-
-		expect( clone ).to.be.instanceof( LivePosition );
-
-		live.detach();
-		clone.detach();
-	} );
-
 	it( 'should listen to a change event of the document that owns this position root', () => {
 		sinon.spy( LivePosition.prototype, 'listenTo' );
 
@@ -87,6 +68,12 @@ describe( 'LivePosition', () => {
 		LivePosition.prototype.stopListening.restore();
 	} );
 
+	it( 'createFromPosition should return LivePosition', () => {
+		let position = LivePosition.createFromPosition( new Position( root, [ 0 ] ) );
+		expect( position ).to.be.instanceof( LivePosition );
+		position.detach();
+	} );
+
 	it( 'createFromParentAndOffset should return LivePosition', () => {
 		let position = LivePosition.createFromParentAndOffset( ul, 0 );
 		expect( position ).to.be.instanceof( LivePosition );

+ 1 - 1
packages/ckeditor5-engine/tests/treemodel/operation/insertoperation.js

@@ -131,7 +131,7 @@ describe( 'InsertOperation', () => {
 
 		expect( reverse ).to.be.an.instanceof( RemoveOperation );
 		expect( reverse.baseVersion ).to.equal( 1 );
-		expect( reverse.sourcePosition ).to.equal( position );
+		expect( reverse.sourcePosition.isEqual( position ) ).to.be.true;
 		expect( reverse.howMany ).to.equal( 7 );
 	} );
 

+ 2 - 2
packages/ckeditor5-engine/tests/treemodel/operation/removeoperation.js

@@ -98,8 +98,8 @@ describe( 'RemoveOperation', () => {
 		expect( reverse ).to.be.an.instanceof( ReinsertOperation );
 		expect( reverse.baseVersion ).to.equal( 1 );
 		expect( reverse.howMany ).to.equal( 2 );
-		expect( reverse.sourcePosition ).to.equal( operation.targetPosition );
-		expect( reverse.targetPosition ).to.equal( position );
+		expect( reverse.sourcePosition.isEqual( operation.targetPosition ) ).to.be.true;
+		expect( reverse.targetPosition.isEqual( position ) ).to.be.true;
 	} );
 
 	it( 'should undo remove set of nodes by applying reverse operation', () => {

+ 6 - 6
packages/ckeditor5-engine/tests/treemodel/operation/transform.js

@@ -86,7 +86,7 @@ describe( 'transform', () => {
 
 			expected = {
 				type: InsertOperation,
-				position: position,
+				position: Position.createFromPosition( position ),
 				baseVersion: baseVersion + 1
 			};
 		} );
@@ -1313,15 +1313,15 @@ describe( 'transform', () => {
 			targetPosition = new Position( root, [ 3, 3, 3 ] );
 			howMany = 2;
 
-			rangeEnd = sourcePosition.clone();
+			rangeEnd = Position.createFromPosition( sourcePosition );
 			rangeEnd.offset += howMany;
 
 			op = new MoveOperation( sourcePosition, howMany, targetPosition, baseVersion );
 
 			expected = {
 				type: MoveOperation,
-				sourcePosition: sourcePosition.clone(),
-				targetPosition: targetPosition.clone(),
+				sourcePosition: Position.createFromPosition( sourcePosition ),
+				targetPosition: Position.createFromPosition( targetPosition ),
 				howMany: howMany,
 				baseVersion: baseVersion + 1
 			};
@@ -1894,7 +1894,7 @@ describe( 'transform', () => {
 
 			it( 'range is same as transforming range and is important: convert to NoOperation', () => {
 				let transformBy = new MoveOperation(
-					op.sourcePosition.clone(),
+					op.sourcePosition,
 					op.howMany,
 					new Position( root, [ 4, 1, 0 ] ),
 					baseVersion
@@ -1911,7 +1911,7 @@ describe( 'transform', () => {
 
 			it( 'range is same as transforming range and is less important: update range path', () => {
 				let transformBy = new MoveOperation(
-					op.sourcePosition.clone(),
+					op.sourcePosition,
 					op.howMany,
 					new Position( root, [ 4, 1, 0 ] ),
 					baseVersion

+ 9 - 9
packages/ckeditor5-engine/tests/treemodel/position.js

@@ -150,6 +150,15 @@ describe( 'position', () => {
 		expect( Position.createAfter( r ) ).to.have.property( 'path' ).that.deep.equals( [ 1, 1, 3 ] );
 	} );
 
+	it( 'should create a copy of given position', () => {
+		let original = new Position( root, [ 1, 2, 3 ] );
+		let position = Position.createFromPosition( original );
+
+		expect( position ).to.be.instanceof( Position );
+		expect( position.isEqual( original ) ).to.be.true;
+		expect( position ).not.to.be.equal( original );
+	} );
+
 	it( 'should throw error if one try to make positions after root', () => {
 		expect( () => {
 			Position.createAfter( root );
@@ -238,15 +247,6 @@ describe( 'position', () => {
 		expect( position.getParentPath() ).to.deep.equal( [ 1, 2 ] );
 	} );
 
-	it( 'should return a new, equal position when cloned', () => {
-		const position = new Position( root, [ 1, 2, 3 ] );
-		const clone = position.clone();
-
-		expect( clone ).not.to.be.equal( position ); // clone is not pointing to the same object as position
-		expect( clone.isEqual( position ) ).to.be.true; // but they are equal in the position-sense
-		expect( clone.path ).not.to.be.equal( position.path ); // make sure the paths are not the same array
-	} );
-
 	describe( 'isBefore', () => {
 		it( 'should return true if given position has same root and is before this position', () => {
 			let position = new Position( root, [ 1, 1, 2 ] );