Kaynağa Gözat

Return previous and next positions from the TreeWalker. Changed Range methods.

Piotr Jasiun 9 yıl önce
ebeveyn
işleme
097c6430e9

+ 18 - 29
packages/ckeditor5-engine/src/treemodel/delta/attributedelta.js

@@ -86,7 +86,7 @@ function changeNode( doc, delta, key, value, node ) {
 
 // Because attribute operation needs to have the same attribute value on the whole range, this function split the range
 // into smaller parts.
-function changeRange( doc, delta, key, value, range ) {
+function changeRange( doc, delta, attributeKey, attributeValue, range ) {
 	// Position of the last split, the beginning of the new range.
 	let lastSplitPosition = range.start;
 
@@ -94,48 +94,37 @@ function changeRange( doc, delta, key, value, range ) {
 	// position of the iterator but the previous one (we need to iterate one more time to get the value after).
 	let position;
 	// Value before the currently position.
-	let valueBefore;
+	let attributeValueBefore;
 	// Value after the currently position.
-	let valueAfter;
-
-	// Because we need not only a node, but also a position, we can not use ( value of range ).
-	const iterator = range[ Symbol.iterator ]();
-	// Iterator state.
-	let next = iterator.next();
-
-	while ( !next.done ) {
-		// We check values only when the range contains given element, that is when the iterator "enters" the element.
-		// To prevent double-checking or not needed checking, we filter-out iterator values for ELEMENT_END position.
-		if ( next.value.type != 'ELEMENT_END' ) {
-			valueAfter = next.value.item.getAttribute( key );
-
-			// At the first run of the iterator the position in undefined. We also do not have a valueBefore, but
-			// because valueAfter may be null, valueBefore may be equal valueAfter ( undefined == null ).
-			if ( position && valueBefore != valueAfter ) {
-				// if valueBefore == value there is nothing to change, so we add operation only if these values are different.
-				if ( valueBefore != value ) {
-					addOperation();
-				}
-
-				lastSplitPosition = position;
+	let attributeValueAfter;
+
+	for ( let value of range.getValues( true ) ) {
+		attributeValueAfter = value.item.getAttribute( attributeKey );
+
+		// At the first run of the iterator the position in undefined. We also do not have a attributeValueBefore, but
+		// because attributeValueAfter may be null, attributeValueBefore may be equal attributeValueAfter ( undefined == null ).
+		if ( position && attributeValueBefore != attributeValueAfter ) {
+			// if attributeValueBefore == attributeValue there is nothing to change, so we add operation only if these values are different.
+			if ( attributeValueBefore != attributeValue ) {
+				addOperation();
 			}
 
-			position = iterator.position;
-			valueBefore = valueAfter;
+			lastSplitPosition = position;
 		}
 
-		next = iterator.next();
+		position = value.nextPosition;
+		attributeValueBefore = attributeValueAfter;
 	}
 
 	// Because position in the loop is not the iterator position (see let position comment), the last position in
 	// the while loop will be last but one position in the range. We need to check the last position manually.
-	if ( position instanceof Position && position != lastSplitPosition && valueBefore != value ) {
+	if ( position instanceof Position && position != lastSplitPosition && attributeValueBefore != attributeValue ) {
 		addOperation();
 	}
 
 	function addOperation() {
 		let range = new Range( lastSplitPosition, position );
-		const operation = new AttributeOperation( range, key, valueBefore, value, doc.version );
+		const operation = new AttributeOperation( range, attributeKey, attributeValueBefore, attributeValue, doc.version );
 
 		doc.applyOperation( operation );
 		delta.addOperation( operation );

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

@@ -90,19 +90,19 @@ export default class AttributeOperation extends Operation {
 	}
 
 	_execute() {
-		for ( let item of this.range.getAllNodes( true ) ) {
+		for ( let item of this.range.getItems( true ) ) {
 			if ( this.oldValue !== null && item.getAttribute( this.key ) !== this.oldValue ) {
 				/**
 				 * The attribute which should be removed does not exists for the given node.
 				 *
 				 * @error operation-attribute-no-attr-to-remove
-				 * @param {core.treeModel.Node} node
+				 * @param {core.treeModel.Node|core.treeModel.TextFragment} item
 				 * @param {String} key
 				 * @param {*} value
 				 */
 				throw new CKEditorError(
 					'operation-attribute-no-attr-to-remove: The attribute which should be removed does not exists for given node.',
-					{ node: item, key: this.key, value: this.oldValue }
+					{ item: item, key: this.key, value: this.oldValue }
 				);
 			}
 

+ 8 - 11
packages/ckeditor5-engine/src/treemodel/range.js

@@ -67,15 +67,6 @@ export default class Range {
 		return this.start.root;
 	}
 
-	/**
-	 * Range iterator.
-	 *
-	 * @see core.treeModel.TreeWalker
-	 */
-	[ Symbol.iterator ]() {
-		return new TreeWalker( { boundaries: this } );
-	}
-
 	/**
 	 * Checks whether this contains given {@link core.treeModel.Position position}.
 	 *
@@ -276,7 +267,7 @@ export default class Range {
 	 * (`false`) objects. Defaults to `false`.
 	 * @returns {Iterable.<core.treeModel.Node|treeModel.TextFragment>}
 	 */
-	*getAllNodes( mergeCharacters ) {
+	*getValues( mergeCharacters ) {
 		let it = new TreeWalker( { boundaries: this, mergeCharacters: mergeCharacters } );
 		let step;
 
@@ -284,11 +275,17 @@ export default class Range {
 			step = it.next();
 
 			if ( step.value && step.value.type != 'ELEMENT_END' ) {
-				yield step.value.item;
+				yield step.value;
 			}
 		} while ( !step.done );
 	}
 
+	*getItems( mergeCharacters ) {
+		for ( let value of this.getValues( mergeCharacters ) ) {
+			yield value.item;
+		}
+	}
+
 	/**
 	 * Returns an iterator that iterates over all {@link core.treeModel.Position positions} that are boundaries or
 	 * contained in this range.

+ 22 - 10
packages/ckeditor5-engine/src/treemodel/treewalker.js

@@ -93,6 +93,10 @@ export default class TreeWalker {
 		this._visitedParent = this.position.parent;
 	}
 
+	[ Symbol.iterator ]() {
+		return this;
+	}
+
 	/**
 	 * Makes a step forward in tree model. Moves the {@link #position} to the next position and returns the encountered value.
 	 *
@@ -101,6 +105,7 @@ export default class TreeWalker {
 	 * @returns {core.treeModel.TreeWalkerItem} return.value Information about taken step.
 	 */
 	next() {
+		const previousPosition = this.position;
 		const position = Position.createFromPosition( this.position );
 		const parent = this._visitedParent;
 
@@ -123,7 +128,7 @@ export default class TreeWalker {
 
 			this._visitedParent = node;
 
-			return formatReturnValue( 'ELEMENT_START', node );
+			return formatReturnValue( 'ELEMENT_START', node, previousPosition, position, 1 );
 		} else if ( node instanceof CharacterProxy ) {
 			if ( this.mergeCharacters ) {
 				let charactersCount = node._nodeListText.text.length - node._index;
@@ -139,12 +144,12 @@ export default class TreeWalker {
 				position.offset = offset;
 				this.position = position;
 
-				return formatReturnValue( 'TEXT', textFragment );
+				return formatReturnValue( 'TEXT', textFragment, previousPosition, position, charactersCount );
 			} else {
 				position.offset++;
 				this.position = position;
 
-				return formatReturnValue( 'CHARACTER', node );
+				return formatReturnValue( 'CHARACTER', node, previousPosition, position, 1 );
 			}
 		} else {
 			position.path.pop();
@@ -153,7 +158,7 @@ export default class TreeWalker {
 
 			this._visitedParent = parent.parent;
 
-			return formatReturnValue( 'ELEMENT_END', parent );
+			return formatReturnValue( 'ELEMENT_END', parent, previousPosition, position );
 		}
 	}
 
@@ -165,6 +170,7 @@ export default class TreeWalker {
 	 * @returns {core.treeModel.TreeWalkerItem} return.value Information about taken step.
 	 */
 	previous() {
+		const previousPosition = this.position;
 		const position = Position.createFromPosition( this.position );
 		const parent = this._visitedParent;
 
@@ -188,7 +194,7 @@ export default class TreeWalker {
 
 			this._visitedParent = node;
 
-			return formatReturnValue( 'ELEMENT_END', node );
+			return formatReturnValue( 'ELEMENT_END', node, position, previousPosition );
 		} else if ( node instanceof CharacterProxy ) {
 			if ( this.mergeCharacters ) {
 				let charactersCount = node._index + 1;
@@ -204,12 +210,12 @@ export default class TreeWalker {
 				position.offset = offset;
 				this.position = position;
 
-				return formatReturnValue( 'TEXT', textFragment );
+				return formatReturnValue( 'TEXT', textFragment, position, previousPosition, charactersCount );
 			} else {
 				position.offset--;
 				this.position = position;
 
-				return formatReturnValue( 'CHARACTER', node );
+				return formatReturnValue( 'CHARACTER', node, position, previousPosition, 1 );
 			}
 		} else {
 			position.path.pop();
@@ -217,17 +223,20 @@ export default class TreeWalker {
 
 			this._visitedParent = parent.parent;
 
-			return formatReturnValue( 'ELEMENT_START', parent );
+			return formatReturnValue( 'ELEMENT_START', parent, position, previousPosition, 1 );
 		}
 	}
 }
 
-function formatReturnValue( type, item ) {
+function formatReturnValue( type, item, previousPosition, nextPosition, length ) {
 	return {
 		done: false,
 		value: {
 			type: type,
-			item: item
+			item: item,
+			previousPosition: previousPosition,
+			nextPosition: nextPosition,
+			length: length
 		}
 	};
 }
@@ -247,4 +256,7 @@ function formatReturnValue( type, item ) {
  * @typedef {Object} core.treeModel.TreeWalkerItem
  * @property {treeModel.TreeWalkerItemType} type
  * @property {treeModel.Node|treeModel.TextFragment} item Value between old and new position of {@link core.treeModel.TreeWalker}.
+ * @property {treeModel.Position} positionBefore Position before item.
+ * @property {Number} [length] Length of the item. For `'ELEMENT_START'` and `'CHARACTER'` it is 1. For `'TEXT'` it is
+ * the length of the text. For `'ELEMENT_END'` it is undefined.
  */

+ 5 - 2
packages/ckeditor5-engine/tests/treemodel/_utils/utils.js

@@ -5,6 +5,8 @@
 
 'use strict';
 
+import TreeWalker from '/ckeditor5/core/treemodel/treewalker.js';
+
 const utils = {
 	/**
 	 * Returns tree structure as a simplified string. Elements are uppercase and characters are lowercase.
@@ -18,9 +20,10 @@ const utils = {
 	 */
 	getNodesAndText( range ) {
 		let txt = '';
+		const treeWalker = new TreeWalker( { boundaries: range } );
 
-		for ( let step of range ) {
-			let node = step.item;
+		for ( let value of treeWalker ) {
+			let node = value.item;
 			let nodeText = node.text || node.character;
 
 			if ( nodeText ) {

+ 2 - 2
packages/ckeditor5-engine/tests/treemodel/delta/attributedelta.js

@@ -135,7 +135,7 @@ describe( 'Batch', () => {
 
 			for ( let delta of batch.deltas ) {
 				for ( let operation of delta.operations ) {
-					count += getIteratorCount( operation.range.getAllNodes() );
+					count += getIteratorCount( operation.range.getValues() );
 				}
 			}
 
@@ -146,7 +146,7 @@ describe( 'Batch', () => {
 			// default: 111---111222---1112------
 			const range = Range.createFromElement( root );
 
-			return Array.from( range.getAllNodes() ).map( node => node.getAttribute( 'a' ) || '-' ).join( '' );
+			return Array.from( range.getValues() ).map( value => value.item.getAttribute( 'a' ) || '-' ).join( '' );
 		}
 
 		describe( 'setAttr', () => {

+ 7 - 7
packages/ckeditor5-engine/tests/treemodel/range.js

@@ -186,7 +186,7 @@ describe( 'Range', () => {
 		} );
 	} );
 
-	describe( 'getAllNodes', () => {
+	describe( 'getValues', () => {
 		it( 'should iterate over all nodes which "starts" in the range', () => {
 			const a = new Text( 'a' );
 			const b = new Text( 'b' );
@@ -205,19 +205,19 @@ describe( 'Range', () => {
 				new Position( root, [ 1, 1 ] )
 			);
 
-			let nodes = Array.from( range.getAllNodes() );
+			let values = Array.from( range.getValues() );
 
-			expect( nodes.length ).to.equal( 3 );
-			expect( nodes[ 0 ].character ).to.equal( 'b' );
-			expect( nodes[ 1 ] ).to.equal( e2 );
-			expect( nodes[ 2 ].character ).to.equal( 'x' );
+			expect( values.length ).to.equal( 3 );
+			expect( values[ 0 ].item.character ).to.equal( 'b' );
+			expect( values[ 1 ].item ).to.equal( e2 );
+			expect( values[ 2 ].item.character ).to.equal( 'x' );
 		} );
 
 		it( 'should merge characters with same attributes', () => {
 			prepareRichRoot( root );
 
 			let range = new Range( new Position( root, [ 0, 0, 3 ] ), new Position( root, [ 3, 0, 2 ] ) );
-			let nodes = Array.from( range.getAllNodes( true ) );
+			let nodes = Array.from( range.getItems( true ) );
 			let nodeNames = mapNodesToNames( nodes );
 
 			expect( nodeNames ).to.deep.equal( [ 'T:st', 'E:p', 'T:lorem ipsum', 'E:p', 'T:foo', 'E:p', 'T:bar', 'E:div', 'E:h', 'T:se' ] );

+ 26 - 1
packages/ckeditor5-engine/tests/treemodel/treewalker.js

@@ -79,8 +79,33 @@ describe( 'range iterator', () => {
 
 			expect( text ).to.equal( expected.text );
 			expect( Array.from( attrs ) ).to.deep.equal( expected.attrs );
+			expect( item.value.length ).to.equal( text.length );
 		} else {
-			expect( item.value ).to.deep.equal( expected );
+			expect( item.value.type ).to.equal( expected.type );
+			expect( item.value.item ).to.equal( expected.item );
+
+			if ( item.value.type == 'ELEMENT_START' ) {
+				expect( item.value.length ).to.equal( 1 );
+			} else {
+				expect( item.value.length ).to.be.undefined;
+			}
+		}
+
+		if ( item.value.type == 'TEXT' ) {
+			expect( item.value.previousPosition ).to.deep.equal( Position.createBefore( item.value.item.first ) );
+		} else if ( item.value.type == 'ELEMENT_END' ) {
+			expect( item.value.previousPosition ).to.deep.equal(
+				Position.createFromParentAndOffset( item.value.item, item.value.item.getChildCount() ) );
+		} else {
+			expect( item.value.previousPosition ).to.deep.equal( Position.createBefore( item.value.item ) );
+		}
+
+		if ( item.value.type == 'TEXT' ) {
+			expect( item.value.nextPosition ).to.deep.equal( Position.createAfter( item.value.item.last ) );
+		} else if ( item.value.type == 'ELEMENT_START' ) {
+			expect( item.value.nextPosition ).to.deep.equal( Position.createFromParentAndOffset( item.value.item, 0 ) );
+		} else {
+			expect( item.value.nextPosition ).to.deep.equal( Position.createAfter( item.value.item ) );
 		}
 	}