Fix #3705: Code window is empty when select a .baml and refresh#3726
Fix #3705: Code window is empty when select a .baml and refresh#3726christophwille wants to merge 1 commit intomasterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes ILSpy issue #3705 where the code window can become empty after selecting a .baml resource and refreshing, by ensuring the relevant assembly is loaded before attempting to re-select the previously selected tree path.
Changes:
- Refactors the refresh path into an async flow so the UI can await assembly metadata loading.
- Preloads the selected assembly’s metadata before calling
FindNodeByPath, improving lookup of lazy-loaded resource nodes. - Keeps the existing refresh throttle entry point (
Refresh()→refreshThrottle.Tick()).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| using (Keyboard.FocusedElement.PreserveFocus()) | ||
| { | ||
| var path = GetPathForNode(SelectedItem); | ||
|
|
||
| ShowAssemblyList(settingsService.AssemblyListManager.LoadList(AssemblyList.ListName)); | ||
|
|
||
| // Ensure assembly loaded before FindNodeByPath to allow lazy-loaded resource nodes to be found | ||
| if (path?.Length > 0) | ||
| { | ||
| foreach (var asm in AssemblyList.GetAssemblies()) | ||
| { | ||
| if (asm.FileName == path[0]) | ||
| { | ||
| await asm.GetMetadataFileAsync().Catch<Exception>(_ => { }); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Keyboard.FocusedElement.PreserveFocus() now spans an await. Since PreserveFocus restores focus unconditionally on Dispose (see ExtensionMethods.cs), a slow GetMetadataFileAsync() can cause focus to be restored later and steal focus from whatever the user clicked during the await. Consider only restoring focus if it hasn't changed since the refresh started (or move the PreserveFocus scope so it doesn't cover the awaited portion).
| using (Keyboard.FocusedElement.PreserveFocus()) | |
| { | |
| var path = GetPathForNode(SelectedItem); | |
| ShowAssemblyList(settingsService.AssemblyListManager.LoadList(AssemblyList.ListName)); | |
| // Ensure assembly loaded before FindNodeByPath to allow lazy-loaded resource nodes to be found | |
| if (path?.Length > 0) | |
| { | |
| foreach (var asm in AssemblyList.GetAssemblies()) | |
| { | |
| if (asm.FileName == path[0]) | |
| { | |
| await asm.GetMetadataFileAsync().Catch<Exception>(_ => { }); | |
| break; | |
| } | |
| } | |
| } | |
| var path = GetPathForNode(SelectedItem); | |
| using (Keyboard.FocusedElement.PreserveFocus()) | |
| { | |
| ShowAssemblyList(settingsService.AssemblyListManager.LoadList(AssemblyList.ListName)); | |
| } | |
| // Ensure assembly loaded before FindNodeByPath to allow lazy-loaded resource nodes to be found | |
| if (path?.Length > 0) | |
| { | |
| foreach (var asm in AssemblyList.GetAssemblies()) | |
| { | |
| if (asm.FileName == path[0]) | |
| { | |
| await asm.GetMetadataFileAsync().Catch<Exception>(_ => { }); | |
| break; | |
| } | |
| } | |
| } | |
| using (Keyboard.FocusedElement.PreserveFocus()) | |
| { |
|
|
||
| private void RefreshInternal() | ||
| { | ||
| _ = RefreshInternalAsync(); |
There was a problem hiding this comment.
RefreshInternal() fires RefreshInternalAsync() without observing the returned task. If RefreshInternalAsync throws (e.g., from ShowAssemblyList, SelectNode, or RefreshDecompiledView), the exception becomes unobserved and can be lost/crash later. Consider attaching the existing task error handling used elsewhere in this file (e.g., call .HandleExceptions() on the returned task, or explicitly .Catch<Exception>(...) + .IgnoreExceptions()).
| _ = RefreshInternalAsync(); | |
| _ = RefreshInternalAsync().HandleExceptions(); |
| { | ||
| _ = RefreshInternalAsync(); | ||
| } | ||
|
|
||
| private async Task RefreshInternalAsync() | ||
| { |
There was a problem hiding this comment.
Because RefreshInternal() can be invoked multiple times (via refreshThrottle.Tick()), the fire-and-forget async refresh allows multiple RefreshInternalAsync executions to overlap/re-enter while awaiting GetMetadataFileAsync(). That can result in out-of-order ShowAssemblyList/SelectNode/RefreshDecompiledView updates. Consider serializing refreshes (e.g., track the currently running refresh task and skip/await it, or use a generation id/cancellation token so only the latest refresh applies).
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
RefreshInternalAsync captures a path and later awaits GetMetadataFileAsync(). During that await, the user can change the selection; when execution resumes, SelectNode(FindNodeByPath(path, ...)) may unexpectedly revert the selection to the stale path. Consider capturing the initial selection/path and, before calling SelectNode/RefreshDecompiledView, verifying the selection is still the same (or using a refresh generation id so stale refreshes don't apply).
| var currentPath = GetPathForNode(SelectedItem); | |
| var selectionChanged = SelectedItem is not null | |
| && !((path == null && currentPath == null) | |
| || (path != null && currentPath != null && path.SequenceEqual(currentPath))); | |
| if (selectionChanged) | |
| return; |
Uh oh!
There was an error while loading. Please reload this page.