Browse Source

De-duplicate bookmark match/folding helpers; enabled glyph wins on a shared line

- Extract MatchesBookmark for the token + assembly identity that tied a bookmark
  to a member; GetLineForBookmark and DocumentContainsBookmarkToken each spelled
  it out (four copies), so a change to the match rule had to be made everywhere.
- Add a FoldingSection overload to FoldingsViewState.Restore and route the
  bookmark folding restore through it, dropping a duplicated checksum + match loop.
- When two bookmarks resolve to the same gutter line, draw the enabled glyph so
  the line never reads as disabled while an active bookmark sits on it.
- Drop an out-of-band reference in a test comment.

Assisted-by: Claude:claude-opus-4-8:Claude Code
pull/3839/head
Siegfried Pammer 1 week ago committed by Siegfried Pammer
parent
commit
fc9d945e95
  1. 2
      ILSpy.Tests/Bookmarks/BookmarkNavigationStepTests.cs
  2. 7
      ILSpy/Bookmarks/BookmarkMargin.cs
  3. 48
      ILSpy/TextView/DecompilerTextView.axaml.cs
  4. 32
      ILSpy/TextView/FoldingsViewState.cs

2
ILSpy.Tests/Bookmarks/BookmarkNavigationStepTests.cs

@ -30,7 +30,7 @@ namespace ICSharpCode.ILSpy.Tests.Bookmarks;
[TestFixture] [TestFixture]
public class BookmarkNavigationStepTests public class BookmarkNavigationStepTests
{ {
// Three bookmarks, the middle one disabled (the reported scenario). // Three bookmarks with the middle one disabled, so next/previous must step over it.
static readonly IReadOnlyList<bool> ThreeMiddleDisabled = new[] { true, false, true }; static readonly IReadOnlyList<bool> ThreeMiddleDisabled = new[] { true, false, true };
[Test] [Test]

7
ILSpy/Bookmarks/BookmarkMargin.cs

@ -210,7 +210,12 @@ namespace ICSharpCode.ILSpy.Bookmarks
{ {
if (bookmark.LineNumber != line) if (bookmark.LineNumber != line)
Dispatcher.UIThread.Post(() => bookmark.UpdateRenderedLineNumber(line), DispatcherPriority.Background); Dispatcher.UIThread.Post(() => bookmark.UpdateRenderedLineNumber(line), DispatcherPriority.Background);
map[line] = bookmark.Enabled ? Images.Bookmark : Images.BookmarkDisable; // When two bookmarks resolve to the same line, an enabled one wins so the line never
// reads as disabled while an active bookmark sits on it.
if (bookmark.Enabled)
map[line] = Images.Bookmark;
else if (!map.ContainsKey(line))
map[line] = Images.BookmarkDisable;
} }
} }
return glyphByLine = map; return glyphByLine = map;

48
ILSpy/TextView/DecompilerTextView.axaml.cs

@ -821,30 +821,9 @@ namespace ICSharpCode.ILSpy.TextView
} }
void RestoreBookmarkFoldings(Bookmarks.BookmarkViewState viewState) void RestoreBookmarkFoldings(Bookmarks.BookmarkViewState viewState)
=> RestoreCurrentFoldings(viewState.ToDecompilerTextViewState().Foldings);
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) if (viewState.ToDecompilerTextViewState().Foldings is { } saved && activeFoldingManager is { } manager)
{ FoldingsViewState.Restore(manager.AllFoldings, saved);
wasExpanded = true;
break;
}
}
folding.IsFolded = !wasExpanded;
}
} }
// Scrolls so <paramref name="line"/> sits in the middle of the viewport. AvaloniaEdit's // Scrolls so <paramref name="line"/> sits in the middle of the viewport. AvaloniaEdit's
@ -888,7 +867,7 @@ namespace ICSharpCode.ILSpy.TextView
return null; return null;
foreach (var method in debug.Methods) foreach (var method in debug.Methods)
{ {
if (method.Token == bookmark.Token && method.AssemblyFullName == bookmark.AssemblyFullName if (MatchesBookmark(method, bookmark)
&& method.TryGetLineForOffset(bookmark.ILOffset, out var bodyLine)) && method.TryGetLineForOffset(bookmark.ILOffset, out var bodyLine))
{ {
if (updateRenderedLine) if (updateRenderedLine)
@ -912,9 +891,7 @@ namespace ICSharpCode.ILSpy.TextView
{ {
foreach (var segment in references) foreach (var segment in references)
{ {
if (segment.IsDefinition && segment.Reference is IEntity entity if (segment.IsDefinition && segment.Reference is IEntity entity && MatchesBookmark(entity, bookmark))
&& (uint)System.Reflection.Metadata.Ecma335.MetadataTokens.GetToken(entity.MetadataToken) == bookmark.Token
&& entity.ParentModule?.MetadataFile?.FullName == bookmark.AssemblyFullName)
{ {
var line = Editor.Document.GetLineByOffset(segment.StartOffset).LineNumber; var line = Editor.Document.GetLineByOffset(segment.StartOffset).LineNumber;
if (updateRenderedLine) if (updateRenderedLine)
@ -926,13 +903,24 @@ namespace ICSharpCode.ILSpy.TextView
return null; return null;
} }
// The token + assembly identity that ties a bookmark to a member in the current document. The
// module identity is checked too, so a token value shared across assemblies can't match the
// wrong member. Both forms (the body-anchor debug map and a reference segment's entity) are
// kept in step here rather than spelled out at each call site.
static bool MatchesBookmark(Bookmarks.MethodDebugInfo method, Bookmarks.Bookmark bookmark)
=> method.Token == bookmark.Token && method.AssemblyFullName == bookmark.AssemblyFullName;
static bool MatchesBookmark(IEntity entity, Bookmarks.Bookmark bookmark)
=> (uint)MetadataTokens.GetToken(entity.MetadataToken) == bookmark.Token
&& entity.ParentModule?.MetadataFile?.FullName == bookmark.AssemblyFullName;
static bool DocumentContainsBookmarkToken(DecompilerTabPageModel model, Bookmarks.Bookmark bookmark) static bool DocumentContainsBookmarkToken(DecompilerTabPageModel model, Bookmarks.Bookmark bookmark)
{ {
if (model.DebugInfo != null) if (model.DebugInfo != null)
{ {
foreach (var method in model.DebugInfo.Methods) foreach (var method in model.DebugInfo.Methods)
{ {
if (method.Token == bookmark.Token && method.AssemblyFullName == bookmark.AssemblyFullName) if (MatchesBookmark(method, bookmark))
return true; return true;
} }
} }
@ -941,9 +929,7 @@ namespace ICSharpCode.ILSpy.TextView
{ {
foreach (var segment in model.References) foreach (var segment in model.References)
{ {
if (segment.Reference is IEntity entity if (segment.Reference is IEntity entity && MatchesBookmark(entity, bookmark))
&& (uint)MetadataTokens.GetToken(entity.MetadataToken) == bookmark.Token
&& entity.ParentModule?.MetadataFile?.FullName == bookmark.AssemblyFullName)
return true; return true;
} }
} }

32
ILSpy/TextView/FoldingsViewState.cs

@ -88,19 +88,33 @@ namespace ICSharpCode.ILSpy.TextView
if (checksum != saved.Checksum) if (checksum != saved.Checksum)
return false; return false;
foreach (var folding in newFoldings) foreach (var folding in newFoldings)
folding.DefaultClosed = !IsExpanded(saved, folding.StartOffset, folding.EndOffset);
return true;
}
/// <summary>
/// Applies <paramref name="saved"/> to live <paramref name="foldings"/> already installed in a
/// FoldingManager by toggling <see cref="FoldingSection.IsFolded"/>. Returns <c>true</c> when the
/// layout still matches the snapshot; <c>false</c> (leaving the foldings untouched) on a mismatch.
/// </summary>
public static bool Restore(IEnumerable<FoldingSection> foldings, Snapshot saved)
{ {
bool wasExpanded = false; var sections = foldings as IReadOnlyList<FoldingSection> ?? foldings.ToList();
foreach (var (start, end) in saved.Expanded) if (Capture(sections).Checksum != saved.Checksum)
return false;
foreach (var folding in sections)
folding.IsFolded = !IsExpanded(saved, folding.StartOffset, folding.EndOffset);
return true;
}
static bool IsExpanded(Snapshot saved, int start, int end)
{ {
if (start == folding.StartOffset && end == folding.EndOffset) foreach (var (s, e) in saved.Expanded)
{ {
wasExpanded = true; if (s == start && e == end)
break;
}
}
folding.DefaultClosed = !wasExpanded;
}
return true; return true;
} }
return false;
}
} }
} }

Loading…
Cancel
Save