-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Changes needed to create a F# language service based on Roslyn workspace model #8557
Changes from 6 commits
e856ec5
6b8514b
8a7a10c
8f1d29d
c3a5a3f
4815875
eb8dbc5
5771b39
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,6 +58,8 @@ | |
<InternalsVisibleToTest Include="Roslyn.Services.Editor.VisualBasic.UnitTests" /> | ||
<InternalsVisibleToTest Include="Roslyn.VisualStudio.Services.UnitTests" /> | ||
<InternalsVisibleToTest Include="RoslynETAHost" /> | ||
<InternalsVisibleToFSharp Include="FSharp.Editor" /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You might want to consider naming these the same way we do for C#/VB/TS. I think uniformity will make things a lot clearer. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're also in the test section. Ideally all your stuff would always be next to the TypeScript stuff. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You mean naming the F# dlls? Currently these are the names of the existing F# LS dlls. @otawfik-ms's plan is to start adding code to those existing dlls and refactor later. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved all the F# IVTs next to the TypeScript ones. |
||
<InternalsVisibleToFSharp Include="FSharp.LanguageService" /> | ||
</ItemGroup> | ||
<ItemGroup> | ||
<!-- Compile items go here --> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
using System.Collections.Generic; | ||
using System.Collections.Immutable; | ||
using System.Globalization; | ||
using System.Linq; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Microsoft.CodeAnalysis.CodeFixes; | ||
|
@@ -57,6 +58,15 @@ public static DiagnosticData ToDiagnosticData(this Diagnostic diagnostic, Projec | |
return DiagnosticData.Create(project.GetDocument(diagnostic.Location.SourceTree), diagnostic); | ||
} | ||
|
||
if (diagnostic.Location.Kind == LocationKind.ExternalFile) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This may affect TypeScript. Can we check to make sure we don't get duplicated errors for TS issues? @vladima can also probably show you how to make a project level error to hit this codepath. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method is only called by the SuggestActionProviders (through There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
{ | ||
var document = project.Documents.FirstOrDefault(d => d.FilePath == diagnostic.Location.GetLineSpan().Path); | ||
if (document != null) | ||
{ | ||
return DiagnosticData.Create(document, diagnostic); | ||
} | ||
} | ||
|
||
return DiagnosticData.Create(project, diagnostic); | ||
} | ||
|
||
|
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.
Shouldn't we remove
CSharpContentType
andVisualBasicContentType
now?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.
Yes done.