Sfoglia il codice sorgente

Add model/view Range.clone() method.

Maciej Gołaszewski 7 anni fa
parent
commit
2e35e803aa

+ 2 - 2
packages/ckeditor5-engine/src/model/differ.js

@@ -879,7 +879,7 @@ export default class Differ {
 				diffs.push( {
 					type: 'attribute',
 					position: range.start,
-					range: Range._createFromRange( range ),
+					range: range.clone(),
 					length: 1,
 					attributeKey: key,
 					attributeOldValue: oldValue,
@@ -898,7 +898,7 @@ export default class Differ {
 			diffs.push( {
 				type: 'attribute',
 				position: range.start,
-				range: Range._createFromRange( range ),
+				range: range.clone(),
 				length: 1,
 				attributeKey: key,
 				attributeOldValue: null,

+ 1 - 1
packages/ckeditor5-engine/src/model/documentselection.js

@@ -750,7 +750,7 @@ class LiveSelection extends Selection {
 			return;
 		}
 
-		const liveRange = LiveRange._createFromRange( range );
+		const liveRange = LiveRange.fromRange( range );
 
 		liveRange.on( 'change:range', ( evt, oldRange, data ) => {
 			this._hasChangedRange = true;

+ 22 - 3
packages/ckeditor5-engine/src/model/liverange.js

@@ -41,6 +41,25 @@ export default class LiveRange extends Range {
 	}
 
 	/**
+	 * Creates a {@link module:engine/model/range~Range range instance}, which is equal to this live range.
+	 *
+	 * @returns {module:engine/model/range~Range}
+	 */
+	toRange() {
+		return new Range( this.start, this.end );
+	}
+
+	/**
+	 * Creates a `LiveRange` instance that is equal to position.
+	 *
+	 * @param {module:engine/model/range~Range} range
+	 * @returns {module:engine/model/range~Range}
+	 */
+	static fromRange( range ) {
+		return new LiveRange( range.start, range.end );
+	}
+
+	/**
 	 * @see module:engine/model/range~Range._createIn
 	 * @static
 	 * @protected
@@ -72,7 +91,7 @@ export default class LiveRange extends Range {
 	 * @see module:engine/model/range~Range._createFromRange
 	 * @static
 	 * @protected
-	 * @method module:engine/model/liverange~LiveRange._createFromRange
+	 * @method module:engine/model/liverange~LiveRange.fromRange
 	 * @param {module:engine/model/range~Range} range
 	 * @returns {module:engine/model/liverange~LiveRange}
 	 */
@@ -150,7 +169,7 @@ function transform( operation ) {
 			}
 		}
 
-		const oldRange = Range._createFromRange( this );
+		const oldRange = this.toRange();
 
 		this.start = result.start;
 		this.end = result.end;
@@ -158,7 +177,7 @@ function transform( operation ) {
 		this.fire( 'change:range', oldRange, { deletionPosition } );
 	} else if ( contentChanged ) {
 		// If range boundaries have not changed, but there was change inside the range, fire `change:content` event.
-		this.fire( 'change:content', Range._createFromRange( this ), { deletionPosition } );
+		this.fire( 'change:content', this.toRange(), { deletionPosition } );
 	}
 }
 

+ 3 - 4
packages/ckeditor5-engine/src/model/markercollection.js

@@ -8,7 +8,6 @@
  */
 
 import LiveRange from './liverange';
-import Range from './range';
 import EmitterMixin from '@ckeditor/ckeditor5-utils/src/emittermixin';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 import mix from '@ckeditor/ckeditor5-utils/src/mix';
@@ -100,7 +99,7 @@ export default class MarkerCollection {
 			let hasChanged = false;
 
 			if ( !oldRange.isEqual( range ) ) {
-				oldMarker._attachLiveRange( LiveRange._createFromRange( range ) );
+				oldMarker._attachLiveRange( LiveRange.fromRange( range ) );
 				hasChanged = true;
 			}
 
@@ -121,7 +120,7 @@ export default class MarkerCollection {
 			return oldMarker;
 		}
 
-		const liveRange = LiveRange._createFromRange( range );
+		const liveRange = LiveRange.fromRange( range );
 		const marker = new Marker( markerName, liveRange, managedUsingOperations, affectsData );
 
 		this._markers.set( markerName, marker );
@@ -428,7 +427,7 @@ class Marker {
 			throw new CKEditorError( 'marker-destroyed: Cannot use a destroyed marker instance.' );
 		}
 
-		return Range._createFromRange( this._liveRange );
+		return this._liveRange.toRange();
 	}
 
 	/**

+ 1 - 1
packages/ckeditor5-engine/src/model/operation/attributeoperation.js

@@ -49,7 +49,7 @@ export default class AttributeOperation extends Operation {
 		 * @readonly
 		 * @member {module:engine/model/range~Range}
 		 */
-		this.range = Range._createFromRange( range );
+		this.range = range.clone();
 
 		/**
 		 * Key of an attribute to change or remove.

+ 2 - 2
packages/ckeditor5-engine/src/model/operation/markeroperation.js

@@ -41,7 +41,7 @@ export default class MarkerOperation extends Operation {
 		 * @readonly
 		 * @member {module:engine/model/range~Range}
 		 */
-		this.oldRange = oldRange ? Range._createFromRange( oldRange ) : null;
+		this.oldRange = oldRange ? oldRange.clone() : null;
 
 		/**
 		 * Marker range after the change.
@@ -49,7 +49,7 @@ export default class MarkerOperation extends Operation {
 		 * @readonly
 		 * @member {module:engine/model/range~Range}
 		 */
-		this.newRange = newRange ? Range._createFromRange( newRange ) : null;
+		this.newRange = newRange ? newRange.clone() : null;
 
 		/**
 		 * Specifies whether the marker operation affects the data produced by the data pipeline

+ 1 - 1
packages/ckeditor5-engine/src/model/operation/transform.js

@@ -1047,7 +1047,7 @@ setTransformation( MarkerOperation, InsertOperation, ( a, b ) => {
 setTransformation( MarkerOperation, MarkerOperation, ( a, b, context ) => {
 	if ( a.name == b.name ) {
 		if ( context.aIsStrong ) {
-			a.oldRange = b.newRange ? Range._createFromRange( b.newRange ) : null;
+			a.oldRange = b.newRange ? b.newRange.clone() : null;
 		} else {
 			return [ new NoOperation( 0 ) ];
 		}

+ 14 - 16
packages/ckeditor5-engine/src/model/range.js

@@ -207,7 +207,7 @@ export default class Range {
 			}
 		} else {
 			// Ranges do not intersect, return the original range.
-			ranges.push( Range._createFromRange( this ) );
+			ranges.push( new Range( this.start, this.end ) );
 		}
 
 		return ranges;
@@ -423,7 +423,7 @@ export default class Range {
 				return [ this._getTransformedByMergeOperation( operation ) ];
 		}
 
-		return [ Range._createFromRange( this ) ];
+		return [ new Range( this.start, this.end ) ];
 	}
 
 	/**
@@ -434,7 +434,7 @@ export default class Range {
 	 * @returns {Array.<module:engine/model/range~Range>} Range which is the result of transformation.
 	 */
 	getTransformedByOperations( operations ) {
-		const ranges = [ Range._createFromRange( this ) ];
+		const ranges = [ new Range( this.start, this.end ) ];
 
 		for ( const operation of operations ) {
 			for ( let i = 0; i < ranges.length; i++ ) {
@@ -487,6 +487,15 @@ export default class Range {
 	}
 
 	/**
+	 * Returns a new range that is equal to current range.
+	 *
+	 * @returns {module:engine/model/range~Range}
+	 */
+	clone() {
+		return new this.constructor( this.start, this.end );
+	}
+
+	/**
 	 * Returns a result of transforming a copy of this range by insert operation.
 	 *
 	 * One or more ranges may be returned as a result of this transformation.
@@ -654,7 +663,7 @@ export default class Range {
 				)
 			];
 		} else {
-			const range = Range._createFromRange( this );
+			const range = new Range( this.start, this.end );
 
 			range.start = range.start._getTransformedByInsertion( insertPosition, howMany );
 			range.end = range.end._getTransformedByInsertion( insertPosition, howMany );
@@ -798,17 +807,6 @@ export default class Range {
 	}
 
 	/**
-	 * Creates a new instance of `Range` which is equal to passed range.
-	 *
-	 * @protected
-	 * @param {module:engine/model/range~Range} range Range to clone.
-	 * @returns {module:engine/model/range~Range}
-	 */
-	static _createFromRange( range ) {
-		return new this( range.start, range.end );
-	}
-
-	/**
 	 * Creates a range inside an {@link module:engine/model/element~Element element} which starts before the first child of
 	 * that element and ends after the last child of that element.
 	 *
@@ -856,7 +854,7 @@ export default class Range {
 			 */
 			throw new CKEditorError( 'range-create-from-ranges-empty-array: At least one range has to be passed.' );
 		} else if ( ranges.length == 1 ) {
-			return this._createFromRange( ranges[ 0 ] );
+			return ranges[ 0 ].clone();
 		}
 
 		// 1. Set the first range in `ranges` array as a reference range.

+ 4 - 4
packages/ckeditor5-engine/src/model/selection.js

@@ -242,7 +242,7 @@ export default class Selection {
 	 */
 	* getRanges() {
 		for ( const range of this._ranges ) {
-			yield Range._createFromRange( range );
+			yield new Range( range.start, range.end );
 		}
 	}
 
@@ -265,7 +265,7 @@ export default class Selection {
 			}
 		}
 
-		return first ? Range._createFromRange( first ) : null;
+		return first ? new Range( first.start, first.end ) : null;
 	}
 
 	/**
@@ -287,7 +287,7 @@ export default class Selection {
 			}
 		}
 
-		return last ? Range._createFromRange( last ) : null;
+		return last ? new Range( last.start, last.end ) : null;
 	}
 
 	/**
@@ -700,7 +700,7 @@ export default class Selection {
 	 */
 	_pushRange( range ) {
 		this._checkRange( range );
-		this._ranges.push( Range._createFromRange( range ) );
+		this._ranges.push( new Range( range.start, range.end ) );
 	}
 
 	/**

+ 5 - 12
packages/ckeditor5-engine/src/view/range.js

@@ -248,7 +248,7 @@ export default class Range {
 			}
 		} else {
 			// Ranges do not intersect, return the original range.
-			ranges.push( Range._createFromRange( this ) );
+			ranges.push( this.clone() );
 		}
 
 		return ranges;
@@ -326,6 +326,10 @@ export default class Range {
 		return this.start.getCommonAncestor( this.end );
 	}
 
+	clone() {
+		return new Range( this.start, this.end );
+	}
+
 	/**
 	 * Returns an iterator that iterates over all {@link module:engine/view/item~Item view items} that are in this range and returns
 	 * them.
@@ -406,17 +410,6 @@ export default class Range {
 	}
 
 	/**
-	 * Creates and returns a new instance of Range which is equal to passed range.
-	 *
-	 * @protected
-	 * @param {module:engine/view/range~Range} range Range to clone.
-	 * @returns {module:engine/view/range~Range}
-	 */
-	static _createFromRange( range ) {
-		return new this( range.start, range.end );
-	}
-
-	/**
 	 * Creates a new range, spreading from specified {@link module:engine/view/position~Position position} to a position moved by
 	 * given `shift`. If `shift` is a negative value, shifted position is treated as the beginning of the range.
 	 *

+ 4 - 4
packages/ckeditor5-engine/src/view/selection.js

@@ -240,7 +240,7 @@ export default class Selection {
 	 */
 	* getRanges() {
 		for ( const range of this._ranges ) {
-			yield Range._createFromRange( range );
+			yield range.clone();
 		}
 	}
 
@@ -261,7 +261,7 @@ export default class Selection {
 			}
 		}
 
-		return first ? Range._createFromRange( first ) : null;
+		return first ? first.clone() : null;
 	}
 
 	/**
@@ -280,7 +280,7 @@ export default class Selection {
 			}
 		}
 
-		return last ? Range._createFromRange( last ) : null;
+		return last ? last.clone() : null;
 	}
 
 	/**
@@ -689,7 +689,7 @@ export default class Selection {
 			}
 		}
 
-		this._ranges.push( Range._createFromRange( range ) );
+		this._ranges.push( new Range( range.start, range.end ) );
 	}
 
 	/**

+ 2 - 5
packages/ckeditor5-engine/tests/manual/highlight.js

@@ -5,9 +5,6 @@
 
 /* global console, window, document */
 
-import ModelRange from '../../src/model/range';
-import ViewPosition from '../../src/view/position';
-
 import {
 	upcastElementToElement,
 } from '../../src/conversion/upcast-converters';
@@ -51,7 +48,7 @@ class FancyWidget extends Plugin {
 			model: 'fancywidget',
 			view: ( modelItem, viewWriter ) => {
 				const widgetElement = viewWriter.createContainerElement( 'figure', { class: 'fancy-widget' } );
-				viewWriter.insert( ViewPosition._createAt( widgetElement ), viewWriter.createText( 'widget' ) );
+				viewWriter.insert( viewWriter.createPositionAt( widgetElement ), viewWriter.createText( 'widget' ) );
 
 				return toWidget( widgetElement, viewWriter );
 			}
@@ -127,7 +124,7 @@ ClassicEditor.create( global.document.querySelector( '#editor' ), {
 
 function addMarker( editor, color ) {
 	editor.model.change( writer => {
-		const range = ModelRange._createFromRange( editor.model.document.selection.getFirstRange() );
+		const range = editor.model.document.selection.getFirstRange();
 		writer.addMarker( 'marker:' + color, { range, usingOperation: false } );
 	} );
 }

+ 1 - 1
packages/ckeditor5-engine/tests/manual/markers.js

@@ -108,7 +108,7 @@ function uid() {
 
 function addHighlight( color ) {
 	model.change( writer => {
-		const range = Range._createFromRange( model.document.selection.getFirstRange() );
+		const range = model.document.selection.getFirstRange();
 		const name = 'highlight:' + color + ':' + uid();
 
 		markerNames.push( name );

+ 3 - 9
packages/ckeditor5-engine/tests/model/liverange.js

@@ -79,15 +79,9 @@ describe( 'LiveRange', () => {
 		range.detach();
 	} );
 
-	it( '_createFromRange should return LiveRange', () => {
-		const range = LiveRange._createFromRange( new Range( new Position( root, [ 0 ] ), new Position( root, [ 1 ] ) ) );
-		expect( range ).to.be.instanceof( LiveRange );
-		range.detach();
-	} );
-
 	it( 'should fire change:range event with when its boundaries are changed', () => {
 		const live = new LiveRange( new Position( root, [ 0, 1, 4 ] ), new Position( root, [ 0, 2, 2 ] ) );
-		const copy = Range._createFromRange( live );
+		const copy = live.toRange();
 
 		const spy = sinon.spy();
 		live.on( 'change:range', spy );
@@ -657,7 +651,7 @@ describe( 'LiveRange', () => {
 
 		beforeEach( () => {
 			live = new LiveRange( new Position( root, [ 0, 1, 4 ] ), new Position( root, [ 0, 2, 2 ] ) );
-			clone = Range._createFromRange( live );
+			clone = live.toRange();
 
 			spy = sinon.spy();
 			live.on( 'change:content', spy );
@@ -742,7 +736,7 @@ describe( 'LiveRange', () => {
 		beforeEach( () => {
 			otherRoot = doc.createRoot( '$root', 'otherRoot' );
 			live = new LiveRange( new Position( root, [ 0, 1, 4 ] ), new Position( root, [ 0, 2, 2 ] ) );
-			clone = Range._createFromRange( live );
+			clone = live.toRange();
 
 			spy = sinon.spy();
 			live.on( 'change', spy );

+ 4 - 4
packages/ckeditor5-engine/tests/model/range.js

@@ -108,7 +108,7 @@ describe( 'Range', () => {
 
 	describe( 'isIntersecting()', () => {
 		it( 'should return true if given range is equal', () => {
-			const otherRange = Range._createFromRange( range );
+			const otherRange = range.clone();
 			expect( range.isIntersecting( otherRange ) ).to.be.true;
 		} );
 
@@ -187,7 +187,7 @@ describe( 'Range', () => {
 
 		describe( '_createFromRange()', () => {
 			it( 'should create a new instance of Range that is equal to passed range', () => {
-				const clone = Range._createFromRange( range );
+				const clone = range.clone();
 
 				expect( clone ).not.to.be.equal( range ); // clone is not pointing to the same object as position
 				expect( clone.isEqual( range ) ).to.be.true; // but they are equal in the position-sense
@@ -386,7 +386,7 @@ describe( 'Range', () => {
 		} );
 
 		it( 'should return true if ranges are equal and check is not strict', () => {
-			const otherRange = Range._createFromRange( range );
+			const otherRange = range.clone();
 
 			expect( range.containsRange( otherRange, true ) ).to.be.true;
 		} );
@@ -760,7 +760,7 @@ describe( 'Range', () => {
 		} );
 
 		it( 'should return a range equal to both ranges if both ranges are equal', () => {
-			const otherRange = Range._createFromRange( range );
+			const otherRange = range.clone();
 			const common = range.getIntersection( otherRange );
 
 			expect( common.isEqual( range ) ).to.be.true;

+ 3 - 13
packages/ckeditor5-engine/tests/view/range.js

@@ -318,7 +318,7 @@ describe( 'Range', () => {
 		} );
 
 		it( 'should return false if ranges are equal', () => {
-			const otherRange = Range._createFromRange( range );
+			const otherRange = range.clone();
 
 			expect( range.containsRange( otherRange ) ).to.be.false;
 		} );
@@ -330,7 +330,7 @@ describe( 'Range', () => {
 		} );
 
 		it( 'should return true if ranges are equal and check is not strict', () => {
-			const otherRange = Range._createFromRange( range );
+			const otherRange = range.clone();
 
 			expect( range.containsRange( otherRange, true ) ).to.be.true;
 		} );
@@ -378,7 +378,7 @@ describe( 'Range', () => {
 		describe( 'isIntersecting', () => {
 			it( 'should return true if given range is equal', () => {
 				const range = Range._createFromParentsAndOffsets( t1, 0, t3, 2 );
-				const otherRange = Range._createFromRange( range );
+				const otherRange = range.clone();
 				expect( range.isIntersecting( otherRange ) ).to.be.true;
 				expect( otherRange.isIntersecting( range ) ).to.be.true;
 			} );
@@ -719,16 +719,6 @@ describe( 'Range', () => {
 				expect( range.start.offset ).to.deep.equal( 2 );
 			} );
 		} );
-
-		describe( '_createFromRange', () => {
-			it( 'should create a new instance of Range that is equal to passed range', () => {
-				const range = new Range( new Position( foz, 0 ), new Position( foz, 2 ) );
-				const clone = Range._createFromRange( range );
-
-				expect( clone ).not.to.be.equal( range ); // clone is not pointing to the same object as position
-				expect( clone.isEqual( range ) ).to.be.true; // but they are equal in the position-sense
-			} );
-		} );
 	} );
 
 	describe( 'getCommonAncestor()', () => {