Browse Source

Address bookmarks review: lifecycle and sentinel fixes

Tidies up the bookmarks feature in response to review feedback:

- BookmarkMargin subscribed to the shared BookmarkManager.Changed event
  in its constructor and never unsubscribed, so the singleton kept a
  closed tab's margin (and its pulse timer) alive. The subscription now
  follows the margin's time in the visual tree.
- LineHighlightAdorner.DisplayLineHighlight always added a fresh renderer
  with its own timers; navigating repeatedly within the highlight
  lifetime stacked them. Existing highlights are now dismissed first.
- ApplyOutput always built a new DecompiledDebugInfo, contradicting the
  property's documented contract (null for non-C#, the Empty sentinel
  when C# yielded no methods). It now honors that contract.
- The throwaway StringWriter used for sequence-point capture is disposed.

The MemberName doc comment still described a stale-token guard that was
deliberately removed: a token can resolve to a compiler-generated member
(e.g. a local function) whose name differs from the stored display name,
and a name check would wrongly reject a valid navigation. Comment fixed
to match the actual token-only navigation.

Assisted-by: Claude:claude-opus-4-8:Claude Code
pull/3839/head
Christoph Wille 3 weeks ago committed by Siegfried Pammer
parent
commit
17e600bdfd
  1. 7
      ILSpy/Bookmarks/Bookmark.cs
  2. 20
      ILSpy/Bookmarks/BookmarkMargin.cs
  3. 6
      ILSpy/TextView/DecompilerTabPageModel.cs
  4. 6
      ILSpy/TextView/LineHighlightAdorner.cs

7
ILSpy/Bookmarks/Bookmark.cs

@ -65,9 +65,10 @@ namespace ICSharpCode.ILSpy.Bookmarks @@ -65,9 +65,10 @@ namespace ICSharpCode.ILSpy.Bookmarks
public int ILOffset { get; init; }
/// <summary>
/// Display name of the anchored member, shown in the pane's "Location" column and used as a
/// stale-token guard: if a reloaded module's token no longer resolves to this member, the
/// bookmark is treated as missing rather than silently pointing at the wrong code.
/// Display name of the anchored member, shown in the pane's "Location" column. Navigation
/// resolves purely by token and does not compare this name: a token can legitimately point at
/// a compiler-generated member (e.g. a local function) whose resolved name differs from this
/// stored display name, and a name check would wrongly reject that valid navigation.
/// </summary>
public required string MemberName { get; init; }

20
ILSpy/Bookmarks/BookmarkMargin.cs

@ -53,12 +53,28 @@ namespace ICSharpCode.ILSpy.Bookmarks @@ -53,12 +53,28 @@ namespace ICSharpCode.ILSpy.Bookmarks
{
this.owner = owner;
manager = AppEnv.AppComposition.TryGetExport<BookmarkManager>();
if (manager != null)
manager.Changed += OnBookmarksChanged;
pulseTimer = new DispatcherTimer { Interval = TimeSpan.FromMilliseconds(16) };
pulseTimer.Tick += OnPulseTick;
}
// The manager is a shared singleton, so its Changed event would otherwise keep a closed tab's
// margin (and its pulse timer) alive. Track the subscription to the margin's time in the tree.
protected override void OnAttachedToVisualTree(VisualTreeAttachmentEventArgs e)
{
base.OnAttachedToVisualTree(e);
if (manager != null)
manager.Changed += OnBookmarksChanged;
}
protected override void OnDetachedFromVisualTree(VisualTreeAttachmentEventArgs e)
{
base.OnDetachedFromVisualTree(e);
if (manager != null)
manager.Changed -= OnBookmarksChanged;
pulseTimer.Stop();
pulseLine = -1;
}
void OnBookmarksChanged(object? sender, EventArgs e) => InvalidateVisual();
/// <summary>Plays the one-shot bounce on the bookmark glyph at <paramref name="line"/>.</summary>

6
ILSpy/TextView/DecompilerTabPageModel.cs

@ -734,7 +734,11 @@ namespace ICSharpCode.ILSpy.TextView @@ -734,7 +734,11 @@ namespace ICSharpCode.ILSpy.TextView
Foldings = output.Foldings;
References = output.References;
DefinitionLookup = output.DefinitionLookup;
DebugInfo = new Bookmarks.DecompiledDebugInfo(output.MethodDebugInfos);
DebugInfo = syntaxExtension != ".cs"
? null
: output.MethodDebugInfos.Count > 0
? new Bookmarks.DecompiledDebugInfo(output.MethodDebugInfos)
: Bookmarks.DecompiledDebugInfo.Empty;
UIElements = output.UIElements;
Text = text;
}

6
ILSpy/TextView/LineHighlightAdorner.cs

@ -18,6 +18,7 @@ @@ -18,6 +18,7 @@
using System;
using System.Diagnostics;
using System.Linq;
using AvaloniaEdit.Editing;
using AvaloniaEdit.Rendering;
@ -89,6 +90,11 @@ namespace ICSharpCode.ILSpy.TextView @@ -89,6 +90,11 @@ namespace ICSharpCode.ILSpy.TextView
{
ArgumentNullException.ThrowIfNull(textArea);
// Clear any still-running highlight first, so quick successive navigations replace rather
// than stack adorners (each carries its own pair of timers driving redraws).
foreach (var existing in textArea.TextView.BackgroundRenderers.OfType<LineHighlightAdorner>().ToArray())
existing.Dismiss();
var adorner = new LineHighlightAdorner(textArea, line);
textArea.TextView.BackgroundRenderers.Add(adorner);
adorner.frameTimer.Start();

Loading…
Cancel
Save