Move to a simpler event source system for nav bars #54432
Conversation
| TaggerEventSources.OnTextChanged(subjectBuffer), | ||
| TaggerEventSources.OnDocumentActiveContextChanged(subjectBuffer), | ||
| TaggerEventSources.OnWorkspaceChanged(subjectBuffer, asyncListener), | ||
| TaggerEventSources.OnWorkspaceRegistrationChanged(subjectBuffer)); |
There was a problem hiding this comment.
we already have a system for connecting to events and abstracting over them. This system also handles hooking up buffers to workspaces, making it so that this type doesn't need to track workspaces at all. Also, a new CompilationAvailableTaggerEventSource source was added as we've seen a bug when roslyn starts where the information can be slightly inaccurate (since we use a frozen-partial snapshot), and we want to move from that to correct data once the compilation is actually available.
| if (_workspace != null) | ||
| { | ||
| _workspace.DocumentActiveContextChanged -= this.OnDocumentActiveContextChanged; | ||
| _workspace.WorkspaceChanged -= this.OnWorkspaceChanged; |
There was a problem hiding this comment.
much simpler. no more explicit event hookups on our end to a lot of disparate sources.
| } | ||
|
|
||
| private void OnViewFocused(object? sender, EventArgs e) | ||
| { | ||
| AssertIsForeground(); | ||
| StartSelectedItemUpdateTask(delay: TaggerConstants.ShortDelay); |
There was a problem hiding this comment.
the need for this fine-grained delays was not really necessary. We need to do the work anyways, and the user isn't going to notice us cmoputing things 50 vs 200 ms later for things like navbars.
| // it looks like we can re-use previous model | ||
| return lastCompletedModel; | ||
| } | ||
| } |
There was a problem hiding this comment.
the semantic version is computed based on things like file versions. but it's not really safe to use when dealing with fvrozen partial semantics as the file versions may be the same, but the overall symbol information might not be. now that we just recompute whenever an appropriate change happens, we really don't need this.
ghost
added this to the 
Nav bars manually connects up to multiple events, and also has to replicate a lot of the logic for handling those events that the tagging system already has. This moves us a much more similar system (and a followup PR will take us even close in terms of how events are processed). Can be reviewed one commit at a time. Explanations inlined.
Note tehre is a followup PR to this coming. However, i wanted to break the overall change into two pieces.