diff --git a/Terminal.Gui/Input/Responder.cs b/Terminal.Gui/Input/Responder.cs deleted file mode 100644 index a7f01a4bb..000000000 --- a/Terminal.Gui/Input/Responder.cs +++ /dev/null @@ -1,65 +0,0 @@ -using System.Reflection; - -namespace Terminal.Gui; - -/// Responder base class implemented by objects that want to participate on keyboard and mouse input. -public class Responder : IDisposable -{ - private bool _disposedValue; - - /// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resource. - - public void Dispose () - { - // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method - Disposing?.Invoke (this, EventArgs.Empty); - Dispose (true); - GC.SuppressFinalize (this); -#if DEBUG_IDISPOSABLE - WasDisposed = true; - - foreach (Responder instance in Instances.Where (x => x.WasDisposed).ToList ()) - { - Instances.Remove (instance); - } -#endif - } - - /// Event raised when has been called to signal that this object is being disposed. - [CanBeNull] - public event EventHandler Disposing; - - /// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources. - /// - /// If disposing equals true, the method has been called directly or indirectly by a user's code. Managed and - /// unmanaged resources can be disposed. If disposing equals false, the method has been called by the runtime from - /// inside the finalizer and you should not reference other objects. Only unmanaged resources can be disposed. - /// - /// - protected virtual void Dispose (bool disposing) - { - if (!_disposedValue) - { - if (disposing) - { - // TODO: dispose managed state (managed objects) - } - - _disposedValue = true; - } - } - -#if DEBUG_IDISPOSABLE - /// For debug purposes to verify objects are being disposed properly - public bool WasDisposed; - - /// For debug purposes to verify objects are being disposed properly - public int DisposedCount = 0; - - /// For debug purposes - public static List Instances = new (); - - /// For debug purposes - public Responder () { Instances.Add (this); } -#endif -} diff --git a/Terminal.Gui/View/View.cs b/Terminal.Gui/View/View.cs index 24ab96b87..7af099f79 100644 --- a/Terminal.Gui/View/View.cs +++ b/Terminal.Gui/View/View.cs @@ -74,8 +74,10 @@ namespace Terminal.Gui; /// To flag the entire view for redraw call . /// /// -/// The method is called when the size or layout of a view has changed. The will -/// cause to be called on the next so there is normally no reason to direclty call +/// The method is called when the size or layout of a view has changed. The +/// will +/// cause to be called on the next so there is normally +/// no reason to direclty call /// see . /// /// @@ -107,7 +109,7 @@ namespace Terminal.Gui; #endregion API Docs -public partial class View : Responder, ISupportInitializeNotification +public partial class View : IDisposable, ISupportInitializeNotification { #region Constructors and Initialization @@ -135,6 +137,10 @@ public partial class View : Responder, ISupportInitializeNotification /// public View () { +#if DEBUG_IDISPOSABLE + Instances.Add (this); +#endif + SetupAdornments (); SetupCommands (); @@ -524,8 +530,16 @@ public partial class View : Responder, ISupportInitializeNotification /// public override string ToString () { return $"{GetType ().Name}({Id}){Frame}"; } - /// - protected override void Dispose (bool disposing) + private bool _disposedValue; + + /// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources. + /// + /// If disposing equals true, the method has been called directly or indirectly by a user's code. Managed and + /// unmanaged resources can be disposed. If disposing equals false, the method has been called by the runtime from + /// inside the finalizer and you should not reference other objects. Only unmanaged resources can be disposed. + /// + /// + protected virtual void Dispose (bool disposing) { LineCanvas.Dispose (); @@ -540,7 +554,49 @@ public partial class View : Responder, ISupportInitializeNotification subview.Dispose (); } - base.Dispose (disposing); + if (!_disposedValue) + { + if (disposing) + { + // TODO: dispose managed state (managed objects) + } + + _disposedValue = true; + } + Debug.Assert (InternalSubviews.Count == 0); } + + /// + /// Riased when the is being disposed. + /// + public event EventHandler? Disposing; + + /// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resource. + public void Dispose () + { + // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method + Disposing?.Invoke (this, EventArgs.Empty); + Dispose (true); + GC.SuppressFinalize (this); +#if DEBUG_IDISPOSABLE + WasDisposed = true; + + foreach (View instance in Instances.Where (x => x.WasDisposed).ToList ()) + { + Instances.Remove (instance); + } +#endif + } + +#if DEBUG_IDISPOSABLE + /// For debug purposes to verify objects are being disposed properly + public bool WasDisposed { get; set; } + + /// For debug purposes to verify objects are being disposed properly + public int DisposedCount { get; set; } = 0; + + /// For debug purposes + public static List Instances { get; set; } = []; +#endif } diff --git a/UICatalog/UICatalog.cs b/UICatalog/UICatalog.cs index 70b6c583d..d6a9fd107 100644 --- a/UICatalog/UICatalog.cs +++ b/UICatalog/UICatalog.cs @@ -607,12 +607,12 @@ public class UICatalogApp // Validate there are no outstanding Responder-based instances // after a scenario was selected to run. This proves the main UI Catalog // 'app' closed cleanly. - foreach (Responder? inst in Responder.Instances) + foreach (View? inst in View.Instances) { Debug.Assert (inst.WasDisposed); } - Responder.Instances.Clear (); + View.Instances.Clear (); // Validate there are no outstanding Application.RunState-based instances // after a scenario was selected to run. This proves the main UI Catalog diff --git a/UnitTests/Application/ApplicationTests.cs b/UnitTests/Application/ApplicationTests.cs index c0289893e..3428fcd15 100644 --- a/UnitTests/Application/ApplicationTests.cs +++ b/UnitTests/Application/ApplicationTests.cs @@ -13,7 +13,7 @@ public class ApplicationTests ConfigurationManager.Locations = ConfigurationManager.ConfigLocations.None; #if DEBUG_IDISPOSABLE - Responder.Instances.Clear (); + View.Instances.Clear (); RunState.Instances.Clear (); #endif } @@ -66,7 +66,7 @@ public class ApplicationTests Assert.True (shutdown); #if DEBUG_IDISPOSABLE - Assert.Empty (Responder.Instances); + Assert.Empty (View.Instances); #endif lock (_timeoutLock) { @@ -393,7 +393,7 @@ public class ApplicationTests // Validate there are no outstanding Responder-based instances // after a scenario was selected to run. This proves the main UI Catalog // 'app' closed cleanly. - Assert.Empty (Responder.Instances); + Assert.Empty (View.Instances); #endif } diff --git a/UnitTests/Application/KeyboardTests.cs b/UnitTests/Application/KeyboardTests.cs index 9673e7b60..4c33361e6 100644 --- a/UnitTests/Application/KeyboardTests.cs +++ b/UnitTests/Application/KeyboardTests.cs @@ -11,7 +11,7 @@ public class KeyboardTests { _output = output; #if DEBUG_IDISPOSABLE - Responder.Instances.Clear (); + View.Instances.Clear (); RunState.Instances.Clear (); #endif } @@ -671,7 +671,7 @@ public class KeyboardTests Assert.True (shutdown); #if DEBUG_IDISPOSABLE - Assert.Empty (Responder.Instances); + Assert.Empty (View.Instances); #endif lock (_timeoutLock) { diff --git a/UnitTests/Application/Mouse/ApplicationMouseTests.cs b/UnitTests/Application/Mouse/ApplicationMouseTests.cs index 437ac9058..b180a768c 100644 --- a/UnitTests/Application/Mouse/ApplicationMouseTests.cs +++ b/UnitTests/Application/Mouse/ApplicationMouseTests.cs @@ -13,7 +13,7 @@ public class ApplicationMouseTests { _output = output; #if DEBUG_IDISPOSABLE - Responder.Instances.Clear (); + View.Instances.Clear (); RunState.Instances.Clear (); #endif } diff --git a/UnitTests/Application/RunStateTests.cs b/UnitTests/Application/RunStateTests.cs index ff34ab5d4..5b8a53c58 100644 --- a/UnitTests/Application/RunStateTests.cs +++ b/UnitTests/Application/RunStateTests.cs @@ -8,7 +8,7 @@ public class RunStateTests public RunStateTests () { #if DEBUG_IDISPOSABLE - Responder.Instances.Clear (); + View.Instances.Clear (); RunState.Instances.Clear (); #endif } diff --git a/UnitTests/Input/ResponderTests.cs b/UnitTests/Input/ResponderTests.cs index 1b40889a3..ed899aede 100644 --- a/UnitTests/Input/ResponderTests.cs +++ b/UnitTests/Input/ResponderTests.cs @@ -4,110 +4,7 @@ namespace Terminal.Gui.InputTests; public class ResponderTests { - // Generic lifetime (IDisposable) tests - [Fact] - [TestRespondersDisposed] - public void Dispose_Works () - { - var r = new Responder (); -#if DEBUG_IDISPOSABLE - Assert.Single (Responder.Instances); -#endif - - r.Dispose (); -#if DEBUG_IDISPOSABLE - Assert.Empty (Responder.Instances); -#endif - } - - [Fact] - public void Disposing_Event_Notify_All_Subscribers_On_The_First_Container () - { - #if DEBUG_IDISPOSABLE - // Only clear before because need to test after assert - Responder.Instances.Clear (); - #endif - - var container1 = new View { Id = "Container1" }; - var count = 0; - - var view = new View { Id = "View" }; - view.Disposing += View_Disposing; - container1.Add (view); - Assert.Equal (container1, view.SuperView); - - void View_Disposing (object sender, EventArgs e) - { - count++; - Assert.Equal (view, sender); - container1.Remove ((View)sender); - } - - Assert.Single (container1.Subviews); - - var container2 = new View { Id = "Container2" }; - - container2.Add (view); - Assert.Equal (container2, view.SuperView); - Assert.Equal (container1.Subviews.Count, container2.Subviews.Count); - container2.Dispose (); - - Assert.Empty (container1.Subviews); - Assert.Empty (container2.Subviews); - Assert.Equal (1, count); - Assert.Null (view.SuperView); - - container1.Dispose (); - - #if DEBUG_IDISPOSABLE - Assert.Empty (Responder.Instances); - #endif - } - - [Fact] - public void Disposing_Event_Notify_All_Subscribers_On_The_Second_Container () - { - #if DEBUG_IDISPOSABLE - // Only clear before because need to test after assert - Responder.Instances.Clear (); - #endif - - var container1 = new View { Id = "Container1" }; - - var view = new View { Id = "View" }; - container1.Add (view); - Assert.Equal (container1, view.SuperView); - Assert.Single (container1.Subviews); - - var container2 = new View { Id = "Container2" }; - var count = 0; - - view.Disposing += View_Disposing; - container2.Add (view); - Assert.Equal (container2, view.SuperView); - - void View_Disposing (object sender, EventArgs e) - { - count++; - Assert.Equal (view, sender); - container2.Remove ((View)sender); - } - - Assert.Equal (container1.Subviews.Count, container2.Subviews.Count); - container1.Dispose (); - - Assert.Empty (container1.Subviews); - Assert.Empty (container2.Subviews); - Assert.Equal (1, count); - Assert.Null (view.SuperView); - - container2.Dispose (); - - #if DEBUG_IDISPOSABLE - Assert.Empty (Responder.Instances); - #endif - } - + [Fact] public void KeyPressed_Handled_True_Cancels_KeyPress () @@ -125,82 +22,6 @@ public class ResponderTests r.Dispose (); } - [Fact] - [TestRespondersDisposed] - public void New_Initializes () - { - var r = new Responder (); - Assert.NotNull (r); - Assert.Equal ("Terminal.Gui.Responder", r.ToString ()); - r.Dispose (); - } - - [Fact] - [TestRespondersDisposed] - public void New_Methods_Return_False () - { - var r = new View (); - - //Assert.False (r.OnKeyDown (new KeyEventArgs () { Key = Key.Unknown })); - Assert.False (r.NewKeyDownEvent (new Key { KeyCode = KeyCode.Null })); - Assert.False (r.NewKeyDownEvent (new Key { KeyCode = KeyCode.Null })); - Assert.False (r.NewMouseEvent (new MouseEventArgs { Flags = MouseFlags.AllEvents })); - - var v = new View (); - //Assert.False (r.OnEnter (v)); - v.Dispose (); - - v = new View (); - //Assert.False (r.OnLeave (v)); - v.Dispose (); - - r.Dispose (); - } - - [Fact] - public void Responder_Not_Notifying_Dispose () - { - // Only clear before because need to test after assert - #if DEBUG_IDISPOSABLE - Responder.Instances.Clear (); - #endif - var container1 = new View { Id = "Container1" }; - - var view = new View { Id = "View" }; - container1.Add (view); - Assert.Equal (container1, view.SuperView); - - Assert.Single (container1.Subviews); - - var container2 = new View { Id = "Container2" }; - - container2.Add (view); - Assert.Equal (container2, view.SuperView); - Assert.Equal (container1.Subviews.Count, container2.Subviews.Count); - container1.Dispose (); - - Assert.Empty (container1.Subviews); - Assert.NotEmpty (container2.Subviews); - Assert.Single (container2.Subviews); - Assert.Null (view.SuperView); - - // Trying access disposed properties - #if DEBUG_IDISPOSABLE - Assert.True (container2.Subviews [0].WasDisposed); - #endif - Assert.False (container2.Subviews [0].CanFocus); - Assert.Null (container2.Subviews [0].Margin); - Assert.Null (container2.Subviews [0].Border); - Assert.Null (container2.Subviews [0].Padding); - Assert.Null (view.SuperView); - - container2.Dispose (); - - #if DEBUG_IDISPOSABLE - Assert.Empty (Responder.Instances); - #endif - } - public class DerivedView : View { protected override bool OnKeyDown (Key keyEvent) { return true; } diff --git a/UnitTests/TestHelpers.cs b/UnitTests/TestHelpers.cs index 27ebc377c..1b841ee65 100644 --- a/UnitTests/TestHelpers.cs +++ b/UnitTests/TestHelpers.cs @@ -86,13 +86,13 @@ public class AutoInitShutdownAttribute : BeforeAfterTestAttribute Application.Shutdown (); #if DEBUG_IDISPOSABLE - if (Responder.Instances.Count == 0) + if (View.Instances.Count == 0) { - Assert.Empty (Responder.Instances); + Assert.Empty (View.Instances); } else { - Responder.Instances.Clear (); + View.Instances.Clear (); } #endif } @@ -103,7 +103,7 @@ public class AutoInitShutdownAttribute : BeforeAfterTestAttribute //finally { #if DEBUG_IDISPOSABLE - Responder.Instances.Clear (); + View.Instances.Clear (); Application.ResetState (true); #endif } @@ -126,13 +126,13 @@ public class AutoInitShutdownAttribute : BeforeAfterTestAttribute #if DEBUG_IDISPOSABLE // Clear out any lingering Responder instances from previous tests - if (Responder.Instances.Count == 0) + if (View.Instances.Count == 0) { - Assert.Empty (Responder.Instances); + Assert.Empty (View.Instances); } else { - Responder.Instances.Clear (); + View.Instances.Clear (); } #endif Application.Init ((ConsoleDriver)Activator.CreateInstance (_driverType)); @@ -160,7 +160,7 @@ public class TestRespondersDisposed : BeforeAfterTestAttribute base.After (methodUnderTest); #if DEBUG_IDISPOSABLE - Assert.Empty (Responder.Instances); + Assert.Empty (View.Instances); #endif } @@ -172,8 +172,8 @@ public class TestRespondersDisposed : BeforeAfterTestAttribute #if DEBUG_IDISPOSABLE // Clear out any lingering Responder instances from previous tests - Responder.Instances.Clear (); - Assert.Empty (Responder.Instances); + View.Instances.Clear (); + Assert.Empty (View.Instances); #endif } } diff --git a/UnitTests/UICatalog/ScenarioTests.cs b/UnitTests/UICatalog/ScenarioTests.cs index f6fc56d2d..27cc5d661 100644 --- a/UnitTests/UICatalog/ScenarioTests.cs +++ b/UnitTests/UICatalog/ScenarioTests.cs @@ -10,7 +10,7 @@ public class ScenarioTests : TestsAllViews public ScenarioTests (ITestOutputHelper output) { #if DEBUG_IDISPOSABLE - Responder.Instances.Clear (); + View.Instances.Clear (); #endif _output = output; } @@ -68,7 +68,7 @@ public class ScenarioTests : TestsAllViews Assert.True (shutdown); #if DEBUG_IDISPOSABLE - Assert.Empty (Responder.Instances); + Assert.Empty (View.Instances); #endif lock (_timeoutLock) @@ -836,7 +836,7 @@ public class ScenarioTests : TestsAllViews ConfigurationManager.Reset (); #if DEBUG_IDISPOSABLE - Assert.Empty (Responder.Instances); + Assert.Empty (View.Instances); #endif } diff --git a/UnitTests/View/Layout/Dim.Tests.cs b/UnitTests/View/Layout/Dim.Tests.cs index 7b59f8132..9734de3c8 100644 --- a/UnitTests/View/Layout/Dim.Tests.cs +++ b/UnitTests/View/Layout/Dim.Tests.cs @@ -470,8 +470,8 @@ public class DimTests #if DEBUG_IDISPOSABLE // HACK: Force clean up of Responders to avoid having to Dispose all the Views created above. - Responder.Instances.Clear (); - Assert.Empty (Responder.Instances); + View.Instances.Clear (); + Assert.Empty (View.Instances); #endif } diff --git a/UnitTests/View/Layout/Pos.ViewTests.cs b/UnitTests/View/Layout/Pos.ViewTests.cs index 8ad4efb07..b97ae2111 100644 --- a/UnitTests/View/Layout/Pos.ViewTests.cs +++ b/UnitTests/View/Layout/Pos.ViewTests.cs @@ -249,7 +249,7 @@ public class PosViewTests (ITestOutputHelper output) #if DEBUG_IDISPOSABLE // HACK: Force clean up of Responders to avoid having to Dispose all the Views created above. - Responder.Instances.Clear (); + View.Instances.Clear (); #endif } diff --git a/UnitTests/View/ViewTests.cs b/UnitTests/View/ViewTests.cs index a0175bc3b..6595cbc14 100644 --- a/UnitTests/View/ViewTests.cs +++ b/UnitTests/View/ViewTests.cs @@ -6,6 +6,155 @@ namespace Terminal.Gui.ViewTests; public class ViewTests (ITestOutputHelper output) { + // Generic lifetime (IDisposable) tests + [Fact] + [TestRespondersDisposed] + public void Dispose_Works () + { + var r = new View (); +#if DEBUG_IDISPOSABLE + Assert.Single (View.Instances); +#endif + + r.Dispose (); +#if DEBUG_IDISPOSABLE + Assert.Empty (View.Instances); +#endif + } + + [Fact] + public void Disposing_Event_Notify_All_Subscribers_On_The_First_Container () + { +#if DEBUG_IDISPOSABLE + // Only clear before because need to test after assert + View.Instances.Clear (); +#endif + + var container1 = new View { Id = "Container1" }; + var count = 0; + + var view = new View { Id = "View" }; + view.Disposing += View_Disposing; + container1.Add (view); + Assert.Equal (container1, view.SuperView); + + void View_Disposing (object sender, EventArgs e) + { + count++; + Assert.Equal (view, sender); + container1.Remove ((View)sender); + } + + Assert.Single (container1.Subviews); + + var container2 = new View { Id = "Container2" }; + + container2.Add (view); + Assert.Equal (container2, view.SuperView); + Assert.Equal (container1.Subviews.Count, container2.Subviews.Count); + container2.Dispose (); + + Assert.Empty (container1.Subviews); + Assert.Empty (container2.Subviews); + Assert.Equal (1, count); + Assert.Null (view.SuperView); + + container1.Dispose (); + +#if DEBUG_IDISPOSABLE + Assert.Empty (View.Instances); +#endif + } + + [Fact] + public void Disposing_Event_Notify_All_Subscribers_On_The_Second_Container () + { +#if DEBUG_IDISPOSABLE + // Only clear before because need to test after assert + View.Instances.Clear (); +#endif + + var container1 = new View { Id = "Container1" }; + + var view = new View { Id = "View" }; + container1.Add (view); + Assert.Equal (container1, view.SuperView); + Assert.Single (container1.Subviews); + + var container2 = new View { Id = "Container2" }; + var count = 0; + + view.Disposing += View_Disposing; + container2.Add (view); + Assert.Equal (container2, view.SuperView); + + void View_Disposing (object sender, EventArgs e) + { + count++; + Assert.Equal (view, sender); + container2.Remove ((View)sender); + } + + Assert.Equal (container1.Subviews.Count, container2.Subviews.Count); + container1.Dispose (); + + Assert.Empty (container1.Subviews); + Assert.Empty (container2.Subviews); + Assert.Equal (1, count); + Assert.Null (view.SuperView); + + container2.Dispose (); + +#if DEBUG_IDISPOSABLE + Assert.Empty (View.Instances); +#endif + } + + + [Fact] + public void Not_Notifying_Dispose () + { + // Only clear before because need to test after assert +#if DEBUG_IDISPOSABLE + View.Instances.Clear (); +#endif + var container1 = new View { Id = "Container1" }; + + var view = new View { Id = "View" }; + container1.Add (view); + Assert.Equal (container1, view.SuperView); + + Assert.Single (container1.Subviews); + + var container2 = new View { Id = "Container2" }; + + container2.Add (view); + Assert.Equal (container2, view.SuperView); + Assert.Equal (container1.Subviews.Count, container2.Subviews.Count); + container1.Dispose (); + + Assert.Empty (container1.Subviews); + Assert.NotEmpty (container2.Subviews); + Assert.Single (container2.Subviews); + Assert.Null (view.SuperView); + + // Trying access disposed properties +#if DEBUG_IDISPOSABLE + Assert.True (container2.Subviews [0].WasDisposed); +#endif + Assert.False (container2.Subviews [0].CanFocus); + Assert.Null (container2.Subviews [0].Margin); + Assert.Null (container2.Subviews [0].Border); + Assert.Null (container2.Subviews [0].Padding); + Assert.Null (view.SuperView); + + container2.Dispose (); + +#if DEBUG_IDISPOSABLE + Assert.Empty (View.Instances); +#endif + } + [Fact] [AutoInitShutdown] public void Clear_Viewport_Can_Use_Driver_AddRune_Or_AddStr_Methods () @@ -428,7 +577,7 @@ At 0,0 Assert.NotNull (view.Padding); #if DEBUG_IDISPOSABLE - Assert.Equal (4, Responder.Instances.Count); + Assert.Equal (4, View.Instances.Count); #endif view.Dispose (); @@ -948,7 +1097,7 @@ At 0,0 super.Dispose (); #if DEBUG_IDISPOSABLE - Assert.Empty (Responder.Instances); + Assert.Empty (View.Instances); #endif // Default Constructor