From cca906e8fcb8ae0b4cde25cae2e3c5c787cd6a1f Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Tue, 23 Nov 2021 21:45:13 -0500 Subject: [PATCH] Changeset: Avoid unnecessary `StringAssembler` class --- src/node/utils/ExportHtml.js | 30 +++--------- src/node/utils/ExportTxt.js | 6 +-- src/node/utils/padDiff.js | 8 +-- src/static/js/Changeset.js | 49 +++++++++---------- src/tests/frontend/easysync-helper.js | 38 ++++++-------- .../frontend/specs/easysync-mutations.js | 6 +-- 6 files changed, 54 insertions(+), 83 deletions(-) diff --git a/src/node/utils/ExportHtml.js b/src/node/utils/ExportHtml.js index 3d5f4cc72..0d585a88a 100644 --- a/src/node/utils/ExportHtml.js +++ b/src/node/utils/ExportHtml.js @@ -125,7 +125,7 @@ const getHTMLFromAtext = async (pad, atext, authorColors) => { // becomes // Just bold Bold and italics Just italics const taker = Changeset.stringIterator(text); - const assem = Changeset.stringAssembler(); + let assem = ''; const openTags = []; const getSpanClassFor = (i) => { @@ -161,16 +161,7 @@ const getHTMLFromAtext = async (pad, atext, authorColors) => { const emitOpenTag = (i) => { openTags.unshift(i); const spanClass = getSpanClassFor(i); - - if (spanClass) { - assem.append(''); - } else { - assem.append('<'); - assem.append(tags[i]); - assem.append('>'); - } + assem += spanClass ? `` : `<${tags[i]}>`; }; // this closes an open tag and removes its reference from openTags @@ -178,14 +169,7 @@ const getHTMLFromAtext = async (pad, atext, authorColors) => { openTags.shift(); const spanClass = getSpanClassFor(i); const spanWithData = isSpanWithData(i); - - if (spanClass || spanWithData) { - assem.append(''); - } else { - assem.append(''); - } + assem += spanClass || spanWithData ? '' : ``; }; 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(``); + assem += ``; processNextChars(urlLength); - assem.append(''); + assem += ''; }); } processNextChars(text.length - idx); - return _processSpaces(assem.toString()); + return _processSpaces(assem); }; // end getLineHTML const pieces = [css]; diff --git a/src/node/utils/ExportTxt.js b/src/node/utils/ExportTxt.js index 9511dd0e7..07c2a67e3 100644 --- a/src/node/utils/ExportTxt.js +++ b/src/node/utils/ExportTxt.js @@ -67,7 +67,7 @@ const getTXTFromAtext = (pad, atext, authorColors) => { // becomes // Just bold Bold and italics Just italics 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 diff --git a/src/node/utils/padDiff.js b/src/node/utils/padDiff.js index d3fcea611..a7271e8aa 100644 --- a/src/node/utils/padDiff.js +++ b/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) => { diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js index d747de138..1b5e18e9b 100644 --- a/src/static/js/Changeset.js +++ b/src/static/js/Changeset.js @@ -731,7 +731,11 @@ class StringAssembler { /** * @returns {StringAssembler} */ -exports.stringAssembler = () => new StringAssembler(); +exports.stringAssembler = () => { + padutils.warnDeprecated( + 'Changeset.stringAssembler() is deprecated; build a string manually instead'); + return new StringAssembler(); +}; /** * @typedef {object} StringArrayLike @@ -1158,7 +1162,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 '+': @@ -1167,7 +1171,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 @@ -1183,12 +1187,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; }; /** @@ -1477,7 +1481,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; @@ -1487,16 +1491,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); }; /** @@ -1920,7 +1920,7 @@ class Builder { constructor(oldLen) { this._oldLen = oldLen; this._ops = []; - this._charBank = new StringAssembler(); + this._charBank = ''; } /** @@ -1966,7 +1966,7 @@ class Builder { */ insert(text, attribs = '', pool = null) { this._ops.push(...opsFromText('+', text, attribs, pool)); - this._charBank.append(text); + this._charBank += text; return this; } @@ -1998,7 +1998,7 @@ class Builder { oldLen: this._oldLen, newLen: this._oldLen + lengthChange, ops: serializedOps, - charBank: this._charBank.toString(), + charBank: this._charBank, }; } @@ -2181,20 +2181,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) => { @@ -2407,12 +2407,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 = { diff --git a/src/tests/frontend/easysync-helper.js b/src/tests/frontend/easysync-helper.js index d79366d5b..7da6de97f 100644 --- a/src/tests/frontend/easysync-helper.js +++ b/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]; }; diff --git a/src/tests/frontend/specs/easysync-mutations.js b/src/tests/frontend/specs/easysync-mutations.js index 5594213f8..64526cca9 100644 --- a/src/tests/frontend/specs/easysync-mutations.js +++ b/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) => {