diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js index 949002c7d..fd34f39e4 100644 --- a/src/static/js/Changeset.js +++ b/src/static/js/Changeset.js @@ -639,43 +639,34 @@ exports.stringAssembler = () => ({ * actually a newline, but for the purposes of N and L values, the caller should pretend it is, and * for things to work right in that case, the input to the `insert` method should be a single line * with no newlines. - * - * @typedef {object} TextLinesMutator - * @property {Function} close - - * @property {Function} hasMore - - * @property {Function} insert - - * @property {Function} remove - - * @property {Function} removeLines - - * @property {Function} skip - - * @property {Function} skipLines - */ - -/** - * @param {(string[]|StringArrayLike)} lines - Lines to mutate (in place). - * @returns {TextLinesMutator} - */ -const textLinesMutator = (lines) => { +class TextLinesMutator { /** - * curSplice holds values that will be passed as arguments to lines.splice() to insert, delete, or - * change lines: - * - curSplice[0] is an index into the lines array. - * - curSplice[1] is the number of lines that will be removed from the lines array starting at - * the index. - * - The other elements represent mutated (changed by ops) lines or new lines (added by ops) to - * insert at the index. - * - * @type {[number, number?, ...string[]?]} + * @param {(string[]|StringArrayLike)} lines - Lines to mutate (in place). */ - const curSplice = [0, 0]; - let inSplice = false; - - // position in lines after curSplice is applied: - let curLine = 0; - let curCol = 0; - // invariant: if (inSplice) then (curLine is in curSplice[0] + curSplice.length - {2,3}) && - // curLine >= curSplice[0] - // invariant: if (inSplice && (curLine >= curSplice[0] + curSplice.length - 2)) then - // curCol == 0 + constructor(lines) { + this._lines = lines; + /** + * this._curSplice holds values that will be passed as arguments to this._lines.splice() to + * insert, delete, or change lines: + * - this._curSplice[0] is an index into the this._lines array. + * - this._curSplice[1] is the number of lines that will be removed from the this._lines array + * starting at the index. + * - The other elements represent mutated (changed by ops) lines or new lines (added by ops) + * to insert at the index. + * + * @type {[number, number?, ...string[]?]} + */ + this._curSplice = [0, 0]; + this._inSplice = false; + // position in lines after curSplice is applied: + this._curLine = 0; + this._curCol = 0; + // invariant: if (inSplice) then (curLine is in curSplice[0] + curSplice.length - {2,3}) && + // curLine >= curSplice[0] + // invariant: if (inSplice && (curLine >= curSplice[0] + curSplice.length - 2)) then + // curCol == 0 + } /** * Get a line from `lines` at given index. @@ -683,13 +674,13 @@ const textLinesMutator = (lines) => { * @param {number} idx - an index * @returns {string} */ - const linesGet = (idx) => { - if ('get' in lines) { - return lines.get(idx); + _linesGet(idx) { + if ('get' in this._lines) { + return this._lines.get(idx); } else { - return lines[idx]; + return this._lines[idx]; } - }; + } /** * Return a slice from `lines`. @@ -698,51 +689,50 @@ const textLinesMutator = (lines) => { * @param {number} end - the end index * @returns {string[]} */ - const linesSlice = (start, end) => { - if (lines.slice) { - return lines.slice(start, end); + _linesSlice(start, end) { + // can be unimplemented if removeLines's return value not needed + if (this._lines.slice) { + return this._lines.slice(start, end); } else { return []; } - }; + } /** * Return the length of `lines`. * * @returns {number} */ - const linesLength = () => { - if ((typeof lines.length) === 'number') { - return lines.length; + _linesLength() { + if (typeof this._lines.length === 'number') { + return this._lines.length; } else { - return lines.length(); + return this._lines.length(); } - }; + } /** * Starts a new splice. */ - const enterSplice = () => { - curSplice[0] = curLine; - curSplice[1] = 0; + _enterSplice() { + this._curSplice[0] = this._curLine; + this._curSplice[1] = 0; // TODO(doc) when is this the case? // check all enterSplice calls and changes to curCol - if (curCol > 0) { - putCurLineInSplice(); - } - inSplice = true; - }; + if (this._curCol > 0) this._putCurLineInSplice(); + this._inSplice = true; + } /** * Changes the lines array according to the values in curSplice and resets curSplice. Called via * close or TODO(doc). */ - const leaveSplice = () => { - lines.splice(...curSplice); - curSplice.length = 2; - curSplice[0] = curSplice[1] = 0; - inSplice = false; - }; + _leaveSplice() { + this._lines.splice(...this._curSplice); + this._curSplice.length = 2; + this._curSplice[0] = this._curSplice[1] = 0; + this._inSplice = false; + } /** * Indicates if curLine is already in the splice. This is necessary because the last element in @@ -752,20 +742,23 @@ const textLinesMutator = (lines) => { * * @returns {boolean} true if curLine is in splice */ - const isCurLineInSplice = () => (curLine - curSplice[0] < (curSplice.length - 2)); + _isCurLineInSplice() { + return this._curLine - this._curSplice[0] < this._curSplice.length - 2; + } /** * Incorporates current line into the splice and marks its old position to be deleted. * * @returns {number} the index of the added line in curSplice */ - const putCurLineInSplice = () => { - if (!isCurLineInSplice()) { - curSplice.push(linesGet(curSplice[0] + curSplice[1])); - curSplice[1]++; + _putCurLineInSplice() { + if (!this._isCurLineInSplice()) { + this._curSplice.push(this._linesGet(this._curSplice[0] + this._curSplice[1])); + this._curSplice[1]++; } - return 2 + curLine - curSplice[0]; // TODO should be the same as curSplice.length - 1 - }; + // TODO should be the same as this._curSplice.length - 1 + return 2 + this._curLine - this._curSplice[0]; + } /** * It will skip some newlines by putting them into the splice. @@ -773,30 +766,30 @@ const textLinesMutator = (lines) => { * @param {number} L - * @param {boolean} includeInSplice - indicates if attributes are present */ - const skipLines = (L, includeInSplice) => { + skipLines(L, includeInSplice) { if (!L) return; if (includeInSplice) { - if (!inSplice) enterSplice(); + if (!this._inSplice) this._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++; + this._curCol = 0; + this._putCurLineInSplice(); + this._curLine++; } } else { - if (inSplice) { + if (this._inSplice) { if (L > 1) { // TODO(doc) figure out why single lines are incorporated into splice instead of ignored - leaveSplice(); + this._leaveSplice(); } else { - putCurLineInSplice(); + this._putCurLineInSplice(); } } - curLine += L; - curCol = 0; + this._curLine += L; + this._curCol = 0; } // tests case foo in remove(), which isn't otherwise covered in current impl - }; + } /** * Skip some characters. Can contain newlines. @@ -805,20 +798,20 @@ const textLinesMutator = (lines) => { * @param {number} L - number of newlines to skip * @param {boolean} includeInSplice - indicates if attributes are present */ - const skip = (N, L, includeInSplice) => { + skip(N, L, includeInSplice) { if (!N) return; if (L) { - skipLines(L, includeInSplice); + this.skipLines(L, includeInSplice); } else { - if (includeInSplice && !inSplice) enterSplice(); - if (inSplice) { + if (includeInSplice && !this._inSplice) this._enterSplice(); + if (this._inSplice) { // although the line is put into splice curLine is not increased, because // only some chars are skipped, not the whole line - putCurLineInSplice(); + this._putCurLineInSplice(); } - curCol += N; + this._curCol += N; } - }; + } /** * Remove whole lines from lines array. @@ -826,9 +819,9 @@ const textLinesMutator = (lines) => { * @param {number} L - number of lines to remove * @returns {string} */ - const removeLines = (L) => { + removeLines(L) { if (!L) return ''; - if (!inSplice) enterSplice(); + if (!this._inSplice) this._enterSplice(); /** * Gets a string of joined lines after the end of the splice. @@ -837,32 +830,32 @@ const textLinesMutator = (lines) => { * @returns {string} joined lines */ const nextKLinesText = (k) => { - const m = curSplice[0] + curSplice[1]; - return linesSlice(m, m + k).join(''); + const m = this._curSplice[0] + this._curSplice[1]; + return this._linesSlice(m, m + k).join(''); }; let removed = ''; - if (isCurLineInSplice()) { - if (curCol === 0) { - removed = curSplice[curSplice.length - 1]; - curSplice.length--; + if (this._isCurLineInSplice()) { + if (this._curCol === 0) { + removed = this._curSplice[this._curSplice.length - 1]; + this._curSplice.length--; removed += nextKLinesText(L - 1); - curSplice[1] += L - 1; + this._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; + this._curSplice[1] += L - 1; + const sline = this._curSplice.length - 1; + removed = this._curSplice[sline].substring(this._curCol) + removed; + this._curSplice[sline] = this._curSplice[sline].substring(0, this._curCol) + + this._linesGet(this._curSplice[0] + this._curSplice[1]); + this._curSplice[1] += 1; } } else { removed = nextKLinesText(L); - curSplice[1] += L; + this._curSplice[1] += L; } return removed; - }; + } /** * Remove text from lines array. @@ -871,18 +864,18 @@ const textLinesMutator = (lines) => { * @param {number} L - lines to delete * @returns {string} */ - const remove = (N, L) => { + remove(N, L) { if (!N) return ''; - if (L) return removeLines(L); - if (!inSplice) enterSplice(); + if (L) return this.removeLines(L); + if (!this._inSplice) this._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); + const sline = this._putCurLineInSplice(); + const removed = this._curSplice[sline].substring(this._curCol, this._curCol + N); + this._curSplice[sline] = this._curSplice[sline].substring(0, this._curCol) + + this._curSplice[sline].substring(this._curCol + N); return removed; - }; + } /** * Inserts text into lines array. @@ -890,82 +883,69 @@ const textLinesMutator = (lines) => { * @param {string} text - the text to insert * @param {number} L - number of newlines in text */ - const insert = (text, L) => { + insert(text, L) { if (!text) return; - if (!inSplice) enterSplice(); + if (!this._inSplice) this._enterSplice(); if (L) { const newLines = exports.splitTextLines(text); - if (isCurLineInSplice()) { - const sline = curSplice.length - 1; + if (this._isCurLineInSplice()) { + const sline = this._curSplice.length - 1; /** @type {string} */ - const theLine = curSplice[sline]; - const lineCol = curCol; + const theLine = this._curSplice[sline]; + const lineCol = this._curCol; // insert the first new line - curSplice[sline] = theLine.substring(0, lineCol) + newLines[0]; - curLine++; + this._curSplice[sline] = theLine.substring(0, lineCol) + newLines[0]; + this._curLine++; newLines.splice(0, 1); // insert the remaining new lines - curSplice.push(...newLines); - curLine += newLines.length; + this._curSplice.push(...newLines); + this._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? + this._curSplice.push(theLine.substring(lineCol)); + this._curCol = 0; // TODO(doc) why is this not set to the length of last line? } else { - curSplice.push(...newLines); - curLine += newLines.length; + this._curSplice.push(...newLines); + this._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 sline = this._putCurLineInSplice(); + if (!this._curSplice[sline]) { const err = new Error( 'curSplice[sline] not populated, actual curSplice contents is ' + - `${JSON.stringify(curSplice)}. Possibly related to ` + + `${JSON.stringify(this._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; + this._curSplice[sline] = this._curSplice[sline].substring(0, this._curCol) + text + + this._curSplice[sline].substring(this._curCol); + this._curCol += text.length; } - }; + } /** * Checks if curLine (the line we are in when curSplice is applied) is the last line in `lines`. * * @returns {boolean} indicates if there are lines left */ - const hasMore = () => { - let docLines = linesLength(); - if (inSplice) { - docLines += curSplice.length - 2 - curSplice[1]; + hasMore() { + let docLines = this._linesLength(); + if (this._inSplice) { + docLines += this._curSplice.length - 2 - this._curSplice[1]; } - return curLine < docLines; - }; + return this._curLine < docLines; + } /** * Closes the splice */ - const close = () => { - if (inSplice) { - leaveSplice(); - } - }; - - const self = { - skip, - remove, - insert, - close, - hasMore, - removeLines, - skipLines, - }; - return self; -}; + close() { + if (this._inSplice) this._leaveSplice(); + } +} /** * Apply operations to other operations. @@ -1106,7 +1086,7 @@ exports.mutateTextLines = (cs, lines) => { const unpacked = exports.unpack(cs); const csIter = exports.opIterator(unpacked.ops); const bankIter = exports.stringIterator(unpacked.charBank); - const mut = textLinesMutator(lines); + const mut = new TextLinesMutator(lines); while (csIter.hasNext()) { const op = csIter.next(); switch (op.opcode) { @@ -1239,7 +1219,7 @@ 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 = textLinesMutator(lines); + const mut = new TextLinesMutator(lines); /** @type {?OpIter} */ let lineIter = null; @@ -2310,7 +2290,7 @@ const followAttributes = (att1, att2, pool) => { }; exports.exportedForTestingOnly = { + TextLinesMutator, followAttributes, - textLinesMutator, toSplices, }; diff --git a/src/tests/frontend/specs/easysync.js b/src/tests/frontend/specs/easysync.js index 79e81f41f..3ffd2a211 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.exportedForTestingOnly.textLinesMutator(lines); + const mu = new 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.exportedForTestingOnly.textLinesMutator(lines); + mu = new 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.exportedForTestingOnly.textLinesMutator(lines); + mu = new 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.exportedForTestingOnly.textLinesMutator(lines); + mu = new Changeset.exportedForTestingOnly.TextLinesMutator(lines); expect(mu.hasMore()).to.be(true); mu.remove(6, 3); expect(mu.hasMore()).to.be(true);