Implement F1 HelpContextService #1966
Conversation
|
Fabulous. Really fabulous! |
| content = "." -> true | ||
| | _ -> false | ||
|
|
||
| let keyword = |
There was a problem hiding this comment.
Is there a helper we can use here instead, like tryClassifyAtPosition? #1964 may be relevant.
There was a problem hiding this comment.
I missed that one, thanks. It's not quite applicable here because it doesn't give back the categories that are needed here. But the code helped me do a small cleanup.
|
Super happy to see this! I think we can leave the old F1 help code for now. Once we ship RTM of Visual Studio, one of the tasks we'll have is to delete all the dead code in the old language service. #1679 is tracking that. |
Just a heads up that the really big challenge will be to bring across the massive amount of testing in VisualFsharp.Tests that sits on top of this code, all of which is valuable. This testing is currently exercising the old language service code, but that in turn exercises the FCS API. Direct FCS testing doesn't include many editor tests because there is so much testing via this route. The testing should be adjusted to sit on top of either FCS directly, or on the unit-testable parts of FSharp.Editor. We can afford to lose a little bit of the testing along the way. But there is really important stuff here that represents many man years of investment and crucial corner-case bugs in both FCS, the F# compiler itself and even type providers. My guess is that moving this testing across is going to be a sufficiently large and boring job that we'll not see it done for some time.... :/ :) |
|
Yup, agreed. Hmmm. Maybe I can spend some time on that this Christmas break. |
|
@cartermp It's a horrible, horrible job, please don't :) Spend some time on F# Azure Notebooks intellisense instead :) |
|
Close/reopen to retrigger build |
|
Awesome. I may be wrong but I cannot remember F1 working ever, starting from VS 2012. |
|
All old tests have been enabled now. I'm pretty sure the CI errors are because #1967 has not been merged. |
|
Really good work especially on the tests. Can you delete the corresponding tests in the old test suite as part of this PR please? That will avoid someone duplicating the work later. thanks |
|
Done. I didn't delete any of the old code though. |
|
Rebased against master. Still works. |
rojepp
changed the title
| let (s', poslist') = extractPos 0 row s poslist | ||
| (row+1, (s'::lines),poslist')) (1,[],[]) testLines | ||
| List.rev l , List.rev p | ||
| let fileName = "C:\\test.fs" |
There was a problem hiding this comment.
absolute path and root of C? can we make it relative or use an environment variable or a temporary folder?
There was a problem hiding this comment.
@KevinRansom I believe the file name is dummy. It is to create correct FSharpProjecOptions for testing. File contents are provided directly without reading from disks.
|
Then it probably doesn't need the path qualifier. |
|
@rojepp Kevin |
* WIP: implement F1 HelpContextService Implements dotnet#1938 * Small cleanup * Enable context help tests for provided types * Delete old tests * Rebase against master
Initial stab at implementing #1938
I haven't yet deleted any of the old F1 help code or tests.
I've copied all existing unit tests. I am not sure how much I can delete. :/
Two tests involving type providers and assembly references are commented out, all others pass.