From 29ccd947cafbf39deaf92203cd28a33179773e51 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Tue, 5 May 2026 00:17:17 +0200 Subject: [PATCH] About page participates in navigation history MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generalises NavigationHistory into NavigationHistory with two subtypes — TreeNodeEntry (a tree-node selection in a specific tab) and StaticPageEntry (a static page like About in a specific tab) — mirroring the WPF host's NavigationState/ViewState pair. Assisted-by: Claude:claude-opus-4-7:Claude Code --- ILSpy.Tests/Commands/HelpCommandTests.cs | 73 ++++++++++++++ ILSpy.Tests/Navigation/NavigationTests.cs | 7 +- ILSpy/Commands/AboutCommand.cs | 11 ++- ILSpy/Docking/DockWorkspace.cs | 110 +++++++++++++++------ ILSpy/NavigationEntry.cs | 111 ++++++++++++++++++++++ ILSpy/NavigationHistory.cs | 18 ++-- ILSpy/TextView/DecompilerTabPageModel.cs | 8 ++ ILSpy/Views/MainToolBar.axaml.cs | 8 +- 8 files changed, 298 insertions(+), 48 deletions(-) create mode 100644 ILSpy/NavigationEntry.cs diff --git a/ILSpy.Tests/Commands/HelpCommandTests.cs b/ILSpy.Tests/Commands/HelpCommandTests.cs index 6d11000df..f072d87a7 100644 --- a/ILSpy.Tests/Commands/HelpCommandTests.cs +++ b/ILSpy.Tests/Commands/HelpCommandTests.cs @@ -29,7 +29,9 @@ using ICSharpCode.ILSpy.Properties; using ILSpy.AppEnv; using ILSpy.Commands; using ILSpy.Docking; +using ILSpy.Navigation; using ILSpy.TextView; +using ILSpy.TreeNodes; using ILSpy.ViewModels; using ILSpy.Views; @@ -123,4 +125,75 @@ public class HelpCommandTests var licenseTab = (DecompilerTabPageModel)documents.ActiveDockable!; licenseTab.Text.Should().Contain("Permission is hereby granted"); } + + [AvaloniaTest] + public async Task About_Page_Lands_On_Back_History_And_Round_Trips_Through_Navigation() + { + // Opening the About page records a StaticPageEntry on the back stack and the About + // tab is marked IsStaticContent so that subsequent tree-node selections route to + // (or open) a real decompiler tab without overwriting the About content. Pressing + // Back returns to the previous tree-node selection (re-activating its decompiler tab), + // pressing Forward re-activates the About tab with content intact. + + // Arrange — boot, select a tree node so there's a tree-node entry to compare against. + 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.SelectedItem = method; + await vm.DockWorkspace.WaitForDecompiledTextAsync(); + var decompilerTab = vm.DockWorkspace.ActiveDecompilerTab!; + var methodText = decompilerTab.Text; + + // NavigationHistory collapses entries within 0.5s — wait past the window so opening + // About records its own entry instead of replacing the tree-node entry. + await Task.Delay(600); + + // Act 1 — open the About page. + var registry = AppComposition.Current.GetExport(); + var aboutCmd = registry.Commands + .Single(c => c.Metadata.Header == nameof(Resources._About)) + .CreateExport().Value; + var documents = ((ILSpyDockFactory)vm.DockWorkspace.Factory).Documents!; + aboutCmd.Execute(null); + await Waiters.WaitForAsync( + () => documents.ActiveDockable is DecompilerTabPageModel { IsStaticContent: true }); + var aboutTab = (DecompilerTabPageModel)documents.ActiveDockable!; + var aboutText = aboutTab.Text; + + // Assert (mid-test) — the back stack now ends with the tree-node entry; the About + // page is the current entry (so Back goes there). + vm.DockWorkspace.BackHistory.Should().NotBeEmpty(); + vm.DockWorkspace.BackHistory.Last().Should().BeOfType(); + var lastBack = (TreeNodeEntry)vm.DockWorkspace.BackHistory.Last(); + lastBack.Node.GetType().Should().Be(typeof(MethodTreeNode)); + + // Act 2 — press Back from the About page. + vm.DockWorkspace.NavigateBackCommand.CanExecute(null).Should().BeTrue(); + vm.DockWorkspace.NavigateBackCommand.Execute(null); + await Waiters.WaitForAsync( + () => ReferenceEquals(documents.ActiveDockable, decompilerTab)); + + // Assert — the decompiler tab is active again; About tab still exists with content + // preserved. + ReferenceEquals(vm.AssemblyTreeModel.SelectedItem, method).Should().BeTrue(); + decompilerTab.Text.Should().Be(methodText); + documents.VisibleDockables!.Should().Contain(aboutTab); + aboutTab.Text.Should().Be(aboutText, "the About tab content must survive a Back navigation"); + + // Act 3 — press Forward to return to the About page. + vm.DockWorkspace.NavigateForwardCommand.CanExecute(null).Should().BeTrue(); + vm.DockWorkspace.NavigateForwardCommand.Execute(null); + await Waiters.WaitForAsync( + () => ReferenceEquals(documents.ActiveDockable, aboutTab)); + + // Assert — About is active again, content intact. + aboutTab.Text.Should().Be(aboutText); + } } diff --git a/ILSpy.Tests/Navigation/NavigationTests.cs b/ILSpy.Tests/Navigation/NavigationTests.cs index 6c02fe9aa..036cba973 100644 --- a/ILSpy.Tests/Navigation/NavigationTests.cs +++ b/ILSpy.Tests/Navigation/NavigationTests.cs @@ -132,11 +132,14 @@ public class NavigationTests await Waiters.WaitForAsync(() => flyout.Items.OfType().Count() >= 2); // Assert 1 — newest-first ordering: index 0 is the immediate previous selection - // (methodB), index 1 is the one before that (methodA). + // (methodB), index 1 is the one before that (methodA). Each menu item carries a + // TreeNodeEntry wrapping the original tree node. var items = flyout.Items.OfType().ToList(); ((string)items[0].Header!).Should().Be((string)methodB.Text); ((string)items[1].Header!).Should().Be((string)methodA.Text); - items[1].CommandParameter.Should().BeSameAs(methodA); + items[1].CommandParameter.Should().BeOfType(); + var entry = (global::ILSpy.Navigation.TreeNodeEntry)items[1].CommandParameter!; + ReferenceEquals(entry.Node, methodA).Should().BeTrue(); // Act 3 — multi-step jump: clicking methodA pops two entries off the back stack in one go. items[1].Command!.Execute(items[1].CommandParameter); diff --git a/ILSpy/Commands/AboutCommand.cs b/ILSpy/Commands/AboutCommand.cs index 77c5ca5f4..9318bbed7 100644 --- a/ILSpy/Commands/AboutCommand.cs +++ b/ILSpy/Commands/AboutCommand.cs @@ -80,10 +80,12 @@ namespace ILSpy.Commands foreach (var (phrase, uri) in Links) output.AddVisualLineElementGenerator(new ResourceLinkGenerator(phrase, uri)); - OpenInNewTab(Resources.About, output, ".txt"); + var tab = OpenInNewTab(Resources.About, output, ".txt"); + tab.IsStaticContent = true; + dockWorkspace.RecordStaticPage(tab, new Uri("resource:aboutpage")); } - void OpenInNewTab(string title, AvaloniaEditTextOutput output, string syntaxExtension) + DecompilerTabPageModel OpenInNewTab(string title, AvaloniaEditTextOutput output, string syntaxExtension) { var tab = new DecompilerTabPageModel { Title = title, @@ -100,6 +102,7 @@ namespace ILSpy.Commands }; tab.OpenUriRequested += OnOpenUri; dockWorkspace.OpenNewTab(tab); + return tab; } bool OnOpenUri(Uri uri) @@ -119,7 +122,9 @@ namespace ILSpy.Commands using var reader = new StreamReader(stream); var output = new AvaloniaEditTextOutput { Title = resourceName }; output.Write(reader.ReadToEnd()); - OpenInNewTab(resourceName, output, ".txt"); + var tab = OpenInNewTab(resourceName, output, ".txt"); + tab.IsStaticContent = true; + dockWorkspace.RecordStaticPage(tab, new Uri("resource:" + resourceName)); } static string GetDotnetProductVersion() diff --git a/ILSpy/Docking/DockWorkspace.cs b/ILSpy/Docking/DockWorkspace.cs index 97a060b0f..f93c7f16c 100644 --- a/ILSpy/Docking/DockWorkspace.cs +++ b/ILSpy/Docking/DockWorkspace.cs @@ -48,22 +48,22 @@ namespace ILSpy.Docking readonly ILSpyDockFactory factory; readonly AssemblyTreeModel assemblyTreeModel; readonly LanguageService languageService; - readonly NavigationHistory history = new(); - // Set true while a Back/Forward navigation is rewriting AssemblyTreeModel.SelectedItem, - // so the SelectedItem change notification doesn't push a fresh entry onto the stack and - // undo the navigation we just did. + readonly NavigationHistory history = new(); + // Set true while a Back/Forward navigation is rewriting state (SelectedItem, + // active dockable) so the change notifications don't push fresh entries onto the + // stack and undo the navigation we just did. bool suppressHistoryRecording; public IFactory Factory => factory; public IRelayCommand NavigateBackCommand { get; } public IRelayCommand NavigateForwardCommand { get; } - public IRelayCommand NavigateToHistoryCommand { get; } + public IRelayCommand NavigateToHistoryCommand { get; } // Read-only history snapshots for the Back/Forward split-button dropdowns; oldest-first. // The UI reverses these for newest-first display. - public IReadOnlyList BackHistory => history.BackEntries; - public IReadOnlyList ForwardHistory => history.ForwardEntries; + public IReadOnlyList BackHistory => history.BackEntries; + public IReadOnlyList ForwardHistory => history.ForwardEntries; public IRootDock Layout { get; } @@ -80,8 +80,8 @@ namespace ILSpy.Docking this.languageService = languageService; NavigateBackCommand = new RelayCommand(NavigateBack, () => history.CanNavigateBack); NavigateForwardCommand = new RelayCommand(NavigateForward, () => history.CanNavigateForward); - NavigateToHistoryCommand = new RelayCommand(NavigateToHistory, - node => node != null && (history.BackEntries.Contains(node) || history.ForwardEntries.Contains(node))); + NavigateToHistoryCommand = new RelayCommand(NavigateToHistory, + entry => entry != null && (history.BackEntries.Contains(entry) || history.ForwardEntries.Contains(entry))); factory = new ILSpyDockFactory(assemblyTreeModel, searchPaneModel, analyzerTreeViewModel); Layout = factory.CreateLayout(); if (factory.InitialDecompilerTab is { } initialTab) @@ -147,16 +147,39 @@ namespace ILSpy.Docking { if (e.PropertyName == nameof(AssemblyTreeModel.SelectedItem)) { - RecordHistory(assemblyTreeModel.SelectedItem); ShowSelectedNode(); + RecordTreeNodeSelection(assemblyTreeModel.SelectedItem); } } - void RecordHistory(SharpTreeNode? node) + void RecordTreeNodeSelection(SharpTreeNode? node) { if (suppressHistoryRecording || node == null) return; - history.Record(node); + var tab = ResolveDecompilerTab(); + if (tab == null) + return; + RecordHistoryEntry(new TreeNodeEntry(tab, node)); + } + + /// + /// Records a static-page entry (e.g. About) into the navigation history. The caller + /// has already opened via . The tab + /// should have set so that + /// subsequent tree-node selections route to a different tab and leave it intact. + /// + public void RecordStaticPage(TabPageModel tab, Uri uri) + { + ArgumentNullException.ThrowIfNull(tab); + ArgumentNullException.ThrowIfNull(uri); + if (suppressHistoryRecording) + return; + RecordHistoryEntry(new StaticPageEntry(tab, uri)); + } + + void RecordHistoryEntry(NavigationEntry entry) + { + history.Record(entry); NavigateBackCommand.NotifyCanExecuteChanged(); NavigateForwardCommand.NotifyCanExecuteChanged(); } @@ -172,24 +195,29 @@ namespace ILSpy.Docking ApplyNavigationTarget(target); } - void NavigateToHistory(SharpTreeNode? node) + void NavigateToHistory(NavigationEntry? entry) { - if (node == null) + if (entry == null) return; - bool forward = history.ForwardEntries.Contains(node); - if (!forward && !history.BackEntries.Contains(node)) + bool forward = history.ForwardEntries.Contains(entry); + if (!forward && !history.BackEntries.Contains(entry)) return; - var target = history.GoTo(node, forward); + var target = history.GoTo(entry, forward); if (target != null) ApplyNavigationTarget(target); } - void ApplyNavigationTarget(SharpTreeNode target) + void ApplyNavigationTarget(NavigationEntry target) { suppressHistoryRecording = true; try { - assemblyTreeModel.SelectedItem = target; + if (factory.Documents?.VisibleDockables is { } docs && docs.Contains(target.Tab)) + factory.SetActiveDockable(target.Tab); + if (target is TreeNodeEntry treeNode) + assemblyTreeModel.SelectedItem = treeNode.Node; + // StaticPageEntry: just reactivating the tab is enough — its content was + // preserved because IsStaticContent kept tree-node selections from targeting it. } finally { @@ -203,7 +231,7 @@ namespace ILSpy.Docking { if (e.PropertyName == nameof(LanguageService.CurrentLanguage)) { - if (GetActiveDecompilerTab() is { } tab) + if (GetDecompilerContentTab() is { } tab) { tab.Language = languageService.CurrentLanguage; // Re-decompile by re-assigning the same node so the tab refreshes for the new language. @@ -231,30 +259,52 @@ namespace ILSpy.Docking var nodes = assemblyTreeModel.SelectedItems.OfType().ToArray(); if (nodes.Length == 0) return; - var tab = GetActiveDecompilerTab(); + var tab = ResolveDecompilerTab(); if (tab == null) + return; + if (factory.Documents?.ActiveDockable != tab) { - tab = factory.InitialDecompilerTab; - if (tab == null || factory.Documents == null) - return; - factory.AddDockable(factory.Documents, tab); factory.SetActiveDockable(tab); - factory.SetFocusedDockable(factory.Documents, tab); + if (factory.Documents != null) + factory.SetFocusedDockable(factory.Documents, tab); } tab.Language = languageService.CurrentLanguage; tab.CurrentNodes = nodes; } - public DecompilerTabPageModel? ActiveDecompilerTab => GetActiveDecompilerTab(); + public DecompilerTabPageModel? ActiveDecompilerTab => GetDecompilerContentTab(); + + /// + /// Returns the tab tree-node selections should be rendered into. Prefers the active + /// dockable when it's a non-static decompiler tab, falls back to any other non-static + /// decompiler tab, and otherwise lazily attaches the factory's initial decompiler tab. + /// Returns null if the document dock isn't realised yet. + /// + DecompilerTabPageModel? ResolveDecompilerTab() + { + if (factory.Documents == null) + return null; + if (GetDecompilerContentTab() is { } existing) + return existing; + var tab = factory.InitialDecompilerTab; + if (tab == null) + return null; + factory.AddDockable(factory.Documents, tab); + factory.SetActiveDockable(tab); + factory.SetFocusedDockable(factory.Documents, tab); + return tab; + } - DecompilerTabPageModel? GetActiveDecompilerTab() + // Decompiler-content tabs only — static-content tabs (About / License) are excluded so + // tree-node selections never overwrite them. + DecompilerTabPageModel? GetDecompilerContentTab() { - if (factory.Documents?.ActiveDockable is DecompilerTabPageModel active) + if (factory.Documents?.ActiveDockable is DecompilerTabPageModel { IsStaticContent: false } active) return active; if (factory.Documents?.VisibleDockables != null) { foreach (var d in factory.Documents.VisibleDockables) - if (d is DecompilerTabPageModel m) + if (d is DecompilerTabPageModel { IsStaticContent: false } m) return m; } return null; diff --git a/ILSpy/NavigationEntry.cs b/ILSpy/NavigationEntry.cs new file mode 100644 index 000000000..9d464e00a --- /dev/null +++ b/ILSpy/NavigationEntry.cs @@ -0,0 +1,111 @@ +// 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.Runtime.CompilerServices; + +using Avalonia.Media; + +using ICSharpCode.ILSpyX.TreeView; + +using ILSpy.ViewModels; + +namespace ILSpy.Navigation +{ + /// + /// One stop on the back/forward stack: a (tab, target) pair where the target is either a + /// tree-node selection or a static-page URI. The tab pointer lets navigation jump back to + /// the document the user was viewing when this entry was recorded. + /// + public abstract class NavigationEntry : IEquatable + { + public TabPageModel Tab { get; } + + protected NavigationEntry(TabPageModel tab) + { + Tab = tab ?? throw new ArgumentNullException(nameof(tab)); + } + + /// Header text shown in the back/forward dropdown. + public abstract object DisplayText { get; } + + /// Icon shown next to the entry in the back/forward dropdown. + public virtual IImage? DisplayIcon => null; + + public abstract bool Equals(NavigationEntry? other); + + public override bool Equals(object? obj) => Equals(obj as NavigationEntry); + + public abstract override int GetHashCode(); + } + + /// + /// History entry for a tree-node selection. Replaying activates the recorded tab and sets + /// the assembly-tree selection to . + /// + public sealed class TreeNodeEntry : NavigationEntry + { + public SharpTreeNode Node { get; } + + public TreeNodeEntry(TabPageModel tab, SharpTreeNode node) + : base(tab) + { + Node = node ?? throw new ArgumentNullException(nameof(node)); + } + + public override object DisplayText => Node.Text ?? string.Empty; + + public override IImage? DisplayIcon => Node.Icon as IImage; + + public override bool Equals(NavigationEntry? other) => + other is TreeNodeEntry t + && ReferenceEquals(t.Tab, Tab) + && ReferenceEquals(t.Node, Node); + + public override int GetHashCode() => HashCode.Combine( + RuntimeHelpers.GetHashCode(Tab), + RuntimeHelpers.GetHashCode(Node)); + } + + /// + /// History entry for a static page (e.g. the About tab). Replaying just reactivates the + /// recorded tab — the tab keeps its content because static pages opt out of being + /// targeted by tree-node decompilation. + /// + public sealed class StaticPageEntry : NavigationEntry + { + public Uri Uri { get; } + + public StaticPageEntry(TabPageModel tab, Uri uri) + : base(tab) + { + Uri = uri ?? throw new ArgumentNullException(nameof(uri)); + } + + public override object DisplayText => Tab.Title ?? Uri.ToString(); + + public override bool Equals(NavigationEntry? other) => + other is StaticPageEntry s + && ReferenceEquals(s.Tab, Tab) + && Equals(s.Uri, Uri); + + public override int GetHashCode() => HashCode.Combine( + RuntimeHelpers.GetHashCode(Tab), + Uri); + } +} diff --git a/ILSpy/NavigationHistory.cs b/ILSpy/NavigationHistory.cs index 5db8f4f1b..5be4a60e9 100644 --- a/ILSpy/NavigationHistory.cs +++ b/ILSpy/NavigationHistory.cs @@ -24,10 +24,10 @@ namespace ILSpy.Navigation /// /// Two-stack browser-style history. Rapid successive calls (within /// 0.5 s) replace the current entry instead of pushing, so a tree refresh that re-selects - /// the same node doesn't pollute the back stack with duplicates. Equality is reference- - /// based — fine for tree nodes which are reused for the lifetime of an assembly load. + /// the same node doesn't pollute the back stack with duplicates. Equality is delegated + /// to the entry's own implementation. /// - internal sealed class NavigationHistory where T : class + internal sealed class NavigationHistory where T : class, IEquatable { const double NavigationSecondsBeforeNewEntry = 0.5; @@ -73,7 +73,7 @@ namespace ILSpy.Navigation var stack = forward ? this.forward : back; if (!stack.Contains(target)) return null; - while (!ReferenceEquals(stack[^1], target)) + while (!stack[^1].Equals(target)) { if (forward) GoForward(); @@ -91,7 +91,7 @@ namespace ILSpy.Navigation } /// Records a new history entry. Discards the forward stack. - public void Record(T node) + public void Record(T entry) { var navigationTime = DateTime.Now; var period = navigationTime - lastNavigationTime; @@ -100,15 +100,15 @@ namespace ILSpy.Navigation { // Rapid successive selections collapse into a single entry — protects against // tree refreshes that re-issue SelectedItem. - current = node; + current = entry; } else { - if (current != null && !ReferenceEquals(current, node)) + if (current != null && !current.Equals(entry)) back.Add(current); // Avoid duplicate stack entries for the same target. - back.RemoveAll(n => ReferenceEquals(n, node)); - current = node; + back.RemoveAll(n => n.Equals(entry)); + current = entry; } forward.Clear(); diff --git a/ILSpy/TextView/DecompilerTabPageModel.cs b/ILSpy/TextView/DecompilerTabPageModel.cs index 3d2b106a2..ee8602464 100644 --- a/ILSpy/TextView/DecompilerTabPageModel.cs +++ b/ILSpy/TextView/DecompilerTabPageModel.cs @@ -141,6 +141,14 @@ namespace ILSpy.TextView IReadOnlyList currentNodes = System.Array.Empty(); + /// + /// True for tabs whose content is a static page (e.g. About) rather than the result + /// of decompiling a tree-node selection. Static tabs are excluded from the lookup + /// that resolves "the current decompile target", so subsequent tree-node clicks open + /// or reuse a different tab and leave the static content intact. + /// + public bool IsStaticContent { get; set; } + [RelayCommand] void CancelDecompilation() { diff --git a/ILSpy/Views/MainToolBar.axaml.cs b/ILSpy/Views/MainToolBar.axaml.cs index 44181b25f..a8660f741 100644 --- a/ILSpy/Views/MainToolBar.axaml.cs +++ b/ILSpy/Views/MainToolBar.axaml.cs @@ -71,14 +71,14 @@ public partial class MainToolBar : UserControl var entries = forward ? vm.DockWorkspace.ForwardHistory : vm.DockWorkspace.BackHistory; // Stacks are oldest-first; reverse so the most recent appears at the top of the menu. // WPF caps the dropdown at 20 entries; mirror that. - foreach (var node in entries.Reverse().Take(MaxHistoryDropdownEntries)) + foreach (var entry in entries.Reverse().Take(MaxHistoryDropdownEntries)) { var item = new MenuItem { - Header = node.Text?.ToString() ?? string.Empty, + Header = entry.DisplayText?.ToString() ?? string.Empty, Command = vm.DockWorkspace.NavigateToHistoryCommand, - CommandParameter = node, + CommandParameter = entry, }; - if (node.Icon is IImage icon) + if (entry.DisplayIcon is { } icon) item.Icon = new Image { Width = 16, Height = 16, Source = icon }; menu.Items.Add(item); }