From 4a65c2c8ff6036222a2b2c48e5161c82c199e8c9 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Sun, 21 Mar 2021 14:13:50 -0400
Subject: [PATCH 01/22] Changeset: Unexport unnecessarily exported functions

These functions aren't used outside of this file.
---
 CHANGELOG.md                         |   4 +
 src/static/js/Changeset.js           | 121 ++++++++++++++-------------
 src/tests/frontend/specs/easysync.js |  14 ++--
 3 files changed, 75 insertions(+), 64 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5ce91f39d32..0cf6d1ded35 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -43,6 +43,10 @@
   * Changes to the `src/static/js/Changeset.js` library:
     * `opIterator()`: The unused start index parameter has been removed, as has
       the unused `lastIndex()` method on the returned object.
+    * Several functions that should have never been public are no longer
+      exported: `applyZip()`, `assert()`, `clearOp()`, `cloneOp()`, `copyOp()`,
+      `error()`, `followAttributes()`, `opString()`, `stringOp()`,
+      `textLinesMutator()`, `toBaseTen()`, `toSplices()`.
 
 ### Notable enhancements
 
diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js
index f43a236cf14..fa19623405e 100644
--- a/src/static/js/Changeset.js
+++ b/src/static/js/Changeset.js
@@ -35,7 +35,7 @@ const AttributePool = require('./AttributePool');
  *
  * @param {string} msg - Just some message
  */
-exports.error = (msg) => {
+const error = (msg) => {
   const e = new Error(msg);
   e.easysync = true;
   throw e;
@@ -49,8 +49,8 @@ exports.error = (msg) => {
  * @param {string} msg - error message to include in the exception
  * @type {(b: boolean, msg: string) => asserts b}
  */
-exports.assert = (b, msg) => {
-  if (!b) exports.error(`Failed assertion: ${msg}`);
+const assert = (b, msg) => {
+  if (!b) error(`Failed assertion: ${msg}`);
 };
 
 /**
@@ -147,7 +147,7 @@ exports.opIterator = (opsStr) => {
   const nextRegexMatch = () => {
     const result = regex.exec(opsStr);
     if (result[0] === '?') {
-      exports.error('Hit error opcode in op stream');
+      error('Hit error opcode in op stream');
     }
 
     return result;
@@ -163,7 +163,7 @@ exports.opIterator = (opsStr) => {
       op.chars = exports.parseNum(regexResult[4]);
       regexResult = nextRegexMatch();
     } else {
-      exports.clearOp(op);
+      clearOp(op);
     }
     return op;
   };
@@ -181,7 +181,7 @@ exports.opIterator = (opsStr) => {
  *
  * @param {Op} op - object to clear
  */
-exports.clearOp = (op) => {
+const clearOp = (op) => {
   op.opcode = '';
   op.chars = 0;
   op.lines = 0;
@@ -207,7 +207,7 @@ exports.newOp = (optOpcode) => ({
  * @param {Op} op1 - src Op
  * @param {Op} op2 - dest Op
  */
-exports.copyOp = (op1, op2) => {
+const copyOp = (op1, op2) => {
   op2.opcode = op1.opcode;
   op2.chars = op1.chars;
   op2.lines = op1.lines;
@@ -280,13 +280,13 @@ exports.checkRep = (cs) => {
         break;
       case '-':
         oldPos += o.chars;
-        exports.assert(oldPos <= oldLen, `${oldPos} > ${oldLen} in ${cs}`);
+        assert(oldPos <= oldLen, `${oldPos} > ${oldLen} in ${cs}`);
         break;
       case '+':
       {
         calcNewLen += o.chars;
         numInserted += o.chars;
-        exports.assert(calcNewLen <= newLen, `${calcNewLen} > ${newLen} in ${cs}`);
+        assert(calcNewLen <= newLen, `${calcNewLen} > ${newLen} in ${cs}`);
         break;
       }
     }
@@ -301,7 +301,7 @@ exports.checkRep = (cs) => {
 
   assem.endDocument();
   const normalized = exports.pack(oldLen, calcNewLen, assem.toString(), charBank);
-  exports.assert(normalized === cs, 'Invalid changeset (checkRep failed)');
+  assert(normalized === cs, 'Invalid changeset (checkRep failed)');
 
   return cs;
 };
@@ -458,7 +458,7 @@ exports.mergingOpAssembler = () => {
         }
       } else {
         flush();
-        exports.copyOp(op, bufOp);
+        copyOp(op, bufOp);
       }
     }
   };
@@ -474,7 +474,7 @@ exports.mergingOpAssembler = () => {
 
   const clear = () => {
     assem.clear();
-    exports.clearOp(bufOp);
+    clearOp(bufOp);
   };
   return {
     append,
@@ -536,7 +536,7 @@ exports.stringIterator = (str) => {
   const getnewLines = () => newLines;
 
   const assertRemaining = (n) => {
-    exports.assert(n <= remaining(), `!(${n} <= ${remaining()})`);
+    assert(n <= remaining(), `!(${n} <= ${remaining()})`);
   };
 
   const take = (n) => {
@@ -633,7 +633,7 @@ exports.stringAssembler = () => {
  * @param {(string[]|StringArrayLike)} lines - Lines to mutate (in place).
  * @returns {TextLinesMutator}
  */
-exports.textLinesMutator = (lines) => {
+const textLinesMutator = (lines) => {
   /**
    * curSplice holds values that will be passed as arguments to lines.splice() to insert, delete, or
    * change lines:
@@ -990,7 +990,7 @@ exports.textLinesMutator = (lines) => {
  *         other out), `opOut.opcode` MUST be set to the empty string.
  * @returns {string} the integrated changeset
  */
-exports.applyZip = (in1, in2, func) => {
+const applyZip = (in1, in2, func) => {
   const iter1 = exports.opIterator(in1);
   const iter2 = exports.opIterator(in2);
   const assem = exports.smartOpAssembler();
@@ -1020,7 +1020,7 @@ exports.unpack = (cs) => {
   const headerRegex = /Z:([0-9a-z]+)([><])([0-9a-z]+)|/;
   const headerMatch = headerRegex.exec(cs);
   if ((!headerMatch) || (!headerMatch[0])) {
-    exports.error(`Not a exports: ${cs}`);
+    error(`Not a exports: ${cs}`);
   }
   const oldLen = exports.parseNum(headerMatch[1]);
   const changeSign = (headerMatch[2] === '>') ? 1 : -1;
@@ -1064,8 +1064,7 @@ exports.pack = (oldLen, newLen, opsStr, bank) => {
  */
 exports.applyToText = (cs, str) => {
   const unpacked = exports.unpack(cs);
-  exports.assert(
-      str.length === unpacked.oldLen, `mismatched apply: ${str.length} / ${unpacked.oldLen}`);
+  assert(str.length === unpacked.oldLen, `mismatched apply: ${str.length} / ${unpacked.oldLen}`);
   const csIter = exports.opIterator(unpacked.ops);
   const bankIter = exports.stringIterator(unpacked.charBank);
   const strIter = exports.stringIterator(str);
@@ -1113,7 +1112,7 @@ exports.mutateTextLines = (cs, lines) => {
   const unpacked = exports.unpack(cs);
   const csIter = exports.opIterator(unpacked.ops);
   const bankIter = exports.stringIterator(unpacked.charBank);
-  const mut = exports.textLinesMutator(lines);
+  const mut = textLinesMutator(lines);
   while (csIter.hasNext()) {
     const op = csIter.next();
     switch (op.opcode) {
@@ -1205,12 +1204,12 @@ exports.composeAttributes = (att1, att2, resultIsMutation, pool) => {
  * @param {Op} opOut - Mutated to hold the result of applying `csOp` to `attOp`.
  * @param {AttributePool} pool - Can be null if definitely not needed.
  */
-exports._slicerZipperFunc = (attOp, csOp, opOut, pool) => {
+const slicerZipperFunc = (attOp, csOp, opOut, pool) => {
   if (attOp.opcode === '-') {
-    exports.copyOp(attOp, opOut);
+    copyOp(attOp, opOut);
     attOp.opcode = '';
   } else if (!attOp.opcode) {
-    exports.copyOp(csOp, opOut);
+    copyOp(csOp, opOut);
     csOp.opcode = '';
   } else {
     switch (csOp.opcode) {
@@ -1247,7 +1246,7 @@ exports._slicerZipperFunc = (attOp, csOp, opOut, pool) => {
       case '+':
       {
         // insert
-        exports.copyOp(csOp, opOut);
+        copyOp(csOp, opOut);
         csOp.opcode = '';
         break;
       }
@@ -1281,7 +1280,7 @@ exports._slicerZipperFunc = (attOp, csOp, opOut, pool) => {
       }
       case '':
       {
-        exports.copyOp(attOp, opOut);
+        copyOp(attOp, opOut);
         attOp.opcode = '';
         break;
       }
@@ -1300,8 +1299,8 @@ exports._slicerZipperFunc = (attOp, csOp, opOut, pool) => {
 exports.applyToAttribution = (cs, astr, pool) => {
   const unpacked = exports.unpack(cs);
 
-  return exports.applyZip(astr, unpacked.ops,
-      (op1, op2, opOut) => exports._slicerZipperFunc(op1, op2, opOut, pool));
+  return applyZip(astr, unpacked.ops,
+      (op1, op2, opOut) => slicerZipperFunc(op1, op2, opOut, pool));
 };
 
 exports.mutateAttributionLines = (cs, lines, pool) => {
@@ -1310,7 +1309,7 @@ exports.mutateAttributionLines = (cs, lines, pool) => {
   const csBank = unpacked.charBank;
   let csBankIndex = 0;
   // treat the attribution lines as text lines, mutating a line at a time
-  const mut = exports.textLinesMutator(lines);
+  const mut = textLinesMutator(lines);
 
   /** @type {?OpIter} */
   let lineIter = null;
@@ -1336,7 +1335,7 @@ exports.mutateAttributionLines = (cs, lines, pool) => {
     }
     lineAssem.append(op);
     if (op.lines > 0) {
-      exports.assert(op.lines === 1, `Can't have op.lines of ${op.lines} in attribution lines`);
+      assert(op.lines === 1, `Can't have op.lines of ${op.lines} in attribution lines`);
       // ship it to the mut
       mut.insert(lineAssem.toString(), 1);
       lineAssem = null;
@@ -1360,13 +1359,13 @@ exports.mutateAttributionLines = (cs, lines, pool) => {
     } else if (csOp.opcode === '+') {
       if (csOp.lines > 1) {
         const firstLineLen = csBank.indexOf('\n', csBankIndex) + 1 - csBankIndex;
-        exports.copyOp(csOp, opOut);
+        copyOp(csOp, opOut);
         csOp.chars -= firstLineLen;
         csOp.lines--;
         opOut.lines = 1;
         opOut.chars = firstLineLen;
       } else {
-        exports.copyOp(csOp, opOut);
+        copyOp(csOp, opOut);
         csOp.opcode = '';
       }
       outputMutOp(opOut);
@@ -1376,7 +1375,7 @@ exports.mutateAttributionLines = (cs, lines, pool) => {
       if ((!attOp.opcode) && isNextMutOp()) {
         nextMutOp(attOp);
       }
-      exports._slicerZipperFunc(attOp, csOp, opOut, pool);
+      slicerZipperFunc(attOp, csOp, opOut, pool);
       if (opOut.opcode) {
         outputMutOp(opOut);
         opOut.opcode = '';
@@ -1384,7 +1383,7 @@ exports.mutateAttributionLines = (cs, lines, pool) => {
     }
   }
 
-  exports.assert(!lineAssem, `line assembler not finished:${cs}`);
+  assert(!lineAssem, `line assembler not finished:${cs}`);
   mut.close();
 };
 
@@ -1427,7 +1426,7 @@ exports.splitAttributionLines = (attrOps, text) => {
     let numLines = op.lines;
     while (numLines > 1) {
       const newlineEnd = text.indexOf('\n', pos) + 1;
-      exports.assert(newlineEnd > 0, 'newlineEnd <= 0 in splitAttributionLines');
+      assert(newlineEnd > 0, 'newlineEnd <= 0 in splitAttributionLines');
       op.chars = newlineEnd - pos;
       op.lines = 1;
       appendOp(op);
@@ -1465,19 +1464,19 @@ exports.compose = (cs1, cs2, pool) => {
   const unpacked2 = exports.unpack(cs2);
   const len1 = unpacked1.oldLen;
   const len2 = unpacked1.newLen;
-  exports.assert(len2 === unpacked2.oldLen, 'mismatched composition of two changesets');
+  assert(len2 === unpacked2.oldLen, 'mismatched composition of two changesets');
   const len3 = unpacked2.newLen;
   const bankIter1 = exports.stringIterator(unpacked1.charBank);
   const bankIter2 = exports.stringIterator(unpacked2.charBank);
   const bankAssem = exports.stringAssembler();
 
-  const newOps = exports.applyZip(unpacked1.ops, unpacked2.ops, (op1, op2, opOut) => {
+  const newOps = applyZip(unpacked1.ops, unpacked2.ops, (op1, op2, opOut) => {
     const op1code = op1.opcode;
     const op2code = op2.opcode;
     if (op1code === '+' && op2code === '-') {
       bankIter1.skip(Math.min(op1.chars, op2.chars));
     }
-    exports._slicerZipperFunc(op1, op2, opOut, pool);
+    slicerZipperFunc(op1, op2, opOut, pool);
     if (opOut.opcode === '+') {
       if (op2code === '+') {
         bankAssem.append(bankIter2.take(opOut.chars));
@@ -1560,7 +1559,7 @@ exports.makeSplice = (oldFullText, spliceStart, numRemoved, newText, optNewTextA
  * @param {string} cs - Changeset
  * @returns {[number, number, string][]}
  */
-exports.toSplices = (cs) => {
+const toSplices = (cs) => {
   const unpacked = exports.unpack(cs);
   /** @type {[number, number, string][]} */
   const splices = [];
@@ -1601,7 +1600,7 @@ exports.toSplices = (cs) => {
 exports.characterRangeFollow = (cs, startChar, endChar, insertionsAfter) => {
   let newStartChar = startChar;
   let newEndChar = endChar;
-  const splices = exports.toSplices(cs);
+  const splices = toSplices(cs);
   let lengthChangeSoFar = 0;
   for (let i = 0; i < splices.length; i++) {
     const splice = splices[i];
@@ -1796,7 +1795,9 @@ exports.cloneAText = (atext) => {
       text: atext.text,
       attribs: atext.attribs,
     };
-  } else { exports.error('atext is null'); }
+  } else {
+    error('atext is null');
+  }
 };
 
 /**
@@ -2040,7 +2041,7 @@ exports.subattribution = (astr, start, optEnd) => {
           csOp.lines++;
         }
 
-        exports._slicerZipperFunc(attOp, csOp, opOut, null);
+        slicerZipperFunc(attOp, csOp, opOut, null);
         if (opOut.opcode) {
           assem.append(opOut);
           opOut.opcode = '';
@@ -2240,7 +2241,7 @@ exports.follow = (cs1, cs2, reverseInsertOrder, pool) => {
   const unpacked2 = exports.unpack(cs2);
   const len1 = unpacked1.oldLen;
   const len2 = unpacked2.oldLen;
-  exports.assert(len1 === len2, 'mismatched follow - cannot transform cs1 on top of cs2');
+  assert(len1 === len2, 'mismatched follow - cannot transform cs1 on top of cs2');
   const chars1 = exports.stringIterator(unpacked1.charBank);
   const chars2 = exports.stringIterator(unpacked2.charBank);
 
@@ -2250,7 +2251,7 @@ exports.follow = (cs1, cs2, reverseInsertOrder, pool) => {
 
   const hasInsertFirst = exports.attributeTester(['insertorder', 'first'], pool);
 
-  const newOps = exports.applyZip(unpacked1.ops, unpacked2.ops, (op1, op2, opOut) => {
+  const newOps = applyZip(unpacked1.ops, unpacked2.ops, (op1, op2, opOut) => {
     if (op1.opcode === '+' || op2.opcode === '+') {
       let whichToDo;
       if (op2.opcode !== '+') {
@@ -2289,7 +2290,7 @@ exports.follow = (cs1, cs2, reverseInsertOrder, pool) => {
       } else {
         // whichToDo == 2
         chars2.skip(op2.chars);
-        exports.copyOp(op2, opOut);
+        copyOp(op2, opOut);
         op2.opcode = '';
       }
     } else if (op1.opcode === '-') {
@@ -2308,7 +2309,7 @@ exports.follow = (cs1, cs2, reverseInsertOrder, pool) => {
         op2.opcode = '';
       }
     } else if (op2.opcode === '-') {
-      exports.copyOp(op2, opOut);
+      copyOp(op2, opOut);
       if (!op1.opcode) {
         op2.opcode = '';
       } else if (op2.chars <= op1.chars) {
@@ -2328,17 +2329,17 @@ exports.follow = (cs1, cs2, reverseInsertOrder, pool) => {
         op1.opcode = '';
       }
     } else if (!op1.opcode) {
-      exports.copyOp(op2, opOut);
+      copyOp(op2, opOut);
       op2.opcode = '';
     } else if (!op2.opcode) {
       // @NOTE: Critical bugfix for EPL issue #1625. We do not copy op1 here
       // in order to prevent attributes from leaking into result changesets.
-      // exports.copyOp(op1, opOut);
+      // copyOp(op1, opOut);
       op1.opcode = '';
     } else {
       // both keeps
       opOut.opcode = '=';
-      opOut.attribs = exports.followAttributes(op1.attribs, op2.attribs, pool);
+      opOut.attribs = followAttributes(op1.attribs, op2.attribs, pool);
       if (op1.chars <= op2.chars) {
         opOut.chars = op1.chars;
         opOut.lines = op1.lines;
@@ -2374,7 +2375,7 @@ exports.follow = (cs1, cs2, reverseInsertOrder, pool) => {
   return exports.pack(oldLen, newLen, newOps, unpacked2.charBank);
 };
 
-exports.followAttributes = (att1, att2, pool) => {
+const followAttributes = (att1, att2, pool) => {
   // The merge of two sets of attribute changes to the same text
   // takes the lexically-earlier value if there are two values
   // for the same key.  Otherwise, all key/value changes from
@@ -2416,19 +2417,19 @@ exports.composeWithDeletions = (cs1, cs2, pool) => {
   const unpacked2 = exports.unpack(cs2);
   const len1 = unpacked1.oldLen;
   const len2 = unpacked1.newLen;
-  exports.assert(len2 === unpacked2.oldLen, 'mismatched composition of two changesets');
+  assert(len2 === unpacked2.oldLen, 'mismatched composition of two changesets');
   const len3 = unpacked2.newLen;
   const bankIter1 = exports.stringIterator(unpacked1.charBank);
   const bankIter2 = exports.stringIterator(unpacked2.charBank);
   const bankAssem = exports.stringAssembler();
 
-  const newOps = exports.applyZip(unpacked1.ops, unpacked2.ops, (op1, op2, opOut) => {
+  const newOps = applyZip(unpacked1.ops, unpacked2.ops, (op1, op2, opOut) => {
     const op1code = op1.opcode;
     const op2code = op2.opcode;
     if (op1code === '+' && op2code === '-') {
       bankIter1.skip(Math.min(op1.chars, op2.chars));
     }
-    exports._slicerZipperFuncWithDeletions(op1, op2, opOut, pool);
+    slicerZipperFuncWithDeletions(op1, op2, opOut, pool);
     if (opOut.opcode === '+') {
       if (op2code === '+') {
         bankAssem.append(bankIter2.take(opOut.chars));
@@ -2441,19 +2442,19 @@ exports.composeWithDeletions = (cs1, cs2, pool) => {
   return exports.pack(len1, len3, newOps, bankAssem.toString());
 };
 
-// This function is 95% like _slicerZipperFunc, we just changed two lines to
+// This function is 95% like slicerZipperFunc, we just changed two lines to
 // ensure it merges the attribs of deletions properly.
 // This is necassary for correct paddiff. But to ensure these changes doesn't
 // affect anything else, we've created a seperate function only used for paddiffs
-exports._slicerZipperFuncWithDeletions = (attOp, csOp, opOut, pool) => {
+const slicerZipperFuncWithDeletions = (attOp, csOp, opOut, pool) => {
   // attOp is the op from the sequence that is being operated on, either an
   // attribution string or the earlier of two exportss being composed.
   // pool can be null if definitely not needed.
   if (attOp.opcode === '-') {
-    exports.copyOp(attOp, opOut);
+    copyOp(attOp, opOut);
     attOp.opcode = '';
   } else if (!attOp.opcode) {
-    exports.copyOp(csOp, opOut);
+    copyOp(csOp, opOut);
     csOp.opcode = '';
   } else {
     switch (csOp.opcode) {
@@ -2490,7 +2491,7 @@ exports._slicerZipperFuncWithDeletions = (attOp, csOp, opOut, pool) => {
       case '+':
       {
         // insert
-        exports.copyOp(csOp, opOut);
+        copyOp(csOp, opOut);
         csOp.opcode = '';
         break;
       }
@@ -2524,10 +2525,16 @@ exports._slicerZipperFuncWithDeletions = (attOp, csOp, opOut, pool) => {
       }
       case '':
       {
-        exports.copyOp(attOp, opOut);
+        copyOp(attOp, opOut);
         attOp.opcode = '';
         break;
       }
     }
   }
 };
+
+exports.exportedForTestingOnly = {
+  followAttributes,
+  textLinesMutator,
+  toSplices,
+};
diff --git a/src/tests/frontend/specs/easysync.js b/src/tests/frontend/specs/easysync.js
index 5c4c47ae43d..121218407e4 100644
--- a/src/tests/frontend/specs/easysync.js
+++ b/src/tests/frontend/specs/easysync.js
@@ -92,7 +92,7 @@ describe('easysync', function () {
   const runMutationTest = (testId, origLines, muts, correct) => {
     it(`runMutationTest#${testId}`, async function () {
       let lines = origLines.slice();
-      const mu = Changeset.textLinesMutator(lines);
+      const mu = Changeset.exportedForTestingOnly.textLinesMutator(lines);
       applyMutations(mu, muts);
       mu.close();
       expect(lines).to.eql(correct);
@@ -211,7 +211,7 @@ describe('easysync', function () {
     const lines = ['1\n', '2\n', '3\n', '4\n'];
     let mu;
 
-    mu = Changeset.textLinesMutator(lines);
+    mu = Changeset.exportedForTestingOnly.textLinesMutator(lines);
     expect(mu.hasMore()).to.be(true);
     mu.skip(8, 4);
     expect(mu.hasMore()).to.be(false);
@@ -219,7 +219,7 @@ describe('easysync', function () {
     expect(mu.hasMore()).to.be(false);
 
     // still 1,2,3,4
-    mu = Changeset.textLinesMutator(lines);
+    mu = Changeset.exportedForTestingOnly.textLinesMutator(lines);
     expect(mu.hasMore()).to.be(true);
     mu.remove(2, 1);
     expect(mu.hasMore()).to.be(true);
@@ -235,7 +235,7 @@ describe('easysync', function () {
     expect(mu.hasMore()).to.be(false);
 
     // 2,3,4,5 now
-    mu = Changeset.textLinesMutator(lines);
+    mu = Changeset.exportedForTestingOnly.textLinesMutator(lines);
     expect(mu.hasMore()).to.be(true);
     mu.remove(6, 3);
     expect(mu.hasMore()).to.be(true);
@@ -610,8 +610,8 @@ describe('easysync', function () {
 
     const testFollow = (a, b, afb, bfa, merge) => {
       it(`testFollow manual #${++n}`, async function () {
-        expect(Changeset.followAttributes(a, b, p)).to.equal(afb);
-        expect(Changeset.followAttributes(b, a, p)).to.equal(bfa);
+        expect(Changeset.exportedForTestingOnly.followAttributes(a, b, p)).to.equal(afb);
+        expect(Changeset.exportedForTestingOnly.followAttributes(b, a, p)).to.equal(bfa);
         expect(Changeset.composeAttributes(a, afb, true, p)).to.equal(merge);
         expect(Changeset.composeAttributes(b, bfa, true, p)).to.equal(merge);
       });
@@ -701,7 +701,7 @@ describe('easysync', function () {
       [5, 8, '123456789'],
       [9, 17, 'abcdefghijk'],
     ];
-    expect(Changeset.toSplices(cs)).to.eql(correctSplices);
+    expect(Changeset.exportedForTestingOnly.toSplices(cs)).to.eql(correctSplices);
   });
 
   const testCharacterRangeFollow = (testId, cs, oldRange, insertionsAfter, correctNewRange) => {

From b29e59419e2aabe6f85fe37fa3d4a287b0b9796e Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Sat, 16 Oct 2021 17:54:03 -0400
Subject: [PATCH 02/22] Changeset: Factor out duplicate code

---
 src/node/utils/padDiff.js  |   2 +-
 src/static/js/Changeset.js | 131 +++----------------------------------
 2 files changed, 9 insertions(+), 124 deletions(-)

diff --git a/src/node/utils/padDiff.js b/src/node/utils/padDiff.js
index 67c5f68ffd8..193f24458ec 100644
--- a/src/node/utils/padDiff.js
+++ b/src/node/utils/padDiff.js
@@ -160,7 +160,7 @@ PadDiff.prototype._createDiffAtext = async function () {
       if (superChangeset == null) {
         superChangeset = changeset;
       } else {
-        superChangeset = Changeset.composeWithDeletions(superChangeset, changeset, this._pad.pool);
+        superChangeset = Changeset.compose(superChangeset, changeset, this._pad.pool);
       }
     }
 
diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js
index fa19623405e..fda143021f6 100644
--- a/src/static/js/Changeset.js
+++ b/src/static/js/Changeset.js
@@ -1221,7 +1221,10 @@ const slicerZipperFunc = (attOp, csOp, opOut, pool) => {
             opOut.opcode = '-';
             opOut.chars = csOp.chars;
             opOut.lines = csOp.lines;
-            opOut.attribs = '';
+            // csOp is a remove op and remove ops normally never have any attributes, so this should
+            // normally be the empty string. However, padDiff.js adds attributes to remove ops and
+            // needs them preserved so they are copied here.
+            opOut.attribs = csOp.attribs;
           }
           attOp.chars -= csOp.chars;
           attOp.lines -= csOp.lines;
@@ -1235,7 +1238,10 @@ const slicerZipperFunc = (attOp, csOp, opOut, pool) => {
             opOut.opcode = '-';
             opOut.chars = attOp.chars;
             opOut.lines = attOp.lines;
-            opOut.attribs = '';
+            // csOp is a remove op and remove ops normally never have any attributes, so this should
+            // normally be the empty string. However, padDiff.js adds attributes to remove ops and
+            // needs them preserved so they are copied here.
+            opOut.attribs = csOp.attribs;
           }
           csOp.chars -= attOp.chars;
           csOp.lines -= attOp.lines;
@@ -2412,127 +2418,6 @@ const followAttributes = (att1, att2, pool) => {
   return buf.toString();
 };
 
-exports.composeWithDeletions = (cs1, cs2, pool) => {
-  const unpacked1 = exports.unpack(cs1);
-  const unpacked2 = exports.unpack(cs2);
-  const len1 = unpacked1.oldLen;
-  const len2 = unpacked1.newLen;
-  assert(len2 === unpacked2.oldLen, 'mismatched composition of two changesets');
-  const len3 = unpacked2.newLen;
-  const bankIter1 = exports.stringIterator(unpacked1.charBank);
-  const bankIter2 = exports.stringIterator(unpacked2.charBank);
-  const bankAssem = exports.stringAssembler();
-
-  const newOps = applyZip(unpacked1.ops, unpacked2.ops, (op1, op2, opOut) => {
-    const op1code = op1.opcode;
-    const op2code = op2.opcode;
-    if (op1code === '+' && op2code === '-') {
-      bankIter1.skip(Math.min(op1.chars, op2.chars));
-    }
-    slicerZipperFuncWithDeletions(op1, op2, opOut, pool);
-    if (opOut.opcode === '+') {
-      if (op2code === '+') {
-        bankAssem.append(bankIter2.take(opOut.chars));
-      } else {
-        bankAssem.append(bankIter1.take(opOut.chars));
-      }
-    }
-  });
-
-  return exports.pack(len1, len3, newOps, bankAssem.toString());
-};
-
-// This function is 95% like slicerZipperFunc, we just changed two lines to
-// ensure it merges the attribs of deletions properly.
-// This is necassary for correct paddiff. But to ensure these changes doesn't
-// affect anything else, we've created a seperate function only used for paddiffs
-const slicerZipperFuncWithDeletions = (attOp, csOp, opOut, pool) => {
-  // attOp is the op from the sequence that is being operated on, either an
-  // attribution string or the earlier of two exportss being composed.
-  // pool can be null if definitely not needed.
-  if (attOp.opcode === '-') {
-    copyOp(attOp, opOut);
-    attOp.opcode = '';
-  } else if (!attOp.opcode) {
-    copyOp(csOp, opOut);
-    csOp.opcode = '';
-  } else {
-    switch (csOp.opcode) {
-      case '-':
-      {
-        if (csOp.chars <= attOp.chars) {
-          // delete or delete part
-          if (attOp.opcode === '=') {
-            opOut.opcode = '-';
-            opOut.chars = csOp.chars;
-            opOut.lines = csOp.lines;
-            opOut.attribs = csOp.attribs; // changed by yammer
-          }
-          attOp.chars -= csOp.chars;
-          attOp.lines -= csOp.lines;
-          csOp.opcode = '';
-          if (!attOp.chars) {
-            attOp.opcode = '';
-          }
-        } else {
-          // delete and keep going
-          if (attOp.opcode === '=') {
-            opOut.opcode = '-';
-            opOut.chars = attOp.chars;
-            opOut.lines = attOp.lines;
-            opOut.attribs = csOp.attribs; // changed by yammer
-          }
-          csOp.chars -= attOp.chars;
-          csOp.lines -= attOp.lines;
-          attOp.opcode = '';
-        }
-        break;
-      }
-      case '+':
-      {
-        // insert
-        copyOp(csOp, opOut);
-        csOp.opcode = '';
-        break;
-      }
-      case '=':
-      {
-        if (csOp.chars <= attOp.chars) {
-          // keep or keep part
-          opOut.opcode = attOp.opcode;
-          opOut.chars = csOp.chars;
-          opOut.lines = csOp.lines;
-          opOut.attribs = exports.composeAttributes(
-              attOp.attribs, csOp.attribs, attOp.opcode === '=', pool);
-          csOp.opcode = '';
-          attOp.chars -= csOp.chars;
-          attOp.lines -= csOp.lines;
-          if (!attOp.chars) {
-            attOp.opcode = '';
-          }
-        } else {
-          // keep and keep going
-          opOut.opcode = attOp.opcode;
-          opOut.chars = attOp.chars;
-          opOut.lines = attOp.lines;
-          opOut.attribs = exports.composeAttributes(
-              attOp.attribs, csOp.attribs, attOp.opcode === '=', pool);
-          attOp.opcode = '';
-          csOp.chars -= attOp.chars;
-          csOp.lines -= attOp.lines;
-        }
-        break;
-      }
-      case '':
-      {
-        copyOp(attOp, opOut);
-        attOp.opcode = '';
-        break;
-      }
-    }
-  }
-};
-
 exports.exportedForTestingOnly = {
   followAttributes,
   textLinesMutator,

From 7fa9b0711621448b3aa289ed86dc137b4e7fd556 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Mon, 27 Sep 2021 17:25:31 -0400
Subject: [PATCH 03/22] Changeset: Invert conditions to improve readability

---
 src/static/js/Changeset.js | 369 +++++++++++++++++--------------------
 1 file changed, 168 insertions(+), 201 deletions(-)

diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js
index fda143021f6..b5e72f47710 100644
--- a/src/static/js/Changeset.js
+++ b/src/static/js/Changeset.js
@@ -425,41 +425,39 @@ exports.mergingOpAssembler = () => {
   let bufOpAdditionalCharsAfterNewline = 0;
 
   const flush = (isEndDocument) => {
-    if (bufOp.opcode) {
-      if (isEndDocument && bufOp.opcode === '=' && !bufOp.attribs) {
-        // final merged keep, leave it implicit
-      } else {
+    if (!bufOp.opcode) return;
+    if (isEndDocument && bufOp.opcode === '=' && !bufOp.attribs) {
+      // final merged keep, leave it implicit
+    } else {
+      assem.append(bufOp);
+      if (bufOpAdditionalCharsAfterNewline) {
+        bufOp.chars = bufOpAdditionalCharsAfterNewline;
+        bufOp.lines = 0;
         assem.append(bufOp);
-        if (bufOpAdditionalCharsAfterNewline) {
-          bufOp.chars = bufOpAdditionalCharsAfterNewline;
-          bufOp.lines = 0;
-          assem.append(bufOp);
-          bufOpAdditionalCharsAfterNewline = 0;
-        }
+        bufOpAdditionalCharsAfterNewline = 0;
       }
-      bufOp.opcode = '';
     }
+    bufOp.opcode = '';
   };
 
   const append = (op) => {
-    if (op.chars > 0) {
-      if (bufOp.opcode === op.opcode && bufOp.attribs === op.attribs) {
-        if (op.lines > 0) {
-          // bufOp and additional chars are all mergeable into a multi-line op
-          bufOp.chars += bufOpAdditionalCharsAfterNewline + op.chars;
-          bufOp.lines += op.lines;
-          bufOpAdditionalCharsAfterNewline = 0;
-        } else if (bufOp.lines === 0) {
-          // both bufOp and op are in-line
-          bufOp.chars += op.chars;
-        } else {
-          // append in-line text to multi-line bufOp
-          bufOpAdditionalCharsAfterNewline += op.chars;
-        }
+    if (op.chars <= 0) return;
+    if (bufOp.opcode === op.opcode && bufOp.attribs === op.attribs) {
+      if (op.lines > 0) {
+        // bufOp and additional chars are all mergeable into a multi-line op
+        bufOp.chars += bufOpAdditionalCharsAfterNewline + op.chars;
+        bufOp.lines += op.lines;
+        bufOpAdditionalCharsAfterNewline = 0;
+      } else if (bufOp.lines === 0) {
+        // both bufOp and op are in-line
+        bufOp.chars += op.chars;
       } else {
-        flush();
-        copyOp(op, bufOp);
+        // append in-line text to multi-line bufOp
+        bufOpAdditionalCharsAfterNewline += op.chars;
       }
+    } else {
+      flush();
+      copyOp(op, bufOp);
     }
   };
 
@@ -762,31 +760,28 @@ const textLinesMutator = (lines) => {
    * @param {boolean} includeInSplice - indicates if attributes are present
    */
   const skipLines = (L, includeInSplice) => {
-    if (L) {
-      if (includeInSplice) {
-        if (!inSplice) {
-          enterSplice();
-        }
-        // TODO(doc) should this count the number of characters that are skipped to check?
-        for (let i = 0; i < L; i++) {
-          curCol = 0;
+    if (!L) return;
+    if (includeInSplice) {
+      if (!inSplice) enterSplice();
+      // TODO(doc) should this count the number of characters that are skipped to check?
+      for (let i = 0; i < L; i++) {
+        curCol = 0;
+        putCurLineInSplice();
+        curLine++;
+      }
+    } else {
+      if (inSplice) {
+        if (L > 1) {
+          // TODO(doc) figure out why single lines are incorporated into splice instead of ignored
+          leaveSplice();
+        } else {
           putCurLineInSplice();
-          curLine++;
-        }
-      } else {
-        if (inSplice) {
-          if (L > 1) {
-            // TODO(doc) figure out why single lines are incorporated into splice instead of ignored
-            leaveSplice();
-          } else {
-            putCurLineInSplice();
-          }
         }
-        curLine += L;
-        curCol = 0;
       }
-      // tests case foo in remove(), which isn't otherwise covered in current impl
+      curLine += L;
+      curCol = 0;
     }
+    // tests case foo in remove(), which isn't otherwise covered in current impl
   };
 
   /**
@@ -797,20 +792,17 @@ const textLinesMutator = (lines) => {
    * @param {boolean} includeInSplice - indicates if attributes are present
    */
   const skip = (N, L, includeInSplice) => {
-    if (N) {
-      if (L) {
-        skipLines(L, includeInSplice);
-      } else {
-        if (includeInSplice && !inSplice) {
-          enterSplice();
-        }
-        if (inSplice) {
-          // although the line is put into splice curLine is not increased, because
-          // only some chars are skipped, not the whole line
-          putCurLineInSplice();
-        }
-        curCol += N;
+    if (!N) return;
+    if (L) {
+      skipLines(L, includeInSplice);
+    } else {
+      if (includeInSplice && !inSplice) enterSplice();
+      if (inSplice) {
+        // although the line is put into splice curLine is not increased, because
+        // only some chars are skipped, not the whole line
+        putCurLineInSplice();
       }
+      curCol += N;
     }
   };
 
@@ -821,41 +813,39 @@ const textLinesMutator = (lines) => {
    * @returns {string}
    */
   const removeLines = (L) => {
-    let removed = '';
-    if (L) {
-      if (!inSplice) {
-        enterSplice();
-      }
+    if (!L) return '';
+    if (!inSplice) enterSplice();
 
-      /**
-       * Gets a string of joined lines after the end of the splice.
-       *
-       * @param {number} k - number of lines
-       * @returns {string} joined lines
-       */
-      const nextKLinesText = (k) => {
-        const m = curSplice[0] + curSplice[1];
-        return linesSlice(m, m + k).join('');
-      };
-      if (isCurLineInSplice()) {
-        if (curCol === 0) {
-          removed = curSplice[curSplice.length - 1];
-          curSplice.length--;
-          removed += nextKLinesText(L - 1);
-          curSplice[1] += L - 1;
-        } else {
-          removed = nextKLinesText(L - 1);
-          curSplice[1] += L - 1;
-          const sline = curSplice.length - 1;
-          removed = curSplice[sline].substring(curCol) + removed;
-          curSplice[sline] = curSplice[sline].substring(0, curCol) +
-              linesGet(curSplice[0] + curSplice[1]);
-          curSplice[1] += 1;
-        }
+    /**
+     * Gets a string of joined lines after the end of the splice.
+     *
+     * @param {number} k - number of lines
+     * @returns {string} joined lines
+     */
+    const nextKLinesText = (k) => {
+      const m = curSplice[0] + curSplice[1];
+      return linesSlice(m, m + k).join('');
+    };
+
+    let removed = '';
+    if (isCurLineInSplice()) {
+      if (curCol === 0) {
+        removed = curSplice[curSplice.length - 1];
+        curSplice.length--;
+        removed += nextKLinesText(L - 1);
+        curSplice[1] += L - 1;
       } else {
-        removed = nextKLinesText(L);
-        curSplice[1] += L;
+        removed = nextKLinesText(L - 1);
+        curSplice[1] += L - 1;
+        const sline = curSplice.length - 1;
+        removed = curSplice[sline].substring(curCol) + removed;
+        curSplice[sline] = curSplice[sline].substring(0, curCol) +
+            linesGet(curSplice[0] + curSplice[1]);
+        curSplice[1] += 1;
       }
+    } else {
+      removed = nextKLinesText(L);
+      curSplice[1] += L;
     }
     return removed;
   };
@@ -868,22 +858,15 @@ const textLinesMutator = (lines) => {
    * @returns {string}
    */
   const remove = (N, L) => {
-    let removed = '';
-    if (N) {
-      if (L) {
-        return removeLines(L);
-      } else {
-        if (!inSplice) {
-          enterSplice();
-        }
-        // although the line is put into splice, curLine is not increased, because
-        // only some chars are removed not the whole line
-        const sline = putCurLineInSplice();
-        removed = curSplice[sline].substring(curCol, curCol + N);
-        curSplice[sline] = curSplice[sline].substring(0, curCol) +
-            curSplice[sline].substring(curCol + N);
-      }
-    }
+    if (!N) return '';
+    if (L) return removeLines(L);
+    if (!inSplice) enterSplice();
+    // although the line is put into splice, curLine is not increased, because
+    // only some chars are removed not the whole line
+    const sline = putCurLineInSplice();
+    const removed = curSplice[sline].substring(curCol, curCol + N);
+    curSplice[sline] = curSplice[sline].substring(0, curCol) +
+        curSplice[sline].substring(curCol + N);
     return removed;
   };
 
@@ -894,44 +877,41 @@ const textLinesMutator = (lines) => {
    * @param {number} L - number of newlines in text
    */
   const insert = (text, L) => {
-    if (text) {
-      if (!inSplice) {
-        enterSplice();
-      }
-      if (L) {
-        const newLines = exports.splitTextLines(text);
-        if (isCurLineInSplice()) {
-          const sline = curSplice.length - 1;
-          /** @type {string} */
-          const theLine = curSplice[sline];
-          const lineCol = curCol;
-          // insert the first new line
-          curSplice[sline] = theLine.substring(0, lineCol) + newLines[0];
-          curLine++;
-          newLines.splice(0, 1);
-          // insert the remaining new lines
-          Array.prototype.push.apply(curSplice, newLines);
-          curLine += newLines.length;
-          // insert the remaining chars from the "old" line (e.g. the line we were in
-          // when we started to insert new lines)
-          curSplice.push(theLine.substring(lineCol));
-          curCol = 0; // TODO(doc) why is this not set to the length of last line?
-        } else {
-          Array.prototype.push.apply(curSplice, newLines);
-          curLine += newLines.length;
-        }
+    if (!text) return;
+    if (!inSplice) enterSplice();
+    if (L) {
+      const newLines = exports.splitTextLines(text);
+      if (isCurLineInSplice()) {
+        const sline = curSplice.length - 1;
+        /** @type {string} */
+        const theLine = curSplice[sline];
+        const lineCol = curCol;
+        // insert the first new line
+        curSplice[sline] = theLine.substring(0, lineCol) + newLines[0];
+        curLine++;
+        newLines.splice(0, 1);
+        // insert the remaining new lines
+        Array.prototype.push.apply(curSplice, newLines);
+        curLine += newLines.length;
+        // insert the remaining chars from the "old" line (e.g. the line we were in
+        // when we started to insert new lines)
+        curSplice.push(theLine.substring(lineCol));
+        curCol = 0; // TODO(doc) why is this not set to the length of last line?
       } else {
-        // there are no additional lines
-        // although the line is put into splice, curLine is not increased, because
-        // there may be more chars in the line (newline is not reached)
-        const sline = putCurLineInSplice();
-        if (!curSplice[sline]) {
-          console.error('curSplice[sline] not populated, actual curSplice contents is ', curSplice, '. Possibly related to https://github.com/ether/etherpad-lite/issues/2802');
-        }
-        curSplice[sline] = curSplice[sline].substring(0, curCol) + text +
-            curSplice[sline].substring(curCol);
-        curCol += text.length;
+        Array.prototype.push.apply(curSplice, newLines);
+        curLine += newLines.length;
+      }
+    } else {
+      // there are no additional lines
+      // although the line is put into splice, curLine is not increased, because
+      // there may be more chars in the line (newline is not reached)
+      const sline = putCurLineInSplice();
+      if (!curSplice[sline]) {
+        console.error('curSplice[sline] not populated, actual curSplice contents is ', curSplice, '. Possibly related to https://github.com/ether/etherpad-lite/issues/2802');
       }
+      curSplice[sline] = curSplice[sline].substring(0, curCol) + text +
+          curSplice[sline].substring(curCol);
+      curCol += text.length;
     }
   };
 
@@ -1171,15 +1151,14 @@ exports.composeAttributes = (att1, att2, resultIsMutation, pool) => {
     let found = false;
     for (let i = 0; i < atts.length; i++) {
       const oldPair = atts[i];
-      if (oldPair[0] === pair[0]) {
-        if (pair[1] || resultIsMutation) {
-          oldPair[1] = pair[1];
-        } else {
-          atts.splice(i, 1);
-        }
-        found = true;
-        break;
+      if (oldPair[0] !== pair[0]) continue;
+      if (pair[1] || resultIsMutation) {
+        oldPair[1] = pair[1];
+      } else {
+        atts.splice(i, 1);
       }
+      found = true;
+      break;
     }
     if ((!found) && (pair[1] || resultIsMutation)) {
       atts.push(pair);
@@ -1340,12 +1319,11 @@ exports.mutateAttributionLines = (cs, lines, pool) => {
       lineAssem = exports.mergingOpAssembler();
     }
     lineAssem.append(op);
-    if (op.lines > 0) {
-      assert(op.lines === 1, `Can't have op.lines of ${op.lines} in attribution lines`);
-      // ship it to the mut
-      mut.insert(lineAssem.toString(), 1);
-      lineAssem = null;
-    }
+    if (op.lines <= 0) return;
+    assert(op.lines === 1, `Can't have op.lines of ${op.lines} in attribution lines`);
+    // ship it to the mut
+    mut.insert(lineAssem.toString(), 1);
+    lineAssem = null;
   };
 
   const csOp = exports.newOp();
@@ -1505,16 +1483,11 @@ exports.compose = (cs1, cs2, pool) => {
  */
 exports.attributeTester = (attribPair, pool) => {
   const never = (attribs) => false;
-  if (!pool) {
-    return never;
-  }
+  if (!pool) return never;
   const attribNum = pool.putAttrib(attribPair, true);
-  if (attribNum < 0) {
-    return never;
-  } else {
-    const re = new RegExp(`\\*${exports.numToString(attribNum)}(?!\\w)`);
-    return (attribs) => re.test(attribs);
-  }
+  if (attribNum < 0) return never;
+  const re = new RegExp(`\\*${exports.numToString(attribNum)}(?!\\w)`);
+  return (attribs) => re.test(attribs);
 };
 
 /**
@@ -1796,14 +1769,11 @@ exports.applyToAText = (cs, atext, pool) => ({
  * @returns {AText}
  */
 exports.cloneAText = (atext) => {
-  if (atext) {
-    return {
-      text: atext.text,
-      attribs: atext.attribs,
-    };
-  } else {
-    error('atext is null');
-  }
+  if (!atext) error('atext is null');
+  return {
+    text: atext.text,
+    attribs: atext.attribs,
+  };
 };
 
 /**
@@ -1902,14 +1872,13 @@ exports.opAttributeValue = (op, key, pool) => exports.attribsAttributeValue(op.a
  * @returns {string}
  */
 exports.attribsAttributeValue = (attribs, key, pool) => {
+  if (!attribs) return '';
   let value = '';
-  if (attribs) {
-    exports.eachAttribNumber(attribs, (n) => {
-      if (pool.getAttribKey(n) === key) {
-        value = pool.getAttribValue(n);
-      }
-    });
-  }
+  exports.eachAttribNumber(attribs, (n) => {
+    if (pool.getAttribKey(n) === key) {
+      value = pool.getAttribValue(n);
+    }
+  });
   return value;
 };
 
@@ -2038,20 +2007,19 @@ exports.subattribution = (astr, start, optEnd) => {
   const opOut = exports.newOp();
 
   const doCsOp = () => {
-    if (csOp.chars) {
-      while (csOp.opcode && (attOp.opcode || iter.hasNext())) {
-        if (!attOp.opcode) iter.next(attOp);
+    if (!csOp.chars) return;
+    while (csOp.opcode && (attOp.opcode || iter.hasNext())) {
+      if (!attOp.opcode) iter.next(attOp);
 
-        if (csOp.opcode && attOp.opcode && csOp.chars >= attOp.chars &&
-              attOp.lines > 0 && csOp.lines <= 0) {
-          csOp.lines++;
-        }
+      if (csOp.opcode && attOp.opcode && csOp.chars >= attOp.chars &&
+          attOp.lines > 0 && csOp.lines <= 0) {
+        csOp.lines++;
+      }
 
-        slicerZipperFunc(attOp, csOp, opOut, null);
-        if (opOut.opcode) {
-          assem.append(opOut);
-          opOut.opcode = '';
-        }
+      slicerZipperFunc(attOp, csOp, opOut, null);
+      if (opOut.opcode) {
+        assem.append(opOut);
+        opOut.opcode = '';
       }
     }
   };
@@ -2399,13 +2367,12 @@ const followAttributes = (att1, att2, pool) => {
     const pair1 = pool.getAttrib(exports.parseNum(a));
     for (let i = 0; i < atts.length; i++) {
       const pair2 = atts[i];
-      if (pair1[0] === pair2[0]) {
-        if (pair1[1] <= pair2[1]) {
-          // winner of merge is pair1, delete this attribute
-          atts.splice(i, 1);
-        }
-        break;
+      if (pair1[0] !== pair2[0]) continue;
+      if (pair1[1] <= pair2[1]) {
+        // winner of merge is pair1, delete this attribute
+        atts.splice(i, 1);
       }
+      break;
     }
     return '';
   });

From 37bb297e76a3da6939941fcdd143dac9c06f333e Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Thu, 4 Mar 2021 20:37:37 -0500
Subject: [PATCH 04/22] Changeset: Improve logged error message

I saw this on a production system today and wanted more information.
---
 src/static/js/Changeset.js | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js
index b5e72f47710..bdf25f7de89 100644
--- a/src/static/js/Changeset.js
+++ b/src/static/js/Changeset.js
@@ -907,7 +907,11 @@ const textLinesMutator = (lines) => {
       // there may be more chars in the line (newline is not reached)
       const sline = putCurLineInSplice();
       if (!curSplice[sline]) {
-        console.error('curSplice[sline] not populated, actual curSplice contents is ', curSplice, '. Possibly related to https://github.com/ether/etherpad-lite/issues/2802');
+        const err = new Error(
+            'curSplice[sline] not populated, actual curSplice contents is ' +
+            `${JSON.stringify(curSplice)}. Possibly related to ` +
+            'https://github.com/ether/etherpad-lite/issues/2802');
+        console.error(err.stack || err.toString());
       }
       curSplice[sline] = curSplice[sline].substring(0, curCol) + text +
           curSplice[sline].substring(curCol);

From 7ec0d5f385b9e27077b311a48d604f91c5aedfbc Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Sun, 21 Mar 2021 14:58:05 -0400
Subject: [PATCH 05/22] Changeset: Remove unnecessary `linesApplySplice()`

---
 src/static/js/Changeset.js | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js
index bdf25f7de89..220a3947bb2 100644
--- a/src/static/js/Changeset.js
+++ b/src/static/js/Changeset.js
@@ -654,15 +654,6 @@ const textLinesMutator = (lines) => {
   // invariant: if (inSplice && (curLine >= curSplice[0] + curSplice.length - 2)) then
   //            curCol == 0
 
-  /**
-   * Adds and/or removes entries at a specific offset in `lines`. Called when leaving the splice.
-   *
-   * @param {[number, number?, ...string[]?]} s - curSplice
-   */
-  const linesApplySplice = (s) => {
-    lines.splice(...s);
-  };
-
   /**
    * Get a line from `lines` at given index.
    *
@@ -724,7 +715,7 @@ const textLinesMutator = (lines) => {
    * close or TODO(doc).
    */
   const leaveSplice = () => {
-    linesApplySplice(curSplice);
+    lines.splice(...curSplice);
     curSplice.length = 2;
     curSplice[0] = curSplice[1] = 0;
     inSplice = false;

From 18a6b7279cb97deaa3f12c636365277eeacddbe1 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Thu, 21 Oct 2021 03:35:03 -0400
Subject: [PATCH 06/22] Changeset: Only pass strings to `parseNum()`

---
 src/static/js/Changeset.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js
index 220a3947bb2..020c34e10b5 100644
--- a/src/static/js/Changeset.js
+++ b/src/static/js/Changeset.js
@@ -158,7 +158,7 @@ exports.opIterator = (opsStr) => {
     const op = optOp || exports.newOp();
     if (regexResult[0]) {
       op.attribs = regexResult[1];
-      op.lines = exports.parseNum(regexResult[2] || 0);
+      op.lines = exports.parseNum(regexResult[2] || '0');
       op.opcode = regexResult[3];
       op.chars = exports.parseNum(regexResult[4]);
       regexResult = nextRegexMatch();

From 02ef78e174f6ac38fb76b8031bff583ab16f7582 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Sat, 2 Oct 2021 18:05:24 -0400
Subject: [PATCH 07/22] Changeset: Make sure `opOut` is cleared

`slicerZipperFunc()` previously assumed the provided `opOut` argument
was a null Op. Enforce this by clearing it at the beginning.
---
 src/static/js/Changeset.js | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js
index 020c34e10b5..9dc10170172 100644
--- a/src/static/js/Changeset.js
+++ b/src/static/js/Changeset.js
@@ -1179,6 +1179,7 @@ exports.composeAttributes = (att1, att2, resultIsMutation, pool) => {
  * @param {AttributePool} pool - Can be null if definitely not needed.
  */
 const slicerZipperFunc = (attOp, csOp, opOut, pool) => {
+  clearOp(opOut);
   if (attOp.opcode === '-') {
     copyOp(attOp, opOut);
     attOp.opcode = '';
@@ -2221,6 +2222,7 @@ exports.follow = (cs1, cs2, reverseInsertOrder, pool) => {
   const hasInsertFirst = exports.attributeTester(['insertorder', 'first'], pool);
 
   const newOps = applyZip(unpacked1.ops, unpacked2.ops, (op1, op2, opOut) => {
+    clearOp(opOut);
     if (op1.opcode === '+' || op2.opcode === '+') {
       let whichToDo;
       if (op2.opcode !== '+') {

From 94f550767171f9986b84825de1f6d01c9e690873 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Sat, 2 Oct 2021 18:17:11 -0400
Subject: [PATCH 08/22] Changeset: Improve `copyOp()` API

Use `Object.assign()` to implement `copyOp()`, which simplifies the
code and provides a return value. Also make the second op optional.
---
 src/static/js/Changeset.js | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js
index 9dc10170172..4d3c30db527 100644
--- a/src/static/js/Changeset.js
+++ b/src/static/js/Changeset.js
@@ -205,14 +205,10 @@ exports.newOp = (optOpcode) => ({
  * Copies op1 to op2
  *
  * @param {Op} op1 - src Op
- * @param {Op} op2 - dest Op
+ * @param {Op} [op2] - dest Op. If not given, a new Op is used.
+ * @returns {Op} `op2`
  */
-const copyOp = (op1, op2) => {
-  op2.opcode = op1.opcode;
-  op2.chars = op1.chars;
-  op2.lines = op1.lines;
-  op2.attribs = op1.attribs;
-};
+const copyOp = (op1, op2 = exports.newOp()) => Object.assign(op2, op1);
 
 /**
  * Serializes a sequence of Ops.

From 1955e7b263a6eb9cf9fa8d687e3a90ef7c585d94 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Wed, 13 Oct 2021 17:00:50 -0400
Subject: [PATCH 09/22] Changeset: Replace output params with return values

This improves readability and reduces the chances of introducing a
bug.
---
 src/node/utils/padDiff.js  |   6 +--
 src/static/js/Changeset.js | 100 +++++++++++++------------------------
 2 files changed, 39 insertions(+), 67 deletions(-)

diff --git a/src/node/utils/padDiff.js b/src/node/utils/padDiff.js
index 193f24458ec..fa612c7c2a2 100644
--- a/src/node/utils/padDiff.js
+++ b/src/node/utils/padDiff.js
@@ -273,7 +273,7 @@ PadDiff.prototype._createDeletionChangeset = function (cs, startAText, apool) {
   let curChar = 0;
   let curLineOpIter = null;
   let curLineOpIterLine;
-  const curLineNextOp = Changeset.newOp('+');
+  let curLineNextOp = Changeset.newOp('+');
 
   const unpacked = Changeset.unpack(cs);
   const csIter = Changeset.opIterator(unpacked.ops);
@@ -287,7 +287,7 @@ PadDiff.prototype._createDeletionChangeset = function (cs, startAText, apool) {
       let indexIntoLine = 0;
       let done = false;
       while (!done) {
-        curLineOpIter.next(curLineNextOp);
+        curLineNextOp = curLineOpIter.next();
         if (indexIntoLine + curLineNextOp.chars >= curChar) {
           curLineNextOp.chars -= (curChar - indexIntoLine);
           done = true;
@@ -307,7 +307,7 @@ PadDiff.prototype._createDeletionChangeset = function (cs, startAText, apool) {
       }
 
       if (!curLineNextOp.chars) {
-        curLineOpIter.next(curLineNextOp);
+        curLineNextOp = curLineOpIter.next();
       }
 
       const charsToUse = Math.min(numChars, curLineNextOp.chars);
diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js
index 4d3c30db527..ead0e97a9f7 100644
--- a/src/static/js/Changeset.js
+++ b/src/static/js/Changeset.js
@@ -947,7 +947,7 @@ const textLinesMutator = (lines) => {
  * @param {string} in2 - second Op string
  * @param {Function} func - Callback that applies an operation to another operation. Will be called
  *     multiple times depending on the number of operations in `in1` and `in2`. `func` has signature
- *     `f(op1, op2, opOut)`:
+ *     `opOut = f(op1, op2)`:
  *       - `op1` is the current operation from `in1`. `func` is expected to mutate `op1` to
  *         partially or fully consume it, and MUST set `op1.opcode` to the empty string once `op1`
  *         is fully consumed. If `op1` is not fully consumed, `func` will be called again with the
@@ -956,9 +956,9 @@ const textLinesMutator = (lines) => {
  *         the empty string.
  *       - `op2` is the current operation from `in2`, to apply to `op1`. Has the same consumption
  *         and advancement semantics as `op1`.
- *       - `opOut` MUST be mutated to reflect the result of applying `op2` (before consumption) to
- *         `op1` (before consumption). If there is no result (perhaps `op1` and `op2` cancelled each
- *         other out), `opOut.opcode` MUST be set to the empty string.
+ *       - `opOut` is the result of applying `op2` (before consumption) to `op1` (before
+ *         consumption). If there is no result (perhaps `op1` and `op2` cancelled each other out),
+ *         either `opOut` must be nullish or `opOut.opcode` must be the empty string.
  * @returns {string} the integrated changeset
  */
 const applyZip = (in1, in2, func) => {
@@ -967,15 +967,11 @@ const applyZip = (in1, in2, func) => {
   const assem = exports.smartOpAssembler();
   const op1 = exports.newOp();
   const op2 = exports.newOp();
-  const opOut = exports.newOp();
   while (op1.opcode || iter1.hasNext() || op2.opcode || iter2.hasNext()) {
     if ((!op1.opcode) && iter1.hasNext()) iter1.next(op1);
     if ((!op2.opcode) && iter2.hasNext()) iter2.next(op2);
-    func(op1, op2, opOut);
-    if (opOut.opcode) {
-      assem.append(opOut);
-      opOut.opcode = '';
-    }
+    const opOut = func(op1, op2);
+    if (opOut && opOut.opcode) assem.append(opOut);
   }
   assem.endDocument();
   return assem.toString();
@@ -1171,11 +1167,11 @@ exports.composeAttributes = (att1, att2, resultIsMutation, pool) => {
  * @param {Op} attOp - The op from the sequence that is being operated on, either an attribution
  *     string or the earlier of two exportss being composed.
  * @param {Op} csOp -
- * @param {Op} opOut - Mutated to hold the result of applying `csOp` to `attOp`.
  * @param {AttributePool} pool - Can be null if definitely not needed.
+ * @returns {Op} The result of applying `csOp` to `attOp`.
  */
-const slicerZipperFunc = (attOp, csOp, opOut, pool) => {
-  clearOp(opOut);
+const slicerZipperFunc = (attOp, csOp, pool) => {
+  const opOut = exports.newOp();
   if (attOp.opcode === '-') {
     copyOp(attOp, opOut);
     attOp.opcode = '';
@@ -1263,6 +1259,7 @@ const slicerZipperFunc = (attOp, csOp, opOut, pool) => {
       }
     }
   }
+  return opOut;
 };
 
 /**
@@ -1275,9 +1272,7 @@ const slicerZipperFunc = (attOp, csOp, opOut, pool) => {
  */
 exports.applyToAttribution = (cs, astr, pool) => {
   const unpacked = exports.unpack(cs);
-
-  return applyZip(astr, unpacked.ops,
-      (op1, op2, opOut) => slicerZipperFunc(op1, op2, opOut, pool));
+  return applyZip(astr, unpacked.ops, (op1, op2) => slicerZipperFunc(op1, op2, pool));
 };
 
 exports.mutateAttributionLines = (cs, lines, pool) => {
@@ -1293,16 +1288,13 @@ exports.mutateAttributionLines = (cs, lines, pool) => {
 
   const isNextMutOp = () => (lineIter && lineIter.hasNext()) || mut.hasMore();
 
-  const nextMutOp = (destOp) => {
+  const nextMutOp = () => {
     if ((!(lineIter && lineIter.hasNext())) && mut.hasMore()) {
       const line = mut.removeLines(1);
       lineIter = exports.opIterator(line);
     }
-    if (lineIter && lineIter.hasNext()) {
-      lineIter.next(destOp);
-    } else {
-      destOp.opcode = '';
-    }
+    if (!lineIter || !lineIter.hasNext()) return exports.newOp();
+    return lineIter.next();
   };
   let lineAssem = null;
 
@@ -1318,13 +1310,10 @@ exports.mutateAttributionLines = (cs, lines, pool) => {
     lineAssem = null;
   };
 
-  const csOp = exports.newOp();
-  const attOp = exports.newOp();
-  const opOut = exports.newOp();
+  let csOp = exports.newOp();
+  let attOp = exports.newOp();
   while (csOp.opcode || csIter.hasNext() || attOp.opcode || isNextMutOp()) {
-    if ((!csOp.opcode) && csIter.hasNext()) {
-      csIter.next(csOp);
-    }
+    if (!csOp.opcode && csIter.hasNext()) csOp = csIter.next();
     if ((!csOp.opcode) && (!attOp.opcode) && (!lineAssem) && (!(lineIter && lineIter.hasNext()))) {
       break; // done
     } else if (csOp.opcode === '=' && csOp.lines > 0 && (!csOp.attribs) &&
@@ -1333,29 +1322,22 @@ exports.mutateAttributionLines = (cs, lines, pool) => {
       mut.skipLines(csOp.lines);
       csOp.opcode = '';
     } else if (csOp.opcode === '+') {
+      const opOut = copyOp(csOp);
       if (csOp.lines > 1) {
         const firstLineLen = csBank.indexOf('\n', csBankIndex) + 1 - csBankIndex;
-        copyOp(csOp, opOut);
         csOp.chars -= firstLineLen;
         csOp.lines--;
         opOut.lines = 1;
         opOut.chars = firstLineLen;
       } else {
-        copyOp(csOp, opOut);
         csOp.opcode = '';
       }
       outputMutOp(opOut);
       csBankIndex += opOut.chars;
-      opOut.opcode = '';
     } else {
-      if ((!attOp.opcode) && isNextMutOp()) {
-        nextMutOp(attOp);
-      }
-      slicerZipperFunc(attOp, csOp, opOut, pool);
-      if (opOut.opcode) {
-        outputMutOp(opOut);
-        opOut.opcode = '';
-      }
+      if (!attOp.opcode && isNextMutOp()) attOp = nextMutOp();
+      const opOut = slicerZipperFunc(attOp, csOp, pool);
+      if (opOut.opcode) outputMutOp(opOut);
     }
   }
 
@@ -1446,13 +1428,13 @@ exports.compose = (cs1, cs2, pool) => {
   const bankIter2 = exports.stringIterator(unpacked2.charBank);
   const bankAssem = exports.stringAssembler();
 
-  const newOps = applyZip(unpacked1.ops, unpacked2.ops, (op1, op2, opOut) => {
+  const newOps = applyZip(unpacked1.ops, unpacked2.ops, (op1, op2) => {
     const op1code = op1.opcode;
     const op2code = op2.opcode;
     if (op1code === '+' && op2code === '-') {
       bankIter1.skip(Math.min(op1.chars, op2.chars));
     }
-    slicerZipperFunc(op1, op2, opOut, pool);
+    const opOut = slicerZipperFunc(op1, op2, pool);
     if (opOut.opcode === '+') {
       if (op2code === '+') {
         bankAssem.append(bankIter2.take(opOut.chars));
@@ -1460,6 +1442,7 @@ exports.compose = (cs1, cs2, pool) => {
         bankAssem.append(bankIter1.take(opOut.chars));
       }
     }
+    return opOut;
   });
 
   return exports.pack(len1, len3, newOps, bankAssem.toString());
@@ -1788,9 +1771,8 @@ exports.copyAText = (atext1, atext2) => {
 exports.appendATextToAssembler = (atext, assem) => {
   // intentionally skips last newline char of atext
   const iter = exports.opIterator(atext.attribs);
-  const op = exports.newOp();
   while (iter.hasNext()) {
-    iter.next(op);
+    const op = iter.next();
     if (!iter.hasNext()) {
       // last op, exclude final newline
       if (op.lines <= 1) {
@@ -1994,25 +1976,19 @@ exports.makeAttribsString = (opcode, attribs, pool) => {
 exports.subattribution = (astr, start, optEnd) => {
   const iter = exports.opIterator(astr);
   const assem = exports.smartOpAssembler();
-  const attOp = exports.newOp();
+  let attOp = exports.newOp();
   const csOp = exports.newOp();
-  const opOut = exports.newOp();
 
   const doCsOp = () => {
     if (!csOp.chars) return;
     while (csOp.opcode && (attOp.opcode || iter.hasNext())) {
-      if (!attOp.opcode) iter.next(attOp);
-
+      if (!attOp.opcode) attOp = iter.next();
       if (csOp.opcode && attOp.opcode && csOp.chars >= attOp.chars &&
           attOp.lines > 0 && csOp.lines <= 0) {
         csOp.lines++;
       }
-
-      slicerZipperFunc(attOp, csOp, opOut, null);
-      if (opOut.opcode) {
-        assem.append(opOut);
-        opOut.opcode = '';
-      }
+      const opOut = slicerZipperFunc(attOp, csOp, null);
+      if (opOut.opcode) assem.append(opOut);
     }
   };
 
@@ -2025,10 +2001,7 @@ exports.subattribution = (astr, start, optEnd) => {
     if (attOp.opcode) {
       assem.append(attOp);
     }
-    while (iter.hasNext()) {
-      iter.next(attOp);
-      assem.append(attOp);
-    }
+    while (iter.hasNext()) assem.append(iter.next());
   } else {
     csOp.opcode = '=';
     csOp.chars = optEnd - start;
@@ -2067,7 +2040,7 @@ exports.inverse = (cs, lines, alines, pool) => {
   let curChar = 0;
   let curLineOpIter = null;
   let curLineOpIterLine;
-  const curLineNextOp = exports.newOp('+');
+  let curLineNextOp = exports.newOp('+');
 
   const unpacked = exports.unpack(cs);
   const csIter = exports.opIterator(unpacked.ops);
@@ -2081,7 +2054,7 @@ exports.inverse = (cs, lines, alines, pool) => {
       let indexIntoLine = 0;
       let done = false;
       while (!done && curLineOpIter.hasNext()) {
-        curLineOpIter.next(curLineNextOp);
+        curLineNextOp = curLineOpIter.next();
         if (indexIntoLine + curLineNextOp.chars >= curChar) {
           curLineNextOp.chars -= (curChar - indexIntoLine);
           done = true;
@@ -2099,9 +2072,7 @@ exports.inverse = (cs, lines, alines, pool) => {
         curLineNextOp.chars = 0;
         curLineOpIter = exports.opIterator(alinesGet(curLine));
       }
-      if (!curLineNextOp.chars) {
-        curLineOpIter.next(curLineNextOp);
-      }
+      if (!curLineNextOp.chars) curLineNextOp = curLineOpIter.next();
       const charsToUse = Math.min(numChars, curLineNextOp.chars);
       func(charsToUse, curLineNextOp.attribs, charsToUse === curLineNextOp.chars &&
           curLineNextOp.lines > 0);
@@ -2217,8 +2188,8 @@ exports.follow = (cs1, cs2, reverseInsertOrder, pool) => {
 
   const hasInsertFirst = exports.attributeTester(['insertorder', 'first'], pool);
 
-  const newOps = applyZip(unpacked1.ops, unpacked2.ops, (op1, op2, opOut) => {
-    clearOp(opOut);
+  const newOps = applyZip(unpacked1.ops, unpacked2.ops, (op1, op2) => {
+    const opOut = exports.newOp();
     if (op1.opcode === '+' || op2.opcode === '+') {
       let whichToDo;
       if (op2.opcode !== '+') {
@@ -2336,6 +2307,7 @@ exports.follow = (cs1, cs2, reverseInsertOrder, pool) => {
         newLen += opOut.chars;
         break;
     }
+    return opOut;
   });
   newLen += oldLen - oldPos;
 

From 44d99733c63f33c85e97a8edce3d50c8ba192846 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Thu, 30 Sep 2021 13:39:02 -0400
Subject: [PATCH 10/22] Changeset: Check `.hasNext()` before calling `.next()`

---
 src/node/utils/padDiff.js        | 4 ++--
 src/static/js/Changeset.js       | 4 +++-
 src/static/js/linestylefilter.js | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/node/utils/padDiff.js b/src/node/utils/padDiff.js
index fa612c7c2a2..900a452c79c 100644
--- a/src/node/utils/padDiff.js
+++ b/src/node/utils/padDiff.js
@@ -286,7 +286,7 @@ PadDiff.prototype._createDeletionChangeset = function (cs, startAText, apool) {
       curLineOpIterLine = curLine;
       let indexIntoLine = 0;
       let done = false;
-      while (!done) {
+      while (!done && curLineOpIter.hasNext()) {
         curLineNextOp = curLineOpIter.next();
         if (indexIntoLine + curLineNextOp.chars >= curChar) {
           curLineNextOp.chars -= (curChar - indexIntoLine);
@@ -307,7 +307,7 @@ PadDiff.prototype._createDeletionChangeset = function (cs, startAText, apool) {
       }
 
       if (!curLineNextOp.chars) {
-        curLineNextOp = curLineOpIter.next();
+        curLineNextOp = curLineOpIter.hasNext() ? curLineOpIter.next() : Changeset.newOp();
       }
 
       const charsToUse = Math.min(numChars, curLineNextOp.chars);
diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js
index ead0e97a9f7..07e17e5b724 100644
--- a/src/static/js/Changeset.js
+++ b/src/static/js/Changeset.js
@@ -2072,7 +2072,9 @@ exports.inverse = (cs, lines, alines, pool) => {
         curLineNextOp.chars = 0;
         curLineOpIter = exports.opIterator(alinesGet(curLine));
       }
-      if (!curLineNextOp.chars) curLineNextOp = curLineOpIter.next();
+      if (!curLineNextOp.chars) {
+        curLineNextOp = curLineOpIter.hasNext() ? curLineOpIter.next() : exports.newOp();
+      }
       const charsToUse = Math.min(numChars, curLineNextOp.chars);
       func(charsToUse, curLineNextOp.attribs, charsToUse === curLineNextOp.chars &&
           curLineNextOp.lines > 0);
diff --git a/src/static/js/linestylefilter.js b/src/static/js/linestylefilter.js
index ac8df82f477..84668ea46eb 100644
--- a/src/static/js/linestylefilter.js
+++ b/src/static/js/linestylefilter.js
@@ -108,7 +108,7 @@ linestylefilter.getLineStyleFilter = (lineLength, aline, textAndClassFunc, apool
     let nextOp, nextOpClasses;
 
     const goNextOp = () => {
-      nextOp = attributionIter.next();
+      nextOp = attributionIter.hasNext() ? attributionIter.next() : Changeset.newOp();
       nextOpClasses = (nextOp.opcode && attribsToClasses(nextOp.attribs));
     };
     goNextOp();

From ca5bdddc599fe165c4c47e9df8daaf471d454efe Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Mon, 8 Nov 2021 23:35:03 -0500
Subject: [PATCH 11/22] Changeset: Use `break` instead of `done` variable

---
 src/node/utils/padDiff.js  | 8 +++-----
 src/static/js/Changeset.js | 8 +++-----
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/src/node/utils/padDiff.js b/src/node/utils/padDiff.js
index 900a452c79c..670e8d6a17c 100644
--- a/src/node/utils/padDiff.js
+++ b/src/node/utils/padDiff.js
@@ -285,15 +285,13 @@ PadDiff.prototype._createDeletionChangeset = function (cs, startAText, apool) {
       curLineOpIter = Changeset.opIterator(aLinesGet(curLine));
       curLineOpIterLine = curLine;
       let indexIntoLine = 0;
-      let done = false;
-      while (!done && curLineOpIter.hasNext()) {
+      while (curLineOpIter.hasNext()) {
         curLineNextOp = curLineOpIter.next();
         if (indexIntoLine + curLineNextOp.chars >= curChar) {
           curLineNextOp.chars -= (curChar - indexIntoLine);
-          done = true;
-        } else {
-          indexIntoLine += curLineNextOp.chars;
+          break;
         }
+        indexIntoLine += curLineNextOp.chars;
       }
     }
 
diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js
index 07e17e5b724..395dbc2dd99 100644
--- a/src/static/js/Changeset.js
+++ b/src/static/js/Changeset.js
@@ -2052,15 +2052,13 @@ exports.inverse = (cs, lines, alines, pool) => {
       curLineOpIter = exports.opIterator(alinesGet(curLine));
       curLineOpIterLine = curLine;
       let indexIntoLine = 0;
-      let done = false;
-      while (!done && curLineOpIter.hasNext()) {
+      while (curLineOpIter.hasNext()) {
         curLineNextOp = curLineOpIter.next();
         if (indexIntoLine + curLineNextOp.chars >= curChar) {
           curLineNextOp.chars -= (curChar - indexIntoLine);
-          done = true;
-        } else {
-          indexIntoLine += curLineNextOp.chars;
+          break;
         }
+        indexIntoLine += curLineNextOp.chars;
       }
     }
 

From 42d4d8269c7e3c1323080aff3f49c2827f1c466b Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Sun, 21 Mar 2021 19:48:30 -0400
Subject: [PATCH 12/22] Changeset: Refactor `appendATextToAssembler()` for
 readability

---
 src/static/js/Changeset.js | 43 ++++++++++++++++----------------------
 1 file changed, 18 insertions(+), 25 deletions(-)

diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js
index 395dbc2dd99..9cf1057605c 100644
--- a/src/static/js/Changeset.js
+++ b/src/static/js/Changeset.js
@@ -1771,33 +1771,26 @@ exports.copyAText = (atext1, atext2) => {
 exports.appendATextToAssembler = (atext, assem) => {
   // intentionally skips last newline char of atext
   const iter = exports.opIterator(atext.attribs);
+  let lastOp = null;
   while (iter.hasNext()) {
-    const op = iter.next();
-    if (!iter.hasNext()) {
-      // last op, exclude final newline
-      if (op.lines <= 1) {
-        op.lines = 0;
-        op.chars--;
-        if (op.chars) {
-          assem.append(op);
-        }
-      } else {
-        const nextToLastNewlineEnd =
-        atext.text.lastIndexOf('\n', atext.text.length - 2) + 1;
-        const lastLineLength = atext.text.length - nextToLastNewlineEnd - 1;
-        op.lines--;
-        op.chars -= (lastLineLength + 1);
-        assem.append(op);
-        op.lines = 0;
-        op.chars = lastLineLength;
-        if (op.chars) {
-          assem.append(op);
-        }
-      }
-    } else {
-      assem.append(op);
-    }
+    if (lastOp != null) assem.append(lastOp);
+    lastOp = iter.next();
+  }
+  if (lastOp == null) return;
+  // exclude final newline
+  if (lastOp.lines <= 1) {
+    lastOp.lines = 0;
+    lastOp.chars--;
+  } else {
+    const nextToLastNewlineEnd = atext.text.lastIndexOf('\n', atext.text.length - 2) + 1;
+    const lastLineLength = atext.text.length - nextToLastNewlineEnd - 1;
+    lastOp.lines--;
+    lastOp.chars -= (lastLineLength + 1);
+    assem.append(lastOp);
+    lastOp.lines = 0;
+    lastOp.chars = lastLineLength;
   }
+  if (lastOp.chars) assem.append(lastOp);
 };
 
 /**

From efeb69b4ea361fc28da0f475b0331b189b53f865 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Mon, 22 Mar 2021 20:18:09 -0400
Subject: [PATCH 13/22] Changeset: Simplify `slicerZipperFunc()`

---
 src/static/js/Changeset.js | 112 ++++++++++---------------------------
 1 file changed, 31 insertions(+), 81 deletions(-)

diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js
index 9cf1057605c..b15c35f6990 100644
--- a/src/static/js/Changeset.js
+++ b/src/static/js/Changeset.js
@@ -1172,92 +1172,42 @@ exports.composeAttributes = (att1, att2, resultIsMutation, pool) => {
  */
 const slicerZipperFunc = (attOp, csOp, pool) => {
   const opOut = exports.newOp();
-  if (attOp.opcode === '-') {
+  if (!attOp.opcode) {
+    copyOp(csOp, opOut);
+    csOp.opcode = '';
+  } else if (!csOp.opcode) {
+    copyOp(attOp, opOut);
+    attOp.opcode = '';
+  } else if (attOp.opcode === '-') {
     copyOp(attOp, opOut);
     attOp.opcode = '';
-  } else if (!attOp.opcode) {
+  } else if (csOp.opcode === '+') {
     copyOp(csOp, opOut);
     csOp.opcode = '';
   } else {
-    switch (csOp.opcode) {
-      case '-':
-      {
-        if (csOp.chars <= attOp.chars) {
-          // delete or delete part
-          if (attOp.opcode === '=') {
-            opOut.opcode = '-';
-            opOut.chars = csOp.chars;
-            opOut.lines = csOp.lines;
-            // csOp is a remove op and remove ops normally never have any attributes, so this should
-            // normally be the empty string. However, padDiff.js adds attributes to remove ops and
-            // needs them preserved so they are copied here.
-            opOut.attribs = csOp.attribs;
-          }
-          attOp.chars -= csOp.chars;
-          attOp.lines -= csOp.lines;
-          csOp.opcode = '';
-          if (!attOp.chars) {
-            attOp.opcode = '';
-          }
-        } else {
-          // delete and keep going
-          if (attOp.opcode === '=') {
-            opOut.opcode = '-';
-            opOut.chars = attOp.chars;
-            opOut.lines = attOp.lines;
-            // csOp is a remove op and remove ops normally never have any attributes, so this should
-            // normally be the empty string. However, padDiff.js adds attributes to remove ops and
-            // needs them preserved so they are copied here.
-            opOut.attribs = csOp.attribs;
-          }
-          csOp.chars -= attOp.chars;
-          csOp.lines -= attOp.lines;
-          attOp.opcode = '';
-        }
-        break;
-      }
-      case '+':
-      {
-        // insert
-        copyOp(csOp, opOut);
-        csOp.opcode = '';
-        break;
-      }
-      case '=':
-      {
-        if (csOp.chars <= attOp.chars) {
-          // keep or keep part
-          opOut.opcode = attOp.opcode;
-          opOut.chars = csOp.chars;
-          opOut.lines = csOp.lines;
-          opOut.attribs = exports.composeAttributes(
-              attOp.attribs, csOp.attribs, attOp.opcode === '=', pool);
-          csOp.opcode = '';
-          attOp.chars -= csOp.chars;
-          attOp.lines -= csOp.lines;
-          if (!attOp.chars) {
-            attOp.opcode = '';
-          }
-        } else {
-          // keep and keep going
-          opOut.opcode = attOp.opcode;
-          opOut.chars = attOp.chars;
-          opOut.lines = attOp.lines;
-          opOut.attribs = exports.composeAttributes(
-              attOp.attribs, csOp.attribs, attOp.opcode === '=', pool);
-          attOp.opcode = '';
-          csOp.chars -= attOp.chars;
-          csOp.lines -= attOp.lines;
-        }
-        break;
-      }
-      case '':
-      {
-        copyOp(attOp, opOut);
-        attOp.opcode = '';
-        break;
-      }
-    }
+    opOut.opcode = {
+      '+': {
+        '-': '', // The '-' cancels out (some of) the '+', leaving any remainder for the next call.
+        '=': '+',
+      },
+      '=': {
+        '-': '-',
+        '=': '=',
+      },
+    }[attOp.opcode][csOp.opcode];
+    const [fullyConsumedOp, partiallyConsumedOp] = [attOp, csOp].sort((a, b) => a.chars - b.chars);
+    opOut.chars = fullyConsumedOp.chars;
+    opOut.lines = fullyConsumedOp.lines;
+    opOut.attribs = csOp.opcode === '-'
+      // csOp is a remove op and remove ops normally never have any attributes, so this should
+      // normally be the empty string. However, padDiff.js adds attributes to remove ops and needs
+      // them preserved so they are copied here.
+      ? csOp.attribs
+      : exports.composeAttributes(attOp.attribs, csOp.attribs, attOp.opcode === '=', pool);
+    partiallyConsumedOp.chars -= fullyConsumedOp.chars;
+    partiallyConsumedOp.lines -= fullyConsumedOp.lines;
+    if (!partiallyConsumedOp.chars) partiallyConsumedOp.opcode = '';
+    fullyConsumedOp.opcode = '';
   }
   return opOut;
 };

From 097f2623c63eeab475c060504e90a21ce40fd6f4 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Mon, 22 Mar 2021 20:23:59 -0400
Subject: [PATCH 14/22] Changeset: Add sanity checks to `slicerZipperFunc()`

---
 src/static/js/Changeset.js | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js
index b15c35f6990..50277e023fd 100644
--- a/src/static/js/Changeset.js
+++ b/src/static/js/Changeset.js
@@ -1185,6 +1185,17 @@ const slicerZipperFunc = (attOp, csOp, pool) => {
     copyOp(csOp, opOut);
     csOp.opcode = '';
   } else {
+    for (const op of [attOp, csOp]) {
+      assert(op.chars >= op.lines, `op has more newlines than chars: ${op.toString()}`);
+    }
+    assert(
+        attOp.chars < csOp.chars ? attOp.lines <= csOp.lines
+        : attOp.chars > csOp.chars ? attOp.lines >= csOp.lines
+        : attOp.lines === csOp.lines,
+        'line count mismatch when composing changesets A*B; ' +
+        `opA: ${attOp.toString()} opB: ${csOp.toString()}`);
+    assert(['+', '='].includes(attOp.opcode), `unexpected opcode in op: ${attOp.toString()}`);
+    assert(['-', '='].includes(csOp.opcode), `unexpected opcode in op: ${csOp.toString()}`);
     opOut.opcode = {
       '+': {
         '-': '', // The '-' cancels out (some of) the '+', leaving any remainder for the next call.

From 0ae8fb144192d7f2fbc894fb50e3502b7a4d2735 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Mon, 11 Oct 2021 04:31:28 -0400
Subject: [PATCH 15/22] Changeset: Use string concatenation instead of array
 join

People report that string concatenation is faster. Also, I think it's
more readable.
---
 src/static/js/Changeset.js | 34 ++++++++++++----------------------
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js
index 50277e023fd..85ee357000c 100644
--- a/src/static/js/Changeset.js
+++ b/src/static/js/Changeset.js
@@ -482,24 +482,22 @@ exports.mergingOpAssembler = () => {
  * @returns {OpAssembler}
  */
 exports.opAssembler = () => {
-  const pieces = [];
+  let serialized = '';
 
   /**
    * @param {Op} op - Operation to add. Ownership remains with the caller.
    */
   const append = (op) => {
-    pieces.push(op.attribs);
-    if (op.lines) {
-      pieces.push('|', exports.numToString(op.lines));
-    }
-    pieces.push(op.opcode);
-    pieces.push(exports.numToString(op.chars));
+    if (op.attribs != null) serialized += op.attribs;
+    if (op.lines) serialized += `|${exports.numToString(op.lines)}`;
+    if (op.opcode != null) serialized += op.opcode;
+    serialized += exports.numToString(op.chars);
   };
 
-  const toString = () => pieces.join('');
+  const toString = () => serialized;
 
   const clear = () => {
-    pieces.length = 0;
+    serialized = '';
   };
   return {
     append,
@@ -573,22 +571,14 @@ exports.stringIterator = (str) => {
 /**
  * @returns {StringAssembler}
  */
-exports.stringAssembler = () => {
-  const pieces = [];
-
+exports.stringAssembler = () => ({
+  _str: '',
   /**
    * @param {string} x -
    */
-  const append = (x) => {
-    pieces.push(String(x));
-  };
-
-  const toString = () => pieces.join('');
-  return {
-    append,
-    toString,
-  };
-};
+  append(x) { this._str += String(x); },
+  toString() { return this._str; },
+});
 
 /**
  * @typedef {object} StringArrayLike

From 9c17b03660fba65a118ebc71f8fdbd001985e8a7 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Sun, 3 Oct 2021 04:47:29 -0400
Subject: [PATCH 16/22] Changeset: Require Op opcode and attribs to be strings

---
 src/static/js/Changeset.js        | 6 ++++--
 src/static/js/contentcollector.js | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js
index 85ee357000c..426835915cb 100644
--- a/src/static/js/Changeset.js
+++ b/src/static/js/Changeset.js
@@ -488,9 +488,11 @@ exports.opAssembler = () => {
    * @param {Op} op - Operation to add. Ownership remains with the caller.
    */
   const append = (op) => {
-    if (op.attribs != null) serialized += op.attribs;
+    if (!op.opcode) throw new TypeError('null op');
+    if (typeof op.attribs !== 'string') throw new TypeError('attribs must be a string');
+    serialized += op.attribs;
     if (op.lines) serialized += `|${exports.numToString(op.lines)}`;
-    if (op.opcode != null) serialized += op.opcode;
+    serialized += op.opcode;
     serialized += exports.numToString(op.chars);
   };
 
diff --git a/src/static/js/contentcollector.js b/src/static/js/contentcollector.js
index 3d2bd9aa8da..54c6288048c 100644
--- a/src/static/js/contentcollector.js
+++ b/src/static/js/contentcollector.js
@@ -92,7 +92,7 @@ const makeContentCollector = (collectStyles, abrowser, apool, className2Author)
         attribsBuilder = Changeset.smartOpAssembler();
       },
       textOfLine: (i) => textArray[i],
-      appendText: (txt, attrString) => {
+      appendText: (txt, attrString = '') => {
         textArray[textArray.length - 1] += txt;
         op.attribs = attrString;
         op.chars = txt.length;

From 6d5b737140979c03391efb89332d2d879134eda8 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Mon, 11 Oct 2021 17:37:43 -0400
Subject: [PATCH 17/22] Changeset: Replace `.apply()` with spread operator

---
 src/static/js/Changeset.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js
index 426835915cb..dab1e5afc52 100644
--- a/src/static/js/Changeset.js
+++ b/src/static/js/Changeset.js
@@ -870,14 +870,14 @@ const textLinesMutator = (lines) => {
         curLine++;
         newLines.splice(0, 1);
         // insert the remaining new lines
-        Array.prototype.push.apply(curSplice, newLines);
+        curSplice.push(...newLines);
         curLine += newLines.length;
         // insert the remaining chars from the "old" line (e.g. the line we were in
         // when we started to insert new lines)
         curSplice.push(theLine.substring(lineCol));
         curCol = 0; // TODO(doc) why is this not set to the length of last line?
       } else {
-        Array.prototype.push.apply(curSplice, newLines);
+        curSplice.push(...newLines);
         curLine += newLines.length;
       }
     } else {

From 1cad5d881a4f383006d56fc3e7644be17dc24867 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Mon, 11 Oct 2021 18:14:01 -0400
Subject: [PATCH 18/22] Changeset: Use `for...of` iteration to improve
 readability

---
 src/static/js/Changeset.js | 32 ++++++++++----------------------
 1 file changed, 10 insertions(+), 22 deletions(-)

diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js
index dab1e5afc52..e70966d5ff4 100644
--- a/src/static/js/Changeset.js
+++ b/src/static/js/Changeset.js
@@ -1146,9 +1146,9 @@ exports.composeAttributes = (att1, att2, resultIsMutation, pool) => {
   });
   atts.sort();
   const buf = exports.stringAssembler();
-  for (let i = 0; i < atts.length; i++) {
+  for (const att of atts) {
     buf.append('*');
-    buf.append(exports.numToString(pool.putAttrib(atts[i])));
+    buf.append(exports.numToString(pool.putAttrib(att)));
   }
   return buf.toString();
 };
@@ -1306,8 +1306,7 @@ exports.mutateAttributionLines = (cs, lines, pool) => {
  */
 exports.joinAttributionLines = (theAlines) => {
   const assem = exports.mergingOpAssembler();
-  for (let i = 0; i < theAlines.length; i++) {
-    const aline = theAlines[i];
+  for (const aline of theAlines) {
     const iter = exports.opIterator(aline);
     while (iter.hasNext()) {
       assem.append(iter.next());
@@ -1507,10 +1506,8 @@ const toSplices = (cs) => {
 exports.characterRangeFollow = (cs, startChar, endChar, insertionsAfter) => {
   let newStartChar = startChar;
   let newEndChar = endChar;
-  const splices = toSplices(cs);
   let lengthChangeSoFar = 0;
-  for (let i = 0; i < splices.length; i++) {
-    const splice = splices[i];
+  for (const splice of toSplices(cs)) {
     const spliceStart = splice[0] + lengthChangeSoFar;
     const spliceEnd = splice[1] + lengthChangeSoFar;
     const newTextLength = splice[2].length;
@@ -1906,8 +1903,7 @@ exports.makeAttribsString = (opcode, attribs, pool) => {
       attribs.sort();
     }
     const result = [];
-    for (let i = 0; i < attribs.length; i++) {
-      const pair = attribs[i];
+    for (const pair of attribs) {
       if (opcode === '=' || (opcode === '+' && pair[1])) {
         result.push(`*${exports.numToString(pool.putAttrib(pair))}`);
       }
@@ -2072,23 +2068,15 @@ exports.inverse = (cs, lines, alines, pool) => {
     };
   };
 
-  const attribKeys = [];
-  const attribValues = [];
   while (csIter.hasNext()) {
     const csOp = csIter.next();
     if (csOp.opcode === '=') {
       if (csOp.attribs) {
-        attribKeys.length = 0;
-        attribValues.length = 0;
-        exports.eachAttribNumber(csOp.attribs, (n) => {
-          attribKeys.push(pool.getAttribKey(n));
-          attribValues.push(pool.getAttribValue(n));
-        });
+        const csAttribs = [];
+        exports.eachAttribNumber(csOp.attribs, (n) => csAttribs.push(pool.getAttrib(n)));
         const undoBackToAttribs = cachedStrFunc((attribs) => {
           const backAttribs = [];
-          for (let i = 0; i < attribKeys.length; i++) {
-            const appliedKey = attribKeys[i];
-            const appliedValue = attribValues[i];
+          for (const [appliedKey, appliedValue] of csAttribs) {
             const oldValue = exports.attribsAttributeValue(attribs, appliedKey, pool);
             if (appliedValue !== oldValue) {
               backAttribs.push([appliedKey, oldValue]);
@@ -2289,9 +2277,9 @@ const followAttributes = (att1, att2, pool) => {
   });
   // we've only removed attributes, so they're already sorted
   const buf = exports.stringAssembler();
-  for (let i = 0; i < atts.length; i++) {
+  for (const att of atts) {
     buf.append('*');
-    buf.append(exports.numToString(pool.putAttrib(atts[i])));
+    buf.append(exports.numToString(pool.putAttrib(att)));
   }
   return buf.toString();
 };

From 9401ae876bd76ca6b2afb7753d6f48393a89e86f Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Mon, 11 Oct 2021 21:11:28 -0400
Subject: [PATCH 19/22] Changeset: Sort attributes by keys, not full string rep

---
 src/static/js/Changeset.js | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js
index e70966d5ff4..78d5d3edc11 100644
--- a/src/static/js/Changeset.js
+++ b/src/static/js/Changeset.js
@@ -1089,6 +1089,15 @@ exports.mutateTextLines = (cs, lines) => {
   mut.close();
 };
 
+/**
+ * Sorts an array of attributes by key.
+ *
+ * @param {Attribute[]} attribs - The array of attributes to sort in place.
+ * @returns {Attribute[]} The `attribs` array.
+ */
+const sortAttribs =
+    (attribs) => attribs.sort((a, b) => (a[0] > b[0] ? 1 : 0) - (a[0] < b[0] ? 1 : 0));
+
 /**
  * Composes two attribute strings (see below) into one.
  *
@@ -1144,9 +1153,8 @@ exports.composeAttributes = (att1, att2, resultIsMutation, pool) => {
     }
     return '';
   });
-  atts.sort();
   const buf = exports.stringAssembler();
-  for (const att of atts) {
+  for (const att of sortAttribs(atts)) {
     buf.append('*');
     buf.append(exports.numToString(pool.putAttrib(att)));
   }
@@ -1900,7 +1908,7 @@ exports.makeAttribsString = (opcode, attribs, pool) => {
   } else if (pool && attribs.length) {
     if (attribs.length > 1) {
       attribs = attribs.slice();
-      attribs.sort();
+      sortAttribs(attribs);
     }
     const result = [];
     for (const pair of attribs) {

From b62534a6b22e498eadda95dd77dea13f055ba44f Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Tue, 12 Oct 2021 02:00:56 -0400
Subject: [PATCH 20/22] Changeset: Use Maps to simplify attribute processing

---
 src/static/js/Changeset.js | 44 ++++++++++++--------------------------
 1 file changed, 14 insertions(+), 30 deletions(-)

diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js
index 78d5d3edc11..2df33ec4651 100644
--- a/src/static/js/Changeset.js
+++ b/src/static/js/Changeset.js
@@ -1129,32 +1129,23 @@ exports.composeAttributes = (att1, att2, resultIsMutation, pool) => {
     return att2;
   }
   if (!att2) return att1;
-  const atts = [];
+  const atts = new Map();
   att1.replace(/\*([0-9a-z]+)/g, (_, a) => {
-    atts.push(pool.getAttrib(exports.parseNum(a)));
+    const [key, val] = pool.getAttrib(exports.parseNum(a));
+    atts.set(key, val);
     return '';
   });
   att2.replace(/\*([0-9a-z]+)/g, (_, a) => {
-    const pair = pool.getAttrib(exports.parseNum(a));
-    let found = false;
-    for (let i = 0; i < atts.length; i++) {
-      const oldPair = atts[i];
-      if (oldPair[0] !== pair[0]) continue;
-      if (pair[1] || resultIsMutation) {
-        oldPair[1] = pair[1];
-      } else {
-        atts.splice(i, 1);
-      }
-      found = true;
-      break;
-    }
-    if ((!found) && (pair[1] || resultIsMutation)) {
-      atts.push(pair);
+    const [key, val] = pool.getAttrib(exports.parseNum(a));
+    if (val || resultIsMutation) {
+      atts.set(key, val);
+    } else {
+      atts.delete(key);
     }
     return '';
   });
   const buf = exports.stringAssembler();
-  for (const att of sortAttribs(atts)) {
+  for (const att of sortAttribs([...atts])) {
     buf.append('*');
     buf.append(exports.numToString(pool.putAttrib(att)));
   }
@@ -2265,22 +2256,15 @@ const followAttributes = (att1, att2, pool) => {
   // to produce the merged set.
   if ((!att2) || (!pool)) return '';
   if (!att1) return att2;
-  const atts = [];
+  const atts = new Map();
   att2.replace(/\*([0-9a-z]+)/g, (_, a) => {
-    atts.push(pool.getAttrib(exports.parseNum(a)));
+    const [key, val] = pool.getAttrib(exports.parseNum(a));
+    atts.set(key, val);
     return '';
   });
   att1.replace(/\*([0-9a-z]+)/g, (_, a) => {
-    const pair1 = pool.getAttrib(exports.parseNum(a));
-    for (let i = 0; i < atts.length; i++) {
-      const pair2 = atts[i];
-      if (pair1[0] !== pair2[0]) continue;
-      if (pair1[1] <= pair2[1]) {
-        // winner of merge is pair1, delete this attribute
-        atts.splice(i, 1);
-      }
-      break;
-    }
+    const [key, val] = pool.getAttrib(exports.parseNum(a));
+    if (atts.has(key) && val <= atts.get(key)) atts.delete(key);
     return '';
   });
   // we've only removed attributes, so they're already sorted

From 4f4a775d9e44ae6caac8bdcad573d122aca00c7d Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Mon, 25 Oct 2021 05:19:35 -0400
Subject: [PATCH 21/22] Changeset: Improve handling of missing attribute in old
 pool

---
 src/static/js/Changeset.js | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js
index 2df33ec4651..d261db74899 100644
--- a/src/static/js/Changeset.js
+++ b/src/static/js/Changeset.js
@@ -1563,18 +1563,10 @@ exports.moveOpsToNewPool = (cs, oldPool, newPool) => {
   // order of attribs stays the same
   return upToDollar.replace(/\*([0-9a-z]+)/g, (_, a) => {
     const oldNum = exports.parseNum(a);
-    let pair = oldPool.getAttrib(oldNum);
-
-    /*
-     * Setting an empty pair. Required for when delete pad contents / attributes
-     * while another user has the timeslider open.
-     *
-     * Fixes https://github.com/ether/etherpad-lite/issues/3932
-     */
-    if (!pair) {
-      pair = [];
-    }
-
+    const pair = oldPool.getAttrib(oldNum);
+    // The attribute might not be in the old pool if the user is viewing the current revision in the
+    // timeslider and text is deleted. See: https://github.com/ether/etherpad-lite/issues/3932
+    if (!pair) return '';
     const newNum = newPool.putAttrib(pair);
     return `*${exports.numToString(newNum)}`;
   }) + fromDollar;

From 1bbe0d921514cec6a118df4c3e1b7ba276c7ca7b Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Mon, 25 Oct 2021 05:17:18 -0400
Subject: [PATCH 22/22] Changeset: Use `in` check to help TypeScript narrowing

---
 src/static/js/Changeset.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js
index d261db74899..21de89c43da 100644
--- a/src/static/js/Changeset.js
+++ b/src/static/js/Changeset.js
@@ -649,7 +649,7 @@ const textLinesMutator = (lines) => {
    * @returns {string}
    */
   const linesGet = (idx) => {
-    if (lines.get) {
+    if ('get' in lines) {
       return lines.get(idx);
     } else {
       return lines[idx];