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/node/utils/padDiff.js b/src/node/utils/padDiff.js index 67c5f68ffd8..670e8d6a17c 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); } } @@ -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); @@ -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.next(curLineNextOp); + 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; } } @@ -307,7 +305,7 @@ PadDiff.prototype._createDeletionChangeset = function (cs, startAText, apool) { } if (!curLineNextOp.chars) { - curLineOpIter.next(curLineNextOp); + 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 f43a236cf14..21de89c43da 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; @@ -158,12 +158,12 @@ 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(); } 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; @@ -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` */ -exports.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. @@ -280,13 +276,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 +297,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; }; @@ -425,41 +421,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(); - exports.copyOp(op, bufOp); + // append in-line text to multi-line bufOp + bufOpAdditionalCharsAfterNewline += op.chars; } + } else { + flush(); + copyOp(op, bufOp); } }; @@ -474,7 +468,7 @@ exports.mergingOpAssembler = () => { const clear = () => { assem.clear(); - exports.clearOp(bufOp); + clearOp(bufOp); }; return { append, @@ -488,24 +482,24 @@ 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.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)}`; + serialized += op.opcode; + serialized += exports.numToString(op.chars); }; - const toString = () => pieces.join(''); + const toString = () => serialized; const clear = () => { - pieces.length = 0; + serialized = ''; }; return { append, @@ -536,7 +530,7 @@ exports.stringIterator = (str) => { const getnewLines = () => newLines; const assertRemaining = (n) => { - exports.assert(n <= remaining(), `!(${n} <= ${remaining()})`); + assert(n <= remaining(), `!(${n} <= ${remaining()})`); }; const take = (n) => { @@ -579,22 +573,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 @@ -633,7 +619,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: @@ -656,15 +642,6 @@ exports.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. * @@ -672,7 +649,7 @@ exports.textLinesMutator = (lines) => { * @returns {string} */ const linesGet = (idx) => { - if (lines.get) { + if ('get' in lines) { return lines.get(idx); } else { return lines[idx]; @@ -726,7 +703,7 @@ exports.textLinesMutator = (lines) => { * close or TODO(doc). */ const leaveSplice = () => { - linesApplySplice(curSplice); + lines.splice(...curSplice); curSplice.length = 2; curSplice[0] = curSplice[1] = 0; inSplice = false; @@ -762,31 +739,28 @@ exports.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 +771,17 @@ exports.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 +792,39 @@ exports.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 +837,15 @@ exports.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 +856,45 @@ exports.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 + 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 { - // 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; + curSplice.push(...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]) { + 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); + curCol += text.length; } }; @@ -976,7 +939,7 @@ exports.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 @@ -985,26 +948,22 @@ exports.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 */ -exports.applyZip = (in1, in2, func) => { +const applyZip = (in1, in2, func) => { const iter1 = exports.opIterator(in1); const iter2 = exports.opIterator(in2); 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(); @@ -1020,7 +979,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 +1023,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 +1071,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) { @@ -1131,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. * @@ -1162,36 +1129,25 @@ 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]) { - 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 ''; }); - atts.sort(); const buf = exports.stringAssembler(); - for (let i = 0; i < atts.length; i++) { + for (const att of sortAttribs([...atts])) { buf.append('*'); - buf.append(exports.numToString(pool.putAttrib(atts[i]))); + buf.append(exports.numToString(pool.putAttrib(att))); } return buf.toString(); }; @@ -1202,91 +1158,60 @@ 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`. */ -exports._slicerZipperFunc = (attOp, csOp, opOut, pool) => { - if (attOp.opcode === '-') { - exports.copyOp(attOp, opOut); +const slicerZipperFunc = (attOp, csOp, pool) => { + const opOut = exports.newOp(); + 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) { - exports.copyOp(csOp, opOut); + } 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; - opOut.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; - opOut.attribs = ''; - } - csOp.chars -= attOp.chars; - csOp.lines -= attOp.lines; - attOp.opcode = ''; - } - break; - } - case '+': - { - // insert - exports.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 '': - { - exports.copyOp(attOp, opOut); - attOp.opcode = ''; - break; - } + 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. + '=': '+', + }, + '=': { + '-': '-', + '=': '=', + }, + }[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; }; /** @@ -1299,9 +1224,7 @@ 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) => slicerZipperFunc(op1, op2, pool)); }; exports.mutateAttributionLines = (cs, lines, pool) => { @@ -1310,23 +1233,20 @@ 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; 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; @@ -1335,21 +1255,17 @@ exports.mutateAttributionLines = (cs, lines, pool) => { lineAssem = exports.mergingOpAssembler(); } lineAssem.append(op); - if (op.lines > 0) { - exports.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(); - 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) && @@ -1358,33 +1274,26 @@ 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; - exports.copyOp(csOp, opOut); csOp.chars -= firstLineLen; csOp.lines--; opOut.lines = 1; opOut.chars = firstLineLen; } else { - exports.copyOp(csOp, opOut); csOp.opcode = ''; } outputMutOp(opOut); csBankIndex += opOut.chars; - opOut.opcode = ''; } else { - if ((!attOp.opcode) && isNextMutOp()) { - nextMutOp(attOp); - } - exports._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); } } - exports.assert(!lineAssem, `line assembler not finished:${cs}`); + assert(!lineAssem, `line assembler not finished:${cs}`); mut.close(); }; @@ -1396,8 +1305,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()); @@ -1427,7 +1335,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 +1373,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) => { 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); + const opOut = slicerZipperFunc(op1, op2, pool); if (opOut.opcode === '+') { if (op2code === '+') { bankAssem.append(bankIter2.take(opOut.chars)); @@ -1485,6 +1393,7 @@ exports.compose = (cs1, cs2, pool) => { bankAssem.append(bankIter1.take(opOut.chars)); } } + return opOut; }); return exports.pack(len1, len3, newOps, bankAssem.toString()); @@ -1500,16 +1409,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); }; /** @@ -1560,7 +1464,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,10 +1505,8 @@ exports.toSplices = (cs) => { exports.characterRangeFollow = (cs, startChar, endChar, insertionsAfter) => { let newStartChar = startChar; let newEndChar = endChar; - const splices = exports.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; @@ -1661,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; @@ -1791,12 +1685,11 @@ exports.applyToAText = (cs, atext, pool) => ({ * @returns {AText} */ exports.cloneAText = (atext) => { - if (atext) { - return { - text: atext.text, - attribs: atext.attribs, - }; - } else { exports.error('atext is null'); } + if (!atext) error('atext is null'); + return { + text: atext.text, + attribs: atext.attribs, + }; }; /** @@ -1819,34 +1712,26 @@ 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(); + let lastOp = null; while (iter.hasNext()) { - iter.next(op); - 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); }; /** @@ -1895,14 +1780,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; }; @@ -2007,11 +1891,10 @@ exports.makeAttribsString = (opcode, attribs, pool) => { } else if (pool && attribs.length) { if (attribs.length > 1) { attribs = attribs.slice(); - attribs.sort(); + sortAttribs(attribs); } 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))}`); } @@ -2026,26 +1909,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) { - 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++; - } - - exports._slicerZipperFunc(attOp, csOp, opOut, null); - if (opOut.opcode) { - assem.append(opOut); - opOut.opcode = ''; - } + if (!csOp.chars) return; + while (csOp.opcode && (attOp.opcode || iter.hasNext())) { + if (!attOp.opcode) attOp = iter.next(); + if (csOp.opcode && attOp.opcode && csOp.chars >= attOp.chars && + attOp.lines > 0 && csOp.lines <= 0) { + csOp.lines++; } + const opOut = slicerZipperFunc(attOp, csOp, null); + if (opOut.opcode) assem.append(opOut); } }; @@ -2058,10 +1934,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; @@ -2100,7 +1973,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); @@ -2112,15 +1985,13 @@ exports.inverse = (cs, lines, alines, pool) => { curLineOpIter = exports.opIterator(alinesGet(curLine)); curLineOpIterLine = curLine; let indexIntoLine = 0; - let done = false; - while (!done && curLineOpIter.hasNext()) { - curLineOpIter.next(curLineNextOp); + 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; } } @@ -2133,7 +2004,7 @@ exports.inverse = (cs, lines, alines, pool) => { curLineOpIter = exports.opIterator(alinesGet(curLine)); } if (!curLineNextOp.chars) { - curLineOpIter.next(curLineNextOp); + curLineNextOp = curLineOpIter.hasNext() ? curLineOpIter.next() : exports.newOp(); } const charsToUse = Math.min(numChars, curLineNextOp.chars); func(charsToUse, curLineNextOp.attribs, charsToUse === curLineNextOp.chars && @@ -2188,23 +2059,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]); @@ -2240,7 +2103,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 +2113,8 @@ 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) => { + const opOut = exports.newOp(); if (op1.opcode === '+' || op2.opcode === '+') { let whichToDo; if (op2.opcode !== '+') { @@ -2289,7 +2153,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 +2172,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 +2192,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; @@ -2368,13 +2232,14 @@ exports.follow = (cs1, cs2, reverseInsertOrder, pool) => { newLen += opOut.chars; break; } + return opOut; }); newLen += oldLen - oldPos; 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 @@ -2383,151 +2248,28 @@ exports.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]) { - 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 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(); }; -exports.composeWithDeletions = (cs1, cs2, pool) => { - const unpacked1 = exports.unpack(cs1); - const unpacked2 = exports.unpack(cs2); - const len1 = unpacked1.oldLen; - const len2 = unpacked1.newLen; - exports.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 op1code = op1.opcode; - const op2code = op2.opcode; - if (op1code === '+' && op2code === '-') { - bankIter1.skip(Math.min(op1.chars, op2.chars)); - } - exports._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 -exports._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); - attOp.opcode = ''; - } else if (!attOp.opcode) { - exports.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 - exports.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 '': - { - exports.copyOp(attOp, opOut); - attOp.opcode = ''; - break; - } - } - } +exports.exportedForTestingOnly = { + followAttributes, + textLinesMutator, + toSplices, }; 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; 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(); 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) => {