Pause taggers when they are not visible.#60371
Pause taggers when they are not visible.#60371CyrusNajmabadi merged 23 commits intodotnet:main-vs-depsfrom
Conversation
in progress Add support for pausing taggers when they lose visibility Make async Flip Simplify
|
/azp run |
|
Azure Pipelines successfully started running 4 pipeline(s). |
...ditorFeatures/Core/ExternalAccess/VSTypeScript/Api/VSTypeScriptAsynchronousTaggerProvider.cs
Outdated
Show resolved
Hide resolved
…ScriptAsynchronousTaggerProvider.cs
… into pauseTaggers
src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs
Outdated
Show resolved
Hide resolved
|
Re-targeting from |
src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs
Outdated
Show resolved
Hide resolved
…ider.TagSource_ProduceTags.cs
… into pauseTaggers
jasonmalinowski
left a comment
There was a problem hiding this comment.
This looks great, and as you said nice and simple!
| { | ||
| internal abstract class AbstractTaggerEventSource : ITaggerEventSource | ||
| { | ||
| private bool _paused; |
There was a problem hiding this comment.
We didn't want to disconnect entirely? On one hand I don't know if it really hurts much to stay connected, but given we already had a Connect/Disconnect pair, do we need a second pair?
There was a problem hiding this comment.
So the contract of Disconnect is that it's somewhat like Dispose. It's can fundamentally put the event source into an unusable state (and is doc'ed as such). I suppose we could actually try to make it so that you could repeatedly call Connect/Disconnect... but i'm warier of that.
| // any time visibility changes, resume tagging on all taggers. Any non-visible taggers will pause | ||
| // themselves immediately afterwards. | ||
| Resume(); |
There was a problem hiding this comment.
I admit it wasn't immediately obvious why this wasn't calling Pause if it was invisible, but I see the idea here is we only enter the paused state once we did have an event, which also doubles as the "we need to recompute once we regain focus" bit. Not sure if that deserves a comment here or perhaps on the paused flag.
| /// Pauses this event source and prevents it from firing the <see cref="Changed"/> event. Can be called many | ||
| /// times (but subsequence calls have no impact if already paused). Must be called on the UI thread. | ||
| /// </summary> | ||
| void Pause(); |
There was a problem hiding this comment.
This interface is implemented by TS, this change break them. FYI @dibarbet
http://index/?leftProject=Microsoft.CodeAnalysis.EditorFeatures&leftSymbol=nawphull0qjk&file=Tagging%5CITaggerEventSource.cs
No point in doing work here that the user is not even going to see.