feat(json-crdt-extensions): 🎸 improve OverlayPoint marker operations

Author: streamich

src/json-crdt-extensions/peritext/overlay/OverlayPoint.ts

@@ -25,14 +25,15 @@ export class OverlayPoint extends Point implements Printable, HeadlessNode {
   public readonly layers: Slice[] = [];
 
   /**
-   * Collapsed slices - markers/block splits, which represent a single point in
-   * the text, even if the start and end of the slice are different.
-   *
-   * @todo Rename to `markers`?
+   * Collapsed slices - markers (block splits), which represent a single point
+   * in the text, even if the start and end of the slice are different.
    */
-  public readonly points: Slice[] = [];
+  public readonly markers: Slice[] = [];
 
-  /** Hash of text contents until the next {@link OverlayPoint}. */
+  /**
+   * Hash of text contents until the next {@link OverlayPoint}. This field is
+   * modified by the {@link Overlay} tree.
+   */
   public hash: number = 0;
 
   public removeSlice(slice: Slice): void {
@@ -50,7 +51,7 @@ export class OverlayPoint extends Point implements Printable, HeadlessNode {
       }
     }
     this.removeLayer(slice);
-    this.removePoint(slice);
+    this.removeMarker(slice);
   }
 
   // ------------------------------------------------------------------- layers
@@ -109,32 +110,49 @@ export class OverlayPoint extends Point implements Printable, HeadlessNode {
 
   // ------------------------------------------------------------------ markers
 
-  public addPoint(slice: Slice): void {
-    const points = this.points;
+  /**
+   * Inserts a slice to the list of markers which represent a single point in
+   * the text, even if the start and end of the slice are different. The
+   * operation is idempotent, so inserting the same slice twice will not change
+   * the state of the point. The markers are sorted by the slice ID.
+   *
+   * @param slice Slice to add to the marker list.
+   */
+  public addMarker(slice: Slice): void {
+    const points = this.markers;
     const length = points.length;
     if (!length) {
       points.push(slice);
       return;
     }
-    // We attempt to insert from the end of the list, as it is the most likely.
+    // We attempt to insert from the end of the list, as it is the most likely
+    // scenario. And `.push()` is more efficient than `.unshift()`.
     const lastSlice = points[length - 1];
     const sliceId = slice.id;
-    if (compare(lastSlice.id, sliceId) < 0) {
+    const cmp = compare(lastSlice.id, sliceId);
+    if (cmp < 0) {
       points.push(slice);
       return;
-    }
+    } else if (!cmp) return;
     for (let i = length - 2; i >= 0; i--) {
       const currSlice = points[i];
-      if (compare(currSlice.id, sliceId) < 0) {
+      const cmp = compare(currSlice.id, sliceId);
+      if (cmp < 0) {
         points.splice(i + 1, 0, slice);
         return;
-      }
+      } else if (!cmp) return;
     }
     points.unshift(slice);
   }
 
-  public removePoint(slice: Slice): void {
-    const points = this.points;
+  /**
+   * Removes a slice from the list of markers, which represent a single point in
+   * the text, even if the start and end of the slice are different.
+   *
+   * @param slice Slice to remove from the marker list.
+   */
+  public removeMarker(slice: Slice): void {
+    const points = this.markers;
     const length = points.length;
     for (let i = 0; i < length; i++) {
       if (points[i] === slice) {

src/json-crdt-extensions/peritext/overlay/__tests__/OverlayPoint.spec.ts

@@ -119,3 +119,116 @@ describe('layers', () => {
     expect(point.layers.length).toBe(0);
   });
 });
+
+describe('markers', () => {
+  test('can add a marker', () => {
+    const {peritext, getPoint} = setupOverlayPoint();
+    const marker = peritext.slices.insSplit(peritext.rangeAt(5, 0), '<p>');
+    const point = getPoint(marker.start);
+    expect(point.markers.length).toBe(0);
+    point.addMarker(marker);
+    expect(point.markers.length).toBe(1);
+    expect(point.markers[0]).toBe(marker);
+  });
+
+  test('inserting same marker twice is a no-op', () => {
+    const {peritext, getPoint} = setupOverlayPoint();
+    const marker = peritext.slices.insSplit(peritext.rangeAt(5, 0), '<p>');
+    const point = getPoint(marker.start);
+    expect(point.markers.length).toBe(0);
+    point.addMarker(marker);
+    point.addMarker(marker);
+    point.addMarker(marker);
+    point.addMarker(marker);
+    expect(point.markers.length).toBe(1);
+    expect(point.markers[0]).toBe(marker);
+  });
+
+  test('can add two markers with the same start position', () => {
+    const {peritext, getPoint} = setupOverlayPoint();
+    const marker1 = peritext.slices.insSplit(peritext.rangeAt(5, 0), '<p>');
+    const marker2 = peritext.slices.insSplit(peritext.rangeAt(5, 0), '<p>');
+    const point = getPoint(marker1.start);
+    expect(point.markers.length).toBe(0);
+    point.addMarker(marker1);
+    expect(point.markers.length).toBe(1);
+    point.addMarker(marker2);
+    point.addMarker(marker2);
+    expect(point.markers.length).toBe(2);
+    expect(point.markers[0]).toBe(marker1);
+    expect(point.markers[1]).toBe(marker2);
+  });
+
+  test('orders markers by their ID', () => {
+    const {peritext, getPoint} = setupOverlayPoint();
+    const marker1 = peritext.slices.insSplit(peritext.rangeAt(5, 0), '<p>');
+    const marker2 = peritext.slices.insSplit(peritext.rangeAt(5, 0), '<p>');
+    const point = getPoint(marker1.start);
+    point.addMarker(marker2);
+    point.addMarker(marker1);
+    point.addMarker(marker2);
+    point.addMarker(marker1);
+    point.addMarker(marker2);
+    point.addMarker(marker1);
+    expect(point.markers[0]).toBe(marker1);
+    expect(point.markers[1]).toBe(marker2);
+  });
+
+  test('can add tree markers and sort them correctly', () => {
+    const {peritext, getPoint} = setupOverlayPoint();
+    const marker1 = peritext.slices.insSplit(peritext.rangeAt(5, 0), '<p>');
+    const marker2 = peritext.slices.insSplit(peritext.rangeAt(5, 0), '<p>');
+    const marker3 = peritext.slices.insSplit(peritext.rangeAt(5, 0), '<p>');
+    const point = getPoint(marker1.start);
+    point.addMarker(marker3);
+    point.addMarker(marker3);
+    point.addMarker(marker2);
+    point.addMarker(marker2);
+    point.addMarker(marker3);
+    point.addMarker(marker1);
+    point.addMarker(marker3);
+    point.addMarker(marker3);
+    expect(point.markers.length).toBe(3);
+    expect(point.markers[0]).toBe(marker1);
+    expect(point.markers[1]).toBe(marker2);
+    expect(point.markers[2]).toBe(marker3);
+  });
+
+  test('can add tree markers by appending them', () => {
+    const {peritext, getPoint} = setupOverlayPoint();
+    const marker1 = peritext.slices.insSplit(peritext.rangeAt(6, 0), '<p>');
+    const marker2 = peritext.slices.insSplit(peritext.rangeAt(6, 0), '<p>');
+    const marker3 = peritext.slices.insSplit(peritext.rangeAt(6, 0), '<p>');
+    const point = getPoint(marker2.start);
+    point.addMarker(marker1);
+    point.addMarker(marker2);
+    point.addMarker(marker3);
+    expect(point.markers[0]).toBe(marker1);
+    expect(point.markers[1]).toBe(marker2);
+    expect(point.markers[2]).toBe(marker3);
+  });
+
+  test('can remove markers', () => {
+    const {peritext, getPoint} = setupOverlayPoint();
+    const marker1 = peritext.slices.insSplit(peritext.rangeAt(6, 0), '<p>');
+    const marker2 = peritext.slices.insSplit(peritext.rangeAt(6, 0), '<p>');
+    const marker3 = peritext.slices.insSplit(peritext.rangeAt(6, 0), '<p>');
+    const point = getPoint(marker1.start);
+    point.addMarker(marker2);
+    point.addMarker(marker1);
+    point.addMarker(marker1);
+    point.addMarker(marker1);
+    point.addMarker(marker3);
+    expect(point.markers[0]).toBe(marker1);
+    expect(point.markers[1]).toBe(marker2);
+    expect(point.markers[2]).toBe(marker3);
+    point.removeMarker(marker2);
+    expect(point.markers[0]).toBe(marker1);
+    expect(point.markers[1]).toBe(marker3);
+    point.removeMarker(marker1);
+    expect(point.markers[0]).toBe(marker3);
+    point.removeMarker(marker1);
+    point.removeMarker(marker3);
+    expect(point.markers.length).toBe(0);
+  });
+});