From 97e9ea3f6290aae09f3fb799af947dce2bc975ac Mon Sep 17 00:00:00 2001 From: webzwo0i Date: Wed, 20 Oct 2021 16:29:03 +0200 Subject: [PATCH 1/6] easysync tests: add some more smartOpAssembler tests --- .../frontend/specs/easysync-assembler.js | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/tests/frontend/specs/easysync-assembler.js b/src/tests/frontend/specs/easysync-assembler.js index 80a6636d223..9464b6affb1 100644 --- a/src/tests/frontend/specs/easysync-assembler.js +++ b/src/tests/frontend/specs/easysync-assembler.js @@ -92,6 +92,40 @@ 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', + ]); + + assem.appendOpWithText('-', 'test', '*3*4*5', pool); + assem.appendOpWithText('-', 'test', '*3*4*5', pool); + assem.appendOpWithText('-', 'test', '*1*4*5', pool); + 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', + ]); + + assem.appendOpWithText('-', 'test\ntest', '*3*4*5', pool); + assem.appendOpWithText('-', '\ntest\n', '*3*4*5', pool); + assem.appendOpWithText('-', '\ntest', '*1*4*5', pool); + 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([ From fe5d43871f4d5dc1f32e85a774714440722965c1 Mon Sep 17 00:00:00 2001 From: webzwo0i Date: Sat, 30 Oct 2021 23:29:41 +0200 Subject: [PATCH 2/6] textLinesMutator: Fix removal at the end of the lines array ...in case we are in the middle of a line (curCol !== 0) and we remove everything up to the end of the line. Before this commit a string 'undefined' could make it into the splice. --- src/static/js/Changeset.js | 10 +++++--- .../frontend/specs/easysync-mutations.js | 23 +++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js index 53b3f2c8f09..7076910a692 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; } diff --git a/src/tests/frontend/specs/easysync-mutations.js b/src/tests/frontend/specs/easysync-mutations.js index c10d34519f2..dc997188e03 100644 --- a/src/tests/frontend/specs/easysync-mutations.js +++ b/src/tests/frontend/specs/easysync-mutations.js @@ -137,6 +137,29 @@ 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']); + it('mutatorHasMore', async function () { const lines = ['1\n', '2\n', '3\n', '4\n']; let mu; From 01b04a6f92f56fe12818cd07e3159184472576d2 Mon Sep 17 00:00:00 2001 From: webzwo0i Date: Sat, 30 Oct 2021 23:42:15 +0200 Subject: [PATCH 3/6] textLinesMutator: Fix insertions at the end of `lines` The new insertions are directly pushed to curSplice now instead of trying to incorporate an undefined line into the splice, which would result in an error: TypeError: Cannot read property 'substring' of undefined --- src/static/js/Changeset.js | 10 ++++++++-- .../frontend/specs/easysync-mutations.js | 19 +++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js index 7076910a692..95958c12c83 100644 --- a/src/static/js/Changeset.js +++ b/src/static/js/Changeset.js @@ -983,11 +983,17 @@ class TextLinesMutator { 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-mutations.js b/src/tests/frontend/specs/easysync-mutations.js index dc997188e03..2ecf1bb0116 100644 --- a/src/tests/frontend/specs/easysync-mutations.js +++ b/src/tests/frontend/specs/easysync-mutations.js @@ -160,6 +160,25 @@ describe('easysync-mutations', function () { ['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']); + it('mutatorHasMore', async function () { const lines = ['1\n', '2\n', '3\n', '4\n']; let mu; From f6235ce7a0f1123f76e17bb5963a631a3434d428 Mon Sep 17 00:00:00 2001 From: webzwo0i Date: Sun, 31 Oct 2021 00:17:48 +0200 Subject: [PATCH 4/6] textLinesMutator: Fix insertions with newlines at the end of a line In such cases the remaining part of the old line is directly pushed to the splice but we need to ensure it is not an empty string. Before this commit an empty string was pushed to the splice. --- src/static/js/Changeset.js | 5 +++-- src/tests/frontend/specs/easysync-mutations.js | 5 +++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js index 95958c12c83..f14433a7dd5 100644 --- a/src/static/js/Changeset.js +++ b/src/static/js/Changeset.js @@ -977,8 +977,9 @@ 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; diff --git a/src/tests/frontend/specs/easysync-mutations.js b/src/tests/frontend/specs/easysync-mutations.js index 2ecf1bb0116..cf988c3b7bc 100644 --- a/src/tests/frontend/specs/easysync-mutations.js +++ b/src/tests/frontend/specs/easysync-mutations.js @@ -179,6 +179,11 @@ describe('easysync-mutations', function () { ['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; From 6d224f55a6018952a59c5183c71a1ca1fd1a0a28 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 29 Nov 2021 22:52:39 -0500 Subject: [PATCH 5/6] fixup! easysync tests: add some more smartOpAssembler tests --- .../frontend/specs/easysync-assembler.js | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/tests/frontend/specs/easysync-assembler.js b/src/tests/frontend/specs/easysync-assembler.js index 9464b6affb1..bd66e75f1d2 100644 --- a/src/tests/frontend/specs/easysync-assembler.js +++ b/src/tests/frontend/specs/easysync-assembler.js @@ -102,9 +102,14 @@ describe('easysync-assembler', function () { 'attr5,5', ]); - assem.appendOpWithText('-', 'test', '*3*4*5', pool); - assem.appendOpWithText('-', 'test', '*3*4*5', pool); - assem.appendOpWithText('-', 'test', '*1*4*5', pool); + 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'); }); @@ -119,9 +124,14 @@ describe('easysync-assembler', function () { 'attr5,5', ]); - assem.appendOpWithText('-', 'test\ntest', '*3*4*5', pool); - assem.appendOpWithText('-', '\ntest\n', '*3*4*5', pool); - assem.appendOpWithText('-', '\ntest', '*1*4*5', pool); + 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'); }); From f8b41bacfa878016ca37ce77e95fa5676226c0e2 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 29 Nov 2021 22:39:04 -0500 Subject: [PATCH 6/6] fixup! textLinesMutator: Fix insertions with newlines at the end of a line --- src/tests/frontend/specs/easysync-mutations.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tests/frontend/specs/easysync-mutations.js b/src/tests/frontend/specs/easysync-mutations.js index cf988c3b7bc..1cf8d16843f 100644 --- a/src/tests/frontend/specs/easysync-mutations.js +++ b/src/tests/frontend/specs/easysync-mutations.js @@ -184,6 +184,7 @@ describe('easysync-mutations', function () { ['insert', 'a'], ['insert', 'c\n', 1], ], ['ac\n']); + it('mutatorHasMore', async function () { const lines = ['1\n', '2\n', '3\n', '4\n']; let mu;