Fixes #3172. Application.ResetState wasn't resetting all it should (#…

…3173)

* Removed resharper settings from editorconfig

* Fixed Applicaton.ResetState. New unit tests

* Tweaked pull request template

Terminal.Gui/Application.cs

@@ -32,6 +32,77 @@ namespace Terminal.Gui;
 /// TODO: Flush this out.
 /// </remarks>
 public static partial class Application {
+
+	// IMPORTANT: Ensure all property/fields are reset here. See Init_ResetState_Resets_Properties unit test.
+	// Encapsulate all setting of initial state for Application; Having
+	// this in a function like this ensures we don't make mistakes in
+	// guaranteeing that the state of this singleton is deterministic when Init
+	// starts running and after Shutdown returns.
+	internal static void ResetState ()
+	{
+		// Shutdown is the bookend for Init. As such it needs to clean up all resources
+		// Init created. Apps that do any threading will need to code defensively for this.
+		// e.g. see Issue #537
+		foreach (var t in _topLevels) {
+			t.Running = false;
+			t.Dispose ();
+		}
+		_topLevels.Clear ();
+		Current = null;
+		Top?.Dispose ();
+		Top = null;
+
+		// MainLoop stuff
+		MainLoop?.Dispose ();
+		MainLoop = null;
+		_mainThreadId = -1;
+		Iteration = null;
+		EndAfterFirstIteration = false;
+
+		// Driver stuff
+		if (Driver != null) {
+			Driver.SizeChanged -= Driver_SizeChanged;
+			Driver.KeyDown -= Driver_KeyDown;
+			Driver.KeyUp -= Driver_KeyUp;
+			Driver.MouseEvent -= Driver_MouseEvent;
+			Driver?.End ();
+			Driver = null;
+		}
+		// Don't reset ForceDriver; it needs to be set before Init is called.
+		//ForceDriver = string.Empty;
+		Force16Colors = false;
+		_forceFakeConsole = false;
+
+		// Run State stuff
+		NotifyNewRunState = null;
+		NotifyStopRunState = null;
+		MouseGrabView = null;
+		_initialized = false;
+
+		// Mouse
+		_mouseEnteredView = null;
+		WantContinuousButtonPressedView = null;
+		MouseEvent = null;
+		GrabbedMouse = null;
+		UnGrabbingMouse = null;
+		GrabbedMouse = null;
+		UnGrabbedMouse = null;
+
+		// Keyboard
+		AlternateBackwardKey = Key.Empty;
+		AlternateForwardKey = Key.Empty;
+		QuitKey = Key.Empty;
+		KeyDown = null;
+		KeyUp = null;
+		SizeChanging = null;
+
+		// Reset synchronization context to allow the user to run async/await,
+		// as the main loop has been ended, the synchronization context from 
+		// gui.cs does no longer process any callbacks. See #1084 for more details:
+		// (https://github.com/gui-cs/Terminal.Gui/issues/1084).
+		SynchronizationContext.SetSynchronizationContext (syncContext: null);
+	}
+
 	/// <summary>
 	/// Gets the <see cref="ConsoleDriver"/> that has been selected. See also <see cref="ForceDriver"/>.
 	/// </summary>
@@ -66,7 +137,7 @@ public static partial class Application {
 	/// </summary>
 	public static List<CultureInfo> SupportedCultures => _cachedSupportedCultures;
 
-	static List<CultureInfo> GetSupportedCultures ()
+	internal static List<CultureInfo> GetSupportedCultures ()
 	{
 		var culture = CultureInfo.GetCultures (CultureTypes.AllCultures);
 
@@ -241,55 +312,6 @@ public static void Shutdown ()
 		ResetState ();
 		PrintJsonErrors ();
 	}
-
-	// Encapsulate all setting of initial state for Application; Having
-	// this in a function like this ensures we don't make mistakes in
-	// guaranteeing that the state of this singleton is deterministic when Init
-	// starts running and after Shutdown returns.
-	static void ResetState ()
-	{
-		// Shutdown is the bookend for Init. As such it needs to clean up all resources
-		// Init created. Apps that do any threading will need to code defensively for this.
-		// e.g. see Issue #537
-		foreach (var t in _topLevels) {
-			t.Running = false;
-			t.Dispose ();
-		}
-		_topLevels.Clear ();
-		Current = null;
-		Top?.Dispose ();
-		Top = null;
-
-		// BUGBUG: OverlappedTop is not cleared here, but it should be?
-
-		MainLoop?.Dispose ();
-		MainLoop = null;
-		if (Driver != null) {
-			Driver.SizeChanged -= Driver_SizeChanged;
-			Driver.KeyDown -= Driver_KeyDown;
-			Driver.KeyUp -= Driver_KeyUp;
-			Driver.MouseEvent -= Driver_MouseEvent;
-			Driver?.End ();
-			Driver = null;
-		}
-		Iteration = null;
-		MouseEvent = null;
-		KeyDown = null;
-		KeyUp = null;
-		SizeChanging = null;
-		_mainThreadId = -1;
-		NotifyNewRunState = null;
-		NotifyStopRunState = null;
-		_initialized = false;
-		MouseGrabView = null;
-		_mouseEnteredView = null;
-
-		// Reset synchronization context to allow the user to run async/await,
-		// as the main loop has been ended, the synchronization context from 
-		// gui.cs does no longer process any callbacks. See #1084 for more details:
-		// (https://github.com/gui-cs/Terminal.Gui/issues/1084).
-		SynchronizationContext.SetSynchronizationContext (syncContext: null);
-	}
 	#endregion Initialization (Init/Shutdown)
 
 	#region Run (Begin, Run, End, Stop)
@@ -881,7 +903,7 @@ public static void End (RunState runState)
 	/// </summary>
 	// BUGBUG: Techncally, this is not the full lst of TopLevels. THere be dragons hwre. E.g. see how Toplevel.Id is used. What
 	// about TopLevels that are just a SubView of another View?
-	static readonly Stack<Toplevel> _topLevels = new Stack<Toplevel> ();
+	internal static readonly Stack<Toplevel> _topLevels = new Stack<Toplevel> ();
 
 	/// <summary>
 	/// The <see cref="Toplevel"/> object used for the application on startup (<seealso cref="Application.Top"/>)
@@ -1141,7 +1163,7 @@ static void OnUnGrabbedMouse (View view)
 	}
 
 	// Used by OnMouseEvent to track the last view that was clicked on.
-	static View _mouseEnteredView;
+	internal static View _mouseEnteredView;
 
 	/// <summary>
 	/// Event fired when a mouse move or click occurs. Coordinates are screen relative.
@@ -1333,7 +1355,7 @@ bool FrameHandledMouseEvent (Adornment frame)
 	#endregion Mouse handling
 
 	#region Keyboard handling
-	static Key _alternateForwardKey = new Key (KeyCode.PageDown | KeyCode.CtrlMask);
+	static Key _alternateForwardKey = Key.Empty; // Defined in config.json
 
 	/// <summary>
 	/// Alternative key to navigate forwards through views. Ctrl+Tab is the primary key.
@@ -1358,7 +1380,7 @@ static void OnAlternateForwardKeyChanged (KeyChangedEventArgs e)
 		}
 	}
 
-	static Key _alternateBackwardKey = new Key (KeyCode.PageUp | KeyCode.CtrlMask);
+	static Key _alternateBackwardKey = Key.Empty; // Defined in config.json
 
 	/// <summary>
 	/// Alternative key to navigate backwards through views. Shift+Ctrl+Tab is the primary key.
@@ -1383,7 +1405,7 @@ static void OnAlternateBackwardKeyChanged (KeyChangedEventArgs oldKey)
 		}
 	}
 
-	static Key _quitKey = new Key (KeyCode.Q | KeyCode.CtrlMask);
+	static Key _quitKey = Key.Empty; // Defined in config.json
 
 	/// <summary>
 	/// Gets or sets the key to quit the application.

Terminal.Gui/Configuration/ConfigProperty.cs

@@ -82,7 +82,9 @@ public bool Apply ()
 	{
 		if (PropertyValue != null) {
 			try {
-				PropertyInfo?.SetValue (null, DeepMemberwiseCopy (PropertyValue, PropertyInfo?.GetValue (null)));
+				if (PropertyInfo?.GetValue (null) != null) {
+					PropertyInfo?.SetValue (null, DeepMemberwiseCopy (PropertyValue, PropertyInfo?.GetValue (null)));
+				}
 			} catch (TargetInvocationException tie) {
 				// Check if there is an inner exception
 				if (tie.InnerException != null) {

Terminal.Gui/Views/ToplevelOverlapped.cs

@@ -42,7 +42,7 @@ public static List<Toplevel> OverlappedChildren {
 	/// </summary>
 	public static Toplevel OverlappedTop {
 		get {
-			if (Top.IsOverlappedContainer) {
+			if (Top is { IsOverlappedContainer: true }) {
 				return Top;
 			}
 			return null;

UnitTests/Application/ApplicationTests.cs

@@ -1,7 +1,10 @@
 using System;
+using System.Collections.Generic;
 using System.Diagnostics;
+using System.Globalization;
 using System.IO;
 using System.Linq;
+using System.Reflection;
 using System.Runtime.InteropServices;
 using System.Threading;
 using System.Threading.Tasks;
@@ -665,6 +668,90 @@ public void Internal_Properties_Correct ()
 		Assert.False (Application.MoveToOverlappedChild (Application.Top));
 	}
 
+	[Fact]
+	public void Init_ResetState_Resets_Properties ()
+	{
+		ConfigurationManager.ThrowOnJsonErrors = true;
+		// For all the fields/properties of Application, check that they are reset to their default values
+
+		// Set some values
+
+		Application.Init ();
+		Application._initialized = true;
+
+		// Reset
+		Application.ResetState ();
+
+		void CheckReset ()
+		{
+			// Check that all fields and properties are set to their default values
+
+			// Public Properties
+			Assert.Null (Application.Top);
+			Assert.Null (Application.Current);
+			Assert.Null (Application.MouseGrabView);
+			Assert.Null (Application.WantContinuousButtonPressedView);
+			// Don't check Application.ForceDriver
+			// Assert.Empty (Application.ForceDriver);
+			Assert.False (Application.Force16Colors);
+			Assert.Null (Application.Driver);
+			Assert.Null (Application.MainLoop);
+			Assert.False (Application.EndAfterFirstIteration);
+			Assert.Equal (Key.Empty, Application.AlternateBackwardKey);
+			Assert.Equal (Key.Empty, Application.AlternateForwardKey);
+			Assert.Equal (Key.Empty, Application.QuitKey);
+			Assert.Null (Application.OverlappedChildren);
+			Assert.Null (Application.OverlappedTop);
+
+			// Internal properties
+			Assert.False (Application._initialized);
+			Assert.Equal (Application.GetSupportedCultures (), Application.SupportedCultures);
+			Assert.False (Application._forceFakeConsole);
+			Assert.Equal (-1, Application._mainThreadId);
+			Assert.Empty (Application._topLevels);
+			Assert.Null (Application._mouseEnteredView);
+
+			// Events - Can't check
+			//Assert.Null (Application.NotifyNewRunState);
+			//Assert.Null (Application.NotifyNewRunState);
+			//Assert.Null (Application.Iteration);
+			//Assert.Null (Application.SizeChanging);
+			//Assert.Null (Application.GrabbedMouse);
+			//Assert.Null (Application.UnGrabbingMouse);
+			//Assert.Null (Application.GrabbedMouse);
+			//Assert.Null (Application.UnGrabbedMouse);
+			//Assert.Null (Application.MouseEvent);
+			//Assert.Null (Application.KeyDown);
+			//Assert.Null (Application.KeyUp);
+		}
+
+		CheckReset ();
+
+		// Set the values that can be set
+		Application._initialized = true;
+		Application._forceFakeConsole = true;
+		Application._mainThreadId = 1;
+		//Application._topLevels = new List<Toplevel> ();
+		Application._mouseEnteredView = new View ();
+		//Application.SupportedCultures = new List<CultureInfo> ();
+		Application.Force16Colors = true;
+		//Application.ForceDriver = "driver";
+		Application.EndAfterFirstIteration = true;
+		Application.AlternateBackwardKey = Key.A;
+		Application.AlternateForwardKey = Key.B;
+		Application.QuitKey = Key.C;
+		//Application.OverlappedChildren = new List<View> ();
+		//Application.OverlappedTop = 
+		Application._mouseEnteredView = new View ();
+		//Application.WantContinuousButtonPressedView = new View ();
+
+		Application.ResetState ();
+		CheckReset ();
+
+		ConfigurationManager.ThrowOnJsonErrors = false;
+
+	}
+
 	// Invoke Tests
 	// TODO: Test with threading scenarios
 	[Fact]

UnitTests/Views/StatusBarTests.cs

@@ -14,12 +14,14 @@ public StatusBarTests (ITestOutputHelper output)
 		[Fact]
 		public void StatusItem_Constructor ()
 		{
+			Application.Init ();
 			var si = new StatusItem (Application.QuitKey, $"{Application.QuitKey} to Quit", null);
 			Assert.Equal (KeyCode.CtrlMask | KeyCode.Q, si.Shortcut);
 			Assert.Equal ($"{Application.QuitKey} to Quit", si.Title);
 			Assert.Null (si.Action);
 			si = new StatusItem (Application.QuitKey, $"{Application.QuitKey} to Quit", () => { });
 			Assert.NotNull (si.Action);
+			Application.Shutdown ();
 		}
 
 		[Fact]

pull_request_template.md

@@ -1,4 +1,10 @@
-Fixes #_____ - Include a terse summary of the change or which issue is fixed.
+## Fixes
+
+- Fixes #_____
+
+## Proposed Changes/Todos
+
+- [ ] Todo 1
 
 ## Pull Request checklist: