From 9548f455988f92266d61b68c321827482fe1d0f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Fri, 20 Oct 2017 13:40:29 +0200 Subject: [PATCH 01/16] Other: Make Position immutable. --- src/model/delta/basic-transformations.js | 28 +- src/model/liveposition.js | 6 +- src/model/position.js | 254 ++++++++++----- src/model/range.js | 20 +- src/model/treewalker.js | 44 +-- tests/model/delta/transform/_utils/utils.js | 24 +- tests/model/delta/transform/movedelta.js | 4 +- tests/model/delta/transform/splitdelta.js | 13 +- tests/model/liverange.js | 10 +- tests/model/operation/transform.js | 326 ++++++++++---------- tests/model/position.js | 24 -- tests/model/range.js | 3 +- 12 files changed, 413 insertions(+), 343 deletions(-) diff --git a/src/model/delta/basic-transformations.js b/src/model/delta/basic-transformations.js index 5d25305a7..ecf49b165 100644 --- a/src/model/delta/basic-transformations.js +++ b/src/model/delta/basic-transformations.js @@ -73,8 +73,7 @@ addTransformationCase( AttributeDelta, SplitDelta, ( a, b, context ) => { const additionalAttributeDelta = new AttributeDelta(); const rangeStart = splitPosition.getShiftedBy( 1 ); - const rangeEnd = Position.createFromPosition( rangeStart ); - rangeEnd.path.push( 0 ); + const rangeEnd = rangeStart.getMovedToChild(); const oldValue = b._cloneOperation.nodes.getNode( 0 ).getAttribute( operation.key ); @@ -236,7 +235,7 @@ addTransformationCase( SplitDelta, SplitDelta, ( a, b, context ) => { a._cloneOperation instanceof ReinsertOperation && b._cloneOperation instanceof ReinsertOperation && a._cloneOperation.sourcePosition.offset > b._cloneOperation.sourcePosition.offset ) { - a._cloneOperation.sourcePosition.offset--; + a._cloneOperation.sourcePosition = a._cloneOperation.sourcePosition.getShiftedBy( -1 ); } // `a` splits closer or at same offset. @@ -317,29 +316,24 @@ addTransformationCase( SplitDelta, WrapDelta, ( a, b, context ) => { // Wrapping element is the element inserted by WrapDelta (re)insert operation. // It is inserted after the wrapped range, but the wrapped range will be moved inside it. // Having this in mind, it is correct to use wrapped range start position as the position before wrapping element. - const splitNodePos = Position.createFromPosition( b.range.start ); + // Now, `splitNodePos` points before wrapping element. // To get a position before last children of that element, we expand position's `path` member by proper offset. - splitNodePos.path.push( b.howMany - 1 ); + const splitNodePos = b.range.start.getMovedToChild( b.howMany - 1 ); // SplitDelta insert operation position should be right after the node we split. - const insertPos = splitNodePos.getShiftedBy( 1 ); - delta._cloneOperation.position = insertPos; + delta._cloneOperation.position = splitNodePos.getShiftedBy( 1 ); // 2. Fix move operation source position. // Nodes moved by SplitDelta will be moved from new position, modified by WrapDelta. // To obtain that new position, `splitNodePos` will be used, as this is the node we are extracting children from. - const sourcePos = Position.createFromPosition( splitNodePos ); // Nothing changed inside split node so it is correct to use the original split position offset. - sourcePos.path.push( a.position.offset ); - delta._moveOperation.sourcePosition = sourcePos; + delta._moveOperation.sourcePosition = splitNodePos.getMovedToChild( a.position.offset ); // 3. Fix move operation target position. // SplitDelta move operation target position should be inside the node inserted by operation above. // Since the node is empty, we will insert at offset 0. - const targetPos = Position.createFromPosition( insertPos ); - targetPos.path.push( 0 ); - delta._moveOperation.targetPosition = targetPos; + delta._moveOperation.targetPosition = splitNodePos.getShiftedBy( 1 ).getMovedToChild(); return [ delta ]; } @@ -434,13 +428,17 @@ addTransformationCase( WrapDelta, SplitDelta, ( a, b, context ) => { const delta = a.clone(); // Move wrapping element insert position one node further so it is after the split node insertion. - delta._insertOperation.position.offset++; + delta._insertOperation.position = delta._insertOperation.position.getShiftedBy( 1 ); // Include the split node copy. delta._moveOperation.howMany++; // Change the path to wrapping element in move operation. - delta._moveOperation.targetPosition.path[ delta._moveOperation.targetPosition.path.length - 2 ]++; + const index = delta._moveOperation.targetPosition.path.length - 2; + + const path = delta._moveOperation.targetPosition.path.slice(); + path[ index ] += 1; + delta._moveOperation.targetPosition = delta._moveOperation.targetPosition.getMovedToPath( path ); return [ delta ]; } diff --git a/src/model/liveposition.js b/src/model/liveposition.js index 9a19412d1..f9f2213d0 100644 --- a/src/model/liveposition.js +++ b/src/model/liveposition.js @@ -7,7 +7,7 @@ * @module engine/model/liveposition */ -import Position from './position'; +import Position, { setPath, setRoot } from './position'; import Range from './range'; import EmitterMixin from '@ckeditor/ckeditor5-utils/src/emittermixin'; import mix from '@ckeditor/ckeditor5-utils/src/mix'; @@ -194,8 +194,8 @@ function transform( type, range, position ) { if ( !this.isEqual( transformed ) ) { const oldPosition = Position.createFromPosition( this ); - this.path = transformed.path; - this.root = transformed.root; + setPath( this, transformed.path ); + setRoot( this, transformed.root ); this.fire( 'change', oldPosition ); } diff --git a/src/model/position.js b/src/model/position.js index 7d199b617..d3037b96c 100644 --- a/src/model/position.js +++ b/src/model/position.js @@ -13,6 +13,9 @@ import compareArrays from '@ckeditor/ckeditor5-utils/src/comparearrays'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; import Text from './text'; +const _path = Symbol( 'path' ); +const _root = Symbol( 'root' ); + /** * Represents a position in the model tree. * @@ -66,64 +69,66 @@ export default class Position { // Normalize the root and path (if element was passed). path = root.getPath().concat( path ); - root = root.root; - - /** - * Root of the position path. - * - * @readonly - * @member {module:engine/model/element~Element|module:engine/model/documentfragment~DocumentFragment} - * module:engine/model/position~Position#root - */ - this.root = root; - - /** - * Position of the node in the tree. **Path contains offsets, not indexes.** - * - * Position can be placed before, after or in a {@link module:engine/model/node~Node node} if that node has - * {@link module:engine/model/node~Node#offsetSize} greater than `1`. Items in position path are - * {@link module:engine/model/node~Node#startOffset starting offsets} of position ancestors, starting from direct root children, - * down to the position offset in it's parent. - * - * ROOT - * |- P before: [ 0 ] after: [ 1 ] - * |- UL before: [ 1 ] after: [ 2 ] - * |- LI before: [ 1, 0 ] after: [ 1, 1 ] - * | |- foo before: [ 1, 0, 0 ] after: [ 1, 0, 3 ] - * |- LI before: [ 1, 1 ] after: [ 1, 2 ] - * |- bar before: [ 1, 1, 0 ] after: [ 1, 1, 3 ] - * - * `foo` and `bar` are representing {@link module:engine/model/text~Text text nodes}. Since text nodes has offset size - * greater than `1` you can place position offset between their start and end: - * - * ROOT - * |- P - * |- UL - * |- LI - * | |- f^o|o ^ has path: [ 1, 0, 1 ] | has path: [ 1, 0, 2 ] - * |- LI - * |- b^a|r ^ has path: [ 1, 1, 1 ] | has path: [ 1, 1, 2 ] - * - * @member {Array.} module:engine/model/position~Position#path - */ - this.path = path; + + // Make path immutable + Object.freeze( path ); + + setRoot( this, root.root ); + setPath( this, path ); } /** - * Offset at which this position is located in its {@link module:engine/model/position~Position#parent parent}. It is equal - * to the last item in position {@link module:engine/model/position~Position#path path}. + * Position of the node in the tree. **Path contains offsets, not indexes.** + * + * Position can be placed before, after or in a {@link module:engine/model/node~Node node} if that node has + * {@link module:engine/model/node~Node#offsetSize} greater than `1`. Items in position path are + * {@link module:engine/model/node~Node#startOffset starting offsets} of position ancestors, starting from direct root children, + * down to the position offset in it's parent. + * + * ROOT + * |- P before: [ 0 ] after: [ 1 ] + * |- UL before: [ 1 ] after: [ 2 ] + * |- LI before: [ 1, 0 ] after: [ 1, 1 ] + * | |- foo before: [ 1, 0, 0 ] after: [ 1, 0, 3 ] + * |- LI before: [ 1, 1 ] after: [ 1, 2 ] + * |- bar before: [ 1, 1, 0 ] after: [ 1, 1, 3 ] + * + * `foo` and `bar` are representing {@link module:engine/model/text~Text text nodes}. Since text nodes has offset size + * greater than `1` you can place position offset between their start and end: + * + * ROOT + * |- P + * |- UL + * |- LI + * | |- f^o|o ^ has path: [ 1, 0, 1 ] | has path: [ 1, 0, 2 ] + * |- LI + * |- b^a|r ^ has path: [ 1, 1, 1 ] | has path: [ 1, 1, 2 ] + * + * @member {Array.} module:engine/model/position~Position#path + */ + get path() { + return this[ _path ]; + } + + /** + * Root of the position path. * - * @type {Number} + * @readonly + * @member {module:engine/model/element~Element|module:engine/model/documentfragment~DocumentFragment} + * module:engine/model/position~Position#root */ - get offset() { - return last( this.path ); + get root() { + return this[ _root ]; } /** - * @param {Number} newOffset + * Offset at which this position is located in its {@link module:engine/model/position~Position#parent parent}. It is equal + * to the last item in position {@link module:engine/model/position~Position#path path}. + * + * @type {Number} */ - set offset( newOffset ) { - this.path[ this.path.length - 1 ] = newOffset; + get offset() { + return getOffset( this.path ); } /** @@ -340,6 +345,31 @@ export default class Position { return i === 0 ? null : ancestorsA[ i - 1 ]; } + /** + * Returns a new instance of `Position`, that has same {@link #root root} but it's path is set to `path` value. + * + * @param {Array.} path Position path. See {@link module:engine/model/position~Position#path}. + * @returns {module:engine/model/position~Position} Moved position. + */ + getMovedToPath( path ) { + return new Position( this.root, path ); + } + + /** + * Returns a new instance of `Position`, that has same {@link #parent parent} but it's offset + * is set to `offset` value. + * + * @param {Number} offset Position offset. See {@link module:engine/model/position~Position#offset}. + * @returns {module:engine/model/position~Position} Moved position. + */ + getShiftedTo( offset ) { + const path = this.path.slice(); + + setOffset( path, offset ); + + return this.getMovedToPath( path ); + } + /** * Returns a new instance of `Position`, that has same {@link #parent parent} but it's offset * is shifted by `shift` value (can be a negative value). @@ -348,12 +378,31 @@ export default class Position { * @returns {module:engine/model/position~Position} Shifted position. */ getShiftedBy( shift ) { - const shifted = Position.createFromPosition( this ); + const newOffset = this.offset + shift; + + return this.getShiftedTo( newOffset < 0 ? 0 : newOffset ); + } + + /** + * Returns a new instance of `Position`, that has same {@link #root root} but it's moved in path by one level up. + * + * @returns {module:engine/model/position~Position} Moved position. + */ + getMovedToParent() { + return this.getMovedToPath( this.getParentPath() ); + } + + /** + * Returns a new instance of `Position`, that has same {@link #root root} but it's moved in path by one level down. + * + * @returns {module:engine/model/position~Position} Moved position. + */ + getMovedToChild( offsetInChild = 0 ) { + const path = this.path.slice(); - const offset = shifted.offset + shift; - shifted.offset = offset < 0 ? 0 : offset; + path.push( offsetInChild ); - return shifted; + return this.getMovedToPath( path ); } /** @@ -433,13 +482,13 @@ export default class Position { return true; case 'before': - left = Position.createFromPosition( this ); - right = Position.createFromPosition( otherPosition ); + left = this; // eslint-disable-line consistent-this + right = otherPosition; break; case 'after': - left = Position.createFromPosition( otherPosition ); - right = Position.createFromPosition( this ); + left = otherPosition; + right = this; // eslint-disable-line consistent-this break; default: @@ -459,19 +508,30 @@ export default class Position { return false; } - left.path = left.path.slice( 0, -1 ); + left = left.getMovedToParent().getShiftedBy( 1 ); leftParent = leftParent.parent; - left.offset++; } else { if ( right.offset !== 0 ) { return false; } - right.path = right.path.slice( 0, -1 ); + right = right.getMovedToParent(); } } } + /** + * Converts `Position` to plain object and returns it. + * + * @returns {Object} `Position` converted to plain object. + */ + toJSON() { + return { + root: this.root.toJSON(), + path: this.path + }; + } + /** * Returns a copy of this position that is updated by removing `howMany` nodes starting from `deletePosition`. * It may happen that this position is in a removed node. If that is the case, `null` is returned instead. @@ -482,14 +542,14 @@ export default class Position { * @returns {module:engine/model/position~Position|null} Transformed position or `null`. */ _getTransformedByDeletion( deletePosition, howMany ) { - const transformed = Position.createFromPosition( this ); - // This position can't be affected if deletion was in a different root. if ( this.root != deletePosition.root ) { - return transformed; + return this; } - if ( compareArrays( deletePosition.getParentPath(), this.getParentPath() ) == 'same' ) { + const comparisonResult = compareArrays( deletePosition.getParentPath(), this.getParentPath() ); + + if ( comparisonResult == 'same' ) { // If nodes are removed from the node that is pointed by this position... if ( deletePosition.offset < this.offset ) { // And are removed from before an offset of that position... @@ -497,11 +557,10 @@ export default class Position { // Position is in removed range, it's no longer in the tree. return null; } else { - // Decrement the offset accordingly. - transformed.offset -= howMany; + return this.getShiftedBy( -howMany ); } } - } else if ( compareArrays( deletePosition.getParentPath(), this.getParentPath() ) == 'prefix' ) { + } else if ( comparisonResult == 'prefix' ) { // If nodes are removed from a node that is on a path to this position... const i = deletePosition.path.length - 1; @@ -513,12 +572,14 @@ export default class Position { return null; } else { // Otherwise, decrement index on that path. - transformed.path[ i ] -= howMany; + const path = this.path.slice(); + path[ i ] -= howMany; + return this.getMovedToPath( path ); } } } - return transformed; + return this; } /** @@ -533,11 +594,9 @@ export default class Position { * @returns {module:engine/model/position~Position} Transformed position. */ _getTransformedByInsertion( insertPosition, howMany, insertBefore ) { - const transformed = Position.createFromPosition( this ); - // This position can't be affected if insertion was in a different root. if ( this.root != insertPosition.root ) { - return transformed; + return this; } if ( compareArrays( insertPosition.getParentPath(), this.getParentPath() ) == 'same' ) { @@ -545,7 +604,7 @@ export default class Position { if ( insertPosition.offset < this.offset || ( insertPosition.offset == this.offset && insertBefore ) ) { // And are inserted before an offset of that position... // "Push" this positions offset. - transformed.offset += howMany; + return this.getShiftedBy( howMany ); } } else if ( compareArrays( insertPosition.getParentPath(), this.getParentPath() ) == 'prefix' ) { // If nodes are inserted in a node that is on a path to this position... @@ -554,11 +613,13 @@ export default class Position { if ( insertPosition.offset <= this.path[ i ] ) { // And are inserted before next node of that path... // "Push" the index on that path. - transformed.path[ i ] += howMany; + const path = this.path.slice(); + path[ i ] += howMany; + return this.getMovedToPath( path ); } } - return transformed; + return this; } /** @@ -626,18 +687,18 @@ export default class Position { 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. - const combined = Position.createFromPosition( target ); + let combinedPath = target.path.slice(); // Then we have to update the rest of the path. // Fix the offset because this position might be after `from` position and we have to reflect that. - combined.offset = combined.offset + this.path[ i ] - source.offset; + setOffset( combinedPath, getOffset( combinedPath ) + this.path[ i ] - source.offset ); // Then, add the rest of the path. // If this position is at the same level as `from` position nothing will get added. - combined.path = combined.path.concat( this.path.slice( i + 1 ) ); + combinedPath = combinedPath.concat( this.path.slice( i + 1 ) ); - return combined; + return new Position( target.root, combinedPath ); } /** @@ -781,6 +842,41 @@ export default class Position { } } +/** + * Method used to expose root setter to child classes. + * @protected + * @param {module:engine/model/position~Position} position Position of which root should be modified. + * @param {module:engine/model/element~Element|module:engine/model/documentfragment~DocumentFragment} root Root of the position. + */ +export function setRoot( position, root ) { + position[ _root ] = root; +} + +/** + * Method used to expose path setter to child classes. + * @protected + * @param {module:engine/model/position~Position} position Position of which path should be modified. + * @param {Array.} path Position path. See {@link module:engine/model/position~Position#path}. + */ +export function setPath( position, path ) { + position[ _path ] = path; +} + +// Helper for setting offset on give path array. +// @private +// @param {Array.} path Position path. See {@link module:engine/model/position~Position#path}. +function getOffset( path ) { + return last( path ) || 0; +} + +// Helper for setting offset on give path array. +// @private +// @param {Array.} path Position path. See {@link module:engine/model/position~Position#path}. +// @param {Number} newOffset Offset to set. +function setOffset( path, newOffset ) { + path[ path.length - 1 ] = newOffset; +} + /** * A flag indicating whether this position is `'before'` or `'after'` or `'same'` as given position. * If positions are in different roots `'different'` flag is returned. diff --git a/src/model/range.js b/src/model/range.js index e8fcd56bf..353309c28 100644 --- a/src/model/range.js +++ b/src/model/range.js @@ -30,7 +30,7 @@ export default class Range { * @readonly * @member {module:engine/model/position~Position} */ - this.start = Position.createFromPosition( start ); + this.start = start; /** * End position. @@ -38,7 +38,7 @@ export default class Range { * @readonly * @member {module:engine/model/position~Position} */ - this.end = end ? Position.createFromPosition( end ) : Position.createFromPosition( start ); + this.end = end ? end : start; } /** @@ -280,7 +280,7 @@ export default class Range { const ranges = []; const diffAt = this.start.getCommonPath( this.end ).length; - const pos = Position.createFromPosition( this.start ); + let pos = this.start; let posParent = pos.parent; // Go up. @@ -291,8 +291,8 @@ export default class Range { ranges.push( new Range( pos, pos.getShiftedBy( howMany ) ) ); } - pos.path = pos.path.slice( 0, -1 ); - pos.offset++; + pos = pos.getMovedToParent().getShiftedBy( 1 ); + posParent = posParent.parent; } @@ -305,8 +305,7 @@ export default class Range { ranges.push( new Range( pos, pos.getShiftedBy( howMany ) ) ); } - pos.offset = offset; - pos.path.push( 0 ); + pos = pos.getShiftedTo( offset ).getMovedToChild(); } return ranges; @@ -755,9 +754,8 @@ export default class Range { */ static createCollapsedAt( itemOrPosition, offset ) { const start = Position.createAt( itemOrPosition, offset ); - const end = Position.createFromPosition( start ); - return new Range( start, end ); + return new Range( start, start ); } /** @@ -810,7 +808,7 @@ export default class Range { // Since ranges are sorted, start with the range with index that is closest to reference range index. for ( let i = refIndex - 1; i >= 0; i++ ) { if ( ranges[ i ].end.isEqual( result.start ) ) { - result.start = Position.createFromPosition( ranges[ i ].start ); + result.start = ranges[ i ].start; } else { // If ranges are not starting/ending at the same position there is no point in looking further. break; @@ -821,7 +819,7 @@ export default class Range { // Since ranges are sorted, start with the range with index that is closest to reference range index. for ( let i = refIndex + 1; i < ranges.length; i++ ) { if ( ranges[ i ].start.isEqual( result.end ) ) { - result.end = Position.createFromPosition( ranges[ i ].end ); + result.end = ranges[ i ].end; } else { // If ranges are not starting/ending at the same position there is no point in looking further. break; diff --git a/src/model/treewalker.js b/src/model/treewalker.js index 8a9ff919b..d5096d3dc 100644 --- a/src/model/treewalker.js +++ b/src/model/treewalker.js @@ -204,33 +204,35 @@ export default class TreeWalker { */ _next() { const previousPosition = this.position; - const position = Position.createFromPosition( this.position ); + + let position = this.position; const parent = this._visitedParent; // We are at the end of the root. - if ( parent.parent === null && position.offset === parent.maxOffset ) { + if ( parent.parent === null && this.position.offset === parent.maxOffset ) { return { done: true }; } // We reached the walker boundary. - if ( parent === this._boundaryEndParent && position.offset == this.boundaries.end.offset ) { + if ( parent === this._boundaryEndParent && this.position.offset == this.boundaries.end.offset ) { return { done: true }; } - const node = position.textNode ? position.textNode : position.nodeAfter; + const node = this.position.textNode ? this.position.textNode : this.position.nodeAfter; if ( node instanceof Element ) { if ( !this.shallow ) { // Manual operations on path internals for optimization purposes. Here and in the rest of the method. - position.path.push( 0 ); + position = position.getMovedToChild(); + this._visitedParent = node; } else { - position.offset++; + position = position.getShiftedBy( 1 ); } this.position = position; - return formatReturnValue( 'elementStart', node, previousPosition, position, 1 ); + return formatReturnValue( 'elementStart', node, previousPosition, this.position, 1 ); } else if ( node instanceof Text ) { let charactersCount; @@ -249,14 +251,15 @@ export default class TreeWalker { const offsetInTextNode = position.offset - node.startOffset; const item = new TextProxy( node, offsetInTextNode, charactersCount ); - position.offset += charactersCount; + position = position.getShiftedBy( charactersCount ); + this.position = position; - return formatReturnValue( 'text', item, previousPosition, position, charactersCount ); + return formatReturnValue( 'text', item, previousPosition, this.position, charactersCount ); } else { // `node` is not set, we reached the end of current `parent`. - position.path.pop(); - position.offset++; + position = position.getMovedToParent().getShiftedBy( 1 ); + this.position = position; this._visitedParent = parent.parent; @@ -278,27 +281,28 @@ export default class TreeWalker { */ _previous() { const previousPosition = this.position; - const position = Position.createFromPosition( this.position ); + let position = this.position; const parent = this._visitedParent; // We are at the beginning of the root. - if ( parent.parent === null && position.offset === 0 ) { + if ( parent.parent === null && this.position.offset === 0 ) { return { done: true }; } // We reached the walker boundary. - if ( parent == this._boundaryStartParent && position.offset == this.boundaries.start.offset ) { + if ( parent == this._boundaryStartParent && this.position.offset == this.boundaries.start.offset ) { return { done: true }; } // Get node just before current position - const node = position.textNode ? position.textNode : position.nodeBefore; + const node = this.position.textNode ? this.position.textNode : this.position.nodeBefore; if ( node instanceof Element ) { - position.offset--; + position = position.getShiftedBy( -1 ); if ( !this.shallow ) { - position.path.push( node.maxOffset ); + position = position.getMovedToChild( node.maxOffset ); + this.position = position; this._visitedParent = node; @@ -330,13 +334,15 @@ export default class TreeWalker { const offsetInTextNode = position.offset - node.startOffset; const item = new TextProxy( node, offsetInTextNode - charactersCount, charactersCount ); - position.offset -= charactersCount; + position = position.getShiftedBy( -charactersCount ); + this.position = position; return formatReturnValue( 'text', item, previousPosition, position, charactersCount ); } else { // `node` is not set, we reached the beginning of current `parent`. - position.path.pop(); + position = position.getMovedToParent(); + this.position = position; this._visitedParent = parent.parent; diff --git a/tests/model/delta/transform/_utils/utils.js b/tests/model/delta/transform/_utils/utils.js index aa7a56316..5ee8dcc49 100644 --- a/tests/model/delta/transform/_utils/utils.js +++ b/tests/model/delta/transform/_utils/utils.js @@ -68,12 +68,9 @@ export function getMarkerDelta( name, oldRange, newRange, version ) { export function getMergeDelta( position, howManyInPrev, howManyInNext, version ) { const delta = new MergeDelta(); - const sourcePosition = Position.createFromPosition( position ); - sourcePosition.path.push( 0 ); + const sourcePosition = position.getMovedToChild(); - const targetPosition = Position.createFromPosition( position ); - targetPosition.offset--; - targetPosition.path.push( howManyInPrev ); + const targetPosition = position.getShiftedBy( -1 ).getMovedToChild( howManyInPrev ); const move = new MoveOperation( sourcePosition, howManyInNext, targetPosition, version ); move.isSticky = true; @@ -129,12 +126,9 @@ export function getRenameDelta( position, oldName, newName, baseVersion ) { export function getSplitDelta( position, nodeCopy, howManyMove, version ) { const delta = new SplitDelta(); - const insertPosition = Position.createFromPosition( position ); - insertPosition.path = insertPosition.getParentPath(); - insertPosition.offset++; + const insertPosition = position.getMovedToParent().getShiftedBy( 1 ); - const targetPosition = Position.createFromPosition( insertPosition ); - targetPosition.path.push( 0 ); + const targetPosition = insertPosition.getMovedToChild(); delta.addOperation( new InsertOperation( insertPosition, [ nodeCopy ], version ) ); @@ -153,8 +147,8 @@ export function getWrapDelta( range, element, version ) { const insert = new InsertOperation( range.end, element, version ); - const targetPosition = Position.createFromPosition( range.end ); - targetPosition.path.push( 0 ); + const targetPosition = range.end.getMovedToChild(); + const move = new MoveOperation( range.start, range.end.offset - range.start.offset, targetPosition, version + 1 ); delta.addOperation( insert ); @@ -168,14 +162,12 @@ export function getWrapDelta( range, element, version ) { export function getUnwrapDelta( positionBefore, howManyChildren, version ) { const delta = new UnwrapDelta(); - const sourcePosition = Position.createFromPosition( positionBefore ); - sourcePosition.path.push( 0 ); + const sourcePosition = positionBefore.getMovedToChild(); const move = new MoveOperation( sourcePosition, howManyChildren, positionBefore, version ); move.isSticky = true; - const removePosition = Position.createFromPosition( positionBefore ); - removePosition.offset += howManyChildren; + const removePosition = positionBefore.getShiftedBy( howManyChildren ); const gy = sourcePosition.root.document.graveyard; const gyPos = Position.createAt( gy, 0 ); diff --git a/tests/model/delta/transform/movedelta.js b/tests/model/delta/transform/movedelta.js index b5134f77e..bdfa82edc 100644 --- a/tests/model/delta/transform/movedelta.js +++ b/tests/model/delta/transform/movedelta.js @@ -135,8 +135,8 @@ describe( 'transform', () => { } ); it( 'move range in merged node #2', () => { - moveDelta._moveOperation.sourcePosition.path = [ 3, 3, 1 ]; - moveDelta._moveOperation.targetPosition.path = [ 3, 3, 4 ]; + moveDelta._moveOperation.sourcePosition = new Position( root, [ 3, 3, 1 ] ); + moveDelta._moveOperation.targetPosition = new Position( root, [ 3, 3, 4 ] ); const mergePosition = new Position( root, [ 3, 3 ] ); const mergeDelta = getMergeDelta( mergePosition, 1, 4, baseVersion ); diff --git a/tests/model/delta/transform/splitdelta.js b/tests/model/delta/transform/splitdelta.js index 0b3cfc0fd..6c0ddfc3c 100644 --- a/tests/model/delta/transform/splitdelta.js +++ b/tests/model/delta/transform/splitdelta.js @@ -6,6 +6,7 @@ import transformations from '../../../../src/model/delta/basic-transformations'; // eslint-disable-line no-unused-vars import deltaTransform from '../../../../src/model/delta/transform'; + const transform = deltaTransform.transform; import Element from '../../../../src/model/element'; @@ -967,10 +968,8 @@ describe( 'transform', () => { baseVersion = removeDelta.operations.length; const newInsertPosition = removeOperation.targetPosition.getShiftedBy( 2 ); - const newMoveSourcePosition = removeOperation.targetPosition.getShiftedBy( 1 ); - newMoveSourcePosition.path.push( 2 ); - const newMoveTargetPosition = Position.createAt( newInsertPosition ); - newMoveTargetPosition.path.push( 0 ); + const newMoveSourcePosition = removeOperation.targetPosition.getShiftedBy( 1 ).getMovedToChild( 2 ); + const newMoveTargetPosition = newInsertPosition.getMovedToChild( ); expectDelta( transformed[ 0 ], { type: SplitDelta, @@ -1003,10 +1002,8 @@ describe( 'transform', () => { baseVersion = removeDelta.operations.length; const newInsertPosition = removeOperation.targetPosition.getShiftedBy( 2 ); - const newMoveSourcePosition = removeOperation.targetPosition.getShiftedBy( 1 ); - newMoveSourcePosition.path.push( 3 ); - const newMoveTargetPosition = Position.createAt( newInsertPosition ); - newMoveTargetPosition.path.push( 0 ); + const newMoveSourcePosition = removeOperation.targetPosition.getShiftedBy( 1 ).getMovedToChild( 3 ); + const newMoveTargetPosition = newInsertPosition.getMovedToChild( ); expectDelta( transformed[ 0 ], { type: SplitDelta, diff --git a/tests/model/liverange.js b/tests/model/liverange.js index 15fac530b..6a57795e7 100644 --- a/tests/model/liverange.js +++ b/tests/model/liverange.js @@ -208,7 +208,7 @@ describe( 'LiveRange', () => { } ); it( 'is at the live range start position and live range is collapsed', () => { - live.end.path = [ 0, 1, 4 ]; + live.end = new Position( live.end.root, [ 0, 1, 4 ] ); const insertRange = new Range( new Position( root, [ 0, 1, 4 ] ), new Position( root, [ 0, 1, 8 ] ) ); @@ -372,7 +372,7 @@ describe( 'LiveRange', () => { } ); it( 'is equal to live range', () => { - live.end.path = [ 0, 1, 7 ]; + live.end = new Position( live.end.root, [ 0, 1, 7 ] ); const moveSource = new Position( root, [ 0, 1, 4 ] ); const moveRange = new Range( new Position( root, [ 0, 3, 0 ] ), new Position( root, [ 0, 3, 3 ] ) ); @@ -389,7 +389,7 @@ describe( 'LiveRange', () => { } ); it( 'contains live range', () => { - live.end.path = [ 0, 1, 7 ]; + live.end = new Position( live.end.root, [ 0, 1, 7 ] ); const moveSource = new Position( root, [ 0, 1, 3 ] ); const moveRange = new Range( new Position( root, [ 0, 3, 0 ] ), new Position( root, [ 0, 3, 9 ] ) ); @@ -406,7 +406,7 @@ describe( 'LiveRange', () => { } ); it( 'is intersecting with live range and points to live range', () => { - live.end.path = [ 0, 1, 12 ]; + live.end = new Position( live.end.root, [ 0, 1, 12 ] ); const moveSource = new Position( root, [ 0, 1, 2 ] ); const moveRange = new Range( new Position( root, [ 0, 1, 7 ] ), new Position( root, [ 0, 1, 10 ] ) ); @@ -656,7 +656,7 @@ describe( 'LiveRange', () => { } ); it( 'from the range to the range', () => { - live.end.path = [ 0, 1, 12 ]; + live.end = new Position( live.end.root, [ 0, 1, 12 ] ); const moveSource = new Position( root, [ 0, 1, 6 ] ); const moveRange = new Range( new Position( root, [ 0, 1, 8 ] ), new Position( root, [ 0, 1, 10 ] ) ); diff --git a/tests/model/operation/transform.js b/tests/model/operation/transform.js index a53a1671e..955894582 100644 --- a/tests/model/operation/transform.js +++ b/tests/model/operation/transform.js @@ -38,8 +38,7 @@ describe( 'transform', () => { if ( params.hasOwnProperty( i ) ) { if ( i == 'type' ) { expect( op, 'type' ).to.be.instanceof( params[ i ] ); - } - else if ( params[ i ] instanceof Array ) { + } else if ( params[ i ] instanceof Array ) { expect( op[ i ].length, i ).to.equal( params[ i ].length ); for ( let j = 0; j < params[ i ].length; j++ ) { @@ -54,6 +53,14 @@ describe( 'transform', () => { } } + function getPositionMovedInPath( position, index, howMany ) { + const path = position.path.slice(); + + path[ index ] += howMany; + + return position.getMovedToPath( path ); + } + describe( 'InsertOperation', () => { let nodeC, nodeD, position; @@ -94,7 +101,7 @@ describe( 'transform', () => { ); const transOp = transform( op, transformBy ); - expected.position.offset += 2; + expected.position = expected.position.getShiftedBy( 2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -108,7 +115,7 @@ describe( 'transform', () => { ); const transOp = transform( op, transformBy ); - expected.position.offset += 2; + expected.position = expected.position.getShiftedBy( 2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -148,7 +155,7 @@ describe( 'transform', () => { ); const transOp = transform( op, transformBy ); - expected.position.offset += 2; + expected.position = expected.position.getShiftedBy( 2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -175,7 +182,7 @@ describe( 'transform', () => { ); const transOp = transform( op, transformBy ); - expected.position.path[ 1 ] += 2; + expected.position = getPositionMovedInPath( expected.position, 1, 2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -253,7 +260,7 @@ describe( 'transform', () => { ); const transOp = transform( op, transformBy ); - expected.position.offset--; + expected.position = expected.position.getShiftedBy( -1 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -282,7 +289,7 @@ describe( 'transform', () => { ); const transOp = transform( op, transformBy ); - expected.position.offset += 2; + expected.position = expected.position.getShiftedBy( 2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -311,7 +318,7 @@ describe( 'transform', () => { ); const transOp = transform( op, transformBy ); - expected.position.offset += 2; + expected.position = expected.position.getShiftedBy( 2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -340,7 +347,7 @@ describe( 'transform', () => { ); const transOp = transform( op, transformBy, { isStrong: true, insertBefore: true } ); - expected.position.offset += 2; + expected.position = expected.position.getShiftedBy( 2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -369,7 +376,7 @@ describe( 'transform', () => { ); const transOp = transform( op, transformBy ); - expected.position.path[ 1 ] -= 1; + expected.position = getPositionMovedInPath( expected.position, 1, -1 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -398,7 +405,7 @@ describe( 'transform', () => { ); const transOp = transform( op, transformBy ); - expected.position.path[ 1 ] += 2; + expected.position = getPositionMovedInPath( expected.position, 1, 2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -427,7 +434,7 @@ describe( 'transform', () => { ); const transOp = transform( op, transformBy ); - expected.position.path = [ 1, 2, 2 ]; + expected.position = new Position( expected.position.root, [ 1, 2, 2 ] ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -442,7 +449,7 @@ describe( 'transform', () => { ); const transOp = transform( op, transformBy ); - expected.position.path = [ 1, 2 ]; + expected.position = new Position( expected.position.root, [ 1, 2 ] ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -532,7 +539,7 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start.offset += 2; + expected.range.start = expected.range.start.getShiftedBy( 2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -547,7 +554,7 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start.offset += 2; + expected.range.start = expected.range.start.getShiftedBy( 2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -575,8 +582,8 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start.path[ 0 ] += 2; - expected.range.end.path[ 0 ] += 2; + expected.range.start = getPositionMovedInPath( expected.range.start, 0, 2 ); + expected.range.end = getPositionMovedInPath( expected.range.end, 0, 2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -606,12 +613,12 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 2 ); - expected.range.start.path = [ 1, 3, 3 ]; + expected.range.start = new Position( expected.range.start.root, [ 1, 3, 3 ] ); expectOperation( transOp[ 0 ], expected ); expected.range.start = op.range.start; - expected.range.end.path = [ 1, 3, 1 ]; + expected.range.end = new Position( expected.range.end.root, [ 1, 3, 1 ] ); expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); @@ -723,11 +730,11 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 2 ); - expected.range.end.path = [ 1, 4, 2 ]; + expected.range.end = new Position( expected.range.end.root, [ 1, 4, 2 ] ); expectOperation( transOp[ 0 ], expected ); - expected.range.start.path = [ 1, 4, 2 ]; + expected.range.start = new Position( expected.range.start.root, [ 1, 4, 2 ] ); expected.range.end = op.range.end; expected.oldValue = 'another'; expected.baseVersion++; @@ -749,12 +756,12 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 2 ); - expected.range.start.path = [ 2, 1 ]; + expected.range.start = new Position( expected.range.start.root, [ 2, 1 ] ); expectOperation( transOp[ 0 ], expected ); expected.range.start = op.range.start; - expected.range.end.path = [ 2, 1 ]; + expected.range.end = new Position( expected.range.end.root, [ 2, 1 ] ); expected.oldValue = null; expected.baseVersion++; @@ -774,18 +781,18 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 3 ); - expected.range.end.path = [ 1, 4, 1 ]; + expected.range.end = new Position( expected.range.end.root, [ 1, 4, 1 ] ); expectOperation( transOp[ 0 ], expected ); - expected.range.start.path = [ 2, 1 ]; + expected.range.start = new Position( expected.range.start.root, [ 2, 1 ] ); expected.range.end = op.range.end; expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); - expected.range.start.path = [ 1, 4, 1 ]; - expected.range.end.path = [ 2, 1 ]; + expected.range.start = new Position( expected.range.start.root, [ 1, 4, 1 ] ); + expected.range.end = new Position( expected.range.end.root, [ 2, 1 ] ); expected.oldValue = null; expected.baseVersion++; @@ -860,7 +867,7 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.end.path = [ 1, 4, 2 ]; + expected.range.end = new Position( expected.range.end.root, [ 1, 4, 2 ] ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -878,7 +885,7 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start.path = [ 2, 1 ]; + expected.range.start = new Position( expected.range.start.root, [ 2, 1 ] ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -897,11 +904,11 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 2 ); - expected.range.end.path = [ 1, 4, 1 ]; + expected.range.end = new Position( expected.range.end.root, [ 1, 4, 1 ] ); expectOperation( transOp[ 0 ], expected ); - expected.range.start.path = [ 2, 1 ]; + expected.range.start = new Position( expected.range.start.root, [ 2, 1 ] ); expected.range.end = op.range.end; expected.baseVersion++; @@ -952,7 +959,7 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start.offset -= 2; + expected.range.start = expected.range.start.getShiftedBy( -2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -968,7 +975,7 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start.offset += 2; + expected.range.start = expected.range.start.getShiftedBy( 2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -984,8 +991,8 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start.path[ 0 ]--; - expected.range.end.path[ 0 ]--; + expected.range.start = getPositionMovedInPath( expected.range.start, 0, -1 ); + expected.range.end = getPositionMovedInPath( expected.range.end, 0, -1 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1015,7 +1022,7 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start.path[ 1 ] += 2; + expected.range.start = getPositionMovedInPath( expected.range.start, 1, 2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1047,12 +1054,12 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 2 ); - expected.range.end.path = [ 2, 1 ]; + expected.range.end = new Position( expected.range.end.root, [ 2, 1 ] ); expectOperation( transOp[ 0 ], expected ); - expected.range.start.path = [ 4 ]; - expected.range.end.path = [ 5, 4 ]; + expected.range.start = new Position( expected.range.start.root, [ 4 ] ); + expected.range.end = new Position( expected.range.end.root, [ 5, 4 ] ); expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); @@ -1070,12 +1077,12 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 2 ); - expected.range.start.path = [ 1, 1 ]; + expected.range.start = new Position( expected.range.start.root, [ 1, 1 ] ); expectOperation( transOp[ 0 ], expected ); - expected.range.start.path = [ 0, 1 ]; - expected.range.end.path = [ 0, 3 ]; + expected.range.start = new Position( expected.range.start.root, [ 0, 1 ] ); + expected.range.end = new Position( expected.range.end.root, [ 0, 3 ] ); expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); @@ -1091,8 +1098,8 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start.path = [ 1, 4, 1, 2 ]; - expected.range.end.path = [ 1, 4, 2, 2, 4 ]; + expected.range.start = new Position( expected.range.start.root, [ 1, 4, 1, 2 ] ); + expected.range.end = new Position( expected.range.end.root, [ 1, 4, 2, 2, 4 ] ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1112,8 +1119,8 @@ describe( 'transform', () => { expectOperation( transOp[ 0 ], expected ); - expected.range.start.path = [ 3, 2 ]; - expected.range.end.path = [ 3, 4 ]; + expected.range.start = new Position( expected.range.start.root, [ 3, 2 ] ); + expected.range.end = new Position( expected.range.end.root, [ 3, 4 ] ); expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); @@ -1131,12 +1138,12 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 2 ); - expected.range.start.path = [ 1, 6 ]; + expected.range.start = new Position( expected.range.start.root, [ 1, 6 ] ); expectOperation( transOp[ 0 ], expected ); expected.range.start = op.range.start; - expected.range.end.path = [ 1, 4 ]; + expected.range.end = new Position( expected.range.end.root, [ 1, 4 ] ); expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); @@ -1154,19 +1161,19 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 3 ); - expected.range.start.path = [ 5 ]; - expected.range.end.path = [ 5, 2, 4 ]; + expected.range.start = new Position( expected.range.start.root, [ 5 ] ); + expected.range.end = new Position( expected.range.end.root, [ 5, 2, 4 ] ); expectOperation( transOp[ 0 ], expected ); - expected.range.start.path = [ 1, 1 ]; - expected.range.end.path = [ 2 ]; + expected.range.start = new Position( expected.range.start.root, [ 1, 1 ] ); + expected.range.end = new Position( expected.range.end.root, [ 2 ] ); expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); - expected.range.start.path = [ 3 ]; - expected.range.end.path = [ 5 ]; + expected.range.start = new Position( expected.range.start.root, [ 3 ] ); + expected.range.end = new Position( expected.range.end.root, [ 5 ] ); expected.baseVersion++; expectOperation( transOp[ 2 ], expected ); @@ -1234,8 +1241,8 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start.offset += 2; - expected.range.end.offset += 2; + expected.range.start = expected.range.start.getShiftedBy( 2 ); + expected.range.end = expected.range.end.getShiftedBy( 2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1250,8 +1257,8 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start.offset += 2; - expected.range.end.offset += 2; + expected.range.start = expected.range.start.getShiftedBy( 2 ); + expected.range.end = expected.range.end.getShiftedBy( 2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1269,8 +1276,8 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start.offset--; - expected.range.end.offset--; + expected.range.start = expected.range.start.getShiftedBy( -1 ); + expected.range.end = expected.range.end.getShiftedBy( -1 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1286,8 +1293,8 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start.offset += 2; - expected.range.end.offset += 2; + expected.range.start = expected.range.start.getShiftedBy( 2 ); + expected.range.end = expected.range.end.getShiftedBy( 2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1305,12 +1312,12 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 2 ); - expected.range.end.offset -= 2; + expected.range.end = expected.range.end.getShiftedBy( -2 ); expectOperation( transOp[ 0 ], expected ); - expected.range.start.path = [ 2, 4, 1 ]; - expected.range.end.path = [ 2, 4, 3 ]; + expected.range.start = new Position( expected.range.start.root, [ 2, 4, 1 ] ); + expected.range.end = new Position( expected.range.end.root, [ 2, 4, 3 ] ); expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); @@ -1328,13 +1335,13 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 2 ); - expected.range.start.offset -= 1; - expected.range.end.offset -= 2; + expected.range.start = expected.range.start.getShiftedBy( -1 ); + expected.range.end = expected.range.end.getShiftedBy( -2 ); expectOperation( transOp[ 0 ], expected ); - expected.range.start.path = [ 2, 4, 2 ]; - expected.range.end.path = [ 2, 4, 3 ]; + expected.range.start = new Position( expected.range.start.root, [ 2, 4, 2 ] ); + expected.range.end = new Position( expected.range.end.root, [ 2, 4, 3 ] ); expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); @@ -1350,8 +1357,8 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start.path = [ 2, 4, 2, 1 ]; - expected.range.end.path = [ 2, 4, 2, 4 ]; + expected.range.start = new Position( expected.range.start.root, [ 2, 4, 2, 1 ] ); + expected.range.end = new Position( expected.range.end.root, [ 2, 4, 2, 4 ] ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1369,12 +1376,12 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 2 ); - expected.range.end.offset--; + expected.range.end = expected.range.end.getShiftedBy( -1 ); expectOperation( transOp[ 0 ], expected ); - expected.range.start.path = [ 2, 4, 1 ]; - expected.range.end.path = [ 2, 4, 2 ]; + expected.range.start = new Position( expected.range.start.root, [ 2, 4, 1 ] ); + expected.range.end = new Position( expected.range.end.root, [ 2, 4, 2 ] ); expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); @@ -1390,8 +1397,8 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start.path = [ 2, 4, 1 ]; - expected.range.end.path = [ 2, 4, 4 ]; + expected.range.start = new Position( expected.range.start.root, [ 2, 4, 1 ] ); + expected.range.end = new Position( expected.range.end.root, [ 2, 4, 4 ] ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1409,13 +1416,13 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 2 ); - expected.range.start.offset = 4; - expected.range.end.offset = 6; + expected.range.start = expected.range.start.getShiftedTo( 4 ); + expected.range.end = expected.range.end.getShiftedTo( 6 ); expectOperation( transOp[ 0 ], expected ); - expected.range.start.offset = op.range.start.offset; - expected.range.end.offset = 2; + expected.range.start = expected.range.start.getShiftedTo( op.range.start.offset ); + expected.range.end = expected.range.end.getShiftedTo( 2 ); expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); @@ -1433,19 +1440,19 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 3 ); - expected.range.start.offset = 3; - expected.range.end.offset = 4; + expected.range.start = expected.range.start.getShiftedTo( 3 ); + expected.range.end = expected.range.end.getShiftedTo( 4 ); expectOperation( transOp[ 0 ], expected ); - expected.range.start.offset = 0; - expected.range.end.offset = 1; + expected.range.start = expected.range.start.getShiftedTo( 0 ); + expected.range.end = expected.range.end.getShiftedTo( 1 ); expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); - expected.range.start.offset = 2; - expected.range.end.offset = 3; + expected.range.start = expected.range.start.getShiftedTo( 2 ); + expected.range.end = expected.range.end.getShiftedTo( 3 ); expected.baseVersion++; expectOperation( transOp[ 2 ], expected ); @@ -1473,14 +1480,14 @@ describe( 'transform', () => { baseVersion ); - transformBy.targetPosition.path = [ 0 ]; + transformBy.targetPosition = new Position( transformBy.targetPosition.root, [ 0 ] ); const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); - expected.range.start.path = [ 4, 0 ]; - expected.range.end.path = [ 4, 4 ]; + expected.range.start = new Position( expected.range.start.root, [ 4, 0 ] ); + expected.range.end = new Position( expected.range.end.root, [ 4, 4 ] ); expectOperation( transOp[ 0 ], expected ); } ); @@ -1493,7 +1500,7 @@ describe( 'transform', () => { baseVersion ); - transformBy.targetPosition.path = [ 4 ]; + transformBy.targetPosition = new Position( transformBy.targetPosition.root, [ 4 ] ); const transOp = transform( op, transformBy ); @@ -1696,8 +1703,7 @@ describe( 'transform', () => { targetPosition = new Position( root, [ 3, 3, 3 ] ); howMany = 2; - rangeEnd = Position.createFromPosition( sourcePosition ); - rangeEnd.offset += howMany; + rangeEnd = sourcePosition.getShiftedBy( howMany ); op = new MoveOperation( sourcePosition, howMany, targetPosition, baseVersion ); @@ -1746,7 +1752,7 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.sourcePosition.offset += 2; + expected.sourcePosition = expected.sourcePosition.getShiftedBy( 2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1774,7 +1780,7 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.sourcePosition.path[ 1 ] += 2; + expected.sourcePosition = getPositionMovedInPath( expected.sourcePosition, 1, 2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1802,7 +1808,7 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.targetPosition.offset += 2; + expected.targetPosition = expected.targetPosition.getShiftedBy( 2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1830,7 +1836,7 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.targetPosition.path[ 1 ] += 2; + expected.targetPosition = getPositionMovedInPath( expected.targetPosition, 1, 2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1858,7 +1864,7 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.targetPosition.offset += 2; + expected.targetPosition = expected.targetPosition.getShiftedBy( 2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1886,7 +1892,7 @@ describe( 'transform', () => { const transOp = transform( op, transformBy, { isStrong: true, insertBefore: true } ); - expected.targetPosition.offset += 2; + expected.targetPosition = expected.targetPosition.getShiftedBy( 2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2011,7 +2017,7 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.sourcePosition.offset += 2; + expected.sourcePosition = expected.sourcePosition.getShiftedBy( 2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2041,7 +2047,7 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.sourcePosition.offset -= 2; + expected.sourcePosition = expected.sourcePosition.getShiftedBy( -2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2071,7 +2077,7 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.sourcePosition.path[ 1 ] += 2; + expected.sourcePosition = getPositionMovedInPath( expected.sourcePosition, 1, 2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2101,7 +2107,7 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.sourcePosition.path[ 1 ] -= 1; + expected.sourcePosition = getPositionMovedInPath( expected.sourcePosition, 1, -1 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2131,7 +2137,7 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.targetPosition.offset += 2; + expected.targetPosition = expected.targetPosition.getShiftedBy( 2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2161,7 +2167,7 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.targetPosition.offset -= 2; + expected.targetPosition = expected.targetPosition.getShiftedBy( -2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2191,7 +2197,7 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.targetPosition.path[ 1 ] += 2; + expected.targetPosition = getPositionMovedInPath( expected.targetPosition, 1, 2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2221,7 +2227,7 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.targetPosition.path[ 1 ] -= 2; + expected.targetPosition = getPositionMovedInPath( expected.targetPosition, 1, -2 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2256,7 +2262,7 @@ describe( 'transform', () => { expected.howMany = 1; expectOperation( transOp[ 0 ], expected ); - expected.sourcePosition.path = [ 2, 2, 6 ]; + expected.sourcePosition = new Position( expected.sourcePosition.root, [ 2, 2, 6 ] ); expected.targetPosition = targetPosition.getShiftedBy( 1 ); expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); @@ -2274,7 +2280,7 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 1 ); - expected.sourcePosition.offset = 6; + expected.sourcePosition = expected.sourcePosition.getShiftedTo( 6 ); expectOperation( transOp[ 0 ], expected ); } ); @@ -2355,7 +2361,7 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.sourcePosition.path = [ 4, 3, 4 ]; + expected.sourcePosition = new Position( expected.sourcePosition.root, [ 4, 3, 4 ] ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2371,7 +2377,7 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.targetPosition.path = [ 0, 2, 3 ]; + expected.targetPosition = new Position( expected.targetPosition.root, [ 0, 2, 3 ] ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2423,7 +2429,7 @@ describe( 'transform', () => { const transOp = transform( op, transformBy, { isStrong: true } ); - expected.sourcePosition.path = [ 4, 1, 0 ]; + expected.sourcePosition = new Position( expected.sourcePosition.root, [ 4, 1, 0 ] ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2456,14 +2462,14 @@ describe( 'transform', () => { const transOp = transform( op, transformBy, { isStrong: true } ); - expected.sourcePosition.path = [ 4, 1, 1 ]; + expected.sourcePosition = new Position( expected.sourcePosition.root, [ 4, 1, 1 ] ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); } ); it( 'range contains transforming range and target and is important: update range path and target', () => { - op.targetPosition.path = [ 2, 2, 7 ]; + op.targetPosition = new Position( op.targetPosition.root, [ 2, 2, 7 ] ); const transformBy = new MoveOperation( new Position( root, [ 2, 2, 3 ] ), @@ -2476,14 +2482,14 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 1 ); - expected.sourcePosition.path = [ 4, 1, 1 ]; - expected.targetPosition.path = [ 4, 1, 4 ]; + expected.sourcePosition = new Position( expected.sourcePosition.root, [ 4, 1, 1 ] ); + expected.targetPosition = new Position( expected.targetPosition.root, [ 4, 1, 4 ] ); expectOperation( transOp[ 0 ], expected ); } ); it( 'range contains transforming range and target and is less important: update range path and target', () => { - op.targetPosition.path = [ 2, 2, 7 ]; + op.targetPosition = new Position( op.targetPosition.root, [ 2, 2, 7 ] ); const transformBy = new MoveOperation( new Position( root, [ 2, 2, 3 ] ), @@ -2496,8 +2502,8 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 1 ); - expected.sourcePosition.path = [ 4, 1, 1 ]; - expected.targetPosition.path = [ 4, 1, 4 ]; + expected.sourcePosition = new Position( expected.sourcePosition.root, [ 4, 1, 1 ] ); + expected.targetPosition = new Position( expected.targetPosition.root, [ 4, 1, 4 ] ); expectOperation( transOp[ 0 ], expected ); } ); @@ -2512,7 +2518,7 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.sourcePosition.path = [ 2, 2, 3 ]; + expected.sourcePosition = new Position( expected.sourcePosition.root, [ 2, 2, 3 ] ); expected.howMany = 1; expect( transOp.length ).to.equal( 1 ); @@ -2600,12 +2606,12 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 2 ); - expected.sourcePosition.path = [ 2, 2, 3 ]; + expected.sourcePosition = new Position( expected.sourcePosition.root, [ 2, 2, 3 ] ); expected.howMany = 1; expectOperation( transOp[ 0 ], expected ); - expected.sourcePosition.path = [ 2, 2, 5 ]; + expected.sourcePosition = new Position( expected.sourcePosition.root, [ 2, 2, 5 ] ); expected.howMany = 2; expected.targetPosition = targetPosition.getShiftedBy( 1 ); expected.baseVersion++; @@ -2627,12 +2633,12 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 2 ); - expected.sourcePosition.path = [ 2, 2, 3 ]; + expected.sourcePosition = new Position( expected.sourcePosition.root, [ 2, 2, 3 ] ); expected.howMany = 1; expectOperation( transOp[ 0 ], expected ); - expected.sourcePosition.path = [ 2, 2, 5 ]; + expected.sourcePosition = new Position( expected.sourcePosition.root, [ 2, 2, 5 ] ); expected.howMany = 2; expected.targetPosition = targetPosition.getShiftedBy( 1 ); expected.baseVersion++; @@ -2681,7 +2687,7 @@ describe( 'transform', () => { } ); it( 'range intersects, target inside transforming range and is important: split into two operations', () => { - op.targetPosition.path = [ 2, 2, 7 ]; + op.targetPosition = new Position( op.targetPosition.root, [ 2, 2, 7 ] ); const transformBy = new MoveOperation( new Position( root, [ 2, 2, 5 ] ), @@ -2694,21 +2700,21 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 2 ); - expected.sourcePosition.path = [ 2, 2, 4 ]; - expected.targetPosition.path = [ 4, 1, 2 ]; + expected.sourcePosition = new Position( expected.sourcePosition.root, [ 2, 2, 4 ] ); + expected.targetPosition = new Position( expected.targetPosition.root, [ 4, 1, 2 ] ); expected.howMany = 1; expectOperation( transOp[ 0 ], expected ); - expected.sourcePosition.path = [ 4, 1, 0 ]; - expected.targetPosition.path = [ 4, 1, 3 ]; + expected.sourcePosition = new Position( expected.sourcePosition.root, [ 4, 1, 0 ] ); + expected.targetPosition = new Position( expected.targetPosition.root, [ 4, 1, 3 ] ); expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); } ); it( 'range intersects, target inside transforming range and is less important: shrink range', () => { - op.targetPosition.path = [ 2, 2, 7 ]; + op.targetPosition = new Position( op.targetPosition.root, [ 2, 2, 7 ] ); const transformBy = new MoveOperation( new Position( root, [ 2, 2, 5 ] ), @@ -2721,8 +2727,8 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 1 ); - expected.sourcePosition.path = [ 2, 2, 4 ]; - expected.targetPosition.path = [ 4, 1, 2 ]; + expected.sourcePosition = new Position( expected.sourcePosition.root, [ 2, 2, 4 ] ); + expected.targetPosition = new Position( expected.targetPosition.root, [ 4, 1, 2 ] ); expected.howMany = 1; expectOperation( transOp[ 0 ], expected ); @@ -2742,7 +2748,7 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 2 ); - expected.sourcePosition.path = [ 2, 2, 4 ]; + expected.sourcePosition = new Position( expected.sourcePosition.root, [ 2, 2, 4 ] ); expected.howMany = 1; expectOperation( transOp[ 0 ], expected ); @@ -2767,19 +2773,19 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 3 ); - expected.sourcePosition.path = [ 2, 2, 4 ]; + expected.sourcePosition = new Position( expected.sourcePosition.root, [ 2, 2, 4 ] ); expected.howMany = 1; expectOperation( transOp[ 0 ], expected ); - expected.sourcePosition.path = [ 4, 1, 0 ]; + expected.sourcePosition = new Position( expected.sourcePosition.root, [ 4, 1, 0 ] ); expected.targetPosition = targetPosition.getShiftedBy( 1 ); expected.howMany = 2; expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); - expected.sourcePosition.path = [ 2, 2, 4 ]; + expected.sourcePosition = new Position( expected.sourcePosition.root, [ 2, 2, 4 ] ); expected.targetPosition = targetPosition.getShiftedBy( 3 ); expected.howMany = 1; expected.baseVersion++; @@ -3060,7 +3066,7 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 1 ); - expected.position.offset = 4; + expected.position = expected.position.getShiftedTo( 4 ); expectOperation( transOp[ 0 ], expected ); } ); @@ -3089,7 +3095,7 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 1 ); - expected.position.path = [ 0, 4, 2 ]; + expected.position = new Position( expected.position.root, [ 0, 4, 2 ] ); expectOperation( transOp[ 0 ], expected ); } ); @@ -3219,7 +3225,7 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 1 ); - expected.position.offset = 0; + expected.position = expected.position.getShiftedTo( 0 ); expectOperation( transOp[ 0 ], expected ); } ); @@ -3236,7 +3242,7 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 1 ); - expected.position.path = [ 0, 0, 2 ]; + expected.position = new Position( expected.position.root, [ 0, 0, 2 ] ); expectOperation( transOp[ 0 ], expected ); } ); @@ -3253,7 +3259,7 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 1 ); - expected.position.path = [ 2, 6 ]; + expected.position = new Position( expected.position.root, [ 2, 6 ] ); expectOperation( transOp[ 0 ], expected ); } ); @@ -3270,7 +3276,7 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 1 ); - expected.position.path = [ 2, 6, 2 ]; + expected.position = new Position( expected.position.root, [ 2, 6, 2 ] ); expectOperation( transOp[ 0 ], expected ); } ); @@ -3287,7 +3293,7 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 1 ); - expected.position.offset = 4; + expected.position = expected.position.getShiftedTo( 4 ); expectOperation( transOp[ 0 ], expected ); } ); @@ -3304,7 +3310,7 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 1 ); - expected.position.offset = 0; + expected.position = expected.position.getShiftedTo( 0 ); expectOperation( transOp[ 0 ], expected ); } ); @@ -3336,8 +3342,8 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); expected.newRange = null; - expected.oldRange.start.offset = 3; - expected.oldRange.end.offset = 6; + expected.oldRange.start = expected.oldRange.start.getShiftedTo( 3 ); + expected.oldRange.end = expected.oldRange.end.getShiftedTo( 6 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -3351,8 +3357,8 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); expected.oldRange = null; - expected.newRange.start.offset = 12; - expected.newRange.end.offset = 14; + expected.newRange.start = expected.newRange.start.getShiftedTo( 12 ); + expected.newRange.end = expected.newRange.end.getShiftedTo( 14 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -3388,8 +3394,8 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); expected.newRange = null; - expected.oldRange.start.offset = 0; - expected.oldRange.end.offset = 3; + expected.oldRange.start = expected.oldRange.start.getShiftedTo( 0 ); + expected.oldRange.end = expected.oldRange.end.getShiftedTo( 3 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -3399,10 +3405,10 @@ describe( 'transform', () => { const transformBy = new MoveOperation( Position.createAt( root, 2 ), 2, Position.createAt( root, 20 ), baseVersion ); const transOp = transform( op, transformBy ); - expected.oldRange.start.offset = 1; - expected.oldRange.end.offset = 2; - expected.newRange.start.offset = 8; - expected.newRange.end.offset = 10; + expected.oldRange.start = expected.oldRange.start.getShiftedTo( 1 ); + expected.oldRange.end = expected.oldRange.end.getShiftedTo( 2 ); + expected.newRange.start = expected.newRange.start.getShiftedTo( 8 ); + expected.newRange.end = expected.newRange.end.getShiftedTo( 10 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -3416,8 +3422,8 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); expected.oldRange = null; - expected.newRange.start.offset = 10; - expected.newRange.end.offset = 14; + expected.newRange.start = expected.newRange.start.getShiftedTo( 10 ); + expected.newRange.end = expected.newRange.end.getShiftedTo( 14 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -3427,10 +3433,10 @@ describe( 'transform', () => { const transformBy = new MoveOperation( Position.createAt( root, 20 ), 4, Position.createAt( root, 2 ), baseVersion ); const transOp = transform( op, transformBy ); - expected.oldRange.start.offset = 1; - expected.oldRange.end.offset = 8; - expected.newRange.start.offset = 14; - expected.newRange.end.offset = 16; + expected.oldRange.start = expected.oldRange.start.getShiftedTo( 1 ); + expected.oldRange.end = expected.oldRange.end.getShiftedTo( 8 ); + expected.newRange.start = expected.newRange.start.getShiftedTo( 14 ); + expected.newRange.end = expected.newRange.end.getShiftedTo( 16 ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); diff --git a/tests/model/position.js b/tests/model/position.js index 1b5a23eb5..0424b3e8f 100644 --- a/tests/model/position.js +++ b/tests/model/position.js @@ -291,14 +291,6 @@ describe( 'Position', () => { expect( new Position( root, [ 1, 0, 3 ] ) ).to.have.property( 'index' ).that.equals( 1 ); } ); - it( 'should be able to set offset', () => { - const position = new Position( root, [ 1, 0, 2 ] ); - position.offset = 4; - - expect( position.offset ).to.equal( 4 ); - expect( position.path ).to.deep.equal( [ 1, 0, 4 ] ); - } ); - it( 'should have nodeBefore if it is not inside a text node', () => { expect( new Position( root, [ 0 ] ).nodeBefore ).to.be.null; expect( new Position( root, [ 1 ] ).nodeBefore ).to.equal( p ); @@ -605,14 +597,6 @@ describe( 'Position', () => { } ); describe( '_getTransformedByInsertion()', () => { - it( 'should return a new Position instance', () => { - const position = new Position( root, [ 0 ] ); - const transformed = position._getTransformedByInsertion( new Position( root, [ 2 ] ), 4, false ); - - expect( transformed ).not.to.equal( position ); - expect( transformed ).to.be.instanceof( Position ); - } ); - it( 'should increment offset if insertion is in the same parent and closer offset', () => { const position = new Position( root, [ 1, 2, 3 ] ); const transformed = position._getTransformedByInsertion( new Position( root, [ 1, 2, 2 ] ), 2, false ); @@ -665,14 +649,6 @@ describe( 'Position', () => { } ); describe( '_getTransformedByDeletion()', () => { - it( 'should return a new Position instance', () => { - const position = new Position( root, [ 0 ] ); - const transformed = position._getTransformedByDeletion( new Position( root, [ 2 ] ), 4 ); - - expect( transformed ).not.to.equal( position ); - expect( transformed ).to.be.instanceof( Position ); - } ); - it( 'should return null if original position is inside one of removed nodes', () => { const position = new Position( root, [ 1, 2 ] ); const transformed = position._getTransformedByDeletion( new Position( root, [ 0 ] ), 2 ); diff --git a/tests/model/range.js b/tests/model/range.js index adf1bb9c2..44b5dd741 100644 --- a/tests/model/range.js +++ b/tests/model/range.js @@ -855,7 +855,8 @@ describe( 'Range', () => { } ); it( 'move inside the range', () => { - range.end.offset = 6; + range.end = range.end.getShiftedTo( 6 ); + const start = new Position( root, [ 3 ] ); const target = new Position( root, [ 5 ] ); const delta = getMoveDelta( start, 1, target, 1 ); From b67f106ea42912353e29f886ffc574183f9b0e45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Mon, 23 Oct 2017 18:34:11 +0200 Subject: [PATCH 02/16] Other: Make Range immutable. --- src/conversion/viewconversiondispatcher.js | 5 +- src/dev-utils/view.js | 2 +- src/model/liverange.js | 6 +- src/model/operation/transform.js | 61 ++-- src/model/range.js | 89 ++++-- tests/model/liverange.js | 20 +- tests/model/operation/transform.js | 327 +++++++++++++++------ tests/model/range.js | 74 +++-- 8 files changed, 404 insertions(+), 180 deletions(-) diff --git a/src/conversion/viewconversiondispatcher.js b/src/conversion/viewconversiondispatcher.js index 4a9100cac..0e6fc576f 100644 --- a/src/conversion/viewconversiondispatcher.js +++ b/src/conversion/viewconversiondispatcher.js @@ -303,10 +303,11 @@ function extractMarkersFromModelFragment( modelItem ) { // When marker of given name is not stored it means that we have found the beginning of the range. if ( !markers.has( markerName ) ) { - markers.set( markerName, new ModelRange( ModelPosition.createFromPosition( currentPosition ) ) ); + markers.set( markerName, new ModelRange( currentPosition ) ); // Otherwise is means that we have found end of the marker range. } else { - markers.get( markerName ).end = ModelPosition.createFromPosition( currentPosition ); + const oldMarker = markers.get( markerName ); + markers.set( markerName, new ModelRange( oldMarker.start, currentPosition ) ); } // Remove marker element from DocumentFragment. diff --git a/src/dev-utils/view.js b/src/dev-utils/view.js index fc6cacc3f..18603c371 100644 --- a/src/dev-utils/view.js +++ b/src/dev-utils/view.js @@ -550,7 +550,7 @@ class RangeParser { if ( item.bracket == ELEMENT_RANGE_START_TOKEN || item.bracket == TEXT_RANGE_START_TOKEN ) { range = new Range( item.position, item.position ); } else { - range.end = item.position; + range = new Range( range.start, item.position ); ranges.push( range ); range = null; } diff --git a/src/model/liverange.js b/src/model/liverange.js index 32b7ad8de..b2c714072 100644 --- a/src/model/liverange.js +++ b/src/model/liverange.js @@ -7,7 +7,7 @@ * @module engine/model/liverange */ -import Range from './range'; +import Range, { setStart, setEnd } from './range'; import EmitterMixin from '@ckeditor/ckeditor5-utils/src/emittermixin'; import mix from '@ckeditor/ckeditor5-utils/src/mix'; @@ -178,8 +178,8 @@ function transform( changeType, deltaType, batch, targetRange, sourcePosition ) // If range boundaries have changed, fire `change:range` event. const oldRange = Range.createFromRange( this ); - this.start = updated.start; - this.end = updated.end; + setStart( this, updated.start ); + setEnd( this, updated.end ); this.fire( 'change:range', oldRange, { type: changeType, diff --git a/src/model/operation/transform.js b/src/model/operation/transform.js index 13cc541ef..1cbe1c7ee 100644 --- a/src/model/operation/transform.js +++ b/src/model/operation/transform.js @@ -179,25 +179,29 @@ const ot = { // Take the start and the end of the range and transform them by deletion of moved nodes. // Note that if rangeB was inside AttributeOperation range, only difference.end will be transformed. // This nicely covers the joining simplification we did in the previous step. - difference.start = difference.start._getTransformedByDeletion( b.sourcePosition, b.howMany ); - difference.end = difference.end._getTransformedByDeletion( b.sourcePosition, b.howMany ); + const range = new Range( + difference.start._getTransformedByDeletion( b.sourcePosition, b.howMany ), + difference.end._getTransformedByDeletion( b.sourcePosition, b.howMany ) + ); // MoveOperation pastes nodes into target position. We acknowledge this by proper transformation. // Note that since we operate on transformed difference range, we should transform by // previously transformed target position. // Note that we do not use Position._getTransformedByMove on range boundaries because we need to // transform by insertion a range as a whole, since newTargetPosition might be inside that range. - ranges = difference._getTransformedByInsertion( b.getMovedRangeStart(), b.howMany, true, false ).reverse(); + ranges = range._getTransformedByInsertion( b.getMovedRangeStart(), b.howMany, true, false ).reverse(); } if ( common !== null ) { // Here we do not need to worry that newTargetPosition is inside moved range, because that // would mean that the MoveOperation targets into itself, and that is incorrect operation. // Instead, we calculate the new position of that part of original range. - common.start = common.start._getCombined( b.sourcePosition, b.getMovedRangeStart() ); - common.end = common.end._getCombined( b.sourcePosition, b.getMovedRangeStart() ); + const range = new Range( + common.start._getCombined( b.sourcePosition, b.getMovedRangeStart() ), + common.end._getCombined( b.sourcePosition, b.getMovedRangeStart() ) + ); - ranges.push( common ); + ranges.push( range ); } // Map transformed range(s) to operations and return them. @@ -376,7 +380,7 @@ const ot = { // Setting and evaluating some variables that will be used in special cases and default algorithm. // // Create ranges from `MoveOperations` properties. - const rangeA = Range.createFromPositionAndShift( a.sourcePosition, a.howMany ); + let rangeA = Range.createFromPositionAndShift( a.sourcePosition, a.howMany ); const rangeB = Range.createFromPositionAndShift( b.sourcePosition, b.howMany ); // Assign `context.isStrong` to a different variable, because the value may change during execution of @@ -428,8 +432,10 @@ const ot = { if ( bTargetsToA && rangeA.containsRange( rangeB, true ) ) { // There is a mini-special case here, where `rangeB` is on other level than `rangeA`. That's why // we need to transform `a` operation anyway. - rangeA.start = rangeA.start._getTransformedByMove( b.sourcePosition, b.targetPosition, b.howMany, !includeB ); - rangeA.end = rangeA.end._getTransformedByMove( b.sourcePosition, b.targetPosition, b.howMany, includeB ); + rangeA = new Range( + rangeA.start._getTransformedByMove( b.sourcePosition, b.targetPosition, b.howMany, !includeB ), + rangeA.end._getTransformedByMove( b.sourcePosition, b.targetPosition, b.howMany, includeB ) + ); return makeMoveOperationsFromRanges( [ rangeA ], newTargetPosition, a ); } @@ -444,8 +450,10 @@ const ot = { if ( aTargetsToB && rangeB.containsRange( rangeA, true ) ) { // `a` operation is "moved together" with `b` operation. // Here, just move `rangeA` "inside" `rangeB`. - rangeA.start = rangeA.start._getCombined( b.sourcePosition, b.getMovedRangeStart() ); - rangeA.end = rangeA.end._getCombined( b.sourcePosition, b.getMovedRangeStart() ); + rangeA = new Range( + rangeA.start._getCombined( b.sourcePosition, b.getMovedRangeStart() ), + rangeA.end._getCombined( b.sourcePosition, b.getMovedRangeStart() ) + ); return makeMoveOperationsFromRanges( [ rangeA ], newTargetPosition, a ); } @@ -466,8 +474,10 @@ const ot = { // Transform `rangeA` by `b` operation and make operation out of it, and that's all. // Note that this is a simplified version of default case, but here we treat the common part (whole `rangeA`) // like a one difference part. - rangeA.start = rangeA.start._getTransformedByMove( b.sourcePosition, b.targetPosition, b.howMany, !includeB ); - rangeA.end = rangeA.end._getTransformedByMove( b.sourcePosition, b.targetPosition, b.howMany, includeB ); + rangeA = new Range( + rangeA.start._getTransformedByMove( b.sourcePosition, b.targetPosition, b.howMany, !includeB ), + rangeA.end._getTransformedByMove( b.sourcePosition, b.targetPosition, b.howMany, includeB ) + ); return makeMoveOperationsFromRanges( [ rangeA ], newTargetPosition, a ); } @@ -500,10 +510,12 @@ const ot = { // This is an array with one or two ranges. Two ranges if `rangeB` is inside `rangeA`. const difference = rangeA.getDifference( rangeB ); - for ( const range of difference ) { + for ( const rangeInDiff of difference ) { // Transform those ranges by `b` operation. For example if `b` moved range from before those ranges, fix those ranges. - range.start = range.start._getTransformedByDeletion( b.sourcePosition, b.howMany ); - range.end = range.end._getTransformedByDeletion( b.sourcePosition, b.howMany ); + const range = new Range( + rangeInDiff.start._getTransformedByDeletion( b.sourcePosition, b.howMany ), + rangeInDiff.end._getTransformedByDeletion( b.sourcePosition, b.howMany ) + ); // If `b` operation targets into `rangeA` on the same level, spread `rangeA` into two ranges. const shouldSpread = compareArrays( range.start.getParentPath(), b.getMovedRangeStart().getParentPath() ) == 'same'; @@ -513,12 +525,14 @@ const ot = { } // Then, we have to manage the "common part" of both move ranges. - const common = rangeA.getIntersection( rangeB ); + const intersectionRange = rangeA.getIntersection( rangeB ); - if ( common !== null && isStrong && !bTargetsToA ) { + if ( intersectionRange !== null && isStrong && !bTargetsToA ) { // Calculate the new position of that part of original range. - common.start = common.start._getCombined( b.sourcePosition, b.getMovedRangeStart() ); - common.end = common.end._getCombined( b.sourcePosition, b.getMovedRangeStart() ); + const common = new Range( + intersectionRange.start._getCombined( b.sourcePosition, b.getMovedRangeStart() ), + intersectionRange.end._getCombined( b.sourcePosition, b.getMovedRangeStart() ) + ); // Take care of proper range order. // @@ -627,9 +641,10 @@ function joinRanges( ranges ) { } else if ( ranges.length == 1 ) { return ranges[ 0 ]; } else { - ranges[ 0 ].end = ranges[ ranges.length - 1 ].end; - - return ranges[ 0 ]; + return new Range( + ranges[ 0 ].start, + ranges[ ranges.length - 1 ].end + ); } } diff --git a/src/model/range.js b/src/model/range.js index 353309c28..b16056094 100644 --- a/src/model/range.js +++ b/src/model/range.js @@ -11,6 +11,9 @@ import Position from './position'; import TreeWalker from './treewalker'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; +const _start = Symbol( 'start' ); +const _end = Symbol( 'end' ); + /** * Range class. Range is iterable. */ @@ -24,21 +27,8 @@ export default class Range { * @param {module:engine/model/position~Position} [end] End position. If not set, range will be collapsed at `start` position. */ constructor( start, end = null ) { - /** - * Start position. - * - * @readonly - * @member {module:engine/model/position~Position} - */ - this.start = start; - - /** - * End position. - * - * @readonly - * @member {module:engine/model/position~Position} - */ - this.end = end ? end : start; + setStart( this, start ); + setEnd( this, end ? end : start ); } /** @@ -57,6 +47,26 @@ export default class Range { yield* new TreeWalker( { boundaries: this, ignoreElementEnd: true } ); } + /** + * Start position. + * + * @readonly + * @member {module:engine/model/position~Position} + */ + get start() { + return this[ _start ]; + } + + /** + * End position. + * + * @readonly + * @member {module:engine/model/position~Position} + */ + get end() { + return this[ _end ]; + } + /** * Returns whether the range is collapsed, that is if {@link #start} and * {@link #end} positions are equal. @@ -465,6 +475,18 @@ export default class Range { return this.start.getCommonAncestor( this.end ); } + /** + * Converts `Range` to plain object and returns it. + * + * @returns {Object} `Range` converted to plain object. + */ + toJSON() { + return { + start: this.start.toJSON(), + end: this.end.toJSON() + }; + } + /** * Returns a range that is a result of transforming this range by a change in the model document. * @@ -612,15 +634,13 @@ export default class Range { ) ]; } else { - const range = Range.createFromRange( this ); - const insertBeforeStart = !isSticky; - const insertBeforeEnd = range.isCollapsed ? true : isSticky; + const insertBeforeEnd = this.isCollapsed ? true : isSticky; - range.start = range.start._getTransformedByInsertion( insertPosition, howMany, insertBeforeStart ); - range.end = range.end._getTransformedByInsertion( insertPosition, howMany, insertBeforeEnd ); + const start = this.start._getTransformedByInsertion( insertPosition, howMany, insertBeforeStart ); + const end = this.end._getTransformedByInsertion( insertPosition, howMany, insertBeforeEnd ); - return [ range ]; + return [ new Range( start, end ) ]; } } @@ -802,13 +822,13 @@ export default class Range { // 4. At this moment we don't need the original range. // We are going to modify the result and we need to return a new instance of Range. // We have to create a copy of the reference range. - const result = new this( ref.start, ref.end ); + let result = new this( ref.start, ref.end ); // 5. Ranges should be checked and glued starting from the range that is closest to the reference range. // Since ranges are sorted, start with the range with index that is closest to reference range index. for ( let i = refIndex - 1; i >= 0; i++ ) { if ( ranges[ i ].end.isEqual( result.start ) ) { - result.start = ranges[ i ].start; + result = new this( ranges[ i ].start, result.end ); } else { // If ranges are not starting/ending at the same position there is no point in looking further. break; @@ -819,7 +839,7 @@ export default class Range { // Since ranges are sorted, start with the range with index that is closest to reference range index. for ( let i = refIndex + 1; i < ranges.length; i++ ) { if ( ranges[ i ].start.isEqual( result.end ) ) { - result.end = ranges[ i ].end; + result = new this( result.start, ranges[ i ].end ); } else { // If ranges are not starting/ending at the same position there is no point in looking further. break; @@ -840,3 +860,24 @@ export default class Range { return new this( Position.fromJSON( json.start, doc ), Position.fromJSON( json.end, doc ) ); } } + +/** + * Method used to expose start setter to child classes. + * @protected + * @param {module:engine/model/range~Range} range Range of which start position should be sent. + * @param {module:engine/model/position~Position} position Position to set as range start. + * See {@link module:engine/model/range~Range#start}. + */ +export function setStart( range, position ) { + range[ _start ] = position; +} + +/** + * Method used to expose end setter to child classes. + * @protected + * @param {module:engine/model/range~Range} range Range of which end position should be sent. + * @param {module:engine/model/position~Position} position Position to set as range end. See {@link module:engine/model/range~Range#end}. + */ +export function setEnd( range, position ) { + range[ _end ] = position; +} diff --git a/tests/model/liverange.js b/tests/model/liverange.js index 6a57795e7..1780c368d 100644 --- a/tests/model/liverange.js +++ b/tests/model/liverange.js @@ -208,7 +208,9 @@ describe( 'LiveRange', () => { } ); it( 'is at the live range start position and live range is collapsed', () => { - live.end = new Position( live.end.root, [ 0, 1, 4 ] ); + live = new LiveRange( live.start, new Position( live.end.root, [ 0, 1, 4 ] ) ); + spy = sinon.spy(); + live.on( 'change:range', spy ); const insertRange = new Range( new Position( root, [ 0, 1, 4 ] ), new Position( root, [ 0, 1, 8 ] ) ); @@ -372,7 +374,9 @@ describe( 'LiveRange', () => { } ); it( 'is equal to live range', () => { - live.end = new Position( live.end.root, [ 0, 1, 7 ] ); + live = new LiveRange( live.start, new Position( live.end.root, [ 0, 1, 7 ] ) ); + spy = sinon.spy(); + live.on( 'change:range', spy ); const moveSource = new Position( root, [ 0, 1, 4 ] ); const moveRange = new Range( new Position( root, [ 0, 3, 0 ] ), new Position( root, [ 0, 3, 3 ] ) ); @@ -389,7 +393,9 @@ describe( 'LiveRange', () => { } ); it( 'contains live range', () => { - live.end = new Position( live.end.root, [ 0, 1, 7 ] ); + live = new LiveRange( live.start, new Position( live.end.root, [ 0, 1, 7 ] ) ); + spy = sinon.spy(); + live.on( 'change:range', spy ); const moveSource = new Position( root, [ 0, 1, 3 ] ); const moveRange = new Range( new Position( root, [ 0, 3, 0 ] ), new Position( root, [ 0, 3, 9 ] ) ); @@ -406,7 +412,9 @@ describe( 'LiveRange', () => { } ); it( 'is intersecting with live range and points to live range', () => { - live.end = new Position( live.end.root, [ 0, 1, 12 ] ); + live = new LiveRange( live.start, new Position( live.end.root, [ 0, 1, 12 ] ) ); + spy = sinon.spy(); + live.on( 'change:range', spy ); const moveSource = new Position( root, [ 0, 1, 2 ] ); const moveRange = new Range( new Position( root, [ 0, 1, 7 ] ), new Position( root, [ 0, 1, 10 ] ) ); @@ -656,7 +664,9 @@ describe( 'LiveRange', () => { } ); it( 'from the range to the range', () => { - live.end = new Position( live.end.root, [ 0, 1, 12 ] ); + live = new LiveRange( live.start, new Position( live.end.root, [ 0, 1, 12 ] ) ); + spy = sinon.spy(); + live.on( 'change:content', spy ); const moveSource = new Position( root, [ 0, 1, 6 ] ); const moveRange = new Range( new Position( root, [ 0, 1, 8 ] ), new Position( root, [ 0, 1, 10 ] ) ); diff --git a/tests/model/operation/transform.js b/tests/model/operation/transform.js index 955894582..644d66e86 100644 --- a/tests/model/operation/transform.js +++ b/tests/model/operation/transform.js @@ -539,7 +539,10 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start = expected.range.start.getShiftedBy( 2 ); + expected.range = new Range( + expected.range.start.getShiftedBy( 2 ), + expected.range.end + ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -554,7 +557,10 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start = expected.range.start.getShiftedBy( 2 ); + expected.range = new Range( + expected.range.start.getShiftedBy( 2 ), + expected.range.end + ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -582,8 +588,10 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start = getPositionMovedInPath( expected.range.start, 0, 2 ); - expected.range.end = getPositionMovedInPath( expected.range.end, 0, 2 ); + expected.range = new Range( + getPositionMovedInPath( expected.range.start, 0, 2 ), + getPositionMovedInPath( expected.range.end, 0, 2 ) + ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -613,12 +621,17 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 2 ); - expected.range.start = new Position( expected.range.start.root, [ 1, 3, 3 ] ); + expected.range = new Range( + new Position( expected.range.start.root, [ 1, 3, 3 ] ), + expected.range.end + ); expectOperation( transOp[ 0 ], expected ); - expected.range.start = op.range.start; - expected.range.end = new Position( expected.range.end.root, [ 1, 3, 1 ] ); + expected.range = new Range( + op.range.start, + new Position( expected.range.end.root, [ 1, 3, 1 ] ) + ); expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); @@ -730,12 +743,18 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 2 ); - expected.range.end = new Position( expected.range.end.root, [ 1, 4, 2 ] ); + expected.range = new Range( + expected.range.start, + new Position( expected.range.end.root, [ 1, 4, 2 ] ) + ); expectOperation( transOp[ 0 ], expected ); - expected.range.start = new Position( expected.range.start.root, [ 1, 4, 2 ] ); - expected.range.end = op.range.end; + expected.range = new Range( + new Position( expected.range.start.root, [ 1, 4, 2 ] ), + op.range.end + ); + expected.oldValue = 'another'; expected.baseVersion++; @@ -756,12 +775,17 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 2 ); - expected.range.start = new Position( expected.range.start.root, [ 2, 1 ] ); + expected.range = new Range( + new Position( expected.range.start.root, [ 2, 1 ] ), + expected.range.end + ); expectOperation( transOp[ 0 ], expected ); - expected.range.start = op.range.start; - expected.range.end = new Position( expected.range.end.root, [ 2, 1 ] ); + expected.range = new Range( + op.range.start, + new Position( expected.range.end.root, [ 2, 1 ] ) + ); expected.oldValue = null; expected.baseVersion++; @@ -781,18 +805,26 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 3 ); - expected.range.end = new Position( expected.range.end.root, [ 1, 4, 1 ] ); + expected.range = new Range( + expected.range.start, + new Position( expected.range.end.root, [ 1, 4, 1 ] ) + ); expectOperation( transOp[ 0 ], expected ); - expected.range.start = new Position( expected.range.start.root, [ 2, 1 ] ); - expected.range.end = op.range.end; + expected.range = new Range( + new Position( expected.range.start.root, [ 2, 1 ] ), + op.range.end + ); expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); - expected.range.start = new Position( expected.range.start.root, [ 1, 4, 1 ] ); - expected.range.end = new Position( expected.range.end.root, [ 2, 1 ] ); + expected.range = new Range( + new Position( expected.range.start.root, [ 1, 4, 1 ] ), + new Position( expected.range.end.root, [ 2, 1 ] ) + ); + expected.oldValue = null; expected.baseVersion++; @@ -867,7 +899,10 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.end = new Position( expected.range.end.root, [ 1, 4, 2 ] ); + expected.range = new Range( + expected.range.start, + new Position( expected.range.end.root, [ 1, 4, 2 ] ) + ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -885,7 +920,10 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start = new Position( expected.range.start.root, [ 2, 1 ] ); + expected.range = new Range( + new Position( expected.range.start.root, [ 2, 1 ] ), + expected.range.end + ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -904,12 +942,17 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 2 ); - expected.range.end = new Position( expected.range.end.root, [ 1, 4, 1 ] ); + expected.range = new Range( + expected.range.start, + new Position( expected.range.end.root, [ 1, 4, 1 ] ) + ); expectOperation( transOp[ 0 ], expected ); - expected.range.start = new Position( expected.range.start.root, [ 2, 1 ] ); - expected.range.end = op.range.end; + expected.range = new Range( + new Position( expected.range.start.root, [ 2, 1 ] ), + op.range.end + ); expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); @@ -959,7 +1002,10 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start = expected.range.start.getShiftedBy( -2 ); + expected.range = new Range( + expected.range.start.getShiftedBy( -2 ), + expected.range.end + ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -975,7 +1021,10 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start = expected.range.start.getShiftedBy( 2 ); + expected.range = new Range( + expected.range.start.getShiftedBy( 2 ), + expected.range.end + ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -991,8 +1040,10 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start = getPositionMovedInPath( expected.range.start, 0, -1 ); - expected.range.end = getPositionMovedInPath( expected.range.end, 0, -1 ); + expected.range = new Range( + getPositionMovedInPath( expected.range.start, 0, -1 ), + getPositionMovedInPath( expected.range.end, 0, -1 ) + ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1022,7 +1073,10 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start = getPositionMovedInPath( expected.range.start, 1, 2 ); + expected.range = new Range( + getPositionMovedInPath( expected.range.start, 1, 2 ), + expected.range.end + ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1054,12 +1108,17 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 2 ); - expected.range.end = new Position( expected.range.end.root, [ 2, 1 ] ); + expected.range = new Range( + expected.range.start, + new Position( expected.range.end.root, [ 2, 1 ] ) + ); expectOperation( transOp[ 0 ], expected ); - expected.range.start = new Position( expected.range.start.root, [ 4 ] ); - expected.range.end = new Position( expected.range.end.root, [ 5, 4 ] ); + expected.range = new Range( + new Position( expected.range.start.root, [ 4 ] ), + new Position( expected.range.end.root, [ 5, 4 ] ) + ); expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); @@ -1077,12 +1136,17 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 2 ); - expected.range.start = new Position( expected.range.start.root, [ 1, 1 ] ); + expected.range = new Range( + new Position( expected.range.start.root, [ 1, 1 ] ), + expected.range.end + ); expectOperation( transOp[ 0 ], expected ); - expected.range.start = new Position( expected.range.start.root, [ 0, 1 ] ); - expected.range.end = new Position( expected.range.end.root, [ 0, 3 ] ); + expected.range = new Range( + new Position( expected.range.start.root, [ 0, 1 ] ), + new Position( expected.range.end.root, [ 0, 3 ] ) + ); expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); @@ -1098,8 +1162,10 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start = new Position( expected.range.start.root, [ 1, 4, 1, 2 ] ); - expected.range.end = new Position( expected.range.end.root, [ 1, 4, 2, 2, 4 ] ); + expected.range = new Range( + new Position( expected.range.start.root, [ 1, 4, 1, 2 ] ), + new Position( expected.range.end.root, [ 1, 4, 2, 2, 4 ] ) + ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1119,8 +1185,10 @@ describe( 'transform', () => { expectOperation( transOp[ 0 ], expected ); - expected.range.start = new Position( expected.range.start.root, [ 3, 2 ] ); - expected.range.end = new Position( expected.range.end.root, [ 3, 4 ] ); + expected.range = new Range( + new Position( expected.range.start.root, [ 3, 2 ] ), + new Position( expected.range.end.root, [ 3, 4 ] ) + ); expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); @@ -1138,12 +1206,17 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 2 ); - expected.range.start = new Position( expected.range.start.root, [ 1, 6 ] ); + expected.range = new Range( + new Position( expected.range.start.root, [ 1, 6 ] ), + expected.range.end + ); expectOperation( transOp[ 0 ], expected ); - expected.range.start = op.range.start; - expected.range.end = new Position( expected.range.end.root, [ 1, 4 ] ); + expected.range = new Range( + op.range.start, + new Position( expected.range.end.root, [ 1, 4 ] ) + ); expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); @@ -1161,19 +1234,25 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 3 ); - expected.range.start = new Position( expected.range.start.root, [ 5 ] ); - expected.range.end = new Position( expected.range.end.root, [ 5, 2, 4 ] ); + expected.range = new Range( + new Position( expected.range.start.root, [ 5 ] ), + new Position( expected.range.end.root, [ 5, 2, 4 ] ) + ); expectOperation( transOp[ 0 ], expected ); - expected.range.start = new Position( expected.range.start.root, [ 1, 1 ] ); - expected.range.end = new Position( expected.range.end.root, [ 2 ] ); + expected.range = new Range( + new Position( expected.range.start.root, [ 1, 1 ] ), + new Position( expected.range.end.root, [ 2 ] ) + ); expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); - expected.range.start = new Position( expected.range.start.root, [ 3 ] ); - expected.range.end = new Position( expected.range.end.root, [ 5 ] ); + expected.range = new Range( + new Position( expected.range.start.root, [ 3 ] ), + new Position( expected.range.end.root, [ 5 ] ) + ); expected.baseVersion++; expectOperation( transOp[ 2 ], expected ); @@ -1241,8 +1320,10 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start = expected.range.start.getShiftedBy( 2 ); - expected.range.end = expected.range.end.getShiftedBy( 2 ); + expected.range = new Range( + expected.range.start.getShiftedBy( 2 ), + expected.range.end.getShiftedBy( 2 ) + ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1257,8 +1338,10 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start = expected.range.start.getShiftedBy( 2 ); - expected.range.end = expected.range.end.getShiftedBy( 2 ); + expected.range = new Range( + expected.range.start.getShiftedBy( 2 ), + expected.range.end.getShiftedBy( 2 ) + ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1276,8 +1359,10 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start = expected.range.start.getShiftedBy( -1 ); - expected.range.end = expected.range.end.getShiftedBy( -1 ); + expected.range = new Range( + expected.range.start.getShiftedBy( -1 ), + expected.range.end.getShiftedBy( -1 ) + ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1293,8 +1378,10 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start = expected.range.start.getShiftedBy( 2 ); - expected.range.end = expected.range.end.getShiftedBy( 2 ); + expected.range = new Range( + expected.range.start.getShiftedBy( 2 ), + expected.range.end.getShiftedBy( 2 ) + ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1312,12 +1399,17 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 2 ); - expected.range.end = expected.range.end.getShiftedBy( -2 ); + expected.range = new Range( + expected.range.start, + expected.range.end.getShiftedBy( -2 ) + ); expectOperation( transOp[ 0 ], expected ); - expected.range.start = new Position( expected.range.start.root, [ 2, 4, 1 ] ); - expected.range.end = new Position( expected.range.end.root, [ 2, 4, 3 ] ); + expected.range = new Range( + new Position( expected.range.start.root, [ 2, 4, 1 ] ), + new Position( expected.range.end.root, [ 2, 4, 3 ] ) + ); expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); @@ -1335,13 +1427,17 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 2 ); - expected.range.start = expected.range.start.getShiftedBy( -1 ); - expected.range.end = expected.range.end.getShiftedBy( -2 ); + expected.range = new Range( + expected.range.start.getShiftedBy( -1 ), + expected.range.end.getShiftedBy( -2 ) + ); expectOperation( transOp[ 0 ], expected ); - expected.range.start = new Position( expected.range.start.root, [ 2, 4, 2 ] ); - expected.range.end = new Position( expected.range.end.root, [ 2, 4, 3 ] ); + expected.range = new Range( + new Position( expected.range.start.root, [ 2, 4, 2 ] ), + new Position( expected.range.end.root, [ 2, 4, 3 ] ) + ); expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); @@ -1357,8 +1453,10 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start = new Position( expected.range.start.root, [ 2, 4, 2, 1 ] ); - expected.range.end = new Position( expected.range.end.root, [ 2, 4, 2, 4 ] ); + expected.range = new Range( + new Position( expected.range.start.root, [ 2, 4, 2, 1 ] ), + new Position( expected.range.end.root, [ 2, 4, 2, 4 ] ) + ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1376,12 +1474,17 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 2 ); - expected.range.end = expected.range.end.getShiftedBy( -1 ); + expected.range = new Range( + expected.range.start, + expected.range.end.getShiftedBy( -1 ) + ); expectOperation( transOp[ 0 ], expected ); - expected.range.start = new Position( expected.range.start.root, [ 2, 4, 1 ] ); - expected.range.end = new Position( expected.range.end.root, [ 2, 4, 2 ] ); + expected.range = new Range( + new Position( expected.range.start.root, [ 2, 4, 1 ] ), + new Position( expected.range.end.root, [ 2, 4, 2 ] ) + ); expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); @@ -1397,8 +1500,10 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); - expected.range.start = new Position( expected.range.start.root, [ 2, 4, 1 ] ); - expected.range.end = new Position( expected.range.end.root, [ 2, 4, 4 ] ); + expected.range = new Range( + new Position( expected.range.start.root, [ 2, 4, 1 ] ), + new Position( expected.range.end.root, [ 2, 4, 4 ] ) + ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1416,13 +1521,17 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 2 ); - expected.range.start = expected.range.start.getShiftedTo( 4 ); - expected.range.end = expected.range.end.getShiftedTo( 6 ); + expected.range = new Range( + expected.range.start.getShiftedTo( 4 ), + expected.range.end.getShiftedTo( 6 ) + ); expectOperation( transOp[ 0 ], expected ); - expected.range.start = expected.range.start.getShiftedTo( op.range.start.offset ); - expected.range.end = expected.range.end.getShiftedTo( 2 ); + expected.range = new Range( + expected.range.start.getShiftedTo( op.range.start.offset ), + expected.range.end.getShiftedTo( 2 ) + ); expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); @@ -1440,19 +1549,25 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 3 ); - expected.range.start = expected.range.start.getShiftedTo( 3 ); - expected.range.end = expected.range.end.getShiftedTo( 4 ); + expected.range = new Range( + expected.range.start.getShiftedTo( 3 ), + expected.range.end.getShiftedTo( 4 ) + ); expectOperation( transOp[ 0 ], expected ); - expected.range.start = expected.range.start.getShiftedTo( 0 ); - expected.range.end = expected.range.end.getShiftedTo( 1 ); + expected.range = new Range( + expected.range.start.getShiftedTo( 0 ), + expected.range.end.getShiftedTo( 1 ) + ); expected.baseVersion++; expectOperation( transOp[ 1 ], expected ); - expected.range.start = expected.range.start.getShiftedTo( 2 ); - expected.range.end = expected.range.end.getShiftedTo( 3 ); + expected.range = new Range( + expected.range.start.getShiftedTo( 2 ), + expected.range.end.getShiftedTo( 3 ) + ); expected.baseVersion++; expectOperation( transOp[ 2 ], expected ); @@ -1486,8 +1601,10 @@ describe( 'transform', () => { expect( transOp.length ).to.equal( 1 ); - expected.range.start = new Position( expected.range.start.root, [ 4, 0 ] ); - expected.range.end = new Position( expected.range.end.root, [ 4, 4 ] ); + expected.range = new Range( + new Position( expected.range.start.root, [ 4, 0 ] ), + new Position( expected.range.end.root, [ 4, 4 ] ) + ); expectOperation( transOp[ 0 ], expected ); } ); @@ -3342,8 +3459,10 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); expected.newRange = null; - expected.oldRange.start = expected.oldRange.start.getShiftedTo( 3 ); - expected.oldRange.end = expected.oldRange.end.getShiftedTo( 6 ); + expected.oldRange = new Range( + expected.oldRange.start.getShiftedTo( 3 ), + expected.oldRange.end.getShiftedTo( 6 ) + ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -3357,8 +3476,10 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); expected.oldRange = null; - expected.newRange.start = expected.newRange.start.getShiftedTo( 12 ); - expected.newRange.end = expected.newRange.end.getShiftedTo( 14 ); + expected.newRange = new Range( + expected.newRange.start.getShiftedTo( 12 ), + expected.newRange.end.getShiftedTo( 14 ) + ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -3394,8 +3515,10 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); expected.newRange = null; - expected.oldRange.start = expected.oldRange.start.getShiftedTo( 0 ); - expected.oldRange.end = expected.oldRange.end.getShiftedTo( 3 ); + expected.oldRange = new Range( + expected.oldRange.start.getShiftedTo( 0 ), + expected.oldRange.end.getShiftedTo( 3 ) + ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -3405,10 +3528,14 @@ describe( 'transform', () => { const transformBy = new MoveOperation( Position.createAt( root, 2 ), 2, Position.createAt( root, 20 ), baseVersion ); const transOp = transform( op, transformBy ); - expected.oldRange.start = expected.oldRange.start.getShiftedTo( 1 ); - expected.oldRange.end = expected.oldRange.end.getShiftedTo( 2 ); - expected.newRange.start = expected.newRange.start.getShiftedTo( 8 ); - expected.newRange.end = expected.newRange.end.getShiftedTo( 10 ); + expected.oldRange = new Range( + expected.oldRange.start.getShiftedTo( 1 ), + expected.oldRange.end.getShiftedTo( 2 ) + ); + expected.newRange = new Range( + expected.newRange.start.getShiftedTo( 8 ), + expected.newRange.end.getShiftedTo( 10 ) + ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -3422,8 +3549,10 @@ describe( 'transform', () => { const transOp = transform( op, transformBy ); expected.oldRange = null; - expected.newRange.start = expected.newRange.start.getShiftedTo( 10 ); - expected.newRange.end = expected.newRange.end.getShiftedTo( 14 ); + expected.newRange = new Range( + expected.newRange.start.getShiftedTo( 10 ), + expected.newRange.end.getShiftedTo( 14 ) + ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -3433,10 +3562,14 @@ describe( 'transform', () => { const transformBy = new MoveOperation( Position.createAt( root, 20 ), 4, Position.createAt( root, 2 ), baseVersion ); const transOp = transform( op, transformBy ); - expected.oldRange.start = expected.oldRange.start.getShiftedTo( 1 ); - expected.oldRange.end = expected.oldRange.end.getShiftedTo( 8 ); - expected.newRange.start = expected.newRange.start.getShiftedTo( 14 ); - expected.newRange.end = expected.newRange.end.getShiftedTo( 16 ); + expected.oldRange = new Range( + expected.oldRange.start.getShiftedTo( 1 ), + expected.oldRange.end.getShiftedTo( 8 ) + ); + expected.newRange = new Range( + expected.newRange.start.getShiftedTo( 14 ), + expected.newRange.end.getShiftedTo( 16 ) + ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); diff --git a/tests/model/range.js b/tests/model/range.js index 44b5dd741..d32d86aca 100644 --- a/tests/model/range.js +++ b/tests/model/range.js @@ -855,7 +855,7 @@ describe( 'Range', () => { } ); it( 'move inside the range', () => { - range.end = range.end.getShiftedTo( 6 ); + range = new Range( range.start, range.end.getShiftedTo( 6 ) ); const start = new Position( root, [ 3 ] ); const target = new Position( root, [ 5 ] ); @@ -978,8 +978,10 @@ describe( 'Range', () => { describe( 'by SplitDelta', () => { it( 'split inside range', () => { - range.start = new Position( root, [ 0, 2 ] ); - range.end = new Position( root, [ 0, 4 ] ); + range = new Range( + new Position( root, [ 0, 2 ] ), + new Position( root, [ 0, 4 ] ) + ); const delta = getSplitDelta( new Position( root, [ 0, 3 ] ), new Element( 'p' ), 3, 1 ); @@ -991,8 +993,10 @@ describe( 'Range', () => { } ); it( 'split at the beginning of multi-element range', () => { - range.start = new Position( root, [ 0, 4 ] ); - range.end = new Position( root, [ 1, 2 ] ); + range = new Range( + new Position( root, [ 0, 4 ] ), + new Position( root, [ 1, 2 ] ) + ); const delta = getSplitDelta( new Position( root, [ 0, 4 ] ), new Element( 'p' ), 3, 1 ); @@ -1004,8 +1008,10 @@ describe( 'Range', () => { } ); it( 'split inside range which starts at the beginning of split element', () => { - range.start = new Position( root, [ 0, 0 ] ); - range.end = new Position( root, [ 0, 4 ] ); + range = new Range( + new Position( root, [ 0, 0 ] ), + new Position( root, [ 0, 4 ] ) + ); const delta = getSplitDelta( new Position( root, [ 0, 3 ] ), new Element( 'p' ), 3, 1 ); @@ -1017,8 +1023,10 @@ describe( 'Range', () => { } ); it( 'split inside range which end is at the end of split element', () => { - range.start = new Position( root, [ 0, 3 ] ); - range.end = new Position( root, [ 0, 6 ] ); + range = new Range( + new Position( root, [ 0, 3 ] ), + new Position( root, [ 0, 6 ] ) + ); const delta = getSplitDelta( new Position( root, [ 0, 4 ] ), new Element( 'p' ), 2, 1 ); @@ -1030,8 +1038,10 @@ describe( 'Range', () => { } ); it( 'split element which has collapsed range at the end', () => { - range.start = new Position( root, [ 0, 6 ] ); - range.end = new Position( root, [ 0, 6 ] ); + range = new Range( + new Position( root, [ 0, 6 ] ), + new Position( root, [ 0, 6 ] ) + ); const delta = getSplitDelta( new Position( root, [ 0, 3 ] ), new Element( 'p' ), 3, 1 ); @@ -1045,8 +1055,10 @@ describe( 'Range', () => { describe( 'by MergeDelta', () => { it( 'merge element with collapsed range', () => { - range.start = new Position( root, [ 1, 0 ] ); - range.end = new Position( root, [ 1, 0 ] ); + range = new Range( + new Position( root, [ 1, 0 ] ), + new Position( root, [ 1, 0 ] ) + ); const delta = getMergeDelta( new Position( root, [ 1 ] ), 3, 3, 1 ); @@ -1107,8 +1119,10 @@ describe( 'Range', () => { describe( 'by WrapDelta', () => { it( 'maintans start position when wrapping element in which the range starts and ends', () => { //

f[o]o

bar

- range.start = new Position( root, [ 0, 1 ] ); - range.end = new Position( root, [ 0, 2 ] ); + range = new Range( + new Position( root, [ 0, 1 ] ), + new Position( root, [ 0, 2 ] ) + ); const wrapRange = new Range( new Position( root, [ 0 ] ), new Position( root, [ 1 ] ) ); const wrapElement = new Element( 'w' ); @@ -1124,8 +1138,10 @@ describe( 'Range', () => { it( 'maintans start position when wrapping element in which the range starts but not ends', () => { //

f[oo

b]ar

- range.start = new Position( root, [ 0, 1 ] ); - range.end = new Position( root, [ 1, 1 ] ); + range = new Range( + new Position( root, [ 0, 1 ] ), + new Position( root, [ 1, 1 ] ) + ); const wrapRange = new Range( new Position( root, [ 0 ] ), new Position( root, [ 1 ] ) ); const wrapElement = new Element( 'w' ); @@ -1141,8 +1157,10 @@ describe( 'Range', () => { it( 'maintans end position when wrapping element in which the range ends but not starts', () => { //

f[oo

b]ar

- range.start = new Position( root, [ 0, 1 ] ); - range.end = new Position( root, [ 1, 1 ] ); + range = new Range( + new Position( root, [ 0, 1 ] ), + new Position( root, [ 1, 1 ] ) + ); const wrapRange = new Range( new Position( root, [ 1 ] ), new Position( root, [ 2 ] ) ); const wrapElement = new Element( 'w' ); @@ -1160,8 +1178,10 @@ describe( 'Range', () => { describe( 'by UnwrapDelta', () => { it( 'maintans start position when wrapping element in which the range starts and ends', () => { //

f[o]o

bar

- range.start = new Position( root, [ 0, 0, 1 ] ); - range.end = new Position( root, [ 0, 0, 2 ] ); + range = new Range( + new Position( root, [ 0, 0, 1 ] ), + new Position( root, [ 0, 0, 2 ] ) + ); const unwrapPosition = new Position( root, [ 0 ] ); const delta = getUnwrapDelta( unwrapPosition, 1, 1 ); @@ -1176,8 +1196,10 @@ describe( 'Range', () => { it( 'maintans start position when wrapping element in which the range starts but not ends', () => { //

f[oo

b]ar

- range.start = new Position( root, [ 0, 0, 1 ] ); - range.end = new Position( root, [ 1, 1 ] ); + range = new Range( + new Position( root, [ 0, 0, 1 ] ), + new Position( root, [ 1, 1 ] ) + ); const unwrapPosition = new Position( root, [ 0 ] ); const delta = getUnwrapDelta( unwrapPosition, 1, 1 ); @@ -1195,8 +1217,10 @@ describe( 'Range', () => { it( 'maintans end position when wrapping element in which the range ends but not starts', () => { //

f[oo

b]ar

 - range.start = new Position( root, [ 0, 1 ] ); - range.end = new Position( root, [ 1, 0, 1 ] ); + range = new Range( + new Position( root, [ 0, 1 ] ), + new Position( root, [ 1, 0, 1 ] ) + ); const unwrapPosition = new Position( root, [ 1 ] ); const delta = getUnwrapDelta( unwrapPosition, 1, 1 ); From 6d3611923256bba85332cc826dd8b6e64ce978d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Tue, 24 Oct 2017 13:02:56 +0200 Subject: [PATCH 03/16] Other: Remove non semantic methods from Position. --- src/model/delta/basic-transformations.js | 22 ++++++--- src/model/position.js | 49 +++++---------------- src/model/range.js | 11 ++++- src/model/treewalker.js | 15 +++++-- tests/model/delta/transform/_utils/utils.js | 27 +++++++++--- tests/model/delta/transform/splitdelta.js | 20 +++++++-- tests/model/operation/transform.js | 2 +- 7 files changed, 87 insertions(+), 59 deletions(-) diff --git a/src/model/delta/basic-transformations.js b/src/model/delta/basic-transformations.js index ecf49b165..e2cb2ed46 100644 --- a/src/model/delta/basic-transformations.js +++ b/src/model/delta/basic-transformations.js @@ -73,7 +73,10 @@ addTransformationCase( AttributeDelta, SplitDelta, ( a, b, context ) => { const additionalAttributeDelta = new AttributeDelta(); const rangeStart = splitPosition.getShiftedBy( 1 ); - const rangeEnd = rangeStart.getMovedToChild(); + + const rangeEndPath = rangeStart.path.slice(); + rangeEndPath.push( 0 ); + const rangeEnd = new Position( rangeStart.root, rangeEndPath ); const oldValue = b._cloneOperation.nodes.getNode( 0 ).getAttribute( operation.key ); @@ -319,7 +322,10 @@ addTransformationCase( SplitDelta, WrapDelta, ( a, b, context ) => { // Now, `splitNodePos` points before wrapping element. // To get a position before last children of that element, we expand position's `path` member by proper offset. - const splitNodePos = b.range.start.getMovedToChild( b.howMany - 1 ); + const splitPath = b.range.start.path.slice(); + splitPath.push( b.howMany - 1 ); + + const splitNodePos = new Position( b.range.start.root, splitPath ); // SplitDelta insert operation position should be right after the node we split. delta._cloneOperation.position = splitNodePos.getShiftedBy( 1 ); @@ -328,12 +334,18 @@ addTransformationCase( SplitDelta, WrapDelta, ( a, b, context ) => { // Nodes moved by SplitDelta will be moved from new position, modified by WrapDelta. // To obtain that new position, `splitNodePos` will be used, as this is the node we are extracting children from. // Nothing changed inside split node so it is correct to use the original split position offset. - delta._moveOperation.sourcePosition = splitNodePos.getMovedToChild( a.position.offset ); + const sourcePath = splitNodePos.path.slice(); + sourcePath.push( a.position.offset ); + + delta._moveOperation.sourcePosition = new Position( splitNodePos.root, sourcePath ); // 3. Fix move operation target position. // SplitDelta move operation target position should be inside the node inserted by operation above. // Since the node is empty, we will insert at offset 0. - delta._moveOperation.targetPosition = splitNodePos.getShiftedBy( 1 ).getMovedToChild(); + const targetPath = splitNodePos.getShiftedBy( 1 ).path.slice(); + targetPath.push( 0 ); + + delta._moveOperation.targetPosition = new Position( splitNodePos.root, targetPath ); return [ delta ]; } @@ -438,7 +450,7 @@ addTransformationCase( WrapDelta, SplitDelta, ( a, b, context ) => { const path = delta._moveOperation.targetPosition.path.slice(); path[ index ] += 1; - delta._moveOperation.targetPosition = delta._moveOperation.targetPosition.getMovedToPath( path ); + delta._moveOperation.targetPosition = new Position( delta._moveOperation.targetPosition.root, path ); return [ delta ]; } diff --git a/src/model/position.js b/src/model/position.js index d3037b96c..f7e8214bf 100644 --- a/src/model/position.js +++ b/src/model/position.js @@ -345,16 +345,6 @@ export default class Position { return i === 0 ? null : ancestorsA[ i - 1 ]; } - /** - * Returns a new instance of `Position`, that has same {@link #root root} but it's path is set to `path` value. - * - * @param {Array.} path Position path. See {@link module:engine/model/position~Position#path}. - * @returns {module:engine/model/position~Position} Moved position. - */ - getMovedToPath( path ) { - return new Position( this.root, path ); - } - /** * Returns a new instance of `Position`, that has same {@link #parent parent} but it's offset * is set to `offset` value. @@ -367,7 +357,7 @@ export default class Position { setOffset( path, offset ); - return this.getMovedToPath( path ); + return new Position( this.root, path ); } /** @@ -383,28 +373,6 @@ export default class Position { return this.getShiftedTo( newOffset < 0 ? 0 : newOffset ); } - /** - * Returns a new instance of `Position`, that has same {@link #root root} but it's moved in path by one level up. - * - * @returns {module:engine/model/position~Position} Moved position. - */ - getMovedToParent() { - return this.getMovedToPath( this.getParentPath() ); - } - - /** - * Returns a new instance of `Position`, that has same {@link #root root} but it's moved in path by one level down. - * - * @returns {module:engine/model/position~Position} Moved position. - */ - getMovedToChild( offsetInChild = 0 ) { - const path = this.path.slice(); - - path.push( offsetInChild ); - - return this.getMovedToPath( path ); - } - /** * Checks whether this position is after given position. * @@ -508,14 +476,17 @@ export default class Position { return false; } - left = left.getMovedToParent().getShiftedBy( 1 ); + const path = left.getParentPath(); + path[ path.length - 1 ]++; + left = new Position( left.root, path ); + leftParent = leftParent.parent; } else { if ( right.offset !== 0 ) { return false; } - right = right.getMovedToParent(); + right = new Position( right.root, right.getParentPath() ); } } } @@ -573,8 +544,10 @@ export default class Position { } else { // Otherwise, decrement index on that path. const path = this.path.slice(); + path[ i ] -= howMany; - return this.getMovedToPath( path ); + + return new Position( this.root, path ); } } } @@ -614,8 +587,10 @@ export default class Position { // And are inserted before next node of that path... // "Push" the index on that path. const path = this.path.slice(); + path[ i ] += howMany; - return this.getMovedToPath( path ); + + return new Position( this.root, path ); } } diff --git a/src/model/range.js b/src/model/range.js index b16056094..8d97af7d4 100644 --- a/src/model/range.js +++ b/src/model/range.js @@ -301,7 +301,10 @@ export default class Range { ranges.push( new Range( pos, pos.getShiftedBy( howMany ) ) ); } - pos = pos.getMovedToParent().getShiftedBy( 1 ); + const path = pos.getParentPath(); + path[ path.length - 1 ]++; + + pos = new Position( pos.root, path ); posParent = posParent.parent; } @@ -315,7 +318,11 @@ export default class Range { ranges.push( new Range( pos, pos.getShiftedBy( howMany ) ) ); } - pos = pos.getShiftedTo( offset ).getMovedToChild(); + const path = pos.getParentPath(); + path.push( offset ); + path.push( 0 ); + + pos = new Position( pos.root, path ); } return ranges; diff --git a/src/model/treewalker.js b/src/model/treewalker.js index d5096d3dc..9d9634ae1 100644 --- a/src/model/treewalker.js +++ b/src/model/treewalker.js @@ -223,7 +223,9 @@ export default class TreeWalker { if ( node instanceof Element ) { if ( !this.shallow ) { // Manual operations on path internals for optimization purposes. Here and in the rest of the method. - position = position.getMovedToChild(); + const path = position.path.slice(); + path.push( 0 ); + position = new Position( position.root, path ); this._visitedParent = node; } else { @@ -258,7 +260,9 @@ export default class TreeWalker { return formatReturnValue( 'text', item, previousPosition, this.position, charactersCount ); } else { // `node` is not set, we reached the end of current `parent`. - position = position.getMovedToParent().getShiftedBy( 1 ); + const path = position.getParentPath(); + path[ path.length - 1 ]++; + position = new Position( position.root, path ); this.position = position; this._visitedParent = parent.parent; @@ -301,7 +305,10 @@ export default class TreeWalker { position = position.getShiftedBy( -1 ); if ( !this.shallow ) { - position = position.getMovedToChild( node.maxOffset ); + const path = position.path.slice(); + path.push( node.maxOffset ); + + position = new Position( position.root, path ); this.position = position; this._visitedParent = node; @@ -341,7 +348,7 @@ export default class TreeWalker { return formatReturnValue( 'text', item, previousPosition, position, charactersCount ); } else { // `node` is not set, we reached the beginning of current `parent`. - position = position.getMovedToParent(); + position = new Position( position.root, position.getParentPath() ); this.position = position; this._visitedParent = parent.parent; diff --git a/tests/model/delta/transform/_utils/utils.js b/tests/model/delta/transform/_utils/utils.js index 5ee8dcc49..ff23e9c00 100644 --- a/tests/model/delta/transform/_utils/utils.js +++ b/tests/model/delta/transform/_utils/utils.js @@ -68,9 +68,14 @@ export function getMarkerDelta( name, oldRange, newRange, version ) { export function getMergeDelta( position, howManyInPrev, howManyInNext, version ) { const delta = new MergeDelta(); - const sourcePosition = position.getMovedToChild(); + const sourcePath = position.path.slice(); + sourcePath.push( 0 ); + const sourcePosition = new Position( position.root, sourcePath ); - const targetPosition = position.getShiftedBy( -1 ).getMovedToChild( howManyInPrev ); + const targetPath = position.getShiftedBy( -1 ).path.slice(); + targetPath.push( howManyInPrev ); + + const targetPosition = new Position( position.root, targetPath ); const move = new MoveOperation( sourcePosition, howManyInNext, targetPosition, version ); move.isSticky = true; @@ -126,9 +131,15 @@ export function getRenameDelta( position, oldName, newName, baseVersion ) { export function getSplitDelta( position, nodeCopy, howManyMove, version ) { const delta = new SplitDelta(); - const insertPosition = position.getMovedToParent().getShiftedBy( 1 ); + const insertPath = position.getParentPath(); + insertPath[ insertPath.length - 1 ]++; + + const insertPosition = new Position( position.root, insertPath ); + + const targetPath = insertPosition.path.slice(); + targetPath.push( 0 ); - const targetPosition = insertPosition.getMovedToChild(); + const targetPosition = new Position( insertPosition.root, targetPath ); delta.addOperation( new InsertOperation( insertPosition, [ nodeCopy ], version ) ); @@ -147,7 +158,9 @@ export function getWrapDelta( range, element, version ) { const insert = new InsertOperation( range.end, element, version ); - const targetPosition = range.end.getMovedToChild(); + const targetPath = range.end.path.slice(); + targetPath.push( 0 ); + const targetPosition = new Position( range.end.root, targetPath ); const move = new MoveOperation( range.start, range.end.offset - range.start.offset, targetPosition, version + 1 ); @@ -162,7 +175,9 @@ export function getWrapDelta( range, element, version ) { export function getUnwrapDelta( positionBefore, howManyChildren, version ) { const delta = new UnwrapDelta(); - const sourcePosition = positionBefore.getMovedToChild(); + const sourcePath = positionBefore.path.slice(); + sourcePath.push( 0 ); + const sourcePosition = new Position( positionBefore.root, sourcePath ); const move = new MoveOperation( sourcePosition, howManyChildren, positionBefore, version ); move.isSticky = true; diff --git a/tests/model/delta/transform/splitdelta.js b/tests/model/delta/transform/splitdelta.js index 6c0ddfc3c..7d16555f2 100644 --- a/tests/model/delta/transform/splitdelta.js +++ b/tests/model/delta/transform/splitdelta.js @@ -968,8 +968,15 @@ describe( 'transform', () => { baseVersion = removeDelta.operations.length; const newInsertPosition = removeOperation.targetPosition.getShiftedBy( 2 ); - const newMoveSourcePosition = removeOperation.targetPosition.getShiftedBy( 1 ).getMovedToChild( 2 ); - const newMoveTargetPosition = newInsertPosition.getMovedToChild( ); + + const newSourcePath = removeOperation.targetPosition.getShiftedBy( 1 ).path.slice(); + newSourcePath.push( 2 ); + const newMoveSourcePosition = new Position( removeOperation.targetPosition.root, newSourcePath ); + + const newMoveTargetPath = newInsertPosition.path.slice(); + newMoveTargetPath.push( 0 ); + + const newMoveTargetPosition = new Position( newInsertPosition.root, newMoveTargetPath ); expectDelta( transformed[ 0 ], { type: SplitDelta, @@ -1002,8 +1009,13 @@ describe( 'transform', () => { baseVersion = removeDelta.operations.length; const newInsertPosition = removeOperation.targetPosition.getShiftedBy( 2 ); - const newMoveSourcePosition = removeOperation.targetPosition.getShiftedBy( 1 ).getMovedToChild( 3 ); - const newMoveTargetPosition = newInsertPosition.getMovedToChild( ); + const newMoveSourcePath = removeOperation.targetPosition.getShiftedBy( 1 ).path.slice(); + newMoveSourcePath.push( 3 ); + const newMoveSourcePosition = new Position( removeOperation.targetPosition.root, newMoveSourcePath ); + + const newMoveTargetPath = newInsertPosition.path.slice(); + newMoveTargetPath.push( 0 ); + const newMoveTargetPosition = new Position( newInsertPosition.root, newMoveTargetPath ); expectDelta( transformed[ 0 ], { type: SplitDelta, diff --git a/tests/model/operation/transform.js b/tests/model/operation/transform.js index 644d66e86..ebf3bc455 100644 --- a/tests/model/operation/transform.js +++ b/tests/model/operation/transform.js @@ -58,7 +58,7 @@ describe( 'transform', () => { path[ index ] += howMany; - return position.getMovedToPath( path ); + return new Position( position.root, path ); } describe( 'InsertOperation', () => { From d3a36451e6f457906c3fef35b49572fb4449b02c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Tue, 24 Oct 2017 14:30:42 +0200 Subject: [PATCH 04/16] Other: Remove redundant Position.createFromPosition calls. --- src/model/documentselection.js | 4 +--- src/model/operation/insertoperation.js | 2 +- src/model/operation/renameoperation.js | 4 ++-- src/model/selection.js | 4 ++-- src/model/treewalker.js | 4 ++-- 5 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/model/documentselection.js b/src/model/documentselection.js index 7db3fb86b..765123d97 100644 --- a/src/model/documentselection.js +++ b/src/model/documentselection.js @@ -7,7 +7,6 @@ * @module engine/model/documentselection */ -import Position from './position'; import Range from './range'; import LiveRange from './liverange'; import Text from './text'; @@ -666,10 +665,9 @@ export default class DocumentSelection extends Selection { _fixGraveyardSelection( liveRange, removedRangeStart ) { // The start of the removed range is the closest position to the `liveRange` - the original selection range. // This is a good candidate for a fixed selection range. - const positionCandidate = Position.createFromPosition( removedRangeStart ); // Find a range that is a correct selection range and is closest to the start of removed range. - const selectionRange = this._document.getNearestSelectionRange( positionCandidate ); + const selectionRange = this._document.getNearestSelectionRange( removedRangeStart ); // Remove the old selection range before preparing and adding new selection range. This order is important, // because new range, in some cases, may intersect with old range (it depends on `getNearestSelectionRange()` result). diff --git a/src/model/operation/insertoperation.js b/src/model/operation/insertoperation.js index 26ff844e2..43718dc0d 100644 --- a/src/model/operation/insertoperation.js +++ b/src/model/operation/insertoperation.js @@ -37,7 +37,7 @@ export default class InsertOperation extends Operation { * @readonly * @member {module:engine/model/position~Position} module:engine/model/operation/insertoperation~InsertOperation#position */ - this.position = Position.createFromPosition( position ); + this.position = position; /** * List of nodes to insert. diff --git a/src/model/operation/renameoperation.js b/src/model/operation/renameoperation.js index 502cd6828..ed7aeac47 100644 --- a/src/model/operation/renameoperation.js +++ b/src/model/operation/renameoperation.js @@ -66,7 +66,7 @@ export default class RenameOperation extends Operation { * @returns {module:engine/model/operation/renameoperation~RenameOperation} Clone of this operation. */ clone() { - return new RenameOperation( Position.createFromPosition( this.position ), this.oldName, this.newName, this.baseVersion ); + return new RenameOperation( this.position, this.oldName, this.newName, this.baseVersion ); } /** @@ -75,7 +75,7 @@ export default class RenameOperation extends Operation { * @returns {module:engine/model/operation/renameoperation~RenameOperation} */ getReversed() { - return new RenameOperation( Position.createFromPosition( this.position ), this.newName, this.oldName, this.baseVersion + 1 ); + return new RenameOperation( this.position, this.newName, this.oldName, this.baseVersion + 1 ); } /** diff --git a/src/model/selection.js b/src/model/selection.js index 27569afe8..d3fd43e84 100644 --- a/src/model/selection.js +++ b/src/model/selection.js @@ -240,7 +240,7 @@ export default class Selection { getFirstPosition() { const first = this.getFirstRange(); - return first ? Position.createFromPosition( first.start ) : null; + return first ? first.start : null; } /** @@ -255,7 +255,7 @@ export default class Selection { getLastPosition() { const lastRange = this.getLastRange(); - return lastRange ? Position.createFromPosition( lastRange.end ) : null; + return lastRange ? lastRange.end : null; } /** diff --git a/src/model/treewalker.js b/src/model/treewalker.js index 9d9634ae1..e1bb83d52 100644 --- a/src/model/treewalker.js +++ b/src/model/treewalker.js @@ -85,9 +85,9 @@ export default class TreeWalker { * @member {module:engine/model/position~Position} module:engine/model/treewalker~TreeWalker#position */ if ( options.startPosition ) { - this.position = Position.createFromPosition( options.startPosition ); + this.position = options.startPosition; } else { - this.position = Position.createFromPosition( this.boundaries[ this.direction == 'backward' ? 'end' : 'start' ] ); + this.position = this.boundaries[ this.direction == 'backward' ? 'end' : 'start' ]; } /** From 2fc0f814600688433fdb1eb051a65b890fbacba7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Tue, 24 Oct 2017 16:24:27 +0200 Subject: [PATCH 05/16] Other: Make view Position immutable. --- src/view/position.js | 46 +++++++++++++++++++++++++----------------- src/view/treewalker.js | 17 ++++++++-------- src/view/writer.js | 22 ++++++++++---------- 3 files changed, 47 insertions(+), 38 deletions(-) diff --git a/src/view/position.js b/src/view/position.js index b06bdf184..e72004016 100644 --- a/src/view/position.js +++ b/src/view/position.js @@ -13,6 +13,9 @@ import compareArrays from '@ckeditor/ckeditor5-utils/src/comparearrays'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; import EditableElement from './editableelement'; +const _parent = Symbol( 'parent' ); +const _offset = Symbol( 'offset' ); + /** * Position in the tree. Position is always located before or after a node. */ @@ -24,20 +27,28 @@ export default class Position { * @param {Number} offset Position offset. */ constructor( parent, offset ) { - /** - * Position parent. - * - * @member {module:engine/view/node~Node|module:engine/view/documentfragment~DocumentFragment} - * module:engine/view/position~Position#parent - */ - this.parent = parent; - - /** - * Position offset. - * - * @member {Number} module:engine/view/position~Position#offset - */ - this.offset = offset; + this[ _parent ] = parent; + this[ _offset ] = offset; + } + + /** + * Position parent. + * + * @readonly + * @type {module:engine/view/node~Node|module:engine/view/documentfragment~DocumentFragment} + */ + get parent() { + return this[ _parent ]; + } + + /** + * Position offset. + * + * @readonly + * @type {Number} + */ + get offset() { + return this[ _offset ]; } /** @@ -129,12 +140,9 @@ export default class Position { * @returns {module:engine/view/position~Position} Shifted position. */ getShiftedBy( shift ) { - const shifted = Position.createFromPosition( this ); - - const offset = shifted.offset + shift; - shifted.offset = offset < 0 ? 0 : offset; + const offset = this.offset + shift; - return shifted; + return new Position( this.parent, offset < 0 ? 0 : offset ); } /** diff --git a/src/view/treewalker.js b/src/view/treewalker.js index 838f09280..479e79f15 100644 --- a/src/view/treewalker.js +++ b/src/view/treewalker.js @@ -73,9 +73,9 @@ export default class TreeWalker { * @member {module:engine/view/position~Position} module:engine/view/treewalker~TreeWalker#position */ if ( options.startPosition ) { - this.position = Position.createFromPosition( options.startPosition ); + this.position = options.startPosition; } else { - this.position = Position.createFromPosition( options.boundaries[ options.direction == 'backward' ? 'end' : 'start' ] ); + this.position = options.boundaries[ options.direction == 'backward' ? 'end' : 'start' ]; } /** @@ -222,7 +222,7 @@ export default class TreeWalker { if ( !this.shallow ) { position = new Position( node, 0 ); } else { - position.offset++; + position = position.getShiftedBy( 1 ); } this.position = position; @@ -245,7 +245,7 @@ export default class TreeWalker { position = Position.createAfter( item ); } else { // If not just keep moving forward. - position.offset++; + position = position.getShiftedBy( 1 ); } this.position = position; @@ -266,7 +266,8 @@ export default class TreeWalker { const textProxy = new TextProxy( parent, position.offset, textLength ); - position.offset += textLength; + position = position.getShiftedBy( textLength ); + this.position = position; return this._formatReturnValue( 'text', textProxy, previousPosition, position, textLength ); @@ -334,7 +335,7 @@ export default class TreeWalker { return this._formatReturnValue( 'elementEnd', node, previousPosition, position ); } } else { - position.offset--; + position = position.getShiftedBy( -1 ); this.position = position; return this._formatReturnValue( 'elementStart', node, previousPosition, position, 1 ); @@ -358,7 +359,7 @@ export default class TreeWalker { position = Position.createBefore( item ); } else { // If not just keep moving backward. - position.offset--; + position = position.getShiftedBy( -1 ); } this.position = position; @@ -377,7 +378,7 @@ export default class TreeWalker { textLength = 1; } - position.offset -= textLength; + position = position.getShiftedBy( -textLength ); const textProxy = new TextProxy( parent, position.offset, textLength ); diff --git a/src/view/writer.js b/src/view/writer.js index ffbd49b07..0b9a060a2 100644 --- a/src/view/writer.js +++ b/src/view/writer.js @@ -305,7 +305,7 @@ export function insert( position, nodes ) { const insertionPosition = _breakAttributes( position, true ); const length = container.insertChildren( insertionPosition.offset, nodes ); - const endPosition = insertionPosition.getShiftedBy( length ); + let endPosition = insertionPosition.getShiftedBy( length ); const start = mergeAttributes( insertionPosition ); // When no nodes were inserted - return collapsed range. @@ -314,7 +314,7 @@ export function insert( position, nodes ) { } else { // If start position was merged - move end position. if ( !start.isEqual( insertionPosition ) ) { - endPosition.offset--; + endPosition = endPosition.getShiftedBy( -1 ); } const end = mergeAttributes( endPosition ); @@ -447,7 +447,7 @@ export function move( sourceRange, targetPosition ) { nodes = remove( sourceRange ); - targetPosition.offset += ( parent.childCount - countBefore ); + targetPosition = targetPosition.getShiftedBy( parent.childCount - countBefore ); } else { nodes = remove( sourceRange ); } @@ -512,7 +512,7 @@ export function wrap( range, attribute ) { // If start position was merged - move end position back. if ( !start.isEqual( newRange.start ) ) { - newRange.end.offset--; + newRange.end = newRange.end.getShiftedBy( -1 ); } const end = mergeAttributes( newRange.end ); @@ -626,7 +626,7 @@ export function unwrap( range, attribute ) { // If start position was merged - move end position back. if ( !start.isEqual( newRange.start ) ) { - newRange.end.offset--; + newRange.end = newRange.end.getShiftedBy( -1 ); } const end = mergeAttributes( newRange.end ); @@ -702,12 +702,12 @@ function _breakAttributesRange( range, forceSplitText = false ) { return new Range( position, position ); } - const breakEnd = _breakAttributes( rangeEnd, forceSplitText ); + let breakEnd = _breakAttributes( rangeEnd, forceSplitText ); const count = breakEnd.parent.childCount; const breakStart = _breakAttributes( rangeStart, forceSplitText ); // Calculate new break end offset. - breakEnd.offset += breakEnd.parent.childCount - count; + breakEnd = breakEnd.getShiftedBy( breakEnd.parent.childCount - count ); return new Range( breakStart, breakEnd ); } @@ -857,8 +857,8 @@ function unwrapChildren( parent, startOffset, endOffset, attribute ) { // Merge at each unwrap. let offsetChange = 0; - for ( const position of unwrapPositions ) { - position.offset -= offsetChange; + for ( let position of unwrapPositions ) { + position = position.getShiftedBy( -offsetChange ); // Do not merge with elements outside selected children. if ( position.offset == startOffset || position.offset == endOffset ) { @@ -918,8 +918,8 @@ function wrapChildren( parent, startOffset, endOffset, attribute ) { // Merge at each wrap. let offsetChange = 0; - for ( const position of wrapPositions ) { - position.offset -= offsetChange; + for ( let position of wrapPositions ) { + position = position.getShiftedBy( -offsetChange ); // Do not merge with elements outside selected children. if ( position.offset == startOffset ) { From 9f3a6aa6841e3a4810ecebf5fb6ada6e9746bbdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Tue, 24 Oct 2017 16:36:38 +0200 Subject: [PATCH 06/16] Other: Remove excessive view Position.createFromPosition calls. --- src/view/range.js | 7 +++---- src/view/selection.js | 10 ++++------ src/view/treewalker.js | 4 ++-- src/view/writer.js | 8 ++++---- tests/view/range.js | 4 ++-- tests/view/selection.js | 10 +++++----- 6 files changed, 20 insertions(+), 23 deletions(-) diff --git a/src/view/range.js b/src/view/range.js index 991b69340..9f5cd0b60 100644 --- a/src/view/range.js +++ b/src/view/range.js @@ -28,14 +28,14 @@ export default class Range { * * @member {module:engine/view/position~Position} */ - this.start = Position.createFromPosition( start ); + this.start = start; /** * End position. * * @member {module:engine/view/position~Position} */ - this.end = end ? Position.createFromPosition( end ) : Position.createFromPosition( start ); + this.end = end ? end : start; } /** @@ -451,9 +451,8 @@ export default class Range { */ static createCollapsedAt( itemOrPosition, offset ) { const start = Position.createAt( itemOrPosition, offset ); - const end = Position.createFromPosition( start ); - return new Range( start, end ); + return new Range( start, start ); } } diff --git a/src/view/selection.js b/src/view/selection.js index 3d96afb40..d48551259 100644 --- a/src/view/selection.js +++ b/src/view/selection.js @@ -132,9 +132,8 @@ export default class Selection { return null; } const range = this._ranges[ this._ranges.length - 1 ]; - const anchor = this._lastRangeBackward ? range.end : range.start; - return Position.createFromPosition( anchor ); + return this._lastRangeBackward ? range.end : range.start; } /** @@ -148,9 +147,8 @@ export default class Selection { return null; } const range = this._ranges[ this._ranges.length - 1 ]; - const focus = this._lastRangeBackward ? range.start : range.end; - return Position.createFromPosition( focus ); + return this._lastRangeBackward ? range.start : range.end; } /** @@ -281,7 +279,7 @@ export default class Selection { getFirstPosition() { const firstRange = this.getFirstRange(); - return firstRange ? Position.createFromPosition( firstRange.start ) : null; + return firstRange ? firstRange.start : null; } /** @@ -294,7 +292,7 @@ export default class Selection { getLastPosition() { const lastRange = this.getLastRange(); - return lastRange ? Position.createFromPosition( lastRange.end ) : null; + return lastRange ? lastRange.end : null; } /** diff --git a/src/view/treewalker.js b/src/view/treewalker.js index 479e79f15..7979da760 100644 --- a/src/view/treewalker.js +++ b/src/view/treewalker.js @@ -187,7 +187,7 @@ export default class TreeWalker { * @returns {module:engine/view/treewalker~TreeWalkerValue} return.value Information about taken step. */ _next() { - let position = Position.createFromPosition( this.position ); + let position = this.position; const previousPosition = this.position; const parent = position.parent; @@ -293,7 +293,7 @@ export default class TreeWalker { * @returns {module:engine/view/treewalker~TreeWalkerValue} return.value Information about taken step. */ _previous() { - let position = Position.createFromPosition( this.position ); + let position = this.position; const previousPosition = this.position; const parent = position.parent; diff --git a/src/view/writer.js b/src/view/writer.js index 0b9a060a2..7a1017db4 100644 --- a/src/view/writer.js +++ b/src/view/writer.js @@ -355,7 +355,7 @@ export function remove( range ) { // Merge after removing. const mergePosition = mergeAttributes( breakStart ); range.start = mergePosition; - range.end = Position.createFromPosition( mergePosition ); + range.end = mergePosition; // Return removed nodes. return new DocumentFragment( removed ); @@ -537,7 +537,7 @@ export function wrapPosition( position, attribute ) { // Return same position when trying to wrap with attribute similar to position parent. if ( attribute.isSimilar( position.parent ) ) { - return movePositionToTextNode( Position.createFromPosition( position ) ); + return movePositionToTextNode( position ); } // When position is inside text node - break it and place new position between two text nodes. @@ -752,12 +752,12 @@ function _breakAttributes( position, forceSplitText = false ) { // There are no attributes to break and text nodes breaking is not forced. if ( !forceSplitText && positionParent.is( 'text' ) && isContainerOrFragment( positionParent.parent ) ) { - return Position.createFromPosition( position ); + return position; } // Position's parent is container, so no attributes to break. if ( isContainerOrFragment( positionParent ) ) { - return Position.createFromPosition( position ); + return position; } // Break text and start again in new position. diff --git a/tests/view/range.js b/tests/view/range.js index b33596bdb..1fba91170 100644 --- a/tests/view/range.js +++ b/tests/view/range.js @@ -25,8 +25,8 @@ describe( 'Range', () => { const range = new Range( start, end ); expect( range ).to.be.an.instanceof( Range ); - expect( range ).to.have.property( 'start' ).that.not.equals( start ); - expect( range ).to.have.property( 'end' ).that.not.equals( end ); + expect( range ).to.have.property( 'start' ).that.equals( start ); + expect( range ).to.have.property( 'end' ).that.equals( end ); expect( range.start.parent ).to.equal( start.parent ); expect( range.end.parent ).to.equal( end.parent ); expect( range.start.offset ).to.equal( start.offset ); diff --git a/tests/view/selection.js b/tests/view/selection.js index e0660bbc0..5e1b8bf50 100644 --- a/tests/view/selection.js +++ b/tests/view/selection.js @@ -59,7 +59,7 @@ describe( 'Selection', () => { const anchor = selection.anchor; expect( anchor.isEqual( range1.start ) ).to.be.true; - expect( anchor ).to.not.equal( range1.start ); + expect( anchor ).to.equal( range1.start ); } ); it( 'should return end of single range in selection when added as backward', () => { @@ -67,7 +67,7 @@ describe( 'Selection', () => { const anchor = selection.anchor; expect( anchor.isEqual( range1.end ) ).to.be.true; - expect( anchor ).to.not.equal( range1.end ); + expect( anchor ).to.equal( range1.end ); } ); it( 'should get anchor from last inserted range', () => { @@ -95,7 +95,7 @@ describe( 'Selection', () => { const focus = selection.focus; expect( focus.isEqual( range1.start ) ).to.be.true; - expect( focus ).to.not.equal( range1.start ); + expect( focus ).to.equal( range1.start ); } ); it( 'should get focus from last inserted range', () => { @@ -410,7 +410,7 @@ describe( 'Selection', () => { const position = selection.getFirstPosition(); expect( position.isEqual( range2.start ) ).to.be.true; - expect( position ).to.not.equal( range2.start ); + expect( position ).to.equal( range2.start ); } ); it( 'should return null if no ranges are present', () => { @@ -427,7 +427,7 @@ describe( 'Selection', () => { const position = selection.getLastPosition(); expect( position.isEqual( range3.end ) ).to.be.true; - expect( position ).to.not.equal( range3.end ); + expect( position ).to.equal( range3.end ); } ); it( 'should return null if no ranges are present', () => { From 8eae307521621a246c12d37fbcc5a6038edd482e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Wed, 25 Oct 2017 12:33:10 +0200 Subject: [PATCH 07/16] Other: Make view Range immutable. --- src/view/range.js | 41 +++++++++++++++++++++++++------------ src/view/writer.js | 26 ++++++++++++++--------- tests/view/writer/remove.js | 25 +++++++++++----------- 3 files changed, 56 insertions(+), 36 deletions(-) diff --git a/src/view/range.js b/src/view/range.js index 9f5cd0b60..4098a9589 100644 --- a/src/view/range.js +++ b/src/view/range.js @@ -10,6 +10,9 @@ import Position from './position'; import TreeWalker from './treewalker'; +const _start = Symbol( 'start' ); +const _end = Symbol( 'end' ); + /** * Tree view range. */ @@ -23,19 +26,8 @@ export default class Range { * @param {module:engine/view/position~Position} [end] End position. If not set, range will be collapsed at `start` position. */ constructor( start, end = null ) { - /** - * Start position. - * - * @member {module:engine/view/position~Position} - */ - this.start = start; - - /** - * End position. - * - * @member {module:engine/view/position~Position} - */ - this.end = end ? end : start; + this[ _start ] = start; + this[ _end ] = end ? end : start; } /** @@ -53,9 +45,30 @@ export default class Range { yield* new TreeWalker( { boundaries: this, ignoreElementEnd: true } ); } + /** + * Start position. + * + * @readonly + * @type {module:engine/view/position~Position} + */ + get start() { + return this[ _start ]; + } + + /** + * End position. + * + * @readonly + * @type {module:engine/view/position~Position} + */ + get end() { + return this[ _end ]; + } + /** * Returns whether the range is collapsed, that is it start and end positions are equal. * + * @readonly * @type {Boolean} */ get isCollapsed() { @@ -66,6 +79,7 @@ export default class Range { * Returns whether this range is flat, that is if {@link module:engine/view/range~Range#start start} position and * {@link module:engine/view/range~Range#end end} position are in the same {@link module:engine/view/position~Position#parent parent}. * + * @readonly * @type {Boolean} */ get isFlat() { @@ -75,6 +89,7 @@ export default class Range { /** * Range root element. * + * @readonly * @type {module:engine/view/element~Element|module:engine/view/documentfragment~DocumentFragment} */ get root() { diff --git a/src/view/writer.js b/src/view/writer.js index 7a1017db4..06ba7daaf 100644 --- a/src/view/writer.js +++ b/src/view/writer.js @@ -353,9 +353,7 @@ export function remove( range ) { const removed = parentContainer.removeChildren( breakStart.offset, count ); // Merge after removing. - const mergePosition = mergeAttributes( breakStart ); - range.start = mergePosition; - range.end = mergePosition; + mergeAttributes( breakStart ); // Return removed nodes. return new DocumentFragment( removed ); @@ -406,17 +404,20 @@ export function clear( range, element ) { // If we have found element to remove. if ( rangeToRemove ) { + let rangeEnd = rangeToRemove.end; + let rangeStart = rangeToRemove.start; + // We need to check if element range stick out of the given range and truncate if it is. if ( rangeToRemove.end.isAfter( range.end ) ) { - rangeToRemove.end = range.end; + rangeEnd = range.end; } if ( rangeToRemove.start.isBefore( range.start ) ) { - rangeToRemove.start = range.start; + rangeStart = range.start; } // At the end we remove range with found element. - remove( rangeToRemove ); + remove( new Range( rangeStart, rangeEnd ) ); } } } @@ -511,10 +512,13 @@ export function wrap( range, attribute ) { const start = mergeAttributes( newRange.start ); // If start position was merged - move end position back. + let rangeEnd = newRange.end; + if ( !start.isEqual( newRange.start ) ) { - newRange.end = newRange.end.getShiftedBy( -1 ); + rangeEnd = rangeEnd.getShiftedBy( -1 ); } - const end = mergeAttributes( newRange.end ); + + const end = mergeAttributes( rangeEnd ); return new Range( start, end ); } @@ -625,10 +629,12 @@ export function unwrap( range, attribute ) { const start = mergeAttributes( newRange.start ); // If start position was merged - move end position back. + let rangeEnd = newRange.end; + if ( !start.isEqual( newRange.start ) ) { - newRange.end = newRange.end.getShiftedBy( -1 ); + rangeEnd = rangeEnd.getShiftedBy( -1 ); } - const end = mergeAttributes( newRange.end ); + const end = mergeAttributes( rangeEnd ); return new Range( start, end ); } diff --git a/tests/view/writer/remove.js b/tests/view/writer/remove.js index ecced8a9e..db3edca6d 100644 --- a/tests/view/writer/remove.js +++ b/tests/view/writer/remove.js @@ -15,8 +15,7 @@ import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; describe( 'writer', () => { /** - * Executes test using `parse` and `stringify` utils functions. Uses range delimiters `[]{}` to create and - * test ranges. + * Executes test using `parse` and `stringify` utils functions. Uses range delimiters `[]{}` to create ranges. * * @param {String} input * @param {String} expectedResult @@ -27,7 +26,7 @@ describe( 'writer', () => { const range = selection.getFirstRange(); const removed = remove( range ); - expect( stringify( view, range, { showType: true, showPriority: true } ) ).to.equal( expectedResult ); + expect( stringify( view, null, { showType: true, showPriority: true } ) ).to.equal( expectedResult ); expect( stringify( removed, null, { showType: true, showPriority: true } ) ).to.equal( expectedRemoved ); } @@ -60,21 +59,21 @@ describe( 'writer', () => { } ); it( 'should remove single text node', () => { - test( '[foobar]', '[]', 'foobar' ); + test( '[foobar]', '', 'foobar' ); } ); it( 'should not leave empty text nodes', () => { - test( '{foobar}', '[]', 'foobar' ); + test( '{foobar}', '', 'foobar' ); } ); it( 'should remove part of the text node', () => { - test( 'f{oob}ar', 'f{}ar', 'oob' ); + test( 'f{oob}ar', 'far', 'oob' ); } ); it( 'should remove parts of nodes #1', () => { test( 'f{ooba}r', - 'f[]r', + 'fr', 'ooba' ); } ); @@ -82,7 +81,7 @@ describe( 'writer', () => { it( 'should support unicode', () => { test( 'நி{லைக்}கு', - 'நி[]கு', + 'நிகு', 'லைக்' ); } ); @@ -92,7 +91,7 @@ describe( 'writer', () => { '' + 'foo[bar]bazqux' + '', - 'foo{}bazqux', + 'foobazqux', 'bar' ); } ); @@ -102,19 +101,19 @@ describe( 'writer', () => { '' + 'fo{obarba}zqux' + '', - 'fo{}zqux', + 'fozqux', 'obarba' ); } ); it( 'should remove part of the text node in document fragment', () => { - test( 'fo{ob}ar', 'fo{}ar', 'ob' ); + test( 'fo{ob}ar', 'foar', 'ob' ); } ); it( 'should remove EmptyElement', () => { test( 'foo[]bar', - 'foo{}bar', + 'foobar', '' ); } ); @@ -133,7 +132,7 @@ describe( 'writer', () => { it( 'should remove UIElement', () => { test( 'foo[]bar', - 'foo{}bar', + 'foobar', '' ); } ); From 74eca6af34db7f53cb4e381d2f04d607d3d33f31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Wed, 25 Oct 2017 13:19:03 +0200 Subject: [PATCH 08/16] Docs: Update `writer.remove()` method docs. --- src/view/writer.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/view/writer.js b/src/view/writer.js index 06ba7daaf..1c9fe58d1 100644 --- a/src/view/writer.js +++ b/src/view/writer.js @@ -331,8 +331,7 @@ export function insert( position, nodes ) { * same parent container. * * @function module:engine/view/writer~writer.remove - * @param {module:engine/view/range~Range} range Range to remove from container. After removing, it will be updated - * to a collapsed range showing the new position. + * @param {module:engine/view/range~Range} range Range to remove from container. * @returns {module:engine/view/documentfragment~DocumentFragment} Document fragment containing removed nodes. */ export function remove( range ) { From 1aaa955f0d1c5724e91ea6094c29c6e7c404cd5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Fri, 17 Nov 2017 08:33:27 +0100 Subject: [PATCH 09/16] Code style: Name variables accordingly in operation/transform.js. --- src/model/operation/transform.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/model/operation/transform.js b/src/model/operation/transform.js index 1cbe1c7ee..3f602a417 100644 --- a/src/model/operation/transform.js +++ b/src/model/operation/transform.js @@ -179,7 +179,7 @@ const ot = { // Take the start and the end of the range and transform them by deletion of moved nodes. // Note that if rangeB was inside AttributeOperation range, only difference.end will be transformed. // This nicely covers the joining simplification we did in the previous step. - const range = new Range( + const differenceTransformed = new Range( difference.start._getTransformedByDeletion( b.sourcePosition, b.howMany ), difference.end._getTransformedByDeletion( b.sourcePosition, b.howMany ) ); @@ -189,19 +189,19 @@ const ot = { // previously transformed target position. // Note that we do not use Position._getTransformedByMove on range boundaries because we need to // transform by insertion a range as a whole, since newTargetPosition might be inside that range. - ranges = range._getTransformedByInsertion( b.getMovedRangeStart(), b.howMany, true, false ).reverse(); + ranges = differenceTransformed._getTransformedByInsertion( b.getMovedRangeStart(), b.howMany, true, false ).reverse(); } if ( common !== null ) { // Here we do not need to worry that newTargetPosition is inside moved range, because that // would mean that the MoveOperation targets into itself, and that is incorrect operation. // Instead, we calculate the new position of that part of original range. - const range = new Range( + const commonTransformed = new Range( common.start._getCombined( b.sourcePosition, b.getMovedRangeStart() ), common.end._getCombined( b.sourcePosition, b.getMovedRangeStart() ) ); - ranges.push( range ); + ranges.push( commonTransformed ); } // Map transformed range(s) to operations and return them. From 2ad039bc1228c90cd7f5f2206247df668ba9eb03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Fri, 17 Nov 2017 08:40:50 +0100 Subject: [PATCH 10/16] Other: Make transformation helper method always return copy of Position. --- src/model/position.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/model/position.js b/src/model/position.js index f7e8214bf..51e1b4116 100644 --- a/src/model/position.js +++ b/src/model/position.js @@ -515,7 +515,7 @@ export default class Position { _getTransformedByDeletion( deletePosition, howMany ) { // This position can't be affected if deletion was in a different root. if ( this.root != deletePosition.root ) { - return this; + return Position.createFromPosition( this ); } const comparisonResult = compareArrays( deletePosition.getParentPath(), this.getParentPath() ); @@ -552,7 +552,7 @@ export default class Position { } } - return this; + return Position.createFromPosition( this ); } /** @@ -569,7 +569,7 @@ export default class Position { _getTransformedByInsertion( insertPosition, howMany, insertBefore ) { // This position can't be affected if insertion was in a different root. if ( this.root != insertPosition.root ) { - return this; + return Position.createFromPosition( this ); } if ( compareArrays( insertPosition.getParentPath(), this.getParentPath() ) == 'same' ) { @@ -594,7 +594,7 @@ export default class Position { } } - return this; + return Position.createFromPosition( this ); } /** From ed738389f91dcdec2f7f37b9cdddd4fb82eac34c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Fri, 17 Nov 2017 09:02:23 +0100 Subject: [PATCH 11/16] Code style: Don't use Symbols for protected properties in model's Position and Range. --- src/model/liveposition.js | 6 +++--- src/model/liverange.js | 6 +++--- src/model/position.js | 45 ++++++++++++++++----------------------- src/model/range.js | 45 +++++++++++++++------------------------ 4 files changed, 41 insertions(+), 61 deletions(-) diff --git a/src/model/liveposition.js b/src/model/liveposition.js index f9f2213d0..ab6cdb7c9 100644 --- a/src/model/liveposition.js +++ b/src/model/liveposition.js @@ -7,7 +7,7 @@ * @module engine/model/liveposition */ -import Position, { setPath, setRoot } from './position'; +import Position from './position'; import Range from './range'; import EmitterMixin from '@ckeditor/ckeditor5-utils/src/emittermixin'; import mix from '@ckeditor/ckeditor5-utils/src/mix'; @@ -194,8 +194,8 @@ function transform( type, range, position ) { if ( !this.isEqual( transformed ) ) { const oldPosition = Position.createFromPosition( this ); - setPath( this, transformed.path ); - setRoot( this, transformed.root ); + this._path = transformed.path; + this._root = transformed.root; this.fire( 'change', oldPosition ); } diff --git a/src/model/liverange.js b/src/model/liverange.js index b2c714072..a9785c9b6 100644 --- a/src/model/liverange.js +++ b/src/model/liverange.js @@ -7,7 +7,7 @@ * @module engine/model/liverange */ -import Range, { setStart, setEnd } from './range'; +import Range from './range'; import EmitterMixin from '@ckeditor/ckeditor5-utils/src/emittermixin'; import mix from '@ckeditor/ckeditor5-utils/src/mix'; @@ -178,8 +178,8 @@ function transform( changeType, deltaType, batch, targetRange, sourcePosition ) // If range boundaries have changed, fire `change:range` event. const oldRange = Range.createFromRange( this ); - setStart( this, updated.start ); - setEnd( this, updated.end ); + this._start = updated.start; + this._end = updated.end; this.fire( 'change:range', oldRange, { type: changeType, diff --git a/src/model/position.js b/src/model/position.js index 51e1b4116..ae2f83e2f 100644 --- a/src/model/position.js +++ b/src/model/position.js @@ -13,9 +13,6 @@ import compareArrays from '@ckeditor/ckeditor5-utils/src/comparearrays'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; import Text from './text'; -const _path = Symbol( 'path' ); -const _root = Symbol( 'root' ); - /** * Represents a position in the model tree. * @@ -73,8 +70,22 @@ export default class Position { // Make path immutable Object.freeze( path ); - setRoot( this, root.root ); - setPath( this, path ); + /** + * Root of the position path. + * + * @protected + * @member {module:engine/model/element~Element|module:engine/model/documentfragment~DocumentFragment} + * module:engine/model/position~Position#_root + */ + this._root = root.root; + + /** + * Position of the node in the tree. + * + * @protected + * @member {Array.} module:engine/model/position~Position#_path + */ + this._path = path; } /** @@ -107,7 +118,7 @@ export default class Position { * @member {Array.} module:engine/model/position~Position#path */ get path() { - return this[ _path ]; + return this._path; } /** @@ -118,7 +129,7 @@ export default class Position { * module:engine/model/position~Position#root */ get root() { - return this[ _root ]; + return this._root; } /** @@ -817,26 +828,6 @@ export default class Position { } } -/** - * Method used to expose root setter to child classes. - * @protected - * @param {module:engine/model/position~Position} position Position of which root should be modified. - * @param {module:engine/model/element~Element|module:engine/model/documentfragment~DocumentFragment} root Root of the position. - */ -export function setRoot( position, root ) { - position[ _root ] = root; -} - -/** - * Method used to expose path setter to child classes. - * @protected - * @param {module:engine/model/position~Position} position Position of which path should be modified. - * @param {Array.} path Position path. See {@link module:engine/model/position~Position#path}. - */ -export function setPath( position, path ) { - position[ _path ] = path; -} - // Helper for setting offset on give path array. // @private // @param {Array.} path Position path. See {@link module:engine/model/position~Position#path}. diff --git a/src/model/range.js b/src/model/range.js index 8d97af7d4..317c09c21 100644 --- a/src/model/range.js +++ b/src/model/range.js @@ -11,9 +11,6 @@ import Position from './position'; import TreeWalker from './treewalker'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; -const _start = Symbol( 'start' ); -const _end = Symbol( 'end' ); - /** * Range class. Range is iterable. */ @@ -27,8 +24,21 @@ export default class Range { * @param {module:engine/model/position~Position} [end] End position. If not set, range will be collapsed at `start` position. */ constructor( start, end = null ) { - setStart( this, start ); - setEnd( this, end ? end : start ); + /** + * Start position. + * + * @protected + * @member {module:engine/model/position~Position} + */ + this._start = start; + + /** + * End position. + * + * @protected + * @member {module:engine/model/position~Position} + */ + this._end = end ? end : start; } /** @@ -54,7 +64,7 @@ export default class Range { * @member {module:engine/model/position~Position} */ get start() { - return this[ _start ]; + return this._start; } /** @@ -64,7 +74,7 @@ export default class Range { * @member {module:engine/model/position~Position} */ get end() { - return this[ _end ]; + return this._end; } /** @@ -867,24 +877,3 @@ export default class Range { return new this( Position.fromJSON( json.start, doc ), Position.fromJSON( json.end, doc ) ); } } - -/** - * Method used to expose start setter to child classes. - * @protected - * @param {module:engine/model/range~Range} range Range of which start position should be sent. - * @param {module:engine/model/position~Position} position Position to set as range start. - * See {@link module:engine/model/range~Range#start}. - */ -export function setStart( range, position ) { - range[ _start ] = position; -} - -/** - * Method used to expose end setter to child classes. - * @protected - * @param {module:engine/model/range~Range} range Range of which end position should be sent. - * @param {module:engine/model/position~Position} position Position to set as range end. See {@link module:engine/model/range~Range#end}. - */ -export function setEnd( range, position ) { - range[ _end ] = position; -} From 010cfc6cd45c86dbe3fbbba357d1886743ac9f0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Fri, 17 Nov 2017 09:15:23 +0100 Subject: [PATCH 12/16] Other: User positions rather then creating new range in loops in Range.createFromRanges(). --- src/model/range.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/model/range.js b/src/model/range.js index 317c09c21..8936706bb 100644 --- a/src/model/range.js +++ b/src/model/range.js @@ -839,13 +839,14 @@ export default class Range { // 4. At this moment we don't need the original range. // We are going to modify the result and we need to return a new instance of Range. // We have to create a copy of the reference range. - let result = new this( ref.start, ref.end ); + let start = ref.start; + let end = ref.end; // 5. Ranges should be checked and glued starting from the range that is closest to the reference range. // Since ranges are sorted, start with the range with index that is closest to reference range index. for ( let i = refIndex - 1; i >= 0; i++ ) { - if ( ranges[ i ].end.isEqual( result.start ) ) { - result = new this( ranges[ i ].start, result.end ); + if ( ranges[ i ].end.isEqual( start ) ) { + start = ranges[ i ].start; } else { // If ranges are not starting/ending at the same position there is no point in looking further. break; @@ -855,15 +856,15 @@ export default class Range { // 6. Ranges should be checked and glued starting from the range that is closest to the reference range. // Since ranges are sorted, start with the range with index that is closest to reference range index. for ( let i = refIndex + 1; i < ranges.length; i++ ) { - if ( ranges[ i ].start.isEqual( result.end ) ) { - result = new this( result.start, ranges[ i ].end ); + if ( ranges[ i ].start.isEqual( end ) ) { + end = ranges[ i ].end; } else { // If ranges are not starting/ending at the same position there is no point in looking further. break; } } - return result; + return new this( start, end ); } /** From f0760962d7ef5430153c7b7e416adf5c440d539d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Fri, 17 Nov 2017 09:20:24 +0100 Subject: [PATCH 13/16] Code style: Remove confusing position variable from TreeWalker methods. --- src/view/treewalker.js | 96 +++++++++++++++++------------------------- 1 file changed, 39 insertions(+), 57 deletions(-) diff --git a/src/view/treewalker.js b/src/view/treewalker.js index 7979da760..a9e3dfcf8 100644 --- a/src/view/treewalker.js +++ b/src/view/treewalker.js @@ -187,17 +187,16 @@ export default class TreeWalker { * @returns {module:engine/view/treewalker~TreeWalkerValue} return.value Information about taken step. */ _next() { - let position = this.position; const previousPosition = this.position; - const parent = position.parent; + const parent = this.position.parent; // We are at the end of the root. - if ( parent.parent === null && position.offset === parent.childCount ) { + if ( parent.parent === null && this.position.offset === parent.childCount ) { return { done: true }; } // We reached the walker boundary. - if ( parent === this._boundaryEndParent && position.offset == this.boundaries.end.offset ) { + if ( parent === this._boundaryEndParent && this.position.offset == this.boundaries.end.offset ) { return { done: true }; } @@ -206,32 +205,29 @@ export default class TreeWalker { // Text is a specific parent because it contains string instead of child nodes. if ( parent instanceof Text ) { - if ( position.isAtEnd ) { + if ( this.position.isAtEnd ) { // Prevent returning "elementEnd" for Text node. Skip that value and return the next walker step. this.position = Position.createAfter( parent ); return this._next(); } - node = parent.data[ position.offset ]; + node = parent.data[ this.position.offset ]; } else { - node = parent.getChild( position.offset ); + node = parent.getChild( this.position.offset ); } if ( node instanceof Element ) { if ( !this.shallow ) { - position = new Position( node, 0 ); + this.position = new Position( node, 0 ); } else { - position = position.getShiftedBy( 1 ); + this.position = this.position.getShiftedBy( 1 ); } - this.position = position; - - return this._formatReturnValue( 'elementStart', node, previousPosition, position, 1 ); + return this._formatReturnValue( 'elementStart', node, previousPosition, this.position, 1 ); } else if ( node instanceof Text ) { if ( this.singleCharacters ) { - position = new Position( node, 0 ); - this.position = position; + this.position = new Position( node, 0 ); return this._next(); } else { @@ -242,15 +238,13 @@ export default class TreeWalker { if ( node == this._boundaryEndParent ) { charactersCount = this.boundaries.end.offset; item = new TextProxy( node, 0, charactersCount ); - position = Position.createAfter( item ); + this.position = Position.createAfter( item ); } else { // If not just keep moving forward. - position = position.getShiftedBy( 1 ); + this.position = this.position.getShiftedBy( 1 ); } - this.position = position; - - return this._formatReturnValue( 'text', item, previousPosition, position, charactersCount ); + return this._formatReturnValue( 'text', item, previousPosition, this.position, charactersCount ); } } else if ( typeof node == 'string' ) { let textLength; @@ -261,25 +255,22 @@ export default class TreeWalker { // Check if text stick out of walker range. const endOffset = parent === this._boundaryEndParent ? this.boundaries.end.offset : parent.data.length; - textLength = endOffset - position.offset; + textLength = endOffset - this.position.offset; } - const textProxy = new TextProxy( parent, position.offset, textLength ); - - position = position.getShiftedBy( textLength ); + const textProxy = new TextProxy( parent, this.position.offset, textLength ); - this.position = position; + this.position = this.position.getShiftedBy( textLength ); - return this._formatReturnValue( 'text', textProxy, previousPosition, position, textLength ); + return this._formatReturnValue( 'text', textProxy, previousPosition, this.position, textLength ); } else { // `node` is not set, we reached the end of current `parent`. - position = Position.createAfter( parent ); - this.position = position; + this.position = Position.createAfter( parent ); if ( this.ignoreElementEnd ) { return this._next(); } else { - return this._formatReturnValue( 'elementEnd', parent, previousPosition, position ); + return this._formatReturnValue( 'elementEnd', parent, previousPosition, this.position ); } } } @@ -293,17 +284,16 @@ export default class TreeWalker { * @returns {module:engine/view/treewalker~TreeWalkerValue} return.value Information about taken step. */ _previous() { - let position = this.position; const previousPosition = this.position; - const parent = position.parent; + const parent = this.position.parent; // We are at the beginning of the root. - if ( parent.parent === null && position.offset === 0 ) { + if ( parent.parent === null && this.position.offset === 0 ) { return { done: true }; } // We reached the walker boundary. - if ( parent == this._boundaryStartParent && position.offset == this.boundaries.start.offset ) { + if ( parent == this._boundaryStartParent && this.position.offset == this.boundaries.start.offset ) { return { done: true }; } @@ -312,38 +302,35 @@ export default class TreeWalker { // Text {@link module:engine/view/text~Text} element is a specific parent because contains string instead of child nodes. if ( parent instanceof Text ) { - if ( position.isAtStart ) { + if ( this.position.isAtStart ) { // Prevent returning "elementStart" for Text node. Skip that value and return the next walker step. this.position = Position.createBefore( parent ); return this._previous(); } - node = parent.data[ position.offset - 1 ]; + node = parent.data[ this.position.offset - 1 ]; } else { - node = parent.getChild( position.offset - 1 ); + node = parent.getChild( this.position.offset - 1 ); } if ( node instanceof Element ) { if ( !this.shallow ) { - position = new Position( node, node.childCount ); - this.position = position; + this.position = new Position( node, node.childCount ); if ( this.ignoreElementEnd ) { return this._previous(); } else { - return this._formatReturnValue( 'elementEnd', node, previousPosition, position ); + return this._formatReturnValue( 'elementEnd', node, previousPosition, this.position ); } } else { - position = position.getShiftedBy( -1 ); - this.position = position; + this.position = this.position.getShiftedBy( -1 ); - return this._formatReturnValue( 'elementStart', node, previousPosition, position, 1 ); + return this._formatReturnValue( 'elementStart', node, previousPosition, this.position, 1 ); } } else if ( node instanceof Text ) { if ( this.singleCharacters ) { - position = new Position( node, node.data.length ); - this.position = position; + this.position = new Position( node, node.data.length ); return this._previous(); } else { @@ -356,15 +343,13 @@ export default class TreeWalker { item = new TextProxy( node, offset, node.data.length - offset ); charactersCount = item.data.length; - position = Position.createBefore( item ); + this.position = Position.createBefore( item ); } else { // If not just keep moving backward. - position = position.getShiftedBy( -1 ); + this.position = this.position.getShiftedBy( -1 ); } - this.position = position; - - return this._formatReturnValue( 'text', item, previousPosition, position, charactersCount ); + return this._formatReturnValue( 'text', item, previousPosition, this.position, charactersCount ); } } else if ( typeof node == 'string' ) { let textLength; @@ -373,24 +358,21 @@ export default class TreeWalker { // Check if text stick out of walker range. const startOffset = parent === this._boundaryStartParent ? this.boundaries.start.offset : 0; - textLength = position.offset - startOffset; + textLength = this.position.offset - startOffset; } else { textLength = 1; } - position = position.getShiftedBy( -textLength ); - - const textProxy = new TextProxy( parent, position.offset, textLength ); + this.position = this.position.getShiftedBy( -textLength ); - this.position = position; + const textProxy = new TextProxy( parent, this.position.offset, textLength ); - return this._formatReturnValue( 'text', textProxy, previousPosition, position, textLength ); + return this._formatReturnValue( 'text', textProxy, previousPosition, this.position, textLength ); } else { // `node` is not set, we reached the beginning of current `parent`. - position = Position.createBefore( parent ); - this.position = position; + this.position = Position.createBefore( parent ); - return this._formatReturnValue( 'elementStart', parent, previousPosition, position, 1 ); + return this._formatReturnValue( 'elementStart', parent, previousPosition, this.position, 1 ); } } From 3c496a5cf778639302a713422dce36b2b4999f56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Fri, 17 Nov 2017 11:50:16 +0100 Subject: [PATCH 14/16] Other: Remove redundant check from getOffset in Position. --- src/model/position.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/model/position.js b/src/model/position.js index ae2f83e2f..9aa9f1672 100644 --- a/src/model/position.js +++ b/src/model/position.js @@ -832,7 +832,7 @@ export default class Position { // @private // @param {Array.} path Position path. See {@link module:engine/model/position~Position#path}. function getOffset( path ) { - return last( path ) || 0; + return last( path ); } // Helper for setting offset on give path array. From 44553fe61e8f22f0eb76ecf55e585d7c04395cfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Mon, 20 Nov 2017 15:39:20 +0100 Subject: [PATCH 15/16] Other: Remove range copying from selection getRanges(), getFirstRange() and getLastRange() methods. --- src/model/selection.js | 16 +++++++--------- src/view/selection.js | 16 +++++++--------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/src/model/selection.js b/src/model/selection.js index d3fd43e84..07b30854e 100644 --- a/src/model/selection.js +++ b/src/model/selection.js @@ -174,18 +174,16 @@ export default class Selection { } /** - * Returns an iterator that iterates over copies of selection ranges. + * Returns an iterator that iterates over selection ranges. * * @returns {Iterator.} */ - * getRanges() { - for ( const range of this._ranges ) { - yield Range.createFromRange( range ); - } + getRanges() { + return this._ranges[ Symbol.iterator ](); } /** - * Returns a copy of the first range in the selection. + * Returns first range in the selection. * First range is the one which {@link module:engine/model/range~Range#start start} position * {@link module:engine/model/position~Position#isBefore is before} start position of all other ranges * (not to confuse with the first range added to the selection). @@ -203,11 +201,11 @@ export default class Selection { } } - return first ? Range.createFromRange( first ) : null; + return first; } /** - * Returns a copy of the last range in the selection. + * Returns last range in the selection. * Last range is the one which {@link module:engine/model/range~Range#end end} position * {@link module:engine/model/position~Position#isAfter is after} end position of all other ranges (not to confuse with the range most * recently added to the selection). @@ -225,7 +223,7 @@ export default class Selection { } } - return last ? Range.createFromRange( last ) : null; + return last; } /** diff --git a/src/view/selection.js b/src/view/selection.js index d48551259..41157f1db 100644 --- a/src/view/selection.js +++ b/src/view/selection.js @@ -220,18 +220,16 @@ export default class Selection { } /** - * Returns an iterator that contains copies of all ranges added to the selection. + * Returns an iterator that contains all ranges added to the selection. * * @returns {Iterator.} */ - * getRanges() { - for ( const range of this._ranges ) { - yield Range.createFromRange( range ); - } + getRanges() { + return this._ranges[ Symbol.iterator ](); } /** - * Returns copy of the first range in the selection. First range is the one which + * Returns first range in the selection. First range is the one which * {@link module:engine/view/range~Range#start start} position {@link module:engine/view/position~Position#isBefore is before} start * position of all other ranges (not to confuse with the first range added to the selection). * Returns `null` if no ranges are added to selection. @@ -247,11 +245,11 @@ export default class Selection { } } - return first ? Range.createFromRange( first ) : null; + return first; } /** - * Returns copy of the last range in the selection. Last range is the one which {@link module:engine/view/range~Range#end end} + * Returns last range in the selection. Last range is the one which {@link module:engine/view/range~Range#end end} * position {@link module:engine/view/position~Position#isAfter is after} end position of all other ranges (not to confuse * with the last range added to the selection). Returns `null` if no ranges are added to selection. * @@ -266,7 +264,7 @@ export default class Selection { } } - return last ? Range.createFromRange( last ) : null; + return last; } /** From a2db1b060c7ed4e59da4a5e61d9837f5e3c4fa86 Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Fri, 24 Nov 2017 14:12:54 +0100 Subject: [PATCH 16/16] Changed: Minor fixes. --- src/dev-utils/view.js | 3 +-- src/model/position.js | 24 ++++------------- src/model/range.js | 8 ++---- src/model/selection.js | 4 +-- src/model/treewalker.js | 57 ++++++++++++++--------------------------- src/view/position.js | 11 +++----- src/view/range.js | 11 +++----- 7 files changed, 37 insertions(+), 81 deletions(-) diff --git a/src/dev-utils/view.js b/src/dev-utils/view.js index a0e160186..5694203b2 100644 --- a/src/dev-utils/view.js +++ b/src/dev-utils/view.js @@ -541,8 +541,7 @@ class RangeParser { throw new Error( `Parse error - end of range was found '${ item.bracket }' but range was not started before.` ); } - // When second start of range is found when one is already opened - selection does not allow intersecting - // ranges. + // When second start of range is found when one is already opened - selection does not allow intersecting ranges. if ( range && ( item.bracket == ELEMENT_RANGE_START_TOKEN || item.bracket == TEXT_RANGE_START_TOKEN ) ) { throw new Error( `Parse error - start of range was found '${ item.bracket }' but one range is already started.` ); } diff --git a/src/model/position.js b/src/model/position.js index 9aa9f1672..ede7c577b 100644 --- a/src/model/position.js +++ b/src/model/position.js @@ -139,7 +139,7 @@ export default class Position { * @type {Number} */ get offset() { - return getOffset( this.path ); + return last( this.path ); } /** @@ -365,8 +365,7 @@ export default class Position { */ getShiftedTo( offset ) { const path = this.path.slice(); - - setOffset( path, offset ); + path[ path.length - 1 ] = offset; return new Position( this.root, path ); } @@ -678,7 +677,9 @@ export default class Position { // Then we have to update the rest of the path. // Fix the offset because this position might be after `from` position and we have to reflect that. - setOffset( combinedPath, getOffset( combinedPath ) + this.path[ i ] - source.offset ); + const oldOffset = last( combinedPath ); + const newOffset = oldOffset + this.path[ i ] - source.offset; + combinedPath[ combinedPath.length - 1 ] = newOffset; // Then, add the rest of the path. // If this position is at the same level as `from` position nothing will get added. @@ -828,21 +829,6 @@ export default class Position { } } -// Helper for setting offset on give path array. -// @private -// @param {Array.} path Position path. See {@link module:engine/model/position~Position#path}. -function getOffset( path ) { - return last( path ); -} - -// Helper for setting offset on give path array. -// @private -// @param {Array.} path Position path. See {@link module:engine/model/position~Position#path}. -// @param {Number} newOffset Offset to set. -function setOffset( path, newOffset ) { - path[ path.length - 1 ] = newOffset; -} - /** * A flag indicating whether this position is `'before'` or `'after'` or `'same'` as given position. * If positions are in different roots `'different'` flag is returned. diff --git a/src/model/range.js b/src/model/range.js index 8936706bb..d5b376cfe 100644 --- a/src/model/range.js +++ b/src/model/range.js @@ -311,11 +311,7 @@ export default class Range { ranges.push( new Range( pos, pos.getShiftedBy( howMany ) ) ); } - const path = pos.getParentPath(); - path[ path.length - 1 ]++; - - pos = new Position( pos.root, path ); - + pos = Position.createAfter( posParent ); posParent = posParent.parent; } @@ -844,7 +840,7 @@ export default class Range { // 5. Ranges should be checked and glued starting from the range that is closest to the reference range. // Since ranges are sorted, start with the range with index that is closest to reference range index. - for ( let i = refIndex - 1; i >= 0; i++ ) { + for ( let i = refIndex - 1; i >= 0; i-- ) { if ( ranges[ i ].end.isEqual( start ) ) { start = ranges[ i ].start; } else { diff --git a/src/model/selection.js b/src/model/selection.js index 07b30854e..6060a3af0 100644 --- a/src/model/selection.js +++ b/src/model/selection.js @@ -236,9 +236,9 @@ export default class Selection { * @returns {module:engine/model/position~Position|null} */ getFirstPosition() { - const first = this.getFirstRange(); + const firstRange = this.getFirstRange(); - return first ? first.start : null; + return firstRange ? firstRange.start : null; } /** diff --git a/src/model/treewalker.js b/src/model/treewalker.js index e1bb83d52..fe9415251 100644 --- a/src/model/treewalker.js +++ b/src/model/treewalker.js @@ -204,8 +204,6 @@ export default class TreeWalker { */ _next() { const previousPosition = this.position; - - let position = this.position; const parent = this._visitedParent; // We are at the end of the root. @@ -223,17 +221,15 @@ export default class TreeWalker { if ( node instanceof Element ) { if ( !this.shallow ) { // Manual operations on path internals for optimization purposes. Here and in the rest of the method. - const path = position.path.slice(); + const path = this.position.path.slice(); path.push( 0 ); - position = new Position( position.root, path ); + this.position = new Position( this.position.root, path ); this._visitedParent = node; } else { - position = position.getShiftedBy( 1 ); + this.position = this.position.getShiftedBy( 1 ); } - this.position = position; - return formatReturnValue( 'elementStart', node, previousPosition, this.position, 1 ); } else if ( node instanceof Text ) { let charactersCount; @@ -247,30 +243,24 @@ export default class TreeWalker { offset = this.boundaries.end.offset; } - charactersCount = offset - position.offset; + charactersCount = offset - this.position.offset; } - const offsetInTextNode = position.offset - node.startOffset; + const offsetInTextNode = this.position.offset - node.startOffset; const item = new TextProxy( node, offsetInTextNode, charactersCount ); - position = position.getShiftedBy( charactersCount ); - - this.position = position; + this.position = this.position.getShiftedBy( charactersCount ); return formatReturnValue( 'text', item, previousPosition, this.position, charactersCount ); } else { // `node` is not set, we reached the end of current `parent`. - const path = position.getParentPath(); - path[ path.length - 1 ]++; - position = new Position( position.root, path ); - - this.position = position; + this.position = Position.createAfter( parent ); this._visitedParent = parent.parent; if ( this.ignoreElementEnd ) { return this._next(); } else { - return formatReturnValue( 'elementEnd', parent, previousPosition, position ); + return formatReturnValue( 'elementEnd', parent, previousPosition, this.position ); } } } @@ -285,7 +275,6 @@ export default class TreeWalker { */ _previous() { const previousPosition = this.position; - let position = this.position; const parent = this._visitedParent; // We are at the beginning of the root. @@ -302,26 +291,22 @@ export default class TreeWalker { const node = this.position.textNode ? this.position.textNode : this.position.nodeBefore; if ( node instanceof Element ) { - position = position.getShiftedBy( -1 ); + this.position = this.position.getShiftedBy( -1 ); if ( !this.shallow ) { - const path = position.path.slice(); + const path = this.position.path.slice(); path.push( node.maxOffset ); - position = new Position( position.root, path ); - - this.position = position; + this.position = new Position( this.position.root, path ); this._visitedParent = node; if ( this.ignoreElementEnd ) { return this._previous(); } else { - return formatReturnValue( 'elementEnd', node, previousPosition, position ); + return formatReturnValue( 'elementEnd', node, previousPosition, this.position ); } } else { - this.position = position; - - return formatReturnValue( 'elementStart', node, previousPosition, position, 1 ); + return formatReturnValue( 'elementStart', node, previousPosition, this.position, 1 ); } } else if ( node instanceof Text ) { let charactersCount; @@ -335,25 +320,21 @@ export default class TreeWalker { offset = this.boundaries.start.offset; } - charactersCount = position.offset - offset; + charactersCount = this.position.offset - offset; } - const offsetInTextNode = position.offset - node.startOffset; + const offsetInTextNode = this.position.offset - node.startOffset; const item = new TextProxy( node, offsetInTextNode - charactersCount, charactersCount ); - position = position.getShiftedBy( -charactersCount ); - - this.position = position; + this.position = this.position.getShiftedBy( -charactersCount ); - return formatReturnValue( 'text', item, previousPosition, position, charactersCount ); + return formatReturnValue( 'text', item, previousPosition, this.position, charactersCount ); } else { // `node` is not set, we reached the beginning of current `parent`. - position = new Position( position.root, position.getParentPath() ); - - this.position = position; + this.position = Position.createBefore( parent ); this._visitedParent = parent.parent; - return formatReturnValue( 'elementStart', parent, previousPosition, position, 1 ); + return formatReturnValue( 'elementStart', parent, previousPosition, this.position, 1 ); } } } diff --git a/src/view/position.js b/src/view/position.js index e72004016..67d8bf91b 100644 --- a/src/view/position.js +++ b/src/view/position.js @@ -13,9 +13,6 @@ import compareArrays from '@ckeditor/ckeditor5-utils/src/comparearrays'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; import EditableElement from './editableelement'; -const _parent = Symbol( 'parent' ); -const _offset = Symbol( 'offset' ); - /** * Position in the tree. Position is always located before or after a node. */ @@ -27,8 +24,8 @@ export default class Position { * @param {Number} offset Position offset. */ constructor( parent, offset ) { - this[ _parent ] = parent; - this[ _offset ] = offset; + this._parent = parent; + this._offset = offset; } /** @@ -38,7 +35,7 @@ export default class Position { * @type {module:engine/view/node~Node|module:engine/view/documentfragment~DocumentFragment} */ get parent() { - return this[ _parent ]; + return this._parent; } /** @@ -48,7 +45,7 @@ export default class Position { * @type {Number} */ get offset() { - return this[ _offset ]; + return this._offset; } /** diff --git a/src/view/range.js b/src/view/range.js index 4098a9589..32e573e1c 100644 --- a/src/view/range.js +++ b/src/view/range.js @@ -10,9 +10,6 @@ import Position from './position'; import TreeWalker from './treewalker'; -const _start = Symbol( 'start' ); -const _end = Symbol( 'end' ); - /** * Tree view range. */ @@ -26,8 +23,8 @@ export default class Range { * @param {module:engine/view/position~Position} [end] End position. If not set, range will be collapsed at `start` position. */ constructor( start, end = null ) { - this[ _start ] = start; - this[ _end ] = end ? end : start; + this._start = start; + this._end = end ? end : start; } /** @@ -52,7 +49,7 @@ export default class Range { * @type {module:engine/view/position~Position} */ get start() { - return this[ _start ]; + return this._start; } /** @@ -62,7 +59,7 @@ export default class Range { * @type {module:engine/view/position~Position} */ get end() { - return this[ _end ]; + return this._end; } /**