Przeglądaj źródła

Merge pull request #21682 from overleaf/em-revert-ranges-tracker-changes

Revert RangesTracker fix for multiple tracked deletes at same position

GitOrigin-RevId: ee6ade406a86319f5551863e18ed40d508128ecb
Eric Mc Sween 1 rok temu
rodzic
commit
d95981cb0c

+ 7 - 24
libraries/ranges-tracker/index.cjs

@@ -313,11 +313,10 @@ class RangesTracker {
 
     let alreadyMerged = false
     let previousChange = null
-    let trackedDeleteRejected = false
     const movedChanges = []
     const removeChanges = []
     const newChanges = []
-    const trackedDeletesAtOpPosition = []
+
     for (let i = 0; i < this.changes.length; i++) {
       change = this.changes[i]
       const changeStart = change.op.p
@@ -328,22 +327,13 @@ class RangesTracker {
           change.op.p += opLength
           movedChanges.push(change)
         } else if (opStart === changeStart) {
-          // Keep track of tracked deletes that are at the same position as the
-          // insert. At the end of the loop, If we didn't find a tracked delete
-          // to reject, we'll move these deletes after the insert.
-          trackedDeletesAtOpPosition.push(change)
-
-          if (trackedDeleteRejected) {
-            // We rejected a tracked delete. Move the remaining tracked deletes after the insert
-            change.op.p += opLength
-            movedChanges.push(change)
-          } else if (
+          // If we are undoing, then we want to cancel any existing delete ranges if we can.
+          // Check if the insert matches the start of the delete, and just remove it from the delete instead if so.
+          if (
             undoing &&
             change.op.d.length >= op.i.length &&
             change.op.d.slice(0, op.i.length) === op.i
           ) {
-            // If we are undoing, then we want to reject any existing tracked delete if we can.
-            // Check if the insert matches the start of the delete, and just remove it from the delete instead if so.
             change.op.d = change.op.d.slice(op.i.length)
             change.op.p += op.i.length
             if (change.op.d === '') {
@@ -352,7 +342,9 @@ class RangesTracker {
               movedChanges.push(change)
             }
             alreadyMerged = true
-            trackedDeleteRejected = true
+          } else {
+            change.op.p += opLength
+            movedChanges.push(change)
           }
         }
       } else if (change.op.i != null) {
@@ -457,15 +449,6 @@ class RangesTracker {
       previousChange = change
     }
 
-    if (!trackedDeleteRejected) {
-      // We didn't reject any tracked delete. Move all existing tracked deletes
-      // at the insert position after the insert
-      for (const change of trackedDeletesAtOpPosition) {
-        change.op.p += opLength
-        movedChanges.push(change)
-      }
-    }
-
     if (this.track_changes && !alreadyMerged) {
       this._addOp(op, metadata)
     }

+ 0 - 42
libraries/ranges-tracker/test/unit/ranges-tracker-test.js

@@ -4,7 +4,6 @@ const RangesTracker = require('../..')
 describe('RangesTracker', function () {
   describe('with duplicate change ids', function () {
     beforeEach(function () {
-      this.comments = []
       this.changes = [
         { id: 'id1', op: { p: 1, i: 'hello' } },
         { id: 'id2', op: { p: 10, i: 'world' } },
@@ -27,45 +26,4 @@ describe('RangesTracker', function () {
       expect(this.rangesTracker.changes).to.deep.equal([this.changes[2]])
     })
   })
-
-  describe('with multiple tracked deletes at the same position', function () {
-    beforeEach(function () {
-      this.comments = []
-      this.changes = [
-        { id: 'id1', op: { p: 33, d: 'before' } },
-        { id: 'id2', op: { p: 50, d: 'right before' } },
-        { id: 'id3', op: { p: 50, d: 'this one' } },
-        { id: 'id4', op: { p: 50, d: 'right after' } },
-        { id: 'id5', op: { p: 75, d: 'long after' } },
-      ]
-      this.rangesTracker = new RangesTracker(this.changes, this.comments)
-    })
-
-    it('preserves the text order when rejecting changes', function () {
-      this.rangesTracker.applyOp(
-        { p: 50, i: 'this one', u: true },
-        { user_id: 'user-id' }
-      )
-      expect(this.rangesTracker.changes).to.deep.equal([
-        { id: 'id1', op: { p: 33, d: 'before' } },
-        { id: 'id2', op: { p: 50, d: 'right before' } },
-        { id: 'id4', op: { p: 58, d: 'right after' } },
-        { id: 'id5', op: { p: 83, d: 'long after' } },
-      ])
-    })
-
-    it('moves all tracked deletes after the insert if not rejecting changes', function () {
-      this.rangesTracker.applyOp(
-        { p: 50, i: 'some other text', u: true },
-        { user_id: 'user-id' }
-      )
-      expect(this.rangesTracker.changes).to.deep.equal([
-        { id: 'id1', op: { p: 33, d: 'before' } },
-        { id: 'id2', op: { p: 65, d: 'right before' } },
-        { id: 'id3', op: { p: 65, d: 'this one' } },
-        { id: 'id4', op: { p: 65, d: 'right after' } },
-        { id: 'id5', op: { p: 90, d: 'long after' } },
-      ])
-    })
-  })
 })