Bläddra i källkod

Rebased.
Fixed ApplicatonTests.

Tig 1 år sedan
förälder
incheckning
52322a3b73

+ 15 - 1
Terminal.Gui/View/View.cs

@@ -503,9 +503,23 @@ public partial class View : Responder, ISupportInitializeNotification
     /// <returns></returns>
     /// <returns></returns>
     public override string ToString () { return $"{GetType ().Name}({Id}){Frame}"; }
     public override string ToString () { return $"{GetType ().Name}({Id}){Frame}"; }
 
 
-    /// <inheritdoc/>
+    /// <summary>Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources.</summary>
+    /// <remarks>
+    /// <para>
+    ///     Subviews added to this view via <see cref="Add(View)"/> have their lifetime owned by this view and <see cref="Dispose"/> will
+    ///     dispose them. To prevent this, and have the creator of the Subview instance handle disposal, use <see cref="Remove"/> to remove
+    ///     the subview first.
+    /// </para>
+    /// <para>
+    ///     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.
+    /// </para>
+    /// </remarks>
+    /// <param name="disposing"></param>
     protected override void Dispose (bool disposing)
     protected override void Dispose (bool disposing)
     {
     {
+        // BUGBUG: We should only dispose these objects if disposing == true
         LineCanvas.Dispose ();
         LineCanvas.Dispose ();
 
 
         DisposeAdornments ();
         DisposeAdornments ();

+ 4 - 1
UnitTests/Application/ApplicationTests.cs

@@ -40,10 +40,12 @@ public class ApplicationTests
     public void Begin_Sets_Application_Top_To_Console_Size ()
     public void Begin_Sets_Application_Top_To_Console_Size ()
     {
     {
         Assert.Null (Application.Top);
         Assert.Null (Application.Top);
-        Application.Begin (new ());
+        Toplevel top = new ();
+        Application.Begin (top);
         Assert.Equal (new (0, 0, 80, 25), Application.Top.Frame);
         Assert.Equal (new (0, 0, 80, 25), Application.Top.Frame);
         ((FakeDriver)Application.Driver).SetBufferSize (5, 5);
         ((FakeDriver)Application.Driver).SetBufferSize (5, 5);
         Assert.Equal (new (0, 0, 5, 5), Application.Top.Frame);
         Assert.Equal (new (0, 0, 5, 5), Application.Top.Frame);
+        top.Dispose ();
     }
     }
 
 
     [Fact]
     [Fact]
@@ -347,6 +349,7 @@ public class ApplicationTests
         Assert.Null (Application.MouseGrabView); // public
         Assert.Null (Application.MouseGrabView); // public
         Assert.Null (Application.WantContinuousButtonPressedView); // public
         Assert.Null (Application.WantContinuousButtonPressedView); // public
         Assert.False (Application.MoveToOverlappedChild (Application.Top));
         Assert.False (Application.MoveToOverlappedChild (Application.Top));
+        Application.Top.Dispose ();
     }
     }
 
 
     // Invoke Tests
     // Invoke Tests

+ 5 - 0
UnitTests/Application/KeyboardTests.cs

@@ -172,6 +172,7 @@ public class KeyboardTests
         Assert.True (win2.CanFocus);
         Assert.True (win2.CanFocus);
         Assert.True (win2.HasFocus);
         Assert.True (win2.HasFocus);
         Assert.Equal ("win2", ((Window)top.Subviews [^1]).Title);
         Assert.Equal ("win2", ((Window)top.Subviews [^1]).Title);
+        top.Dispose ();
     }
     }
 
 
     [Fact]
     [Fact]
@@ -224,6 +225,7 @@ public class KeyboardTests
         Assert.True (win2.CanFocus);
         Assert.True (win2.CanFocus);
         Assert.False (win2.HasFocus);
         Assert.False (win2.HasFocus);
         Assert.Equal ("win", ((Window)top.Subviews [^1]).Title);
         Assert.Equal ("win", ((Window)top.Subviews [^1]).Title);
+        top.Dispose ();
     }
     }
 
 
     [Fact]
     [Fact]
@@ -358,6 +360,7 @@ public class KeyboardTests
         Assert.True (view.ApplicationCommand);
         Assert.True (view.ApplicationCommand);
         Assert.True (view.HotKeyCommand);
         Assert.True (view.HotKeyCommand);
         Assert.False (view.FocusedCommand);
         Assert.False (view.FocusedCommand);
+        top.Dispose ();
     }
     }
 
 
     [Fact]
     [Fact]
@@ -385,6 +388,7 @@ public class KeyboardTests
         Assert.False (view.ApplicationCommand);
         Assert.False (view.ApplicationCommand);
         Assert.False (view.HotKeyCommand);
         Assert.False (view.HotKeyCommand);
         Assert.False (view.FocusedCommand);
         Assert.False (view.FocusedCommand);
+        top.Dispose ();
     }
     }
 
 
     [Fact]
     [Fact]
@@ -423,6 +427,7 @@ public class KeyboardTests
 
 
         // Reset the QuitKey to avoid throws errors on another tests
         // Reset the QuitKey to avoid throws errors on another tests
         Application.QuitKey = Key.Q.WithCtrl;
         Application.QuitKey = Key.Q.WithCtrl;
+        top.Dispose ();
     }
     }
 
 
     // test Application key Bindings
     // test Application key Bindings

+ 2 - 0
UnitTests/Application/MainLoopTests.cs

@@ -640,6 +640,7 @@ public class MainLoopTests
         await task; // Propagate exception if any occurred
         await task; // Propagate exception if any occurred
 
 
         Assert.Equal (numIncrements * numPasses, tbCounter);
         Assert.Equal (numIncrements * numPasses, tbCounter);
+        top.Dispose ();
     }
     }
 
 
     [Theory]
     [Theory]
@@ -714,6 +715,7 @@ public class MainLoopTests
                                  };
                                  };
 
 
         Application.Run (top);
         Application.Run (top);
+        top.Dispose ();
 
 
         Assert.True (taskCompleted);
         Assert.True (taskCompleted);
         Assert.Equal (clickMe, btn.Text);
         Assert.Equal (clickMe, btn.Text);

+ 4 - 0
UnitTests/Application/MouseTests.cs

@@ -130,6 +130,7 @@ public class MouseTests
 
 
         Application.OnMouseEvent (mouseEvent);
         Application.OnMouseEvent (mouseEvent);
         Assert.Equal (expectedClicked, clicked);
         Assert.Equal (expectedClicked, clicked);
+        top.Dispose ();
     }
     }
 
 
     /// <summary>
     /// <summary>
@@ -224,6 +225,7 @@ public class MouseTests
 
 
         Application.OnMouseEvent (mouseEvent);
         Application.OnMouseEvent (mouseEvent);
         Assert.Equal (expectedClicked, clicked);
         Assert.Equal (expectedClicked, clicked);
+        top.Dispose ();
     }
     }
 
 
     #endregion mouse coordinate tests
     #endregion mouse coordinate tests
@@ -290,6 +292,7 @@ public class MouseTests
                                  };
                                  };
 
 
         Application.Run (top);
         Application.Run (top);
+        top.Dispose ();
     }
     }
 
 
     [Fact]
     [Fact]
@@ -396,6 +399,7 @@ public class MouseTests
         Application.OnMouseEvent (new () { Position = new (0, 0), Flags = MouseFlags.Button1Pressed });
         Application.OnMouseEvent (new () { Position = new (0, 0), Flags = MouseFlags.Button1Pressed });
         Assert.Null (Application.MouseGrabView);
         Assert.Null (Application.MouseGrabView);
         Assert.Equal (0, count);
         Assert.Equal (0, count);
+        top.Dispose ();
     }
     }
     #endregion
     #endregion
 }
 }

+ 1 - 1
UnitTests/TestHelpers.cs

@@ -77,7 +77,7 @@ public class AutoInitShutdownAttribute : BeforeAfterTestAttribute
         if (AutoInit)
         if (AutoInit)
         {
         {
             // TODO: This Dispose call is here until all unit tests that don't correctly dispose Toplevel's they create are fixed.
             // TODO: This Dispose call is here until all unit tests that don't correctly dispose Toplevel's they create are fixed.
-            Application.Top?.Dispose ();
+            //Application.Top?.Dispose ();
             Application.Shutdown ();
             Application.Shutdown ();
 #if DEBUG_IDISPOSABLE
 #if DEBUG_IDISPOSABLE
             if (Responder.Instances.Count == 0)
             if (Responder.Instances.Count == 0)

+ 6 - 4
docfx/docs/newinv2.md

@@ -19,10 +19,12 @@ Apps built with Terminal.Gui now feel modern thanks to these improvements:
 The entire library has been reviewed and simplified. As a result, the API is more consistent and uses modern .NET API standards (e.g. for events). This refactoring resulted in the removal of thousands of lines of code, better unit tests, and higher performance than v1. See [Simplified API](overview.md#simplified-api) for details.
 The entire library has been reviewed and simplified. As a result, the API is more consistent and uses modern .NET API standards (e.g. for events). This refactoring resulted in the removal of thousands of lines of code, better unit tests, and higher performance than v1. See [Simplified API](overview.md#simplified-api) for details.
 
 
 ## `View` Improvements
 ## `View` Improvements
-
-* *Life Cycle Management* - 
-  * In v1, `View` was derived from `Responder` which supported `IDisposable`. In v2, `Responder` has been removed and `View` is the base-class supporting `IDisposable`. 
-  * `Application.Init` no longer automatically creates a toplevel or sets `Applicaton.Top`; app developers must explicitly create the toplevel view and pass it to `Appliation.Run` (or use `Application.Run<myTopLevel>`). Developers are responsible for calling `Dispose` on any toplevel they create before exiting. 
+* *View Lifetime Management is Now Deterministic* - In v1 the rules ofr lifetime management of `View` objects was unclear and led to non-dterministic behavior and hard to diagnose bugs. This was particularly acute in the behavior of `Application.Run`. In v2, the rules are clear and the code and unit test infrastructure tries to enforce them. 
+  * `View` and all subclasses support `IDisposable` and must be disposed (by calling `view.Dispose ()`) by whatever code owns the instance when the instance is longer needed. 
+  * To simplify programming, any `View` added as a Subview another `View` will have it's lifecycle owned by the Superview; when a `View` is disposed, it will call `Dispose` on all the items in the `Subviews` property. Note this behavior is the same as it was in v1, just clarified.
+  * In v1, `Application.End` called `Dispose ()` on `Application.Top` (via `Runstate.Toplevel`). This was incorrect as it meant that after `Application.Run` returned, `Application.Top` had been disposed, and any code that wanted to interogate the results of `Run` by accessing `Application.Top` only worked by accident. This is because GC had not actually happened; if it had the application would have crashed. In v2 `Application.End` does NOT call `Dispose`, and it is the caller to `Application.Run` who is responsible for disposing the `Toplevel` that was either passed to `Application.Run (View)` or created by `Application.Run<T> ()`.
+	* Any code that creates a `Toplevel`, either by using `top = new()` or by calling either `top = Application.Run ()` or `top = ApplicationRun<T>()` must call `top.Dispose` when complete.
+  	*  The exception to this is if `top` is passed to `myView.Add(top)` making it a subview of `myView`. This is because the semantics of `Add` are that the `myView` takes over responsibility for the subviews lifetimes. Of course, if someone calls `myView.Remove(top)` to remove said subview, they then re-take responsbility for `top`'s lifetime and they must call `top.Dispose`.
 * New! *Adornments* -  Adornments are a special form of View that appear outside the `Viewport`: `Margin`, `Border`, and `Padding`.
 * New! *Adornments* -  Adornments are a special form of View that appear outside the `Viewport`: `Margin`, `Border`, and `Padding`.
 * New! *Built-in Scrolling/Virtual Content Area* - In v1, to have a view a user could scroll required either a bespoke scrolling implementation, inheriting from `ScrollView`, or managing the complexity of `ScrollBarView` directly. In v2, the base-View class supports scrolling inherently. The area of a view visible to the user at a given moment was previously a rectangle called `Bounds`. `Bounds.Location` was always `Point.Empty`. In v2 the visible area is a rectangle called `Viewport` which is a protal into the Views content, which can be bigger (or smaller) than the area visible to the user. Causing a view to scroll is as simple as changing `View.Viewport.Location`. The View's content described by `View.GetContentSize()`. See [Layout](layout.md) for details.
 * New! *Built-in Scrolling/Virtual Content Area* - In v1, to have a view a user could scroll required either a bespoke scrolling implementation, inheriting from `ScrollView`, or managing the complexity of `ScrollBarView` directly. In v2, the base-View class supports scrolling inherently. The area of a view visible to the user at a given moment was previously a rectangle called `Bounds`. `Bounds.Location` was always `Point.Empty`. In v2 the visible area is a rectangle called `Viewport` which is a protal into the Views content, which can be bigger (or smaller) than the area visible to the user. Causing a view to scroll is as simple as changing `View.Viewport.Location`. The View's content described by `View.GetContentSize()`. See [Layout](layout.md) for details.
 * New! *`Dim.Auto`* - Automatically sizes the view to fitthe view's Text, SubViews, or ContentArea.
 * New! *`Dim.Auto`* - Automatically sizes the view to fitthe view's Text, SubViews, or ContentArea.