From 8997b42f2b59cfd7694b123e236c356f2c75953d Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sat, 27 Jun 2026 07:08:06 +0200 Subject: [PATCH] Complete bookmark backlog follow-ups Bookmarks now capture and restore the requested decompiler view state, support rendered-line fallback anchors, expose hover and pane location affordances, route bookmark context-menu actions through the clicked offset, and update the JSON sidecar under a mutex so concurrent instances do not overwrite unrelated bookmark edits. The plan file records the completed checklist and the remaining manual smoke-test gap. Assisted-by: CodeAlta:gpt-5.5:CodeAlta --- .../Bookmarks/BookmarkAnchoringTests.cs | 10 +- .../Bookmarks/BookmarkContextMenuTests.cs | 138 +++++++++++++++ ILSpy.Tests/Bookmarks/BookmarkGutterTests.cs | 137 +++++++++++++++ ILSpy.Tests/Bookmarks/BookmarkManagerTests.cs | 129 +++++++++++++- .../Bookmarks/BookmarksPaneStructureTests.cs | 79 +++++++++ ILSpy/App.axaml | 4 + ILSpy/AppEnv/ConfigurationFiles.cs | 4 +- ILSpy/Assets/Icons/Bookmark.GroupDisable.svg | 1 - ILSpy/Assets/Icons/Bookmark.Next.Folder.svg | 1 - .../Assets/Icons/Bookmark.Previous.Folder.svg | 42 ----- ILSpy/Assets/Icons/Bookmark.Window.svg | 1 - ILSpy/Bookmarks/Bookmark.cs | 56 +++++- ILSpy/Bookmarks/BookmarkAnchoring.cs | 48 +++++- ILSpy/Bookmarks/BookmarkManager.cs | 160 ++++++++++++++---- ILSpy/Bookmarks/BookmarkMargin.cs | 115 +++++++++++-- ILSpy/Bookmarks/BookmarkNavigator.cs | 43 ++--- ILSpy/Bookmarks/BookmarkViewState.cs | 100 +++++++++++ ILSpy/Bookmarks/BookmarksPane.axaml | 129 ++++++++------ ILSpy/Bookmarks/BookmarksPaneModel.cs | 3 - .../ToggleBookmarkContextMenuEntry.cs | 12 +- ILSpy/Images.cs | 2 +- ILSpy/TextView/DecompilerTabPageModel.cs | 3 - ILSpy/TextView/DecompilerTextView.axaml.cs | 148 +++++++++++++--- 23 files changed, 1146 insertions(+), 219 deletions(-) create mode 100644 ILSpy.Tests/Bookmarks/BookmarkContextMenuTests.cs create mode 100644 ILSpy.Tests/Bookmarks/BookmarkGutterTests.cs create mode 100644 ILSpy.Tests/Bookmarks/BookmarksPaneStructureTests.cs delete mode 100644 ILSpy/Assets/Icons/Bookmark.GroupDisable.svg delete mode 100644 ILSpy/Assets/Icons/Bookmark.Next.Folder.svg delete mode 100644 ILSpy/Assets/Icons/Bookmark.Previous.Folder.svg delete mode 100644 ILSpy/Assets/Icons/Bookmark.Window.svg create mode 100644 ILSpy/Bookmarks/BookmarkViewState.cs diff --git a/ILSpy.Tests/Bookmarks/BookmarkAnchoringTests.cs b/ILSpy.Tests/Bookmarks/BookmarkAnchoringTests.cs index a2581e404..4e1a7a5aa 100644 --- a/ILSpy.Tests/Bookmarks/BookmarkAnchoringTests.cs +++ b/ILSpy.Tests/Bookmarks/BookmarkAnchoringTests.cs @@ -70,7 +70,13 @@ public class BookmarkAnchoringTests body.Should().NotBeNull("statement lines must produce a body anchor"); token.Should().NotBeNull("definition lines must produce a token anchor"); - // Line 1 is the "// " comment: neither a statement nor a definition. - BookmarkAnchoring.CreateForLine(debugInfo, output.References, document, 1).Should().BeNull(); + + // Line 1 is the "// " comment: neither a statement nor a definition, + // so it falls back to the visible line in the current decompiled document. + var fallback = BookmarkAnchoring.CreateForLine(debugInfo, output.References, document, 1, type); + fallback.Should().NotBeNull(); + fallback!.Kind.Should().Be(BookmarkKind.Line); + fallback.LineNumber.Should().Be(1); + fallback.Token.Should().Be((uint)System.Reflection.Metadata.Ecma335.MetadataTokens.GetToken(type.MetadataToken)); } } diff --git a/ILSpy.Tests/Bookmarks/BookmarkContextMenuTests.cs b/ILSpy.Tests/Bookmarks/BookmarkContextMenuTests.cs new file mode 100644 index 000000000..1acc4430f --- /dev/null +++ b/ILSpy.Tests/Bookmarks/BookmarkContextMenuTests.cs @@ -0,0 +1,138 @@ +// 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.Linq; +using System.Reflection; +using System.Threading.Tasks; + +using Avalonia.Headless.NUnit; +using Avalonia.Threading; + +using AvaloniaEdit.Document; + +using AwesomeAssertions; + +using ICSharpCode.ILSpy; +using ICSharpCode.ILSpy.AppEnv; +using ICSharpCode.ILSpy.Bookmarks; +using ICSharpCode.ILSpy.Properties; +using ICSharpCode.ILSpy.TextView; +using ICSharpCode.ILSpy.TreeNodes; + +using NUnit.Framework; + +namespace ICSharpCode.ILSpy.Tests.Bookmarks; + +[TestFixture] +public class BookmarkContextMenuTests +{ + [AvaloniaTest] + public async Task Toggle_Bookmark_Entry_Acts_On_The_Right_Clicked_Line_Not_The_Caret() + { + var (window, vm) = await TestHarness.BootAsync(3); + var manager = AppComposition.Current.GetExport(); + manager.Clear(); + + var coreLibName = typeof(object).Assembly.GetName().Name!; + var typeNode = vm.AssemblyTreeModel.FindNode(coreLibName, "System", "System.Object"); + vm.AssemblyTreeModel.SelectNode(typeNode); + await vm.DockWorkspace.WaitForDecompiledTextAsync(); + + var view = await window.WaitForComponent(); + for (int i = 0; i < 8; i++) + { + Dispatcher.UIThread.RunJobs(); + await Task.Delay(25); + } + + var bookmarkableLines = Enumerable.Range(1, view.Editor.Document.LineCount) + .Where(view.CanToggleBookmarkAtLine) + .Take(2) + .ToArray(); + bookmarkableLines.Should().HaveCount(2, "two distinct bookmarkable lines are needed to tell the caret from the click"); + + int caretLine = bookmarkableLines[0]; + int clickedLine = bookmarkableLines[1]; + view.Editor.TextArea.Caret.Offset = view.Editor.Document.GetLineByNumber(caretLine).Offset; + int clickedOffset = view.Editor.Document.GetLineByNumber(clickedLine).Offset; + + var toggle = AppComposition.Current.GetExport() + .GetEntry(nameof(Resources.BookmarkToggle)); + toggle.Execute(new TextViewContext { TextView = view, TextLocation = clickedOffset }); + Dispatcher.UIThread.RunJobs(); + + manager.Bookmarks.Should().ContainSingle(); + view.GetLineForBookmark(manager.Bookmarks[0]).Should().Be(clickedLine); + manager.Bookmarks[0].LocationNodeName.Should().Be("System.Object"); + } + + [AvaloniaTest] + public async Task Navigation_Does_Not_Fall_Back_To_Entity_When_Saved_Tree_Path_Is_Missing() + { + var (window, vm) = await TestHarness.BootAsync(3); + var manager = AppComposition.Current.GetExport(); + manager.Clear(); + + var coreLibName = typeof(object).Assembly.GetName().Name!; + var objectNode = vm.AssemblyTreeModel.FindNode(coreLibName, "System", "System.Object"); + vm.AssemblyTreeModel.SelectNode(objectNode); + await vm.DockWorkspace.WaitForDecompiledTextAsync(); + + var view = await window.WaitForComponent(); + for (int i = 0; i < 8; i++) + { + Dispatcher.UIThread.RunJobs(); + await Task.Delay(25); + } + + int line = Enumerable.Range(1, view.Editor.Document.LineCount) + .First(view.CanToggleBookmarkAtLine); + int offset = view.Editor.Document.GetLineByNumber(line).Offset; + AppComposition.Current.GetExport() + .GetEntry(nameof(Resources.BookmarkToggle)) + .Execute(new TextViewContext { TextView = view, TextLocation = offset }); + Dispatcher.UIThread.RunJobs(); + + manager.Bookmarks.Should().ContainSingle(); + var bookmark = manager.Bookmarks[0]; + bookmark.ViewState.Should().NotBeNull(); + bookmark.ViewState = bookmark.ViewState! with { SelectedTreeNodePath = new[] { "Missing assembly", "Missing type" } }; + + var stringNode = vm.AssemblyTreeModel.FindNode(coreLibName, "System", "System.String"); + vm.AssemblyTreeModel.SelectNode(stringNode); + + await AppComposition.Current.GetExport().NavigateToAsync(bookmark); + + ((object?)vm.AssemblyTreeModel.SelectedItem).Should().BeSameAs(stringNode); + } + + [AvaloniaTest] + public async Task Queued_Bookmark_Scroll_Ignores_A_Replaced_Document() + { + var (window, _) = await TestHarness.BootAsync(3); + var view = await window.WaitForComponent(); + view.Editor.Document = new TextDocument(string.Join("\n", Enumerable.Range(1, 30).Select(i => $"line {i}"))); + + var scrollToLine = typeof(DecompilerTextView).GetMethod("ScrollToLine", BindingFlags.Instance | BindingFlags.NonPublic); + scrollToLine.Should().NotBeNull(); + scrollToLine!.Invoke(view, new object?[] { 20, null }); + + view.Editor.Document = new TextDocument("replacement"); + Dispatcher.UIThread.RunJobs(); + } +} diff --git a/ILSpy.Tests/Bookmarks/BookmarkGutterTests.cs b/ILSpy.Tests/Bookmarks/BookmarkGutterTests.cs new file mode 100644 index 000000000..5b50d41e1 --- /dev/null +++ b/ILSpy.Tests/Bookmarks/BookmarkGutterTests.cs @@ -0,0 +1,137 @@ +// 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.Linq; +using System.Threading.Tasks; + +using Avalonia; +using Avalonia.Headless; +using Avalonia.Headless.NUnit; +using Avalonia.Input; +using Avalonia.Interactivity; +using Avalonia.Threading; + +using AvaloniaEdit.Rendering; + +using AwesomeAssertions; + +using ICSharpCode.ILSpy.AppEnv; +using ICSharpCode.ILSpy.Bookmarks; +using ICSharpCode.ILSpy.TextView; +using ICSharpCode.ILSpy.TreeNodes; + +using NUnit.Framework; + +namespace ICSharpCode.ILSpy.Tests.Bookmarks; + +[TestFixture] +public class BookmarkGutterTests +{ + [AvaloniaTest] + public async Task Clicking_Bookmark_Gutter_Toggles_The_Visible_Line() + { + var (window, vm) = await TestHarness.BootAsync(3); + var manager = AppComposition.Current.GetExport(); + manager.Clear(); + + var coreLibName = typeof(object).Assembly.GetName().Name!; + var typeNode = vm.AssemblyTreeModel.FindNode(coreLibName, "System", "System.Object"); + vm.AssemblyTreeModel.SelectNode(typeNode); + await vm.DockWorkspace.WaitForDecompiledTextAsync(); + + var view = await window.WaitForComponent(); + for (int i = 0; i < 8; i++) + { + Dispatcher.UIThread.RunJobs(); + await Task.Delay(25); + } + window.UpdateLayout(); + + var margin = view.Editor.TextArea.LeftMargins.OfType().Single(); + margin.Bounds.Width.Should().BeGreaterThan(0, "the bookmark gutter must reserve hit-testable space"); + + var textView = view.Editor.TextArea.TextView; + textView.EnsureVisualLines(); + var visualLine = textView.VisualLines.First(line => view.CanToggleBookmarkAtLine(line.FirstDocumentLine.LineNumber)); + int documentLine = visualLine.FirstDocumentLine.LineNumber; + double y = visualLine.GetTextLineVisualYPosition(visualLine.TextLines[0], VisualYPosition.LineMiddle) - textView.VerticalOffset; + y.Should().BeInRange(0, margin.Bounds.Height, "the selected visual line must be inside the gutter bounds"); + var point = margin.TranslatePoint(new Point(margin.Bounds.Width / 2, y), window); + point.Should().NotBeNull("the gutter line coordinate must map into the test window"); + + int pressed = 0; + margin.AddHandler(InputElement.PointerPressedEvent, (_, _) => pressed++, RoutingStrategies.Tunnel | RoutingStrategies.Bubble); + + window.MouseDown(point!.Value, MouseButton.Left); + window.MouseUp(point.Value, MouseButton.Left); + Dispatcher.UIThread.RunJobs(); + + pressed.Should().BeGreaterThan(0, "the real pointer event must hit the bookmark gutter"); + manager.Bookmarks.Should().ContainSingle(); + view.GetLineForBookmark(manager.Bookmarks[0]).Should().Be(documentLine); + + window.MouseDown(point.Value, MouseButton.Left); + window.MouseUp(point.Value, MouseButton.Left); + Dispatcher.UIThread.RunJobs(); + + manager.Bookmarks.Should().BeEmpty(); + } + + [AvaloniaTest] + public async Task RightClicking_Bookmark_Gutter_Does_Not_Toggle() + { + var (window, vm) = await TestHarness.BootAsync(3); + var manager = AppComposition.Current.GetExport(); + manager.Clear(); + + var coreLibName = typeof(object).Assembly.GetName().Name!; + var typeNode = vm.AssemblyTreeModel.FindNode(coreLibName, "System", "System.Object"); + vm.AssemblyTreeModel.SelectNode(typeNode); + await vm.DockWorkspace.WaitForDecompiledTextAsync(); + + var view = await window.WaitForComponent(); + for (int i = 0; i < 8; i++) + { + Dispatcher.UIThread.RunJobs(); + await Task.Delay(25); + } + window.UpdateLayout(); + + var margin = view.Editor.TextArea.LeftMargins.OfType().Single(); + var textView = view.Editor.TextArea.TextView; + textView.EnsureVisualLines(); + var visualLine = textView.VisualLines.First(line => view.CanToggleBookmarkAtLine(line.FirstDocumentLine.LineNumber)); + double y = visualLine.GetTextLineVisualYPosition(visualLine.TextLines[0], VisualYPosition.LineMiddle) - textView.VerticalOffset; + var point = margin.TranslatePoint(new Point(margin.Bounds.Width / 2, y), window); + point.Should().NotBeNull("the gutter line coordinate must map into the test window"); + + window.MouseDown(point!.Value, MouseButton.Right); + window.MouseUp(point.Value, MouseButton.Right); + Dispatcher.UIThread.RunJobs(); + + manager.Bookmarks.Should().BeEmpty("a right-click in the gutter must not toggle a bookmark"); + + // The same coordinate still toggles on a left-click, proving the gesture is button-gated + // rather than disabled at that line. + window.MouseDown(point.Value, MouseButton.Left); + window.MouseUp(point.Value, MouseButton.Left); + Dispatcher.UIThread.RunJobs(); + + manager.Bookmarks.Should().ContainSingle("a left-click at the same coordinate still toggles"); + } +} diff --git a/ILSpy.Tests/Bookmarks/BookmarkManagerTests.cs b/ILSpy.Tests/Bookmarks/BookmarkManagerTests.cs index b96e2f0f7..ad782a76e 100644 --- a/ILSpy.Tests/Bookmarks/BookmarkManagerTests.cs +++ b/ILSpy.Tests/Bookmarks/BookmarkManagerTests.cs @@ -70,7 +70,7 @@ public class BookmarkManagerTests return new BookmarkManager(); } - static Bookmark MakeBookmark(uint token, BookmarkKind kind = BookmarkKind.Token, int ilOffset = 0, string name = "") + static Bookmark MakeBookmark(uint token, BookmarkKind kind = BookmarkKind.Token, int ilOffset = 0, string name = "", int lineNumber = 1, string? locationNodeName = null) => new() { Name = name, FileName = @"C:\asm\Sample.dll", @@ -79,7 +79,9 @@ public class BookmarkManagerTests Token = token, Kind = kind, ILOffset = ilOffset, + LineNumber = lineNumber, MemberName = "Sample.Type.Member", + LocationNodeName = locationNodeName, }; [Test] @@ -116,6 +118,17 @@ public class BookmarkManagerTests manager.Bookmarks.Should().HaveCount(2); } + [Test] + public void Line_anchors_with_different_visible_lines_are_distinct() + { + var manager = new BookmarkManager(); + + manager.Toggle(MakeBookmark(0x02000001, BookmarkKind.Line, lineNumber: 3)); + manager.Toggle(MakeBookmark(0x02000001, BookmarkKind.Line, lineNumber: 4)); + + manager.Bookmarks.Should().HaveCount(2); + } + [Test] public void Saved_bookmarks_reload_in_a_fresh_manager() { @@ -132,6 +145,120 @@ public class BookmarkManagerTests reloaded.ILOffset.Should().Be(4); } + [Test] + public void Saved_line_bookmarks_reload_in_a_fresh_manager() + { + var first = new BookmarkManager(); + first.Toggle(MakeBookmark(0x02000001, BookmarkKind.Line, lineNumber: 7, name: "Header", locationNodeName: "Sample.Type")); + + var second = new BookmarkManager(); + + second.Bookmarks.Should().ContainSingle(); + var reloaded = second.Bookmarks[0]; + reloaded.Kind.Should().Be(BookmarkKind.Line); + reloaded.LineNumber.Should().Be(7); + reloaded.LocationNodeName.Should().Be("Sample.Type"); + } + + [Test] + public void Saved_bookmarks_reload_their_view_state_payload() + { + var first = new BookmarkManager(); + var bookmark = MakeBookmark(0x06000001, name: "With view"); + bookmark.ViewState = new BookmarkViewState(1, 42, 120.5, 7.25, 100, + new[] { new BookmarkFoldingRange(10, 20) }, SelectedTreeNodePath: new[] { "Sample", "Sample.Type" }); + first.Toggle(bookmark); + + var second = new BookmarkManager(); + + second.Bookmarks.Should().ContainSingle(); + second.Bookmarks[0].ViewState.Should().NotBeNull(); + second.Bookmarks[0].ViewState!.CaretOffset.Should().Be(42); + second.Bookmarks[0].ViewState!.ExpandedFoldings.Should().ContainSingle() + .Which.Should().Be(new BookmarkFoldingRange(10, 20)); + second.Bookmarks[0].ViewState!.SelectedTreeNodePath.Should().Equal("Sample", "Sample.Type"); + } + + [Test] + public void Display_location_shows_node_name_and_line() + { + var bookmark = MakeBookmark(0x02000001, BookmarkKind.Line, lineNumber: 7, locationNodeName: "Sample.Type.Node"); + bookmark.ViewState = new BookmarkViewState(1, 42, 120.5, 7.25, null, null, + SelectedTreeNodePath: new[] { "Sample", "Sample.Type" }); + + bookmark.DisplayLocation.Should().Be("Sample.Type.Node:7"); + } + + [Test] + public void Display_location_uses_resolved_rendered_line_when_it_was_not_stored() + { + var bookmark = MakeBookmark(0x02000001, BookmarkKind.Token, lineNumber: 0, locationNodeName: "Sample.Type.Node"); + + bookmark.DisplayLocation.Should().Be("Sample.Type.Node"); + + bookmark.UpdateRenderedLineNumber(59); + + bookmark.LineNumber.Should().Be(59); + bookmark.DisplayLocation.Should().Be("Sample.Type.Node:59"); + } + + [Test] + public void Two_managers_adding_different_bookmarks_preserve_both_entries() + { + var first = new BookmarkManager(); + var second = new BookmarkManager(); + + first.Toggle(MakeBookmark(0x06000001, name: "First")); + second.Toggle(MakeBookmark(0x06000002, name: "Second")); + + var reloaded = new BookmarkManager(); + + reloaded.Bookmarks.Select(b => b.Name).Should().BeEquivalentTo("First", "Second"); + } + + [Test] + public void Two_managers_editing_the_same_anchor_keep_the_later_value() + { + var first = new BookmarkManager(); + first.Toggle(MakeBookmark(0x06000001, name: "Original")); + var second = new BookmarkManager(); + + first.Bookmarks[0].Name = "First edit"; + second.Bookmarks[0].Name = "Second edit"; + + var reloaded = new BookmarkManager(); + + reloaded.Bookmarks.Should().ContainSingle(); + reloaded.Bookmarks[0].Name.Should().Be("Second edit"); + } + + [Test] + public void Two_managers_remove_and_add_preserve_the_added_bookmark() + { + var first = new BookmarkManager(); + first.Toggle(MakeBookmark(0x06000001, name: "Remove me")); + var second = new BookmarkManager(); + + first.Remove(first.Bookmarks[0]); + second.Toggle(MakeBookmark(0x06000002, name: "Keep me")); + + var reloaded = new BookmarkManager(); + + reloaded.Bookmarks.Select(b => b.Name).Should().Equal("Keep me"); + } + + [Test] + public void Malformed_bookmark_file_recovers_on_next_update() + { + File.WriteAllText(Path.Combine(tempDir, "ILSpy.Bookmarks.json"), "not json"); + var manager = new BookmarkManager(); + + manager.Toggle(MakeBookmark(0x06000001, name: "Recovered")); + + var reloaded = new BookmarkManager(); + reloaded.Bookmarks.Select(b => b.Name).Should().Equal("Recovered"); + } + [Test] public void Bookmarks_without_a_file_are_dropped_on_load() { diff --git a/ILSpy.Tests/Bookmarks/BookmarksPaneStructureTests.cs b/ILSpy.Tests/Bookmarks/BookmarksPaneStructureTests.cs new file mode 100644 index 000000000..39dae06f5 --- /dev/null +++ b/ILSpy.Tests/Bookmarks/BookmarksPaneStructureTests.cs @@ -0,0 +1,79 @@ +// 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.Linq; + +using Avalonia.Controls; +using Avalonia.Headless.NUnit; +using Avalonia.Threading; + +using AwesomeAssertions; + +using ICSharpCode.ILSpy.Bookmarks; +using ICSharpCode.ILSpy.Properties; +using ICSharpCode.ILSpy.Views.Controls; + +using NUnit.Framework; + +namespace ICSharpCode.ILSpy.Tests.Bookmarks; + +[TestFixture] +public class BookmarksPaneStructureTests +{ + [AvaloniaTest] + public void Toolbar_uses_main_toolbar_chrome_and_button_content() + { + var pane = new BookmarksPane(); + var toolbarBorder = pane.FindControl("ToolbarBorder")!; + var toolbarRoot = pane.FindControl("ToolbarRoot")!; + + toolbarBorder.BorderThickness.Should().Be(new Avalonia.Thickness(0, 0, 0, 1)); + toolbarBorder.MinHeight.Should().Be(29); + toolbarBorder.Padding.Should().Be(new Avalonia.Thickness(3)); + toolbarRoot.Children.OfType().Should().HaveCount(2); + toolbarRoot.Children.OfType - - - - - - - - - - - - + + + + + + + + + + + + + + - - + Binding="{Binding DisplayLocation}" IsReadOnly="True" /> + + + + + + + diff --git a/ILSpy/Bookmarks/BookmarksPaneModel.cs b/ILSpy/Bookmarks/BookmarksPaneModel.cs index a8c7d4c7f..6d86583de 100644 --- a/ILSpy/Bookmarks/BookmarksPaneModel.cs +++ b/ILSpy/Bookmarks/BookmarksPaneModel.cs @@ -71,9 +71,6 @@ namespace ICSharpCode.ILSpy.Bookmarks return navigator.NavigateToAsync(bookmark); } - [RelayCommand] - void Toggle() => ActiveTab?.ToggleBookmarkAtCaret?.Invoke(); - [RelayCommand] void Next() => NavigateRelative(1); diff --git a/ILSpy/Bookmarks/ToggleBookmarkContextMenuEntry.cs b/ILSpy/Bookmarks/ToggleBookmarkContextMenuEntry.cs index 4cd184aa7..b8d055834 100644 --- a/ILSpy/Bookmarks/ToggleBookmarkContextMenuEntry.cs +++ b/ILSpy/Bookmarks/ToggleBookmarkContextMenuEntry.cs @@ -24,17 +24,23 @@ namespace ICSharpCode.ILSpy.Bookmarks { /// /// Right-click -> Toggle Bookmark in the decompiled C# view. Only shown on a line that can - /// actually hold a bookmark (a statement or a definition); the text view decides eligibility. + /// hold a bookmark; the text view decides eligibility from the right-clicked text location. /// [ExportContextMenuEntry(Header = nameof(Resources.BookmarkToggle), Category = nameof(Resources.Editor), Order = 120, Icon = "Images/Bookmark")] [Shared] public sealed class ToggleBookmarkContextMenuEntry : IContextMenuEntry { public bool IsVisible(TextViewContext context) - => context.TextView is { } view && view.CanToggleBookmarkAtRightClick; + => context.TextView is { } view + && context.TextLocation is { } offset + && view.CanToggleBookmarkAtOffset(offset); public bool IsEnabled(TextViewContext context) => true; - public void Execute(TextViewContext context) => context.TextView?.ToggleBookmarkAtRightClick(); + public void Execute(TextViewContext context) + { + if (context.TextView is { } view && context.TextLocation is { } offset) + view.ToggleBookmarkAtOffset(offset); + } } } diff --git a/ILSpy/Images.cs b/ILSpy/Images.cs index 66082e5bb..e0bedebb4 100644 --- a/ILSpy/Images.cs +++ b/ILSpy/Images.cs @@ -87,7 +87,7 @@ namespace ICSharpCode.ILSpy public static readonly IImage ShowAll = LoadSvg(nameof(ShowAll)); // Bookmarks (file names carry dots, so they can't use nameof; "Boomark.Disable" is the - // asset's actual — misspelled — file name). + // asset's actual -- misspelled -- file name). public static readonly IImage Bookmark = LoadSvg(nameof(Bookmark)); public static readonly IImage BookmarkDisable = LoadSvg("Boomark.Disable"); public static readonly IImage BookmarkNext = LoadSvg("Bookmark.Next"); diff --git a/ILSpy/TextView/DecompilerTabPageModel.cs b/ILSpy/TextView/DecompilerTabPageModel.cs index bda097570..ecfdc4910 100644 --- a/ILSpy/TextView/DecompilerTabPageModel.cs +++ b/ILSpy/TextView/DecompilerTabPageModel.cs @@ -245,9 +245,6 @@ namespace ICSharpCode.ILSpy.TextView [ObservableProperty] private Bookmarks.Bookmark? pendingBookmark; - /// Toggles a bookmark on the caret line. Set by the text view; used by the bookmarks pane toolbar. - public System.Action? ToggleBookmarkAtCaret { get; set; } - /// Moves to the next (true) / previous (false) bookmark within this document. Set by the text view. public System.Action? NavigateBookmarkInFile { get; set; } diff --git a/ILSpy/TextView/DecompilerTextView.axaml.cs b/ILSpy/TextView/DecompilerTextView.axaml.cs index d7f2cf300..645f4d664 100644 --- a/ILSpy/TextView/DecompilerTextView.axaml.cs +++ b/ILSpy/TextView/DecompilerTextView.axaml.cs @@ -33,6 +33,7 @@ using Avalonia.Media; using Avalonia.Threading; using Avalonia.VisualTree; +using AvaloniaEdit.Document; using AvaloniaEdit.Folding; using AvaloniaEdit.Highlighting; @@ -43,10 +44,13 @@ using ICSharpCode.Decompiler.Metadata; using ICSharpCode.Decompiler.Output; using ICSharpCode.Decompiler.TypeSystem; using ICSharpCode.ILSpyX; +using ICSharpCode.ILSpyX.TreeView; using ICSharpCode.ILSpy; using ICSharpCode.ILSpy.AppEnv; +using ICSharpCode.ILSpy.AssemblyTree; using ICSharpCode.ILSpy.Options; +using ICSharpCode.ILSpy.TreeNodes; namespace ICSharpCode.ILSpy.TextView { @@ -93,7 +97,6 @@ namespace ICSharpCode.ILSpy.TextView // text. Position-relative menu entries (e.g. "Toggle folding") read this so they act on the // clicked line rather than wherever the caret happens to sit. int? lastRightClickedOffset; - int lastRightClickedLine = -1; Bookmarks.BookmarkMargin? bookmarkMargin; IReadOnlyList contextMenuEntries = Array.Empty(); Popup richPopup = null!; @@ -616,7 +619,6 @@ namespace ICSharpCode.ILSpy.TextView if (!e.GetCurrentPoint(Editor.TextArea.TextView).Properties.IsRightButtonPressed) return; var pos = GetPositionFromPointer(e); - lastRightClickedLine = pos?.Line ?? -1; if (pos == null) { lastRightClickedSegment = null; @@ -673,12 +675,31 @@ namespace ICSharpCode.ILSpy.TextView { if (!ShowsBookmarkableCode || DataContext is not DecompilerTabPageModel model) return null; - return Bookmarks.BookmarkAnchoring.CreateForLine(model.DebugInfo, model.References, Editor.Document, line); + var fallbackOwner = model.CurrentNode is IMemberTreeNode memberNode ? memberNode.Member : null; + var bookmark = Bookmarks.BookmarkAnchoring.CreateForLine(model.DebugInfo, model.References, Editor.Document, line, + fallbackOwner, GetLocationNodeName(model.CurrentNode)); + if (bookmark != null) + { + var displaySettings = AppComposition.TryGetExport()?.DisplaySettings; + var selectedTreeNodePath = AssemblyTreeModel.GetPathForNode(model.CurrentNode); + bookmark.ViewState = Bookmarks.BookmarkViewState.From(GetCurrentViewState(), displaySettings, selectedTreeNodePath); + } + return bookmark; } - /// Whether is a statement or definition line that can hold a bookmark. + static string? GetLocationNodeName(SharpTreeNode? node) + => node is IMemberTreeNode { Member: { } member } ? member.FullName : node?.ToString(); + + /// Whether can hold a bookmark in the current document. internal bool CanToggleBookmarkAtLine(int line) => CreateBookmarkForLine(line) != null; + internal bool CanToggleBookmarkAtOffset(int offset) + { + if (offset < 0 || offset > Editor.Document.TextLength) + return false; + return CanToggleBookmarkAtLine(Editor.Document.GetLineByOffset(offset).LineNumber); + } + /// Adds or removes a bookmark on ; a no-op for non-anchorable lines. internal void ToggleBookmarkAtLine(int line) { @@ -686,14 +707,11 @@ namespace ICSharpCode.ILSpy.TextView BookmarkManager?.Toggle(candidate); } - /// True when the line under the last right-click can hold a bookmark; drives the context-menu entry. - internal bool CanToggleBookmarkAtRightClick => lastRightClickedLine >= 1 && CanToggleBookmarkAtLine(lastRightClickedLine); - - /// Toggles a bookmark on the line under the last right-click (the context-menu target). - internal void ToggleBookmarkAtRightClick() + internal void ToggleBookmarkAtOffset(int offset) { - if (lastRightClickedLine >= 1) - ToggleBookmarkAtLine(lastRightClickedLine); + if (offset < 0 || offset > Editor.Document.TextLength) + return; + ToggleBookmarkAtLine(Editor.Document.GetLineByOffset(offset).LineNumber); } void OnBookmarkKeyDown(object? sender, KeyEventArgs e) @@ -705,9 +723,6 @@ namespace ICSharpCode.ILSpy.TextView } } - // Wired onto the model so the bookmarks-pane toolbar can act on the active document. - void ToggleBookmarkAtCaret() => ToggleBookmarkAtLine(Editor.TextArea.Caret.Line); - // Moves the caret to the next/previous bookmark in this document, ordered by line and relative // to the caret (wrapping around). A no-op when the document holds fewer than one bookmark. void NavigateBookmarkInFile(bool forward) @@ -737,32 +752,74 @@ namespace ICSharpCode.ILSpy.TextView return; if (GetLineForBookmark(bookmark) is { } line) { - ScrollToLine(line); + ScrollToLine(line, bookmark.ViewState); model.PendingBookmark = null; } } - void ScrollToLine(int line) + void ScrollToLine(int line, Bookmarks.BookmarkViewState? viewState = null) { - line = Math.Clamp(line, 1, Editor.Document.LineCount); - Editor.TextArea.Caret.Offset = Editor.Document.GetLineByNumber(line).Offset; + var document = Editor.Document; + line = Math.Clamp(line, 1, document.LineCount); + Editor.TextArea.Caret.Offset = document.GetLineByNumber(line).Offset; // Centre the line and pulse its gutter icon once the layout has caught up. Posting lets a // just-applied document finish measuring, so the visual position is accurate either way -- // whether we got here after a fresh decompile or while the document was already on screen. Dispatcher.UIThread.Post(() => { - CenterLineInView(line); + if (!ReferenceEquals(Editor.Document, document) || line < 1 || line > document.LineCount) + return; + CenterLineInView(document, line); LineHighlightAdorner.DisplayLineHighlight(Editor.TextArea, line); bookmarkMargin?.PulseLine(line); + if (viewState != null) + RestoreBookmarkViewState(viewState); }, DispatcherPriority.Background); } + void RestoreBookmarkViewState(Bookmarks.BookmarkViewState viewState) + { + var state = viewState.ToDecompilerTextViewState(); + RestoreCurrentFoldings(state.Foldings); + Editor.TextArea.Caret.Offset = Math.Clamp(state.CaretOffset, 0, Editor.Document.TextLength); + if (EditorScrollViewer is { } scrollViewer) + scrollViewer.Offset = new Vector(state.HorizontalOffset, state.VerticalOffset); + } + + void RestoreCurrentFoldings(FoldingsViewState.Snapshot? saved) + { + if (saved == null || activeFoldingManager is not { } manager) + return; + + var current = FoldingsViewState.Capture(manager.AllFoldings); + if (current.Checksum != saved.Value.Checksum) + return; + + foreach (var folding in manager.AllFoldings) + { + bool wasExpanded = false; + foreach (var (start, end) in saved.Value.Expanded) + { + if (start == folding.StartOffset && end == folding.EndOffset) + { + wasExpanded = true; + break; + } + } + folding.IsFolded = !wasExpanded; + } + } + // Scrolls so sits in the middle of the viewport. AvaloniaEdit's // ScrollTo* are no-ops in 12.0.0 (#594), so set the ScrollViewer offset directly. - void CenterLineInView(int line) + void CenterLineInView(TextDocument document, int line) { if (EditorScrollViewer is not { } scrollViewer) return; + if (!ReferenceEquals(Editor.Document, document) || line < 1 || line > document.LineCount) + return; var textView = Editor.TextArea.TextView; + if (!ReferenceEquals(textView.Document, document)) + return; double visualTop = textView.GetVisualTopByDocumentLine(line); double target = visualTop - (scrollViewer.Viewport.Height - textView.DefaultLineHeight) / 2; scrollViewer.Offset = new Vector(scrollViewer.Offset.X, Math.Max(0, target)); @@ -774,7 +831,7 @@ namespace ICSharpCode.ILSpy.TextView /// via the definition's position. The module identity is verified so a token value shared /// across assemblies can't place an icon on the wrong line. /// - internal int? GetLineForBookmark(Bookmarks.Bookmark bookmark) + internal int? GetLineForBookmark(Bookmarks.Bookmark bookmark, bool updateRenderedLine = true) { if (DataContext is not DecompilerTabPageModel { SyntaxExtension: ".cs" } model) return null; @@ -787,11 +844,24 @@ namespace ICSharpCode.ILSpy.TextView { if (method.Token == bookmark.Token && method.AssemblyFullName == bookmark.AssemblyFullName && method.TryGetLineForOffset(bookmark.ILOffset, out var bodyLine)) + { + if (updateRenderedLine) + bookmark.UpdateRenderedLineNumber(bodyLine); return bodyLine; + } } return null; } + if (bookmark.Kind == Bookmarks.BookmarkKind.Line) + { + if (bookmark.LineNumber < 1 || bookmark.LineNumber > Editor.Document.LineCount) + return null; + if (DocumentContainsBookmarkToken(model, bookmark)) + return bookmark.LineNumber; + return null; + } + if (model.References is { } references) { foreach (var segment in references) @@ -799,12 +869,42 @@ namespace ICSharpCode.ILSpy.TextView if (segment.IsDefinition && segment.Reference is IEntity entity && (uint)System.Reflection.Metadata.Ecma335.MetadataTokens.GetToken(entity.MetadataToken) == bookmark.Token && entity.ParentModule?.MetadataFile?.FullName == bookmark.AssemblyFullName) - return Editor.Document.GetLineByOffset(segment.StartOffset).LineNumber; + { + var line = Editor.Document.GetLineByOffset(segment.StartOffset).LineNumber; + if (updateRenderedLine) + bookmark.UpdateRenderedLineNumber(line); + return line; + } } } return null; } + static bool DocumentContainsBookmarkToken(DecompilerTabPageModel model, Bookmarks.Bookmark bookmark) + { + if (model.DebugInfo != null) + { + foreach (var method in model.DebugInfo.Methods) + { + if (method.Token == bookmark.Token && method.AssemblyFullName == bookmark.AssemblyFullName) + return true; + } + } + + if (model.References != null) + { + foreach (var segment in model.References) + { + if (segment.Reference is IEntity entity + && (uint)MetadataTokens.GetToken(entity.MetadataToken) == bookmark.Token + && entity.ParentModule?.MetadataFile?.FullName == bookmark.AssemblyFullName) + return true; + } + } + + return false; + } + #endregion protected override void OnDataContextChanged(System.EventArgs e) @@ -817,7 +917,6 @@ namespace ICSharpCode.ILSpy.TextView { previous.PropertyChanged -= OnModelPropertyChanged; previous.CaptureViewState = null; - previous.ToggleBookmarkAtCaret = null; previous.NavigateBookmarkInFile = null; } @@ -829,8 +928,7 @@ namespace ICSharpCode.ILSpy.TextView // records a navigation away. (Re)assigning every DataContext-change handles both // the first attach and an ABA reattach. model.CaptureViewState = GetCurrentViewState; - // Bookmarks-pane toolbar actions that operate on the active document route through these. - model.ToggleBookmarkAtCaret = ToggleBookmarkAtCaret; + // Bookmarks-pane toolbar navigation actions that operate on the active document route through this. model.NavigateBookmarkInFile = NavigateBookmarkInFile; ApplyDocument(model); // Point the breadcrumb at this tab's node (the bar owns its own VM, so feed it the