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

Changed: removed deleting deltas from model.History, added marking deltas as "undo pairs".

Szymon Cofalik 8 лет назад
Родитель
Сommit
5182ec9abd

+ 58 - 78
packages/ckeditor5-engine/src/model/history.js

@@ -10,14 +10,7 @@
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 
 /**
- * `History` keeps the track of all the deltas applied to the {@link module:engine/model/document~Document document}. Deltas stored in
- * `History` might get updated, split into more deltas or even removed. This is used mostly to compress history, instead
- * of keeping all deltas in a state in which they were applied.
- *
- * **Note:** deltas kept in `History` should be used only to transform deltas. It's not advised to use `History` to get
- * original delta basing on it's {@link module:engine/model/delta/delta~Delta#baseVersion baseVersion}. Also, after transforming a
- * delta by deltas from `History`,
- * fix it's base version accordingly (set it to {@link module:engine/model/document~Document#version document version}).
+ * `History` keeps the track of all the deltas applied to the {@link module:engine/model/document~Document document}.
  */
 export default class History {
 	/**
@@ -40,6 +33,26 @@ export default class History {
 		 * @member {Map} module:engine/model/history~History#_historyPoints
 		 */
 		this._historyPoints = new Map();
+
+		/**
+		 * Holds an information which {@link module:engine/model/delta/delta~Delta delta} undoes which
+		 * {@link module:engine/model/delta/delta~Delta delta}.
+		 *
+		 * Keys of the map are "undoing deltas", that is deltas that undone some other deltas. For each key, the
+		 * value is a delta that has been undone by the "undoing delta".
+		 *
+		 * @private
+		 * @member {Map} module:engine/model/history~History#_undoPairs
+		 */
+		this._undoPairs = new Map();
+
+		/**
+		 * Holds all undone deltas.
+		 *
+		 * @private
+		 * @member {Set.<module:engine/model/delta/delta~Delta>} module:engine/model/history~History#_undoneDeltas
+		 */
+		this._undoneDeltas = new Set();
 	}
 
 	/**
@@ -63,7 +76,7 @@ export default class History {
 	 * that deltas from the first one will be returned.
 	 * @param {Number} [to=Number.POSITIVE_INFINITY] Base version up to which deltas should be returned (exclusive).
 	 * Defaults to `Number.POSITIVE_INFINITY` which means that deltas up to the last one will be returned.
-	 * @returns {Iterator.<module:engine/model/delta/delta~Delta>} Deltas added to the history.
+	 * @returns {Iterator.<module:engine/model/delta/delta~Delta>} Deltas added to the history from given base versions range.
 	 */
 	* getDeltas( from = 0, to = Number.POSITIVE_INFINITY ) {
 		// No deltas added, nothing to yield.
@@ -92,90 +105,58 @@ export default class History {
 	}
 
 	/**
-	 * Returns one or more deltas from history that bases on given `baseVersion`. Most often it will be just
-	 * one delta, but if that delta got updated by multiple deltas, all of those updated deltas will be returned.
+	 * Returns delta from history that bases on given `baseVersion`.
 	 *
-	 * @see module:engine/model/history~History#updateDelta
-	 * @param {Number} baseVersion Base version of the delta to retrieve.
-	 * @returns {Array.<module:engine/model/delta/delta~Delta>|null} Delta with given base version or null if no such delta is in history.
+	 * @param {Number} baseVersion Base version of the delta to get.
+	 * @returns {module:engine/model/delta/delta~Delta|null} Delta with given base version or `null` if there is no such delta in history.
 	 */
 	getDelta( baseVersion ) {
-		let index = this._historyPoints.get( baseVersion );
-
-		if ( index === undefined ) {
-			return null;
-		}
-
-		const deltas = [];
-
-		for ( index; index < this._deltas.length; index++ ) {
-			const delta = this._deltas[ index ];
-
-			if ( delta.baseVersion != baseVersion ) {
-				break;
-			}
-
-			deltas.push( delta );
-		}
+		const index = this._historyPoints.get( baseVersion );
 
-		return deltas.length === 0 ? null : deltas;
+		return index === undefined ? null : this._deltas[ index ];
 	}
 
 	/**
-	 * Removes delta from the history. This happens i.e., when a delta is undone by another delta. Both undone delta and
-	 * undoing delta should be removed so they won't have an impact on transforming other deltas.
-	 *
-	 * **Note:** using this method does not change the state of {@link module:engine/model/document~Document model}. It just affects
-	 * the state of `History`.
+	 * Marks in history that one delta is a delta that is undoing the other delta. By marking deltas this way,
+	 * history is keeping more context information about deltas which helps in operational transformation.
 	 *
-	 * **Note:** when some deltas are removed, deltas between them should probably get updated. See
-	 * {@link module:engine/model/history~History#updateDelta}.
-	 *
-	 * **Note:** if delta with `baseVersion` got {@link module:engine/model/history~History#updateDelta updated} by multiple
-	 * deltas, all updated deltas will be removed.
-	 *
-	 * @param {Number} baseVersion Base version of a delta to be removed.
+	 * @param {module:engine/model/delta/delta~Delta} undoneDelta Delta which is undone by `undoingDelta`.
+	 * @param {module:engine/model/delta/delta~Delta} undoingDelta Delta which undoes `undoneDelta`.
 	 */
-	removeDelta( baseVersion ) {
-		this.updateDelta( baseVersion, [] );
+	setDeltaAsUndone( undoneDelta, undoingDelta ) {
+		this._undoPairs.set( undoingDelta, undoneDelta );
+		this._undoneDeltas.add( undoneDelta );
 	}
 
 	/**
-	 * Substitutes delta in history by one or more given deltas.
-	 *
-	 * **Note:** if delta with `baseVersion` was already updated by multiple deltas, all updated deltas will be removed
-	 * and new deltas will be inserted at their position.
+	 * Checks whether given `delta` is undoing any other delta.
 	 *
-	 * **Note:** removed delta won't get updated.
-	 *
-	 * @param {Number} baseVersion Base version of a delta to update.
-	 * @param {Iterable.<module:engine/model/delta/delta~Delta>} updatedDeltas Deltas to be inserted in place of updated delta.
+	 * @param {module:engine/model/delta/delta~Delta} delta Delta to check.
+	 * @returns {Boolean} `true` if given `delta` is undoing any other delta, `false` otherwise.
 	 */
-	updateDelta( baseVersion, updatedDeltas ) {
-		const deltas = this.getDelta( baseVersion );
-
-		// If there are no deltas, stop executing function as there is nothing to update.
-		if ( deltas === null ) {
-			return;
-		}
-
-		// Make sure that every updated delta has correct `baseVersion`.
-		// This is crucial for algorithms in `History` and algorithms using `History`.
-		for ( const delta of updatedDeltas ) {
-			delta.baseVersion = baseVersion;
-		}
-
-		// Put updated deltas in place of old deltas.
-		this._deltas.splice( this._getIndex( baseVersion ), deltas.length, ...updatedDeltas );
+	isUndoingDelta( delta ) {
+		return this._undoPairs.has( delta );
+	}
 
-		// Update history points.
-		const changeBy = updatedDeltas.length - deltas.length;
+	/**
+	 * Checks whether given `delta` has been undone by any other delta.
+	 *
+	 * @param {module:engine/model/delta/delta~Delta} delta Delta to check.
+	 * @returns {Boolean} `true` if given `delta` has been undone any other delta, `false` otherwise.
+	 */
+	isUndoneDelta( delta ) {
+		return this._undoneDeltas.has( delta );
+	}
 
-		for ( const key of this._historyPoints.keys() ) {
-			if ( key > baseVersion ) {
-				this._historyPoints.set( key, this._historyPoints.get( key ) + changeBy );
-			}
-		}
+	/**
+	 * For given `undoingDelta`, returns the delta which has been undone by it.
+	 *
+	 * @param {module:engine/model/delta/delta~Delta} undoingDelta
+	 * @returns {module:engine/model/delta/delta~Delta|undefined} Delta that has been undone by given `undoingDelta` or `undefined`
+	 * if given `undoingDelta` is not undoing any other delta.
+	 */
+	getUndoneDelta( undoingDelta ) {
+		return this._undoPairs.get( undoingDelta );
 	}
 
 	/**
@@ -194,7 +175,6 @@ export default class History {
 
 			if ( baseVersion < 0 || baseVersion >= nextBaseVersion ) {
 				// Base version is too high or too low - it's acceptable situation.
-				// Return -1 because `baseVersion` was correct.
 				return -1;
 			}
 

+ 50 - 70
packages/ckeditor5-engine/tests/model/history.js

@@ -67,23 +67,12 @@ describe( 'History', () => {
 	} );
 
 	describe( 'getDelta', () => {
-		it( 'should return array with one delta with given base version', () => {
+		it( 'should return delta with given base version', () => {
 			const delta = getDelta( 0 );
 			history.addDelta( delta );
 
 			const historyDelta = history.getDelta( 0 );
-			expect( historyDelta ).to.deep.equal( [ delta ] );
-		} );
-
-		it( 'should return array with all updated deltas of delta with given base version', () => {
-			const delta = getDelta( 0 );
-			history.addDelta( delta );
-
-			const deltas = getDeltaSet();
-			history.updateDelta( 0, deltas );
-
-			const historyDelta = history.getDelta( 0 );
-			expect( historyDelta ).to.deep.equal( deltas );
+			expect( historyDelta ).to.equal( delta );
 		} );
 
 		it( 'should return null if delta has not been found in history', () => {
@@ -91,14 +80,6 @@ describe( 'History', () => {
 			expect( history.getDelta( 2 ) ).to.be.null;
 			expect( history.getDelta( 20 ) ).to.be.null;
 		} );
-
-		it( 'should return null if delta has been removed by removeDelta', () => {
-			const delta = getDelta( 0 );
-			history.addDelta( delta );
-			history.removeDelta( 0 );
-
-			expect( history.getDelta( 0 ) ).to.be.null;
-		} );
 	} );
 
 	describe( 'getDeltas', () => {
@@ -134,79 +115,78 @@ describe( 'History', () => {
 		} );
 	} );
 
-	describe( 'updateDelta', () => {
-		it( 'should substitute delta from history by given deltas', () => {
-			history.addDelta( getDelta( 0 ) );
+	describe( 'isUndoingDelta', () => {
+		let undoing, undone;
 
-			const deltas = getDeltaSet();
-			history.updateDelta( 0, deltas );
+		beforeEach( () => {
+			undoing = new Delta();
+			undone = new Delta();
 
-			const historyDeltas = Array.from( history.getDeltas() );
-			expect( historyDeltas ).to.deep.equal( deltas );
-		} );
+			history.addDelta( undone );
+			history.addDelta( undoing );
 
-		it( 'should substitute all updated deltas by new deltas', () => {
-			history.addDelta( getDelta( 0 ) );
+			history.setDeltaAsUndone( undone, undoing );
+		} );
 
-			// Change original single delta to three deltas generated by `getDeltaSet`.
-			// All those deltas should now be seen under base version 0.
-			history.updateDelta( 0, getDeltaSet() );
+		it( 'should return true if delta is an undoing delta', () => {
+			expect( history.isUndoingDelta( undoing ) ).to.be.true;
+		} );
 
-			const deltas = getDeltaSet();
-			// Change all three deltas from base version 0 to new set of deltas.
-			history.updateDelta( 0, deltas );
+		it( 'should return false if delta is not an undoing delta', () => {
+			const delta = new Delta();
 
-			const historyDeltas = Array.from( history.getDeltas() );
-			expect( historyDeltas ).to.deep.equal( deltas );
+			expect( history.isUndoingDelta( undone ) ).to.be.false;
+			expect( history.isUndoingDelta( delta ) ).to.be.false;
 		} );
+	} );
 
-		it( 'should do nothing if deltas for given base version has not been found in history', () => {
-			history.addDelta( getDelta( 0 ) );
-			history.removeDelta( 0 );
+	describe( 'isUndoneDelta', () => {
+		let undoing, undone;
 
-			const deltas = getDeltaSet();
+		beforeEach( () => {
+			undoing = new Delta();
+			undone = new Delta();
 
-			history.updateDelta( 0, deltas );
+			history.addDelta( undone );
+			history.addDelta( undoing );
 
-			expect( Array.from( history.getDeltas() ).length ).to.equal( 0 );
+			history.setDeltaAsUndone( undone, undoing );
 		} );
-	} );
 
-	describe( 'removeDelta', () => {
-		it( 'should remove deltas that do not have graveyard related operations', () => {
-			for ( const delta of getDeltaSet() ) {
-				history.addDelta( delta );
-			}
+		it( 'should return true if delta has been set as undone', () => {
+			expect( history.isUndoneDelta( undone ) ).to.be.true;
+		} );
 
-			history.removeDelta( 3 );
+		it( 'should return false if delta has not been set as undone', () => {
+			const delta = new Delta();
 
-			const deltas = Array.from( history.getDeltas() );
-			expect( deltas.length ).to.equal( 2 );
+			expect( history.isUndoneDelta( undoing ) ).to.be.false;
+			expect( history.isUndoneDelta( delta ) ).to.be.false;
 		} );
+	} );
 
-		it( 'should remove multiple updated deltas', () => {
-			const delta = getDelta( 0 );
-			history.addDelta( delta );
+	describe( 'getUndoneDelta', () => {
+		let undoing, undone;
 
-			const updatedDeltas = getDeltaSet( 0 );
+		beforeEach( () => {
+			undoing = new Delta();
+			undone = new Delta();
 
-			history.updateDelta( 0, updatedDeltas );
-			history.removeDelta( 0 );
+			history.addDelta( undone );
+			history.addDelta( undoing );
 
-			const deltas = Array.from( history.getDeltas() );
-			expect( deltas.length ).to.equal( 0 );
+			history.setDeltaAsUndone( undone, undoing );
 		} );
 
-		it( 'should do nothing if deltas for given base version has not been found in history', () => {
-			const deltas = getDeltaSet();
-
-			for ( const delta of deltas ) {
-				history.addDelta( delta );
-			}
+		it( 'should return undone delta basing on undoing delta', () => {
+			expect( history.getUndoneDelta( undoing ) ).to.equal( undone );
+		} );
 
-			history.removeDelta( 12 );
+		it( 'should return undefined if given delta is not an undoing delta', () => {
+			const delta = new Delta();
 
-			expect( Array.from( history.getDeltas() ) ).to.deep.equal( deltas );
+			expect( history.getUndoneDelta( undone ) ).to.be.undefined;
+			expect( history.getUndoneDelta( delta ) ).to.be.undefined;
 		} );
 	} );
 } );