瀏覽代碼

Added applyInsert/applyDelete methods to comments (#16572)

* Add CommentList to StringFileData

* added more types

* use toRaw

* using Map rather than array for comments

* using Range class

* Comment with ranges:Range[]

* Revert "Comment with ranges:Range[]"

This reverts commit 0783b1837562600637db03cc70c620129061c797.

* Comment with ranges:Range[]

* remove isDeleted

* commentList.toRaw()

* using toRaw

* commentId to id

* Revert "using toRaw"

This reverts commit 0c04ca5836f3befd5ec027bad5bf722e8b27f36c.

* fix merge

* make comment map internal to CommentList

* remove unused type

* fix parameter name in StringFileData

* import types more consistently

* more consistent type def

* Added moveOnInsert/moveOnDelete methods to comments

* use range helper methods

* mergeRanges function

* rename isAfter to startsAfter

* added @ts-checks

* using comment.isEmpty

* rename overlaps to covers

* remove ops in applyDelete

* mege, substract, isInRange

* ranges fixes, added tests

* rename to includes

* using pos,length in applyInsert

* simplify

* extendComment option

* check comment edges

* added inclusive option

* more specific touches()

* refactor mergeRanges()

* comment.addRange()

* remove inclusive option

* refactor using more helper methods

* fix typo

* inserting a comment between ranges tests

* test description fixes

* support only range

* more edge case testing

* added more range tests

* renamed to containsExcludingEdges

* endsAt check edge

* using firstIndex, lastIndex

* contains, containsIndex

* raturn -1 for lastIndex for empty ranges

* rename ranges in tests

* indexStartsAfter

* sort comment ranges

* rename to indexIsAfter, fix Range type

* add range if expandComment is true

* split the range when expandComment is false

* added moveBy and extendBy

* added more tests

* added comments

* prettier

* small fixes/typos

* rename indexIsAfter

* merge when deleting with overlaps

* added overlap test

* same range touches test

* test with overlapping comments

* delete duplicate test

* throw on empty range

GitOrigin-RevId: 1252e3d01f2f5dc1d00ce681c579506038e4d121
Domagoj Kriskovic 2 年之前
父節點
當前提交
b87dad77d9

+ 122 - 1
libraries/overleaf-editor-core/lib/file_data/comment.js

@@ -1,3 +1,4 @@
+// @ts-check
 const Range = require('./range')
 
 /**
@@ -5,13 +6,113 @@ const Range = require('./range')
  */
 
 class Comment {
+  /**
+   * @type {Range[]}
+   */
+  ranges = []
+
+  /**
+   * @type {boolean}
+   */
+  resolved = false
+
   /**
    * @param {Range[]} ranges
    * @param {boolean} [resolved]
    */
   constructor(ranges, resolved = false) {
-    this.ranges = ranges
     this.resolved = resolved
+    for (const range of ranges) {
+      this.addRange(range)
+    }
+  }
+
+  /**
+   *
+   * @param {Range} range
+   */
+  addRange(range) {
+    this.ranges.push(range)
+    this.ranges.sort((a, b) => a.firstIndex - b.firstIndex)
+    this.mergeRanges()
+  }
+
+  /**
+   *
+   * @param {number} cursor
+   * @param {number} length
+   * @param {boolean} [extendComment]
+   */
+  applyInsert(cursor, length, extendComment = false) {
+    let existingRangeExtended = false
+    const splittedRanges = []
+
+    for (const commentRange of this.ranges) {
+      if (commentRange.firstNextIndex === cursor) {
+        // insert right after the comment
+        if (extendComment) {
+          commentRange.extendBy(length)
+          existingRangeExtended = true
+        }
+      } else if (commentRange.firstIndex === cursor) {
+        // insert at the start of the comment
+        if (extendComment) {
+          commentRange.extendBy(length)
+          existingRangeExtended = true
+        } else {
+          commentRange.moveBy(length)
+        }
+      } else if (commentRange.firstIndexIsAfter(cursor)) {
+        // insert before the comment
+        commentRange.moveBy(length)
+      } else if (commentRange.containsIndex(cursor)) {
+        // insert is inside the comment
+        if (extendComment) {
+          commentRange.extendBy(length)
+          existingRangeExtended = true
+        } else {
+          const [rangeUpToCursor, , rangeAfterCursor] = commentRange.insertAt(
+            cursor,
+            length
+          )
+
+          // use current commentRange for the part before the cursor
+          commentRange.length = rangeUpToCursor.length
+          // add the part after the cursor as a new range
+          splittedRanges.push(rangeAfterCursor)
+        }
+      }
+    }
+
+    // add the splitted ranges
+    for (const range of splittedRanges) {
+      this.addRange(range)
+    }
+
+    // if the insert is not inside any range, add a new range
+    if (extendComment && !existingRangeExtended) {
+      this.addRange(new Range(cursor, length))
+    }
+  }
+
+  /**
+   *
+   * @param {Range} deletedRange
+   */
+  applyDelete(deletedRange) {
+    for (const commentRange of this.ranges) {
+      if (commentRange.overlaps(deletedRange)) {
+        commentRange.subtract(deletedRange)
+      } else if (commentRange.startsAfter(deletedRange)) {
+        commentRange.pos -= deletedRange.length
+      }
+    }
+
+    this.mergeRanges()
+  }
+
+  isEmpty() {
+    return this.ranges.length === 0
   }
 
   toRaw() {
@@ -21,6 +122,26 @@ class Comment {
     }
   }
 
+  mergeRanges() {
+    /** @type {Range[]} */
+    const mergedRanges = []
+
+    for (const range of this.ranges) {
+      if (range.isEmpty()) {
+        continue
+      }
+      const lastRange = mergedRanges[mergedRanges.length - 1]
+
+      if (lastRange?.canMerge(range)) {
+        lastRange.merge(range)
+      } else {
+        mergedRanges.push(range)
+      }
+    }
+
+    this.ranges = mergedRanges
+  }
+
   /**
    * @param {CommentRawData} rawComment
    * @returns {Comment}

+ 33 - 4
libraries/overleaf-editor-core/lib/file_data/comment_list.js

@@ -1,7 +1,9 @@
+// @ts-check
 const Comment = require('./comment')
 
 /**
  * @typedef {import("../types").CommentRawData} CommentRawData
+ * @typedef {import("./range")} Range
  */
 
 class CommentList {
@@ -37,10 +39,11 @@ class CommentList {
    * @param {Comment} newComment
    */
   add(id, newComment) {
-    const existing = this.getComment(id)
-    if (existing) {
-      // todo: merge/split ranges
-      existing.ranges = newComment.ranges
+    const existingComment = this.getComment(id)
+    if (existingComment) {
+      for (const range of newComment.ranges) {
+        existingComment.addRange(range)
+      }
     } else {
       this.comments.set(id, newComment)
     }
@@ -63,6 +66,32 @@ class CommentList {
     }
     return new CommentList(comments)
   }
+
+  /**
+   * @param {Range} range
+   * @param {{ commentIds: string[] }} opts
+   */
+  applyInsert(range, opts = { commentIds: [] }) {
+    for (const [commentId, comment] of this.comments) {
+      comment.applyInsert(
+        range.pos,
+        range.length,
+        opts.commentIds.includes(commentId)
+      )
+    }
+  }
+
+  /**
+   * @param {Range} range
+   */
+  applyDelete(range) {
+    for (const [commentId, comment] of this.comments) {
+      comment.applyDelete(range)
+      if (comment.isEmpty()) {
+        this.delete(commentId)
+      }
+    }
+  }
 }
 
 module.exports = CommentList

+ 170 - 0
libraries/overleaf-editor-core/lib/file_data/range.js

@@ -1,3 +1,4 @@
+// @ts-check
 class Range {
   /**
    * @param {number} pos
@@ -8,6 +9,175 @@ class Range {
     this.length = length
   }
 
+  get firstIndex() {
+    return this.pos
+  }
+
+  /**
+   * @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
+  }
+
+  /**
+   * @param {Range} range
+   * @returns {boolean}
+   */
+  startsAfter(range) {
+    return this.firstIndex >= range.firstNextIndex
+  }
+
+  /**
+   * @param {number} pos
+   * @returns {boolean}
+   */
+  firstIndexIsAfter(pos) {
+    return this.firstIndex > pos
+  }
+
+  /**
+   *
+   * @returns {boolean}
+   */
+  isEmpty() {
+    return this.length === 0
+  }
+
+  /**
+   * checks if the range contains a given range
+   * @param {Range} range
+   */
+  contains(range) {
+    return (
+      this.firstIndex <= range.firstIndex && this.lastIndex >= range.lastIndex
+    )
+  }
+
+  /**
+   * checks if the range contains a given index
+   * @param {number} index
+   */
+  containsIndex(index) {
+    return this.firstIndex <= index && this.lastIndex >= index
+  }
+
+  /**
+   * @param {Range} range
+   */
+  overlaps(range) {
+    return (
+      this.firstIndex < range.firstNextIndex &&
+      this.firstNextIndex > range.firstIndex
+    )
+  }
+
+  /**
+   * checks if the range touches a given range
+   * @param {Range} range
+   */
+  touches(range) {
+    return (
+      this.firstNextIndex === range.firstIndex ||
+      this.firstIndex === range.firstNextIndex
+    )
+  }
+
+  /**
+   * @param {Range} range
+   * @returns {number} the length of the intersected range
+   */
+  subtract(range) {
+    if (this.contains(range)) {
+      this.length -= range.length
+      return range.length
+    }
+
+    if (range.contains(this)) {
+      const intersectedLength = this.length
+      this.length = 0
+      return intersectedLength
+    }
+
+    if (range.overlaps(this)) {
+      if (range.firstIndex < this.firstIndex) {
+        const intersectedLength = range.lastIndex - this.firstIndex + 1
+        this.length -= intersectedLength
+        this.pos = range.pos
+        return intersectedLength
+      } else {
+        const intersectedLength = this.lastIndex - range.firstIndex + 1
+        this.length -= intersectedLength
+        return intersectedLength
+      }
+    }
+
+    return 0
+  }
+
+  /**
+   * @param {Range} range
+   * @returns {boolean}
+   */
+  canMerge(range) {
+    return this.overlaps(range) || this.touches(range)
+  }
+
+  /**
+   * @param {Range} range
+   */
+  merge(range) {
+    if (!this.canMerge(range)) {
+      throw new Error('Ranges cannot be merged')
+    }
+    const newFirstIndex = Math.min(this.pos, range.pos)
+    const newLastIndex = Math.max(this.lastIndex, range.lastIndex)
+    this.pos = newFirstIndex
+    this.length = newLastIndex + 1 - newFirstIndex
+  }
+
+  /**
+   * Moves the range by a given number
+   * @param {number} length
+   */
+  moveBy(length) {
+    this.pos += length
+  }
+
+  /**
+   * Extends the range by a given number
+   * @param {number} extensionLength
+   */
+  extendBy(extensionLength) {
+    this.length += extensionLength
+  }
+
+  /**
+   * Splits a range on the index and insert a range with the length provided
+   * @param {number} index
+   * @param {number} length
+   * @returns {[Range, Range, Range]}
+   */
+  insertAt(index, length) {
+    if (!this.containsIndex(index)) {
+      throw new Error('The index must be contained in the range')
+    }
+    const rangeUpToIndex = new Range(this.pos, index - this.pos)
+    const insertedRange = new Range(index, length)
+    const rangeAfterIndex = new Range(
+      index + length,
+      this.length - rangeUpToIndex.length
+    )
+    return [rangeUpToIndex, insertedRange, rangeAfterIndex]
+  }
+
   toRaw() {
     return {
       pos: this.pos,

+ 123 - 0
libraries/overleaf-editor-core/test/comment.test.js

@@ -0,0 +1,123 @@
+// @ts-check
+'use strict'
+
+const { expect } = require('chai')
+const Comment = require('../lib/file_data/comment')
+const Range = require('../lib/file_data/range')
+
+describe('Comment', function () {
+  it('should move ranges to the right of insert', function () {
+    const comment = new Comment([new Range(5, 10)])
+    comment.applyInsert(3, 5, false)
+    expect(comment.ranges).to.eql([new Range(10, 10)])
+  })
+
+  it('should expand the range after insert inside it', function () {
+    const comment = new Comment([new Range(5, 10)])
+    comment.applyInsert(4, 1) // inserting 1 char before the range
+    expect(comment.ranges).to.eql([new Range(6, 10)])
+    comment.applyInsert(6, 1) // inserting 1 char at the edge (without expandCommand = false)
+    expect(comment.ranges).to.eql([new Range(7, 10)])
+    comment.applyInsert(7, 1, true) // inserting 1 char at the edge (with expandCommand = true)
+    expect(comment.ranges).to.eql([new Range(7, 11)])
+    comment.applyInsert(8, 1, true) // inserting 1 char inside the range
+    expect(comment.ranges).to.eql([new Range(7, 12)])
+  })
+
+  it('should split the range if inside another and expandComment is false', function () {
+    const comment1 = new Comment([new Range(5, 10)])
+    comment1.applyInsert(6, 10, false)
+    expect(comment1.ranges).to.eql([new Range(5, 1), new Range(16, 9)])
+
+    // insert at the end of the range
+    const comment2 = new Comment([new Range(5, 10)])
+    comment2.applyInsert(14, 10, false)
+    expect(comment2.ranges).to.eql([new Range(5, 9), new Range(24, 1)])
+  })
+
+  it('should move the range if insert is at range start and expandComment is false', function () {
+    const comment = new Comment([new Range(5, 10)])
+    comment.applyInsert(5, 10, false)
+    expect(comment.ranges).to.eql([new Range(15, 10)])
+  })
+
+  it('should ignore the range if insert is at range end and expandComment is false', function () {
+    const comment = new Comment([new Range(5, 10)])
+    comment.applyInsert(15, 10, false)
+    expect(comment.ranges).to.eql([new Range(5, 10)])
+  })
+
+  it('should expand the range after inserting on the edge of it if expandComment is true', function () {
+    const comment = new Comment([new Range(5, 10)])
+    comment.applyInsert(15, 10, true)
+    expect(comment.ranges).to.eql([new Range(5, 20)])
+  })
+
+  it('should add a new range if expandComment is true and not inside any range', function () {
+    const commentNoRanges = new Comment([])
+    commentNoRanges.applyInsert(5, 10, true)
+    expect(commentNoRanges.ranges).to.eql([new Range(5, 10)])
+
+    const commentWithRanges = new Comment([new Range(5, 10)])
+    commentWithRanges.applyInsert(50, 10, true)
+    expect(commentWithRanges.ranges).to.eql([
+      new Range(5, 10),
+      new Range(50, 10),
+    ])
+  })
+
+  it('should move ranges if delete is before it', function () {
+    const from5to14 = new Comment([new Range(5, 10)])
+    const from3to7 = new Range(3, 5)
+
+    from5to14.applyDelete(from3to7)
+    const from3to9 = new Range(3, 7)
+    expect(from5to14.ranges).to.eql([from3to9])
+  })
+
+  it('should merge ranges after delete', function () {
+    const comment = new Comment([new Range(5, 10), new Range(20, 10)])
+    comment.applyDelete(new Range(7, 18))
+    expect(comment.ranges).to.eql([new Range(5, 7)])
+  })
+
+  it('should merge overlapping ranges', function () {
+    const comment = new Comment([
+      new Range(5, 10),
+      new Range(15, 20),
+      new Range(50, 10),
+    ])
+    comment.mergeRanges()
+    expect(comment.ranges).to.eql([new Range(5, 30), new Range(50, 10)])
+  })
+
+  it('should merge unsorted ranges', function () {
+    const comment = new Comment([
+      new Range(15, 20),
+      new Range(50, 10),
+      new Range(5, 10),
+    ])
+    comment.mergeRanges()
+    expect(comment.ranges).to.eql([new Range(5, 30), new Range(50, 10)])
+  })
+
+  it('should ignore overlapped range', function () {
+    const comment = new Comment([
+      new Range(5, 10),
+      new Range(10, 5),
+      new Range(50, 10),
+    ])
+    comment.mergeRanges()
+    expect(comment.ranges).to.eql([new Range(5, 10), new Range(50, 10)])
+  })
+
+  it('should join touching ranges', function () {
+    const comment = new Comment([
+      new Range(5, 10),
+      new Range(15, 5),
+      new Range(50, 10),
+    ])
+    comment.mergeRanges()
+    expect(comment.ranges).to.eql([new Range(5, 15), new Range(50, 10)])
+  })
+})

+ 311 - 2
libraries/overleaf-editor-core/test/comments_list.test.js

@@ -1,3 +1,4 @@
+// @ts-check
 'use strict'
 
 const { expect } = require('chai')
@@ -73,7 +74,7 @@ describe('commentList', function () {
     ])
   })
 
-  it('should override existing if a new comment has the same id', function () {
+  it('should add range to existing if a new comment has the same id', function () {
     const commentList = new CommentList(
       new Map([
         ['comm1', new Comment([new Range(5, 10)])],
@@ -87,7 +88,10 @@ describe('commentList', function () {
       { id: 'comm1', ranges: [{ pos: 5, length: 10 }], resolved: false },
       {
         id: 'comm2',
-        ranges: [{ pos: 40, length: 10 }],
+        ranges: [
+          { pos: 20, length: 5 },
+          { pos: 40, length: 10 },
+        ],
         resolved: false,
       },
       {
@@ -135,4 +139,309 @@ describe('commentList', function () {
       },
     ])
   })
+
+  describe('inserting a comment between ranges', function () {
+    it('should expand comment on the left', function () {
+      const commentList = CommentList.fromRaw([
+        {
+          id: 'comm1',
+          ranges: [{ pos: 5, length: 10 }],
+        },
+        {
+          id: 'comm2',
+          ranges: [{ pos: 15, length: 10 }],
+        },
+      ])
+
+      commentList.applyInsert(new Range(15, 5), { commentIds: ['comm1'] })
+      expect(commentList.getComments()).to.eql([
+        { id: 'comm1', ranges: [{ pos: 5, length: 15 }], resolved: false },
+        { id: 'comm2', ranges: [{ pos: 20, length: 10 }], resolved: false },
+      ])
+    })
+
+    it('should expand comment on the right', function () {
+      const commentList = CommentList.fromRaw([
+        {
+          id: 'comm1',
+          ranges: [{ pos: 5, length: 10 }],
+        },
+        {
+          id: 'comm2',
+          ranges: [{ pos: 15, length: 10 }],
+        },
+      ])
+
+      commentList.applyInsert(new Range(15, 5), { commentIds: ['comm2'] })
+      expect(commentList.getComments()).to.eql([
+        { id: 'comm1', ranges: [{ pos: 5, length: 10 }], resolved: false },
+        { id: 'comm2', ranges: [{ pos: 15, length: 15 }], resolved: false },
+      ])
+    })
+  })
+
+  it('should delete a text overlapping two comments', function () {
+    const commentList = CommentList.fromRaw([
+      {
+        id: 'comm1',
+        ranges: [{ pos: 5, length: 10 }], // 5-14
+      },
+      {
+        id: 'comm2',
+        ranges: [{ pos: 15, length: 10 }], // 15-24
+      },
+    ])
+
+    commentList.applyDelete(new Range(10, 10)) // 10-19
+    expect(commentList.getComments()).to.eql([
+      { id: 'comm1', ranges: [{ pos: 5, length: 5 }], resolved: false },
+      { id: 'comm2', ranges: [{ pos: 10, length: 5 }], resolved: false },
+    ])
+  })
+
+  describe('move ranges after insert/delete operations', function () {
+    it('expands comments inside inserted text', function () {
+      const commentList = CommentList.fromRaw([
+        {
+          id: 'comm1',
+          ranges: [{ pos: 5, length: 10 }],
+        },
+        {
+          id: 'comm2',
+          ranges: [{ pos: 20, length: 5 }],
+        },
+        {
+          id: 'comm3',
+          ranges: [{ pos: 30, length: 15 }],
+        },
+      ])
+
+      commentList.applyInsert(new Range(7, 5), { commentIds: ['comm1'] })
+      expect(commentList.getComments()).to.eql([
+        { id: 'comm1', ranges: [{ pos: 5, length: 15 }], resolved: false },
+        { id: 'comm2', ranges: [{ pos: 25, length: 5 }], resolved: false },
+        { id: 'comm3', ranges: [{ pos: 35, length: 15 }], resolved: false },
+      ])
+    })
+
+    it('should insert an overlapping comment without overlapped comment id', function () {
+      const commentList = CommentList.fromRaw([
+        {
+          id: 'comm1',
+          ranges: [{ pos: 5, length: 10 }],
+        },
+        {
+          id: 'comm2',
+          ranges: [{ pos: 20, length: 5 }],
+        },
+        {
+          id: 'comm3',
+          ranges: [{ pos: 30, length: 15 }],
+        },
+      ])
+
+      commentList.applyInsert(new Range(7, 5), { commentIds: ['comm2'] })
+      expect(commentList.getComments()).to.eql([
+        {
+          id: 'comm1',
+          ranges: [
+            { pos: 5, length: 2 },
+            { pos: 12, length: 8 },
+          ],
+          resolved: false,
+        },
+        {
+          id: 'comm2',
+          ranges: [
+            { pos: 7, length: 5 },
+            { pos: 25, length: 5 },
+          ],
+          resolved: false,
+        },
+        { id: 'comm3', ranges: [{ pos: 35, length: 15 }], resolved: false },
+      ])
+    })
+
+    it('should insert an overlapping comment with overlapped comment id', function () {
+      const commentList = CommentList.fromRaw([
+        {
+          id: 'comm1',
+          ranges: [{ pos: 5, length: 15 }],
+        },
+        {
+          id: 'comm2',
+          ranges: [{ pos: 20, length: 5 }],
+        },
+        {
+          id: 'comm3',
+          ranges: [{ pos: 30, length: 15 }],
+        },
+      ])
+
+      commentList.applyInsert(new Range(7, 5), {
+        commentIds: ['comm1', 'comm2'],
+      })
+      expect(commentList.getComments()).to.eql([
+        {
+          id: 'comm1',
+          ranges: [{ pos: 5, length: 20 }],
+          resolved: false,
+        },
+        {
+          id: 'comm2',
+          ranges: [
+            { pos: 7, length: 5 },
+            { pos: 25, length: 5 },
+          ],
+          resolved: false,
+        },
+        { id: 'comm3', ranges: [{ pos: 35, length: 15 }], resolved: false },
+      ])
+    })
+
+    it('moves comments after inserted text', function () {
+      const commentList = CommentList.fromRaw([
+        {
+          id: 'comm1',
+          ranges: [{ pos: 5, length: 10 }],
+        },
+        {
+          id: 'comm2',
+          ranges: [{ pos: 20, length: 5 }],
+        },
+        {
+          id: 'comm3',
+          ranges: [{ pos: 30, length: 15 }],
+        },
+      ])
+
+      commentList.applyInsert(new Range(16, 5))
+      expect(commentList.getComments()).to.eql([
+        { id: 'comm1', ranges: [{ pos: 5, length: 10 }], resolved: false },
+        { id: 'comm2', ranges: [{ pos: 25, length: 5 }], resolved: false },
+        { id: 'comm3', ranges: [{ pos: 35, length: 15 }], resolved: false },
+      ])
+    })
+
+    it('does not affect comments outside of inserted text', function () {
+      const commentList = CommentList.fromRaw([
+        {
+          id: 'comm1',
+          ranges: [{ pos: 5, length: 10 }],
+        },
+        {
+          id: 'comm2',
+          ranges: [{ pos: 20, length: 5 }],
+        },
+        {
+          id: 'comm3',
+          ranges: [{ pos: 30, length: 15 }],
+        },
+      ])
+
+      commentList.applyInsert(new Range(50, 5))
+      expect(commentList.getComments()).to.eql([
+        { id: 'comm1', ranges: [{ pos: 5, length: 10 }], resolved: false },
+        { id: 'comm2', ranges: [{ pos: 20, length: 5 }], resolved: false },
+        { id: 'comm3', ranges: [{ pos: 30, length: 15 }], resolved: false },
+      ])
+    })
+
+    it('should move comments if delete happened before it', function () {
+      const commentList = CommentList.fromRaw([
+        {
+          id: 'comm1',
+          ranges: [{ pos: 5, length: 10 }],
+        },
+        {
+          id: 'comm2',
+          ranges: [{ pos: 20, length: 5 }],
+        },
+        {
+          id: 'comm3',
+          ranges: [{ pos: 30, length: 15 }],
+        },
+      ])
+
+      commentList.applyDelete(new Range(0, 4))
+      expect(commentList.getComments()).to.eql([
+        { id: 'comm1', ranges: [{ pos: 1, length: 10 }], resolved: false },
+        { id: 'comm2', ranges: [{ pos: 16, length: 5 }], resolved: false },
+        { id: 'comm3', ranges: [{ pos: 26, length: 15 }], resolved: false },
+      ])
+    })
+
+    describe('should remove part of a comment on delete overlapping', function () {
+      it('should delete intersection from the left', function () {
+        const commentList = CommentList.fromRaw([
+          {
+            id: 'comm1',
+            ranges: [{ pos: 5, length: 10 }],
+          },
+        ])
+
+        commentList.applyDelete(new Range(0, 6))
+        expect(commentList.getComments()).to.eql([
+          { id: 'comm1', ranges: [{ pos: 0, length: 9 }], resolved: false },
+        ])
+      })
+
+      it('should delete intersection from the right', function () {
+        const commentList = CommentList.fromRaw([
+          {
+            id: 'comm1',
+            ranges: [{ pos: 5, length: 10 }],
+          },
+        ])
+        commentList.applyDelete(new Range(7, 10))
+        expect(commentList.getComments()).to.eql([
+          { id: 'comm1', ranges: [{ pos: 5, length: 2 }], resolved: false },
+        ])
+      })
+
+      it('should delete intersection in the middle', function () {
+        const commentList = CommentList.fromRaw([
+          {
+            id: 'comm1',
+            ranges: [{ pos: 5, length: 10 }],
+          },
+        ])
+        commentList.applyDelete(new Range(6, 2))
+        expect(commentList.getComments()).to.eql([
+          { id: 'comm1', ranges: [{ pos: 5, length: 8 }], resolved: false },
+        ])
+      })
+    })
+
+    it('should delete entire comment', function () {
+      const commentList = CommentList.fromRaw([
+        {
+          id: 'comm1',
+          ranges: [{ pos: 5, length: 10 }],
+        },
+        {
+          id: 'comm2',
+          ranges: [{ pos: 20, length: 5 }],
+        },
+        {
+          id: 'comm3',
+          ranges: [{ pos: 30, length: 15 }],
+        },
+      ])
+
+      commentList.applyDelete(new Range(19, 10))
+      expect(commentList.getComments()).to.eql([
+        {
+          id: 'comm1',
+          ranges: [{ pos: 5, length: 10 }],
+          resolved: false,
+        },
+        {
+          id: 'comm3',
+          ranges: [{ pos: 20, length: 15 }],
+          resolved: false,
+        },
+      ])
+    })
+  })
 })

+ 345 - 0
libraries/overleaf-editor-core/test/range.test.js

@@ -0,0 +1,345 @@
+// @ts-check
+'use strict'
+
+const { expect } = require('chai')
+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.lastIndex).to.eql(14)
+  })
+
+  it('should create a range using fromRaw', function () {
+    const from5to14 = Range.fromRaw({ pos: 5, length: 10 })
+    expect(from5to14.firstIndex).to.eql(5)
+    expect(from5to14.lastIndex).to.eql(14)
+  })
+
+  it('should convert to raw', function () {
+    const from5to14 = new Range(5, 10)
+    expect(from5to14.toRaw()).to.eql({ pos: 5, length: 10 })
+  })
+
+  it('should check isEmpty method', function () {
+    const from5to14 = new Range(5, 10)
+    expect(from5to14.isEmpty()).to.be.false
+
+    const range0length = new Range(5, 0)
+    expect(range0length.isEmpty()).to.be.true
+  })
+
+  describe('overlaps', function () {
+    it('same ranges should overlap', function () {
+      const range1 = new Range(1, 3)
+      const range2 = new Range(1, 3)
+      expect(range1.overlaps(range2)).to.eql(true)
+    })
+
+    it('non-touching ranges should not overlap', function () {
+      const from1to3 = new Range(1, 3)
+      const from10to12 = new Range(10, 3)
+      expect(from1to3.overlaps(from10to12)).to.eql(false)
+      expect(from10to12.overlaps(from1to3)).to.eql(false)
+    })
+
+    it('touching ranges should not overlap', function () {
+      const from1to3 = new Range(1, 3)
+      const from4to6 = new Range(4, 3)
+      expect(from1to3.overlaps(from4to6)).to.eql(false)
+      expect(from4to6.overlaps(from1to3)).to.eql(false)
+    })
+
+    it('should overlap', function () {
+      const from1to3 = new Range(1, 3)
+      const from2to4 = new Range(2, 3)
+      expect(from1to3.overlaps(from2to4)).to.eql(true)
+      expect(from2to4.overlaps(from1to3)).to.eql(true)
+    })
+  })
+
+  describe('touches', function () {
+    it('should not touch if ranges are the same', function () {
+      const range1 = new Range(1, 3)
+      const range2 = new Range(1, 3)
+      expect(range1.touches(range2)).to.eql(false)
+      expect(range2.touches(range1)).to.eql(false)
+    })
+
+    it('should return true when ranges touch at one point', function () {
+      const from1to3 = new Range(1, 3)
+      const from4to5 = new Range(4, 2)
+      expect(from1to3.touches(from4to5)).to.eql(true)
+      expect(from4to5.touches(from1to3)).to.eql(true)
+    })
+
+    it('should return false when ranges do not touch', function () {
+      const from1to3 = new Range(1, 3)
+      const from5to6 = new Range(5, 2)
+      expect(from1to3.touches(from5to6)).to.eql(false)
+      expect(from5to6.touches(from1to3)).to.eql(false)
+    })
+
+    it('should return false when ranges overlap', function () {
+      const from1to3 = new Range(1, 3)
+      const from3to4 = new Range(3, 2)
+      expect(from1to3.touches(from3to4)).to.eql(false)
+      expect(from3to4.touches(from1to3)).to.eql(false)
+    })
+  })
+
+  it('should check if range contains another', function () {
+    const from0to2 = new Range(0, 3)
+    const from4to13 = new Range(4, 10)
+    const from4to14 = new Range(4, 11)
+    const from4to15 = new Range(4, 12)
+    const from5to13 = new Range(5, 9)
+    const from5to14 = new Range(5, 10)
+    const from5to15 = new Range(5, 11)
+    const from0to99 = new Range(0, 100)
+
+    expect(from0to2.contains(from0to2)).to.eql(true)
+    expect(from0to2.contains(from4to13)).to.eql(false)
+    expect(from0to2.contains(from4to14)).to.eql(false)
+    expect(from0to2.contains(from4to15)).to.eql(false)
+    expect(from0to2.contains(from5to13)).to.eql(false)
+    expect(from0to2.contains(from5to14)).to.eql(false)
+    expect(from0to2.contains(from5to15)).to.eql(false)
+    expect(from0to2.contains(from0to99)).to.eql(false)
+
+    expect(from4to13.contains(from0to2)).to.eql(false)
+    expect(from4to13.contains(from4to13)).to.eql(true)
+    expect(from4to13.contains(from4to14)).to.eql(false)
+    expect(from4to13.contains(from4to15)).to.eql(false)
+    expect(from4to13.contains(from5to13)).to.eql(true)
+    expect(from4to13.contains(from5to14)).to.eql(false)
+    expect(from4to13.contains(from5to15)).to.eql(false)
+    expect(from4to13.contains(from0to99)).to.eql(false)
+
+    expect(from4to14.contains(from0to2)).to.eql(false)
+    expect(from4to14.contains(from4to13)).to.eql(true)
+    expect(from4to14.contains(from4to14)).to.eql(true)
+    expect(from4to14.contains(from4to15)).to.eql(false)
+    expect(from4to14.contains(from5to13)).to.eql(true)
+    expect(from4to14.contains(from5to14)).to.eql(true)
+    expect(from4to14.contains(from5to15)).to.eql(false)
+    expect(from4to14.contains(from0to99)).to.eql(false)
+
+    expect(from4to15.contains(from0to2)).to.eql(false)
+    expect(from4to15.contains(from4to13)).to.eql(true)
+    expect(from4to15.contains(from4to14)).to.eql(true)
+    expect(from4to15.contains(from4to15)).to.eql(true)
+    expect(from4to15.contains(from5to13)).to.eql(true)
+    expect(from4to15.contains(from5to14)).to.eql(true)
+    expect(from4to15.contains(from5to15)).to.eql(true)
+    expect(from4to15.contains(from0to99)).to.eql(false)
+
+    expect(from5to13.contains(from0to2)).to.eql(false)
+    expect(from5to13.contains(from4to13)).to.eql(false)
+    expect(from5to13.contains(from4to14)).to.eql(false)
+    expect(from5to13.contains(from4to15)).to.eql(false)
+    expect(from5to13.contains(from5to13)).to.eql(true)
+    expect(from5to13.contains(from5to14)).to.eql(false)
+    expect(from5to13.contains(from5to15)).to.eql(false)
+    expect(from5to13.contains(from0to99)).to.eql(false)
+
+    expect(from5to14.contains(from0to2)).to.eql(false)
+    expect(from5to14.contains(from4to13)).to.eql(false)
+    expect(from5to14.contains(from4to14)).to.eql(false)
+    expect(from5to14.contains(from4to15)).to.eql(false)
+    expect(from5to14.contains(from5to13)).to.eql(true)
+    expect(from5to14.contains(from5to14)).to.eql(true)
+    expect(from5to14.contains(from5to15)).to.eql(false)
+    expect(from5to14.contains(from0to99)).to.eql(false)
+
+    expect(from5to15.contains(from0to2)).to.eql(false)
+    expect(from5to15.contains(from4to13)).to.eql(false)
+    expect(from5to15.contains(from4to14)).to.eql(false)
+    expect(from5to15.contains(from4to15)).to.eql(false)
+    expect(from5to15.contains(from5to13)).to.eql(true)
+    expect(from5to15.contains(from5to14)).to.eql(true)
+    expect(from5to15.contains(from5to15)).to.eql(true)
+    expect(from5to15.contains(from0to99)).to.eql(false)
+
+    expect(from0to99.contains(from0to2)).to.eql(true)
+    expect(from0to99.contains(from4to13)).to.eql(true)
+    expect(from0to99.contains(from4to14)).to.eql(true)
+    expect(from0to99.contains(from4to15)).to.eql(true)
+    expect(from0to99.contains(from5to13)).to.eql(true)
+    expect(from0to99.contains(from5to14)).to.eql(true)
+    expect(from0to99.contains(from5to15)).to.eql(true)
+    expect(from0to99.contains(from0to99)).to.eql(true)
+  })
+
+  it('should check if range contains an index', function () {
+    const from5to14 = new Range(5, 10)
+    expect(from5to14.containsIndex(4)).to.eql(false)
+    expect(from5to14.containsIndex(5)).to.eql(true)
+    expect(from5to14.containsIndex(6)).to.eql(true)
+    expect(from5to14.containsIndex(14)).to.eql(true)
+    expect(from5to14.containsIndex(15)).to.eql(false)
+    expect(from5to14.containsIndex(16)).to.eql(false)
+  })
+
+  describe('subtract range from another', function () {
+    it('should not subtract', 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.length).to.eql(6)
+    })
+
+    it('should subtract from the left', 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.lastIndex).to.eql(9)
+    })
+
+    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.lastIndex).to.eql(9)
+    })
+
+    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.lastIndex).to.eql(14)
+    })
+
+    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.lastIndex).to.throw()
+      expect(from5to19.length).to.eql(0)
+    })
+
+    it('should not subtract if ranges do not overlap', function () {
+      const from5to14 = new Range(5, 10)
+      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.lastIndex).to.eql(14)
+    })
+  })
+
+  describe('merge ranges', function () {
+    it('should merge ranges overlaping at the end', function () {
+      const from5to14 = new Range(5, 10)
+      const from10to19 = new Range(10, 10)
+      expect(from5to14.canMerge(from10to19)).to.eql(true)
+      from5to14.merge(from10to19)
+      expect(from5to14.firstIndex).to.eql(5)
+      expect(from5to14.lastIndex).to.eql(19)
+    })
+
+    it('should merge ranges overlaping at the start', function () {
+      const from5to14 = new Range(5, 10)
+      const from0to9 = new Range(0, 10)
+      expect(from5to14.canMerge(from0to9)).to.eql(true)
+      from5to14.merge(from0to9)
+      expect(from5to14.firstIndex).to.eql(0)
+      expect(from5to14.lastIndex).to.eql(14)
+    })
+
+    it('should merge ranges if one is covered by another', function () {
+      const from5to14 = new Range(5, 10)
+      const from0to19 = new Range(0, 20)
+      expect(from5to14.canMerge(from0to19)).to.eql(true)
+      from5to14.merge(from0to19)
+      expect(from5to14.toRaw()).deep.equal(from0to19.toRaw())
+    })
+
+    it('should produce the same length after merge', function () {
+      const from5to14 = new Range(5, 10)
+      const from0to19 = new Range(0, 20)
+      expect(from0to19.canMerge(from5to14)).to.eql(true)
+      from0to19.merge(from5to14)
+      expect(from0to19.firstIndex).to.eql(0)
+      expect(from0to19.lastIndex).to.eql(19)
+    })
+
+    it('should not merge ranges if they do not overlap', function () {
+      const from5to14 = new Range(5, 10)
+      const from20to29 = new Range(20, 10)
+      expect(from5to14.canMerge(from20to29)).to.eql(false)
+      expect(from20to29.canMerge(from5to14)).to.eql(false)
+      expect(() => from5to14.merge(from20to29)).to.throw()
+    })
+  })
+
+  it('should check if range starts after a range', function () {
+    const from0to4 = new Range(0, 5)
+    const from1to5 = new Range(1, 5)
+    const from5to9 = new Range(5, 5)
+    const from6to10 = new Range(6, 5)
+    const from10to14 = new Range(10, 5)
+
+    expect(from0to4.startsAfter(from0to4)).to.eql(false)
+    expect(from0to4.startsAfter(from1to5)).to.eql(false)
+    expect(from0to4.startsAfter(from5to9)).to.eql(false)
+    expect(from0to4.startsAfter(from6to10)).to.eql(false)
+    expect(from0to4.startsAfter(from10to14)).to.eql(false)
+
+    expect(from1to5.startsAfter(from0to4)).to.eql(false)
+    expect(from1to5.startsAfter(from1to5)).to.eql(false)
+    expect(from1to5.startsAfter(from5to9)).to.eql(false)
+    expect(from1to5.startsAfter(from6to10)).to.eql(false)
+    expect(from1to5.startsAfter(from10to14)).to.eql(false)
+
+    expect(from5to9.startsAfter(from0to4)).to.eql(true)
+    expect(from5to9.startsAfter(from1to5)).to.eql(false)
+    expect(from5to9.startsAfter(from5to9)).to.eql(false)
+    expect(from5to9.startsAfter(from6to10)).to.eql(false)
+    expect(from5to9.startsAfter(from10to14)).to.eql(false)
+
+    expect(from6to10.startsAfter(from0to4)).to.eql(true)
+    expect(from6to10.startsAfter(from1to5)).to.eql(true)
+    expect(from6to10.startsAfter(from5to9)).to.eql(false)
+    expect(from6to10.startsAfter(from6to10)).to.eql(false)
+    expect(from6to10.startsAfter(from10to14)).to.eql(false)
+
+    expect(from10to14.startsAfter(from0to4)).to.eql(true)
+    expect(from10to14.startsAfter(from1to5)).to.eql(true)
+    expect(from10to14.startsAfter(from5to9)).to.eql(true)
+    expect(from10to14.startsAfter(from6to10)).to.eql(false)
+    expect(from10to14.startsAfter(from10to14)).to.eql(false)
+  })
+
+  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.firstIndexIsAfter(4)).to.be.true
+    expect(from5to14.firstIndexIsAfter(5)).to.be.false
+    expect(from5to14.firstIndexIsAfter(6)).to.be.false
+    expect(from5to14.firstIndexIsAfter(15)).to.be.false
+    expect(from5to14.firstIndexIsAfter(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.lastIndex).to.eql(17)
+  })
+
+  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.lastIndex).to.eql(17)
+  })
+})

+ 1 - 0
libraries/overleaf-editor-core/test/string_file_data.test.js

@@ -1,3 +1,4 @@
+// @ts-check
 'use strict'
 
 const { expect } = require('chai')

+ 2 - 1
tsconfig.backend.json

@@ -6,6 +6,7 @@
     "noEmit": true,
     "noImplicitAny": false,
     "skipLibCheck": true,
-    "strict": true
+    "strict": true,
+    "target": "ES2022"
   }
 }