Introduce a better system for keeping track if a buffer is visible or not in tagging#60500
Conversation
ghost
added
the
…ider.TagSource_ProduceTags.cs
…roslyn into betterVisibility
| // all open documents, which can lead to cancellation of diagnostic recomputation task | ||
| // for the newly active document. This can lead to a race condition where we end up with | ||
| // stale diagnostics for the active document. We avoid that by always recomputing | ||
| // the diagnostics for the newly active document whenever active document is switched. |
|
|
||
| private void AssociatedViewService_SubjectBuffersConnected(object sender, SubjectBuffersConnectedEventArgs e) | ||
| { | ||
| Contract.ThrowIfFalse(_threadingContext.HasMainThread); |
There was a problem hiding this comment.
should this use https://sourceroslyn.io/#Microsoft.CodeAnalysis.EditorFeatures/Shared/Utilities/ForegroundThreadAffinitizedObject.cs,17 (and for the rest of the contract checks below)
There was a problem hiding this comment.
FTAO is affectively just a legacy wrapper around IThreadingContext. I'd prefer we just remove FTAO and use IThreadingContext in all places instead :)
|
|
||
| // if any of the views were *not* wpf text views, assume the buffer is visible. We don't know how to | ||
| // determine the visibility of this buffer. | ||
| if (views.Any(v => v is not IWpfTextView)) |
There was a problem hiding this comment.
what would be a valid case of us using this and having a non wpf view? Is expected that calling this for a C#/vb/ts/f# buffer would not be?
Just wondering if this a bug in the caller and should throw
There was a problem hiding this comment.
that's a really good question. let me investigate.
There was a problem hiding this comment.
so we can't prove that every one of our buffers is hosted in an IWpfTextView (even in VS). Plugins, for example, coudl create buffers with our content type and associate them with other buffers. i will doc this.
| // something see it is not visible, but then do not hear about its visibility change because we've hooked up | ||
| // our event after that happens. | ||
| var visibilityChangedTaskSource = new TaskCompletionSource<bool>(); | ||
| var callback = new VisibilityCallback(visibilityChangedTaskSource); |
There was a problem hiding this comment.
so this brings me to a question on the original API - any reason why the callback isn't a func or something similar? Seems like each consumer needs a bit more boilerplate to use the ITextBufferVisibilityChangedCallback
There was a problem hiding this comment.
talked offline. pros/cons to both approaches. for a later PR i thought this was nicer. but i don't feel strongly.
|
|
||
| /// <summary> | ||
| /// Registers to hear about visibility changes for this particular buffer. | ||
| /// Registers to hear about visibility changes for this particular buffer. Not: registration will not trigger a |
|
Re-targeting from release/dev17.3 to main-vs-deps so we can delete the release/dev17.3 branch. |
4 participants
IDocumentTrackingService is not great (it doesn't work in repl i've found). This is a much clearer and cleaner association with the textview itself and if it's visible.
This is extracted from teh Pause-Taggers PR here #60371. That way Pause/Unpause is in an independent chunk we merge.