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

@@ -869,11 +869,11 @@ namespace Terminal.Gui {
 		/// <returns></returns>
 		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));

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;
 		}
 

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)]