WIP: Changeset: Refactor to improve API, and other cleanups

CHANGELOG.md

@@ -36,8 +36,14 @@
     * `opAttributeValue()`
   * `opIterator()`: Deprecated in favor of the new `deserializeOps()` generator
     function.
+  * `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.
+  * `builder()`: Deprecated in favor of the new `Builder` class.
   * `newOp()`: Deprecated in favor of the new `Op` class.
 
 # 1.8.16

src/node/db/API.js

@@ -538,7 +538,7 @@ exports.restoreRevision = async (padID, rev) => {
   };
 
   // create a new changeset with a helper builder object
-  const builder = Changeset.builder(oldText.length);
+  const builder = new Changeset.Builder(oldText.length);
 
   // assemble each line into the builder
   eachAttribRun(atext.attribs, (start, end, attribs) => {

src/node/db/Pad.js

@@ -494,21 +494,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/node/handler/PadMessageHandler.js

@@ -582,11 +582,10 @@ const handleUserChanges = async (socket, message) => {
     const wireApool = (new AttributePool()).fromJsonable(apool);
     const pad = await padManager.getPad(thisSession.padId);
 
-    // Verify that the changeset has valid syntax and is in canonical form
-    Changeset.checkRep(changeset);
+    const cs = Changeset.unpack(changeset).validate();
 
     // Validate all added 'author' attribs to be the same value as the current user
-    for (const op of Changeset.deserializeOps(Changeset.unpack(changeset).ops)) {
+    for (const op of Changeset.deserializeOps(cs.ops)) {
       // + can add text with attribs
       // = can change or add attribs
       // - can have attribs, but they are discarded and don't show up in the attribs -
@@ -629,10 +628,10 @@ const handleUserChanges = async (socket, message) => {
 
     const prevText = pad.text();
 
-    if (Changeset.oldLen(rebasedChangeset) !== prevText.length) {
+    if (Changeset.unpack(rebasedChangeset).oldLen !== prevText.length) {
       throw new Error(
           `Can't apply changeset ${rebasedChangeset} with oldLen ` +
-          `${Changeset.oldLen(rebasedChangeset)} to document of length ${prevText.length}`);
+          `${Changeset.unpack(rebasedChangeset).oldLen} to document of length ${prevText.length}`);
     }
 
     const newRev = await pad.appendRevision(rebasedChangeset, thisSession.author);
@@ -759,7 +758,7 @@ const _correctMarkersInPad = (atext, apool) => {
   // create changeset that removes these bad markers
   offset = 0;
 
-  const builder = Changeset.builder(text.length);
+  const builder = new Changeset.Builder(text.length);
 
   badMarkers.forEach((pos) => {
     builder.keepText(text.substring(offset, pos));

src/node/utils/ExportHtml.js

@@ -125,7 +125,7 @@ const getHTMLFromAtext = async (pad, atext, authorColors) => {
     // becomes
     // <b>Just bold <i>Bold and italics</i></b> <i>Just italics</i>
     const taker = Changeset.stringIterator(text);
-    const assem = Changeset.stringAssembler();
+    let assem = '';
     const openTags = [];
 
     const getSpanClassFor = (i) => {
@@ -161,31 +161,15 @@ const getHTMLFromAtext = async (pad, atext, authorColors) => {
     const emitOpenTag = (i) => {
       openTags.unshift(i);
       const spanClass = getSpanClassFor(i);
-
-      if (spanClass) {
-        assem.append('<span class="');
-        assem.append(spanClass);
-        assem.append('">');
-      } else {
-        assem.append('<');
-        assem.append(tags[i]);
-        assem.append('>');
-      }
+      assem += spanClass ? `<span class="${spanClass}">` : `<${tags[i]}>`;
     };
 
     // this closes an open tag and removes its reference from openTags
     const emitCloseTag = (i) => {
       openTags.shift();
       const spanClass = getSpanClassFor(i);
       const spanWithData = isSpanWithData(i);
-
-      if (spanClass || spanWithData) {
-        assem.append('</span>');
-      } else {
-        assem.append('</');
-        assem.append(tags[i]);
-        assem.append('>');
-      }
+      assem += spanClass || spanWithData ? '</span>' : `</${tags[i]}>`;
     };
 
     const urls = padutils.findURLs(text);
@@ -246,7 +230,7 @@ const getHTMLFromAtext = async (pad, atext, authorColors) => {
         // from but they break the abiword parser and are completly useless
         s = s.replace(String.fromCharCode(12), '');
 
-        assem.append(_encodeWhitespace(Security.escapeHTML(s)));
+        assem += _encodeWhitespace(Security.escapeHTML(s));
       } // end iteration over spans in line
 
       // close all the tags that are open after the last op
@@ -269,14 +253,14 @@ const getHTMLFromAtext = async (pad, atext, authorColors) => {
         // https://html.spec.whatwg.org/multipage/links.html#link-type-noopener
         // https://mathiasbynens.github.io/rel-noopener/
         // https://github.com/ether/etherpad-lite/pull/3636
-        assem.append(`<a href="${Security.escapeHTMLAttribute(url)}" rel="noreferrer noopener">`);
+        assem += `<a href="${Security.escapeHTMLAttribute(url)}" rel="noreferrer noopener">`;
         processNextChars(urlLength);
-        assem.append('</a>');
+        assem += '</a>';
       });
     }
     processNextChars(text.length - idx);
 
-    return _processSpaces(assem.toString());
+    return _processSpaces(assem);
   };
   // end getLineHTML
   const pieces = [css];

src/node/utils/ExportTxt.js

@@ -67,7 +67,7 @@ const getTXTFromAtext = (pad, atext, authorColors) => {
     // becomes
     // <b>Just bold <i>Bold and italics</i></b> <i>Just italics</i>
     const taker = Changeset.stringIterator(text);
-    const assem = Changeset.stringAssembler();
+    let assem = '';
 
     let idx = 0;
 
@@ -161,7 +161,7 @@ const getTXTFromAtext = (pad, atext, authorColors) => {
         // plugins from being able to display * at the beginning of a line
         // s = s.replace("*", ""); // Then remove it
 
-        assem.append(s);
+        assem += s;
       } // end iteration over spans in line
 
       const tags2close = [];
@@ -175,7 +175,7 @@ const getTXTFromAtext = (pad, atext, authorColors) => {
     // end processNextChars
 
     processNextChars(text.length - idx);
-    return (assem.toString());
+    return assem;
   };
   // end getLineHTML
 

src/node/utils/ImportHtml.js

@@ -64,7 +64,7 @@ exports.setPadHTML = async (pad, html) => {
   const newAttribs = `${result.lineAttribs.join('|1+1')}|1+1`;
 
   // create a new changeset with a helper builder object
-  const builder = Changeset.builder(1);
+  const builder = new Changeset.Builder(1);
 
   // assemble each line into the builder
   let textIndex = 0;

src/node/utils/padDiff.js

@@ -69,7 +69,7 @@ PadDiff.prototype._createClearAuthorship = async function (rev) {
   const atext = await this._pad.getInternalRevisionAText(rev);
 
   // build clearAuthorship changeset
-  const builder = Changeset.builder(atext.text.length);
+  const builder = new Changeset.Builder(atext.text.length);
   builder.keepText(atext.text, [['author', '']], this._pad.pool);
   const changeset = builder.toString();
 
@@ -206,28 +206,26 @@ PadDiff.prototype._extendChangesetWithAuthor = (changeset, author, apool) => {
   // unpack
   const unpacked = Changeset.unpack(changeset);
 
-  const assem = Changeset.opAssembler();
-
   // create deleted attribs
   const authorAttrib = apool.putAttrib(['author', author || '']);
   const deletedAttrib = apool.putAttrib(['removed', true]);
   const attribs = `*${Changeset.numToString(authorAttrib)}*${Changeset.numToString(deletedAttrib)}`;
 
-  for (const operator of Changeset.deserializeOps(unpacked.ops)) {
-    if (operator.opcode === '-') {
-      // this is a delete operator, extend it with the author
-      operator.attribs = attribs;
-    } else if (operator.opcode === '=' && operator.attribs) {
-      // this is operator changes only attributes, let's mark which author did that
-      operator.attribs += `*${Changeset.numToString(authorAttrib)}`;
+  const serializedOps = Changeset.serializeOps((function* () {
+    for (const operator of Changeset.deserializeOps(unpacked.ops)) {
+      if (operator.opcode === '-') {
+        // this is a delete operator, extend it with the author
+        operator.attribs = attribs;
+      } else if (operator.opcode === '=' && operator.attribs) {
+        // this is operator changes only attributes, let's mark which author did that
+        operator.attribs += `*${Changeset.numToString(authorAttrib)}`;
+      }
+      yield operator;
     }
-
-    // append the new operator to our assembler
-    assem.append(operator);
-  }
+  })());
 
   // return the modified changeset
-  return Changeset.pack(unpacked.oldLen, unpacked.newLen, assem.toString(), unpacked.charBank);
+  return Changeset.pack(unpacked.oldLen, unpacked.newLen, serializedOps, unpacked.charBank);
 };
 
 // this method is 80% like Changeset.inverse. I just changed so instead of reverting,
@@ -264,7 +262,7 @@ PadDiff.prototype._createDeletionChangeset = function (cs, startAText, apool) {
   let curLineNextOp = new Changeset.Op('+');
 
   const unpacked = Changeset.unpack(cs);
-  const builder = Changeset.builder(unpacked.newLen);
+  const builder = new Changeset.Builder(unpacked.newLen);
 
   const consumeAttribRuns = (numChars, func /* (len, attribs, endsLine)*/) => {
     if (!curLineOps || curLineOpsLine !== curLine) {
@@ -330,21 +328,21 @@ PadDiff.prototype._createDeletionChangeset = function (cs, startAText, apool) {
 
   const nextText = (numChars) => {
     let len = 0;
-    const assem = Changeset.stringAssembler();
+    let assem = '';
     const firstString = linesGet(curLine).substring(curChar);
     len += firstString.length;
-    assem.append(firstString);
+    assem += firstString;
 
     let lineNum = curLine + 1;
 
     while (len < numChars) {
       const nextString = linesGet(lineNum);
       len += nextString.length;
-      assem.append(nextString);
+      assem += nextString;
       lineNum++;
     }
 
-    return assem.toString().substring(0, numChars);
+    return assem.substring(0, numChars);
   };
 
   const cachedStrFunc = (func) => {
@@ -440,7 +438,7 @@ PadDiff.prototype._createDeletionChangeset = function (cs, startAText, apool) {
     }
   }
 
-  return Changeset.checkRep(builder.toString());
+  return builder.build().validate().toString();
 };
 
 // export the constructor

src/static/js/AttributeManager.js

@@ -125,7 +125,7 @@ AttributeManager.prototype = _(AttributeManager.prototype).extend({
    * @param attribs an array of attributes
    */
   _setAttributesOnRangeByLine(row, startCol, endCol, attribs) {
-    const builder = Changeset.builder(this.rep.lines.totalWidth());
+    const builder = new Changeset.Builder(this.rep.lines.totalWidth());
     ChangesetUtils.buildKeepToStartOfRange(this.rep, builder, [row, startCol]);
     ChangesetUtils.buildKeepRange(
         this.rep, builder, [row, startCol], [row, endCol], attribs, this.rep.apool);
@@ -285,7 +285,7 @@ AttributeManager.prototype = _(AttributeManager.prototype).extend({
   */
   setAttributeOnLine(lineNum, attributeName, attributeValue) {
     let loc = [0, 0];
-    const builder = Changeset.builder(this.rep.lines.totalWidth());
+    const builder = new Changeset.Builder(this.rep.lines.totalWidth());
     const hasMarker = this.lineHasMarker(lineNum);
 
     ChangesetUtils.buildKeepRange(this.rep, builder, loc, (loc = [lineNum, 0]));
@@ -314,7 +314,7 @@ AttributeManager.prototype = _(AttributeManager.prototype).extend({
    *  @param attributeValue if given only attributes with equal value will be removed
    */
   removeAttributeOnLine(lineNum, attributeName, attributeValue) {
-    const builder = Changeset.builder(this.rep.lines.totalWidth());
+    const builder = new Changeset.Builder(this.rep.lines.totalWidth());
     const hasMarker = this.lineHasMarker(lineNum);
     let found = false;