From ecf585f629b40ced416592c53c470f29fc9c83c3 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Sat, 22 Aug 2026 23:34:50 +1000 Subject: [PATCH] Keep the scroll when a re-sent patch says what the last one said EnqueueInline started the reader at the top for any arrival carrying the key of the entry on screen, whether or not that entry changed. A continuous test runner sends one every few seconds for as long as the test keeps failing, so reading anything past the first screenful meant being thrown back to the top of it on every run. InlineQueue.Fold already reports an identical patch as unchanged and Project hands back the same entry, so the entry instance is the thing to compare. The attached path has always worked this way - SyncKeepsTheScrollWhenNothingChanged pins it - and this is the same rule on the path that owns the queue. --- src/DiffEngineViewer.Tests/ReEnqueueTests.cs | 45 ++++++++++++++++++++ src/DiffEngineViewer/ViewerSession.cs | 26 +++++++---- 2 files changed, 63 insertions(+), 8 deletions(-) create mode 100644 src/DiffEngineViewer.Tests/ReEnqueueTests.cs diff --git a/src/DiffEngineViewer.Tests/ReEnqueueTests.cs b/src/DiffEngineViewer.Tests/ReEnqueueTests.cs new file mode 100644 index 00000000..edb45887 --- /dev/null +++ b/src/DiffEngineViewer.Tests/ReEnqueueTests.cs @@ -0,0 +1,45 @@ +/// +/// A patch arriving for the entry already on screen. A continuous test runner sends one every few +/// seconds for as long as the test keeps failing, and most of them say exactly what the last one +/// said. +/// +public class ReEnqueueTests +{ + [Test] + public async Task An_identical_re_send_leaves_the_scroll_alone() + { + var state = Scrolled(); + + var again = ViewerSession.EnqueueInline(state, Patch(Fixtures.Long(true))); + + await Assert.That(again.Queue[0]).IsSameReferenceAs(state.Queue[0]); + await Assert.That(again.ScrollTop).IsEqualTo(state.ScrollTop); + } + + /// + /// A re-send that says something else is a new comparison, and that one does start at the top. + /// + [Test] + public async Task A_re_send_of_different_content_starts_at_the_top() + { + var state = Scrolled(); + + var again = ViewerSession.EnqueueInline(state, Patch($"{Fixtures.Long(true)}\nand one more line")); + + await Assert.That(again.ScrollTop).IsEqualTo(0); + } + + static SessionState Scrolled() + { + var state = ViewerSession.Apply(Fixtures.Inline(Patch(Fixtures.Long(true))), CommandKind.PageDown); + if (state.ScrollTop == 0) + { + throw new("The entry did not scroll, so nothing below asserts anything."); + } + + return state; + } + + static InlinePatch Patch(string content) => + Fixtures.Patch("A.cs", 1, null, content); +} diff --git a/src/DiffEngineViewer/ViewerSession.cs b/src/DiffEngineViewer/ViewerSession.cs index d59969e7..5b6b805f 100644 --- a/src/DiffEngineViewer/ViewerSession.cs +++ b/src/DiffEngineViewer/ViewerSession.cs @@ -29,19 +29,29 @@ public static SessionState Resize(SessionState state, int columns, int rows) => public static SessionState EnqueueInline(SessionState state, InlinePatch patch) { var key = InlineKey.For(patch.SourceFile, patch.LineHint); - var replacedCurrent = state.Current?.Key == key; + var current = state.Current; var queue = Rebuild(state, Pending(state).Enqueue(patch)); // Grouping can reorder the list, so the selection follows its key rather than its index. - var currentKey = state.Current?.Key; - var selected = currentKey is null ? 0 : IndexOf(queue, currentKey); + var selected = current is null ? 0 : IndexOf(queue, current.Key); + if (selected < 0) + { + selected = 0; + } + + // Start the reader at the top again only when the text under them changed. Folding into an + // entry further down the list is not it, and neither is a re-send of what is already + // there: Fold reports an identical patch as unchanged and Project hands back the same + // entry, so a continuous runner re-sending the same failing snapshot every few seconds + // used to bounce the reader to the top on every run. + var replaced = current is not null && + current.Key == key && + !ReferenceEquals(queue[selected], current); + return Clamp(state with { Queue = queue, - Selected = selected < 0 ? 0 : selected, - // The text under the reader just changed, so start it at the top again. Only when it - // is the item on screen; folding into one further down the list should not move - // anything. - ScrollTop = replacedCurrent ? 0 : state.ScrollTop, + Selected = selected, + ScrollTop = replaced ? 0 : state.ScrollTop, // The open menu indexes the queue it was opened over, which just changed. Menu = null });