Merge pull request #1509 from ckeditor/t/new-ot-docs

Docs: Updated OT documentation. Closes #1465.

src/dev-utils/enableenginedebug.js

@@ -84,7 +84,7 @@ const treeDump = Symbol( '_treeDump' );
 // Maximum number of stored states of model and view document.
 const maxTreeDumpLength = 20;
 
-// Separator used to separate stringified operations
+// Separator used to separate stringified operations.
 const LOG_SEPARATOR = '-------';
 
 // Specified whether debug tools were already enabled.

src/dev-utils/operationreplayer.js

@@ -4,7 +4,7 @@
  */
 
 /**
- * @module engine/dev-utils/replayer
+ * @module engine/dev-utils/operationreplayer
  */
 
 /* global setTimeout */

src/model/differ.js

@@ -119,6 +119,10 @@ export default class Differ {
 	 * @param {module:engine/model/operation/operation~Operation} operation An operation to buffer.
 	 */
 	bufferOperation( operation ) {
+		// Below we take an operation, check its type, then use its parameters in marking (private) methods.
+		// The general rule is to not mark elements inside inserted element. All inserted elements are re-rendered.
+		// Marking changes in them would cause a "double" changing then.
+		//
 		switch ( operation.type ) {
 			case 'insert': {
 				if ( this._isInInsertedElement( operation.position.parent ) ) {

src/model/liveposition.js

@@ -34,8 +34,7 @@ export default class LivePosition extends Position {
 	 * @see module:engine/model/position~Position
 	 * @param {module:engine/model/rootelement~RootElement} root
 	 * @param {Array.<Number>} path
-	 * @param {module:engine/model/position~PositionStickiness} [stickiness] Position stickiness. Defaults to `'toNone'`.
-	 * See {@link module:engine/model/liveposition~LivePosition#stickiness}.
+	 * @param {module:engine/model/position~PositionStickiness} [stickiness]
 	 */
 	constructor( root, path, stickiness = 'toNone' ) {
 		super( root, path, stickiness );
@@ -51,12 +50,6 @@ export default class LivePosition extends Position {
 			);
 		}
 
-		/**
-		 * Position stickiness. See {@link module:engine/model/position~PositionStickiness}.
-		 *
-		 * @member {module:engine/model/position~PositionStickiness} module:engine/model/liveposition~LivePosition#stickiness
-		 */
-
 		bindWithDocument.call( this );
 	}
 
@@ -110,14 +103,10 @@ export default class LivePosition extends Position {
 	 */
 }
 
-/**
- * Binds this `LivePosition` to the {@link module:engine/model/document~Document document} that owns
- * this position's {@link module:engine/model/position~Position#root root}.
- *
- * @ignore
- * @private
- * @method module:engine/model/liveposition~LivePosition.bindWithDocument
- */
+// Binds this `LivePosition` to the {@link module:engine/model/document~Document document} that owns
+// this position's {@link module:engine/model/position~Position#root root}.
+//
+// @private
 function bindWithDocument() {
 	this.listenTo(
 		this.root.document.model,
@@ -135,14 +124,10 @@ function bindWithDocument() {
 	);
 }
 
-/**
- * Updates this position accordingly to the updates applied to the model. Bases on change events.
- *
- * @ignore
- * @private
- * @method transform
- * @param {module:engine/model/operation/operation~Operation} operation Executed operation.
- */
+// Updates this position accordingly to the updates applied to the model. Bases on change events.
+//
+// @private
+// @param {module:engine/model/operation/operation~Operation} operation Executed operation.
 function transform( operation ) {
 	const result = this.getTransformedByOperation( operation );
 

src/model/liverange.js

@@ -83,10 +83,9 @@ export default class LiveRange extends Range {
 	 * @event change:range
 	 * @param {module:engine/model/range~Range} oldRange Range with start and end position equal to start and end position of this live
 	 * range before it got changed.
-	 * @param {Object} data Object with additional information about the change. Those parameters are passed from
-	 * {@link module:engine/model/document~Document#event:change document change event}.
-	 * @param {String} data.type Change type.
-	 * @param {module:engine/model/position~Position|null} deletionPosition Source position for move, remove and reinsert change types.
+	 * @param {Object} data Object with additional information about the change.
+	 * @param {module:engine/model/position~Position|null} data.deletionPosition Source position for remove and merge changes.
+	 * Available if the range was moved to the graveyard root, `null` otherwise.
 	 */
 
 	/**
@@ -96,23 +95,16 @@ export default class LiveRange extends Range {
 	 * @event change:content
 	 * @param {module:engine/model/range~Range} range Range with start and end position equal to start and end position of
 	 * change range.
-	 * @param {Object} data Object with additional information about the change. Those parameters are passed from
-	 * {@link module:engine/model/document~Document#event:change document change event}.
-	 * @param {String} data.type Change type.
-	 * @param {module:engine/model/batch~Batch} data.batch Batch which changed the live range.
-	 * @param {module:engine/model/range~Range} data.range Range containing the result of applied change.
-	 * @param {module:engine/model/position~Position} data.sourcePosition Source position for move, remove and reinsert change types.
+	 * @param {Object} data Object with additional information about the change.
+	 * @param {null} data.deletionPosition Due to the nature of this event, this property is always set to `null`. It is passed
+	 * for compatibility with the {@link module:engine/model/liverange~LiveRange#event:change:range} event.
 	 */
 }
 
-/**
- * Binds this `LiveRange` to the {@link module:engine/model/document~Document document}
- * that owns this range's {@link module:engine/model/range~Range#root root}.
- *
- * @ignore
- * @private
- * @method module:engine/model/liverange~LiveRange#bindWithDocument
- */
+// Binds this `LiveRange` to the {@link module:engine/model/document~Document document}
+// that owns this range's {@link module:engine/model/range~Range#root root}.
+//
+// @private
 function bindWithDocument() {
 	this.listenTo(
 		this.root.document.model,
@@ -130,23 +122,25 @@ function bindWithDocument() {
 	);
 }
 
-/**
- * Updates this range accordingly to the updates applied to the model. Bases on change events.
- *
- * @ignore
- * @private
- * @method transform
- * @param {module:engine/model/operation/operation~Operation} operation Executed operation.
- */
+// Updates this range accordingly to the updates applied to the model. Bases on change events.
+//
+// @private
+// @param {module:engine/model/operation/operation~Operation} operation Executed operation.
 function transform( operation ) {
+	// Transform the range by the operation. Join the result ranges if needed.
 	const ranges = this.getTransformedByOperation( operation );
 	const result = Range.createFromRanges( ranges );
+
 	const boundariesChanged = !result.isEqual( this );
 	const contentChanged = doesOperationChangeRangeContent( this, operation );
+
 	let deletionPosition = null;
 
 	if ( boundariesChanged ) {
+		// If range boundaries have changed, fire `change:range` event.
+		//
 		if ( result.root.rootName == '$graveyard' ) {
+			// If the range was moved to the graveyard root, set `deletionPosition`.
 			if ( operation.type == 'remove' ) {
 				deletionPosition = operation.sourcePosition;
 			} else {
@@ -155,7 +149,6 @@ function transform( operation ) {
 			}
 		}
 
-		// If range boundaries have changed, fire `change:range` event.
 		const oldRange = Range.createFromRange( this );
 
 		this.start = result.start;
@@ -168,6 +161,12 @@ function transform( operation ) {
 	}
 }
 
+// Checks whether given operation changes something inside the range (even if it does not change boundaries).
+//
+// @private
+// @param {module:engine/model/range~Range} range Range to check.
+// @param {module:engine/model/operation/operation~Operation} operation Executed operation.
+// @returns {Boolean}
 function doesOperationChangeRangeContent( range, operation ) {
 	switch ( operation.type ) {
 		case 'insert':

src/model/operation/insertoperation.js

@@ -51,8 +51,8 @@ export default class InsertOperation extends Operation {
 
 		/**
 		 * Flag deciding how the operation should be transformed. If set to `true`, nodes might get additional attributes
-		 * during operational transformation. This happens, when operation insertion position points to inside a
-		 * range which attributes have changed.
+		 * during operational transformation. This happens when the operation insertion position is inside of a range
+		 * where attributes have changed.
 		 *
 		 * @member {Boolean} module:engine/model/operation/insertoperation~InsertOperation#shouldReceiveAttributes
 		 */
@@ -66,6 +66,11 @@ export default class InsertOperation extends Operation {
 		return 'insert';
 	}
 
+	/**
+	 * Total offset size of inserted nodes.
+	 *
+	 * @returns {Number}
+	 */
 	get howMany() {
 		return this.nodes.maxOffset;
 	}

src/model/operation/mergeoperation.js

@@ -16,11 +16,12 @@ import { _move } from './utils';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 
 /**
- * Operation to merge two {@link module:engine/model/element~Element elements}. The merged elements are a parent of given
- * {@link module:engine/model/position~Position position} and the next element.
+ * Operation to merge two {@link module:engine/model/element~Element elements}.
  *
- * Technically, the content of the next element is moved at given {@link module:engine/model/position~Position position}
- * and the element is removed.
+ * The merged element is the parent of {@link ~MergeOperation#sourcePosition} and it is merged into the parent of
+ * {@link ~MergeOperation#targetPosition}. All nodes from the merged element are moved to {@link ~MergeOperation#targetPosition}.
+ *
+ * The merged element is moved to the graveyard at {@link ~MergeOperation#graveyardPosition}.
  *
  * @extends module:engine/model/operation/operation~Operation
  */
@@ -45,20 +46,32 @@ export default class MergeOperation extends Operation {
 		 * @member {module:engine/model/position~Position} module:engine/model/operation/mergeoperation~MergeOperation#sourcePosition
 		 */
 		this.sourcePosition = Position.createFromPosition( sourcePosition );
-		this.sourcePosition.stickiness = 'toPrevious'; // This is, and should always remain, the first position in its parent.
+		// This is, and should always remain, the first position in its parent.
+		this.sourcePosition.stickiness = 'toPrevious';
+
+		/**
+		 * Summary offset size of nodes which will be moved from the merged element to the new parent.
+		 *
+		 * @member {Number} module:engine/model/operation/mergeoperation~MergeOperation#howMany
+		 */
+		this.howMany = howMany;
 
 		/**
 		 * Position which the nodes from the merged elements will be moved to.
 		 *
 		 * @member {module:engine/model/position~Position} module:engine/model/operation/mergeoperation~MergeOperation#targetPosition
 		 */
 		this.targetPosition = Position.createFromPosition( targetPosition );
-		this.targetPosition.stickiness = 'toNext'; // This is, and should always remain, the last position in its parent.
-		// is it? think about reversed split operations, undo, etc.
+		// Except of a rare scenario in `MergeOperation` x `MergeOperation` transformation,
+		// this is, and should always remain, the last position in its parent.
+		this.targetPosition.stickiness = 'toNext';
 
+		/**
+		 * Position in graveyard to which the merged element will be moved.
+		 *
+		 * @member {module:engine/model/position~Position} module:engine/model/operation/mergeoperation~MergeOperation#graveyardPosition
+		 */
 		this.graveyardPosition = Position.createFromPosition( graveyardPosition );
-
-		this.howMany = howMany;
 	}
 
 	/**
@@ -69,7 +82,7 @@ export default class MergeOperation extends Operation {
 	}
 
 	/**
-	 * Position before the merged element (which will be removed). Calculated based on the split position.
+	 * Position before the merged element (which will be deleted).
 	 *
 	 * @readonly
 	 * @type {module:engine/model/position~Position}
@@ -79,8 +92,8 @@ export default class MergeOperation extends Operation {
 	}
 
 	/**
-	 * Artificial range that contains all the nodes from the merged element that will be moved to {@link ~#sourcePosition}.
-	 * The range starts at {@link ~#sourcePosition} and ends in the same parent, at `POSITIVE_INFINITY` offset.
+	 * Artificial range that contains all the nodes from the merged element that will be moved to {@link ~MergeOperation#sourcePosition}.
+	 * The range starts at {@link ~MergeOperation#sourcePosition} and ends in the same parent, at `POSITIVE_INFINITY` offset.
 	 *
 	 * @readonly
 	 * @type {module:engine/model/range~Range}

src/model/operation/moveoperation.js

@@ -42,7 +42,6 @@ export default class MoveOperation extends Operation {
 		 */
 		this.sourcePosition = Position.createFromPosition( sourcePosition );
 		this.sourcePosition.stickiness = 'toNext';
-		// maybe lets change sourcePosition + howMany to a range? flattness will be guaranteed by writer anyway
 
 		/**
 		 * Offset size of moved range.

src/model/operation/operation.js

@@ -38,6 +38,12 @@ export default class Operation {
 		 */
 		this.isDocumentOperation = this.baseVersion !== null;
 
+		/**
+		 * {@link module:engine/model/batch~Batch Batch} to which the operation is added or `null` if the operation is not
+		 * added to any batch yet.
+		 *
+		 * @member {module:engine/model/batch~Batch|null} #batch
+		 */
 		this.batch = null;
 
 		/**
@@ -98,6 +104,7 @@ export default class Operation {
 
 		json.__className = this.constructor.className;
 
+		// Remove reference to the parent `Batch` to avoid circular dependencies.
 		delete json.batch;
 
 		// Only document operations are shared with other clients so it is not necessary to keep this information.

src/model/operation/renameoperation.js

@@ -38,6 +38,7 @@ export default class RenameOperation extends Operation {
 		 * @member {module:engine/model/position~Position} module:engine/model/operation/renameoperation~RenameOperation#position
 		 */
 		this.position = position;
+		// This position sticks to the next node because it is a position before the node that we want to change.
 		this.position.stickiness = 'toNext';
 
 		/**