-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use primitives for batching requests in NavBars #54433
Conversation
/// to show all items. | ||
/// The last fully computed model. | ||
/// </summary> | ||
private NavigationBarModel _model_OnlyAccessOnUIThread; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This gets cleaner once #54443 goes in. Then we don't need to have state for this and can just pass this value through our work queue.
src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController.cs
Outdated
Show resolved
Hide resolved
…nBarController.cs
void Disconnect(); | ||
|
||
void SetWorkspace(Workspace? newWorkspace); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed as there's no need to listen for the workspace (the event sources take care of that). And instead of .Disconnect, this can just be IDisposable.
@@ -97,41 +132,17 @@ internal partial class NavigationBarController : ForegroundThreadAffinitizedObje | |||
TaggerEventSources.OnWorkspaceRegistrationChanged(subjectBuffer)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the line that notifies us when our buffer is attached to a workspace. so no need for other components to tell us.
…udioTodoCommentsService.cs
…abadi/roslyn into asyncBatchingWorkQueue
…entStorageTests.cs Co-authored-by: Sam Harwell <sam@tunnelvisionlabs.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Compiler change LGTM
Azure Pipelines successfully started running 4 pipeline(s). |
Followup to #54432. That should go in first.