Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #2913 - remove Disposed views from Responder.instances when Debugging #2914

Merged
merged 9 commits into from
Oct 25, 2023
56 changes: 15 additions & 41 deletions Terminal.Gui/Core/PosDim.cs
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,6 @@ public static Pos Percent (float n)
return new PosFactor (n / 100);
}

static PosAnchorEnd endNoMargin;

internal class PosAnchorEnd : Pos {
int n;

Expand All @@ -137,6 +135,10 @@ public override string ToString ()
{
return $"Pos.AnchorEnd(margin={n})";
}

public override int GetHashCode () => n.GetHashCode ();

public override bool Equals (object other) => other is PosAnchorEnd anchorEnd && anchorEnd.n == n;
}

/// <summary>
Expand All @@ -158,11 +160,6 @@ public static Pos AnchorEnd (int margin = 0)
if (margin < 0)
throw new ArgumentException ("Margin must be positive");

if (margin == 0) {
if (endNoMargin == null)
endNoMargin = new PosAnchorEnd (0);
return endNoMargin;
}
return new PosAnchorEnd (margin);
}

Expand All @@ -178,8 +175,6 @@ public override string ToString ()
}
}

static PosCenter pCenter;

/// <summary>
/// Returns a <see cref="Pos"/> object that can be used to center the <see cref="View"/>
/// </summary>
Expand All @@ -198,9 +193,7 @@ public override string ToString ()
/// </example>
public static Pos Center ()
{
if (pCenter == null)
pCenter = new PosCenter ();
return pCenter;
return new PosCenter ();
}

internal class PosAbsolute : Pos {
Expand Down Expand Up @@ -269,8 +262,6 @@ public override string ToString ()

}

static PosCombine posCombine;

/// <summary>
/// Adds a <see cref="Terminal.Gui.Pos"/> to a <see cref="Terminal.Gui.Pos"/>, yielding a new <see cref="Pos"/>.
/// </summary>
Expand All @@ -280,12 +271,11 @@ public override string ToString ()
public static Pos operator + (Pos left, Pos right)
{
if (left is PosAbsolute && right is PosAbsolute) {
posCombine = null;
return new PosAbsolute (left.Anchor (0) + right.Anchor (0));
}
PosCombine newPos = new PosCombine (true, left, right);
SetPosCombine (left, newPos);
return posCombine = newPos;
return newPos;
}

/// <summary>
Expand All @@ -297,21 +287,18 @@ public override string ToString ()
public static Pos operator - (Pos left, Pos right)
{
if (left is PosAbsolute && right is PosAbsolute) {
posCombine = null;
return new PosAbsolute (left.Anchor (0) - right.Anchor (0));
}
PosCombine newPos = new PosCombine (false, left, right);
SetPosCombine (left, newPos);
return posCombine = newPos;
return newPos;
}

static void SetPosCombine (Pos left, PosCombine newPos)
{
if (posCombine?.ToString () != newPos.ToString ()) {
var view = left as PosView;
if (view != null) {
view.Target.SetNeedsLayout ();
}
var view = left as PosView;
if (view != null) {
view.Target.SetNeedsLayout ();
}
}

Expand Down Expand Up @@ -554,20 +541,13 @@ internal override int Anchor (int width)
public override bool Equals (object other) => other is DimFill fill && fill.margin == margin;
}

static DimFill zeroMargin;

/// <summary>
/// Initializes a new instance of the <see cref="Dim"/> class that fills the dimension, but leaves the specified number of colums for a margin.
/// </summary>
/// <returns>The Fill dimension.</returns>
/// <param name="margin">Margin to use.</param>
public static Dim Fill (int margin = 0)
{
if (margin == 0) {
if (zeroMargin == null)
zeroMargin = new DimFill (0);
return zeroMargin;
}
return new DimFill (margin);
}

Expand Down Expand Up @@ -618,8 +598,6 @@ public override string ToString ()

}

static DimCombine dimCombine;

/// <summary>
/// Adds a <see cref="Terminal.Gui.Dim"/> to a <see cref="Terminal.Gui.Dim"/>, yielding a new <see cref="Dim"/>.
/// </summary>
Expand All @@ -629,12 +607,11 @@ public override string ToString ()
public static Dim operator + (Dim left, Dim right)
{
if (left is DimAbsolute && right is DimAbsolute) {
dimCombine = null;
return new DimAbsolute (left.Anchor (0) + right.Anchor (0));
}
DimCombine newDim = new DimCombine (true, left, right);
SetDimCombine (left, newDim);
return dimCombine = newDim;
return newDim;
}

/// <summary>
Expand All @@ -646,21 +623,18 @@ public override string ToString ()
public static Dim operator - (Dim left, Dim right)
{
if (left is DimAbsolute && right is DimAbsolute) {
dimCombine = null;
return new DimAbsolute (left.Anchor (0) - right.Anchor (0));
}
DimCombine newDim = new DimCombine (false, left, right);
SetDimCombine (left, newDim);
return dimCombine = newDim;
return newDim;
}

static void SetDimCombine (Dim left, DimCombine newPos)
{
if (dimCombine?.ToString () != newPos.ToString ()) {
var view = left as DimView;
if (view != null) {
view.Target.SetNeedsLayout ();
}
var view = left as DimView;
if (view != null) {
view.Target.SetNeedsLayout ();
}
}

Expand Down
8 changes: 6 additions & 2 deletions Terminal.Gui/Core/Responder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Reflection;

namespace Terminal.Gui {
Expand Down Expand Up @@ -255,7 +255,7 @@ internal static bool IsOverridden (Responder subclass, string method)
}
return m.GetBaseDefinition ().DeclaringType != m.DeclaringType;
}

/// <summary>
/// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources.
/// </summary>
Expand Down Expand Up @@ -291,6 +291,10 @@ public void Dispose ()
GC.SuppressFinalize (this);
#if DEBUG_IDISPOSABLE
WasDisposed = true;

foreach (var instance in Instances.Where (x => x.WasDisposed).ToList ()) {
Instances.Remove (instance);
}
#endif
}
}
Expand Down
4 changes: 4 additions & 0 deletions Terminal.Gui/Core/View.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2934,6 +2934,10 @@ protected bool OnMouseClick (MouseEventArgs args)
/// <inheritdoc/>
protected override void Dispose (bool disposing)
{
height = null;
width = null;
x = null;
y = null;
for (var i = InternalSubviews.Count - 1; i >= 0; i--) {
var subview = InternalSubviews [i];
Remove (subview);
Expand Down
3 changes: 1 addition & 2 deletions UnitTests/Application/ApplicationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,7 @@ public void Init_Shutdown_Toplevel_Not_Disposed ()
Application.Shutdown ();

#if DEBUG_IDISPOSABLE
Assert.Single (Responder.Instances);
Assert.True (Responder.Instances [0].WasDisposed);
Assert.Empty (Responder.Instances);
#endif
}

Expand Down
1 change: 1 addition & 0 deletions UnitTests/Application/RunStateTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ public void Dispose_Cleans_Up_RunState ()
void Init ()
{
Application.Init (new FakeDriver ());

Assert.NotNull (Application.Driver);
a-usr marked this conversation as resolved.
Show resolved Hide resolved
Assert.NotNull (Application.MainLoop);
Assert.NotNull (SynchronizationContext.Current);
Expand Down
8 changes: 2 additions & 6 deletions UnitTests/TopLevels/MdiTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,7 @@ public void Dispose_Toplevel_IsMdiContainer_False_With_Begin_End ()
Application.Shutdown ();

#if DEBUG_IDISPOSABLE
Assert.Equal (2, Responder.Instances.Count);
Assert.True (Responder.Instances [0].WasDisposed);
Assert.True (Responder.Instances [1].WasDisposed);
Assert.Empty (Responder.Instances);
#endif
}

Expand All @@ -49,9 +47,7 @@ public void Dispose_Toplevel_IsMdiContainer_True_With_Begin ()

Application.Shutdown ();
#if DEBUG_IDISPOSABLE
Assert.Equal (2, Responder.Instances.Count);
Assert.True (Responder.Instances [0].WasDisposed);
Assert.True (Responder.Instances [1].WasDisposed);
Assert.Empty (Responder.Instances);
#endif
}

Expand Down
Loading