UseKeysUpDownAsKeysLeftRight and UseSubMenusSingleFrame cannot be both true. (#1789)

* Fixes the throwing an exception if menu item is null.

* UseKeysUpDownAsKeysLeftRight and UseSubMenusSingleFrame cannot be both true.

* Refactoring the code.

* Trying fixing the Run_All_Scenarios unit test.

* Fixing typo to test again.

* Added Attribute suffix to he AutoInitShutdown class.

Example/demo.cs

@@ -651,11 +651,17 @@ static class Demo {
 					new MenuItem [] { menuItems [0], menuItems [1] }),
 				menuItems[3],
 				miUseKeysUpDownAsKeysLeftRight = new MenuItem ("Use_KeysUpDownAsKeysLeftRight", "",
-				() => menu.UseKeysUpDownAsKeysLeftRight = miUseKeysUpDownAsKeysLeftRight.Checked = useKeysUpDownAsKeysLeftRight = !useKeysUpDownAsKeysLeftRight) {
+				() => {
+				menu.UseKeysUpDownAsKeysLeftRight = miUseKeysUpDownAsKeysLeftRight.Checked = useKeysUpDownAsKeysLeftRight = !useKeysUpDownAsKeysLeftRight;
+					miUseSubMenusSingleFrame.Checked = useSubMenusSingleFrame = menu.UseSubMenusSingleFrame;
+					}) {
 					CheckType = MenuItemCheckStyle.Checked, Checked = useKeysUpDownAsKeysLeftRight
 				},
 				miUseSubMenusSingleFrame = new MenuItem ("Use_SubMenusSingleFrame", "",
-				() => menu.UseSubMenusSingleFrame = miUseSubMenusSingleFrame.Checked = useSubMenusSingleFrame = !useSubMenusSingleFrame) {
+				() => {
+				menu.UseSubMenusSingleFrame = miUseSubMenusSingleFrame.Checked = useSubMenusSingleFrame = !useSubMenusSingleFrame;
+					miUseKeysUpDownAsKeysLeftRight.Checked = useKeysUpDownAsKeysLeftRight = menu.UseKeysUpDownAsKeysLeftRight;
+					}) {
 					CheckType = MenuItemCheckStyle.Checked, Checked = useSubMenusSingleFrame
 				},
 				miHeightAsBuffer = new MenuItem ("_Height As Buffer", "", () => {

Terminal.Gui/Views/Menu.cs

@@ -654,7 +654,7 @@ namespace Terminal.Gui {
 				if (current >= barItems.Children.Length) {
 					current = 0;
 				}
-				if (this != host.openCurrentMenu && barItems.Children [current].IsFromSubMenu && host.selectedSub > -1) {
+				if (this != host.openCurrentMenu && barItems.Children [current]?.IsFromSubMenu == true && host.selectedSub > -1) {
 					host.PreviousMenu (true);
 					host.SelectEnabledItem (barItems.Children, current, out current);
 					host.openCurrentMenu = this;
@@ -843,10 +843,21 @@ namespace Terminal.Gui {
 		/// <value>The menu array.</value>
 		public MenuBarItem [] Menus { get; set; }
 
+		private bool useKeysUpDownAsKeysLeftRight = false;
+
 		/// <summary>
 		/// Used for change the navigation key style.
 		/// </summary>
-		public bool UseKeysUpDownAsKeysLeftRight { get; set; } = false;
+		public bool UseKeysUpDownAsKeysLeftRight {
+			get => useKeysUpDownAsKeysLeftRight;
+			set {
+				useKeysUpDownAsKeysLeftRight = value;
+				if (value && UseSubMenusSingleFrame) {
+					UseSubMenusSingleFrame = false;
+					SetNeedsDisplay ();
+				}
+			}
+		}
 
 		static ustring shortcutDelimiter = "+";
 		/// <summary>
@@ -866,10 +877,21 @@ namespace Terminal.Gui {
 		/// </summary>
 		new public static Rune HotKeySpecifier => '_';
 
+		private bool useSubMenusSingleFrame;
+
 		/// <summary>
 		/// Gets or sets if the sub-menus must be displayed in a single or multiple frames.
 		/// </summary>
-		public bool UseSubMenusSingleFrame { get; set; }
+		public bool UseSubMenusSingleFrame {
+			get => useSubMenusSingleFrame;
+			set {
+				useSubMenusSingleFrame = value;
+				if (value && UseKeysUpDownAsKeysLeftRight) {
+					useKeysUpDownAsKeysLeftRight = false;
+					SetNeedsDisplay ();
+				}
+			}
+		}
 
 		/// <summary>
 		/// Initializes a new instance of the <see cref="MenuBar"/>.

UnitTests/ApplicationTests.cs

@@ -1293,45 +1293,47 @@ namespace Terminal.Gui.Core {
 			int numberOfTimeoutsPerThread = 100;
 
 
-			// start lots of threads
-			for (int i = 0; i < numberOfThreads; i++) {
-				
-				var myi = i;
+			lock (Application.Top) {
+				// start lots of threads
+				for (int i = 0; i < numberOfThreads; i++) {
 
-				Task.Run (() => {
-					Task.Delay (100).Wait ();
+					var myi = i;
 
-					// each thread registers lots of 1s timeouts
-					for(int j=0;j< numberOfTimeoutsPerThread; j++) {
+					Task.Run (() => {
+						Task.Delay (100).Wait ();
 
-						Application.MainLoop.AddTimeout (TimeSpan.FromSeconds(1), (s) => {
+						// each thread registers lots of 1s timeouts
+						for (int j = 0; j < numberOfTimeoutsPerThread; j++) {
 
-							// each timeout delegate increments delegatesRun count by 1 every second
-							Interlocked.Increment (ref delegatesRun);
-							return true; 
-						});
-					}
-					 
-					// if this is the first Thread created
-					if (myi == 0) {
+							Application.MainLoop.AddTimeout (TimeSpan.FromSeconds (1), (s) => {
 
-						// let the timeouts run for a bit
-						Task.Delay (5000).Wait ();
+								// each timeout delegate increments delegatesRun count by 1 every second
+								Interlocked.Increment (ref delegatesRun);
+								return true;
+							});
+						}
 
-						// then tell the application to quuit
-						Application.MainLoop.Invoke (() => Application.RequestStop ());
-					}
-				});
-			}
+						// if this is the first Thread created
+						if (myi == 0) {
 
-			// blocks here until the RequestStop is processed at the end of the test
-			Application.Run ();
+							// let the timeouts run for a bit
+							Task.Delay (5000).Wait ();
 
-			// undershoot a bit to be on the safe side.  The 5000 ms wait allows the timeouts to run
-			// a lot but all those timeout delegates could end up going slowly on a slow machine perhaps
-			// so the final number of delegatesRun might vary by computer.  So for this assert we say
-			// that it should have run at least 2 seconds worth of delegates
-			Assert.True (delegatesRun >= numberOfThreads * numberOfTimeoutsPerThread * 2);
+							// then tell the application to quit
+							Application.MainLoop.Invoke (() => Application.RequestStop ());
+						}
+					});
+				}
+
+				// blocks here until the RequestStop is processed at the end of the test
+				Application.Run ();
+
+				// undershoot a bit to be on the safe side.  The 5000 ms wait allows the timeouts to run
+				// a lot but all those timeout delegates could end up going slowly on a slow machine perhaps
+				// so the final number of delegatesRun might vary by computer.  So for this assert we say
+				// that it should have run at least 2 seconds worth of delegates
+				Assert.True (delegatesRun >= numberOfThreads * numberOfTimeoutsPerThread * 2);
+			}
 		}
 	}
 }

UnitTests/AssemblyInfo.cs

@@ -13,7 +13,7 @@ using Xunit;
 // This is necessary because a) Application is a singleton and Init/Shutdown must be called
 // as a pair, and b) all unit test functions should be atomic.
 [AttributeUsage (AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false, Inherited = true)]
-public class AutoInitShutdown : Xunit.Sdk.BeforeAfterTestAttribute {
+public class AutoInitShutdownAttribute : Xunit.Sdk.BeforeAfterTestAttribute {
 
 	static bool _init = false;
 	public override void Before (MethodInfo methodUnderTest)

UnitTests/MenuTests.cs

@@ -188,6 +188,7 @@ Edit
 					new MenuBarItem ("_New", new MenuItem [] {
 						new MenuItem ("_New doc", "Creates new doc.", null, () => false)
 					}),
+					null,
 					new MenuItem ("_Save", "Saves the file.", null, null)
 				})
 			});
@@ -227,6 +228,16 @@ Edit
 			}));
 			Assert.True (menu.IsMenuOpen);
 			Assert.Equal ("_File", miCurrent.Parent.Title);
+			Assert.Equal ("_New", miCurrent.Title);
+
+			Assert.True (mCurrent.MouseEvent (new MouseEvent () {
+				X = 1,
+				Y = 3,
+				Flags = MouseFlags.ReportMousePosition,
+				View = mCurrent
+			}));
+			Assert.True (menu.IsMenuOpen);
+			Assert.Equal ("_File", miCurrent.Parent.Title);
 			Assert.Equal ("_Save", miCurrent.Title);
 
 			// close the menu
@@ -1269,5 +1280,21 @@ Edit
 			pos = GraphViewTests.AssertDriverContentsWithFrameAre (expected, output);
 			Assert.Equal (new Rect (2, 0, 13, 1), pos);
 		}
+
+		[Fact]
+		public void UseKeysUpDownAsKeysLeftRight_And_UseSubMenusSingleFrame_Cannot_Be_Both_True ()
+		{
+			var menu = new MenuBar ();
+			Assert.False (menu.UseKeysUpDownAsKeysLeftRight);
+			Assert.False (menu.UseSubMenusSingleFrame);
+
+			menu.UseKeysUpDownAsKeysLeftRight = true;
+			Assert.True (menu.UseKeysUpDownAsKeysLeftRight);
+			Assert.False (menu.UseSubMenusSingleFrame);
+
+			menu.UseSubMenusSingleFrame = true;
+			Assert.False (menu.UseKeysUpDownAsKeysLeftRight);
+			Assert.True (menu.UseSubMenusSingleFrame);
+		}
 	}
 }

UnitTests/ScenarioTests.cs

@@ -82,7 +82,9 @@ namespace Terminal.Gui {
 					};
 					var token = Application.MainLoop.AddTimeout (TimeSpan.FromMilliseconds (ms), abortCallback);
 
-					var scenario = (Scenario)Activator.CreateInstance (scenarioClass);
+					Scenario scenario = null;
+					var exception = Record.Exception (() => scenario = (Scenario)Activator.CreateInstance (scenarioClass));
+					Assert.Null (exception);
 					scenario.Init (Application.Top, Colors.Base);
 					scenario.Setup ();
 					// There is no need to call Application.Begin because Init already creates the Application.Top