remove updateRangesEffect and rejectChanges from trackChanges extension

GitOrigin-RevId: b93751421d235fd61a80ce64f47aec7998e4f6cf

services/web/frontend/js/features/review-panel-new/context/ranges-context.tsx

@@ -14,7 +14,7 @@ import {
   EditOperation,
 } from '../../../../../types/change'
 import RangesTracker from '@overleaf/ranges-tracker'
-import { rejectChanges } from '@/features/source-editor/extensions/track-changes'
+import { rejectChanges } from '@/features/source-editor/extensions/ranges'
 import { useCodeMirrorViewContext } from '@/features/source-editor/components/codemirror-editor'
 
 export type Ranges = {

services/web/frontend/js/features/source-editor/extensions/ranges.ts

@@ -81,7 +81,7 @@ type Options = {
  * A custom extension that initialises the change manager, passes any updates to it,
  * and produces decorations for tracked changes and comments.
  */
-export const trackChanges = (
+export const ranges = (
   { currentDoc, loadingThreads, ranges, threads }: Options,
   changeManager?: ChangeManager
 ) => {

services/web/frontend/js/features/source-editor/extensions/track-changes.ts

@@ -4,7 +4,6 @@ import {
   StateEffect,
   StateField,
   Transaction,
-  TransactionSpec,
 } from '@codemirror/state'
 import {
   Decoration,
@@ -22,23 +21,14 @@ import {
   StoredComment,
 } from './changes/comments'
 import { invertedEffects } from '@codemirror/commands'
-import {
+import { Change, DeleteOperation } from '../../../../../types/change'
-  Change,
-  DeleteOperation,
-  EditOperation,
-} from '../../../../../types/change'
 import { ChangeManager } from './changes/change-manager'
 import { debugConsole } from '@/utils/debugging'
-import {
+import { isCommentOperation, isDeleteOperation } from '@/utils/operations'
-  isCommentOperation,
-  isDeleteOperation,
-  isInsertOperation,
-} from '@/utils/operations'
 import {
   DocumentContainer,
   RangesTrackerWithResolvedThreadIds,
 } from '@/features/ide-react/editor/document-container'
-import { trackChangesAnnotation } from '@/features/source-editor/extensions/realtime'
 import { Ranges } from '@/features/review-panel-new/context/ranges-context'
 import { Threads } from '@/features/review-panel-new/context/threads-context'
 import { isSplitTestEnabled } from '@/utils/splitTestUtils'
@@ -48,14 +38,6 @@ type RangesData = {
   threads: Threads
 }
 
-const updateRangesEffect = StateEffect.define<RangesData>()
-
-export const updateRanges = (data: RangesData): TransactionSpec => {
-  return {
-    effects: updateRangesEffect.of(data),
-  }
-}
-
 const clearChangesEffect = StateEffect.define()
 const buildChangesEffect = StateEffect.define()
 const restoreDetachedCommentsEffect = StateEffect.define<RangeSet<any>>({
@@ -209,11 +191,6 @@ export const trackChanges = (
                   this.decorations = Decoration.none
                 } else if (effect.is(buildChangesEffect)) {
                   this.decorations = buildChangeDecorations(currentDoc)
-                } else if (effect.is(updateRangesEffect)) {
-                  this.decorations = buildChangeDecorations(
-                    currentDoc,
-                    effect.value
-                  )
                 }
               }
             }
@@ -400,107 +377,6 @@ const createChangeRange = (
   return [calloutWidget.range(from, from), changeMark.range(from, to)]
 }
 
-/**
- * Remove tracked changes from the range tracker when they're rejected,
- * and restore the original content
- */
-export const rejectChanges = (
-  state: EditorState,
-  ranges: DocumentContainer['ranges'],
-  changeIds: string[]
-) => {
-  const changes = ranges!.getChanges(changeIds) as Change<EditOperation>[]
-
-  if (changes.length === 0) {
-    return {}
-  }
-
-  // When doing bulk rejections, adjacent changes might interact with each other.
-  // Consider an insertion with an adjacent deletion (which is a common use-case, replacing words):
-  //
-  //     "foo bar baz" -> "foo quux baz"
-  //
-  // The change above will be modeled with two ops, with the insertion going first:
-  //
-  //     foo quux baz
-  //         |--| -> insertion of "quux", op 1, at position 4
-  //             | -> deletion of "bar", op 2, pushed forward by "quux" to position 8
-  //
-  // When rejecting these changes at once, if the insertion is rejected first, we get unexpected
-  // results. What happens is:
-  //
-  //     1) Rejecting the insertion deletes the added word "quux", i.e., it removes 4 chars
-  //        starting from position 4;
-  //
-  //           "foo quux baz" -> "foo  baz"
-  //                |--| -> 4 characters to be removed
-  //
-  //     2) Rejecting the deletion adds the deleted word "bar" at position 8 (i.e. it will act as if
-  //        the word "quuux" was still present).
-  //
-  //            "foo  baz" -> "foo  bazbar"
-  //                     | -> deletion of "bar" is reverted by reinserting "bar" at position 8
-  //
-  // While the intended result would be "foo bar baz", what we get is:
-  //
-  //      "foo  bazbar" (note "bar" readded at position 8)
-  //
-  // The issue happens because of step 1. To revert the insertion of "quux", 4 characters are deleted
-  // from position 4. This includes the position where the deletion exists; when that position is
-  // cleared, the RangesTracker considers that the deletion is gone and stops tracking/updating it.
-  // As we still hold a reference to it, the code tries to revert it by readding the deleted text, but
-  // does so at the outdated position (position 8, which was valid when "quux" was present).
-  //
-  // To avoid this kind of problem, we need to make sure that reverting operations doesn't affect
-  // subsequent operations that come after. Reverse sorting the operations based on position will
-  // achieve it; in the case above, it makes sure that the the deletion is reverted first:
-  //
-  //     1) Rejecting the deletion adds the deleted word "bar" at position 8
-  //
-  //            "foo quux baz" -> "foo quuxbar baz"
-  //                                       | -> deletion of "bar" is reverted by
-  //                                            reinserting "bar" at position 8
-  //
-  //     2) Rejecting the insertion deletes the added word "quux", i.e., it removes 4 chars
-  //        starting from position 4 and achieves the expected result:
-  //
-  //           "foo quuxbar baz" -> "foo bar baz"
-  //                |--| -> 4 characters to be removed
-
-  changes.sort((a, b) => b.op.p - a.op.p)
-
-  const changesToDispatch = changes.map(change => {
-    const { op } = change
-
-    if (isInsertOperation(op)) {
-      const from = op.p
-      const content = op.i
-      const to = from + content.length
-
-      const text = state.doc.sliceString(from, to)
-
-      if (text !== content) {
-        throw new Error(`Op to be removed does not match editor text`)
-      }
-
-      return { from, to, insert: '' }
-    } else if (isDeleteOperation(op)) {
-      return {
-        from: op.p,
-        to: op.p,
-        insert: op.d,
-      }
-    } else {
-      throw new Error(`unknown change type: ${JSON.stringify(change)}`)
-    }
-  })
-
-  return {
-    changes: changesToDispatch,
-    annotations: [trackChangesAnnotation.of('reject')],
-  }
-}
-
 const trackChangesTheme = EditorView.baseTheme({
   '.cm-line': {
     overflowX: 'hidden', // needed so the callout elements don't overflow (requires line wrapping to be on)

services/web/frontend/js/features/source-editor/hooks/use-codemirror-scope.ts

@@ -63,7 +63,7 @@ import { useUserContext } from '@/shared/context/user-context'
 import { useReferencesContext } from '@/features/ide-react/context/references-context'
 import { setMathPreview } from '@/features/source-editor/extensions/math-preview'
 import { useRangesContext } from '@/features/review-panel-new/context/ranges-context'
-import { updateRanges } from '@/features/source-editor/extensions/track-changes'
+import { updateRanges } from '@/features/source-editor/extensions/ranges'
 import { useThreadsContext } from '@/features/review-panel-new/context/threads-context'
 
 function useCodeMirrorScope(view: EditorView) {