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

Changed model writer to protected operation utils.

Oskar Wróbel 8 лет назад
Родитель
Сommit
1eecff4fc9

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

@@ -10,7 +10,7 @@
 import Operation from './operation';
 import Range from '../range';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
-import writer from '../writer';
+import { setAttribute } from './utils';
 import isEqual from '@ckeditor/ckeditor5-utils/src/lib/lodash/isEqual';
 
 /**
@@ -151,7 +151,7 @@ export default class AttributeOperation extends Operation {
 		// If value to set is same as old value, don't do anything.
 		if ( !isEqual( this.oldValue, this.newValue ) ) {
 			// Execution.
-			writer.setAttribute( this.range, this.key, this.newValue );
+			setAttribute( this.range, this.key, this.newValue );
 		}
 
 		return { range: this.range, key: this.key, oldValue: this.oldValue, newValue: this.newValue };

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

@@ -10,7 +10,7 @@
 import Operation from './operation';
 import Position from '../position';
 import Range from '../range';
-import { remove } from '../writer';
+import { remove } from './utils';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 
 /**

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

@@ -11,7 +11,7 @@ import Operation from './operation';
 import Position from '../position';
 import NodeList from '../nodelist';
 import RemoveOperation from './removeoperation';
-import { insert, normalizeNodes } from '../writer';
+import { insert, normalizeNodes } from './utils';
 import Text from '../text';
 import Element from '../element';
 

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

@@ -12,7 +12,7 @@ import Position from '../position';
 import Range from '../range';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 import compareArrays from '@ckeditor/ckeditor5-utils/src/comparearrays';
-import writer from './../writer';
+import { move } from './utils';
 
 /**
  * Operation to move a range of {@link module:engine/model/item~Item model items}
@@ -184,7 +184,7 @@ export default class MoveOperation extends Operation {
 			}
 		}
 
-		const range = writer.move( Range.createFromPositionAndShift( this.sourcePosition, this.howMany ), this.targetPosition );
+		const range = move( Range.createFromPositionAndShift( this.sourcePosition, this.howMany ), this.targetPosition );
 
 		return {
 			sourcePosition: this.sourcePosition,

+ 29 - 41
packages/ckeditor5-engine/src/model/writer.js → packages/ckeditor5-engine/src/model/operation/utils.js

@@ -4,48 +4,39 @@
  */
 
 /**
- * @module engine/model/writer
+ * @module engine/model/operation/utils
  */
 
-import Node from './node';
-import Text from './text';
-import TextProxy from './textproxy';
-import Range from './range';
-import DocumentFragment from './documentfragment';
-import NodeList from './nodelist';
+import Node from '../node';
+import Text from '../text';
+import TextProxy from '../textproxy';
+import Range from '../range';
+import DocumentFragment from '../documentfragment';
+import NodeList from '../nodelist';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 
 /**
- * Contains functions used for composing model tree, grouped together under "model writer" name. Those functions
- * are built on top of {@link module:engine/model/node~Node node}, and it's child classes', APIs.
+ * Contains functions used for composing model tree by {@link module:engine/model/operation~Operation operations}.
+ * Those functions are built on top of {@link module:engine/model/node~Node node}, and it's child classes', APIs.
  *
- * Model writer API has multiple advantages and it is highly recommended to use it when changing model tree and nodes:
- * * model writer API {@link module:engine/model/writer~writer.normalizeNodes normalizes inserted nodes}, which means that you can insert
- * not only {@link module:engine/model/node~Node nodes}, but also `String`s, {@link module:engine/model/textproxy~TextProxy text proxies}
- * and
- * {@link module:engine/model/documentfragment~DocumentFragment document fragments},
- * * model writer API operates on {@link module:engine/model/position~Position positions}, which means that you have
- * better control over manipulating model tree as positions operate on offsets rather than indexes,
- * * model writer API automatically merges {@link module:engine/model/text~Text text nodes} with same attributes, which means
- * lower memory usage and better efficiency.
- *
- * @namespace writer
+ * @protected
+ * @namespace utils
  */
-const writer = {
+const utils = {
 	insert,
 	remove,
 	move,
 	setAttribute,
-	removeAttribute,
 	normalizeNodes
 };
 
-export default writer;
+export default utils;
 
 /**
  * Inserts given nodes at given position.
  *
- * @function module:engine/model/writer~writer.insert
+ * @protected
+ * @function module:engine/model/operation/utils~utils.insert
  * @param {module:engine/model/position~Position} position Position at which nodes should be inserted.
  * @param {module:engine/model/node~NodeSet} nodes Nodes to insert.
  * @returns {module:engine/model/range~Range} Range spanning over inserted elements.
@@ -75,7 +66,8 @@ export function insert( position, nodes ) {
 /**
  * Removed nodes in given range. Only {@link module:engine/model/range~Range#isFlat flat} ranges are accepted.
  *
- * @function module:engine/model/writer~writer.remove
+ * @protected
+ * @function module:engine/model/operation/utils~utils.remove
  * @param {module:engine/model/range~Range} range Range containing nodes to remove.
  * @returns {Array.<module:engine/model/node~Node>}
  */
@@ -84,9 +76,9 @@ export function remove( range ) {
 		/**
 		 * Trying to remove a range which starts and ends in different element.
 		 *
-		 * @error model-writer-remove-range-not-flat
+		 * @error operation-utils-remove-range-not-flat
 		 */
-		throw new CKEditorError( 'model-writer-remove-range-not-flat: ' +
+		throw new CKEditorError( 'operation-utils-remove-range-not-flat: ' +
 			'Trying to remove a range which starts and ends in different element.' );
 	}
 
@@ -109,6 +101,8 @@ export function remove( range ) {
 /**
  * Moves nodes in given range to given target position. Only {@link module:engine/model/range~Range#isFlat flat} ranges are accepted.
  *
+ * @protected
+ * @function module:engine/model/operation/utils~utils.move
  * @param {module:engine/model/range~Range} sourceRange Range containing nodes to move.
  * @param {module:engine/model/position~Position} targetPosition Position to which nodes should be moved.
  * @returns {module:engine/model/range~Range} Range containing moved nodes.
@@ -118,24 +112,26 @@ export function move( sourceRange, targetPosition ) {
 		/**
 		 * Trying to move a range which starts and ends in different element.
 		 *
-		 * @error model-writer-move-range-not-flat
+		 * @error operation-utils-move-range-not-flat
 		 */
-		throw new CKEditorError( 'model-writer-move-range-not-flat: ' +
+		throw new CKEditorError( 'operation-utils-move-range-not-flat: ' +
 			'Trying to move a range which starts and ends in different element.' );
 	}
 
-	const nodes = this.remove( sourceRange );
+	const nodes = remove( sourceRange );
 
 	// We have to fix `targetPosition` because model changed after nodes from `sourceRange` got removed and
 	// that change might have an impact on `targetPosition`.
 	targetPosition = targetPosition._getTransformedByDeletion( sourceRange.start, sourceRange.end.offset - sourceRange.start.offset );
 
-	return this.insert( targetPosition, nodes );
+	return insert( targetPosition, nodes );
 }
 
 /**
  * Sets given attribute on nodes in given range.
  *
+ * @protected
+ * @function module:engine/model/operation/utils~utils.setAttribute
  * @param {module:engine/model/range~Range} range Range containing nodes that should have the attribute set.
  * @param {String} key Key of attribute to set.
  * @param {*} value Attribute value.
@@ -166,20 +162,12 @@ export function setAttribute( range, key, value ) {
 	_mergeNodesAtIndex( range.end.parent, range.end.index );
 }
 
-/**
- * Removes given attribute from nodes in given range.
- *
- * @param {module:engine/model/range~Range} range Range containing nodes that should have the attribute removed.
- * @param {String} key Key of attribute to remove.
- */
-export function removeAttribute( range, key ) {
-	this.setAttribute( range, key, null );
-}
-
 /**
  * Normalizes given object or an array of objects to an array of {@link module:engine/model/node~Node nodes}. See
  * {@link module:engine/model/node~NodeSet NodeSet} for details on how normalization is performed.
  *
+ * @protected
+ * @function module:engine/model/operation/utils~utils.normalizeNodes
  * @param {module:engine/model/node~NodeSet} nodes Objects to normalize.
  * @returns {Array.<module:engine/model/node~Node>} Normalized nodes.
  */

+ 33 - 56
packages/ckeditor5-engine/tests/model/writer.js → packages/ckeditor5-engine/tests/model/operation/utils.js

@@ -3,21 +3,21 @@
  * For licensing, see LICENSE.md.
  */
 
-import Document from '../../src/model/document';
-import DocumentFragment from '../../src/model/documentfragment';
-import Element from '../../src/model/element';
-import Text from '../../src/model/text';
-import TextProxy from '../../src/model/textproxy';
-import Position from '../../src/model/position';
-import Range from '../../src/model/range';
-import writer from '../../src/model/writer';
-import { getData } from '../../src/dev-utils/model';
+import Document from '../../../src/model/document';
+import DocumentFragment from '../../../src/model/documentfragment';
+import Element from '../../../src/model/element';
+import Text from '../../../src/model/text';
+import TextProxy from '../../../src/model/textproxy';
+import Position from '../../../src/model/position';
+import Range from '../../../src/model/range';
+import utils from '../../../src/model/operation/utils';
+import { getData } from '../../../src/dev-utils/model';
 
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 
 let doc, root;
 
-describe( 'writer', () => {
+describe( 'writer utils', () => {
 	beforeEach( () => {
 		doc = new Document();
 		doc.schema.allow( { name: '$text', inside: '$root' } );
@@ -38,19 +38,19 @@ describe( 'writer', () => {
 
 	describe( 'insert', () => {
 		it( 'should insert nodes between nodes', () => {
-			writer.insert( Position.createAt( root, 3 ), [ 'xxx', new Element( 'p' ) ] );
+			utils.insert( Position.createAt( root, 3 ), [ 'xxx', new Element( 'p' ) ] );
 
 			expectData( 'fooxxx<p></p><$text bold="true">bar</$text><image src="img.jpg"></image>xyz' );
 		} );
 
 		it( 'should split text node if nodes at inserted at offset inside text node', () => {
-			writer.insert( Position.createAt( root, 5 ), new Element( 'p' ) );
+			utils.insert( Position.createAt( root, 5 ), new Element( 'p' ) );
 
 			expectData( 'foo<$text bold="true">ba</$text><p></p><$text bold="true">r</$text><image src="img.jpg"></image>xyz' );
 		} );
 
 		it( 'should merge text nodes if possible', () => {
-			writer.insert( Position.createAt( root, 3 ), new Text( 'xxx', { bold: true } ) );
+			utils.insert( Position.createAt( root, 3 ), new Text( 'xxx', { bold: true } ) );
 
 			expectData( 'foo<$text bold="true">xxxbar</$text><image src="img.jpg"></image>xyz' );
 		} );
@@ -59,21 +59,21 @@ describe( 'writer', () => {
 	describe( 'remove', () => {
 		it( 'should remove nodes in given range', () => {
 			const range = Range.createFromParentsAndOffsets( root, 3, root, 6 );
-			writer.remove( range );
+			utils.remove( range );
 
 			expectData( 'foo<image src="img.jpg"></image>xyz' );
 		} );
 
 		it( 'should split text node if range starts or ends inside text node', () => {
 			const range = Range.createFromParentsAndOffsets( root, 1, root, 5 );
-			writer.remove( range );
+			utils.remove( range );
 
 			expectData( 'f<$text bold="true">r</$text><image src="img.jpg"></image>xyz' );
 		} );
 
 		it( 'should merge text nodes if possible', () => {
 			const range = Range.createFromParentsAndOffsets( root, 3, root, 7 );
-			writer.remove( range );
+			utils.remove( range );
 
 			expectData( 'fooxyz' );
 			expect( root.childCount ).to.equal( 1 );
@@ -81,89 +81,66 @@ describe( 'writer', () => {
 
 		it( 'should throw if given range is not flat', () => {
 			expect( () => {
-				writer.remove( new Range( new Position( root, [ 0 ] ), new Position( root, [ 1, 2 ] ) ) );
-			} ).to.throw( CKEditorError, /model-writer-remove-range-not-flat/ );
+				utils.remove( new Range( new Position( root, [ 0 ] ), new Position( root, [ 1, 2 ] ) ) );
+			} ).to.throw( CKEditorError, /operation-utils-remove-range-not-flat/ );
 		} );
 	} );
 
 	describe( 'move', () => {
 		it( 'should move a range of nodes', () => {
 			const range = Range.createFromParentsAndOffsets( root, 3, root, 6 );
-			writer.move( range, Position.createAt( root, 0 ) );
+			utils.move( range, Position.createAt( root, 0 ) );
 
 			expectData( '<$text bold="true">bar</$text>foo<image src="img.jpg"></image>xyz' );
 		} );
 
-		it( 'should use remove and insert methods', () => {
-			sinon.spy( writer, 'remove' );
-			sinon.spy( writer, 'insert' );
-
-			const range = Range.createFromParentsAndOffsets( root, 3, root, 6 );
-			const position = Position.createAt( root, 0 );
-			writer.move( range, position );
-
-			expect( writer.remove.calledWithExactly( range ) ).to.be.true;
-			expect( writer.insert.calledWith( position ) ).to.be.true;
-		} );
-
 		it( 'should correctly move if target position is in same element as moved range, but after range', () => {
 			const range = Range.createFromParentsAndOffsets( root, 3, root, 6 );
-			writer.move( range, Position.createAt( root, 10 ) );
+			utils.move( range, Position.createAt( root, 10 ) );
 
 			expectData( 'foo<image src="img.jpg"></image>xyz<$text bold="true">bar</$text>' );
 		} );
 
 		it( 'should throw if given range is not flat', () => {
 			expect( () => {
-				writer.move( new Range( new Position( root, [ 0 ] ), new Position( root, [ 1, 2 ] ) ), null );
-			} ).to.throw( CKEditorError, /model-writer-move-range-not-flat/ );
+				utils.move( new Range( new Position( root, [ 0 ] ), new Position( root, [ 1, 2 ] ) ), null );
+			} ).to.throw( CKEditorError, /operation-utils-move-range-not-flat/ );
 		} );
 	} );
 
 	describe( 'setAttribute', () => {
 		it( 'should set attribute on given range of nodes', () => {
 			const range = Range.createFromParentsAndOffsets( root, 6, root, 8 );
-			writer.setAttribute( range, 'newAttr', true );
+			utils.setAttribute( range, 'newAttr', true );
 
 			expectData( 'foo<$text bold="true">bar</$text><image newAttr="true" src="img.jpg"></image><$text newAttr="true">x</$text>yz' );
 		} );
 
 		it( 'should remove attribute if null was passed as a value', () => {
 			const range = Range.createFromParentsAndOffsets( root, 6, root, 7 );
-			writer.setAttribute( range, 'src', null );
+			utils.setAttribute( range, 'src', null );
 
 			expectData( 'foo<$text bold="true">bar</$text><image></image>xyz' );
 		} );
 
 		it( 'should merge nodes if possible', () => {
 			const range = Range.createFromParentsAndOffsets( root, 0, root, 3 );
-			writer.setAttribute( range, 'bold', true );
+			utils.setAttribute( range, 'bold', true );
 
 			expectData( '<$text bold="true">foobar</$text><image src="img.jpg"></image>xyz' );
 		} );
 	} );
-
-	describe( 'removeAttribute', () => {
-		it( 'should use setAttribute', () => {
-			sinon.spy( writer, 'setAttribute' );
-
-			const range = Range.createFromParentsAndOffsets( root, 6, root, 7 );
-			writer.removeAttribute( range, 'src' );
-
-			expect( writer.setAttribute.calledWithExactly( range, 'src', null ) ).to.be.true;
-		} );
-	} );
 } );
 
 describe( 'normalizeNodes', () => {
 	it( 'should change single object into an array', () => {
 		const p = new Element( 'p' );
 
-		expect( writer.normalizeNodes( p ) ).to.deep.equal( [ p ] );
+		expect( utils.normalizeNodes( p ) ).to.deep.equal( [ p ] );
 	} );
 
 	it( 'should change strings to text nodes', () => {
-		const text = writer.normalizeNodes( 'abc' )[ 0 ];
+		const text = utils.normalizeNodes( 'abc' )[ 0 ];
 
 		expect( text ).to.be.instanceof( Text );
 		expect( text.data ).to.equal( 'abc' );
@@ -173,7 +150,7 @@ describe( 'normalizeNodes', () => {
 		const textNode = new Text( 'abc' );
 		const textProxy = new TextProxy( textNode, 1, 1 );
 
-		const text = writer.normalizeNodes( textProxy )[ 0 ];
+		const text = utils.normalizeNodes( textProxy )[ 0 ];
 
 		expect( text ).to.be.instanceof( Text );
 		expect( text.data ).to.equal( 'b' );
@@ -182,11 +159,11 @@ describe( 'normalizeNodes', () => {
 	it( 'should not change elements', () => {
 		const p = new Element( 'p' );
 
-		expect( writer.normalizeNodes( p )[ 0 ] ).to.equal( p );
+		expect( utils.normalizeNodes( p )[ 0 ] ).to.equal( p );
 	} );
 
 	it( 'should omit unrecognized objects', () => {
-		expect( writer.normalizeNodes( 1 ) ).to.deep.equal( [] );
+		expect( utils.normalizeNodes( 1 ) ).to.deep.equal( [] );
 	} );
 
 	it( 'should accept arrays', () => {
@@ -194,7 +171,7 @@ describe( 'normalizeNodes', () => {
 		const image = new Element( 'image' );
 		const nodes = [ 'abc', text, image, 1, 'xyz' ];
 
-		const normalized = writer.normalizeNodes( nodes );
+		const normalized = utils.normalizeNodes( nodes );
 
 		expect( normalized[ 0 ] ).to.be.instanceof( Text );
 		expect( normalized[ 1 ] ).to.equal( text );
@@ -203,7 +180,7 @@ describe( 'normalizeNodes', () => {
 	} );
 
 	it( 'should merge text nodes if mergeTextNodes flag is set to true', () => {
-		const normalized = writer.normalizeNodes( [ 'foo', 'bar' ], true );
+		const normalized = utils.normalizeNodes( [ 'foo', 'bar' ], true );
 
 		expect( normalized.length ).to.equal( 1 );
 		expect( normalized[ 0 ].data ).to.equal( 'foobar' );
@@ -216,7 +193,7 @@ describe( 'normalizeNodes', () => {
 			'xyz'
 		];
 
-		const normalized = writer.normalizeNodes( nodes, true );
+		const normalized = utils.normalizeNodes( nodes, true );
 
 		expect( normalized[ 0 ] ).to.be.instanceof( Text );
 		expect( normalized[ 0 ].getAttribute( 'bold' ) ).to.be.true;