Merge pull request #16872 from overleaf/mj-exclusive-ranges

[overleaf-editor-core] Make ranges exclusive

GitOrigin-RevId: 5b821d5bc1da8a6647305c268abc6185a7fa1f80

libraries/overleaf-editor-core/lib/file_data/comment.js

@@ -33,7 +33,7 @@ class Comment {
    */
   addRange(range) {
     this.ranges.push(range)
-    this.ranges.sort((a, b) => a.firstIndex - b.firstIndex)
+    this.ranges.sort((a, b) => a.start - b.start)
     this.mergeRanges()
   }
 
@@ -48,13 +48,13 @@ class Comment {
     const splittedRanges = []
 
     for (const commentRange of this.ranges) {
-      if (commentRange.firstNextIndex === cursor) {
+      if (cursor === commentRange.end) {
         // insert right after the comment
         if (extendComment) {
           commentRange.extendBy(length)
           existingRangeExtended = true
         }
-      } else if (commentRange.firstIndex === cursor) {
+      } else if (cursor === commentRange.start) {
         // insert at the start of the comment
         if (extendComment) {
           commentRange.extendBy(length)
@@ -62,10 +62,10 @@ class Comment {
         } else {
           commentRange.moveBy(length)
         }
-      } else if (commentRange.firstIndexIsAfter(cursor)) {
+      } else if (commentRange.startIsAfter(cursor)) {
         // insert before the comment
         commentRange.moveBy(length)
-      } else if (commentRange.containsIndex(cursor)) {
+      } else if (commentRange.containsCursor(cursor)) {
         // insert is inside the comment
         if (extendComment) {
           commentRange.extendBy(length)

libraries/overleaf-editor-core/lib/file_data/range.js

@@ -9,21 +9,11 @@ class Range {
     this.length = length
   }
 
-  get firstIndex() {
+  get start() {
     return this.pos
   }
 
-  /**
+  get end() {
-   * @returns {number} for an empty range, lastIndex will be -1
-   */
-  get lastIndex() {
-    if (this.length === 0) {
-      throw new Error('Range is empty')
-    }
-    return this.pos + this.length - 1
-  }
-
-  get firstNextIndex() {
     return this.pos + this.length
   }
 
@@ -32,15 +22,15 @@ class Range {
    * @returns {boolean}
    */
   startsAfter(range) {
-    return this.firstIndex >= range.firstNextIndex
+    return this.start >= range.end
   }
 
   /**
    * @param {number} pos
    * @returns {boolean}
    */
-  firstIndexIsAfter(pos) {
+  startIsAfter(pos) {
-    return this.firstIndex > pos
+    return this.start > pos
   }
 
   /**
@@ -56,27 +46,22 @@ class Range {
    * @param {Range} range
    */
   contains(range) {
-    return (
+    return this.start <= range.start && this.end >= range.end
-      this.firstIndex <= range.firstIndex && this.lastIndex >= range.lastIndex
-    )
   }
 
   /**
-   * checks if the range contains a given index
+   * checks if the range contains a cursor (i.e. is not at the ends of the range)
-   * @param {number} index
+   * @param {number} cursor
    */
-  containsIndex(index) {
+  containsCursor(cursor) {
-    return this.firstIndex <= index && this.lastIndex >= index
+    return this.start <= cursor && this.end >= cursor
   }
 
   /**
    * @param {Range} range
    */
   overlaps(range) {
-    return (
+    return this.start < range.end && this.end > range.start
-      this.firstIndex < range.firstNextIndex &&
-      this.firstNextIndex > range.firstIndex
-    )
   }
 
   /**
@@ -84,10 +69,7 @@ class Range {
    * @param {Range} range
    */
   touches(range) {
-    return (
+    return this.end === range.start || this.start === range.end
-      this.firstNextIndex === range.firstIndex ||
-      this.firstIndex === range.firstNextIndex
-    )
   }
 
   /**
@@ -107,13 +89,13 @@ class Range {
     }
 
     if (range.overlaps(this)) {
-      if (range.firstIndex < this.firstIndex) {
+      if (range.start < this.start) {
-        const intersectedLength = range.lastIndex - this.firstIndex + 1
+        const intersectedLength = range.end - this.start
         this.length -= intersectedLength
         this.pos = range.pos
         return intersectedLength
       } else {
-        const intersectedLength = this.lastIndex - range.firstIndex + 1
+        const intersectedLength = this.end - range.start
         this.length -= intersectedLength
         return intersectedLength
       }
@@ -137,10 +119,10 @@ class Range {
     if (!this.canMerge(range)) {
       throw new Error('Ranges cannot be merged')
     }
-    const newFirstIndex = Math.min(this.pos, range.pos)
+    const newPos = Math.min(this.pos, range.pos)
-    const newLastIndex = Math.max(this.lastIndex, range.lastIndex)
+    const newEnd = Math.max(this.end, range.end)
-    this.pos = newFirstIndex
+    this.pos = newPos
-    this.length = newLastIndex + 1 - newFirstIndex
+    this.length = newEnd - newPos
   }
 
   /**
@@ -160,22 +142,22 @@ class Range {
   }
 
   /**
-   * Splits a range on the index and insert a range with the length provided
+   * Splits a range on the cursor and insert a range with the length provided
-   * @param {number} index
+   * @param {number} cursor
    * @param {number} length
    * @returns {[Range, Range, Range]}
    */
-  insertAt(index, length) {
+  insertAt(cursor, length) {
-    if (!this.containsIndex(index)) {
+    if (!this.containsCursor(cursor)) {
-      throw new Error('The index must be contained in the range')
+      throw new Error('The cursor must be contained in the range')
     }
-    const rangeUpToIndex = new Range(this.pos, index - this.pos)
+    const rangeUpToCursor = new Range(this.pos, cursor - this.pos)
-    const insertedRange = new Range(index, length)
+    const insertedRange = new Range(cursor, length)
-    const rangeAfterIndex = new Range(
+    const rangeAfterCursor = new Range(
-      index + length,
+      cursor + length,
-      this.length - rangeUpToIndex.length
+      this.length - rangeUpToCursor.length
     )
-    return [rangeUpToIndex, insertedRange, rangeAfterIndex]
+    return [rangeUpToCursor, insertedRange, rangeAfterCursor]
   }
 
   toRaw() {

libraries/overleaf-editor-core/test/range.test.js

@@ -7,14 +7,14 @@ const Range = require('../lib/file_data/range')
 describe('Range', function () {
   it('should create a range', function () {
     const from5to14 = new Range(5, 10)
-    expect(from5to14.firstIndex).to.eql(5)
+    expect(from5to14.start).to.eql(5)
-    expect(from5to14.lastIndex).to.eql(14)
+    expect(from5to14.end).to.eql(15)
   })
 
   it('should create a range using fromRaw', function () {
     const from5to14 = Range.fromRaw({ pos: 5, length: 10 })
-    expect(from5to14.firstIndex).to.eql(5)
+    expect(from5to14.start).to.eql(5)
-    expect(from5to14.lastIndex).to.eql(14)
+    expect(from5to14.end).to.eql(15)
   })
 
   it('should convert to raw', function () {
@@ -172,14 +172,14 @@ describe('Range', function () {
     expect(from0to99.contains(from0to99)).to.eql(true)
   })
 
-  it('should check if range contains an index', function () {
+  it('should check if range contains a cursor', function () {
     const from5to14 = new Range(5, 10)
-    expect(from5to14.containsIndex(4)).to.eql(false)
+    expect(from5to14.containsCursor(4)).to.eql(false)
-    expect(from5to14.containsIndex(5)).to.eql(true)
+    expect(from5to14.containsCursor(5)).to.eql(true)
-    expect(from5to14.containsIndex(6)).to.eql(true)
+    expect(from5to14.containsCursor(6)).to.eql(true)
-    expect(from5to14.containsIndex(14)).to.eql(true)
+    expect(from5to14.containsCursor(14)).to.eql(true)
-    expect(from5to14.containsIndex(15)).to.eql(false)
+    expect(from5to14.containsCursor(15)).to.eql(true)
-    expect(from5to14.containsIndex(16)).to.eql(false)
+    expect(from5to14.containsCursor(16)).to.eql(false)
   })
 
   describe('subtract range from another', function () {
@@ -187,7 +187,7 @@ describe('Range', function () {
       const from1to5 = new Range(1, 6)
       const from0to1 = new Range(0, 1)
       expect(from1to5.subtract(from0to1)).to.eql(0)
-      expect(from1to5.firstIndex).to.eql(1)
+      expect(from1to5.start).to.eql(1)
       expect(from1to5.length).to.eql(6)
     })
 
@@ -195,32 +195,32 @@ describe('Range', function () {
       const from5to19 = new Range(5, 15)
       const from15to24 = new Range(15, 10)
       expect(from15to24.subtract(from5to19)).to.eql(5)
-      expect(from15to24.firstIndex).to.eql(5)
+      expect(from15to24.start).to.eql(5)
-      expect(from15to24.lastIndex).to.eql(9)
+      expect(from15to24.end).to.eql(10)
     })
 
     it('should subtract from the right', function () {
       const from10to24 = new Range(10, 15)
       const from5to19 = new Range(5, 15)
       expect(from5to19.subtract(from10to24)).to.eql(10)
-      expect(from5to19.firstIndex).to.eql(5)
+      expect(from5to19.start).to.eql(5)
-      expect(from5to19.lastIndex).to.eql(9)
+      expect(from5to19.end).to.eql(10)
     })
 
     it('should subtract from the middle', function () {
       const from5to19 = new Range(5, 15)
       const from10to14 = new Range(10, 5)
       expect(from5to19.subtract(from10to14)).to.eql(5)
-      expect(from5to19.firstIndex).to.eql(5)
+      expect(from5to19.start).to.eql(5)
-      expect(from5to19.lastIndex).to.eql(14)
+      expect(from5to19.end).to.eql(15)
     })
 
     it('should delete entire range', function () {
       const from0to99 = new Range(0, 100)
       const from5to19 = new Range(5, 15)
       expect(from5to19.subtract(from0to99)).to.eql(15)
-      expect(from5to19.firstIndex).to.eql(5)
+      expect(from5to19.start).to.eql(5)
-      expect(() => from5to19.lastIndex).to.throw()
+      expect(from5to19.end).to.eql(5)
       expect(from5to19.length).to.eql(0)
     })
 
@@ -229,8 +229,8 @@ describe('Range', function () {
       const from20to29 = new Range(20, 10)
       expect(from5to14.subtract(from20to29)).to.eql(0)
       expect(from20to29.subtract(from5to14)).to.eql(0)
-      expect(from5to14.firstIndex).to.eql(5)
+      expect(from5to14.start).to.eql(5)
-      expect(from5to14.lastIndex).to.eql(14)
+      expect(from5to14.end).to.eql(15)
     })
   })
 
@@ -240,8 +240,8 @@ describe('Range', function () {
       const from10to19 = new Range(10, 10)
       expect(from5to14.canMerge(from10to19)).to.eql(true)
       from5to14.merge(from10to19)
-      expect(from5to14.firstIndex).to.eql(5)
+      expect(from5to14.start).to.eql(5)
-      expect(from5to14.lastIndex).to.eql(19)
+      expect(from5to14.end).to.eql(20)
     })
 
     it('should merge ranges overlaping at the start', function () {
@@ -249,8 +249,8 @@ describe('Range', function () {
       const from0to9 = new Range(0, 10)
       expect(from5to14.canMerge(from0to9)).to.eql(true)
       from5to14.merge(from0to9)
-      expect(from5to14.firstIndex).to.eql(0)
+      expect(from5to14.start).to.eql(0)
-      expect(from5to14.lastIndex).to.eql(14)
+      expect(from5to14.end).to.eql(15)
     })
 
     it('should merge ranges if one is covered by another', function () {
@@ -266,8 +266,8 @@ describe('Range', function () {
       const from0to19 = new Range(0, 20)
       expect(from0to19.canMerge(from5to14)).to.eql(true)
       from0to19.merge(from5to14)
-      expect(from0to19.firstIndex).to.eql(0)
+      expect(from0to19.start).to.eql(0)
-      expect(from0to19.lastIndex).to.eql(19)
+      expect(from0to19.end).to.eql(20)
     })
 
     it('should not merge ranges if they do not overlap', function () {
@@ -319,27 +319,27 @@ describe('Range', function () {
 
   it('should check if range starts after a position', function () {
     const from5to14 = new Range(5, 10)
-    expect(from5to14.firstIndexIsAfter(3)).to.be.true
+    expect(from5to14.startIsAfter(3)).to.be.true
-    expect(from5to14.firstIndexIsAfter(4)).to.be.true
+    expect(from5to14.startIsAfter(4)).to.be.true
-    expect(from5to14.firstIndexIsAfter(5)).to.be.false
+    expect(from5to14.startIsAfter(5)).to.be.false
-    expect(from5to14.firstIndexIsAfter(6)).to.be.false
+    expect(from5to14.startIsAfter(6)).to.be.false
-    expect(from5to14.firstIndexIsAfter(15)).to.be.false
+    expect(from5to14.startIsAfter(15)).to.be.false
-    expect(from5to14.firstIndexIsAfter(16)).to.be.false
+    expect(from5to14.startIsAfter(16)).to.be.false
   })
 
   it('should extend the range', function () {
     const from5to14 = new Range(5, 10)
     from5to14.extendBy(3)
     expect(from5to14.length).to.eql(13)
-    expect(from5to14.firstIndex).to.eql(5)
+    expect(from5to14.start).to.eql(5)
-    expect(from5to14.lastIndex).to.eql(17)
+    expect(from5to14.end).to.eql(18)
   })
 
   it('should move the range', function () {
     const from5to14 = new Range(5, 10)
     from5to14.moveBy(3)
     expect(from5to14.length).to.eql(10)
-    expect(from5to14.firstIndex).to.eql(8)
+    expect(from5to14.start).to.eql(8)
-    expect(from5to14.lastIndex).to.eql(17)
+    expect(from5to14.end).to.eql(18)
   })
 })