Changeset: Avoid unnecessary StringAssembler class

Author: rhansen

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/padDiff.js

@@ -328,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) => {

src/static/js/Changeset.js

@@ -727,7 +727,11 @@ class StringAssembler {
 /**
  * @returns {StringAssembler}
  */
-exports.stringAssembler = () => new StringAssembler();
+exports.stringAssembler = () => {
+  padutils.warnWithStack(
+      'Changeset.stringAssembler() is deprecated; build a string manually instead');
+  return new StringAssembler();
+};
 
 /**
  * @typedef {object} StringArrayLike
@@ -1156,7 +1160,7 @@ exports.applyToText = (cs, str) => {
   assert(str.length === unpacked.oldLen, `mismatched apply: ${str.length} / ${unpacked.oldLen}`);
   const bankIter = exports.stringIterator(unpacked.charBank);
   const strIter = exports.stringIterator(str);
-  const assem = new StringAssembler();
+  let assem = '';
   for (const op of exports.deserializeOps(unpacked.ops)) {
     switch (op.opcode) {
       case '+':
@@ -1165,7 +1169,7 @@ exports.applyToText = (cs, str) => {
         if (op.lines !== bankIter.peek(op.chars).split('\n').length - 1) {
           throw new Error(`newline count is wrong in op +; cs:${cs} and text:${str}`);
         }
-        assem.append(bankIter.take(op.chars));
+        assem += bankIter.take(op.chars);
         break;
       case '-':
       // op is - and op.lines 0: no newlines must be in the deleted string
@@ -1181,12 +1185,12 @@ exports.applyToText = (cs, str) => {
         if (op.lines !== strIter.peek(op.chars).split('\n').length - 1) {
           throw new Error(`newline count is wrong in op =; cs:${cs} and text:${str}`);
         }
-        assem.append(strIter.take(op.chars));
+        assem += strIter.take(op.chars);
         break;
     }
   }
-  assem.append(strIter.take(strIter.remaining()));
-  return assem.toString();
+  assem += strIter.take(strIter.remaining());
+  return assem;
 };
 
 /**
@@ -1475,7 +1479,7 @@ exports.compose = (cs1, cs2, pool) => {
   const len3 = unpacked2.newLen;
   const bankIter1 = exports.stringIterator(unpacked1.charBank);
   const bankIter2 = exports.stringIterator(unpacked2.charBank);
-  const bankAssem = new StringAssembler();
+  let bankAssem = '';
 
   const newOps = applyZip(unpacked1.ops, unpacked2.ops, (op1, op2) => {
     const op1code = op1.opcode;
@@ -1485,16 +1489,12 @@ exports.compose = (cs1, cs2, pool) => {
     }
     const opOut = slicerZipperFunc(op1, op2, pool);
     if (opOut.opcode === '+') {
-      if (op2code === '+') {
-        bankAssem.append(bankIter2.take(opOut.chars));
-      } else {
-        bankAssem.append(bankIter1.take(opOut.chars));
-      }
+      bankAssem += (op2code === '+' ? bankIter2 : bankIter1).take(opOut.chars);
     }
     return opOut;
   });
 
-  return exports.pack(len1, len3, newOps, bankAssem.toString());
+  return exports.pack(len1, len3, newOps, bankAssem);
 };
 
 /**
@@ -1923,7 +1923,7 @@ class Builder {
   constructor(oldLen) {
     this._oldLen = oldLen;
     this._ops = [];
-    this._charBank = new StringAssembler();
+    this._charBank = '';
   }
 
   /**
@@ -1969,7 +1969,7 @@ class Builder {
    */
   insert(text, attribs = '', pool = null) {
     this._ops.push(...opsFromText('+', text, attribs, pool));
-    this._charBank.append(text);
+    this._charBank += text;
     return this;
   }
 
@@ -2001,7 +2001,7 @@ class Builder {
       oldLen: this._oldLen,
       newLen: this._oldLen + lengthChange,
       ops: serializedOps,
-      charBank: this._charBank.toString(),
+      charBank: this._charBank,
     };
   }
 
@@ -2184,20 +2184,20 @@ exports.inverse = (cs, lines, alines, pool) => {
 
   const nextText = (numChars) => {
     let len = 0;
-    const assem = new 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) => {
@@ -2410,12 +2410,9 @@ const followAttributes = (att1, att2, pool) => {
     return '';
   });
   // we've only removed attributes, so they're already sorted
-  const buf = new StringAssembler();
-  for (const att of atts) {
-    buf.append('*');
-    buf.append(exports.numToString(pool.putAttrib(att)));
-  }
-  return buf.toString();
+  let buf = '';
+  for (const att of atts) buf += `*${exports.numToString(pool.putAttrib(att))}`;
+  return buf;
 };
 
 exports.exportedForTestingOnly = {

src/tests/frontend/easysync-helper.js

@@ -20,29 +20,19 @@ const poolOrArray = (attribs) => {
 exports.poolOrArray = poolOrArray;
 
 const randomInlineString = (len) => {
-  const assem = Changeset.stringAssembler();
-  for (let i = 0; i < len; i++) {
-    assem.append(String.fromCharCode(randInt(26) + 97));
-  }
-  return assem.toString();
+  let assem = '';
+  for (let i = 0; i < len; i++) assem += String.fromCharCode(randInt(26) + 97);
+  return assem;
 };
 
 const randomMultiline = (approxMaxLines, approxMaxCols) => {
   const numParts = randInt(approxMaxLines * 2) + 1;
-  const txt = Changeset.stringAssembler();
-  txt.append(randInt(2) ? '\n' : '');
+  let txt = '';
+  txt += randInt(2) ? '\n' : '';
   for (let i = 0; i < numParts; i++) {
-    if ((i % 2) === 0) {
-      if (randInt(10)) {
-        txt.append(randomInlineString(randInt(approxMaxCols) + 1));
-      } else {
-        txt.append('\n');
-      }
-    } else {
-      txt.append('\n');
-    }
+    txt += i % 2 === 0 && randInt(10) ? randomInlineString(randInt(approxMaxCols) + 1) : '\n';
   }
-  return txt.toString();
+  return txt;
 };
 exports.randomMultiline = randomMultiline;
 
@@ -165,9 +155,9 @@ const randomTwoPropAttribs = (opcode) => {
 };
 
 const randomTestChangeset = (origText, withAttribs) => {
-  const charBank = Changeset.stringAssembler();
+  let charBank = '';
   let textLeft = origText; // always keep final newline
-  const outTextAssem = Changeset.stringAssembler();
+  let outTextAssem = '';
   const ops = [];
   const oldLen = origText.length;
 
@@ -192,13 +182,13 @@ const randomTestChangeset = (origText, withAttribs) => {
     const o = randomStringOperation(textLeft.length);
     if (o.insert) {
       const txt = o.insert;
-      charBank.append(txt);
-      outTextAssem.append(txt);
+      charBank += txt;
+      outTextAssem += txt;
       appendMultilineOp('+', txt);
     } else if (o.skip) {
       const txt = textLeft.substring(0, o.skip);
       textLeft = textLeft.substring(o.skip);
-      outTextAssem.append(txt);
+      outTextAssem += txt;
       appendMultilineOp('=', txt);
     } else if (o.remove) {
       const txt = textLeft.substring(0, o.remove);
@@ -209,9 +199,9 @@ const randomTestChangeset = (origText, withAttribs) => {
 
   while (textLeft.length > 1) doOp();
   for (let i = 0; i < 5; i++) doOp(); // do some more (only insertions will happen)
-  const outText = `${outTextAssem.toString()}\n`;
+  const outText = `${outTextAssem}\n`;
   const serializedOps = Changeset.serializeOps(Changeset.canonicalizeOps(ops, true));
-  const cs = Changeset.pack(oldLen, outText.length, serializedOps, charBank.toString());
+  const cs = Changeset.pack(oldLen, outText.length, serializedOps, charBank);
   Changeset.checkRep(cs);
   return [cs, outText];
 };

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

@@ -15,7 +15,7 @@ describe('easysync-mutations', function () {
   };
 
   const mutationsToChangeset = (oldLen, arrayOfArrays) => {
-    const bank = Changeset.stringAssembler();
+    let bank = '';
     let oldPos = 0;
     let newLen = 0;
     const ops = (function* () {
@@ -34,7 +34,7 @@ describe('easysync-mutations', function () {
           oldPos += op.chars;
         } else if (a[0] === 'insert') {
           op.opcode = '+';
-          bank.append(a[1]);
+          bank += a[1];
           op.chars = a[1].length;
           op.lines = (a[2] || 0);
           newLen += op.chars;
@@ -44,7 +44,7 @@ describe('easysync-mutations', function () {
     })();
     const serializedOps = Changeset.serializeOps(Changeset.canonicalizeOps(ops, true));
     newLen += oldLen - oldPos;
-    return Changeset.pack(oldLen, newLen, serializedOps, bank.toString());
+    return Changeset.pack(oldLen, newLen, serializedOps, bank);
   };
 
   const runMutationTest = (testId, origLines, muts, correct) => {