From 367fdd9bad96cd8785fa45310bfdbc73e9c41cb8 Mon Sep 17 00:00:00 2001 From: Thomas Nind <31306100+tznind@users.noreply.github.com> Date: Mon, 27 Jun 2022 17:09:31 +0100 Subject: [PATCH] Fix TableView multi selections extending to -1 indexes (#1843) * Fix TableView multi selections extending to -1 indexes * Add to test confirmation that the main active cell also didn't get pushed off * Tidy up formatting * Fixed not calling Application.Shutdown in tests and made it easier to diagnose which test is not shutting down --- Terminal.Gui/Views/TableView.cs | 8 +-- UnitTests/GraphViewTests.cs | 17 +++++- UnitTests/TableViewTests.cs | 96 +++++++++++++++++++++++++++++++++ 3 files changed, 116 insertions(+), 5 deletions(-) diff --git a/Terminal.Gui/Views/TableView.cs b/Terminal.Gui/Views/TableView.cs index 7b94276f4..c67439df7 100644 --- a/Terminal.Gui/Views/TableView.cs +++ b/Terminal.Gui/Views/TableView.cs @@ -869,11 +869,11 @@ namespace Terminal.Gui { /// private TableSelection CreateTableSelection (int pt1X, int pt1Y, int pt2X, int pt2Y) { - var top = Math.Min (pt1Y, pt2Y); - var bot = Math.Max (pt1Y, pt2Y); + var top = Math.Max(Math.Min (pt1Y, pt2Y), 0); + var bot = Math.Max(Math.Max (pt1Y, pt2Y), 0); - var left = Math.Min (pt1X, pt2X); - var right = Math.Max (pt1X, pt2X); + var left = Math.Max(Math.Min (pt1X, pt2X), 0); + var right = Math.Max(Math.Max (pt1X, pt2X), 0); // Rect class is inclusive of Top Left but exclusive of Bottom Right so extend by 1 return new TableSelection (new Point (pt1X, pt1Y), new Rect (left, top, right - left + 1, bot - top + 1)); diff --git a/UnitTests/GraphViewTests.cs b/UnitTests/GraphViewTests.cs index 59188de65..6e41ff2c1 100644 --- a/UnitTests/GraphViewTests.cs +++ b/UnitTests/GraphViewTests.cs @@ -52,12 +52,27 @@ namespace Terminal.Gui.Views { public class GraphViewTests { + static string LastInitFakeDriver; public static FakeDriver InitFakeDriver () { var driver = new FakeDriver (); - Application.Init (driver, new FakeMainLoop (() => FakeConsole.ReadKey (true))); + try { + Application.Init (driver, new FakeMainLoop (() => FakeConsole.ReadKey (true))); + } catch (InvalidOperationException) { + + // close it so that we don't get a thousand of these errors in a row + Application.Shutdown (); + + // but still report a failure and name the test that didn't shut down. Note + // that the test that didn't shutdown won't be the one currently running it will + // be the last one + throw new Exception ("A test did not call shutdown correctly. Test stack trace was:" + LastInitFakeDriver); + } + driver.Init (() => { }); + + LastInitFakeDriver = Environment.StackTrace; return driver; } diff --git a/UnitTests/TableViewTests.cs b/UnitTests/TableViewTests.cs index 0b4a6ca57..ddc9b642e 100644 --- a/UnitTests/TableViewTests.cs +++ b/UnitTests/TableViewTests.cs @@ -550,6 +550,102 @@ namespace Terminal.Gui.Views { Assert.Equal ("R0C0", activatedValue); } + [Fact] + public void TableViewMultiSelect_CannotFallOffLeft() + { + var tv = SetUpMiniTable (); + tv.Table.Rows.Add (1, 2); // add another row (brings us to 2 rows) + + tv.MultiSelect = true; + tv.SelectedColumn = 1; + tv.SelectedRow = 1; + tv.ProcessKey (new KeyEvent (Key.CursorLeft | Key.ShiftMask, new KeyModifiers { Shift = true })); + + Assert.Equal (new Rect (0, 1, 2, 1), tv.MultiSelectedRegions.Single().Rect); + + // this next shift left should be ignored because we are already at the bounds + tv.ProcessKey (new KeyEvent (Key.CursorLeft | Key.ShiftMask, new KeyModifiers { Shift = true })); + + Assert.Equal (new Rect (0, 1, 2, 1), tv.MultiSelectedRegions.Single ().Rect); + + Assert.Equal (0, tv.SelectedColumn); + Assert.Equal (1, tv.SelectedRow); + + Application.Shutdown (); + } + [Fact] + public void TableViewMultiSelect_CannotFallOffRight() + { + var tv = SetUpMiniTable (); + tv.Table.Rows.Add (1, 2); // add another row (brings us to 2 rows) + + tv.MultiSelect = true; + tv.SelectedColumn = 0; + tv.SelectedRow = 1; + tv.ProcessKey (new KeyEvent (Key.CursorRight | Key.ShiftMask, new KeyModifiers { Shift = true })); + + Assert.Equal (new Rect (0, 1, 2, 1), tv.MultiSelectedRegions.Single ().Rect); + + // this next shift right should be ignored because we are already at the right bounds + tv.ProcessKey (new KeyEvent (Key.CursorRight | Key.ShiftMask, new KeyModifiers { Shift = true })); + + Assert.Equal (new Rect (0, 1, 2, 1), tv.MultiSelectedRegions.Single ().Rect); + + Assert.Equal (1, tv.SelectedColumn); + Assert.Equal (1, tv.SelectedRow); + + Application.Shutdown (); + } + [Fact] + public void TableViewMultiSelect_CannotFallOffBottom () + { + var tv = SetUpMiniTable (); + tv.Table.Rows.Add (1, 2); // add another row (brings us to 2 rows) + + tv.MultiSelect = true; + tv.SelectedColumn = 0; + tv.SelectedRow = 0; + tv.ProcessKey (new KeyEvent (Key.CursorRight | Key.ShiftMask, new KeyModifiers { Shift = true })); + tv.ProcessKey (new KeyEvent (Key.CursorDown | Key.ShiftMask, new KeyModifiers { Shift = true })); + + Assert.Equal (new Rect (0, 0, 2, 2), tv.MultiSelectedRegions.Single ().Rect); + + // this next moves should be ignored because we already selected the whole table + tv.ProcessKey (new KeyEvent (Key.CursorRight | Key.ShiftMask, new KeyModifiers { Shift = true })); + tv.ProcessKey (new KeyEvent (Key.CursorDown | Key.ShiftMask, new KeyModifiers { Shift = true })); + + Assert.Equal (new Rect (0, 0, 2, 2), tv.MultiSelectedRegions.Single ().Rect); + Assert.Equal (1, tv.SelectedColumn); + Assert.Equal (1, tv.SelectedRow); + + Application.Shutdown (); + } + + [Fact] + public void TableViewMultiSelect_CannotFallOffTop() + { + var tv = SetUpMiniTable (); + tv.Table.Rows.Add (1, 2); // add another row (brings us to 2 rows) + + tv.MultiSelect = true; + tv.SelectedColumn = 1; + tv.SelectedRow = 1; + tv.ProcessKey (new KeyEvent (Key.CursorLeft | Key.ShiftMask, new KeyModifiers { Shift = true })); + tv.ProcessKey (new KeyEvent (Key.CursorUp | Key.ShiftMask, new KeyModifiers { Shift = true })); + + Assert.Equal (new Rect (0, 0, 2, 2), tv.MultiSelectedRegions.Single ().Rect); + + // this next moves should be ignored because we already selected the whole table + tv.ProcessKey (new KeyEvent (Key.CursorLeft | Key.ShiftMask, new KeyModifiers { Shift = true })); + tv.ProcessKey (new KeyEvent (Key.CursorUp | Key.ShiftMask, new KeyModifiers { Shift = true })); + + Assert.Equal (new Rect (0, 0, 2, 2), tv.MultiSelectedRegions.Single ().Rect); + Assert.Equal (0, tv.SelectedColumn); + Assert.Equal (0, tv.SelectedRow); + + Application.Shutdown (); + } + [Theory] [InlineData (false)] [InlineData (true)]