Skip to content

Commit 849f2aa

Browse files
rhansenrhansen
committed
Changeset: Avoid unnecessary StringAssembler class
1 parent 6a83881 commit 849f2aa

File tree

5 files changed

+54
-83
lines changed

5 files changed

+54
-83
lines changed

‎src/node/utils/ExportHtml.js‎

Lines changed: 7 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ const getHTMLFromAtext = async (pad, atext, authorColors) => {
125125
// becomes
126126
// <b>Just bold <i>Bold and italics</i></b> <i>Just italics</i>
127127
const taker = Changeset.stringIterator(text);
128-
const assem = Changeset.stringAssembler();
128+
let assem = '';
129129
const openTags = [];
130130

131131
const getSpanClassFor = (i) => {
@@ -161,31 +161,15 @@ const getHTMLFromAtext = async (pad, atext, authorColors) => {
161161
const emitOpenTag = (i) => {
162162
openTags.unshift(i);
163163
const spanClass = getSpanClassFor(i);
164-
165-
if (spanClass) {
166-
assem.append('<span class="');
167-
assem.append(spanClass);
168-
assem.append('">');
169-
} else {
170-
assem.append('<');
171-
assem.append(tags[i]);
172-
assem.append('>');
173-
}
164+
assem += spanClass ? `<span class="${spanClass}">` : `<${tags[i]}>`;
174165
};
175166

176167
// this closes an open tag and removes its reference from openTags
177168
const emitCloseTag = (i) => {
178169
openTags.shift();
179170
const spanClass = getSpanClassFor(i);
180171
const spanWithData = isSpanWithData(i);
181-
182-
if (spanClass || spanWithData) {
183-
assem.append('</span>');
184-
} else {
185-
assem.append('</');
186-
assem.append(tags[i]);
187-
assem.append('>');
188-
}
172+
assem += spanClass || spanWithData ? '</span>' : `</${tags[i]}>`;
189173
};
190174

191175
const urls = padutils.findURLs(text);
@@ -246,7 +230,7 @@ const getHTMLFromAtext = async (pad, atext, authorColors) => {
246230
// from but they break the abiword parser and are completly useless
247231
s = s.replace(String.fromCharCode(12), '');
248232

249-
assem.append(_encodeWhitespace(Security.escapeHTML(s)));
233+
assem += _encodeWhitespace(Security.escapeHTML(s));
250234
} // end iteration over spans in line
251235

252236
// close all the tags that are open after the last op
@@ -269,14 +253,14 @@ const getHTMLFromAtext = async (pad, atext, authorColors) => {
269253
// https://html.spec.whatwg.org/multipage/links.html#link-type-noopener
270254
// https://mathiasbynens.github.io/rel-noopener/
271255
// https://github.com/ether/etherpad-lite/pull/3636
272-
assem.append(`<a href="${Security.escapeHTMLAttribute(url)}" rel="noreferrer noopener">`);
256+
assem += `<a href="${Security.escapeHTMLAttribute(url)}" rel="noreferrer noopener">`;
273257
processNextChars(urlLength);
274-
assem.append('</a>');
258+
assem += '</a>';
275259
});
276260
}
277261
processNextChars(text.length - idx);
278262

279-
return _processSpaces(assem.toString());
263+
return _processSpaces(assem);
280264
};
281265
// end getLineHTML
282266
const pieces = [css];

‎src/node/utils/ExportTxt.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ const getTXTFromAtext = (pad, atext, authorColors) => {
6767
// becomes
6868
// <b>Just bold <i>Bold and italics</i></b> <i>Just italics</i>
6969
const taker = Changeset.stringIterator(text);
70-
const assem = Changeset.stringAssembler();
70+
let assem = '';
7171

7272
let idx = 0;
7373

@@ -161,7 +161,7 @@ const getTXTFromAtext = (pad, atext, authorColors) => {
161161
// plugins from being able to display * at the beginning of a line
162162
// s = s.replace("*", ""); // Then remove it
163163

164-
assem.append(s);
164+
assem += s;
165165
} // end iteration over spans in line
166166

167167
const tags2close = [];
@@ -175,7 +175,7 @@ const getTXTFromAtext = (pad, atext, authorColors) => {
175175
// end processNextChars
176176

177177
processNextChars(text.length - idx);
178-
return (assem.toString());
178+
return assem;
179179
};
180180
// end getLineHTML
181181

‎src/node/utils/padDiff.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -328,21 +328,21 @@ PadDiff.prototype._createDeletionChangeset = function (cs, startAText, apool) {
328328

329329
const nextText = (numChars) => {
330330
let len = 0;
331-
const assem = Changeset.stringAssembler();
331+
let assem = '';
332332
const firstString = linesGet(curLine).substring(curChar);
333333
len += firstString.length;
334-
assem.append(firstString);
334+
assem += firstString;
335335

336336
let lineNum = curLine + 1;
337337

338338
while (len < numChars) {
339339
const nextString = linesGet(lineNum);
340340
len += nextString.length;
341-
assem.append(nextString);
341+
assem += nextString;
342342
lineNum++;
343343
}
344344

345-
return assem.toString().substring(0, numChars);
345+
return assem.substring(0, numChars);
346346
};
347347

348348
const cachedStrFunc = (func) => {

‎src/static/js/Changeset.js‎

Lines changed: 23 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -743,7 +743,11 @@ class StringAssembler {
743743
/**
744744
* @returns {StringAssembler}
745745
*/
746-
exports.stringAssembler = () => new StringAssembler();
746+
exports.stringAssembler = () => {
747+
padutils.warnWithStack(
748+
'Changeset.stringAssembler() is deprecated; build a string manually instead');
749+
return new StringAssembler();
750+
};
747751

748752
/**
749753
* @typedef {object} StringArrayLike
@@ -1172,7 +1176,7 @@ exports.applyToText = (cs, str) => {
11721176
assert(str.length === unpacked.oldLen, `mismatched apply: ${str.length} / ${unpacked.oldLen}`);
11731177
const bankIter = exports.stringIterator(unpacked.charBank);
11741178
const strIter = exports.stringIterator(str);
1175-
const assem = new StringAssembler();
1179+
let assem = '';
11761180
for (const op of exports.deserializeOps(unpacked.ops)) {
11771181
switch (op.opcode) {
11781182
case '+':
@@ -1181,7 +1185,7 @@ exports.applyToText = (cs, str) => {
11811185
if (op.lines !== bankIter.peek(op.chars).split('\n').length - 1) {
11821186
throw new Error(`newline count is wrong in op +; cs:${cs} and text:${str}`);
11831187
}
1184-
assem.append(bankIter.take(op.chars));
1188+
assem += bankIter.take(op.chars);
11851189
break;
11861190
case '-':
11871191
// op is - and op.lines 0: no newlines must be in the deleted string
@@ -1197,12 +1201,12 @@ exports.applyToText = (cs, str) => {
11971201
if (op.lines !== strIter.peek(op.chars).split('\n').length - 1) {
11981202
throw new Error(`newline count is wrong in op =; cs:${cs} and text:${str}`);
11991203
}
1200-
assem.append(strIter.take(op.chars));
1204+
assem += strIter.take(op.chars);
12011205
break;
12021206
}
12031207
}
1204-
assem.append(strIter.take(strIter.remaining()));
1205-
return assem.toString();
1208+
assem += strIter.take(strIter.remaining());
1209+
return assem;
12061210
};
12071211

12081212
/**
@@ -1491,7 +1495,7 @@ exports.compose = (cs1, cs2, pool) => {
14911495
const len3 = unpacked2.newLen;
14921496
const bankIter1 = exports.stringIterator(unpacked1.charBank);
14931497
const bankIter2 = exports.stringIterator(unpacked2.charBank);
1494-
const bankAssem = new StringAssembler();
1498+
let bankAssem = '';
14951499

14961500
const newOps = applyZip(unpacked1.ops, unpacked2.ops, (op1, op2) => {
14971501
const op1code = op1.opcode;
@@ -1501,16 +1505,12 @@ exports.compose = (cs1, cs2, pool) => {
15011505
}
15021506
const opOut = slicerZipperFunc(op1, op2, pool);
15031507
if (opOut.opcode === '+') {
1504-
if (op2code === '+') {
1505-
bankAssem.append(bankIter2.take(opOut.chars));
1506-
} else {
1507-
bankAssem.append(bankIter1.take(opOut.chars));
1508-
}
1508+
bankAssem += (op2code === '+' ? bankIter2 : bankIter1).take(opOut.chars);
15091509
}
15101510
return opOut;
15111511
});
15121512

1513-
return exports.pack(len1, len3, newOps, bankAssem.toString());
1513+
return exports.pack(len1, len3, newOps, bankAssem);
15141514
};
15151515

15161516
/**
@@ -1939,7 +1939,7 @@ class Builder {
19391939
constructor(oldLen) {
19401940
this._oldLen = oldLen;
19411941
this._ops = [];
1942-
this._charBank = new StringAssembler();
1942+
this._charBank = '';
19431943
}
19441944

19451945
/**
@@ -1985,7 +1985,7 @@ class Builder {
19851985
*/
19861986
insert(text, attribs = '', pool = null) {
19871987
this._ops.push(...opsFromText('+', text, attribs, pool));
1988-
this._charBank.append(text);
1988+
this._charBank += text;
19891989
return this;
19901990
}
19911991

@@ -2017,7 +2017,7 @@ class Builder {
20172017
oldLen: this._oldLen,
20182018
newLen: this._oldLen + lengthChange,
20192019
ops: serializedOps,
2020-
charBank: this._charBank.toString(),
2020+
charBank: this._charBank,
20212021
};
20222022
}
20232023

@@ -2200,20 +2200,20 @@ exports.inverse = (cs, lines, alines, pool) => {
22002200

22012201
const nextText = (numChars) => {
22022202
let len = 0;
2203-
const assem = new StringAssembler();
2203+
let assem = '';
22042204
const firstString = linesGet(curLine).substring(curChar);
22052205
len += firstString.length;
2206-
assem.append(firstString);
2206+
assem += firstString;
22072207

22082208
let lineNum = curLine + 1;
22092209
while (len < numChars) {
22102210
const nextString = linesGet(lineNum);
22112211
len += nextString.length;
2212-
assem.append(nextString);
2212+
assem += nextString;
22132213
lineNum++;
22142214
}
22152215

2216-
return assem.toString().substring(0, numChars);
2216+
return assem.substring(0, numChars);
22172217
};
22182218

22192219
const cachedStrFunc = (func) => {
@@ -2426,12 +2426,9 @@ const followAttributes = (att1, att2, pool) => {
24262426
return '';
24272427
});
24282428
// we've only removed attributes, so they're already sorted
2429-
const buf = new StringAssembler();
2430-
for (const att of atts) {
2431-
buf.append('*');
2432-
buf.append(exports.numToString(pool.putAttrib(att)));
2433-
}
2434-
return buf.toString();
2429+
let buf = '';
2430+
for (const att of atts) buf += `*${exports.numToString(pool.putAttrib(att))}`;
2431+
return buf;
24352432
};
24362433

24372434
exports.exportedForTestingOnly = {

‎src/tests/frontend/specs/easysync.js‎

Lines changed: 17 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ describe('easysync', function () {
5050
};
5151

5252
const mutationsToChangeset = (oldLen, arrayOfArrays) => {
53-
const bank = Changeset.stringAssembler();
53+
let bank = '';
5454
let oldPos = 0;
5555
let newLen = 0;
5656
const ops = (function* () {
@@ -69,7 +69,7 @@ describe('easysync', function () {
6969
oldPos += op.chars;
7070
} else if (a[0] === 'insert') {
7171
op.opcode = '+';
72-
bank.append(a[1]);
72+
bank += a[1];
7373
op.chars = a[1].length;
7474
op.lines = (a[2] || 0);
7575
newLen += op.chars;
@@ -79,7 +79,7 @@ describe('easysync', function () {
7979
})();
8080
const serializedOps = Changeset.serializeOps(Changeset.canonicalizeOps(ops, true));
8181
newLen += oldLen - oldPos;
82-
return Changeset.pack(oldLen, newLen, serializedOps, bank.toString());
82+
return Changeset.pack(oldLen, newLen, serializedOps, bank);
8383
};
8484

8585
const runMutationTest = (testId, origLines, muts, correct) => {
@@ -350,29 +350,19 @@ describe('easysync', function () {
350350
]);
351351

352352
const randomInlineString = (len) => {
353-
const assem = Changeset.stringAssembler();
354-
for (let i = 0; i < len; i++) {
355-
assem.append(String.fromCharCode(randInt(26) + 97));
356-
}
357-
return assem.toString();
353+
let assem = '';
354+
for (let i = 0; i < len; i++) assem += String.fromCharCode(randInt(26) + 97);
355+
return assem;
358356
};
359357

360358
const randomMultiline = (approxMaxLines, approxMaxCols) => {
361359
const numParts = randInt(approxMaxLines * 2) + 1;
362-
const txt = Changeset.stringAssembler();
363-
txt.append(randInt(2) ? '\n' : '');
360+
let txt = '';
361+
txt += randInt(2) ? '\n' : '';
364362
for (let i = 0; i < numParts; i++) {
365-
if ((i % 2) === 0) {
366-
if (randInt(10)) {
367-
txt.append(randomInlineString(randInt(approxMaxCols) + 1));
368-
} else {
369-
txt.append('\n');
370-
}
371-
} else {
372-
txt.append('\n');
373-
}
363+
txt += i % 2 === 0 && randInt(10) ? randomInlineString(randInt(approxMaxCols) + 1) : '\n';
374364
}
375-
return txt.toString();
365+
return txt;
376366
};
377367

378368
const randomStringOperation = (numCharsLeft) => {
@@ -494,9 +484,9 @@ describe('easysync', function () {
494484
};
495485

496486
const randomTestChangeset = (origText, withAttribs) => {
497-
const charBank = Changeset.stringAssembler();
487+
let charBank = '';
498488
let textLeft = origText; // always keep final newline
499-
const outTextAssem = Changeset.stringAssembler();
489+
let outTextAssem = '';
500490
const ops = [];
501491
const oldLen = origText.length;
502492

@@ -521,13 +511,13 @@ describe('easysync', function () {
521511
const o = randomStringOperation(textLeft.length);
522512
if (o.insert) {
523513
const txt = o.insert;
524-
charBank.append(txt);
525-
outTextAssem.append(txt);
514+
charBank += txt;
515+
outTextAssem += txt;
526516
appendMultilineOp('+', txt);
527517
} else if (o.skip) {
528518
const txt = textLeft.substring(0, o.skip);
529519
textLeft = textLeft.substring(o.skip);
530-
outTextAssem.append(txt);
520+
outTextAssem += txt;
531521
appendMultilineOp('=', txt);
532522
} else if (o.remove) {
533523
const txt = textLeft.substring(0, o.remove);
@@ -538,9 +528,9 @@ describe('easysync', function () {
538528

539529
while (textLeft.length > 1) doOp();
540530
for (let i = 0; i < 5; i++) doOp(); // do some more (only insertions will happen)
541-
const outText = `${outTextAssem.toString()}\n`;
531+
const outText = `${outTextAssem}\n`;
542532
const serializedOps = Changeset.serializeOps(Changeset.canonicalizeOps(ops, true));
543-
const cs = Changeset.pack(oldLen, outText.length, serializedOps, charBank.toString());
533+
const cs = Changeset.pack(oldLen, outText.length, serializedOps, charBank);
544534
Changeset.checkRep(cs);
545535
return [cs, outText];
546536
};

0 commit comments

Comments
 (0)