Add a bookmarks feature for the decompiled C# view#3789
Merged
Conversation
44694e5 to
cd1f32f
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a persistent bookmarks feature to ILSpy’s decompiled C# text view, including anchoring bookmarks via metadata token + IL offset (using captured sequence points) so bookmarks survive re-decompilation and formatting reflow.
Changes:
- Introduces bookmark model/persistence (JSON sidecar), navigation, context-menu entry, and a dockable Bookmarks pane (with import/export).
- Captures per-method IL-offset ↔ line maps during C# decompilation to support stable “body” anchors and gutter placement.
- Adds UI affordances: bookmark gutter margin, Ctrl+B toggle, and a one-shot line highlight adorner on navigation; plus new icons/resources and tests.
Reviewed changes
Copilot reviewed 30 out of 42 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| ILSpy/ViewLocator.cs | Registers the new Bookmarks pane for view resolution. |
| ILSpy/TextView/LineHighlightAdorner.cs | Adds a one-shot line highlight renderer used after bookmark navigation. |
| ILSpy/TextView/DecompilerTextView.axaml.cs | Wires bookmark toggling (Ctrl+B / context menu), in-file navigation, scrolling + highlight/pulse, and line resolution for bookmarks. |
| ILSpy/TextView/DecompilerTabPageModel.cs | Stores per-document debug info maps and a pending bookmark for deferred navigation positioning. |
| ILSpy/TextView/AvaloniaEditTextOutput.cs | Exposes current output line and collects per-method debug maps during decompile output. |
| ILSpy/Properties/Resources.resx | Adds localized strings for bookmark UI, commands, and prompts. |
| ILSpy/Properties/Resources.Designer.cs | Regenerates strongly-typed resource accessors for new bookmark strings. |
| ILSpy/Languages/CSharpLanguage.cs | Captures sequence points and builds per-method IL-offset ↔ line maps during C# WriteCode. |
| ILSpy/Images.cs | Adds bookmark-related icon loads for UI toolbars and gutter. |
| ILSpy/Docking/DockWorkspace.cs | Uses a shared sidecar-path helper for dock layout JSON path resolution. |
| ILSpy/Commands/FilePickers.cs | Adds a single-file OpenAsync picker used by bookmark import. |
| ILSpy/Bookmarks/ToggleBookmarkContextMenuEntry.cs | Adds “Toggle Bookmark” editor context menu entry gated by bookmarkable line. |
| ILSpy/Bookmarks/MethodDebugInfo.cs | Implements MethodDebugInfo and DecompiledDebugInfo (IL-offset ↔ line mapping + anchor resolution). |
| ILSpy/Bookmarks/BookmarksPaneModel.cs | Implements Bookmarks tool pane VM: list binding, navigation, enable/disable/delete, import/export. |
| ILSpy/Bookmarks/BookmarksPane.axaml.cs | Handles double-click activation to navigate to a bookmark. |
| ILSpy/Bookmarks/BookmarksPane.axaml | Defines Bookmarks pane UI: toolbar + DataGrid with editable Name and Enabled checkbox. |
| ILSpy/Bookmarks/BookmarkNavigator.cs | Implements bookmark navigation: ensure assembly loaded, resolve token, select tree node, set pending bookmark. |
| ILSpy/Bookmarks/BookmarkMargin.cs | Adds left gutter margin rendering bookmark glyphs and handling click-to-toggle + pulse animation. |
| ILSpy/Bookmarks/BookmarkManager.cs | Implements bookmark list, persistence, import/export, and change notifications. |
| ILSpy/Bookmarks/BookmarkDialogs.cs | Adds simple modal prompts for missing-assembly removal and import replace/merge choice. |
| ILSpy/Bookmarks/BookmarkAnchoring.cs | Creates bookmark anchors from a clicked line (body anchor via IL offset or token anchor via definition). |
| ILSpy/Bookmarks/Bookmark.cs | Defines bookmark model (anchor fields + editable Name/Enabled) and dedupe key. |
| ILSpy/Assets/Icons/Boomark.Disable.svg | Adds disabled-bookmark icon asset (note: filename is misspelled by design). |
| ILSpy/Assets/Icons/Bookmark.Window.svg | Adds bookmark icon variant asset (currently appears unused). |
| ILSpy/Assets/Icons/Bookmark.svg | Adds bookmark icon asset. |
| ILSpy/Assets/Icons/Bookmark.Previous.svg | Adds “previous bookmark” icon asset. |
| ILSpy/Assets/Icons/Bookmark.Previous.Folder.svg | Adds “previous bookmark (folder)” icon variant (currently appears unused). |
| ILSpy/Assets/Icons/Bookmark.Previous.File.svg | Adds “previous bookmark in file” icon asset. |
| ILSpy/Assets/Icons/Bookmark.Next.svg | Adds “next bookmark” icon asset. |
| ILSpy/Assets/Icons/Bookmark.Next.Folder.svg | Adds “next bookmark (folder)” icon variant (currently appears unused). |
| ILSpy/Assets/Icons/Bookmark.Next.File.svg | Adds “next bookmark in file” icon asset. |
| ILSpy/Assets/Icons/Bookmark.GroupDisable.svg | Adds grouped/disabled icon variant (currently appears unused). |
| ILSpy/Assets/Icons/Bookmark.Clear.svg | Adds “clear/delete bookmark” icon asset. |
| ILSpy/AppEnv/ConfigurationFiles.cs | Adds shared helper for resolving JSON sidecar config file paths next to ILSpy.xml. |
| ILSpy.Tests/Editor/LineHighlightAdornerTests.cs | Tests highlight adorner registration/unregistration lifecycle. |
| ILSpy.Tests/Docking/DockWorkspaceTests.cs | Updates tool-pane count expectations to include Bookmarks pane. |
| ILSpy.Tests/Bookmarks/MethodDebugInfoTests.cs | Unit tests for line↔offset mapping and document-level anchor resolution. |
| ILSpy.Tests/Bookmarks/BookmarkNavigationStepTests.cs | Tests next/previous stepping skips disabled bookmarks and wraps correctly. |
| ILSpy.Tests/Bookmarks/BookmarkManagerTests.cs | Tests toggle behavior, default naming, persistence, and merge/replace import rules. |
| ILSpy.Tests/Bookmarks/BookmarkDebugInfoCaptureTests.cs | Integration test proving sequence-point capture produces usable method maps. |
| ILSpy.Tests/Bookmarks/BookmarkAnchoringTests.cs | Integration test for body vs token anchor creation against real decompiled output. |
| ILSpy.Tests/AppEnv/ConfigurationFilesTests.cs | Tests sidecar path resolution and headless fallback behavior. |
Files not reviewed (1)
- ILSpy/Properties/Resources.Designer.cs: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Closed
8796fa3 to
38a25f6
Compare
There was no way to mark and return to interesting spots in decompiled code. This adds a flat (no folders, no labels) bookmark list: toggle on a line from the context menu, the gutter, or Ctrl+B; see an icon in a new left-margin gutter that honours a disabled state; and manage the list in a dockable pane that auto-registers in the Window menu. Bookmarks anchor by metadata token, never by a raw line number, so they survive re-decompilation and decompiler-setting changes that reflow the C# text: a definition line anchors to its token, while a line inside a method body anchors to the method token plus an IL offset. Recovering an IL offset needs the decompiler's sequence points, which the normal C# output did not carry, so they are captured once at the WriteCode chokepoint and stored as a per-document line/offset map. The map is also what places gutter icons and scrolls navigation to the exact line. The list persists to an ILSpy.Bookmarks.json sidecar next to ILSpy.xml; the path logic is extracted into AppEnv/ConfigurationFiles so the dock layout sidecar shares it. Navigating to a bookmark loads its assembly from disk if it dropped out of the list (and only then offers to remove a bookmark whose file is gone), then centres the line and plays a brief line flash plus a gutter-icon pulse. Disabled bookmarks stay visible but are skipped by the next/previous actions. Assisted-by: Claude:claude-opus-4-8:Claude Code
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
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
Activating a bookmark only positioned the view when a decompiler tab was already the active content: the pending bookmark was set on DockWorkspace.ActiveDecompilerTab, which is null while a metadata table, the Options page, or the About page is showing. Navigating from there selected and decompiled the node but opened at the top with no line highlight. The pending bookmark now travels to the decompiler model that ShowSelectedNode routes the selection to, and is consumed in the document-apply step alongside the view-state restore -- mirroring PendingViewState -- rather than reacting to a property change. The earlier property-change path fired synchronously during selection, scrolled, then a deferred document apply reset the caret to the top with nothing left to re-apply. For a node that is already decompiled (re-shown after an interlude, where no apply step runs) a ScrollToBookmark callback on the model scrolls the live view directly, mirroring NavigateBookmarkInFile. Assisted-by: Claude:claude-opus-4-8:Claude Code
Import read the chosen file through the tolerant sidecar-load path, which maps a missing, empty, or unparseable file all to an empty list. Replace mode then cleared the saved list and wrote the empty result back, so picking a corrupt or non-bookmark JSON file for a Replace import silently destroyed the user's bookmarks. Import now reads through TryReadFrom, which distinguishes a valid (possibly empty) file from a parse/read failure. A failure leaves the live and saved lists untouched and returns false so the pane can report it; a valid empty file still legitimately clears the list in Replace mode. The normal sidecar load keeps its tolerant behavior. Assisted-by: Claude:claude-opus-4-8:Claude Code
The text area paints the I-beam across its whole surface, so the bookmark icon gutter, line numbers, and folding margin inherited it by walking up the visual tree. Those margins are click targets, not text, so the I-beam read wrong over them. Force the normal arrow there, re-applying as the line-number and folding margins come and go with their display settings. Assisted-by: Claude:claude-opus-4-8:Claude Code
Two fixes to the gutter's hover preview, the faint glyph shown on a hovered bookmarkable line: A removal click left the pointer hovering the line it had just cleared, so the hover preview redrew a glyph there immediately and the click looked like a no-op. The just-removed line's preview is now suppressed until the pointer leaves that line; a jitter that stays on the same line keeps it hidden, and a fresh hover after leaving shows it again. The preview also never actually faded: the glyph is an SvgImage, which paints through a custom Skia draw operation that ignores DrawingContext.PushOpacity, so it always rendered fully opaque. The glyph is now rasterized once into a bitmap, which DrawImage does composite at the pushed opacity. Assisted-by: Claude:claude-opus-4-8:Claude Code
Every save reloaded the JSON sidecar into fresh instances and then replaced the whole observable collection with Clear()+Add(). That dropped and re-added every bookmark on each toggle or edit: the bookmarks pane keeps its selection by reference, so it was cleared on every change (leaving the toolbar acting on a stale or null selection), the grid's scroll reset, and a checkbox/name edit made from the grid re-entered the DataGrid mid-edit while its ItemsSource was torn down and rebuilt. Reconcile in place instead: keep the existing instance for any anchor still present (refreshing only its editable Name/Enabled), remove anchors that are gone, and append new ones. A name/enabled edit leaves the membership unchanged, so it now touches the collection not at all; structural changes touch only the affected rows. Instance identity is preserved, so the pane's selection and the gutter's references survive. Assisted-by: Claude:claude-opus-4-8:Claude Code
After scrolling to a navigated bookmark, the deferred apply restored the caret/scroll captured when the bookmark was created, which overwrote the centering that had just been computed for the resolved line. A bookmark re-anchors by token / IL offset, so a decompiler-setting change that reflows the C# moves it to a different line than the one saved in its view state; the stale offset then scrolled that line back off-screen, leaving only the highlight playing where it could not be seen. Restore just the captured foldings now -- and before centering, since collapsing or expanding shifts where lines sit -- and let the centered, re-resolved line be the final caret and scroll position. Assisted-by: Claude:claude-opus-4-8:Claude Code
- Route the bookmarks pane's double-click and next/previous navigation through HandleExceptions() rather than discarding the Task with `_ =`, so an exception raised during navigation reaches the global handler instead of vanishing. - Drop the unused parameterless NextDefaultName() overload. - Rename the misspelled Boomark.Disable.svg asset (and its loader string) to Bookmark.Disable.svg. Assisted-by: Claude:claude-opus-4-8:Claude Code
Two costs on the gutter's hot path: The margin rebuilt its document-line -> glyph map on every Render, resolving each bookmark by scanning the document's reference segments. A scroll, a hover, or the 600 ms post-navigation pulse repaints many times a second, so this rescanned the references per bookmark per frame. Compute the map once and reuse it, clearing the cache only when the document or the bookmark set changes (the model's DebugInfo/References are set before the text, so the document-change hook sees them current). CanToggleBookmarkAtLine, called for every gutter line the pointer moves over, built a full bookmark including a captured editor view state -- a foldings snapshot, scroll offsets, and a tree-path walk -- only to test it for null. It now does an anchor-only check; the view state is captured only when a bookmark is actually created. Assisted-by: Claude:claude-opus-4-8:Claude Code
BookmarkManager carried a byte-for-byte copy of ILSpySettings' private MutexProtector, both guarding read-modify-write of a config sidecar in the same settings directory so parallel ILSpy instances fold their edits in turn instead of clobbering each other. Promote it to a single public type in ICSharpCode.ILSpyX.Settings so the locking semantics live in one place. Assisted-by: Claude:claude-opus-4-8:Claude Code
…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
38a25f6 to
759266e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a flat (no folders, no labels) bookmarks feature for the decompiled C# view:
How the parts interact
flowchart TD WC["CSharpLanguage.WriteCode<br/>captures sequence points"] --> DI["DecompiledDebugInfo<br/>line to/from token + IL offset"] TRIG["Toggle: context menu / Ctrl+B /<br/>gutter click / pane toolbar"] --> AN["BookmarkAnchoring<br/>clicked line to anchor"] DI --> AN AN --> MGR["BookmarkManager<br/>the list + JSON I/O"] MGR --> JSON[("ILSpy.Bookmarks.json")] MGR --> MARGIN["BookmarkMargin<br/>gutter icons + click"] MGR --> PANE["BookmarksPane<br/>grid + toolbar"] PANE --> NAV["BookmarkNavigator<br/>load asm, resolve, select, scroll"] DI --> MARGIN DI --> NAV NAV --> DOC["decompiled document<br/>centre line + flash + pulse"]The
DecompiledDebugInfomap (a line/offset table built from the decompiler's sequence points) is the keystone: it turns a clicked line into an anchor on save, and an anchor back into a line for the gutter and for navigation. TheBookmarkManageris the single source of truth for the list and persists on every change. The margin, pane, and navigator are independent consumers wired only to the manager and the per-document map.Anchoring model
A bookmark never stores a raw line number (those move when settings reflow the C#). It stores a metadata token, with two kinds:
module + token.module + method token + IL offset. The IL offset is stable across re-decompilation and is mapped back to the current line on display.Recovering an IL offset needs the decompiler's sequence points, which the normal C# output did not carry. They are captured once at the
WriteCodechokepoint and stored as a per-document map; the displayed text is unchanged.flowchart LR A["clicked line"] --> B{"starts a statement?"} B -- yes --> C["Body anchor: token + IL offset"] B -- no --> D{"definition on line?"} D -- yes --> E["Token anchor"] D -- no --> F["not bookmarkable"] C --> G["BookmarkManager.Toggle"] E --> G G --> H{"same anchor exists?"} H -- yes --> I["remove"] H -- no --> J["add + default name"]Navigation
flowchart TD A["double-click / next / previous"] --> B["find assembly in list"] B -- "not loaded" --> C["open from disk"] C -- "file gone" --> D["offer to remove bookmark"] B -- found --> E["resolve token to entity"] C -- loaded --> E E -- "token gone" --> F["abort quietly (no dialog)"] E -- ok --> G["find tree node, or nearest enclosing type"] G --> H["select node -> decompile"] H --> I["once document ready:<br/>centre line + line flash + gutter pulse"]The removal dialog fires only when the assembly file is genuinely missing or unreadable. A token that no longer resolves (e.g. a rebuilt assembly) or a compiler-generated member without its own tree node aborts quietly rather than nagging. Disabled bookmarks stay visible in the gutter and list but are skipped by next/previous (both global and in-file).
Notable files
ILSpy/AppEnv/ConfigurationFiles.cs(the dock layout now uses it too)ILSpy/Bookmarks/Bookmark.cs,BookmarkManager.csILSpy/Bookmarks/MethodDebugInfo.csILSpy/Languages/CSharpLanguage.cs(WriteCode)ILSpy/Bookmarks/BookmarkAnchoring.csILSpy/Bookmarks/BookmarkMargin.csILSpy/Bookmarks/BookmarksPane*.{axaml,cs}ILSpy/Bookmarks/BookmarkNavigator.cs,BookmarkDialogs.csILSpy/TextView/LineHighlightAdorner.csILSpy/TextView/DecompilerTextView.axaml.csTesting
Added unit and integration tests (headless Avalonia) covering the config-file path, the manager (toggle / merge-import / JSON round-trip / drop-empty-on-load), the line/offset map and its capture against real decompiled output, both anchor branches end-to-end, the next/previous skip-disabled stepping, and the highlight adorner lifecycle. The full UI suite passes.
Notes
Review updates
Applied while addressing review feedback (branch history was rewritten, so a force-push updated this PR):
BookmarkDebugInfoCollector) that reads each AST node'sILInstructionannotation, instead of re-running the formatter through a throwaway writer and callingCreateSequencePoints. This drops a full second formatting pass (~5x cheaper capture on large types, measured) and removes the cross-pass "both writers must break lines identically" assumption — the line numbers come from the very output being displayed.Clear()+rebuild of theObservableCollection, so editing a name or toggling the enabled checkbox in the pane no longer drops the pane's selection, resets its scroll, or re-enters the grid mid-edit; existing instances are kept (the cross-instance read-modify-write under the named mutex is unchanged).TextDocument; the hover-anchorability check no longer builds a full bookmark + view-state snapshot.MutexProtectorbetween the settings and bookmark sidecars, aMatchesBookmarkhelper for the token+assembly identity, aFoldingSectionoverload ofFoldingsViewState.Restore,HandleExceptions()on the pane's navigation, and the correctedBookmark.Disable.svgasset name.Review fixes applied with Claude Code.
🤖 Generated with Claude Code