Merge pull request #1268 from ckeditor/t/1086

Other: Provided one API for two types of markers, improved docs. Closes #1086.

BREAKING CHANGE: Writer should be now used to set or remove markers, instead of MarkerCollection.

Author: Piotr Jasiun

src/model/document.js

@@ -161,7 +161,7 @@ export default class Document {
 		// Buffer marker changes.
 		// This is not covered in buffering operations because markers may change outside of them (when they
 		// are modified using `model.markers` collection, not through `MarkerOperation`).
-		this.listenTo( model.markers, 'add', ( evt, marker ) => {
+		this.listenTo( model.markers, 'set', ( evt, marker ) => {
 			// TODO: Should filter out changes of markers that are not in document.
 			// Whenever a new marker is added, buffer that change.
 			this.differ.bufferMarkerChange( marker.name, null, marker.getRange() );

src/model/markercollection.js

@@ -15,14 +15,19 @@ import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 import mix from '@ckeditor/ckeditor5-utils/src/mix';
 
 /**
- * Creates, stores and manages {@link ~Marker markers}.
+ * The collection of all {@link module:engine/model/markercollection~Marker markers} attached to the document.
+ * It lets you {@link module:engine/model/markercollection~MarkerCollection#get get} markers or track them using
+ * {@link module:engine/model/markercollection~MarkerCollection#event:set} and
+ * {@link module:engine/model/markercollection~MarkerCollection#event:remove} events.
  *
- * Markers are created by {@link ~MarkerCollection#set setting} a name for a {@link module:engine/model/liverange~LiveRange live range}
- * in `MarkerCollection`. Name is used to group and identify markers. Names have to be unique, but markers can be grouped by
- * using common prefixes, separated with `:`, for example: `user:john` or `search:3`.
+ * To create, change or remove makers use {@link module:engine/model/writer~Writer model writers'} methods:
+ * {@link module:engine/model/writer~Writer#setMarker} or {@link module:engine/model/writer~Writer#removeMarker}. Since
+ * the writer is the only proper way to change the data model it is not possible to change markers directly using this
+ * collection. All markers created by the writer will be automatically added to this collection.
  *
- * Since markers are based on {@link module:engine/model/liverange~LiveRange live ranges}, for efficiency reasons, it's
- * best to create and keep at least markers as possible.
+ * By default there is one marker collection available as {@link module:engine/model/model~Model#markers model property}.
+ *
+ * @see module:engine/model/markercollection~Marker
  */
 export default class MarkerCollection {
 	/**
@@ -78,42 +83,45 @@ export default class MarkerCollection {
 	 * set is different, the marker in collection is removed and then new marker is added. If the range was same, nothing
 	 * happens and `false` is returned.
 	 *
-	 * @fires module:engine/model/markercollection~MarkerCollection#event:add
+	 * @protected
+	 * @fires module:engine/model/markercollection~MarkerCollection#event:set
 	 * @fires module:engine/model/markercollection~MarkerCollection#event:remove
-	 * @param {String|module:engine/model/markercollection~Marker} markerOrName Name of marker to add or Marker instance to update.
+	 * @param {String|module:engine/model/markercollection~Marker} markerOrName Name of marker to set or marker instance to update.
 	 * @param {module:engine/model/range~Range} range Marker range.
+	 * @param {Boolean} managedUsingOperations Specifies whether the marker is managed using operations.
 	 * @returns {module:engine/model/markercollection~Marker} `Marker` instance added to the collection.
 	 */
-	set( markerOrName, range ) {
+	_set( markerOrName, range, managedUsingOperations ) {
 		const markerName = markerOrName instanceof Marker ? markerOrName.name : markerOrName;
 		const oldMarker = this._markers.get( markerName );
 
 		if ( oldMarker ) {
 			const oldRange = oldMarker.getRange();
 
-			if ( oldRange.isEqual( range ) ) {
+			if ( oldRange.isEqual( range ) && managedUsingOperations === oldMarker.managedUsingOperations ) {
 				return oldMarker;
 			}
 
-			this.remove( markerName );
+			this._remove( markerName );
 		}
 
 		const liveRange = LiveRange.createFromRange( range );
-		const marker = new Marker( markerName, liveRange );
+		const marker = new Marker( markerName, liveRange, managedUsingOperations );
 
 		this._markers.set( markerName, marker );
-		this.fire( 'add:' + markerName, marker );
+		this.fire( 'set:' + markerName, marker );
 
 		return marker;
 	}
 
 	/**
 	 * Removes given {@link ~Marker marker} or a marker with given name from the `MarkerCollection`.
 	 *
+	 * @protected
 	 * @param {String} markerOrName Marker or name of a marker to remove.
 	 * @returns {Boolean} `true` if marker was found and removed, `false` otherwise.
 	 */
-	remove( markerOrName ) {
+	_remove( markerOrName ) {
 		const markerName = markerOrName instanceof Marker ? markerOrName.name : markerOrName;
 		const oldMarker = this._markers.get( markerName );
 
@@ -190,10 +198,10 @@ export default class MarkerCollection {
 	}
 
 	/**
-	 * Fired whenever marker is added to `MarkerCollection`.
+	 * Fired whenever marker is added or updated in `MarkerCollection`.
 	 *
-	 * @event add
-	 * @param {module:engine/model/markercollection~Marker} The added marker.
+	 * @event set
+	 * @param {module:engine/model/markercollection~Marker} The set marker.
 	 */
 
 	/**
@@ -209,30 +217,64 @@ mix( MarkerCollection, EmitterMixin );
 /**
  * `Marker` is a continuous parts of model (like a range), is named and represent some kind of information about marked
  * part of model document. In contrary to {@link module:engine/model/node~Node nodes}, which are building blocks of
- * model document tree, markers are not stored directly in document tree. Still, they are document data, by giving
+ * model document tree, markers are not stored directly in document tree but in
+ * {@link module:engine/model/model~Model#markers model markers' collection}. Still, they are document data, by giving
  * additional meaning to the part of a model document between marker start and marker end.
  *
  * In this sense, markers are similar to adding and converting attributes on nodes. The difference is that attribute is
  * connected with a given node (e.g. a character is bold no matter if it gets moved or content around it changes).
- * Markers on the other hand are continuous ranges and are characterised by their start and end position. This means that
+ * Markers on the other hand are continuous ranges and are characterized by their start and end position. This means that
  * any character in the marker is marked by the marker. For example, if a character is moved outside of marker it stops being
  * "special" and the marker is shrunk. Similarly, when a character is moved into the marker from other place in document
  * model, it starts being "special" and the marker is enlarged.
  *
- * Since markers are based on {@link module:engine/model/liverange~LiveRange live ranges}, for efficiency reasons, it's
- * best to create and keep at least markers as possible.
+ * Another upside of markers is that finding marked part of document is fast and easy. Using attributes to mark some nodes
+ * and then trying to find that part of document would require traversing whole document tree. Marker gives instant access
+ * to the range which it is marking at the moment.
+ *
+ * Markers are built from a name and a range.
+ *
+ * Range of the marker is updated automatically when document changes, using
+ * {@link module:engine/model/liverange~LiveRange live range} mechanism.
+ *
+ * Name is used to group and identify markers. Names have to be unique, but markers can be grouped by
+ * using common prefixes, separated with `:`, for example: `user:john` or `search:3`. That's useful in term of creating
+ * namespaces for custom elements (e.g. comments, highlights). You can use this prefixes in
+ * {@link module:engine/model/markercollection~MarkerCollection#event:set} listeners to listen on changes in a group of markers.
+ * For instance: `model.markers.on( 'set:user', callback );` will be called whenever any `user:*` markers changes.
+ *
+ * There are two types of markers.
+ *
+ * 1. Markers managed directly, without using operations. They are added directly by {@link module:engine/model/writer~Writer}
+ * to the {@link module:engine/model/markercollection~MarkerCollection} without any additional mechanism. They can be used
+ * as bookmarks or visual markers. They are great for showing results of the find, or select link when the focus is in the input.
+ *
+ * 1. Markers managed using operations. These markers are also stored in {@link module:engine/model/markercollection~MarkerCollection}
+ * but changes in these markers is managed the same way all other changes in the model structure - using operations.
+ * Therefore, they are handled in the undo stack and synchronized between clients if the collaboration plugin is enabled.
+ * This type of markers is useful for solutions like spell checking or comments.
+ *
+ * Both type of them should be added / updated by {@link module:engine/model/writer~Writer#setMarker}
+ * and removed by {@link module:engine/model/writer~Writer#removeMarker} methods.
+ *
+ *		model.change( ( writer ) => {
+ * 			const marker = writer.setMarker( name, range, { usingOperation: true } );
+ *
+ * 			// ...
+ *
+ * 			writer.removeMarker( marker );
+ *		} );
+ *
+ * See {@link module:engine/model/writer~Writer} to find more examples.
+ *
+ * Since markers need to track change in the document, for efficiency reasons, it is best to create and keep as little
+ * markers as possible and remove them as soon as they are not needed anymore.
  *
  * Markers can be converted to view by adding appropriate converters for
  * {@link module:engine/conversion/modelconversiondispatcher~ModelConversionDispatcher#event:addMarker} and
  * {@link module:engine/conversion/modelconversiondispatcher~ModelConversionDispatcher#event:removeMarker}
  * events, or by building converters for {@link module:engine/conversion/modelconversiondispatcher~ModelConversionDispatcher}
  * using {@link module:engine/conversion/buildmodelconverter~buildModelConverter model converter builder}.
- *
- * Another upside of markers is that finding marked part of document is fast and easy. Using attributes to mark some nodes
- * and then trying to find that part of document would require traversing whole document tree. Marker gives instant access
- * to the range which it is marking at the moment.
- *
- * `Marker` instances are created and destroyed only by {@link ~MarkerCollection MarkerCollection}.
  */
 class Marker {
 	/**
@@ -241,20 +283,29 @@ class Marker {
 	 * @param {String} name Marker name.
 	 * @param {module:engine/model/liverange~LiveRange} liveRange Range marked by the marker.
 	 */
-	constructor( name, liveRange ) {
+	constructor( name, liveRange, managedUsingOperations ) {
 		/**
 		 * Marker's name.
 		 *
 		 * @readonly
-		 * @member {String} #name
+		 * @type {String}
 		 */
 		this.name = name;
 
+		/**
+		 * Flag indicates if the marker is managed using operations or not. See {@link ~Marker marker class description}
+		 * to learn more about marker types. See {@link module:engine/model/writer~Writer#setMarker}.
+		 *
+		 * @readonly
+		 * @type {Boolean}
+		 */
+		this.managedUsingOperations = managedUsingOperations;
+
 		/**
 		 * Range marked by the marker.
 		 *
 		 * @protected
-		 * @member {module:engine/model/liverange~LiveRange} #_liveRange
+		 * @type {module:engine/model/liverange~LiveRange}
 		 */
 		this._liveRange = liveRange;
 

src/model/operation/markeroperation.js

@@ -87,9 +87,9 @@ export default class MarkerOperation extends Operation {
 	 * @inheritDoc
 	 */
 	_execute() {
-		const type = this.newRange ? 'set' : 'remove';
+		const type = this.newRange ? '_set' : '_remove';
 
-		this._markers[ type ]( this.name, this.newRange );
+		this._markers[ type ]( this.name, this.newRange, true );
 	}
 
 	/**