diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js index 53b3f2c8f09..f14433a7dd5 100644 --- a/src/static/js/Changeset.js +++ b/src/static/js/Changeset.js @@ -908,8 +908,10 @@ class TextLinesMutator { let removed = ''; if (this._isCurLineInSplice()) { if (this._curCol === 0) { + // First line to be removed is in splice. removed = this._curSplice[this._curSplice.length - 1]; this._curSplice.length--; + // Next lines to be removed are not in splice. removed += nextKLinesText(L - 1); this._curSplice[1] += L - 1; } else { @@ -917,11 +919,13 @@ class TextLinesMutator { 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; + // Is there a line left? + const remaining = this._linesGet(this._curSplice[0] + this._curSplice[1]) || ''; + this._curSplice[sline] = this._curSplice[sline].substring(0, this._curCol) + remaining; + this._curSplice[1] += remaining ? 1 : 0; } } else { + // Nothing that is removed is in splice. Implies curCol === 0. removed = nextKLinesText(L); this._curSplice[1] += L; } @@ -973,17 +977,24 @@ class TextLinesMutator { 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) - this._curSplice.push(theLine.substring(lineCol)); - this._curCol = 0; // TODO(doc) why is this not set to the length of last line? + const remaining = theLine.substring(lineCol); + if (remaining !== '') this._curSplice.push(remaining); + this._curCol = 0; } else { this._curSplice.push(...newLines); this._curLine += newLines.length; } - } else { + } else if (!this.hasMore()) { // 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). + // increased because there may be more chars in the line (newline is not reached). We are + // inserting at the end of lines. curCol is 0 as curLine is not in splice. + this._curSplice.push(text); + this._curCol += text.length; + } else { + // insert text after curCol const sline = this._putCurLineInSplice(); if (!this._curSplice[sline]) { + // TODO should never happen now const err = new Error( 'curSplice[sline] not populated, actual curSplice contents is ' + `${JSON.stringify(this._curSplice)}. Possibly related to ` + diff --git a/src/tests/frontend/specs/easysync-assembler.js b/src/tests/frontend/specs/easysync-assembler.js index 80a6636d223..bd66e75f1d2 100644 --- a/src/tests/frontend/specs/easysync-assembler.js +++ b/src/tests/frontend/specs/easysync-assembler.js @@ -92,6 +92,50 @@ describe('easysync-assembler', function () { expect(assem.toString()).to.equal('-k'); }); + it('smartOpAssembler append - op with text', async function () { + const assem = Changeset.smartOpAssembler(); + const pool = poolOrArray([ + 'attr1,1', + 'attr2,2', + 'attr3,3', + 'attr4,4', + 'attr5,5', + ]); + + padutils.warnDeprecated.disabledForTestingOnly = true; + try { + assem.appendOpWithText('-', 'test', '*3*4*5', pool); + assem.appendOpWithText('-', 'test', '*3*4*5', pool); + assem.appendOpWithText('-', 'test', '*1*4*5', pool); + } finally { + delete padutils.warnDeprecated.disabledForTestingOnly; + } + assem.endDocument(); + expect(assem.toString()).to.equal('*3*4*5-8*1*4*5-4'); + }); + + it('smartOpAssembler append - op with multiline text', async function () { + const assem = Changeset.smartOpAssembler(); + const pool = poolOrArray([ + 'attr1,1', + 'attr2,2', + 'attr3,3', + 'attr4,4', + 'attr5,5', + ]); + + padutils.warnDeprecated.disabledForTestingOnly = true; + try { + assem.appendOpWithText('-', 'test\ntest', '*3*4*5', pool); + assem.appendOpWithText('-', '\ntest\n', '*3*4*5', pool); + assem.appendOpWithText('-', '\ntest', '*1*4*5', pool); + } finally { + delete padutils.warnDeprecated.disabledForTestingOnly; + } + assem.endDocument(); + expect(assem.toString()).to.equal('*3*4*5|3-f*1*4*5|1-1*1*4*5-4'); + }); + it('smartOpAssembler append + op with text', async function () { const assem = Changeset.smartOpAssembler(); const pool = poolOrArray([ diff --git a/src/tests/frontend/specs/easysync-mutations.js b/src/tests/frontend/specs/easysync-mutations.js index c10d34519f2..1cf8d16843f 100644 --- a/src/tests/frontend/specs/easysync-mutations.js +++ b/src/tests/frontend/specs/easysync-mutations.js @@ -137,6 +137,54 @@ describe('easysync-mutations', function () { ['skip', 1, 1, true], ], ['banana\n', 'cabbage\n', 'duffle\n']); + runMutationTest(8, ['\n', 'fun\n', '\n'], [ + ['remove', 1, 1, '\n'], + ['skip', 3, 0, false], + ['remove', 2, 2, '\n\n'], + ['insert', 'c'], + ], ['func']); + + runMutationTest(9, ['\n', 'fun\n', '\n'], [ + ['remove', 1, 1, '\n'], + ['skip', 3, 0, false], + ['remove', 2, 2, '\n\n'], + ['insert', 'c'], + ['insert', 'a\n', 1], + ['insert', 'c'], + ], ['funca\n', 'c']); + + runMutationTest(10, ['\n', 'fun\n', '\n'], [ + ['remove', 1, 1, '\n'], + ['skip', 2, 0, false], + ['remove', 3, 2, 'n\n\n'], + ['insert', 'z'], + ], ['fuz']); + + // #2836, #5214, #3560 regressions + runMutationTest(11, ['\n'], [ + ['remove', 1, 1, '\n'], + ['insert', 'c', 0], + ], ['c']); + + runMutationTest(12, ['\n'], [ + ['remove', 1, 1, '\n'], + ['insert', 'a\n', 1], + ['insert', 'c'], + ], ['a\n', 'c']); + + runMutationTest(13, ['\n', 'fun\n', '\n'], [ + ['remove', 1, 1, '\n'], + ['skip', 4, 1, false], + ['remove', 1, 1, '\n'], + ['insert', 'c'], + ], ['fun\n', 'c']); + + runMutationTest(14, ['\n'], [ + ['remove', 1, 1, '\n'], + ['insert', 'a'], + ['insert', 'c\n', 1], + ], ['ac\n']); + it('mutatorHasMore', async function () { const lines = ['1\n', '2\n', '3\n', '4\n']; let mu;