From 927a3e7703ab44a20b5c9d1b5bda9f074e279c86 Mon Sep 17 00:00:00 2001 From: webzwo0i Date: Sat, 23 Jan 2016 12:36:07 +0100 Subject: [PATCH 1/3] tests: Add regression tests for #2836 They test if it is possible to delete lines from the array that is mutated by a changeset, and after no line is left insert some characters (with or without new lines). --- src/tests/frontend/specs/easysync.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/tests/frontend/specs/easysync.js b/src/tests/frontend/specs/easysync.js index 5c4c47ae43d..5ccb408c17f 100644 --- a/src/tests/frontend/specs/easysync.js +++ b/src/tests/frontend/specs/easysync.js @@ -178,6 +178,17 @@ describe('easysync', function () { ['skip', 1, 1, true], ], ['banana\n', 'cabbage\n', 'duffle\n']); + // #2836 regressions + runMutationTest(8, ['\n'], [ + ['remove', 1, 1, '\n'], + ['insert', 'c', 0], + ], ['c']); + runMutationTest(9, ['\n'], [ + ['remove', 1, 1, '\n'], + ['insert', 'a'], + ['insert', 'c\n', 1], + ], ['ac\n']); + const poolOrArray = (attribs) => { if (attribs.getAttrib) { return attribs; // it's already an attrib pool From 77e4cde91edb7ca8afab055e5b26b71c193b28ab Mon Sep 17 00:00:00 2001 From: webzwo0i Date: Sat, 23 Jan 2016 12:52:30 +0100 Subject: [PATCH 2/3] Add another regression test that deletes all lines and inserts two strings where the first ends in a newline. --- src/tests/frontend/specs/easysync.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/tests/frontend/specs/easysync.js b/src/tests/frontend/specs/easysync.js index 5ccb408c17f..13eb2ba2c42 100644 --- a/src/tests/frontend/specs/easysync.js +++ b/src/tests/frontend/specs/easysync.js @@ -188,6 +188,11 @@ describe('easysync', function () { ['insert', 'a'], ['insert', 'c\n', 1], ], ['ac\n']); + runMutationTest(10, ['\n'], [ + ['remove', 1, 1, '\n'], + ['insert', 'a\n', 1], + ['insert', 'c'], + ], ['a\n', 'c']); // TODO find out if c must have a newline because of unknown constraints const poolOrArray = (attribs) => { if (attribs.getAttrib) { From a8f09ab01e9b2d686cf3e5ab59ef2e7aaeec36f1 Mon Sep 17 00:00:00 2001 From: webzwo0i Date: Tue, 26 Jan 2016 20:49:29 +0100 Subject: [PATCH 3/3] - fix handling of insertions when there is nothing left in the lines array. - add some more test cases --- src/static/js/Changeset.js | 28 +++++++++++++++++++++++----- src/tests/frontend/specs/easysync.js | 20 ++++++++++++++++---- 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js index 812a91154da..6b6157f2821 100644 --- a/src/static/js/Changeset.js +++ b/src/static/js/Changeset.js @@ -711,9 +711,14 @@ exports.textLinesMutator = (lines) => { 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; + const line = linesGet(curSplice[0] + curSplice[1]); + // if no line follows the splice + if (!line) { + curSplice[sline] = curSplice[sline].substring(0, curCol); + } else { + curSplice[sline] = curSplice[sline].substring(0, curCol) + line; + curSplice[1] += 1; + } } } else { removed = nextKLinesText(L); @@ -775,14 +780,27 @@ exports.textLinesMutator = (lines) => { 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)); + // if nothing is left we don't push an empty string + if (theLine.substring(lineCol)) { + 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; } + } else if (lines_get(curSplice[0] + curSplice[1]) === undefined) { + // find out if there is a line in splice that is not finished processing + if (isCurLineInSplice()) { // if yes, we can add our text to it + const sline = curSplice.length - 1; + curSplice[sline] = + curSplice[sline].substring(0, curCol) + text + curSplice[sline].substring(curCol); + curCol += text.length; + } else { // if no, we need to add the text in a new line + Array.prototype.push.apply(curSplice, [text]); + curCol += text.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(); diff --git a/src/tests/frontend/specs/easysync.js b/src/tests/frontend/specs/easysync.js index 13eb2ba2c42..e361e12fd4e 100644 --- a/src/tests/frontend/specs/easysync.js +++ b/src/tests/frontend/specs/easysync.js @@ -179,16 +179,28 @@ describe('easysync', function () { ], ['banana\n', 'cabbage\n', 'duffle\n']); // #2836 regressions - runMutationTest(8, ['\n'], [ + runMutationTest(8, ['\n', 'foo\n', '\n'], [ + ['remove', 1, 1, '\n'], + ['skip', 4, 1, false], + ['remove', 1, 1, '\n'], + ['insert', 'c'], + ], ['foo\n', 'c']); + runMutationTest(9, ['\n', 'foo\n', '\n'], [ + ['remove', 1, 1, '\n'], + ['skip', 3, 0, false], + ['remove', 2, 2, '\n\n'], + ['insert', 'c'], + ], ['fooc']); + runMutationTest(10, ['\n'], [ ['remove', 1, 1, '\n'], ['insert', 'c', 0], - ], ['c']); - runMutationTest(9, ['\n'], [ + ], ['c']); // TODO find out if c must have a newline because of unknown constraints + runMutationTest(11, ['\n'], [ ['remove', 1, 1, '\n'], ['insert', 'a'], ['insert', 'c\n', 1], ], ['ac\n']); - runMutationTest(10, ['\n'], [ + runMutationTest(12, ['\n'], [ ['remove', 1, 1, '\n'], ['insert', 'a\n', 1], ['insert', 'c'],