diff --git a/src/DiffEngineViewer.Tests/PreviousChangeTests.cs b/src/DiffEngineViewer.Tests/PreviousChangeTests.cs new file mode 100644 index 00000000..f8c5ef43 --- /dev/null +++ b/src/DiffEngineViewer.Tests/PreviousChangeTests.cs @@ -0,0 +1,64 @@ +/// +/// Stepping back through the changed blocks of the entry on screen. Forty rows with changes at 3, +/// 17 and 33 - so rows 2, 16 and 32 - and sixteen body rows to show them in. +/// +public class PreviousChangeTests +{ + /// + /// A block ending on the row immediately above the viewport is one the reader has not been + /// taken to, and it used to be stepped over as though it were the block they were already in. + /// + [Test] + public async Task Lands_on_a_block_ending_just_above_the_viewport() + { + var state = At(17); + + var moved = ViewerSession.Apply(state, CommandKind.PreviousChange); + + await Assert.That(moved.ScrollTop).IsEqualTo(16); + } + + /// + /// The same thing with nothing above it to fall through to, where the result was no movement + /// at all rather than the wrong movement. + /// + [Test] + public async Task Reaches_the_first_block_from_the_row_below_it() + { + var state = At(3); + + var moved = ViewerSession.Apply(state, CommandKind.PreviousChange); + + await Assert.That(moved.ScrollTop).IsEqualTo(2); + } + + /// + /// A viewport that really is inside a block still steps off it, or previous would never leave + /// the block it is in. + /// + [Test] + public async Task Steps_off_the_block_the_viewport_is_in() + { + var state = At(16); + + var moved = ViewerSession.Apply(state, CommandKind.PreviousChange); + + await Assert.That(moved.ScrollTop).IsEqualTo(2); + } + + [Test] + public async Task Stays_at_the_first_block() + { + var state = At(2); + + var moved = ViewerSession.Apply(state, CommandKind.PreviousChange); + + await Assert.That(moved.ScrollTop).IsEqualTo(2); + } + + static SessionState At(int scrollTop) => + Fixtures.File(Fixtures.Long(true), Fixtures.Long(false)) with + { + ScrollTop = scrollTop + }; +} diff --git a/src/DiffEngineViewer/ViewerSession.cs b/src/DiffEngineViewer/ViewerSession.cs index d59969e7..daf70d93 100644 --- a/src/DiffEngineViewer/ViewerSession.cs +++ b/src/DiffEngineViewer/ViewerSession.cs @@ -1039,7 +1039,13 @@ static int NextChange(IReadOnlyList rows, int from) static int PreviousChange(IReadOnlyList rows, int from) { - var index = Math.Min(from, rows.Count) - 1; + // The top row of the viewport, not the one above it. Stepping off from there took the + // block ending immediately above the viewport for the block the viewport was in, and + // skipped past it to the one before - or, with nothing before it, refused to move at all. + var index = Math.Min(from, rows.Count - 1); + + // So step off only when the viewport really is sitting in a block, which is when its top + // row is itself a change. while (index >= 0 && IsChange(rows[index])) {