Skip to content

Fix package search and navigation follow-ups from #4023 - #4025

Merged
siegfriedpammer merged 2 commits into
masterfrom
fix/package-search-followups
Aug 18, 2026
Merged

siegfriedpammer merged 2 commits into
masterfrom
fix/package-search-followups

Conversation

@siegfriedpammer

Copy link
Copy Markdown
Member

Follow-ups to #4023, from a review of that PR after it merged. Two commits, one per half of the original change; no behaviour from #4023 is reverted.

Searching a package (3dfb59ac8)

  • Entries resolved by name collapsed case-only twins. PackageFolder's cache is keyed case-insensitively, which is correct for an assembly reference but not for an archive entry: lib/net10.0/a.dll and lib/net10.0/A.dll are two files and shared one LoadedAssembly, so one was searched twice and the other never. The cache is now keyed by the entry itself; ResolveFileName keeps case-insensitive name lookup for reference resolution.
  • A multi-target package produced indistinguishable results. lib/net472/X.dll, lib/net8.0/X.dll and lib/netstandard2.0/X.dll are three assemblies, and every search hit in them showed X.dll as its location. A nested assembly's file name is now the package-relative path. ShortName and the tree label are unchanged (GetFileNameWithoutExtension handles the path).
  • Cancellation did not reach the expensive part. The token was checked only between top-level list entries, never while expanding a package, so a walk the user had already replaced by typing another character kept extracting entries alongside the run they were waiting for. It is now checked per entry, before the extraction each resolve starts.
  • The omnibar never cancelled its run. Its view model is per document tab and nothing ended the search when the tab went away, so an abandoned run kept its render-priority timer ticking and kept the assembly list alive. It now cancels on detach.

Reaching a package-nested node (482c0bd81)

  • The fix landed in 2 of 5 places. FindTreeNode's MetadataFile arm learned to descend into packages; its LoadedAssembly arm, MetadataNavigator.FindTableNode, and MetadataProtocolHandler kept their own scan of the root's direct children, so a token reference, a metadata:// link and a LoadedAssembly reference still resolved to nothing inside a package. All now route through the one lookup.
  • MetadataNavigator.ResolveRowToTreeNode returned early for the case it was meant to serve. Its GetAssemblies() guard is top-level-only and its result was never used, so double-clicking a metadata row under a package did nothing.
  • FindNamespaceNode's namespace half compared a full name against a node label. Those are only ever equal in flat mode; with nested namespace nodes the label is the last segment, so "System.Collections.Generic" == "System" failed, and the empty-name test matched the first child rather than the global-namespace node. The assembly node already indexes its namespaces by full name.
  • The descent swept the package depth-first. Expanding a package folder resolves and extracts every .dll it holds, so reaching one assembly built the folders ordered ahead of it too. The path now comes from the package's in-memory folder graph, which costs no tree node and reads no entry.

Tests

Four new tests in AssemblyTreeModelTests, over a two-assembly zip: the walk yields both entries with their paths, a cancelled walk stops before the next entry, navigation lands on a node whose module is the nested assembly (the fixture's type handle is 0x02000002 and resolves in nearly every assembly, so asserting the node type alone cannot fail), and the sibling folder is left unexpanded.

ILSpy.Tests 1210 passed / 0 failed, ICSharpCode.ILSpyCmd.Tests 22 passed / 0 failed, clean rebuild of ILSpy.Desktop.slnf and ILSpy.XPlat.slnf with no warnings.


Opened by an AI agent (Claude) on Siegfried's behalf.

🤖 Generated with Claude Code

Searching a package resolves its entries by file name against a case-insensitive
cache, which is right for an assembly reference but wrong for an archive entry:
two entries differing only in case are two files, and they collapsed onto one
LoadedAssembly, so one was searched twice and the other never. Keying the cache
by the entry itself separates them, and the entry's package-relative path
becomes the assembly's file name, which is what tells the copies of one assembly
in a multi-target package apart wherever a search result shows a location.

Cancellation was only checked between top-level list entries, so a walk the user
had already replaced by typing another character kept extracting package entries
alongside the run they were waiting for. The omnibar had no way to end its run at
all: its view model is per document tab and nothing cancelled it when the tab
went away.

Assisted-by: Claude:claude-opus-5[1m]:Claude Code
Resolving a metadata file to its assembly node learned to descend into packages,
but three sibling lookups kept their own scan of the root's direct children, so
a token reference, a metadata:// link and a LoadedAssembly reference still
resolved to nothing inside a package. One of them sat behind a guard whose
result was never used, which returned early for exactly the case it was meant to
serve. Routing all of them through the one lookup fixes them together.

Namespaces were matched by comparing a full name against a node label, which is
only ever equal in flat mode: with nested namespace nodes the label is the last
segment, and the empty-name test matched the first child rather than the global
namespace node. The assembly node already indexes its namespaces by full name.

The descent itself no longer sweeps the package depth-first. Expanding a folder
resolves and extracts every .dll it holds, so the path is taken from the
package's folder graph, which costs no tree node and reads no entry.

Assisted-by: Claude:claude-opus-5[1m]:Claude Code
@siegfriedpammer
siegfriedpammer merged commit 83c0076 into master Aug 18, 2026
17 checks passed
@siegfriedpammer
siegfriedpammer deleted the fix/package-search-followups branch August 26, 2026 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant