Fix squiggle with editor and fsi. #7968
Conversation
8c604e9 to
d68942f
Compare
19f5a9e to
19e986d
Compare
|
Looks good! Can we have a test for this? I don't understand why this wasn't caught by our existing language service testing, see bug.... |
|
We most likely don't have a test for this then. |
There was a problem hiding this comment.
Let's add a regression test (or a few) in a new file in this project: https://github.com/dotnet/fsharp/tree/master/vsintegration/tests/UnitTests
Probably better to have a dedicated "FSI Completion tests" file than the existing CompletionProvider.fs tests
|
@cartermp please feel free to add one. |
1435268 to
74d04d8
Compare
|
@cartermp it was surprisingly hard to get a testcase that actually fails without the fix. However, this one does. |
* VS Unit test case * Include FSharp.Compiler.Interactive in fcs tests. * Do it in an app domain
| @@ -101,5 +101,6 @@ | |||
| <Reference Include="UIAutomationTypes" /> | |||
| <ProjectReference Include="CSharp_Analysis\CSharp_Analysis.csproj" /> | |||
| <ProjectReference Include="..\FSharp.Compiler.Service.ProjectCracker\FSharp.Compiler.Service.ProjectCracker.fsproj" /> | |||
| <ProjectReference Include="$(FSharpSourcesRoot)\fsharp\FSharp.Compiler.Interactive.Settings\FSharp.Compiler.Interactive.Settings.fsproj" /> | |||
There was a problem hiding this comment.
@KevinRansom Do you remember why this line was needed? I assume the FCS testing failed if it wasn't present?
(The line causes the FCS build to also build the latest FSharp.Core, which isn't really meant to happen as FCS is only meant to depend on a public lower-versioned FSharp.Core. This is causing some issues in integrations for some of my PRs)
There was a problem hiding this comment.
@dsyme, I believe it is in support of the added test to ensure the squiggle is found.
vsintegration/tests/UnitTests/FsxCompletionProviderTests.fs
There was a problem hiding this comment.
Hmmm that test isn't included in the FCS testing
There was a problem hiding this comment.
I suspect it might not be needed, will check
|
Yes probably not needed then. If we don't test it, it shouldn't be necessary. |
* VS Unit test case * Include FSharp.Compiler.Interactive in fcs tests. * Do it in an app domain
Visual Studio 16.4 has a regression:
Fixes: #7950
Fixed:
