fix bug when replacing range starting at end of previous part

src/editor/model.js

@@ -408,16 +408,13 @@ export default class EditorModel {
 
     //mostly internal, called from Range.replace
     replaceRange(startPosition, endPosition, parts) {
+        // convert end position to offset, so it is independent of how the document is split into parts
+        // which we'll change when splitting up at the start position
+        const endOffset = endPosition.asOffset(this);
         const newStartPartIndex = this._splitAt(startPosition);
-        const idxDiff = newStartPartIndex - startPosition.index;
-        // if both position are in the same part, and we split it at start position,
-        // the offset of the end position needs to be decreased by the offset of the start position
-        const removedOffset = startPosition.index === endPosition.index ? startPosition.offset : 0;
-        const adjustedEndPosition = new DocumentPosition(
-            endPosition.index + idxDiff,
-            endPosition.offset - removedOffset,
-        );
-        const newEndPartIndex = this._splitAt(adjustedEndPosition);
+        // convert it back to position once split at start
+        endPosition = endOffset.asPosition(this);
+        const newEndPartIndex = this._splitAt(endPosition);
         for (let i = newEndPartIndex - 1; i >= newStartPartIndex; --i) {
             this._removePart(i);
         }

src/editor/offset.js

@@ -0,0 +1,26 @@
+/*
+Copyright 2019 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+export default class DocumentOffset {
+    constructor(offset, atEnd) {
+        this.offset = offset;
+        this.atEnd = atEnd;
+    }
+
+    asPosition(model) {
+        return model.positionForOffset(this.offset, this.atEnd);
+    }
+}

src/editor/position.js

@@ -14,6 +14,8 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
+import DocumentOffset from "./offset";
+
 export default class DocumentPosition {
     constructor(index, offset) {
         this._index = index;
@@ -104,4 +106,18 @@ export default class DocumentPosition {
             }
         }
     }
+
+    asOffset(model) {
+        if (this.index === -1) {
+            return new DocumentOffset(0, true);
+        }
+        let offset = 0;
+        for (let i = 0; i < this.index; ++i) {
+            offset += model.parts[i].text.length;
+        }
+        offset += this.offset;
+        const lastPart = model.parts[this.index];
+        const atEnd = offset >= lastPart.text.length;
+        return new DocumentOffset(offset, atEnd);
+    }
 }

test/editor/range-test.js

@@ -52,7 +52,6 @@ describe('editor/range', function() {
         range.expandBackwardsWhile((index, offset) => model.parts[index].text[offset] !== " ");
         expect(range.text).toBe("world");
         range.replace([pc.roomPill(pillChannel)]);
-        console.log({parts: JSON.stringify(model.serializeParts())});
         expect(model.parts[0].type).toBe("plain");
         expect(model.parts[0].text).toBe("hello ");
         expect(model.parts[1].type).toBe("room-pill");
@@ -73,7 +72,6 @@ describe('editor/range', function() {
         const range = model.startRange(model.positionForOffset(14));  // after "replace"
         range.expandBackwardsWhile((index, offset) => model.parts[index].text[offset] !== " ");
         expect(range.text).toBe("replace");
-        console.log("range.text", {text: range.text});
         range.replace([pc.roomPill(pillChannel)]);
         expect(model.parts[0].type).toBe("plain");
         expect(model.parts[0].text).toBe("try to ");
@@ -83,4 +81,22 @@ describe('editor/range', function() {
         expect(model.parts[2].text).toBe(" me");
         expect(model.parts.length).toBe(3);
     });
+    // bug found while implementing tab completion
+    it('replace a part with an identical part with start position at end of previous part', function() {
+        const renderer = createRenderer();
+        const pc = createPartCreator();
+        const model = new EditorModel([
+            pc.plain("hello "),
+            pc.pillCandidate("man"),
+        ], pc, renderer);
+        const range = model.startRange(model.positionForOffset(9, true));  // before "man"
+        range.expandBackwardsWhile((index, offset) => model.parts[index].text[offset] !== " ");
+        expect(range.text).toBe("man");
+        range.replace([pc.pillCandidate(range.text)]);
+        expect(model.parts[0].type).toBe("plain");
+        expect(model.parts[0].text).toBe("hello ");
+        expect(model.parts[1].type).toBe("pill-candidate");
+        expect(model.parts[1].text).toBe("man");
+        expect(model.parts.length).toBe(2);
+    });
 });