From 09ff9526b8564ac4a75e109cc0abd0c4590af982 Mon Sep 17 00:00:00 2001 From: Mathias Jakobsen Date: Mon, 5 Feb 2024 13:19:45 +0000 Subject: [PATCH] Merge pull request #16872 from overleaf/mj-exclusive-ranges [overleaf-editor-core] Make ranges exclusive GitOrigin-RevId: 5b821d5bc1da8a6647305c268abc6185a7fa1f80 --- .../lib/file_data/comment.js | 10 +-- .../lib/file_data/range.js | 78 +++++++------------ .../overleaf-editor-core/test/range.test.js | 76 +++++++++--------- 3 files changed, 73 insertions(+), 91 deletions(-) diff --git a/libraries/overleaf-editor-core/lib/file_data/comment.js b/libraries/overleaf-editor-core/lib/file_data/comment.js index 844a153a2a..44a87520f5 100644 --- a/libraries/overleaf-editor-core/lib/file_data/comment.js +++ b/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) diff --git a/libraries/overleaf-editor-core/lib/file_data/range.js b/libraries/overleaf-editor-core/lib/file_data/range.js index c131e14e5b..7b59f6b4a7 100644 --- a/libraries/overleaf-editor-core/lib/file_data/range.js +++ b/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 } - /** - * @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() { + get end() { 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) { - return this.firstIndex > pos + startIsAfter(pos) { + return this.start > pos } /** @@ -56,27 +46,22 @@ class Range { * @param {Range} range */ contains(range) { - return ( - this.firstIndex <= range.firstIndex && this.lastIndex >= range.lastIndex - ) + return this.start <= range.start && this.end >= range.end } /** - * checks if the range contains a given index - * @param {number} index + * checks if the range contains a cursor (i.e. is not at the ends of the range) + * @param {number} cursor */ - containsIndex(index) { - return this.firstIndex <= index && this.lastIndex >= index + containsCursor(cursor) { + return this.start <= cursor && this.end >= cursor } /** * @param {Range} range */ overlaps(range) { - return ( - this.firstIndex < range.firstNextIndex && - this.firstNextIndex > range.firstIndex - ) + return this.start < range.end && this.end > range.start } /** @@ -84,10 +69,7 @@ class Range { * @param {Range} range */ touches(range) { - return ( - this.firstNextIndex === range.firstIndex || - this.firstIndex === range.firstNextIndex - ) + return this.end === range.start || this.start === range.end } /** @@ -107,13 +89,13 @@ class Range { } if (range.overlaps(this)) { - if (range.firstIndex < this.firstIndex) { - const intersectedLength = range.lastIndex - this.firstIndex + 1 + if (range.start < this.start) { + 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 newLastIndex = Math.max(this.lastIndex, range.lastIndex) - this.pos = newFirstIndex - this.length = newLastIndex + 1 - newFirstIndex + const newPos = Math.min(this.pos, range.pos) + const newEnd = Math.max(this.end, range.end) + this.pos = newPos + this.length = newEnd - newPos } /** @@ -160,22 +142,22 @@ class Range { } /** - * Splits a range on the index and insert a range with the length provided - * @param {number} index + * Splits a range on the cursor and insert a range with the length provided + * @param {number} cursor * @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') + insertAt(cursor, length) { + if (!this.containsCursor(cursor)) { + throw new Error('The cursor 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 + const rangeUpToCursor = new Range(this.pos, cursor - this.pos) + const insertedRange = new Range(cursor, length) + const rangeAfterCursor = new Range( + cursor + length, + this.length - rangeUpToCursor.length ) - return [rangeUpToIndex, insertedRange, rangeAfterIndex] + return [rangeUpToCursor, insertedRange, rangeAfterCursor] } toRaw() { diff --git a/libraries/overleaf-editor-core/test/range.test.js b/libraries/overleaf-editor-core/test/range.test.js index 1f640b2457..287a92258e 100644 --- a/libraries/overleaf-editor-core/test/range.test.js +++ b/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.lastIndex).to.eql(14) + expect(from5to14.start).to.eql(5) + 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.lastIndex).to.eql(14) + expect(from5to14.start).to.eql(5) + 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.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) + expect(from5to14.containsCursor(4)).to.eql(false) + expect(from5to14.containsCursor(5)).to.eql(true) + expect(from5to14.containsCursor(6)).to.eql(true) + expect(from5to14.containsCursor(14)).to.eql(true) + expect(from5to14.containsCursor(15)).to.eql(true) + 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.lastIndex).to.eql(9) + expect(from15to24.start).to.eql(5) + 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.lastIndex).to.eql(9) + expect(from5to19.start).to.eql(5) + 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.lastIndex).to.eql(14) + expect(from5to19.start).to.eql(5) + 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.lastIndex).to.throw() + expect(from5to19.start).to.eql(5) + 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.lastIndex).to.eql(14) + expect(from5to14.start).to.eql(5) + 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.lastIndex).to.eql(19) + expect(from5to14.start).to.eql(5) + 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.lastIndex).to.eql(14) + expect(from5to14.start).to.eql(0) + 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.lastIndex).to.eql(19) + expect(from0to19.start).to.eql(0) + 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.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 + expect(from5to14.startIsAfter(3)).to.be.true + expect(from5to14.startIsAfter(4)).to.be.true + expect(from5to14.startIsAfter(5)).to.be.false + expect(from5to14.startIsAfter(6)).to.be.false + expect(from5to14.startIsAfter(15)).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.lastIndex).to.eql(17) + expect(from5to14.start).to.eql(5) + 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.lastIndex).to.eql(17) + expect(from5to14.start).to.eql(8) + expect(from5to14.end).to.eql(18) }) })