Changeset: Migrate from opAssembler() to serializeOps()

Author: rhansen

CHANGELOG.md

@@ -25,6 +25,7 @@
     * `opAttributeValue()`
   * `opIterator()`: Deprecated in favor of the new `deserializeOps()` generator
     function.
+  * `opAssembler()`: Deprecated in favor of the new `serializeOps()` function.
   * `appendATextToAssembler()`: Deprecated in favor of the new `opsFromAText()`
     generator function.
   * `newOp()`: Deprecated in favor of the new `Op` class.

src/node/utils/padDiff.js

@@ -206,28 +206,26 @@ PadDiff.prototype._extendChangesetWithAuthor = (changeset, author, apool) => {
   // unpack
   const unpacked = Changeset.unpack(changeset);
 
-  const assem = Changeset.opAssembler();
-
   // create deleted attribs
   const authorAttrib = apool.putAttrib(['author', author || '']);
   const deletedAttrib = apool.putAttrib(['removed', true]);
   const attribs = `*${Changeset.numToString(authorAttrib)}*${Changeset.numToString(deletedAttrib)}`;
 
-  for (const operator of Changeset.deserializeOps(unpacked.ops)) {
-    if (operator.opcode === '-') {
-      // this is a delete operator, extend it with the author
-      operator.attribs = attribs;
-    } else if (operator.opcode === '=' && operator.attribs) {
-      // this is operator changes only attributes, let's mark which author did that
-      operator.attribs += `*${Changeset.numToString(authorAttrib)}`;
+  const serializedOps = Changeset.serializeOps((function* () {
+    for (const operator of Changeset.deserializeOps(unpacked.ops)) {
+      if (operator.opcode === '-') {
+        // this is a delete operator, extend it with the author
+        operator.attribs = attribs;
+      } else if (operator.opcode === '=' && operator.attribs) {
+        // this is operator changes only attributes, let's mark which author did that
+        operator.attribs += `*${Changeset.numToString(authorAttrib)}`;
+      }
+      yield operator;
     }
-
-    // append the new operator to our assembler
-    assem.append(operator);
-  }
+  })());
 
   // return the modified changeset
-  return Changeset.pack(unpacked.oldLen, unpacked.newLen, assem.toString(), unpacked.charBank);
+  return Changeset.pack(unpacked.oldLen, unpacked.newLen, serializedOps, unpacked.charBank);
 };
 
 // this method is 80% like Changeset.inverse. I just changed so instead of reverting,

src/static/js/Changeset.js

@@ -297,14 +297,16 @@ const copyOp = (op1, op2 = new Op()) => Object.assign(op2, op1);
  * @param {Iterable<Op>} ops - Iterable of operations to serialize.
  * @returns {string} A string containing the encoded op data (example: '|5=2p=v*4*5+1').
  */
-const serializeOps = (ops) => {
+exports.serializeOps = (ops) => {
   let res = '';
   for (const op of ops) res += op.toString();
   return res;
 };
 
 /**
  * Serializes a sequence of Ops.
+ *
+ * @deprecated Use `serializeOps` instead.
  */
 class OpAssembler {
   constructor() {
@@ -324,7 +326,7 @@ class OpAssembler {
   }
 
   toString() {
-    return serializeOps(this._ops);
+    return exports.serializeOps(this._ops);
   }
 }
 
@@ -334,12 +336,11 @@ class OpAssembler {
  */
 class MergingOpAssembler {
   constructor() {
-    this._assem = new OpAssembler();
     this.clear();
   }
 
   clear() {
-    this._assem.clear();
+    this._assem = [];
     this._bufOp = new Op();
     // If we get, for example, insertions [xxx\n,yyy], those don't merge, but if we get
     // [xxx\n,yyy,zzz\n], that merges to [xxx\nyyyzzz\n]. This variable stores the length of yyy and
@@ -352,11 +353,11 @@ class MergingOpAssembler {
     if (isEndDocument && this._bufOp.opcode === '=' && !this._bufOp.attribs) {
       // final merged keep, leave it implicit
     } else {
-      this._assem.append(this._bufOp);
+      this._assem.push(copyOp(this._bufOp));
       if (this._bufOpAdditionalCharsAfterNewline) {
         this._bufOp.chars = this._bufOpAdditionalCharsAfterNewline;
         this._bufOp.lines = 0;
-        this._assem.append(this._bufOp);
+        this._assem.push(copyOp(this._bufOp));
         this._bufOpAdditionalCharsAfterNewline = 0;
       }
     }
@@ -390,7 +391,7 @@ class MergingOpAssembler {
 
   toString() {
     this._flush();
-    return this._assem.toString();
+    return exports.serializeOps(this._assem);
   }
 }
 
@@ -590,9 +591,14 @@ exports.smartOpAssembler = () => new SmartOpAssembler();
 exports.mergingOpAssembler = () => new MergingOpAssembler();
 
 /**
+ * @deprecated Use `serializeOps` instead.
  * @returns {OpAssembler}
  */
-exports.opAssembler = () => new OpAssembler();
+exports.opAssembler = () => {
+  padutils.warnWithStack(
+      'Changeset.opAssembler() is deprecated; use Changeset.serializeOps() instead');
+  return new OpAssembler();
+};
 
 /**
  * A custom made String Iterator

src/tests/frontend/specs/easysync-assembler.js

@@ -3,11 +3,9 @@
 const Changeset = require('../../../static/js/Changeset');
 
 describe('easysync-assembler', function () {
-  it('opAssembler', async function () {
+  it('deserialize and serialize', async function () {
     const x = '-c*3*4+6|3=az*asdf0*1*2*3+1=1-1+1*0+1=1-1+1|c=c-1';
-    const assem = Changeset.opAssembler();
-    for (const op of Changeset.deserializeOps(x)) assem.append(op);
-    expect(assem.toString()).to.equal(x);
+    expect(Changeset.serializeOps(Changeset.deserializeOps(x))).to.equal(x);
   });
 
   it('smartOpAssembler', async function () {