From 5d70f32048ccc7c157f5c7a4a3b8800af4b02bb8 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Fri, 8 May 2026 12:48:30 +0200 Subject: [PATCH] Dedupe ShowSelectedNode for live-page filter wiring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Selecting a tree node was firing ShowSelectedNode twice — once via SelectedItems.CollectionChanged and once via SelectedItem PropertyChanged. Each call ran node.CreateTab(), producing a fresh MetadataTablePageModel with a fresh ColumnFilters set. The DataGrid ended up rendering the second page's columns, but the first page's filters were what tests (and any code reading the model) saw — so typing into a header TextBox updated a filter no one was reading. Track the last shown nodes inside ShowSelectedNode and short-circuit when they match. Assisted-by: Claude:claude-opus-4-7:Claude Code --- .../MetadataFilterRowVerificationTest.cs | 131 ++++++++++++++++++ ILSpy/Docking/DockWorkspace.cs | 12 ++ ILSpy/ViewModels/ContentTabPage.cs | 9 +- 3 files changed, 151 insertions(+), 1 deletion(-) create mode 100644 ILSpy.Tests/Metadata/MetadataFilterRowVerificationTest.cs diff --git a/ILSpy.Tests/Metadata/MetadataFilterRowVerificationTest.cs b/ILSpy.Tests/Metadata/MetadataFilterRowVerificationTest.cs new file mode 100644 index 000000000..7a5862c4c --- /dev/null +++ b/ILSpy.Tests/Metadata/MetadataFilterRowVerificationTest.cs @@ -0,0 +1,131 @@ +// 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.Diagnostics; +using System.Linq; +using System.Threading.Tasks; + +using Avalonia.Controls; +using Avalonia.Headless.NUnit; +using Avalonia.Threading; +using Avalonia.VisualTree; + +using AwesomeAssertions; + +using ILSpy.AppEnv; +using ILSpy.Metadata; +using ILSpy.Metadata.CorTables; +using ILSpy.TreeNodes; +using ILSpy.ViewModels; +using ILSpy.Views; + +using NUnit.Framework; + +namespace ICSharpCode.ILSpy.Tests.Metadata; + +/// +/// TEMPORARY verification harness for the per-column filter wiring. Run with +/// ILSPY_TESTS_VISIBLE=1 (or debugger attached) so each step captures a PNG and +/// pops it open in the OS image viewer; otherwise the assertions still execute headlessly +/// but no images are produced. Delete this file once the filter is confirmed working. +/// +[TestFixture] +public class MetadataFilterRowVerificationTest +{ + [AvaloniaTest] + public async Task Step_By_Step_Verify_TypeDef_Name_Filter_With_Screenshots() + { + // === STEP 1: Boot the main window === + var window = AppComposition.Current.GetExport(); + window.Show(); + Dispatcher.UIThread.RunJobs(); + window.CaptureAndShow(label: "step1_window_shown"); + + var vm = (MainWindowViewModel)window.DataContext!; + await vm.AssemblyTreeModel.WaitForAssembliesAsync(minimumCount: 1); + Dispatcher.UIThread.RunJobs(); + window.CaptureAndShow(label: "step2_assemblies_loaded"); + + // === STEP 2: Drill into TypeDef table === + var coreLibName = typeof(object).Assembly.GetName().Name!; + var assemblyNode = vm.AssemblyTreeModel.FindNode(coreLibName); + assemblyNode.EnsureLazyChildren(); + var metadataNode = assemblyNode.Children.OfType().Single(); + metadataNode.EnsureLazyChildren(); + var tablesNode = metadataNode.Children.OfType().Single(); + tablesNode.EnsureLazyChildren(); + var typeDefNode = tablesNode.Children.OfType().Single(); + vm.AssemblyTreeModel.SelectNode(typeDefNode); + + var page = await vm.DockWorkspace.WaitForMetadataTabAsync(); + Dispatcher.UIThread.RunJobs(); + window.CaptureAndShow(label: "step3_typedef_open"); + + // === STEP 3: Wait for the live MetadataTablePage to materialise === + // ContentTabPage opts out of Dock's deferred-content presentation so the wrapper + // view's children (DecompilerTextView + MetadataTablePage) are part of the visual + // tree synchronously. Without that opt-out the headless test can't reach them. + var metadataPage = await window.WaitForComponent(); + TestContext.Out.WriteLine("[VERIFY] MetadataTablePage materialised under the main window."); + var grid = await metadataPage.WaitForComponent(); + TestContext.Out.WriteLine("[VERIFY] Inner DataGrid materialised."); + + // Force a layout pass so DataGrid populates its column-headers panel. + Dispatcher.UIThread.RunJobs(); + await grid.WaitForComponent(); + + // === STEP 4: Find the rendered TextBox for the Name column header === + var headerBox = grid.GetVisualDescendants().OfType() + .FirstOrDefault(tb => tb.FindAncestorOfType() is { } owner + && owner.Content is StackPanel sp + && sp.Children.OfType().FirstOrDefault()?.Text == "Name"); + headerBox.Should().NotBeNull( + "the column-builder bakes a TextBox into the Name column's header — it must reach the rendered visual tree"); + + // === STEP 5: Confirm the rendered TextBox is the exact instance the model holds === + var nameColumn = page.Columns.Single(c => (string?)c.Tag == "Name"); + var modelHeaderBox = ((StackPanel)nameColumn.Header!).Children.OfType().Single(); + ReferenceEquals(headerBox, modelHeaderBox).Should().BeTrue( + "DataGrid must render the StackPanel Header directly without re-templating; otherwise the TextProperty observable wired by the builder is on a different TextBox than the user types into"); + + var nameFilter = page.ColumnFilters.Single(f => f.ColumnName == "Name"); + var totalRows = page.Items.Count; + totalRows.Should().BeGreaterThan(0, "TypeDef must have rows for filtering to be observable"); + + // === STEP 6: Type "System" into the rendered TextBox === + headerBox!.Text = "System"; + Dispatcher.UIThread.RunJobs(); + nameFilter.Text.Should().Be("System", + "setting the rendered TextBox.Text must propagate to ColumnFilter.Text"); + var visibleByPredicate = page.Items.Count(e => MetadataTablePageModel.MatchesFilters(e, page.ColumnFilters)); + visibleByPredicate.Should().BeLessThan(totalRows, + "the predicate must hide at least one TypeDef row when filtering Name on 'System'"); + TestContext.Out.WriteLine($"[VERIFY] After typing 'System': filter='{nameFilter.Text}' visible={visibleByPredicate}/{totalRows}"); + window.CaptureAndShow(label: "step6_typed_System"); + + // === STEP 7: Clear the filter === + headerBox.Text = ""; + Dispatcher.UIThread.RunJobs(); + nameFilter.Text.Should().BeEmpty("clearing the TextBox must clear the filter"); + var visibleAfterClear = page.Items.Count(e => MetadataTablePageModel.MatchesFilters(e, page.ColumnFilters)); + visibleAfterClear.Should().Be(totalRows, "an empty filter must show every row again"); + TestContext.Out.WriteLine($"[VERIFY] After clearing: filter='{nameFilter.Text}' visible={visibleAfterClear}/{totalRows}"); + window.CaptureAndShow(label: "step7_cleared"); + } +} diff --git a/ILSpy/Docking/DockWorkspace.cs b/ILSpy/Docking/DockWorkspace.cs index 94eab4b6e..0e452e2a7 100644 --- a/ILSpy/Docking/DockWorkspace.cs +++ b/ILSpy/Docking/DockWorkspace.cs @@ -256,13 +256,25 @@ namespace ILSpy.Docking // Created lazily on first need. DecompilerTabPageModel? decompilerContent; + ILSpyTreeNode[]? lastShownNodes; + void ShowSelectedNode() { var nodes = assemblyTreeModel.SelectedItems.OfType().ToArray(); if (nodes.Length == 0) + { + lastShownNodes = null; return; + } if (factory.MainTab is not { } main) return; + // SelectedItems.CollectionChanged and SelectedItem PropertyChanged both fan into + // here on a single click, so dedupe to avoid creating two TabPageModels for the + // same selection — the second one's columns would replace the first's, but the + // first's filter wiring would be left dangling on stale ColumnFilter instances. + if (lastShownNodes is { } prev && prev.SequenceEqual(nodes)) + return; + lastShownNodes = nodes; // Tree-node selections always reuse the single document slot. The Document // instance never changes; its inner Content swaps between viewmodels. The diff --git a/ILSpy/ViewModels/ContentTabPage.cs b/ILSpy/ViewModels/ContentTabPage.cs index 9b5a404a0..2310b3dde 100644 --- a/ILSpy/ViewModels/ContentTabPage.cs +++ b/ILSpy/ViewModels/ContentTabPage.cs @@ -20,6 +20,8 @@ using System.ComponentModel; using CommunityToolkit.Mvvm.ComponentModel; +using Dock.Controls.DeferredContentControl; + namespace ILSpy.ViewModels { /// @@ -29,8 +31,13 @@ namespace ILSpy.ViewModels /// toggles which is visible — Dock.Avalonia's add+close-in-the-same-tick semantics /// otherwise leave the previous view rendered when the tab type changes. /// - public sealed partial class ContentTabPage : TabPageModel + public sealed partial class ContentTabPage : TabPageModel, IDeferredContentPresentation { + // Opt out of Dock's deferred presentation: there's exactly one document tab in this + // app, the inner views are already pre-realised by the wrapper view, and headless + // tests can't reach descendants of a control that's still queued for realisation. + bool IDeferredContentPresentation.DeferContentPresentation => false; + [ObservableProperty] private object? content;