From f4ba06f9d37fa6fe9d5e5b97eaa70da27576cf55 Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Sat, 24 Jun 2017 17:49:37 +0200 Subject: [PATCH] Fixed: Remove and reinsert change types needs fixes during range transformations since `RemoveOperation` no longer creates `graveyardHolder`. --- src/model/liverange.js | 4 +- src/model/range.js | 87 ++++++++++++++++++++++-------------------- 2 files changed, 47 insertions(+), 44 deletions(-) diff --git a/src/model/liverange.js b/src/model/liverange.js index 2822c542b..6549eee02 100644 --- a/src/model/liverange.js +++ b/src/model/liverange.js @@ -129,7 +129,7 @@ function transform( changeType, deltaType, targetRange, sourcePosition ) { const howMany = targetRange.end.offset - targetRange.start.offset; let targetPosition = targetRange.start; - if ( changeType == 'move' ) { + if ( changeType == 'move' || changeType == 'remove' || changeType == 'reinsert' ) { // Range._getTransformedByDocumentChange is expecting `targetPosition` to be "before" move // (before transformation). `targetRange.start` is already after the move happened. // We have to revert `targetPosition` to the state before the move. @@ -142,7 +142,7 @@ function transform( changeType, deltaType, targetRange, sourcePosition ) { // // First, this concerns only `move` change, because insert change includes inserted part always (changeType == 'move'). // Second, this is a case only if moved range was intersecting with this range and was inserted into this range (result.length == 3). - if ( changeType == 'move' && result.length == 3 ) { + if ( ( changeType == 'move' || changeType == 'remove' || changeType == 'reinsert' ) && result.length == 3 ) { // `result[ 2 ]` is a "common part" of this range and moved range. We substitute that common part with the whole // `targetRange` because we want to include whole `targetRange` in this range. result[ 2 ] = targetRange; diff --git a/src/model/range.js b/src/model/range.js index d2dd8ab47..1aaed7a02 100644 --- a/src/model/range.js +++ b/src/model/range.js @@ -462,6 +462,7 @@ export default class Range { } else { const sourceRange = Range.createFromPositionAndShift( sourcePosition, howMany ); + // Edge case for merge detla. if ( deltaType == 'merge' && this.isCollapsed && @@ -471,50 +472,52 @@ export default class Range { // Without fix, the range would end up in the graveyard, together with removed element. //

foo

[]bar

->

foobar

[]

->

foobar

->

foo[]bar

return [ new Range( targetPosition.getShiftedBy( this.start.offset ) ) ]; - } else if ( type == 'move' ) { - // In all examples `[]` is `this` and `{}` is `sourceRange`, while `^` is move target position. - // - // Example: - //

xx

^{

a[b

}

c]d

-->

xx

a[b

c]d

- // ^

xx

{

a[b

}

c]d

-->

a[b

xx

c]d

// Note

xx

inclusion. - // {

a[b

}^

c]d

-->

a[b

c]d

- if ( - sourceRange.containsPosition( this.start ) && - this.containsPosition( sourceRange.end ) && - this.end.isAfter( targetPosition ) - ) { - const start = this.start._getCombined( - sourcePosition, - targetPosition._getTransformedByDeletion( sourcePosition, howMany ) - ); - const end = this.end._getTransformedByMove( sourcePosition, targetPosition, howMany, false, false ); - - return [ new Range( start, end ) ]; - } + } + // + // Other edge cases: + // + // In all examples `[]` is `this` and `{}` is `sourceRange`, while `^` is move target position. + // + // Example: + //

xx

^{

a[b

}

c]d

-->

xx

a[b

c]d

+ // ^

xx

{

a[b

}

c]d

-->

a[b

xx

c]d

// Note

xx

inclusion. + // {

a[b

}^

c]d

-->

a[b

c]d

+ if ( + sourceRange.containsPosition( this.start ) && + this.containsPosition( sourceRange.end ) && + this.end.isAfter( targetPosition ) + ) { + const start = this.start._getCombined( + sourcePosition, + targetPosition._getTransformedByDeletion( sourcePosition, howMany ) + ); + const end = this.end._getTransformedByMove( sourcePosition, targetPosition, howMany, false, false ); - // Example: - //

c[d

{

a]b

}^

xx

-->

c[d

a]b

xx

- //

c[d

{

a]b

}

xx

^ -->

c[d

xx

a]b

// Note

xx

inclusion. - //

c[d

^{

a]b

} -->

c[d

a]b

- if ( - sourceRange.containsPosition( this.end ) && - this.containsPosition( sourceRange.start ) && - this.start.isBefore( targetPosition ) - ) { - const start = this.start._getTransformedByMove( - sourcePosition, - targetPosition, - howMany, - true, - false - ); - const end = this.end._getCombined( - sourcePosition, - targetPosition._getTransformedByDeletion( sourcePosition, howMany ) - ); + return [ new Range( start, end ) ]; + } - return [ new Range( start, end ) ]; - } + // Example: + //

c[d

{

a]b

}^

xx

-->

c[d

a]b

xx

+ //

c[d

{

a]b

}

xx

^ -->

c[d

xx

a]b

// Note

xx

inclusion. + //

c[d

^{

a]b

} -->

c[d

a]b

+ if ( + sourceRange.containsPosition( this.end ) && + this.containsPosition( sourceRange.start ) && + this.start.isBefore( targetPosition ) + ) { + const start = this.start._getTransformedByMove( + sourcePosition, + targetPosition, + howMany, + true, + false + ); + const end = this.end._getCombined( + sourcePosition, + targetPosition._getTransformedByDeletion( sourcePosition, howMany ) + ); + + return [ new Range( start, end ) ]; } return this._getTransformedByMove( sourcePosition, targetPosition, howMany );