From 4c1e52537649f7171ac8c85e2d5ba6a4d0874948 Mon Sep 17 00:00:00 2001 From: "Brett V. Forsgren" Date: Thu, 22 Aug 2019 16:04:30 -0700 Subject: [PATCH] update FSharpKernel to use FSharp.Compiler.Scripting --- .../FSharpWorkspaceShim.fsproj | 2 +- .../FSharpKernel.fs | 71 +++++-------------- ...Microsoft.DotNet.Interactive.FSharp.fsproj | 6 ++ .../ExecuteRequestHandler.cs | 23 +++--- .../Formatter.cs | 6 +- .../Events/CodeSubmissionEvaluationFailed.cs | 6 ++ .../FormattedValue.cs | 24 +++++++ Microsoft.DotNet.Interactive/KernelBase.cs | 4 +- NuGet.config | 1 + .../Kernel/FSharpKernelTests.cs | 18 +++-- WorkspaceServer/Kernel/CSharpKernel.cs | 20 +----- eng/Versions.props | 1 + 12 files changed, 90 insertions(+), 92 deletions(-) diff --git a/FSharpWorkspaceShim/FSharpWorkspaceShim.fsproj b/FSharpWorkspaceShim/FSharpWorkspaceShim.fsproj index c06b373c6..41b0ba9c3 100644 --- a/FSharpWorkspaceShim/FSharpWorkspaceShim.fsproj +++ b/FSharpWorkspaceShim/FSharpWorkspaceShim.fsproj @@ -19,7 +19,7 @@ - + diff --git a/Microsoft.DotNet.Interactive.FSharp/FSharpKernel.fs b/Microsoft.DotNet.Interactive.FSharp/FSharpKernel.fs index c46ec2144..2ff9293d5 100644 --- a/Microsoft.DotNet.Interactive.FSharp/FSharpKernel.fs +++ b/Microsoft.DotNet.Interactive.FSharp/FSharpKernel.fs @@ -3,70 +3,35 @@ namespace Microsoft.DotNet.Interactive.FSharp -open System -open System.ComponentModel -open System.Diagnostics -open System.Text open System.Threading.Tasks +open FSharp.Compiler.Scripting open Microsoft.DotNet.Interactive open Microsoft.DotNet.Interactive.Commands open Microsoft.DotNet.Interactive.Events -open MLS.Agent.Tools type FSharpKernel() = inherit KernelBase() - let mutable proc: Process = null - let write (s: string) = proc.StandardInput.Write(s) - let sentinelValue = Guid.NewGuid().ToString() - let sentinelFound = Event<_>() - let stdout = StringBuilder() - let waitForReady () = - async { - write <| sprintf ";;printfn \"\\n%s\";;\n" sentinelValue - do! Async.AwaitEvent sentinelFound.Publish - } - let startProcess () = - async { - if isNull proc then - let outputReceived line = - if line = sentinelValue then - sentinelFound.Trigger() - else - stdout.AppendLine(line) |> ignore - proc <- Dotnet().StartProcess("fsi --nologo", output = Action(outputReceived)) - do! waitForReady() - } - let eval (code: string) = - async { - do! startProcess () - stdout.Clear() |> ignore - write code - do! waitForReady () - let value = stdout.ToString() - // trim garbage - let nl = Environment.NewLine - let headerGarbage = sprintf "val it : unit = ()%s%s" nl nl - let value = if value.StartsWith(headerGarbage) then value.Substring(headerGarbage.Length) else value - let footerGarbage = sprintf "%s%s> %s" nl nl nl - let value = if value.EndsWith(footerGarbage) then value.Substring(0, value.Length - footerGarbage.Length) else value - return value - } - do base.AddDisposable({ new IDisposable with - member __.Dispose() = - if not <| isNull proc then - try - proc.Kill() - with - | :? InvalidOperationException -> () - | :? NotSupportedException -> () - | :? Win32Exception -> () }) + let script = new FSharpScript() + do base.AddDisposable(script) let handleSubmitCode (codeSubmission: SubmitCode) (context: KernelInvocationContext) = async { let codeSubmissionReceived = CodeSubmissionReceived(codeSubmission.Code, codeSubmission) context.OnNext(codeSubmissionReceived) - // submit code - let! value = eval codeSubmission.Code - context.OnNext(ValueProduced(value, codeSubmission, true, [FormattedValue("text/plain", value)])) + let result, errors = + try + script.Eval(codeSubmission.Code) + with + | ex -> Error(ex), [||] + if errors.Length > 0 then + let aggregateErrorMessage = System.String.Join("\n", errors) + context.OnNext(CodeSubmissionEvaluationFailed(aggregateErrorMessage, codeSubmission)) + match result with + | Ok(Some(value)) -> + let value = value.ReflectionValue + let formattedValues = FormattedValue.FromObject(value) + context.OnNext(ValueProduced(value, codeSubmission, true, formattedValues)) + | Ok(None) -> () + | Error(ex) -> context.OnError(ex) context.OnNext(CodeSubmissionEvaluated(codeSubmission)) context.OnCompleted() } diff --git a/Microsoft.DotNet.Interactive.FSharp/Microsoft.DotNet.Interactive.FSharp.fsproj b/Microsoft.DotNet.Interactive.FSharp/Microsoft.DotNet.Interactive.FSharp.fsproj index e3fd56688..e8ba17359 100644 --- a/Microsoft.DotNet.Interactive.FSharp/Microsoft.DotNet.Interactive.FSharp.fsproj +++ b/Microsoft.DotNet.Interactive.FSharp/Microsoft.DotNet.Interactive.FSharp.fsproj @@ -21,7 +21,13 @@ + + + + + + diff --git a/Microsoft.DotNet.Interactive.Jupyter/ExecuteRequestHandler.cs b/Microsoft.DotNet.Interactive.Jupyter/ExecuteRequestHandler.cs index 15d2e3681..a87da41cd 100644 --- a/Microsoft.DotNet.Interactive.Jupyter/ExecuteRequestHandler.cs +++ b/Microsoft.DotNet.Interactive.Jupyter/ExecuteRequestHandler.cs @@ -208,19 +208,20 @@ private void OnValueProduced(ValueProduced valueProduced) private void OnCodeSubmissionEvaluated(CodeSubmissionEvaluated codeSubmissionEvaluated) { - InFlightRequests.TryRemove(codeSubmissionEvaluated.Command, out var openRequest); - - // reply ok - var executeReplyPayload = new ExecuteReplyOk(executionCount: openRequest.ExecutionCount); + if (InFlightRequests.TryRemove(codeSubmissionEvaluated.Command, out var openRequest)) + { + // reply ok + var executeReplyPayload = new ExecuteReplyOk(executionCount: openRequest.ExecutionCount); - // send to server - var executeReply = Message.CreateResponse( - executeReplyPayload, - openRequest.Context.Request); + // send to server + var executeReply = Message.CreateResponse( + executeReplyPayload, + openRequest.Context.Request); - openRequest.Context.ServerChannel.Send(executeReply); - openRequest.Context.RequestHandlerStatus.SetAsIdle(); - openRequest.Dispose(); + openRequest.Context.ServerChannel.Send(executeReply); + openRequest.Context.RequestHandlerStatus.SetAsIdle(); + openRequest.Dispose(); + } } } } \ No newline at end of file diff --git a/Microsoft.DotNet.Interactive.Rendering/Formatter.cs b/Microsoft.DotNet.Interactive.Rendering/Formatter.cs index 969f40635..201b85c78 100644 --- a/Microsoft.DotNet.Interactive.Rendering/Formatter.cs +++ b/Microsoft.DotNet.Interactive.Rendering/Formatter.cs @@ -113,7 +113,7 @@ public static void ResetToDefault() } public static string ToDisplayString( - this object obj, + this object obj, string mimeType = Rendering.PlainTextFormatter.MimeType) { // TODO: (ToDisplayString) rename @@ -143,7 +143,7 @@ public static string ToDisplayString( } public static void FormatTo( - this T obj, + this T obj, TextWriter writer, string mimeType = Rendering.PlainTextFormatter.MimeType) { @@ -181,7 +181,7 @@ internal static Action GetGenericFormatterMethod(thi var writerParam = Expression.Parameter(typeof(TextWriter), "target"); var mimeTypeParam = Expression.Parameter(typeof(string), "target"); - var methodCallExpr = Expression.Call(null, + var methodCallExpr = Expression.Call(null, methodInfo, Expression.Convert(targetParam, type), writerParam, diff --git a/Microsoft.DotNet.Interactive/Events/CodeSubmissionEvaluationFailed.cs b/Microsoft.DotNet.Interactive/Events/CodeSubmissionEvaluationFailed.cs index dc77db616..a72d2c075 100644 --- a/Microsoft.DotNet.Interactive/Events/CodeSubmissionEvaluationFailed.cs +++ b/Microsoft.DotNet.Interactive/Events/CodeSubmissionEvaluationFailed.cs @@ -19,6 +19,12 @@ public CodeSubmissionEvaluationFailed( : message; } + public CodeSubmissionEvaluationFailed( + string message, + SubmitCode submitCode) : this(null, message, submitCode) + { + } + public string Code => ((SubmitCode)Command).Code; public Exception Exception { get; } diff --git a/Microsoft.DotNet.Interactive/FormattedValue.cs b/Microsoft.DotNet.Interactive/FormattedValue.cs index 3eae78368..481aeeba8 100644 --- a/Microsoft.DotNet.Interactive/FormattedValue.cs +++ b/Microsoft.DotNet.Interactive/FormattedValue.cs @@ -2,6 +2,8 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using System.Collections.Generic; +using Microsoft.DotNet.Interactive.Rendering; namespace Microsoft.DotNet.Interactive { @@ -21,5 +23,27 @@ public FormattedValue(string mimeType, object value) public string MimeType { get; } public object Value { get; } + + public static IReadOnlyCollection FromObject(object value) + { + var type = value?.GetType(); + + var mimeType = MimeTypeFor(type); + + var formatted = value.ToDisplayString(mimeType); + + return new FormattedValue[] + { + new FormattedValue(mimeType, formatted) + }; + } + + private static string MimeTypeFor(Type returnValueType) + { + return returnValueType?.IsPrimitive == true || + returnValueType == typeof(string) + ? "text/plain" + : "text/html"; + } } } \ No newline at end of file diff --git a/Microsoft.DotNet.Interactive/KernelBase.cs b/Microsoft.DotNet.Interactive/KernelBase.cs index ae807c380..c38ebb7a4 100644 --- a/Microsoft.DotNet.Interactive/KernelBase.cs +++ b/Microsoft.DotNet.Interactive/KernelBase.cs @@ -127,7 +127,9 @@ private async Task HandleDirectivesAndSubmitCode( var parseResult = BuildDirectiveParser().Parse(currentLine); - if (parseResult.Errors.Count == 0) + if (parseResult.Errors.Count == 0 && + !parseResult.Directives.Any() && // System.CommandLine directives should not be considered as valid + !parseResult.Tokens.Any(t => t.Type == TokenType.Directive)) { modified = true; await _directiveParser.InvokeAsync(parseResult); diff --git a/NuGet.config b/NuGet.config index e06c764c3..40a2c33a7 100644 --- a/NuGet.config +++ b/NuGet.config @@ -7,6 +7,7 @@ + diff --git a/WorkspaceServer.Tests/Kernel/FSharpKernelTests.cs b/WorkspaceServer.Tests/Kernel/FSharpKernelTests.cs index dd4dc0515..b6a0f4e2f 100644 --- a/WorkspaceServer.Tests/Kernel/FSharpKernelTests.cs +++ b/WorkspaceServer.Tests/Kernel/FSharpKernelTests.cs @@ -31,7 +31,7 @@ public async Task it_returns_an_object_value() { var kernel = CreateKernel(); await kernel.SendAsync(new SubmitCode("123")); - AssertLastValue("> val it : int = 123"); + AssertLastValue(123); } [Fact] @@ -39,12 +39,22 @@ public async Task it_remembers_state_between_submissions() { var kernel = CreateKernel(); await kernel.SendAsync(new SubmitCode("let add x y = x + y")); - AssertLastValue("> val add : x:int -> y:int -> int"); await kernel.SendAsync(new SubmitCode("add 2 3")); - AssertLastValue("> val it : int = 5"); + AssertLastValue(5); } - private void AssertLastValue(string value) + [Fact] + public async Task kernel_base_ignores_command_line_directives() + { + // The text `[1;2;3;4]` parses as a System.CommandLine directive; ensure it's not consumed and is passed on to the kernel. + var kernel = CreateKernel(); + await kernel.SendAsync(new SubmitCode(@" +[1;2;3;4] +|> List.sum")); + AssertLastValue(10); + } + + private void AssertLastValue(object value) { KernelEvents.ValuesOnly() .OfType() diff --git a/WorkspaceServer/Kernel/CSharpKernel.cs b/WorkspaceServer/Kernel/CSharpKernel.cs index 9f9872cb0..50141acb6 100644 --- a/WorkspaceServer/Kernel/CSharpKernel.cs +++ b/WorkspaceServer/Kernel/CSharpKernel.cs @@ -142,17 +142,7 @@ private async Task HandleSubmitCode( { if (HasReturnValue) { - var returnValueType = _scriptState.ReturnValue?.GetType(); - - var mimeType = MimeTypeFor(returnValueType); - - var formatted = _scriptState.ReturnValue.ToDisplayString(mimeType); - - var formattedValues = new List - { - new FormattedValue(mimeType, formatted) - }; - + var formattedValues = FormattedValue.FromObject(_scriptState.ReturnValue); context.OnNext( new ValueProduced( _scriptState.ReturnValue, @@ -167,14 +157,6 @@ private async Task HandleSubmitCode( } } - private static string MimeTypeFor(Type returnValueType) - { - return returnValueType?.IsPrimitive == true || - returnValueType == typeof(string) - ? "text/plain" - : "text/html"; - } - private void PublishOutput( string output, KernelInvocationContext context, diff --git a/eng/Versions.props b/eng/Versions.props index 2dac89935..bbfe3aef7 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -43,6 +43,7 @@ $(RestoreSources); https://dotnet.myget.org/F/dotnet-try/api/v3/index.json; https://dotnetfeed.blob.core.windows.net/dotnet-core/index.json; + https://dotnet.myget.org/F/fsharp/api/v3/index.json