Skip to content
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

added ability to clear all diagnostics reproted from a IDiagnosticUpd… #33805

Merged
merged 2 commits into from
Mar 2, 2019

Conversation

heejaechang
Copy link
Contributor

…ateSource

previously IDiagnosticUpdateSource has to clear each diagnostics it reported group by group. that was fine for IDiagnosticUpdateSource that supports incremental update, but some source such as EditAndContinue doesn't support incremental update since their errors (emit errors) come and go as a bulk (whole project). when they update, they need to update everything. so tracking things in group for incremental update is unnecessary for such source.

the new API (Source.Cleared) make it easy for source to clear all diagnostics at once it produced.

…ateSource

previously IDiagnosticUpdateSource has to clear each diagnostics it reported group by group. that was fine for IDiagnosticUpdateSource that supports incremental update, but some source such as EditAndContinue doesn't support incremental update since their errors (emit errors) come and go as a bulk (whole project). when they update, they need to update everything. so tracking things in group for incremental update is unnecessary for such source.

the new API (Source.Cleared) make it easy for source to clear all diagnostics at once it produced.
@heejaechang heejaechang requested a review from a team as a code owner March 1, 2019 22:29
@heejaechang
Copy link
Contributor Author

tagging @tmat

you just need to raise encUpdateSource.Cleared?.Invoke(this, EventArgs.Empty) to clear all diagnostics reported from your source.

@heejaechang
Copy link
Contributor Author

tagging @dotnet/roslyn-ide

{
using (var workspace = new TestWorkspace(TestExportProvider.ExportProviderWithCSharpAndVisualBasic))
{
var set = new ManualResetEvent(false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set [](start = 20, length = 3)

set is an odd name for an event.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so.. wait handle then?

private void RaiseDiagnosticsCleared(object sender)
{
Contract.ThrowIfNull(sender);
var source = (IDiagnosticUpdateSource)sender;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd move this to the caller and strongly type the parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure.

@@ -150,12 +181,48 @@ private bool UpdateDataMap(IDiagnosticUpdateSource source, DiagnosticsUpdatedArg
}
}

private bool ClearDiagnosticsFromDataMapFor(IDiagnosticUpdateSource source, List<DiagnosticsUpdatedArgs> removed)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ClearDiagnosticsFromDataMapFor [](start = 21, length = 30)

Wouldn't ClearDiagnosticsReportedBySource be better name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure.

Copy link
Member

@tmat tmat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@heejaechang heejaechang merged commit bee9f73 into dotnet:master Mar 2, 2019
333fred added a commit to 333fred/roslyn that referenced this pull request Mar 4, 2019
* dotnet/master: (903 commits)
  Add UseEnhancedColors to FeatureOnOffOptionsProvider (dotnet#33802)
  Update TypeStyle.cs
  Text on a InterpolatedVerbatimStringStartToken token (dotnet#33747)
  added ability to clear all diagnostics reproted from a IDiagnosticUpd… (dotnet#33805)
  Use a set instead of map to define processed redundant nodes.
  EnC: Remove dependency on solution from AnalyzeDocumentAsync (dotnet#33796)
  Add workitem links to new redundant assignment tests.
  Move leading trivia with node when removing unused values.
  Remove OOP related feature options. (dotnet#31644)
  Merge pull request dotnet#33631 from CyrusNajmabadi/wrapPriority
  Use the correct iteration count in IterationDataAttribute
  Handle interface members in NullableWalker.AsMemberOfType (dotnet#33764)
  Cleanly work around microsoft/nodejstools#2138
  Implement full spec changes for Index/Range (dotnet#33679)
  Further reduce expectations on deep fluent calls
  Lower expectation for deep fluent call
  Take screenshot after writing logs
  Avoid throwing exception in static constructor
  Offer to add parameter to constructor with no existing parameters (dotnet#33624)
  Add regression test for nullable crash (dotnet#33735)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants