Changeset: Migrate from smartOpAssembler() to canonicalizeOps()

Author: rhansen

CHANGELOG.md

@@ -26,6 +26,8 @@
   * `opAssembler()`: Deprecated in favor of the new `serializeOps()` function.
   * `mergingOpAssembler()`: Deprecated in favor of the new `squashOps()`
     generator function (combined with `serializeOps()`).
+  * `smartOpAssembler()`: Deprecated in favor of the new `canonicalizeOps()`
+    generator function (combined with `serializeOps()`).
   * `appendATextToAssembler()`: Deprecated in favor of the new `opsFromAText()`
     generator function.
   * `newOp()`: Deprecated in favor of the new `Op` class.

src/node/db/Pad.js

@@ -496,21 +496,20 @@ Pad.prototype.copyPadWithoutHistory = async function (destinationID, force) {
 
   const oldAText = this.atext;
 
-  // based on Changeset.makeSplice
-  const assem = Changeset.smartOpAssembler();
-  for (const op of Changeset.opsFromAText(oldAText)) assem.append(op);
-  assem.endDocument();
+  let newLength;
+  const serializedOps = Changeset.serializeOps((function* () {
+    newLength = yield* Changeset.canonicalizeOps(Changeset.opsFromAText(oldAText), true);
+  })());
 
   // although we have instantiated the newPad with '\n', an additional '\n' is
   // added internally, so the pad text on the revision 0 is "\n\n"
   const oldLength = 2;
 
-  const newLength = assem.getLengthChange();
   const newText = oldAText.text;
 
   // create a changeset that removes the previous text and add the newText with
   // all atributes present on the source pad
-  const changeset = Changeset.pack(oldLength, newLength, assem.toString(), newText);
+  const changeset = Changeset.pack(oldLength, newLength, serializedOps, newText);
   newPad.appendRevision(changeset);
 
   await hooks.aCallAll('padCopy', {originalPad: this, destinationID});

src/static/js/Changeset.js

@@ -432,7 +432,7 @@ class MergingOpAssembler {
  * @returns {Generator<Op, number>} The done value indicates how much the sequence of operations
  *     changes the length of the document (in characters).
  */
-const canonicalizeOps = function* (ops, finalize) {
+exports.canonicalizeOps = function* (ops, finalize) {
   let minusOps = [];
   let plusOps = [];
   let keepOps = [];
@@ -519,6 +519,8 @@ const opsFromText = function* (opcode, text, attribs = '', pool = null) {
  *   - strips final "="
  *   - ignores 0-length changes
  *   - reorders consecutive + and - (which MergingOpAssembler doesn't do)
+ *
+ * @deprecated Use `canonicalizeOps` with `serializeOps` instead.
  */
 class SmartOpAssembler {
   constructor() {
@@ -533,7 +535,7 @@ class SmartOpAssembler {
 
   _serialize(finalize) {
     this._serialized = exports.serializeOps((function* () {
-      this._lengthChange = yield* canonicalizeOps(this._ops, finalize);
+      this._lengthChange = yield* exports.canonicalizeOps(this._ops, finalize);
     }).call(this));
   }
 
@@ -586,51 +588,54 @@ exports.checkRep = (cs) => {
   const unpacked = exports.unpack(cs);
   const oldLen = unpacked.oldLen;
   const newLen = unpacked.newLen;
-  const ops = unpacked.ops;
   let charBank = unpacked.charBank;
 
-  const assem = new SmartOpAssembler();
   let oldPos = 0;
   let calcNewLen = 0;
   let numInserted = 0;
-  for (const o of exports.deserializeOps(ops)) {
-    switch (o.opcode) {
-      case '=':
-        oldPos += o.chars;
-        calcNewLen += o.chars;
-        break;
-      case '-':
-        oldPos += o.chars;
-        assert(oldPos <= oldLen, `${oldPos} > ${oldLen} in ${cs}`);
-        break;
-      case '+':
-      {
-        calcNewLen += o.chars;
-        numInserted += o.chars;
-        assert(calcNewLen <= newLen, `${calcNewLen} > ${newLen} in ${cs}`);
-        break;
+  const ops = (function* () {
+    for (const o of exports.deserializeOps(unpacked.ops)) {
+      switch (o.opcode) {
+        case '=':
+          oldPos += o.chars;
+          calcNewLen += o.chars;
+          break;
+        case '-':
+          oldPos += o.chars;
+          assert(oldPos <= oldLen, `${oldPos} > ${oldLen} in ${cs}`);
+          break;
+        case '+':
+          calcNewLen += o.chars;
+          numInserted += o.chars;
+          assert(calcNewLen <= newLen, `${calcNewLen} > ${newLen} in ${cs}`);
+          break;
       }
+      yield o;
     }
-    assem.append(o);
-  }
+  })();
+  const serializedOps = exports.serializeOps(exports.canonicalizeOps(ops, true));
 
   calcNewLen += oldLen - oldPos;
   charBank = charBank.substring(0, numInserted);
   while (charBank.length < numInserted) {
     charBank += '?';
   }
 
-  assem.endDocument();
-  const normalized = exports.pack(oldLen, calcNewLen, assem.toString(), charBank);
+  const normalized = exports.pack(oldLen, calcNewLen, serializedOps, charBank);
   assert(normalized === cs, 'Invalid changeset (checkRep failed)');
 
   return cs;
 };
 
 /**
+ * @deprecated Use `canonicalizeOps` with `serializeOps` instead.
  * @returns {SmartOpAssembler}
  */
-exports.smartOpAssembler = () => new SmartOpAssembler();
+exports.smartOpAssembler = () => {
+  padutils.warnWithStack(
+      'Changeset.smartOpAssembler() is deprecated; use Changeset.canonicalizeOps() instead');
+  return new SmartOpAssembler();
+};
 
 /**
  * @deprecated Use `squashOps` with `serializeOps` instead.
@@ -1079,22 +1084,22 @@ class TextLinesMutator {
  * @returns {string} the integrated changeset
  */
 const applyZip = (in1, in2, func) => {
-  const ops1 = exports.deserializeOps(in1);
-  const ops2 = exports.deserializeOps(in2);
-  let next1 = ops1.next();
-  let next2 = ops2.next();
-  const assem = new SmartOpAssembler();
-  while (!next1.done || !next2.done) {
-    if (!next1.done && !next1.value.opcode) next1 = ops1.next();
-    if (!next2.done && !next2.value.opcode) next2 = ops2.next();
-    if (next1.value == null) next1.value = new Op();
-    if (next2.value == null) next2.value = new Op();
-    if (!next1.value.opcode && !next2.value.opcode) break;
-    const opOut = func(next1.value, next2.value);
-    if (opOut && opOut.opcode) assem.append(opOut);
-  }
-  assem.endDocument();
-  return assem.toString();
+  const ops = (function* () {
+    const ops1 = exports.deserializeOps(in1);
+    const ops2 = exports.deserializeOps(in2);
+    let next1 = ops1.next();
+    let next2 = ops2.next();
+    while (!next1.done || !next2.done) {
+      if (!next1.done && !next1.value.opcode) next1 = ops1.next();
+      if (!next2.done && !next2.value.opcode) next2 = ops2.next();
+      if (next1.value == null) next1.value = new Op();
+      if (next2.value == null) next2.value = new Op();
+      if (!next1.value.opcode && !next2.value.opcode) break;
+      const opOut = func(next1.value, next2.value);
+      if (opOut && opOut.opcode) yield opOut;
+    }
+  })();
+  return exports.serializeOps(exports.canonicalizeOps(ops, true));
 };
 
 /**
@@ -1545,15 +1550,13 @@ exports.makeSplice = (oldFullText, spliceStart, numRemoved, newText, optNewTextA
   const oldText = oldFullText.substring(spliceStart, spliceStart + numRemoved);
   const newLen = oldLen + newText.length - oldText.length;
 
-  const assem = new SmartOpAssembler();
   const ops = (function* () {
     yield* opsFromText('=', oldFullText.substring(0, spliceStart));
     yield* opsFromText('-', oldText);
     yield* opsFromText('+', newText, optNewTextAPairs, pool);
   })();
-  for (const op of ops) assem.append(op);
-  assem.endDocument();
-  return exports.pack(oldLen, newLen, assem.toString(), newText);
+  const serializedOps = exports.serializeOps(exports.canonicalizeOps(ops, true));
+  return exports.pack(oldLen, newLen, serializedOps, newText);
 };
 
 /**
@@ -1675,11 +1678,8 @@ exports.moveOpsToNewPool = (cs, oldPool, newPool) => {
  * @param {string} text - text to insert
  * @returns {string}
  */
-exports.makeAttribution = (text) => {
-  const assem = new SmartOpAssembler();
-  for (const op of opsFromText('+', text)) assem.append(op);
-  return assem.toString();
-};
+exports.makeAttribution =
+    (text) => exports.serializeOps(exports.canonicalizeOps(opsFromText('+', text), false));
 
 /**
  * Iterates over attributes in exports, attribution string, or attribs property of an op and runs
@@ -1932,8 +1932,7 @@ exports.attribsAttributeValue = (attribs, key, pool) => {
  * @returns {Builder}
  */
 exports.builder = (oldLen) => {
-  const assem = new SmartOpAssembler();
-  const o = new Op();
+  const ops = [];
   const charBank = exports.stringAssembler();
 
   const self = {
@@ -1948,12 +1947,12 @@ exports.builder = (oldLen) => {
      * @returns {Builder} this
      */
     keep: (N, L, attribs, pool) => {
-      o.opcode = '=';
+      const o = new Op('=');
       o.attribs = typeof attribs === 'string'
         ? attribs : new AttributeMap(pool).update(attribs || []).toString();
       o.chars = N;
       o.lines = (L || 0);
-      assem.append(o);
+      ops.push(o);
       return self;
     },
 
@@ -1966,7 +1965,7 @@ exports.builder = (oldLen) => {
      * @returns {Builder} this
      */
     keepText: (text, attribs, pool) => {
-      for (const op of opsFromText('=', text, attribs, pool)) assem.append(op);
+      ops.push(...opsFromText('=', text, attribs, pool));
       return self;
     },
 
@@ -1979,7 +1978,7 @@ exports.builder = (oldLen) => {
      * @returns {Builder} this
      */
     insert: (text, attribs, pool) => {
-      for (const op of opsFromText('+', text, attribs, pool)) assem.append(op);
+      ops.push(...opsFromText('+', text, attribs, pool));
       charBank.append(text);
       return self;
     },
@@ -1991,18 +1990,22 @@ exports.builder = (oldLen) => {
      * @returns {Builder} this
      */
     remove: (N, L) => {
-      o.opcode = '-';
+      const o = new Op('-');
       o.attribs = '';
       o.chars = N;
       o.lines = (L || 0);
-      assem.append(o);
+      ops.push(o);
       return self;
     },
 
     toString: () => {
-      assem.endDocument();
-      const newLen = oldLen + assem.getLengthChange();
-      return exports.pack(oldLen, newLen, assem.toString(), charBank.toString());
+      /** @type {number} */
+      let lengthChange;
+      const serializedOps = exports.serializeOps((function* () {
+        lengthChange = yield* exports.canonicalizeOps(ops, true);
+      })());
+      const newLen = oldLen + lengthChange;
+      return exports.pack(oldLen, newLen, serializedOps, charBank.toString());
     },
   };
 
@@ -2037,11 +2040,11 @@ exports.makeAttribsString = (opcode, attribs, pool) => {
 exports.subattribution = (astr, start, optEnd) => {
   const attOps = exports.deserializeOps(astr);
   let attOpsNext = attOps.next();
-  const assem = new SmartOpAssembler();
   let attOp = new Op();
-  const csOp = new Op();
+  const csOp = new Op('-');
+  csOp.chars = start;
 
-  const doCsOp = () => {
+  const doCsOp = function* () {
     if (!csOp.chars) return;
     while (csOp.opcode && (attOp.opcode || !attOpsNext.done)) {
       if (!attOp.opcode) {
@@ -2053,30 +2056,25 @@ exports.subattribution = (astr, start, optEnd) => {
         csOp.lines++;
       }
       const opOut = slicerZipperFunc(attOp, csOp, null);
-      if (opOut.opcode) assem.append(opOut);
+      if (opOut.opcode) yield opOut;
     }
   };
 
-  csOp.opcode = '-';
-  csOp.chars = start;
-
-  doCsOp();
-
-  if (optEnd === undefined) {
-    if (attOp.opcode) {
-      assem.append(attOp);
-    }
-    while (!attOpsNext.done) {
-      assem.append(attOpsNext.value);
-      attOpsNext = attOps.next();
+  const ops = (function* () {
+    yield* doCsOp();
+    if (optEnd === undefined) {
+      if (attOp.opcode) yield attOp;
+      while (!attOpsNext.done) {
+        yield attOpsNext.value;
+        attOpsNext = attOps.next();
+      }
+    } else {
+      csOp.opcode = '=';
+      csOp.chars = optEnd - start;
+      yield* doCsOp();
     }
-  } else {
-    csOp.opcode = '=';
-    csOp.chars = optEnd - start;
-    doCsOp();
-  }
-
-  return assem.toString();
+  })();
+  return exports.serializeOps(exports.canonicalizeOps(ops, false));
 };
 
 exports.inverse = (cs, lines, alines, pool) => {

src/static/js/ace2_inner.js

@@ -522,18 +522,24 @@ function Ace2Inner(editorInfo, cssManagers) {
     const numLines = rep.lines.length();
     const upToLastLine = rep.lines.offsetOfIndex(numLines - 1);
     const lastLineLength = rep.lines.atIndex(numLines - 1).text.length;
-    const assem = Changeset.smartOpAssembler();
-    const o = new Changeset.Op('-');
-    o.chars = upToLastLine;
-    o.lines = numLines - 1;
-    assem.append(o);
-    o.chars = lastLineLength;
-    o.lines = 0;
-    assem.append(o);
-    for (const op of Changeset.opsFromAText(atext)) assem.append(op);
-    const newLen = oldLen + assem.getLengthChange();
-    const changeset = Changeset.checkRep(
-        Changeset.pack(oldLen, newLen, assem.toString(), atext.text.slice(0, -1)));
+    const ops = (function* () {
+      const op1 = new Changeset.Op('-');
+      op1.chars = upToLastLine;
+      op1.lines = numLines - 1;
+      yield op1;
+      const op2 = new Changeset.Op('-');
+      op2.chars = lastLineLength;
+      op2.lines = 0;
+      yield op2;
+      yield* Changeset.opsFromAText(atext);
+    })();
+    let lengthChange;
+    const serializedOps = Changeset.serializeOps((function* () {
+      lengthChange = yield* Changeset.canonicalizeOps(ops, false);
+    })());
+    const newLen = oldLen + lengthChange;
+    const changeset =
+        Changeset.checkRep(Changeset.pack(oldLen, newLen, serializedOps, atext.text.slice(0, -1)));
     performDocumentApplyChangeset(changeset);
 
     performSelectionChange(