-
Notifications
You must be signed in to change notification settings - Fork 683
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
Dedupe Build and Live Diagnostics #6543
Conversation
If anyone has testing thoughts, please share! I'm sure there are corner cases I am not aware of. |
|
The experience with FSA and very large solutions isn't very good. With this change, FSA solutions don't show build errors that would be duplicates. This means, as David pointed out, you could build the solution but not see errors until FSA finishes. Manish is gathering telemetry on FSA solutions so we can understand how common this is. For now, waiting for feedback and tracking in this issue - #6606 |
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.
I assume this change is safe to merge even if the devkit change is not in?
Yes, this is safe without the DevKit change! CSharp extension will listen for any of the IBuildResultDiagnostics events, but it is not a problem that it doesn't receive any. On every file open it will check to see if there are build diagnostics in it's cache. There will be none and it'll skip doing any processing. |
// Check the compiler diagnostic setting. If the setting is "none" or if the file is closed, | ||
// then no live compiler diagnostics are being shown and bulid compiler diagnostics should be added. | ||
// If FSA is on, then this is a no-op as FSA will report all compiler diagnostics |
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 approach sounds good to me as a starting point. I think we should dogfood the experience to confirm if the delay between build completion and background analysis reporting live diagnostics for open files (FSA off) or closed files (FSA on) is acceptable for real world solutions with large number of open files across the solution. Especially if there are compiler errors in the files which we are deferring to background analysis to report diagnostics. Otherwise, we may have to introduce some handshake between this diagnostic source and the existing live diagnostic source such that we report all build reported diagnostics for a file here until live background analysis has analyzed that file and reported live diagnostics for it.
Currently build and live diagnostics are reported independently, causing duplicate errors and errors that don't go away until a re-build. The root cause of this problem is that DevKit extension reports build diagnostics and C# extension reports live diagnostics.
This change creates a separate build diagnostics collection, that filters the diagnostics reported by the build and only shows the ones that are not being shown by live.
Issue: #5728