From b87dad77d916e7fb6e1d52147607c6c34d5e74e7 Mon Sep 17 00:00:00 2001 From: Domagoj Kriskovic Date: Thu, 1 Feb 2024 12:31:11 +0100 Subject: [PATCH] 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 --- .../lib/file_data/comment.js | 123 ++++++- .../lib/file_data/comment_list.js | 37 +- .../lib/file_data/range.js | 170 +++++++++ .../overleaf-editor-core/test/comment.test.js | 123 +++++++ .../test/comments_list.test.js | 313 +++++++++++++++- .../overleaf-editor-core/test/range.test.js | 345 ++++++++++++++++++ .../test/string_file_data.test.js | 1 + tsconfig.backend.json | 3 +- 8 files changed, 1107 insertions(+), 8 deletions(-) create mode 100644 libraries/overleaf-editor-core/test/comment.test.js create mode 100644 libraries/overleaf-editor-core/test/range.test.js diff --git a/libraries/overleaf-editor-core/lib/file_data/comment.js b/libraries/overleaf-editor-core/lib/file_data/comment.js index 83ca6d9229..844a153a2a 100644 --- a/libraries/overleaf-editor-core/lib/file_data/comment.js +++ b/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} diff --git a/libraries/overleaf-editor-core/lib/file_data/comment_list.js b/libraries/overleaf-editor-core/lib/file_data/comment_list.js index b44971afeb..9c7086e184 100644 --- a/libraries/overleaf-editor-core/lib/file_data/comment_list.js +++ b/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 diff --git a/libraries/overleaf-editor-core/lib/file_data/range.js b/libraries/overleaf-editor-core/lib/file_data/range.js index 83bd42d742..c131e14e5b 100644 --- a/libraries/overleaf-editor-core/lib/file_data/range.js +++ b/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, diff --git a/libraries/overleaf-editor-core/test/comment.test.js b/libraries/overleaf-editor-core/test/comment.test.js new file mode 100644 index 0000000000..2dd24e157a --- /dev/null +++ b/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)]) + }) +}) diff --git a/libraries/overleaf-editor-core/test/comments_list.test.js b/libraries/overleaf-editor-core/test/comments_list.test.js index 15e764a38b..ee052c4e41 100644 --- a/libraries/overleaf-editor-core/test/comments_list.test.js +++ b/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, + }, + ]) + }) + }) }) diff --git a/libraries/overleaf-editor-core/test/range.test.js b/libraries/overleaf-editor-core/test/range.test.js new file mode 100644 index 0000000000..1f640b2457 --- /dev/null +++ b/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) + }) +}) diff --git a/libraries/overleaf-editor-core/test/string_file_data.test.js b/libraries/overleaf-editor-core/test/string_file_data.test.js index 1c37954412..2f3e13e2cb 100644 --- a/libraries/overleaf-editor-core/test/string_file_data.test.js +++ b/libraries/overleaf-editor-core/test/string_file_data.test.js @@ -1,3 +1,4 @@ +// @ts-check 'use strict' const { expect } = require('chai') diff --git a/tsconfig.backend.json b/tsconfig.backend.json index 0473518565..b0bc63f668 100644 --- a/tsconfig.backend.json +++ b/tsconfig.backend.json @@ -6,6 +6,7 @@ "noEmit": true, "noImplicitAny": false, "skipLibCheck": true, - "strict": true + "strict": true, + "target": "ES2022" } }