From 99264bd4f514ae54f307b11c0a9a8532d4d41328 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sat, 6 Jun 2026 13:10:03 +0200 Subject: [PATCH] Re-query menu commands via a shared CommandManager Avalonia has no global ICommand re-query signal like WPF's CommandManager.RequerySuggested, so state-dependent main-menu commands were frozen at their startup CanExecute: a NativeMenuItem only re-reads CanExecute when the command raises CanExecuteChanged, and nothing did. The assembly list is empty when the menu is built, so Clear assembly list was disabled forever; Save and Remove-assemblies-with-load-errors had the same latent bug. Add a weak global CommandManager (held weakly so menu/toolbar items aren't pinned) that SimpleCommand routes CanExecuteChanged through -- exactly as WPF's SimpleCommand routed it -- and invalidate it at the central state-change points: tree selection, the active assembly list, and background-load-sweep completion (so load errors surface). One signal, every platform: NativeMenuItem maps CanExecute onto IsEnabled, which the macOS Cocoa, Linux DBus, and in-window menu exporters all track. Assisted-by: Claude:claude-opus-4-8:Claude Code --- .../Commands/ClearAssemblyListCommandTests.cs | 66 +++++++++ .../Commands/SaveCommandEnablementTests.cs | 65 ++++++++ ILSpy/AssemblyTree/AssemblyTreeModel.cs | 10 ++ ILSpy/Commands/CommandManager.cs | 140 ++++++++++++++++++ ILSpy/Commands/SimpleCommand.cs | 20 ++- 5 files changed, 293 insertions(+), 8 deletions(-) create mode 100644 ILSpy.Tests/Commands/ClearAssemblyListCommandTests.cs create mode 100644 ILSpy.Tests/Commands/SaveCommandEnablementTests.cs create mode 100644 ILSpy/Commands/CommandManager.cs diff --git a/ILSpy.Tests/Commands/ClearAssemblyListCommandTests.cs b/ILSpy.Tests/Commands/ClearAssemblyListCommandTests.cs new file mode 100644 index 000000000..8b1a9e27d --- /dev/null +++ b/ILSpy.Tests/Commands/ClearAssemblyListCommandTests.cs @@ -0,0 +1,66 @@ +// Copyright (c) 2026 AlphaSierraPapa for the SharpDevelop Team +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System; +using System.Threading.Tasks; + +using Avalonia.Headless.NUnit; +using Avalonia.Threading; + +using AwesomeAssertions; + +using ICSharpCode.ILSpy.Properties; + +using ILSpy.AppEnv; +using ILSpy.Commands; + +using NUnit.Framework; + +namespace ICSharpCode.ILSpy.Tests; + +[TestFixture] +public class ClearAssemblyListCommandTests +{ + [AvaloniaTest] + public async Task Clear_Assembly_List_Re_Raises_CanExecute_When_The_List_Changes() + { + // The native menu only re-reads CanExecute when CanExecuteChanged fires. The command used to + // never raise it, so it stayed at its startup value -- the list was empty then, leaving the + // menu item disabled forever. Now CanExecuteChanged is routed through the shared CommandManager, + // which the assembly-list change invalidates -- so the item re-queries whenever the list changes. + var (_, vm) = await TestHarness.BootAsync(1); + var command = AppComposition.Current.GetExport() + .GetCommand(nameof(Resources.ClearAssemblyList)); + + command.CanExecute(null).Should().BeTrue("with assemblies loaded the command is enabled"); + + bool raised = false; + // Keep the handler rooted: CommandManager holds subscribers weakly, so an inline lambda with + // no other reference could be collected before the raise. + EventHandler handler = (_, _) => raised = true; + command.CanExecuteChanged += handler; + + vm.AssemblyTreeModel.AssemblyList!.Clear(); + Dispatcher.UIThread.RunJobs(); + + raised.Should().BeTrue( + "clearing the list must re-raise CanExecuteChanged so the menu re-queries the command's enablement"); + command.CanExecute(null).Should().BeFalse("an empty list disables the command"); + GC.KeepAlive(handler); + } +} diff --git a/ILSpy.Tests/Commands/SaveCommandEnablementTests.cs b/ILSpy.Tests/Commands/SaveCommandEnablementTests.cs new file mode 100644 index 000000000..e4c46c8ec --- /dev/null +++ b/ILSpy.Tests/Commands/SaveCommandEnablementTests.cs @@ -0,0 +1,65 @@ +// Copyright (c) 2026 AlphaSierraPapa for the SharpDevelop Team +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System; +using System.Threading.Tasks; + +using Avalonia.Headless.NUnit; +using Avalonia.Threading; + +using AwesomeAssertions; + +using ICSharpCode.ILSpy.Properties; + +using ILSpy.AppEnv; +using ILSpy.Commands; + +using NUnit.Framework; + +namespace ICSharpCode.ILSpy.Tests; + +/// +/// The general fix (routing SimpleCommand.CanExecuteChanged through the shared CommandManager, +/// invalidated on selection changes) must apply to every selection-dependent menu command, not just +/// the one originally reported. Save Code is the canonical case. +/// +[TestFixture] +public class SaveCommandEnablementTests +{ + [AvaloniaTest] + public async Task Save_Command_Enables_When_A_Tree_Node_Is_Selected() + { + var (_, vm) = await TestHarness.BootAsync(1); + var command = AppComposition.Current.GetExport() + .GetCommand(nameof(Resources._SaveCode)); + + command.CanExecute(null).Should().BeFalse("with nothing selected at startup, Save is disabled"); + + bool raised = false; + EventHandler handler = (_, _) => raised = true; // rooted; CommandManager holds handlers weakly + command.CanExecuteChanged += handler; + + vm.AssemblyTreeModel.SelectNode(vm.AssemblyTreeModel.Root!.Children[0]); + Dispatcher.UIThread.RunJobs(); + + raised.Should().BeTrue( + "selecting a node must re-raise CanExecuteChanged so the menu re-queries the command's enablement"); + command.CanExecute(null).Should().BeTrue("a selected tree node enables Save"); + GC.KeepAlive(handler); + } +} diff --git a/ILSpy/AssemblyTree/AssemblyTreeModel.cs b/ILSpy/AssemblyTree/AssemblyTreeModel.cs index e735b3f84..2e4e30a67 100644 --- a/ILSpy/AssemblyTree/AssemblyTreeModel.cs +++ b/ILSpy/AssemblyTree/AssemblyTreeModel.cs @@ -228,6 +228,9 @@ namespace ILSpy.AssemblyTree // Pub-sub fan-out: panes (e.g. Debug Steps) that need to invalidate their state // when the selection moves listen on this message. Util.MessageBus.Send(this, new Util.AssemblyTreeSelectionChangedEventArgs()); + + // Selection-dependent menu commands (Save, Analyze-via-menu, ...) re-evaluate CanExecute. + Commands.CommandManager.InvalidateRequerySuggested(); } // Walks already-materialized children and re-raises Text PropertyChanged so the cell @@ -528,6 +531,9 @@ namespace ILSpy.AssemblyTree } await Task.WhenAll(loadTasks).ConfigureAwait(false); AppEnv.AppLog.Mark("Background-load sweep dispatched"); + // Load errors only surface once a load completes (no list change fires for them), + // so re-evaluate now -- this is what enables "Remove assemblies with load errors". + Commands.CommandManager.InvalidateRequerySuggested(); } catch (Exception ex) { @@ -982,6 +988,10 @@ namespace ILSpy.AssemblyTree } } Util.MessageBus.Send(this, new Util.CurrentAssemblyListChangedEventArgs(e)); + + // List-dependent menu commands (Clear assembly list, Remove assemblies with load errors) + // re-evaluate CanExecute now that the list gained or lost entries. + Commands.CommandManager.InvalidateRequerySuggested(); } // Coalesces burst F5 / programmatic Refresh() calls into a single async pipeline. diff --git a/ILSpy/Commands/CommandManager.cs b/ILSpy/Commands/CommandManager.cs new file mode 100644 index 000000000..348a82838 --- /dev/null +++ b/ILSpy/Commands/CommandManager.cs @@ -0,0 +1,140 @@ +// Copyright (c) 2026 AlphaSierraPapa for the SharpDevelop Team +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System; +using System.Collections.Generic; +using System.Reflection; + +namespace ILSpy.Commands +{ + /// + /// A poor-man's equivalent of WPF's CommandManager.RequerySuggested. Avalonia has no + /// global "re-query your CanExecute" signal, so routes its + /// CanExecuteChanged through this one (exactly as WPF's SimpleCommand routed it through + /// CommandManager.RequerySuggested). Call whenever + /// application state that a command's CanExecute reads might have changed (tree selection, + /// the assembly list, a background load finishing); every bound command then re-evaluates. + /// + /// This works on every platform: a bound NativeMenuItem subscribes to the command's + /// CanExecuteChanged and, when it fires, sets IsEnabled = CanExecute(); the macOS Cocoa, + /// Linux DBus, and in-window menu exporters all track that IsEnabled property. + /// + /// Subscribers are held WEAKLY (by target + method, like ), + /// so a rebuilt menu or toolbar -- whose items subscribe via the routed CanExecuteChanged -- is + /// not pinned for the lifetime of this static type. NOTE: a handler with no other strong reference + /// (e.g. an inline lambda) may be collected before the next raise; callers that need it to survive + /// must keep it rooted, just as with WPF's weak RequerySuggested. + /// + public static class CommandManager + { + static readonly object gate = new(); + static readonly List handlers = new(); + + /// Subscribes (weakly) to the global re-query signal. + public static void AddRequerySuggested(EventHandler handler) + { + ArgumentNullException.ThrowIfNull(handler); + lock (gate) + handlers.Add(new WeakHandler(handler)); + } + + /// Removes a previously added handler. + public static void RemoveRequerySuggested(EventHandler handler) + { + ArgumentNullException.ThrowIfNull(handler); + lock (gate) + { + for (int i = handlers.Count - 1; i >= 0; i--) + { + if (handlers[i].Matches(handler)) + { + handlers.RemoveAt(i); + break; + } + } + } + } + + /// + /// Asks every bound command to re-evaluate its CanExecute. Marshalled to the UI thread + /// (handlers touch UI state such as NativeMenuItem.IsEnabled), so it is safe to call from + /// a background thread (e.g. a load sweep finishing off the thread pool). + /// + public static void InvalidateRequerySuggested() + { + var dispatcher = global::Avalonia.Threading.Dispatcher.UIThread; + if (dispatcher.CheckAccess()) + Raise(); + else + dispatcher.Post(Raise); + } + + static void Raise() + { + // Snapshot under lock so add/remove during dispatch doesn't perturb this raise. + WeakHandler[] snapshot; + lock (gate) + snapshot = handlers.ToArray(); + List? dead = null; + foreach (var h in snapshot) + { + if (!h.TryInvoke()) + (dead ??= new()).Add(h); + } + if (dead != null) + { + lock (gate) + foreach (var h in dead) + handlers.Remove(h); + } + } + + sealed class WeakHandler + { + readonly WeakReference? targetRef; + readonly MethodInfo method; + + public WeakHandler(EventHandler handler) + { + method = handler.Method; + // Static handlers (null Target) are kept until Remove drops them explicitly. + targetRef = handler.Target is null ? null : new WeakReference(handler.Target); + } + + public bool Matches(EventHandler handler) + { + if (method != handler.Method) + return false; + if (targetRef == null) + return handler.Target == null; + return targetRef.TryGetTarget(out var t) && ReferenceEquals(t, handler.Target); + } + + public bool TryInvoke() + { + object? target; + if (targetRef == null) + target = null; + else if (!targetRef.TryGetTarget(out target!)) + return false; // collected -> caller prunes + method.Invoke(target, new object?[] { null, EventArgs.Empty }); + return true; + } + } + } +} diff --git a/ILSpy/Commands/SimpleCommand.cs b/ILSpy/Commands/SimpleCommand.cs index 6a8b01039..92fc5d23b 100644 --- a/ILSpy/Commands/SimpleCommand.cs +++ b/ILSpy/Commands/SimpleCommand.cs @@ -24,22 +24,26 @@ using global::Avalonia.Data; namespace ILSpy.Commands { /// - /// Minimal ICommand base. Avalonia has no global RequerySuggested signal like WPF's - /// CommandManager, so derived commands fire CanExecuteChanged themselves when they care. - /// MenuItems also re-query CanExecute when the parent menu opens, which covers most cases. + /// Minimal ICommand base. Avalonia has no global RequerySuggested signal like WPF's CommandManager, + /// so CanExecuteChanged is routed through our own (mirroring how WPF's + /// SimpleCommand routed it through CommandManager.RequerySuggested). State-change sites call + /// and every bound command re-evaluates -- + /// no per-command wiring, and the menu/toolbar item updates its enabled state on every platform. /// public abstract class SimpleCommand : ICommand { - public event EventHandler? CanExecuteChanged; + public event EventHandler? CanExecuteChanged { + add { if (value is not null) CommandManager.AddRequerySuggested(value); } + remove { if (value is not null) CommandManager.RemoveRequerySuggested(value); } + } public abstract void Execute(object? parameter); public virtual bool CanExecute(object? parameter) => true; - protected void RaiseCanExecuteChanged() - { - CanExecuteChanged?.Invoke(this, EventArgs.Empty); - } + /// Asks all bound commands to re-evaluate CanExecute (a global invalidate -- the + /// re-query is shared, so this is not scoped to this single command). + protected static void RaiseCanExecuteChanged() => CommandManager.InvalidateRequerySuggested(); } ///