Document the client code, add more tests

assets/js/editor/editor_client.js

@@ -1,3 +1,11 @@
+/**
+ * A manager associated with a particular editor instance,
+ * which is responsible for controlling client-server communication
+ * and synchronizing the sent/received changes.
+ *
+ * This class takes `serverAdapter` and `editorAdapter` objects
+ * that encapsulate the logic relevant for each part.
+ */
 export default class EditorClient {
   constructor(serverAdapter, editorAdapter, revision) {
     this.serverAdapter = serverAdapter;
@@ -80,9 +88,11 @@ class AwaitingConfirm {
   }
 
   onServerDelta(delta) {
-    const deltaPrime = this.awaitedDelta.transform(delta);
+    // We consider the incoming delta to happen first
+    // (because that's the case from the server's perspective).
+    const deltaPrime = this.awaitedDelta.transform(delta, "right");
     this.client.applyDelta(deltaPrime);
-    const awaitedDeltaPrime = delta.transform(this.awaitedDelta);
+    const awaitedDeltaPrime = delta.transform(this.awaitedDelta, "left");
     return new AwaitingConfirm(this.client, awaitedDeltaPrime);
   }
 
@@ -108,12 +118,14 @@ class AwaitingWithBuffer {
   }
 
   onServerDelta(delta) {
-    const deltaPrime = this.awaitedDelta.compose(this.buffer).transform(delta);
+    // We consider the incoming delta to happen first
+    // (because that's the case from the server's perspective).
+    const deltaPrime = this.awaitedDelta.compose(this.buffer).transform(delta, "right");
     this.client.applyDelta(deltaPrime);
-    const awaitedDeltaPrime = delta.transform(this.awaitedDelta);
+    const awaitedDeltaPrime = delta.transform(this.awaitedDelta, "left");
     const bufferPrime = this.awaitedDelta
-      .transform(delta)
-      .transform(this.buffer);
+      .transform(delta, "right")
+      .transform(this.buffer, "left");
 
     return new AwaitingWithBuffer(this.client, awaitedDeltaPrime, bufferPrime);
   }

assets/js/editor/hook_server_adapter.js

@@ -1,29 +1,43 @@
 import Delta from "../lib/delta";
 
-export default class HookAdapter {
+/**
+ * Encapsulates logic related to sending/receiving messages from the server.
+ *
+ * Uses the given hook instance socket for the communication.
+ */
+export default class HookServerAdapter {
   constructor(hook, cellId) {
     this.hook = hook;
     this.cellId = cellId;
     this._onDelta = null;
     this._onAcknowledgement = null;
 
-    this.hook.handleEvent(`cell:${this.cellId}:delta`, (delta) => {
+    this.hook.handleEvent(`cell_delta:${this.cellId}`, (delta) => {
       this._onDelta && this._onDelta(new Delta(delta.ops));
     });
 
-    this.hook.handleEvent(`cell:${this.cellId}:acknowledgement`, () => {
+    this.hook.handleEvent(`cell_acknowledgement:${this.cellId}`, () => {
       this._onAcknowledgement && this._onAcknowledgement();
     });
   }
 
+  /**
+   * Registers a callback called whenever a new delta comes from the server.
+   */
   onDelta(callback) {
     this._onDelta = callback;
   }
 
+  /**
+   * Registers a callback called when delta acknowledgement comes from the server.
+   */
   onAcknowledgement(callback) {
     this._onAcknowledgement = callback;
   }
 
+  /**
+   * Sends the given delta to the server.
+   */
   sendDelta(delta, revision) {
     this.hook.pushEvent("cell_delta", {
       cell_id: this.cellId,

assets/js/editor/index.js

@@ -1,26 +1,44 @@
-import * as monaco from "monaco-editor/esm/vs/editor/editor.api";
+import monaco from "./monaco";
 import EditorClient from "./editor_client";
-import MonacoAdapter from "./monaco_adapter";
-import HookAdapter from "./hook_adapter";
+import MonacoEditorAdapter from "./monaco_editor_adapter";
+import HookServerAdapter from "./hook_server_adapter";
 
+/**
+ * A hook managing an editable cell.
+ *
+ * Mounts a Monaco Editor and provides real-time collaboration mechanism
+ * by sending all changes as `Delta` objects to the server
+ * and handling such objects sent by other clients.
+ *
+ * Configuration:
+ *
+ *   * `data-cell-id` - id of the cell being edited
+ *   * `data-type` - editor type (i.e. language), either "markdown" or "elixir" is expected
+ *
+ * Additionally the root element should have a direct `div` child
+ * with `data-source` and `data-revision` providing the initial values.
+ */
 const Editor = {
   mounted() {
-    this.cellId = this.el.dataset.id;
+    this.cellId = this.el.dataset.cellId;
     this.type = this.el.dataset.type;
 
-    const editorContainer = document.createElement("div");
-    this.el.appendChild(editorContainer);
+    const editorContainer = this.el.querySelector("div");
+
+    if (!editorContainer) {
+      throw new Error("Editor Hook root element should have a div child");
+    }
+
+    const source = editorContainer.dataset.source;
+    const revision = +editorContainer.dataset.revision;
 
     this.editor = this.__mountEditor(editorContainer);
 
-    const source = this.el.dataset.source;
-    const revision = +this.el.dataset.revision;
-
     this.editor.getModel().setValue(source);
 
     new EditorClient(
-      new HookAdapter(this, this.cellId),
-      new MonacoAdapter(this.editor),
+      new HookServerAdapter(this, this.cellId),
+      new MonacoEditorAdapter(this.editor),
       revision
     );
   },
@@ -47,13 +65,14 @@ const Editor = {
     editor.getModel().updateOptions({
       tabSize: 2,
     });
+
     editor.updateOptions({
       autoIndent: true,
       tabSize: 2,
       formatOnType: true,
     });
 
-    // Dynamically adjust editor width to the content, see https://github.com/microsoft/monaco-editor/issues/794
+    // Dynamically adjust editor height to the content, see https://github.com/microsoft/monaco-editor/issues/794
     function adjustEditorLayout() {
       const contentHeight = editor.getContentHeight();
       editorContainer.style.height = `${contentHeight}px`;
@@ -67,11 +86,6 @@ const Editor = {
       editor.layout();
     });
 
-    // // Handle Ctrl + Enter
-    // editor.addCommand(monaco.KeyMod.CtrlCmd | monaco.KeyCode.Enter, () => {
-    //   this.pushEvent("evaluate_cell", { cell_id: this.cellId });
-    // });
-
     return editor;
   },
 };

assets/js/editor/monaco.js

@@ -0,0 +1,5 @@
+import * as monaco from "monaco-editor/esm/vs/editor/editor.api";
+
+// TODO: add Elixir language definition
+
+export default monaco;

assets/js/editor/monaco_editor_adapter.js

@@ -1,7 +1,12 @@
+import monaco from "./monaco";
 import Delta from "../lib/delta";
-import * as monaco from "monaco-editor/esm/vs/editor/editor.api";
 
-export default class MonacoAdapter {
+/**
+ * Encapsulates logic related to getting/applying changes to the editor.
+ *
+ * Uses the given Monaco editor instance.
+ */
+export default class MonacoEditorAdapter {
   constructor(editor) {
     this.editor = editor;
     this._onDelta = null;
@@ -15,10 +20,17 @@ export default class MonacoAdapter {
     });
   }
 
+  /**
+   * Registers a callback called whenever the user makes a change
+   * to the editor content. The change is represented by a delta object.
+   */
   onDelta(callback) {
     this._onDelta = callback;
   }
 
+  /**
+   * Applies the given delta to the editor content.
+   */
   applyDelta(delta) {
     const operations = this.__deltaToEditorOperations(delta);
     this.ignoreChange = true;

assets/js/lib/delta.js

@@ -1,8 +1,29 @@
+/**
+ * Delta represents a set of changes introduced to a text document.
+ *
+ * Delta is suitable for Operational Transformation and is hence
+ * our primary building block in collaborative text editing.
+ * For a detailed write-up see https://quilljs.com/docs/delta.
+ * The specification covers rich-text editing, while we only
+ * need to work on plain-text, so we use a strict subset of
+ * the specification, where each operation is either:
+ *
+ *   * `{ retain: number }` - move by the given number of characters
+ *   * `{ insert: string }` - insert the given text at the current position
+ *   * `{ delete: number }` - delete the given number of characters starting from the current position
+ *
+ * This class provides a number of methods for creating and transforming a delta.
+ *
+ * The implementation based on https://github.com/quilljs/delta
+ */
 export default class Delta {
   constructor(ops = []) {
     this.ops = ops;
   }
 
+  /**
+   * Appends a retain operation.
+   */
   retain(length) {
     if (length <= 0) {
       return this;
@@ -11,6 +32,9 @@ export default class Delta {
     return this.append({ retain: length });
   }
 
+   /**
+   * Appends an insert operation.
+   */
   insert(text) {
     if (text === "") {
       return this;
@@ -19,6 +43,9 @@ export default class Delta {
     return this.append({ insert: text });
   }
 
+  /**
+   * Appends a delete operation.
+   */
   delete(length) {
     if (length <= 0) {
       return this;
@@ -27,6 +54,15 @@ export default class Delta {
     return this.append({ delete: length });
   }
 
+  /**
+   * Appends the given operation.
+   *
+   * To maintain the canonical form (uniqueness of representation)
+   * this method follows two rules:
+   *
+   *   * insert always goes before delete
+   *   * operations of the same type are merged into a single operation
+   */
   append(op) {
     if (this.ops.length === 0) {
       this.ops.push(op);
@@ -36,7 +72,7 @@ export default class Delta {
     const lastOp = this.ops.pop();
 
     // Insert and delete are commutative, so we always make sure
-    // to put insert first to preserve the canonical form (uniqueness of representation).
+    // to put insert first to preserve the canonical form.
     if (isInsert(op) && isDelete(lastOp)) {
       return this.append(op).append(lastOp);
     }
@@ -60,6 +96,10 @@ export default class Delta {
     return this;
   }
 
+  /**
+   * Returns a new delta that is equivalent to applying the operations of this delta,
+   * followed by operations of the given delta.
+   */
   compose(other) {
     const thisIter = new Iterator(this.ops);
     const otherIter = new Iterator(other.ops);
@@ -87,16 +127,43 @@ export default class Delta {
       }
     }
 
-    return delta.trim();
+    return delta.__trim();
   }
 
-  transform(other) {
+  /**
+   * Transform the given delta against this delta's operations. Returns a new delta.
+   *
+   * This is the core idea behind Operational Transformation.
+
+   * Let's mark this delta as A and the `other` delta as B
+   * and assume they represent changes applied at the same time
+   * to the same text state. If the current text state reflects
+   * delta A being applied, we would like to apply delta B,
+   * but to preserve its intent we need consider the changes made by A.
+   * We can obtain a new delta B' by transforming it against the delta A,
+   * so that does effectively what the original delta B meant to do.
+   * In our case that would be `A.transform(B, "right")`.
+   *
+   * Transformation should work both ways satisfying the property (assuming B is considered to happen first):
+   *
+   * `A.concat(A.transform(B, "right")) = B.concat(B.transform(A, "left"))`
+   *
+   * In Git analogy this can be thought of as rebasing branch B onto branch A.
+   *
+   * The method takes a `priority` argument that indicates which delta should be
+   * considered "first" and win ties, should be either "left" or "right".
+   */
+  transform(other, priority) {
+    if (priority !== "left" && priority !== "right") {
+      throw new Error(`Invalid priority "${priority}", should be either "left" or "right"`)
+    }
+
     const thisIter = new Iterator(this.ops);
     const otherIter = new Iterator(other.ops);
     const delta = new Delta();
 
     while (thisIter.hasNext() || otherIter.hasNext()) {
-      if (isInsert(thisIter.peek()) && !isInsert(otherIter.peek())) {
+      if (isInsert(thisIter.peek()) && (!isInsert(otherIter.peek()) || priority === "left")) {
         const insertLength = operationLength(thisIter.next());
         delta.retain(insertLength);
       } else if (isInsert(otherIter.peek())) {
@@ -118,10 +185,10 @@ export default class Delta {
       }
     }
 
-    return delta.trim();
+    return delta.__trim();
   }
 
-  trim() {
+  __trim() {
     if (this.ops.length > 0 && isRetain(this.ops[this.ops.length - 1])) {
       this.ops.pop();
     }
@@ -130,6 +197,11 @@ export default class Delta {
   }
 }
 
+/**
+ * Operations iterator simplifying the implementation of the delta methods above.
+ *
+ * Allows for iterating over operation slices by specifying the desired length.
+ */
 class Iterator {
   constructor(ops) {
     this.ops = ops;
@@ -181,7 +253,7 @@ class Iterator {
   }
 }
 
-export function operationLength(op) {
+function operationLength(op) {
   if (isInsert(op)) {
     return op.insert.length;
   }

assets/test/lib/delta.test.js

@@ -118,4 +118,137 @@ describe('Delta', () => {
       expect(a.compose(b)).toEqual(expected);
     });
   });
+
+  describe('transform', () => {
+    it('insert against insert', () => {
+      const a = new Delta().insert('A');
+      const b = new Delta().insert('B');
+      const bPrimeAssumingAFirst = new Delta().retain(1).insert('B');
+      const bPrimeAssumingBFirst = new Delta().insert('B');
+      expect(a.transform(b, "left")).toEqual(bPrimeAssumingAFirst);
+      expect(a.transform(b, "right")).toEqual(bPrimeAssumingBFirst);
+    });
+
+    it('retain against insert', () => {
+      const a = new Delta().insert('A');
+      const b = new Delta().retain(1).insert('B');
+      const bPrime = new Delta().retain(2).insert('B');
+      expect(a.transform(b, "right")).toEqual(bPrime);
+    });
+
+    it('delete against insert', () => {
+      const a = new Delta().insert('A');
+      const b = new Delta().delete(1);
+      const bPrime = new Delta().retain(1).delete(1);
+      expect(a.transform(b, "right")).toEqual(bPrime);
+    });
+
+    it('insert against delete', () => {
+      const a = new Delta().delete(1);
+      const b = new Delta().insert('B');
+      const bPrime = new Delta().insert('B');
+      expect(a.transform(b, "right")).toEqual(bPrime);
+    });
+
+    it('retain against delete', () => {
+      const a = new Delta().delete(1);
+      const b = new Delta().retain(1);
+      const bPrime = new Delta();
+      expect(a.transform(b, "right")).toEqual(bPrime);
+    });
+
+    it('delete against delete', () => {
+      const a = new Delta().delete(1);
+      const b = new Delta().delete(1);
+      const bPrime = new Delta();
+      expect(a.transform(b, "right")).toEqual(bPrime);
+    });
+
+    it('insert against retain', () => {
+      const a = new Delta().retain(1);
+      const b = new Delta().insert('B');
+      const bPrime = new Delta().insert('B');
+      expect(a.transform(b, "right")).toEqual(bPrime);
+    });
+
+    it('retain against retain', () => {
+      const a = new Delta().retain(1).insert('A');
+      const b = new Delta().retain(1).insert('B');
+      const bPrime = new Delta().retain(1).insert('B')
+      expect(a.transform(b, "right")).toEqual(bPrime);
+    });
+
+    it('delete against retain', () => {
+      const a = new Delta().retain(1);
+      const b = new Delta().delete(1);
+      const bPrime = new Delta().delete(1);
+      expect(a.transform(b, "right")).toEqual(bPrime);
+    });
+
+    it('alternating edits', () => {
+      const a = new Delta()
+        .retain(2)
+        .insert('si')
+        .delete(5);
+
+      const b = new Delta()
+        .retain(1)
+        .insert('e')
+        .delete(5)
+        .retain(1)
+        .insert('ow');
+
+      const bPrimeAssumingBFirst = new Delta()
+        .retain(1)
+        .insert('e')
+        .delete(1)
+        .retain(2)
+        .insert('ow');
+
+      const aPrimeAssumingBFirst = new Delta()
+        .retain(2)
+        .insert('si')
+        .delete(1);
+
+      expect(a.transform(b, "right")).toEqual(bPrimeAssumingBFirst);
+      expect(b.transform(a, "right")).toEqual(aPrimeAssumingBFirst);
+    });
+
+    it('conflicting appends', () => {
+      const a = new Delta().retain(3).insert('aa');
+      const b = new Delta().retain(3).insert('bb');
+      const bPrimeAssumingBFirst = new Delta().retain(3).insert('bb');
+      const aPrimeAssumingBFirst = new Delta().retain(5).insert('aa');
+      expect(a.transform(b, "right")).toEqual(bPrimeAssumingBFirst);
+      expect(b.transform(a, "left")).toEqual(aPrimeAssumingBFirst);
+    });
+
+    it('prepend and append', () => {
+      const a = new Delta().insert('aa');
+      const b = new Delta().retain(3).insert('bb');
+      const bPrime = new Delta().retain(5).insert('bb');
+      const aPrime = new Delta().insert('aa');
+      expect(a.transform(b, "right")).toEqual(bPrime);
+      expect(b.transform(a, "right")).toEqual(aPrime);
+    });
+
+    it('trailing deletes with differing lengths', () => {
+      const a = new Delta().retain(2).delete(1);
+      const b = new Delta().delete(3);
+      const bPrime = new Delta().delete(2);
+      const aPrime = new Delta();
+      expect(a.transform(b, "right")).toEqual(bPrime);
+      expect(b.transform(a, "right")).toEqual(aPrime);
+    });
+
+    it('immutability', () => {
+      const a = new Delta().insert('A');
+      const b = new Delta().insert('B');
+      const bPrime = new Delta().retain(1).insert('B');
+      expect(a.transform(b, "left")).toEqual(bPrime);
+
+      expect(a).toEqual(new Delta().insert('A'));
+      expect(b).toEqual(new Delta().insert('B'));
+    });
+  });
 });

lib/live_book_web/live/cell.ex

@@ -15,13 +15,14 @@ defmodule LiveBookWeb.Cell do
         </button>
       </div>
       <div
-        id="cell-<%= @cell.id %>"
+        id="cell-<%= @cell.id %>-editor"
         phx-hook="Editor"
         phx-update="ignore"
-        data-id="<%= @cell.id %>"
-        data-type="<%= @cell.type %>"
-        data-source="<%= @cell.source %>"
-        data-revision="<%= @cell_info.revision %>">
+        data-cell-id="<%= @cell.id %>"
+        data-type="<%= @cell.type %>">
+        <div data-source="<%= @cell.source %>"
+             data-revision="<%= @cell_info.revision %>">
+        </div>
       </div>
     </div>
     """

lib/live_book_web/live/session_live.ex

@@ -175,9 +175,9 @@ defmodule LiveBookWeb.SessionLive do
 
   defp handle_action(socket, {:broadcast_delta, from, cell, delta}) do
     if from == self() do
-      push_event(socket, "cell:#{cell.id}:acknowledgement", %{})
+      push_event(socket, "cell_acknowledgement:#{cell.id}", %{})
     else
-      push_event(socket, "cell:#{cell.id}:delta", DeltaUtils.delta_to_map(delta))
+      push_event(socket, "cell_delta:#{cell.id}", DeltaUtils.delta_to_map(delta))
     end
   end