track non roslyn text buffer changes as well as roslyn text buffer chang... #2515
Conversation
…anges to delay solution crawler There are 2 things incremental processor takes care of @1 is making sure we delay processing any work until there is enough idle (ex, typing) in host. @2 is managing cancellation and pending works. we used to do #1 and #2 only for Roslyn files. and that is usually fine since most of time solution contains only roslyn files. but for mixed solution (ex, Roslyn files + HTML + JS + CSS), #2 still makes sense but #1 doesn't. We want to pause any work while something is going on in other project types as well. we need to make sure we play nice with neighbors as well. now, we don't care where changes are coming from. if there is any change in host, we puase oursevles for a while.
|
@pharring @srivatsn @jasonmalinowski @rchande @shyamnamboodiripad can you take a look? |
|
by the way, this is an attempt to reduce allocation in one of perf tests. |
|
Rather than couple all of this together, I'd feel better if there was a simple IWpfTextViewCreationListener that subscribed to events and then managed it's own state rather than tying this to the VS document provider. I think it'll be less code, would have the advantage that it'll work in non-VS hosts, and doesn't churn what is one of the more subtle pieces of code in our codebase. |
| foreach (var documentKey in documentsToClose) | ||
| // let others know about non roslyn document close | ||
| var buffer = TryGetTextBufferFromDocData(RunningDocumentTable.GetDocumentData(docCookie)); | ||
| if (buffer != null) |
There was a problem hiding this comment.
What happens if the document, after already being open, was then added to the workspace? At this point this code path will get skipped, and the view unsubscribe might not run?
Trying to do this state tracking in this class really isn't the right thing to do. We should just have a separate class to subscribe to the text view notifications.
|
@jasonmalinowski @pharring @rchande can you take a look? |
|
I talked to Jason already about using IWpfTextViewCreationListener. we can't use it due to several reasons. such as dll being loaded into VS even when Roslyn is not used or having circular reference unless we create new dll and etc. |
| private ITextView GetTextViewFromFrame(IVsWindowFrame frame) | ||
| { | ||
| IntPtr unknown; | ||
| if (ErrorHandler.Failed(frame.QueryViewInterface(typeof(IVsCodeWindow).GUID, out unknown)) || unknown == IntPtr.Zero) |
There was a problem hiding this comment.
This isn't the right way to get a code window from a frame. Microsoft.VisualStudio.Shell.VsShellUtilities has a GetTextView implementation which you should study. It deals with things such as split XAML/Designer views.
|
Please fix the GetTextView thing. Otherwise LGTM. |
| return; | ||
| } | ||
|
|
||
| view.TextBuffer.PostChanged -= OnTextChanged; |
There was a problem hiding this comment.
Don't forget to unsubscribe Closed as well.
|
👍 once Paul's comment is taken care of. |
track non roslyn text buffer changes as well as roslyn text buffer chang...
|
@heejaechang Why not call the MPF helper in your implementation of GetTextView? |
...es to delay solution crawler
There are 2 things incremental processor takes care of
@1 is making sure we delay processing any work until there is enough idle (ex, typing) in host.
@2 is managing cancellation and pending works.
we used to do #1 and #2 only for Roslyn files. and that is usually fine since most of time solution contains only roslyn files.
but for mixed solution (ex, Roslyn files + HTML + JS + CSS), #2 still makes sense but #1 doesn't. We want to pause any work while something is going on in other project types as well.
we need to make sure we play nice with neighbors as well.
now, we don't care where changes are coming from. if there is any change in host, we puase oursevles for a while.