diff --git a/src/DiffEngineViewer.Tests/QueueRemovalTests.cs b/src/DiffEngineViewer.Tests/QueueRemovalTests.cs new file mode 100644 index 00000000..024b3576 --- /dev/null +++ b/src/DiffEngineViewer.Tests/QueueRemovalTests.cs @@ -0,0 +1,54 @@ +/// +/// What a removal does to the entry being read. Removals arrive for entries other than that one - +/// a settle from a test that has started passing, a sweep from a group header, a bulk accept that +/// skipped this entry - and the reader is in the middle of a comparison while they do. +/// +public class QueueRemovalTests +{ + [Test] + public async Task Settling_another_entry_leaves_the_one_being_read_where_it_was() + { + var state = Scrolled(out var reading); + + var settled = ViewerSession.Settle(state, state.Queue[0].Key); + + await Assert.That(settled.Current!.Key).IsEqualTo(reading); + await Assert.That(settled.ScrollTop).IsEqualTo(state.ScrollTop); + } + + /// + /// The entry on screen going is the one case where the reader has to be moved, and the top of + /// the next entry is where they go. + /// + [Test] + public async Task Settling_the_entry_being_read_moves_on_to_the_next() + { + var state = Scrolled(out var reading); + + var settled = ViewerSession.Settle(state, reading); + + await Assert.That(settled.Current!.Key).IsNotEqualTo(reading); + await Assert.That(settled.ScrollTop).IsEqualTo(0); + } + + /// + /// Three entries, the middle one selected and scrolled into - so that a removal above it moves + /// every index below, which is the thing being asserted about. + /// + static SessionState Scrolled(out string reading) + { + var state = Fixtures.Inline( + Fixtures.Patch("A.cs", 1, null, Fixtures.Long(true)), + Fixtures.Patch("B.cs", 2, null, Fixtures.Long(true)), + Fixtures.Patch("C.cs", 3, null, Fixtures.Long(true))); + reading = state.Queue[1].Key; + state = ViewerSession.SelectKey(state, reading); + state = ViewerSession.Apply(state, CommandKind.PageDown); + if (state.ScrollTop == 0) + { + throw new("The entry did not scroll, so nothing below asserts anything."); + } + + return state; + } +} diff --git a/src/DiffEngineViewer/ViewerSession.cs b/src/DiffEngineViewer/ViewerSession.cs index d59969e7..eb734e31 100644 --- a/src/DiffEngineViewer/ViewerSession.cs +++ b/src/DiffEngineViewer/ViewerSession.cs @@ -870,16 +870,32 @@ static int IndexOf(IReadOnlyList queue, string key) return -1; } - static SessionState Remove(SessionState state, IReadOnlyList queue, string? message) => - Clamp(state with + /// + /// Drops whatever is no longer in the queue and leaves the reader where they were. + /// + /// Most removals are not of the entry on screen: a settle from a test that has started + /// passing, "Accept all in <solution>" from a header, a sweep that skipped this one. + /// Holding as an index across those quietly changed what + /// was on screen to whatever the old index now landed on, at the top of it. So the selection + /// follows its key, the way and do, and only + /// an entry that is itself gone falls back to advancing by index. + /// + /// + static SessionState Remove(SessionState state, IReadOnlyList queue, string? message) + { + var key = state.Current?.Key; + var selected = key is null ? -1 : IndexOf(queue, key); + return Clamp(state with { Queue = queue, - ScrollTop = 0, + Selected = selected < 0 ? state.Selected : selected, + ScrollTop = selected < 0 ? 0 : state.ScrollTop, Message = message, // Nothing left to manage, so the window has no reason to stay open. Exit = queue.Count == 0, Menu = null }); + } static SessionState Select(SessionState state, int index) {