mirrors/etherpad-lite
1
0
Fork 0
mirror of https://github.com/ether/etherpad-lite.git synced 2025-04-21 07:56:16 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Changeset: Turn textLinesMutator() into a real class

Browse source
This commit is contained in:
Richard Hansen 2021-03-21 14:34:02 -04:00
parent dab881139d
commit fba0bb6dff
2 changed files with 143 additions and 163 deletions
Download patch file Download diff file Expand all files Collapse all files

280
src/static/js/Changeset.js
View file

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

8
src/tests/frontend/specs/easysync.js
View file

@ -92,7 +92,7 @@ describe('easysync', function () {
const runMutationTest = (testId, origLines, muts, correct) => { const runMutationTest = (testId, origLines, muts, correct) => {
it(`runMutationTest#${testId}`, async function () { it(`runMutationTest#${testId}`, async function () {
let lines = origLines.slice(); let lines = origLines.slice();
const mu = Changeset.exportedForTestingOnly.textLinesMutator(lines); const mu = new Changeset.exportedForTestingOnly.TextLinesMutator(lines);
applyMutations(mu, muts); applyMutations(mu, muts);
mu.close(); mu.close();
expect(lines).to.eql(correct); expect(lines).to.eql(correct);
@ -211,7 +211,7 @@ describe('easysync', function () {
const lines = ['1\n', '2\n', '3\n', '4\n']; const lines = ['1\n', '2\n', '3\n', '4\n'];
let mu; let mu;
mu = Changeset.exportedForTestingOnly.textLinesMutator(lines); mu = new Changeset.exportedForTestingOnly.TextLinesMutator(lines);
expect(mu.hasMore()).to.be(true); expect(mu.hasMore()).to.be(true);
mu.skip(8, 4); mu.skip(8, 4);
expect(mu.hasMore()).to.be(false); expect(mu.hasMore()).to.be(false);
@ -219,7 +219,7 @@ describe('easysync', function () {
expect(mu.hasMore()).to.be(false); expect(mu.hasMore()).to.be(false);
// still 1,2,3,4 // still 1,2,3,4
mu = Changeset.exportedForTestingOnly.textLinesMutator(lines); mu = new Changeset.exportedForTestingOnly.TextLinesMutator(lines);
expect(mu.hasMore()).to.be(true); expect(mu.hasMore()).to.be(true);
mu.remove(2, 1); mu.remove(2, 1);
expect(mu.hasMore()).to.be(true); expect(mu.hasMore()).to.be(true);
@ -235,7 +235,7 @@ describe('easysync', function () {
expect(mu.hasMore()).to.be(false); expect(mu.hasMore()).to.be(false);
// 2,3,4,5 now // 2,3,4,5 now
mu = Changeset.exportedForTestingOnly.textLinesMutator(lines); mu = new Changeset.exportedForTestingOnly.TextLinesMutator(lines);
expect(mu.hasMore()).to.be(true); expect(mu.hasMore()).to.be(true);
mu.remove(6, 3); mu.remove(6, 3);
expect(mu.hasMore()).to.be(true); expect(mu.hasMore()).to.be(true);