From ce808b573d30beaaca6e5771acd9ee3acfbf4eb0 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Fri, 15 May 2026 21:46:13 +0200 Subject: [PATCH] Hoist Debug Steps state into the ViewModel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The user-reported "Debug Steps pane is empty after clicking Show Steps" bug was a View-lifecycle issue: `DebugStepsPaneModel` declares `IsVisibleByDefault = false`, so the `DebugSteps` UserControl was realised by the dock factory only when its tab was first activated. By then `BlockILLanguage.DecompileMethod` had already fired `OnStepperUpdated` into the void — no subscriber existed yet. --- ILSpy.Tests/Views/DebugStepsTests.cs | 52 ++++++++ ILSpy/ViewModels/DebugStepsPaneModel.cs | 157 +++++++++++++++++++++++- ILSpy/Views/DebugSteps.axaml | 12 +- ILSpy/Views/DebugSteps.axaml.cs | 137 ++------------------- 4 files changed, 224 insertions(+), 134 deletions(-) diff --git a/ILSpy.Tests/Views/DebugStepsTests.cs b/ILSpy.Tests/Views/DebugStepsTests.cs index ad8bff3bb..ca20fab15 100644 --- a/ILSpy.Tests/Views/DebugStepsTests.cs +++ b/ILSpy.Tests/Views/DebugStepsTests.cs @@ -21,13 +21,19 @@ using System.Linq; using System.Threading.Tasks; +using Avalonia.Controls; using Avalonia.Headless.NUnit; +using Avalonia.VisualTree; using AwesomeAssertions; +using ILSpy; using ILSpy.AppEnv; +using ILSpy.Docking; using ILSpy.Languages; +using ILSpy.TreeNodes; using ILSpy.ViewModels; +using ILSpy.Views; using NUnit.Framework; @@ -67,6 +73,52 @@ public class DebugStepsTests return Task.CompletedTask; } + [AvaloniaTest] + public async Task Debug_Steps_VM_Populates_After_ILAst_Decompile_Regardless_Of_View_Lifecycle() + { + // End-to-end repro of the user-reported "Debug Steps pane is empty" bug: + // 1. Boot the window, load assemblies. + // 2. Select a method. + // 3. Switch the active language to BlockIL (ILAst). + // 4. Wait for the BlockIL decompile to finish — its OnStepperUpdated event fires. + // 5. Assert the DebugStepsPaneModel's Steps property is populated. + // + // Asserting against the VM (not the View) decouples this test from the dock layout's + // view-realisation timing — which is the whole point of the fix that moved state + // from the View into the VM. If `Steps` is populated, any view that binds to it (now + // or later) will render the correct content. + + var window = AppComposition.Current.GetExport(); + window.Show(); + var vm = (MainWindowViewModel)window.DataContext!; + await vm.AssemblyTreeModel.WaitForAssembliesAsync(minimumCount: 3); + + var typeNode = vm.AssemblyTreeModel.FindNode( + "System.Linq", "System.Linq", "System.Linq.Enumerable"); + typeNode.IsExpanded = true; + var method = typeNode.Children.OfType() + .First(m => m.MethodDefinition.Name == "AsEnumerable"); + vm.AssemblyTreeModel.SelectNode(method); + await vm.DockWorkspace.WaitForDecompiledTextAsync(); + + var languageService = AppComposition.Current.GetExport(); + var blockIL = languageService.Languages.OfType() + .Single(l => l.Name == "ILAst"); + languageService.CurrentLanguage = blockIL; + await vm.DockWorkspace.WaitForDecompiledTextAsync(); + + blockIL.Stepper.Steps.Should().NotBeEmpty( + "BlockILLanguage.DecompileMethod must populate context.Stepper.Steps when STEP is defined"); + + var debugStepsVm = AppComposition.Current.GetExport(); + await Waiters.WaitForAsync( + () => debugStepsVm.Steps?.Count > 0, + description: "DebugStepsPaneModel.Steps to be populated after the ILAst decompile"); + + debugStepsVm.Steps.Should().NotBeNullOrEmpty( + "after switching to ILAst and decompiling, the VM's Steps must list the stepper's recorded transforms"); + } + [AvaloniaTest] public Task ILAst_And_TypedIL_Languages_Are_Registered_In_Debug_Builds() { diff --git a/ILSpy/ViewModels/DebugStepsPaneModel.cs b/ILSpy/ViewModels/DebugStepsPaneModel.cs index 832936d52..2d1dceae8 100644 --- a/ILSpy/ViewModels/DebugStepsPaneModel.cs +++ b/ILSpy/ViewModels/DebugStepsPaneModel.cs @@ -18,26 +18,48 @@ #if DEBUG +using System.Collections.Generic; +using System.ComponentModel; using System.Composition; +using Avalonia.Threading; + +using CommunityToolkit.Mvvm.ComponentModel; +using CommunityToolkit.Mvvm.Input; + using ICSharpCode.Decompiler.IL; +using ICSharpCode.Decompiler.IL.Transforms; +using ILSpy.AppEnv; using ILSpy.Commands; +using ILSpy.Docking; +using ILSpy.Languages; +using ILSpy.Util; namespace ILSpy.ViewModels { /// /// Bottom-aligned tool pane that surfaces the Stepper output from - /// . Compiled only in Debug builds — Release - /// users don't see the pane or the languages that populate it. + /// . The ViewModel owns the cross-language / + /// cross-decompile state (active language, current Stepper.Steps list) so it doesn't + /// matter when the matching View materialises — the View just binds to + /// and lights up whenever the language switches to ILAst and a decompile finishes. + /// + /// Compiled only in Debug builds — Release users don't see the pane or the languages + /// that populate it. /// [Export] [ExportToolPane(ContentId = PaneContentId, Alignment = ToolPaneAlignment.Bottom, Order = 1, IsVisibleByDefault = false)] [Shared] - public sealed class DebugStepsPaneModel : ToolPaneModel + public sealed partial class DebugStepsPaneModel : ToolPaneModel { public const string PaneContentId = "DebugSteps"; + readonly LanguageService? languageService; + + ILAstLanguage? activeLanguage; + int lastSelectedStep = int.MaxValue; + /// /// App-wide ILAst writing options shared between the BlockIL language (which reads /// them while emitting the transformed IL) and the DebugSteps view (whose four @@ -49,14 +71,139 @@ namespace ILSpy.ViewModels UseLogicOperationSugar = true, }; + /// Instance accessor for XAML binding (Avalonia's static-source binding is awkward). + public ILAstWritingOptions Options => WritingOptions; + + /// + /// Currently displayed list of recorded transform steps. Re-assigned (not mutated) + /// whenever the active ILAstLanguage's reports a new run, so + /// late-binding views pick up the latest list via the observable change. + /// + [ObservableProperty] + IList? steps; + + /// Two-way bound to the TreeView's selected item. + [ObservableProperty] + Stepper.Node? selectedStep; + + public IRelayCommand ShowStateBeforeCommand { get; } + public IRelayCommand ShowStateAfterCommand { get; } + public IRelayCommand DebugStepCommand { get; } + + /// Design-time / fallback ctor — no dependencies wired. public DebugStepsPaneModel() { Id = PaneContentId; Title = "Debug Steps"; + ShowStateBeforeCommand = new RelayCommand(() => RequestRedecompile(SelectedStep?.BeginStep ?? int.MaxValue, isDebug: false)); + ShowStateAfterCommand = new RelayCommand(() => RequestRedecompile(SelectedStep?.EndStep ?? int.MaxValue, isDebug: false)); + DebugStepCommand = new RelayCommand(() => RequestRedecompile(SelectedStep?.BeginStep ?? int.MaxValue, isDebug: true)); } - /// Instance accessor for XAML binding (Avalonia's static-source binding is awkward). - public ILAstWritingOptions Options => WritingOptions; + [ImportingConstructor] + public DebugStepsPaneModel(LanguageService languageService) : this() + { + this.languageService = languageService; + // DockWorkspace is resolved lazily inside RequestRedecompile — taking it as an + // ImportingConstructor argument creates a MEF cycle (DockWorkspace imports + // ToolPaneRegistry which materialises this VM which would import DockWorkspace). + // Lazy lookup at command-execution time breaks the cycle. + + // Language flips go through LanguageService.CurrentLanguage; the BlockILLanguage + // pumps StepperUpdated when its decompile finishes. The selection-changed event + // is the signal that the user picked a new tree node — clear the step list so + // the previous run's nodes aren't shown against a fresh selection. All three + // subscriptions live here in the long-lived VM so the View's lifecycle (lazy / + // deferred / re-realised) can't lose them. + languageService.PropertyChanged += OnLanguageServiceChanged; + MessageBus.Subscribers += OnSelectionChanged; + WritingOptions.PropertyChanged += OnWritingOptionsChanged; + + TryAttachToCurrentLanguage(); + } + + void OnLanguageServiceChanged(object? sender, PropertyChangedEventArgs e) + { + if (e.PropertyName == nameof(LanguageService.CurrentLanguage)) + Dispatcher.UIThread.Post(TryAttachToCurrentLanguage); + } + + void TryAttachToCurrentLanguage() + { + if (languageService?.CurrentLanguage is ILAstLanguage il) + AttachToLanguage(il); + else + DetachFromLanguage(); + } + + void AttachToLanguage(ILAstLanguage language) + { + if (ReferenceEquals(activeLanguage, language)) + { + // Same language instance — just refresh the steps in case a decompile happened + // while we were detached. + Steps = activeLanguage!.Stepper.Steps; + return; + } + DetachFromLanguage(); + activeLanguage = language; + language.StepperUpdated += OnStepperUpdated; + Steps = language.Stepper.Steps; + } + + void DetachFromLanguage() + { + if (activeLanguage != null) + { + activeLanguage.StepperUpdated -= OnStepperUpdated; + activeLanguage = null; + } + } + + void OnStepperUpdated(object? sender, System.EventArgs e) + { + Dispatcher.UIThread.Post(() => { + if (activeLanguage != null) + { + Steps = activeLanguage.Stepper.Steps; + lastSelectedStep = int.MaxValue; + } + }); + } + + void OnSelectionChanged(object? sender, AssemblyTreeSelectionChangedEventArgs e) + { + // User picked a new tree node — the previous run's stepper is stale until the + // next decompile populates it. + Dispatcher.UIThread.Post(() => { + Steps = null; + lastSelectedStep = int.MaxValue; + }); + } + + void OnWritingOptionsChanged(object? sender, PropertyChangedEventArgs e) + { + // Toggling a checkbox should refresh the editor view so the user sees the effect + // immediately. Triggers the same path as picking "Show state after this step" on + // whatever step was last viewed (defaults to int.MaxValue → full run). + RequestRedecompile(lastSelectedStep, isDebug: false); + } + + void RequestRedecompile(int stepLimit, bool isDebug) + { + lastSelectedStep = stepLimit; + DockWorkspace? dock; + try + { + dock = AppComposition.Current.GetExport(); + } + catch + { + // Composition unavailable in design-time previews; the gesture is a no-op there. + return; + } + dock.ActiveDecompilerTab?.RestartDecompileWithStepLimit(stepLimit, isDebug); + } } } diff --git a/ILSpy/Views/DebugSteps.axaml b/ILSpy/Views/DebugSteps.axaml index 72d183359..42c0e11e4 100644 --- a/ILSpy/Views/DebugSteps.axaml +++ b/ILSpy/Views/DebugSteps.axaml @@ -18,8 +18,10 @@ IsChecked="{Binding Options.ShowChildIndexInBlock, Mode=TwoWay}" /> @@ -29,11 +31,11 @@ + Command="{Binding ShowStateBeforeCommand}" /> + Command="{Binding ShowStateAfterCommand}" /> + Command="{Binding DebugStepCommand}" /> diff --git a/ILSpy/Views/DebugSteps.axaml.cs b/ILSpy/Views/DebugSteps.axaml.cs index cceff6aac..b5c28449e 100644 --- a/ILSpy/Views/DebugSteps.axaml.cs +++ b/ILSpy/Views/DebugSteps.axaml.cs @@ -18,153 +18,42 @@ #if DEBUG -using System; -using System.ComponentModel; - using Avalonia.Controls; using Avalonia.Input; using Avalonia.Interactivity; -using Avalonia.Threading; - -using ICSharpCode.Decompiler.IL.Transforms; -using ILSpy.AppEnv; -using ILSpy.Docking; -using ILSpy.Languages; -using ILSpy.TextView; -using ILSpy.Util; using ILSpy.ViewModels; namespace ILSpy.Views { + /// + /// Thin renderer over . All cross-language / + /// cross-decompile state lives on the ViewModel, so the View has no event subscriptions + /// and no awareness of language switches — it just binds. The two pointer / keyboard + /// handlers below translate user gestures into ViewModel command invocations. + /// public partial class DebugSteps : UserControl { - ILAstLanguage? activeLanguage; - LanguageService? languageService; - int lastSelectedStep = int.MaxValue; - public DebugSteps() { InitializeComponent(); - - MessageBus.Subscribers += OnSelectionChanged; - DebugStepsPaneModel.WritingOptions.PropertyChanged += OnWritingOptionsChanged; - - // Language flips go through LanguageService.CurrentLanguage (not through - // LanguageSettings.LanguageId), so subscribe directly here — the MessageBus path - // would only cover settings-section changes. - languageService = TryGetExport(); - if (languageService != null) - languageService.PropertyChanged += OnLanguageServiceChanged; - - TryAttachToCurrentLanguage(); - } - - void OnLanguageServiceChanged(object? sender, PropertyChangedEventArgs e) - { - if (e.PropertyName == nameof(LanguageService.CurrentLanguage)) - Dispatcher.UIThread.Post(TryAttachToCurrentLanguage); - } - - void TryAttachToCurrentLanguage() - { - var languageService = TryGetExport(); - if (languageService?.CurrentLanguage is ILAstLanguage il) - AttachToLanguage(il); } - void AttachToLanguage(ILAstLanguage language) - { - DetachFromLanguage(); - activeLanguage = language; - language.StepperUpdated += OnStepperUpdated; - RebindStepsTree(language.Stepper); - } - - void DetachFromLanguage() - { - if (activeLanguage != null) - { - activeLanguage.StepperUpdated -= OnStepperUpdated; - activeLanguage = null; - } - } - - void OnStepperUpdated(object? sender, EventArgs e) - { - Dispatcher.UIThread.Post(() => { - if (activeLanguage != null) - RebindStepsTree(activeLanguage.Stepper); - lastSelectedStep = int.MaxValue; - }); - } - - void RebindStepsTree(Stepper stepper) - { - StepsTree.ItemsSource = stepper.Steps; - } + void OnTreeDoubleTapped(object? sender, TappedEventArgs e) + => (DataContext as DebugStepsPaneModel)?.ShowStateAfterCommand.Execute(null); - void OnSelectionChanged(object? sender, AssemblyTreeSelectionChangedEventArgs e) - { - // When the user picks a new tree node, the previous run's stepper is stale — - // blank the tree until the next decompile populates it. - Dispatcher.UIThread.Post(() => { - StepsTree.ItemsSource = null; - lastSelectedStep = int.MaxValue; - }); - } - - void OnWritingOptionsChanged(object? sender, PropertyChangedEventArgs e) - { - // Toggling a checkbox should refresh the editor view so the user sees the effect - // immediately. Triggers the same path as picking "Show state after this step" on - // whatever step was last viewed (defaults to int.MaxValue → full run). - RequestRedecompile(lastSelectedStep, isDebug: false); - } - - void OnShowStateBefore(object? sender, RoutedEventArgs e) - { - if (StepsTree.SelectedItem is Stepper.Node node) - RequestRedecompile(node.BeginStep, isDebug: false); - } - - void OnShowStateAfter(object? sender, RoutedEventArgs e) - { - if (StepsTree.SelectedItem is Stepper.Node node) - RequestRedecompile(node.EndStep, isDebug: false); - } - - void OnDebugStep(object? sender, RoutedEventArgs e) - { - if (StepsTree.SelectedItem is Stepper.Node node) - RequestRedecompile(node.BeginStep, isDebug: true); - } - - void OnStepsTreeKeyDown(object? sender, KeyEventArgs e) + void OnTreeKeyDown(object? sender, KeyEventArgs e) { if (e.Key != Key.Enter && e.Key != Key.Return) return; + if (DataContext is not DebugStepsPaneModel vm) + return; if ((e.KeyModifiers & KeyModifiers.Shift) != 0) - OnShowStateBefore(sender, e); + vm.ShowStateBeforeCommand.Execute(null); else - OnShowStateAfter(sender, e); + vm.ShowStateAfterCommand.Execute(null); e.Handled = true; } - - void RequestRedecompile(int stepLimit, bool isDebug) - { - lastSelectedStep = stepLimit; - var activeTab = TryGetExport()?.ActiveDecompilerTab; - activeTab?.RestartDecompileWithStepLimit(stepLimit, isDebug); - } - - static T? TryGetExport() where T : class - { - try - { return AppComposition.Current.GetExport(); } - catch - { return null; } - } } }