diff --git a/Examples/UICatalog/Scenarios/Menus.cs b/Examples/UICatalog/Scenarios/Menus.cs index 81fee8091..b21eb3ea2 100644 --- a/Examples/UICatalog/Scenarios/Menus.cs +++ b/Examples/UICatalog/Scenarios/Menus.cs @@ -38,13 +38,12 @@ public class Menus : Scenario SchemeName = "Runnable", Source = new ListWrapper (eventSource) }; - eventLog.Border!.Thickness = new (0, 1, 0, 0); + eventLog.Border!.Thickness = new Thickness (0, 1, 0, 0); MenuHost menuHostView = new () { Id = "menuHostView", Title = $"Menu Host - Use {PopoverMenu.DefaultKey} for Popover Menu", - X = 0, Y = 0, Width = Dim.Fill ()! - Dim.Width (eventLog), @@ -72,8 +71,9 @@ public class Menus : Scenario return; } - Logging.Debug ($"{sender.Id} Accepting: {args.Context?.Source?.Title}"); - eventSource.Add ($"{sender.Id} Accepting: {args.Context?.Source?.Title}: "); + string sourceTitle = args.Context?.Source.ToIdentifyingString () ?? "(null)"; + Logging.Debug ($"{sender.Id} Accepting: {sourceTitle}"); + eventSource.Add ($"{sender.Id} Accepting: {sourceTitle}: "); eventLog.MoveDown (); }; @@ -84,8 +84,14 @@ public class Menus : Scenario return; } - Logging.Debug ($"{sender.Id} Accepted: {args.Context?.Source?.Text}"); - eventSource.Add ($"{sender.Id} Accepted: {args.Context?.Source?.Text}: "); + var sourceText = "(null)"; + + if (args.Context?.TryGetSource (out View? sourceView) == true) + { + sourceText = sourceView.Text; + } + Logging.Debug ($"{sender.Id} Accepted: {sourceText}"); + eventSource.Add ($"{sender.Id} Accepted: {sourceText}: "); eventLog.MoveDown (); }; @@ -106,8 +112,7 @@ public class Menus : Scenario CanFocus = true; BorderStyle = LineStyle.Dashed; - AddCommand ( - Command.Context, + AddCommand (Command.Context, _ => { ContextMenu?.MakeVisible (); @@ -132,20 +137,9 @@ public class Menus : Scenario //MouseBindings.ReplaceCommands (MouseFlags.LeftButtonClicked, Command.Cancel); - Label lastCommandLabel = new () - { - Title = "_Last Command:", - X = 15, - Y = 10 - }; + Label lastCommandLabel = new () { Title = "_Last Command:", X = 15, Y = 10 }; - View lastCommandText = new () - { - X = Pos.Right (lastCommandLabel) + 1, - Y = Pos.Top (lastCommandLabel), - Height = Dim.Auto (), - Width = Dim.Auto () - }; + View lastCommandText = new () { X = Pos.Right (lastCommandLabel) + 1, Y = Pos.Top (lastCommandLabel), Height = Dim.Auto (), Width = Dim.Auto () }; Add (lastCommandLabel, lastCommandText); @@ -161,8 +155,7 @@ public class Menus : Scenario AddCommand (Command.SaveAs, HandleCommand); HotKeyBindings.Add (Key.A.WithCtrl, Command.SaveAs); - AddCommand ( - Command.Quit, + AddCommand (Command.Quit, _ => { Logging.Debug ("MenuHost Command.Quit - RequestStop"); @@ -190,28 +183,17 @@ public class Menus : Scenario App?.Keyboard.KeyBindings.Remove (Key.F5); App?.Keyboard.KeyBindings.Add (Key.F5, this, Command.Edit); - var menuBar = new MenuBar - { - Title = "MenuHost MenuBar" - }; + var menuBar = new MenuBar { Title = "MenuHost MenuBar" }; MenuHost host = this; menuBar.EnableForDesign (ref host); Add (menuBar); - Label lastAcceptedLabel = new () - { - Title = "Last Accepted:", - X = Pos.Left (lastCommandLabel), - Y = Pos.Bottom (lastCommandLabel) - }; + Label lastAcceptedLabel = new () { Title = "Last Accepted:", X = Pos.Left (lastCommandLabel), Y = Pos.Bottom (lastCommandLabel) }; View lastAcceptedText = new () { - X = Pos.Right (lastAcceptedLabel) + 1, - Y = Pos.Top (lastAcceptedLabel), - Height = Dim.Auto (), - Width = Dim.Auto () + X = Pos.Right (lastAcceptedLabel) + 1, Y = Pos.Top (lastAcceptedLabel), Height = Dim.Auto (), Width = Dim.Auto () }; Add (lastAcceptedLabel, lastAcceptedText); @@ -222,13 +204,11 @@ public class Menus : Scenario // CB. // So that is needed is to mirror the two check boxes. var autoSaveMenuItemCb = menuBar.GetMenuItemsWithTitle ("_Auto Save").FirstOrDefault ()?.CommandView as CheckBox; - Debug.Assert (autoSaveMenuItemCb is not null); + Debug.Assert (autoSaveMenuItemCb is { }); CheckBox autoSaveStatusCb = new () { - Title = "AutoSave Status (MenuItem Binding to F10)", - X = Pos.Left (lastAcceptedLabel), - Y = Pos.Bottom (lastAcceptedLabel) + Title = "AutoSave Status (MenuItem Binding to F10)", X = Pos.Left (lastAcceptedLabel), Y = Pos.Bottom (lastAcceptedLabel) }; autoSaveStatusCb.ValueChanged += (_, _) => { autoSaveMenuItemCb!.Value = autoSaveStatusCb.Value; }; @@ -243,43 +223,42 @@ public class Menus : Scenario // If the user clicks on the MenuItem, Accept will be raised. CheckBox enableOverwriteStatusCb = new () { - Title = "Enable Overwrite (View Binding to Ctrl+W)", - X = Pos.Left (autoSaveStatusCb), - Y = Pos.Bottom (autoSaveStatusCb) + Title = "Enable Overwrite (View Binding to Ctrl+W)", X = Pos.Left (autoSaveStatusCb), Y = Pos.Bottom (autoSaveStatusCb) }; // The source of truth is our status CB; any time it changes, update the menu item var enableOverwriteMenuItemCb = menuBar.GetMenuItemsWithTitle ("Overwrite").FirstOrDefault ()?.CommandView as CheckBox; enableOverwriteStatusCb.ValueChanged += (_, _) => - { - if (enableOverwriteMenuItemCb is not null) - { - enableOverwriteMenuItemCb.Value = enableOverwriteStatusCb.Value; - } - }; + { + if (enableOverwriteMenuItemCb is { }) + { + enableOverwriteMenuItemCb.Value = enableOverwriteStatusCb.Value; + } + }; menuBar.Accepted += (_, args) => { - if (args.Context?.Source is not MenuItem mi || mi.CommandView != enableOverwriteMenuItemCb) + if (!(args.Context?.TryGetSource (out View? sourceView) == true) + || sourceView is not MenuItem mi + || mi.CommandView != enableOverwriteMenuItemCb) { return; } - Logging.Debug ($"menuBar.Accepted: {args.Context.Source?.Title}"); + Logging.Debug ($"menuBar.Accepted: {args.Context?.Source.ToIdentifyingString ()}"); // Set Cancel to true to stop propagation of Accepting to superview args.Handled = true; // Since overwrite uses a MenuItem.Command the menu item CB is the source of truth enableOverwriteStatusCb.Value = ((CheckBox)mi.CommandView).Value; - lastAcceptedText.Text = args.Context?.Source?.Title!; + lastAcceptedText.Text = sourceView.Title; }; HotKeyBindings.Add (Key.W.WithCtrl, Command.EnableOverwrite); - AddCommand ( - Command.EnableOverwrite, + AddCommand (Command.EnableOverwrite, ctx => { // The command was invoked. Toggle the status Cb. @@ -297,41 +276,40 @@ public class Menus : Scenario // If the user clicks on the MenuItem, Accept will be raised. CheckBox editModeStatusCb = new () { - Title = "EditMode (App Binding to F5)", - X = Pos.Left (enableOverwriteStatusCb), - Y = Pos.Bottom (enableOverwriteStatusCb) + Title = "EditMode (App Binding to F5)", X = Pos.Left (enableOverwriteStatusCb), Y = Pos.Bottom (enableOverwriteStatusCb) }; // The source of truth is our status CB; any time it changes, update the menu item var editModeMenuItemCb = menuBar.GetMenuItemsWithTitle ("EditMode").FirstOrDefault ()?.CommandView as CheckBox; editModeStatusCb.ValueChanged += (_, _) => - { - if (editModeMenuItemCb is not null) - { - editModeMenuItemCb.Value = editModeStatusCb.Value; - } - }; + { + if (editModeMenuItemCb is { }) + { + editModeMenuItemCb.Value = editModeStatusCb.Value; + } + }; menuBar.Accepted += (_, args) => { - if (args.Context?.Source is not MenuItem mi || mi.CommandView != editModeMenuItemCb) + if (!(args.Context?.TryGetSource (out View? sourceView) == true) + || sourceView is not MenuItem mi + || mi.CommandView != editModeMenuItemCb) { return; } - Logging.Debug ($"menuBar.Accepted: {args.Context.Source?.Title}"); + Logging.Debug ($"menuBar.Accepted: {args.Context?.Source.ToIdentifyingString ()}"); // Set Cancel to true to stop propagation of Accepting to superview args.Handled = true; // Since overwrite uses a MenuItem.Command the menu item CB is the source of truth editModeMenuItemCb.Value = ((CheckBox)mi.CommandView).Value; - lastAcceptedText.Text = args.Context?.Source?.Title!; + lastAcceptedText.Text = sourceView.Title; }; - AddCommand ( - Command.Edit, + AddCommand (Command.Edit, ctx => { // The command was invoked. Toggle the status Cb. @@ -343,11 +321,7 @@ public class Menus : Scenario Add (editModeStatusCb); // Set up the Context Menu - ContextMenu = new () - { - Title = "ContextMenu", - Id = "ContextMenu" - }; + ContextMenu = new PopoverMenu { Title = "ContextMenu", Id = "ContextMenu" }; ContextMenu.EnableForDesign (ref host); App?.Popover?.Register (ContextMenu); @@ -359,25 +333,22 @@ public class Menus : Scenario // we need to subscribe to the ContextMenu's Accepted event. ContextMenu!.Accepted += (_, args) => { - Logging.Debug ($"ContextMenu.Accepted: {args.Context?.Source?.Title}"); + Logging.Debug ($"ContextMenu.Accepted: {args.Context?.Source.ToIdentifyingString ()}"); // Forward the event to the MenuHost - if (args.Context is not null) + if (args.Context is { }) { //InvokeCommand (args.Context.Command); } }; // Add a button to open the contextmenu - var openBtn = new Button - { - X = Pos.Center (), Y = 4, Text = "_Open Menu", IsDefault = true - }; + var openBtn = new Button { X = Pos.Center (), Y = 4, Text = "_Open Menu", IsDefault = true }; openBtn.Accepting += (_, e) => { e.Handled = true; - Logging.Trace ($"openBtn.Accepting - Sending F9. {e.Context?.Source?.Title}"); + Logging.Trace ($"openBtn.Accepting - Sending F9. {e.Context?.Source.ToIdentifyingString ()}"); NewKeyDownEvent (menuBar.Key); }; @@ -409,7 +380,7 @@ public class Menus : Scenario /// protected override void Dispose (bool disposing) { - if (ContextMenu is not null) + if (ContextMenu is { }) { ContextMenu.Dispose (); ContextMenu = null; @@ -428,22 +399,20 @@ public class Menus : Scenario // Configure Serilog to write logs to a file _logLevelSwitch.MinimumLevel = LogEventLevel.Verbose; - Log.Logger = new LoggerConfiguration () - .MinimumLevel.ControlledBy (_logLevelSwitch) - .Enrich.FromLogContext () // Enables dynamic enrichment - .WriteTo.Debug () - .WriteTo.File ( - _logFilePath, - rollingInterval: RollingInterval.Day, - outputTemplate: "{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} [{Level:u3}] {Message:lj}{NewLine}{Exception}") - .CreateLogger (); + Log.Logger = new LoggerConfiguration ().MinimumLevel.ControlledBy (_logLevelSwitch) + .Enrich.FromLogContext () // Enables dynamic enrichment + .WriteTo.Debug () + .WriteTo.File (_logFilePath, + rollingInterval: RollingInterval.Day, + outputTemplate: + "{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} [{Level:u3}] {Message:lj}{NewLine}{Exception}") + .CreateLogger (); // Create a logger factory compatible with Microsoft.Extensions.Logging using ILoggerFactory loggerFactory = LoggerFactory.Create (builder => { - builder - .AddSerilog (dispose: true) // Integrate Serilog with ILogger - .SetMinimumLevel (LogLevel.Trace); // Set minimum log level + builder.AddSerilog (dispose: true) // Integrate Serilog with ILogger + .SetMinimumLevel (LogLevel.Trace); // Set minimum log level }); // Get an ILogger instance diff --git a/Examples/UICatalog/Scenarios/MouseTester.cs b/Examples/UICatalog/Scenarios/MouseTester.cs index 6411a5c82..51c2e0b96 100644 --- a/Examples/UICatalog/Scenarios/MouseTester.cs +++ b/Examples/UICatalog/Scenarios/MouseTester.cs @@ -268,63 +268,63 @@ public class MouseTester : Scenario demo.Activating += (_, args) => { - commandLogList.Add ($"{args.Context!.Source!.Id}:{args.Context!.Command}"); + commandLogList.Add ($"{args.Context!.Source.ToIdentifyingString()}:{args.Context!.Command}"); commandLog.MoveEnd (); args.Handled = true; }; demo.Accepting += (_, args) => { - commandLogList.Add ($"{args.Context!.Source!.Id}:{args.Context!.Command}"); + commandLogList.Add ($"{args.Context!.Source.ToIdentifyingString()}:{args.Context!.Command}"); commandLog.MoveEnd (); args.Handled = true; }; demo.CommandNotBound += (_, args) => { - commandLogList.Add ($"{args.Context!.Source!.Id}:{args.Context!.Command}"); + commandLogList.Add ($"{args.Context!.Source.ToIdentifyingString()}:{args.Context!.Command}"); commandLog.MoveEnd (); args.Handled = true; }; demoInPadding.Activating += (_, args) => { - commandLogList.Add ($"{args.Context!.Source!.Id}:{args.Context!.Command}"); + commandLogList.Add ($"{args.Context!.Source.ToIdentifyingString()}:{args.Context!.Command}"); commandLog.MoveEnd (); args.Handled = true; }; demoInPadding.Accepting += (_, args) => { - commandLogList.Add ($"{args.Context!.Source!.Id}:{args.Context!.Command}"); + commandLogList.Add ($"{args.Context!.Source.ToIdentifyingString()}:{args.Context!.Command}"); commandLog.MoveEnd (); args.Handled = true; }; sub1.Activating += (_, args) => { - commandLogList.Add ($"{args.Context!.Source!.Id}:{args.Context!.Command}"); + commandLogList.Add ($"{args.Context!.Source.ToIdentifyingString()}:{args.Context!.Command}"); commandLog.MoveEnd (); args.Handled = true; }; sub1.Accepting += (_, args) => { - commandLogList.Add ($"{args.Context!.Source!.Id}:{args.Context!.Command}"); + commandLogList.Add ($"{args.Context!.Source.ToIdentifyingString()}:{args.Context!.Command}"); commandLog.MoveEnd (); args.Handled = true; }; sub2.Activating += (_, args) => { - commandLogList.Add ($"{args.Context!.Source!.Id}:{args.Context!.Command}"); + commandLogList.Add ($"{args.Context!.Source.ToIdentifyingString()}:{args.Context!.Command}"); commandLog.MoveEnd (); args.Handled = true; }; sub2.Accepting += (_, args) => { - commandLogList.Add ($"{args.Context!.Source!.Id}:{args.Context!.Command}"); + commandLogList.Add ($"{args.Context!.Source.ToIdentifyingString()}:{args.Context!.Command}"); commandLog.MoveEnd (); args.Handled = true; }; diff --git a/Examples/UICatalog/UICatalogRunnable.cs b/Examples/UICatalog/UICatalogRunnable.cs index 7acbed55a..da5f3b5ee 100644 --- a/Examples/UICatalog/UICatalogRunnable.cs +++ b/Examples/UICatalog/UICatalogRunnable.cs @@ -265,10 +265,12 @@ public sealed class UICatalogRunnable : Runnable _diagnosticFlagsSelector.Activating += (_, args) => { - _diagnosticFlags = - (ViewDiagnosticFlags)(int)args.Context!.Source! - .Data!; // (ViewDiagnosticFlags)_diagnosticFlagsSelector.Value; - Diagnostics = _diagnosticFlags; + if (args.Context?.TryGetSource (out View? sourceView) == true) + { + _diagnosticFlags = + (ViewDiagnosticFlags)(int)sourceView.Data!; // (ViewDiagnosticFlags)_diagnosticFlagsSelector.Value; + Diagnostics = _diagnosticFlags; + } }; var diagFlagMenuItem = new MenuItem { CommandView = _diagnosticFlagsSelector, HelpText = "View Diagnostics" }; diff --git a/Terminal.Gui/Input/CommandContext.cs b/Terminal.Gui/Input/CommandContext.cs index 621918327..c0e690704 100644 --- a/Terminal.Gui/Input/CommandContext.cs +++ b/Terminal.Gui/Input/CommandContext.cs @@ -14,7 +14,8 @@ /// /// /// -/// . +/// +/// . #pragma warning restore CS1574, CS0419 // XML comment has cref attribute that could not be resolved public record struct CommandContext : ICommandContext { @@ -22,21 +23,24 @@ public record struct CommandContext : ICommandContext /// Initializes a new instance with the specified . /// /// The command being invoked. - /// The view that is the source of the command invocation. + /// A weak reference to the view that is the source of the command invocation. /// The binding that triggered the command, if any. - public CommandContext (Command command, View? source, IInputBinding? binding) + public CommandContext (Command command, WeakReference? source, IInputBinding? binding) { Command = command; Binding = binding; Source = source; } - /// + /// public Command Command { get; set; } - /// - public View? Source { get; set; } + /// + public WeakReference? Source { get; set; } - /// + /// public IInputBinding? Binding { get; set; } + + /// + public override string ToString () => $"{Command} (Source={Source.ToIdentifyingString ()}, Binding={Binding})"; } diff --git a/Terminal.Gui/Input/CommandContextExtensions.cs b/Terminal.Gui/Input/CommandContextExtensions.cs new file mode 100644 index 000000000..72896e548 --- /dev/null +++ b/Terminal.Gui/Input/CommandContextExtensions.cs @@ -0,0 +1,44 @@ +namespace Terminal.Gui.Input; + +/// +/// Extension methods for . +/// +public static class CommandContextExtensions +{ + /// The command context. + extension (ICommandContext? context) + { + /// + /// Tries to get the source from a command context. + /// + /// + /// When this method returns, contains the source View if the context is not null and the source weak reference + /// target is still alive; otherwise, null. + /// + /// + /// if the context is not null and the source weak reference target is still alive; + /// otherwise, . + /// + /// + /// + /// This is a convenience method to simplify accessing the source view from a command context. + /// It combines null-checking the context and retrieving the weak reference target in one call. + /// + /// + /// Example usage: + /// + /// if (commandContext.TryGetSource(out View? view)) + /// { + /// // use view + /// } + /// + /// + /// + public bool TryGetSource (out View? source) + { + source = null; + + return context?.Source?.TryGetTarget (out source) == true; + } + } +} diff --git a/Terminal.Gui/Input/ICommandContext.cs b/Terminal.Gui/Input/ICommandContext.cs index 2ea0550cc..4fb8c8f50 100644 --- a/Terminal.Gui/Input/ICommandContext.cs +++ b/Terminal.Gui/Input/ICommandContext.cs @@ -2,8 +2,7 @@ #pragma warning disable CS1574 // XML comment has cref attribute that could not be resolved /// -/// Describes the context in which a is being invoked. -/// inherits from this interface. +/// Describes the context in which a is being invoked. /// When a is invoked, /// a context object is passed to Command handlers as an reference. /// @@ -18,10 +17,14 @@ public interface ICommandContext public Command Command { get; set; } /// - /// The View that was the source of the command invocation, if any. + /// A weak reference to the View that was the source of the command invocation, if any. /// (e.g. the view the user clicked on or the view that had focus when a key was pressed). + /// Use Source?.TryGetTarget(out View? view) to safely access the source view. /// - public View? Source { get; set; } + /// + /// Uses WeakReference to prevent memory leaks when views are disposed during command propagation. + /// + public WeakReference? Source { get; set; } /// /// The binding that triggered the command. diff --git a/Terminal.Gui/ViewBase/View.Command.cs b/Terminal.Gui/ViewBase/View.Command.cs index 91ba4709e..07a2b7a10 100644 --- a/Terminal.Gui/ViewBase/View.Command.cs +++ b/Terminal.Gui/ViewBase/View.Command.cs @@ -473,7 +473,7 @@ public partial class View // Command APIs _commandImplementations.TryGetValue (Command.NotBound, out implementation); } - return implementation! (new CommandContext { Command = command, Source = this, Binding = binding }); + return implementation! (new CommandContext { Command = command, Source = new WeakReference (this), Binding = binding }); } /// @@ -516,6 +516,6 @@ public partial class View // Command APIs _commandImplementations.TryGetValue (Command.NotBound, out implementation); } - return implementation! (new CommandContext { Command = command, Source = this, Binding = null }); + return implementation! (new CommandContext { Command = command, Source = new WeakReference (this), Binding = null }); } } diff --git a/Terminal.Gui/ViewBase/ViewExtensions.cs b/Terminal.Gui/ViewBase/ViewExtensions.cs new file mode 100644 index 000000000..65b6196ed --- /dev/null +++ b/Terminal.Gui/ViewBase/ViewExtensions.cs @@ -0,0 +1,40 @@ +namespace Terminal.Gui.ViewBase; + +/// +/// Extension methods for to support debugging and logging. +/// +public static class ViewExtensions +{ + /// The view to identify. + extension (View? view) + { + /// + /// Returns a formatted string that identifies the View for debugging/logging purposes. + /// + /// A string identifying the View using Id, Title, Text, or type name. + public string ToIdentifyingString () + { + if (view is null) + { + return "(null)"; + } + + if (!string.IsNullOrEmpty (view.Id)) + { + return $"{view.Id}"; + } + + if (!string.IsNullOrEmpty (view.Title)) + { + return $"(\"{view.Title}\")"; + } + + if (!string.IsNullOrEmpty (view.Text)) + { + return $"(\"{view.Text}\")"; + } + + return view.GetType ().Name; + } + } +} diff --git a/Terminal.Gui/ViewBase/WeakReferenceExtensions.cs b/Terminal.Gui/ViewBase/WeakReferenceExtensions.cs new file mode 100644 index 000000000..b432c1716 --- /dev/null +++ b/Terminal.Gui/ViewBase/WeakReferenceExtensions.cs @@ -0,0 +1,58 @@ +namespace Terminal.Gui.ViewBase; + +/// +/// Extension methods for when T is . +/// +public static class WeakReferenceExtensions +{ + /// The weak reference to format. + extension (WeakReference? weakRef) + { + /// + /// Returns a formatted string representation of the to a View. + /// + /// A string identifying the referenced View, or "(null)" if the reference is null or dead. + public string ToIdentifyingString () + { + if (weakRef is null || !weakRef.TryGetTarget (out View? view)) + { + return "(null)"; + } + + return view.ToIdentifyingString (); + } + + /// + /// Tries to get the source from a . + /// + /// + /// When this method returns, contains the target View if the weak reference is not null and the target is still alive; + /// otherwise, null. + /// + /// + /// if the weak reference is not null and the target is still alive; + /// otherwise, . + /// + /// + /// + /// This is a convenience method to simplify the common pattern of checking a weak reference + /// and retrieving its target. It's particularly useful with . + /// + /// + /// Example usage: + /// + /// if (commandContext.Source.TryGetSource(out View? view)) + /// { + /// // use view + /// } + /// + /// + /// + public bool TryGetSource (out View? source) + { + source = null; + + return weakRef?.TryGetTarget (out source) == true; + } + } +} diff --git a/Terminal.Gui/Views/ComboBox.cs b/Terminal.Gui/Views/ComboBox.cs index 100f6758e..a7753dfa0 100644 --- a/Terminal.Gui/Views/ComboBox.cs +++ b/Terminal.Gui/Views/ComboBox.cs @@ -84,7 +84,7 @@ public class ComboBox : View, IDesignable AddCommand (Command.Accept, ctx => { - if (ctx?.Source == _search) + if (ctx?.TryGetSource (out View? sourceView) == true && sourceView == _search) { return null; } @@ -851,7 +851,7 @@ public class ComboBox : View, IDesignable } else if (isMousePositionValid) { - return RaiseAccepting (new CommandContext (Command.Accept, this, new InputBinding ())) == true; + return RaiseAccepting (new CommandContext (Command.Accept, new WeakReference (this), new InputBinding ())) == true; } else { diff --git a/Terminal.Gui/Views/Dialog.cs b/Terminal.Gui/Views/Dialog.cs index 9196e5b9d..4db8bc3a3 100644 --- a/Terminal.Gui/Views/Dialog.cs +++ b/Terminal.Gui/Views/Dialog.cs @@ -116,13 +116,13 @@ public class Dialog : Dialog /// protected override bool OnAccepting (CommandEventArgs args) { - if (!Buttons.Contains (args.Context?.Source)) + if (!args.Context.TryGetSource (out View? sourceView) || !Buttons.Contains (sourceView)) { return false; } - int buttonIndex = Buttons.IndexOf (args.Context?.Source); - Result = buttonIndex != -1 ? buttonIndex : Buttons.IndexOf (Buttons.FirstOrDefault (b => b.IsDefault)); + int buttonIndex = Array.IndexOf (Buttons, sourceView); + Result = buttonIndex != -1 ? buttonIndex : Array.IndexOf (Buttons, Buttons.FirstOrDefault (b => b.IsDefault)); RequestStop (); return true; diff --git a/Terminal.Gui/Views/DialogTResult.cs b/Terminal.Gui/Views/DialogTResult.cs index 658dd7f46..5e135ea57 100644 --- a/Terminal.Gui/Views/DialogTResult.cs +++ b/Terminal.Gui/Views/DialogTResult.cs @@ -190,12 +190,12 @@ public class Dialog : Runnable, IDesignable /// protected override bool OnAccepting (CommandEventArgs args) { - if (!Buttons.Contains (args.Context?.Source)) + if (!args.Context.TryGetSource (out View? sourceView) || !Buttons.Contains (sourceView)) { return false; } - if (Buttons.FirstOrDefault (v => v is Button { IsDefault: true }) == args.Context?.Source) + if (Buttons.FirstOrDefault (v => v is Button { IsDefault: true }) == sourceView) { // Default button pressed return false; diff --git a/Terminal.Gui/Views/Menu/MenuBar.cs b/Terminal.Gui/Views/Menu/MenuBar.cs index 4d17ddb61..828760063 100644 --- a/Terminal.Gui/Views/Menu/MenuBar.cs +++ b/Terminal.Gui/Views/Menu/MenuBar.cs @@ -451,10 +451,10 @@ public class MenuBar : Menu, IDesignable /// protected override void OnAccepted (CommandEventArgs args) { - // Logging.Debug ($"{Title} ({args.Context?.Source?.Title}) Command: {args.Context?.Command}"); + // Logging.Debug ($"{Title} ({args.Context?.Source.ToIdentifyingString ()}) Command: {args.Context?.Command}"); base.OnAccepted (args); - if (SubViews.OfType ().Contains (args.Context?.Source)) + if (args.Context.TryGetSource (out View? sourceView) && SubViews.OfType ().Contains (sourceView)) { return; } @@ -465,10 +465,10 @@ public class MenuBar : Menu, IDesignable /// protected override bool OnAccepting (CommandEventArgs args) { - // Logging.Debug ($"{Title} ({args.Context?.Source?.Title})"); + // Logging.Debug ($"{Title} ({args.Context?.Source.ToIdentifyingString ()})"); // TODO: Ensure sourceMenuBar is actually one of our bar items - if (Visible && Enabled && args.Context?.Source is MenuBarItem { PopoverMenuOpen: false } sourceMenuBarItem) + if (Visible && Enabled && args.Context.TryGetSource (out View? sourceView) && sourceView is MenuBarItem { PopoverMenuOpen: false } sourceMenuBarItem) { if (!CanFocus) { diff --git a/Terminal.Gui/Views/Menu/PopoverMenu.cs b/Terminal.Gui/Views/Menu/PopoverMenu.cs index 5fb8ace7d..e2d69c6b0 100644 --- a/Terminal.Gui/Views/Menu/PopoverMenu.cs +++ b/Terminal.Gui/Views/Menu/PopoverMenu.cs @@ -619,13 +619,13 @@ public class PopoverMenu : PopoverBaseImpl, IDesignable private void MenuAccepted (object? sender, CommandEventArgs e) { - // Logging.Debug ($"{Title} ({e.Context?.Source?.Title}) Command: {e.Context?.Command}"); + // Logging.Debug ($"{Title} ({e.Context?.Source.ToIdentifyingString ()}) Command: {e.Context?.Command}"); - if (e.Context?.Source is MenuItem { SubMenu: null }) + if (e.Context.TryGetSource (out View? sourceView) && sourceView is MenuItem { SubMenu: null }) { HideAndRemoveSubMenu (_root); } - else if (e.Context?.Source is MenuItem { SubMenu: { } } menuItemWithSubMenu) + else if (e.Context.TryGetSource (out View? sourceView2) && sourceView2 is MenuItem { SubMenu: { } } menuItemWithSubMenu) { ShowSubMenu (menuItemWithSubMenu); } @@ -668,7 +668,7 @@ public class PopoverMenu : PopoverBaseImpl, IDesignable } // Only raise Accepted if the command came from one of our MenuItems - if (GetMenuItemsOfAllSubMenus ().Contains (args.Context?.Source)) + if (args.Context.TryGetSource (out View? sourceView) && GetMenuItemsOfAllSubMenus ().Contains (sourceView)) { // Logging.Debug ($"{Title} - Calling RaiseAccepted {args.Context?.Command}"); RaiseAccepted (args.Context); diff --git a/Terminal.Gui/Views/ScrollBar/ScrollSlider.cs b/Terminal.Gui/Views/ScrollBar/ScrollSlider.cs index 6c60e80eb..3c942f45e 100644 --- a/Terminal.Gui/Views/ScrollBar/ScrollSlider.cs +++ b/Terminal.Gui/Views/ScrollBar/ScrollSlider.cs @@ -239,7 +239,7 @@ public class ScrollSlider : View, IOrientation, IDesignable OnScrolled (distance); Scrolled?.Invoke (this, new (in distance)); - RaiseActivating (new CommandContext (Command.Activate, this, new KeyBinding ([Command.Activate], null, distance))); + RaiseActivating (new CommandContext (Command.Activate, new WeakReference (this), new KeyBinding ([Command.Activate], null, distance))); } /// diff --git a/Terminal.Gui/Views/Selectors/OptionSelector.cs b/Terminal.Gui/Views/Selectors/OptionSelector.cs index 460a6a609..e05a504c8 100644 --- a/Terminal.Gui/Views/Selectors/OptionSelector.cs +++ b/Terminal.Gui/Views/Selectors/OptionSelector.cs @@ -70,7 +70,7 @@ public class OptionSelector : SelectorBase, IDesignable return true; } - if (!CanFocus || args.Context?.Source is not CheckBox checkBox) + if (!CanFocus || !args.Context.TryGetSource (out View? source) || source is not CheckBox checkBox) { Cycle (); diff --git a/Terminal.Gui/Views/Shortcut.cs b/Terminal.Gui/Views/Shortcut.cs index ef63c4793..be6e9580d 100644 --- a/Terminal.Gui/Views/Shortcut.cs +++ b/Terminal.Gui/Views/Shortcut.cs @@ -258,7 +258,9 @@ public class Shortcut : View, IOrientation, IDesignable { KeyBinding? keyBinding = commandContext?.Binding as KeyBinding?; - Logging.Debug ($"{Title} ({commandContext?.Source?.Title}) Command: {commandContext?.Command}"); + string sourceTitle = commandContext?.TryGetSource (out View? sourceView) == true ? sourceView.Title : "(null)"; + + Logging.Debug ($"{Title} ({sourceTitle}) Command: {commandContext?.Command}"); if (keyBinding is { } kb && kb.Data != this) { @@ -267,12 +269,12 @@ public class Shortcut : View, IOrientation, IDesignable // If this causes CommandView to raise Accept, we eat it KeyBinding updatedBinding = kb with { Data = this }; - Logging.Debug ($"{Title} ({commandContext?.Source?.Title}) - Invoking Activate on CommandView ({CommandView.GetType ().Name})."); + Logging.Debug ($"{Title} ({sourceTitle}) - Invoking Activate on CommandView ({CommandView.GetType ().Name})."); CommandView.InvokeCommand (Command.Activate, updatedBinding); } - Logging.Debug ($"{Title} ({commandContext?.Source?.Title}) - RaiseActivating ..."); + Logging.Debug ($"{Title} ({sourceTitle}) - RaiseActivating ..."); if (RaiseActivating (commandContext) is true) { @@ -282,7 +284,7 @@ public class Shortcut : View, IOrientation, IDesignable if (CanFocus && SuperView is { CanFocus: true }) { // The default HotKey handler sets Focus - Logging.Debug ($"{Title} ({commandContext?.Source?.Title}) - SetFocus..."); + Logging.Debug ($"{Title} ({sourceTitle}) - SetFocus..."); SetFocus (); } @@ -290,10 +292,10 @@ public class Shortcut : View, IOrientation, IDesignable if (commandContext is { Source: null }) { - commandContext.Source = this; + commandContext.Source = new WeakReference (this); } - Logging.Debug ($"{Title} ({commandContext?.Source?.Title}) - Calling RaiseAccepting..."); + Logging.Debug ($"{Title} ({sourceTitle}) - Calling RaiseAccepting..."); cancel = RaiseAccepting (commandContext) is true; if (cancel) @@ -303,7 +305,7 @@ public class Shortcut : View, IOrientation, IDesignable if (Action is { }) { - Logging.Debug ($"{Title} ({commandContext?.Source?.Title}) - Invoke Action..."); + Logging.Debug ($"{Title} ({sourceTitle}) - Invoke Action..."); Action.Invoke (); // Assume if there's a subscriber to Action, it's handled. diff --git a/Terminal.sln.DotSettings b/Terminal.sln.DotSettings index ecd0800e7..ea0979050 100644 --- a/Terminal.sln.DotSettings +++ b/Terminal.sln.DotSettings @@ -532,6 +532,7 @@ True True True + True True True True diff --git a/Tests/UnitTestsParallelizable/Input/CommandContextTests.cs b/Tests/UnitTestsParallelizable/Input/CommandContextTests.cs index a25a8970a..597781f34 100644 --- a/Tests/UnitTestsParallelizable/Input/CommandContextTests.cs +++ b/Tests/UnitTestsParallelizable/Input/CommandContextTests.cs @@ -16,10 +16,12 @@ public class CommandContextTests View sourceView = new () { Id = "sourceView" }; KeyBinding keyBinding = new ([Command.Activate]) { Key = Key.Enter }; - CommandContext ctx = new () { Command = Command.Activate, Source = sourceView, Binding = keyBinding }; + CommandContext ctx = new () { Command = Command.Activate, Source = new WeakReference(sourceView), Binding = keyBinding }; Assert.Equal (Command.Activate, ctx.Command); - Assert.Equal (sourceView, ctx.Source); + Assert.NotNull (ctx.Source); + Assert.True (ctx.Source.TryGetTarget (out View? view)); + Assert.Equal (sourceView, view); Assert.NotNull (ctx.Binding); if (ctx.Binding is KeyBinding kb) @@ -38,10 +40,12 @@ public class CommandContextTests View sourceView = new () { Id = "sourceView" }; MouseBinding mouseBinding = new ([Command.Activate], MouseFlags.LeftButtonClicked); - CommandContext ctx = new () { Command = Command.Activate, Source = sourceView, Binding = mouseBinding }; + CommandContext ctx = new () { Command = Command.Activate, Source = new WeakReference(sourceView), Binding = mouseBinding }; Assert.Equal (Command.Activate, ctx.Command); - Assert.Equal (sourceView, ctx.Source); + Assert.NotNull (ctx.Source); + Assert.True (ctx.Source.TryGetTarget (out View? view)); + Assert.Equal (sourceView, view); Assert.NotNull (ctx.Binding); if (ctx.Binding is MouseBinding mb) @@ -62,7 +66,7 @@ public class CommandContextTests [Fact] public void CommandContext_ImplementsICommandContext () { - CommandContext ctx = new () { Command = Command.Accept, Source = new View () }; + CommandContext ctx = new () { Command = Command.Accept, Source = new WeakReference(new View ()) }; ICommandContext iCtx = ctx; @@ -76,12 +80,14 @@ public class CommandContextTests View originalSource = new () { Id = "original" }; View newSource = new () { Id = "new" }; - CommandContext ctx = new () { Command = Command.Accept, Source = originalSource }; + CommandContext ctx = new () { Command = Command.Accept, Source = new WeakReference(originalSource) }; ICommandContext iCtx = ctx; - iCtx.Source = newSource; + iCtx.Source = new WeakReference(newSource); - Assert.Equal (newSource, iCtx.Source); + Assert.NotNull (iCtx.Source); + Assert.True (iCtx.Source.TryGetTarget (out View? view)); + Assert.Equal (newSource, view); } #endregion @@ -94,7 +100,7 @@ public class CommandContextTests ICommandContext ctx = new CommandContext { Command = Command.Activate, - Source = new View (), + Source = new WeakReference(new View ()), Binding = new KeyBinding ([Command.Activate]) { Key = Key.Enter } }; @@ -115,7 +121,7 @@ public class CommandContextTests MouseBinding mouseBinding = new ([Command.Activate], MouseFlags.LeftButtonClicked) { Source = new View { Id = "mouseSource" } }; mouseBinding.MouseEvent = new Mouse { Flags = MouseFlags.LeftButtonClicked, Position = new Point (10, 20) }; - ICommandContext ctx = new CommandContext { Command = Command.Activate, Source = new View (), Binding = mouseBinding }; + ICommandContext ctx = new CommandContext { Command = Command.Activate, Source = new WeakReference(new View ()), Binding = mouseBinding }; // This is the actual pattern used in production code if (ctx.Binding is MouseBinding { MouseEvent: { } mouse }) @@ -137,7 +143,7 @@ public class CommandContextTests MouseEvent = null // Explicitly set to null }; - ICommandContext ctx = new CommandContext { Command = Command.Activate, Source = new View (), Binding = mouseBinding }; + ICommandContext ctx = new CommandContext { Command = Command.Activate, Source = new WeakReference(new View ()), Binding = mouseBinding }; // Pattern should NOT match when MouseEvent is null bool matched = ctx.Binding is MouseBinding { MouseEvent: { } }; @@ -151,7 +157,7 @@ public class CommandContextTests ICommandContext ctx = new CommandContext { Command = Command.Activate, - Source = new View (), + Source = new WeakReference(new View ()), Binding = new KeyBinding ([Command.Activate]) }; @@ -173,10 +179,12 @@ public class CommandContextTests KeyBinding keyBinding = new ([Command.Activate]) { Key = Key.A, Source = bindingSource }; - CommandContext ctx = new () { Command = Command.Activate, Source = contextSource, Binding = keyBinding }; + CommandContext ctx = new () { Command = Command.Activate, Source = new WeakReference(contextSource), Binding = keyBinding }; // Both sources are accessible - Assert.Equal ("contextSource", ctx.Source?.Id); + Assert.NotNull (ctx.Source); + Assert.True (ctx.Source.TryGetTarget (out View? ctxView)); + Assert.Equal ("contextSource", ctxView!.Id); if (ctx.Binding is KeyBinding kb) { @@ -196,10 +204,12 @@ public class CommandContextTests MouseBinding mouseBinding = new ([Command.Activate], MouseFlags.LeftButtonClicked) { Source = bindingSource }; - CommandContext ctx = new () { Command = Command.Activate, Source = contextSource, Binding = mouseBinding }; + CommandContext ctx = new () { Command = Command.Activate, Source = new WeakReference(contextSource), Binding = mouseBinding }; // Both sources are accessible - Assert.Equal ("contextSource", ctx.Source?.Id); + Assert.NotNull (ctx.Source); + Assert.True (ctx.Source.TryGetTarget (out View? ctxView)); + Assert.Equal ("contextSource", ctxView!.Id); if (ctx.Binding is MouseBinding mb) { @@ -220,7 +230,7 @@ public class CommandContextTests { KeyBinding keyBinding = new ([Command.Accept]) { Key = Key.Enter, Source = new View { Id = "keySource" } }; - CommandContext ctx = new () { Command = Command.Accept, Source = new View { Id = "invoker" }, Binding = keyBinding }; + CommandContext ctx = new () { Command = Command.Accept, Source = new WeakReference(new View { Id = "invoker" }), Binding = keyBinding }; CommandEventArgs args = new () { Context = ctx }; @@ -242,7 +252,7 @@ public class CommandContextTests { MouseBinding mouseBinding = new ([Command.Activate], MouseFlags.RightButtonClicked) { Source = new View { Id = "mouseSource" } }; - CommandContext ctx = new () { Command = Command.Activate, Source = new View { Id = "invoker" }, Binding = mouseBinding }; + CommandContext ctx = new () { Command = Command.Activate, Source = new WeakReference(new View { Id = "invoker" }), Binding = mouseBinding }; CommandEventArgs args = new () { Context = ctx }; @@ -341,4 +351,85 @@ public class CommandContextTests } #endregion + + #region TryGetSource Extension Method Tests + + [Fact] + public void TryGetSource_WithValidWeakReference_ReturnsTrue () + { + View sourceView = new () { Id = "testView" }; + WeakReference weakRef = new (sourceView); + + bool result = weakRef.TryGetSource (out View? retrievedView); + + Assert.True (result); + Assert.NotNull (retrievedView); + Assert.Equal (sourceView, retrievedView); + Assert.Equal ("testView", retrievedView!.Id); + } + + [Fact] + public void TryGetSource_WithNullWeakReference_ReturnsFalse () + { + WeakReference? weakRef = null; + + bool result = weakRef.TryGetSource (out View? retrievedView); + + Assert.False (result); + Assert.Null (retrievedView); + } + + [Fact] + public void CommandContext_TryGetSource_WithValidSource_ReturnsTrue () + { + View sourceView = new () { Id = "contextSource" }; + CommandContext ctx = new () { Command = Command.Accept, Source = new WeakReference (sourceView) }; + + bool result = ctx.TryGetSource (out View? retrievedView); + + Assert.True (result); + Assert.NotNull (retrievedView); + Assert.Equal (sourceView, retrievedView); + Assert.Equal ("contextSource", retrievedView!.Id); + } + + [Fact] + public void CommandContext_TryGetSource_WithNullContext_ReturnsFalse () + { + ICommandContext? ctx = null; + + bool result = ctx.TryGetSource (out View? retrievedView); + + Assert.False (result); + Assert.Null (retrievedView); + } + + [Fact] + public void CommandContext_TryGetSource_WithNullSource_ReturnsFalse () + { + CommandContext ctx = new () { Command = Command.Accept, Source = null }; + + bool result = ctx.TryGetSource (out View? retrievedView); + + Assert.False (result); + Assert.Null (retrievedView); + } + + [Fact] + public void CommandContext_TryGetSource_CanBeUsedInPatternMatching () + { + View sourceView = new Button { Id = "testButton" }; + CommandContext ctx = new () { Command = Command.Accept, Source = new WeakReference (sourceView) }; + + if (ctx.TryGetSource (out View? view) && view is Button button) + { + Assert.Equal ("testButton", button.Id); + } + else + { + Assert.Fail ("Should have retrieved Button from context"); + } + } + + #endregion } diff --git a/Tests/UnitTestsParallelizable/Input/InputBindingTests.cs b/Tests/UnitTestsParallelizable/Input/InputBindingTests.cs index 0e4cef9a9..1e75fe0e5 100644 --- a/Tests/UnitTestsParallelizable/Input/InputBindingTests.cs +++ b/Tests/UnitTestsParallelizable/Input/InputBindingTests.cs @@ -166,10 +166,12 @@ public class InputBindingTests View source = new () { Id = "contextSource" }; InputBinding binding = new ([Command.Activate], source, "contextData"); - CommandContext ctx = new () { Command = Command.Activate, Source = source, Binding = binding }; + CommandContext ctx = new () { Command = Command.Activate, Source = new WeakReference(source), Binding = binding }; Assert.Equal (Command.Activate, ctx.Command); - Assert.Equal (source, ctx.Source); + Assert.NotNull (ctx.Source); + Assert.True (ctx.Source.TryGetTarget (out View? view)); + Assert.Equal (source, view); Assert.NotNull (ctx.Binding); if (ctx.Binding is InputBinding ib)