Fix TryDotnet issue "Compile errors are duplicated in output" #3572
Conversation
This fixes a bug in TryDotNet that occurs when a user enters invalid code. - Ensured project diagnostics are added only if their Id is not already present in the main diagnostics collection
ocallesp
changed the title
| @@ -291,9 +291,17 @@ private static IEnumerable<Diagnostic> GetDiagnostics(string code, CompileResult | |||
| projectDiagnostics = projectDiags; | |||
| } | |||
|
|
|||
| var allDiagnostics = diagnostics.Concat(projectDiagnostics); | |||
There was a problem hiding this comment.
I wonder if we need the project diagnostics at all. Are there ever diagnostics here that are interesting from the point of view of Try .NET?
There was a problem hiding this comment.
Diagnostics contain errors introduced by the user, while ProjectDiagnostics cover errors across the entire project.
Using only Diagnostics works well for our primary focus on user-introduced errors.
For debugging purposes and to catch potential errors from changes to the background project, it might be useful to include ProjectDiagnostics. However, since the project is pre-built and we assume deployment doesn't occur with build errors, we can safely remove ProjectDiagnostics from GetDiagnostics.
| @@ -277,23 +277,14 @@ private async Task<Workspace> AppendCodeToWorkspaceAsync(string code, int positi | |||
| private static IEnumerable<Diagnostic> GetDiagnostics(string code, CompileResult result) | |||
| { | |||
| var diagnostics = Enumerable.Empty<SerializableDiagnostic>(); | |||
There was a problem hiding this comment.
This assignment could be moved to an else block below now.
There was a problem hiding this comment.
Good. I made the suggested change. Can you approve this cleanup change #3574
jonsequitur
added
Area-Try .NET
Fixes dotnet/try#1184
This fixes a bug in TryDotNet that occurs when a user enters invalid code.