fsprojects / IfSharp

F# for Jupyter Notebooks
Other
442 stars 71 forks source link

.NET Core + Intellisense #203

Closed rudihorn closed 5 years ago

rudihorn commented 5 years ago

Description

I'm very interested in getting the .Net core version of IFsharp running and one of the issues is Intellisense not working. I'm aware that this is experimental, but I've done some diagnosing to try and find the source of the issue. With a little help I might be able to make a PR to fix this problem.

Currently when an Intellisense request is issued, it gets to GetDeclarations in Evaluation.fs and fails on line 119 because checkFileAnswer = FSharpCheckFileAnswer.Aborted. This is because the current implementation expects a script.fsx file in the current notebook working directory. The interactive evaluator has a fairly simple setup. It doesn't mention anything about a script.fsx file and I don't see any reference to this file elsewhere. I have the slight suspicion that creating this interactive file was part of old behaviour that doesn't happen for the .Net core version, but could be implemented.

I'm assuming the easiest solution is to generate this script.fsx file. The interactive F# session seems to have three streams, output error and print. Does the default behaviour of the interactive F# shell normally produce this script.fsx file? I have found very little documentation of any of this interactive stuff, and haven't quite figured out where the source code is either. Maybe someone has some more insight on this.

Repro steps

Start IFSharp with .Net core, type in something that would normally execute some Intellisense request. Output of shell.log in the *.ipynb directory:

2019-03-18 13:42:47 - Exceptionunexpected
2019-03-18 13:42:47 -    at IfSharp.Kernel.Evaluation.GetDeclarations(String source, Int32 lineNumber, Int32 charIndex) in /home/user/IfSharp/src/IfSharp.Kernel/Evaluation.fs:line 119
2019-03-18 13:42:47 -    at IfSharp.Kernel.IfSharpKernel.intellisenseRequest(KernelMessage msg, IntellisenseRequest content) in /home/user/IfSharp/src/IfSharp.Kernel/Kernel.fs:line 478
2019-03-18 13:42:47 -    at IfSharp.Kernel.IfSharpKernel.doShell() in /home/user/IfSharp/src/IfSharp.Kernel/Kernel.fs:line 639

Related information

rudihorn commented 5 years ago

Upon looking a little closer I'm assuming the GetProjectOptionsFromScript produces the script.fsx file and that it is an issue / bug with the GetProjectOptionsFromScript function.

rudihorn commented 5 years ago

In reference to https://github.com/fsharp/FSharp.Compiler.Service/issues/847, it seems to be that adding assumeDotNetFramework = false solves the problem. Is it worth trying to implement a workaround for IFSharp?

rudihorn commented 5 years ago

I've implemented a hack in a fork of the project: https://github.com/rudihorn/IfSharp

cgravill commented 5 years ago

Great to see this getting use - it is experimental but it seems feasible to sort out the issues.

On the Intellisense, it's actually working for me on a Windows machine: image

I don't know the history on the "script.fsx" approach, but that file doesn't actually get read: https://fsharp.github.io/FSharp.Compiler.Service/reference/fsharp-compiler-sourcecodeservices-fsharpchecker.html it's just to get the options. It's meant for a situation with multiple files.

I think what's happening on my machine is Intellisense is being done as if the source code were .NET Framework rather than .NET Core. Which happens to work on the simple examples I've tried. On your machine perhaps you don't even have .NET Framework / Mono?

Your change of assumeDotNetFramework = false seems like the right fix to me. It just needs to be made conditional on being in the .NET Core via the runtime parameter:

https://github.com/fsprojects/IfSharp/blob/b7c02064378e6a10573a0ecb83802ed5b19343d5/src/IfSharp.Kernel/Kernel.fs#L38

rudihorn commented 5 years ago

I do have Mono 5.18.1.0 installed, as I wasn't able to get IfSharp to compile without it. There are some general issues though, like I've been having problems getting type providers to work (I will open a separate issue regarding, though I feel it is related). Is it worth adding this code as a conditional and then submitting a PR?

cgravill commented 5 years ago

Yes, please do put in a PR. Based on the linked issue, you've identified a real issue and fix, the experimental .NET Core Intellisense is only working in some simple cases as presently implemented.

cgravill commented 5 years ago

Btw, we might need more of the fixes others have made: https://github.com/fable-compiler/Fable/commit/10dace4e361cd34028540fd91bdc33510b1c6e8c but perhaps start with the minimal fix that works for you for now?

cgravill commented 5 years ago

Closed via #204