Skip to content

Commit 1bd5e37

Browse files
BDisptig
andauthored
Fixes #4391. Weird situation where ForceDriver with args doesn't persists on open scenario (#4395)
* Fixes #4391. Weird situation where ForceDriver with args doesn't persists on open scenario * Prevents change ForceDriver if app it's already initialized and allowing also initialize with driver instead of only by driver name. * Add dispose into FakeDriverBase and reset ForceDriver * Moving test to Application folder * Fix unit test * Remove unnecessary GlobalTestSetup * Add GC.SuppressFinalize * Revert "Add GC.SuppressFinalize" This reverts commit 2bd7cd7. * Reset MouseGrabView * Avoid CI warnings * Add GlobalTestSetup in all test that use Application * Trying to fix unit test * Reverting scope changes * Remove UICatalog testing Run * Force re-run CI test * Fix merge errors * Fix ansi for the red background color code * Fix more ANSI color code unit tests --------- Co-authored-by: Tig <tig@users.noreply.github.com>
1 parent a6258ed commit 1bd5e37

18 files changed

+177
-58
lines changed

Examples/UICatalog/UICatalog.cs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,9 @@ namespace UICatalog;
5555
/// </remarks>
5656
public class UICatalog
5757
{
58-
private static string? _forceDriver = null;
58+
private static string? _forceDriver;
59+
private static string? _uiCatalogDriver;
60+
private static string? _scenarioDriver;
5961

6062
public static string LogFilePath { get; set; } = string.Empty;
6163
public static LoggingLevelSwitch LogLevelSwitch { get; } = new ();
@@ -194,6 +196,8 @@ private static int Main (string [] args)
194196

195197
UICatalogMain (Options);
196198

199+
Debug.Assert (Application.ForceDriver == string.Empty);
200+
197201
return 0;
198202
}
199203

@@ -255,7 +259,9 @@ private static Scenario RunUICatalogTopLevel ()
255259

256260
Application.Init (driverName: _forceDriver);
257261

258-
var top = Application.Run<UICatalogTop> ();
262+
_uiCatalogDriver = Application.Driver!.GetName ();
263+
264+
Toplevel top = Application.Run<UICatalogTop> ();
259265
top.Dispose ();
260266
Application.Shutdown ();
261267
VerifyObjectsWereDisposed ();
@@ -421,6 +427,8 @@ private static void UICatalogMain (UICatalogCommandLineOptions options)
421427
Application.InitializedChanged += ApplicationOnInitializedChanged;
422428
#endif
423429

430+
Application.ForceDriver = _forceDriver;
431+
424432
scenario.Main ();
425433
scenario.Dispose ();
426434

@@ -439,6 +447,8 @@ void ApplicationOnInitializedChanged (object? sender, EventArgs<bool> e)
439447
if (e.Value)
440448
{
441449
sw.Start ();
450+
_scenarioDriver = Application.Driver!.GetName ();
451+
Debug.Assert (_scenarioDriver == _uiCatalogDriver);
442452
}
443453
else
444454
{

Terminal.Gui/App/Application.Driver.cs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,21 @@ public static bool Force16Colors
2828
public static string ForceDriver
2929
{
3030
get => ApplicationImpl.Instance.ForceDriver;
31-
set => ApplicationImpl.Instance.ForceDriver = value;
31+
set
32+
{
33+
if (!string.IsNullOrEmpty (ApplicationImpl.Instance.ForceDriver) && value != Driver?.GetName ())
34+
{
35+
// ForceDriver cannot be changed if it has a valid value
36+
return;
37+
}
38+
39+
if (ApplicationImpl.Instance.Initialized && value != Driver?.GetName ())
40+
{
41+
throw new InvalidOperationException ($"The {nameof (ForceDriver)} can only be set before initialized.");
42+
}
43+
44+
ApplicationImpl.Instance.ForceDriver = value;
45+
}
3246
}
3347

3448
/// <inheritdoc cref="IApplication.Sixel"/>

Terminal.Gui/App/ApplicationImpl.Driver.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ private void CreateDriver (string? driverName)
3434
bool factoryIsFake = _componentFactory is IComponentFactory<ConsoleKeyInfo>;
3535

3636
// Then check driverName
37-
bool nameIsWindows = driverName?.Contains ("win", StringComparison.OrdinalIgnoreCase) ?? false;
37+
bool nameIsWindows = driverName?.Contains ("windows", StringComparison.OrdinalIgnoreCase) ?? false;
3838
bool nameIsDotNet = driverName?.Contains ("dotnet", StringComparison.OrdinalIgnoreCase) ?? false;
3939
bool nameIsUnix = driverName?.Contains ("unix", StringComparison.OrdinalIgnoreCase) ?? false;
4040
bool nameIsFake = driverName?.Contains ("fake", StringComparison.OrdinalIgnoreCase) ?? false;

Terminal.Gui/App/ApplicationImpl.Lifecycle.cs

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ public void Init (string? driverName = null)
3333
_driverName = ForceDriver;
3434
}
3535

36-
// Debug.Assert (Navigation is null);
37-
// Navigation = new ();
36+
// Debug.Assert (Navigation is null);
37+
// Navigation = new ();
3838

3939
//Debug.Assert (Popover is null);
4040
//Popover = new ();
@@ -62,7 +62,7 @@ public void Init (string? driverName = null)
6262
_keyboard.PrevTabGroupKey = existingPrevTabGroupKey;
6363
}
6464

65-
CreateDriver (driverName ?? _driverName);
65+
CreateDriver (_driverName);
6666
Screen = Driver!.Screen;
6767
Initialized = true;
6868

@@ -145,9 +145,13 @@ private static void AssertNoEventSubscribers (string eventName, Delegate? eventD
145145
}
146146
#endif
147147

148+
private bool _isResetingState;
149+
148150
/// <inheritdoc/>
149151
public void ResetState (bool ignoreDisposed = false)
150152
{
153+
_isResetingState = true;
154+
151155
// Shutdown is the bookend for Init. As such it needs to clean up all resources
152156
// Init created. Apps that do any threading will need to code defensively for this.
153157
// e.g. see Issue #537
@@ -231,7 +235,14 @@ public void ResetState (bool ignoreDisposed = false)
231235
// === 9. Clear graphics ===
232236
Sixel.Clear ();
233237

234-
// === 10. Reset synchronization context ===
238+
// === 10. Reset ForceDriver ===
239+
// Note: ForceDriver and Force16Colors are reset
240+
// If they need to persist across Init/Shutdown cycles
241+
// then the user of the library should manage that state
242+
Force16Colors = false;
243+
ForceDriver = string.Empty;
244+
245+
// === 11. Reset synchronization context ===
235246
// IMPORTANT: Always reset sync context, even if not initialized
236247
// This ensures cleanup works correctly even if Shutdown is called without Init
237248
// Reset synchronization context to allow the user to run async/await,
@@ -240,8 +251,7 @@ public void ResetState (bool ignoreDisposed = false)
240251
// (https://github.com/gui-cs/Terminal.Gui/issues/1084).
241252
SynchronizationContext.SetSynchronizationContext (null);
242253

243-
// Note: ForceDriver and Force16Colors are NOT reset;
244-
// they need to persist across Init/Shutdown cycles
254+
_isResetingState = false;
245255
}
246256

247257
/// <summary>

Terminal.Gui/App/Mouse/MouseImpl.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,7 @@ public void ResetState ()
254254
// Do not clear LastMousePosition; Popover's require it to stay set with last mouse pos.
255255
CachedViewsUnderMouse.Clear ();
256256
MouseEvent = null;
257+
MouseGrabView = null;
257258
}
258259

259260
// Mouse grab functionality merged from MouseGrabHandler

Tests/UnitTests/FakeDriverBase.cs

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ namespace UnitTests;
44
/// Enables tests to create a FakeDriver for testing purposes.
55
/// </summary>
66
[Collection ("Global Test Setup")]
7-
public abstract class FakeDriverBase
7+
public abstract class FakeDriverBase : IDisposable
88
{
99
/// <summary>
1010
/// Creates a new FakeDriver instance with the specified buffer size.
@@ -19,14 +19,20 @@ protected static IDriver CreateFakeDriver (int width = 80, int height = 25)
1919
var output = new FakeOutput ();
2020

2121
DriverImpl driver = new (
22-
new FakeInputProcessor (null),
23-
new OutputBufferImpl (),
24-
output,
25-
new AnsiRequestScheduler (new AnsiResponseParser ()),
26-
new SizeMonitorImpl (output));
22+
new FakeInputProcessor (null),
23+
new OutputBufferImpl (),
24+
output,
25+
new AnsiRequestScheduler (new AnsiResponseParser ()),
26+
new SizeMonitorImpl (output));
2727

2828
driver.SetScreenSize (width, height);
2929

3030
return driver;
3131
}
32+
33+
/// <inheritdoc />
34+
public void Dispose ()
35+
{
36+
Application.ResetState (true);
37+
}
3238
}

Tests/UnitTests/TestsAllViews.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
#nullable enable
22
using System.Reflection;
3-
using System.Drawing;
43

54
namespace UnitTests;
65

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
using UnitTests;
2+
3+
namespace UnitTests_Parallelizable.ApplicationTests;
4+
5+
public class ApplicationForceDriverTests : FakeDriverBase
6+
{
7+
[Fact]
8+
public void ForceDriver_Does_Not_Changes_If_It_Has_Valid_Value ()
9+
{
10+
Assert.False (Application.Initialized);
11+
Assert.Null (Application.Driver);
12+
Assert.Equal (string.Empty, Application.ForceDriver);
13+
14+
Application.ForceDriver = "fake";
15+
Assert.Equal ("fake", Application.ForceDriver);
16+
17+
Application.ForceDriver = "dotnet";
18+
Assert.Equal ("fake", Application.ForceDriver);
19+
}
20+
21+
[Fact]
22+
public void ForceDriver_Throws_If_Initialized_Changed_To_Another_Value ()
23+
{
24+
IDriver driver = CreateFakeDriver ();
25+
26+
Assert.False (Application.Initialized);
27+
Assert.Null (Application.Driver);
28+
Assert.Equal (string.Empty, Application.ForceDriver);
29+
30+
Application.Init (driverName: "fake");
31+
Assert.True (Application.Initialized);
32+
Assert.NotNull (Application.Driver);
33+
Assert.Equal ("fake", Application.Driver.GetName ());
34+
Assert.Equal (string.Empty, Application.ForceDriver);
35+
36+
Assert.Throws<InvalidOperationException> (() => Application.ForceDriver = "dotnet");
37+
38+
Application.ForceDriver = "fake";
39+
Assert.Equal ("fake", Application.ForceDriver);
40+
}
41+
}

Tests/UnitTestsParallelizable/Application/ApplicationPopoverTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#nullable enable
1+
#nullable enable
22
using Moq;
33
using Terminal.Gui.App;
44

0 commit comments

Comments
 (0)