fix(vcs): keep merge editor panes scrollable while they scroll together (#758)

Scrolling a pane set the other two panes' scrollTop; their scroll events arrived a frame later, after the sync guard was cleared, were taken as user scrolls and written back, so the pane being scrolled was pulled back. Sync now maps positions by content and ignores the echoes of its own writes; a pane that cannot scroll no longer drags the others. Next/Previous Difference (F7 / Shift+F7) go through the keymap instead of a fixed key handler. Fixes #744.

src/features/vcs/MergeEditor.tsx

@@ -8,13 +8,17 @@ import { keymap } from "@codemirror/view";
 import { unifiedMergeView } from "@codemirror/merge";
 import type { MergeSides } from "@/bindings";
 import { embeddedEditorExtensions } from "@/editor/api";
+import { setContext } from "@/platform/context";
 import { tx, useTxTemplate } from "@/platform/i18n";
 import { call, commands } from "@/platform/ipc";
+import { shortcutLabel } from "@/platform/keymap";
 import { Button, IconButton, showContextMenu, Spinner, T } from "@/ui";
 import { editorService, type EditorInput } from "@/workbench/contributions";
 import { blameSlot, toggleAnnotations } from "./blame";
 import { languageFor, viewerBase } from "./cm";
 import { confirmDialog } from "./dialogs";
+import { focusedDiffNavigator, setFocusedDiffNavigator, type DiffNavigator } from "./diffNavigator";
+import { syncMergeScroll } from "./mergeScroll";
 import { initialResult, mergeTitles, resolutionText, type Resolution, type ResultConflict } from "./conflicts";
 import { baseName } from "./model";
 import { errorMessage, readWorkingText, runOp, writeFileText } from "./ops";
@@ -220,28 +224,10 @@ export function MergeEditor({ input }: { input: EditorInput }) {
     setLeft(result.state.field(conflictField).conflicts.length);
     oursView.current = ours;
     theirsView.current = theirs;
-    // Scroll the three panes together (by scroll ratio), like IDEA's merge view.
-    const views = [ours, result, theirs];
-    let syncing = false;
-    const onScroll = (from: EditorView) => () => {
-      if (syncing) return;
-      syncing = true;
-      const s = from.scrollDOM;
-      const ratio = s.scrollTop / Math.max(1, s.scrollHeight - s.clientHeight);
-      for (const other of views) {
-        if (other === from) continue;
-        const o = other.scrollDOM;
-        o.scrollTop = ratio * Math.max(0, o.scrollHeight - o.clientHeight);
-      }
-      syncing = false;
-    };
-    const offs = views.map((v) => {
-      const h = onScroll(v);
-      v.scrollDOM.addEventListener("scroll", h, { passive: true });
-      return () => v.scrollDOM.removeEventListener("scroll", h);
-    });
+    // Scroll the three panes together, lined up through their changes.
+    const stopSync = syncMergeScroll({ ours, result, theirs });
     return () => {
-      offs.forEach((off) => off());
+      stopSync();
       ours.destroy();
       theirs.destroy();
       result.destroy();
@@ -313,17 +299,31 @@ export function MergeEditor({ input }: { input: EditorInput }) {
     }
   };
 
-  // IDEA's merge view: F7 / Shift+F7 jump between conflicts; each side pane can
-  // annotate its version with Git Blame from the context menu.
-  // Each side pane blames on the branch it belongs to, so lines that only exist on
-  // the other branch show their real history instead of "Not Committed Yet".
+  // Next / Previous Difference (F7 / Shift+F7 by default) jump between conflicts through
+  // the keymap, as in the diff viewer.
+  // `goto` reads only refs, so the first render's one stays valid.
+  const [nav] = useState<DiffNavigator>(() => ({
+    next: () => goto(1),
+    prev: () => goto(-1),
+    jumpToSource: () => void editorService().openFile(path, { focus: true }),
+    applyChange: () => false,
+    file: () => {},
+    showSettings: () => {},
+  }));
+  useEffect(
+    () => () => {
+      if (focusedDiffNavigator() === nav) {
+        setFocusedDiffNavigator(null);
+        setContext("diffEditorFocus", false);
+      }
+    },
+    [nav],
+  );
+
+  // Each side pane can annotate its version with Git Blame from the context menu, on
+  // the branch it belongs to, so lines that only exist on the other branch show their
+  // real history instead of "Not Committed Yet".
   const otherRev = state?.sides.commit ?? "HEAD";
-  const onKeyDown = (e: React.KeyboardEvent) => {
-    if (e.key === "F7") {
-      e.preventDefault();
-      goto(e.shiftKey ? -1 : 1);
-    }
-  };
 
   const sideMenu = (e: React.MouseEvent, viewRef: { current: EditorView | null }, text: string, revision?: string) => {
     const view = viewRef.current;
@@ -358,10 +358,34 @@ export function MergeEditor({ input }: { input: EditorInput }) {
     );
 
   return (
-    <div className={styles.root} onKeyDown={onKeyDown}>
+    <div
+      className={styles.root}
+      onFocusCapture={() => {
+        setFocusedDiffNavigator(nav);
+        setContext("diffEditorFocus", true);
+      }}
+      onBlurCapture={(e) => {
+        if (!e.currentTarget.contains(e.relatedTarget as Node | null) && focusedDiffNavigator() === nav) {
+          setFocusedDiffNavigator(null);
+          setContext("diffEditorFocus", false);
+        }
+      }}
+    >
       <div className={styles.toolbar} role="toolbar" aria-label={tx("Merge")}>
-        <IconButton icon="arrow-up" title="Previous Conflict" size="small" onClick={() => goto(-1)} />
-        <IconButton icon="arrow-down" title="Next Conflict" size="small" onClick={() => goto(1)} />
+        <IconButton
+          icon="arrow-up"
+          title="Previous Conflict"
+          shortcut={shortcutLabel("diff.prevDifference")}
+          size="small"
+          onClick={() => goto(-1)}
+        />
+        <IconButton
+          icon="arrow-down"
+          title="Next Conflict"
+          shortcut={shortcutLabel("diff.nextDifference")}
+          size="small"
+          onClick={() => goto(1)}
+        />
         <span className={styles.sep} />
         <Button size="small" onClick={() => acceptAll("ours")}>
           <T>Accept All Yours</T>

src/features/vcs/__tests__/mergeScroll.test.ts

@@ -0,0 +1,28 @@
+import { describe, expect, it } from "vitest";
+import { ScrollEchoes } from "../mergeScroll";
+
+describe("ScrollEchoes", () => {
+  it("recognizes the scroll event a move causes, once", () => {
+    const echoes = new ScrollEchoes<string>();
+    echoes.mark("result", 120);
+    // The browser may round the position it scrolled to.
+    expect(echoes.take("result", 120.5)).toBe(true);
+    // The next event of that pane is the user's.
+    expect(echoes.take("result", 120.5)).toBe(false);
+  });
+
+  it("treats a pane the user scrolled past the move as the user's", () => {
+    const echoes = new ScrollEchoes<string>();
+    echoes.mark("theirs", 120);
+    expect(echoes.take("theirs", 180)).toBe(false);
+    expect(echoes.take("ours", 0)).toBe(false);
+  });
+
+  it("keeps moves of each pane apart", () => {
+    const echoes = new ScrollEchoes<string>();
+    echoes.mark("ours", 10);
+    echoes.mark("theirs", 40);
+    expect(echoes.take("theirs", 40)).toBe(true);
+    expect(echoes.take("ours", 10)).toBe(true);
+  });
+});

src/features/vcs/diffScroll.ts

@@ -23,7 +23,7 @@ export interface Band {
 }
 
 /** Pixel [top, bottom] of a chunk's range in `view`; an empty range is the boundary it sits on. */
-function extent(view: EditorView, from: number, to: number): [number, number] {
+export function extent(view: EditorView, from: number, to: number): [number, number] {
   const len = view.state.doc.length;
   if (from > len) {
     const end = view.lineBlockAt(len).bottom;
@@ -345,6 +345,6 @@ export class UnalignedPanes {
 }
 
 /** Offset of the document's top within the pane's scrolled content. */
-function docOffset(view: EditorView): number {
+export function docOffset(view: EditorView): number {
   return view.documentTop - view.scrollDOM.getBoundingClientRect().top + view.scrollDOM.scrollTop;
 }

src/features/vcs/mergeScroll.ts

@@ -0,0 +1,98 @@
+/**
+ * Synchronized scrolling of the merge editor's three panes. Each side pane lines up with
+ * the result through the changes between them (the diff viewer's SyncScrollSupport
+ * mapping), and one side follows the other through the result.
+ */
+import { Chunk } from "@codemirror/merge";
+import type { Text } from "@codemirror/state";
+import type { EditorView } from "@codemirror/view";
+import { bandAt, docOffset, extent, transferY } from "./diffScroll";
+
+export type MergePane = "ours" | "result" | "theirs";
+const PANES: readonly MergePane[] = ["ours", "result", "theirs"];
+
+/**
+ * Scroll positions a pane was moved to by another one. A pane's scroll event arrives a
+ * frame after the move, so it must be recognized as that move's echo: following it would
+ * write back to the pane the user is scrolling and cancel the scroll in progress.
+ */
+export class ScrollEchoes<K> {
+  private readonly moved = new Map<K, number>();
+
+  /** `pane` was moved to `top`. */
+  mark(pane: K, top: number) {
+    this.moved.set(pane, top);
+  }
+
+  /** Whether a scroll event of `pane` at `top` is the echo of the last move (consumed either way). */
+  take(pane: K, top: number): boolean {
+    const moved = this.moved.get(pane);
+    this.moved.delete(pane);
+    return moved !== undefined && Math.abs(top - moved) <= 1;
+  }
+}
+
+/** Starts scrolling `views` together; returns the function that stops it. */
+export function syncMergeScroll(views: Record<MergePane, EditorView>): () => void {
+  const built = new Map<MergePane, { a: Text; b: Text; chunks: readonly Chunk[] }>();
+  const chunks = (side: MergePane) => {
+    const a = views[side].state.doc;
+    const b = views.result.state.doc;
+    const hit = built.get(side);
+    if (hit && hit.a === a && hit.b === b) return hit.chunks;
+    const fresh = Chunk.build(a, b);
+    built.set(side, { a, b, chunks: fresh });
+    return fresh;
+  };
+  /** Document y of `side` → the result (`toResult`), or back. */
+  const transfer = (side: MergePane, y: number, toResult: boolean) => {
+    const a = views[side];
+    const b = views.result;
+    const from = toResult ? "a" : "b";
+    const band = bandAt(
+      chunks(side),
+      (c) => extent(a, c.fromA, c.toA),
+      (c) => extent(b, c.fromB, c.toB),
+      a.lineBlockAt(a.state.doc.length).bottom,
+      b.lineBlockAt(b.state.doc.length).bottom,
+      y,
+      from,
+    );
+    return transferY([band], y, from);
+  };
+  const map = (from: MergePane, to: MergePane, y: number) => {
+    if (to === "result") return transfer(from, y, true);
+    if (from === "result") return transfer(to, y, false);
+    return transfer(to, transfer(from, y, true), false);
+  };
+
+  const echoes = new ScrollEchoes<MergePane>();
+  /** Scrolls the other panes to match `from`, anchored a third down the pane. */
+  const follow = (from: MergePane) => {
+    const master = views[from];
+    const anchor = master.scrollDOM.clientHeight / 3;
+    const at = master.scrollDOM.scrollTop + anchor - docOffset(master);
+    for (const to of PANES) {
+      if (to === from) continue;
+      const el = views[to].scrollDOM;
+      const top = Math.max(
+        0,
+        Math.min(map(from, to, at) + docOffset(views[to]) - anchor, el.scrollHeight - el.clientHeight),
+      );
+      if (Math.abs(el.scrollTop - top) < 1) continue;
+      const before = el.scrollTop;
+      el.scrollTop = top;
+      if (el.scrollTop !== before) echoes.mark(to, el.scrollTop);
+    }
+  };
+
+  const offs = PANES.map((pane) => {
+    const el = views[pane].scrollDOM;
+    const onScroll = () => {
+      if (!echoes.take(pane, el.scrollTop)) follow(pane);
+    };
+    el.addEventListener("scroll", onScroll, { passive: true });
+    return () => el.removeEventListener("scroll", onScroll);
+  });
+  return () => offs.forEach((off) => off());
+}