update FSharpKernel to use FSharp.Compiler.Scripting #399
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rchande
reviewed
Aug 26, 2019
|
|
||
| // reply ok | ||
| var executeReplyPayload = new ExecuteReplyOk(executionCount: openRequest.ExecutionCount); | ||
| if (InFlightRequests.TryRemove(codeSubmissionEvaluated.Command, out var openRequest)) |
There was a problem hiding this comment.
This feels like it's working around a bug? What's the case where we do multiple evaluations of the same command?
Contributor
There was a problem hiding this comment.
I hit this as well. There can be multiple replies to a given a command and if one arrives after the command is marked as completed, there was a NullReferenceException here.
jonsequitur
approved these changes
Aug 26, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This allows us to return proper
System.Objects which means we get the default formatting forIEnumerable<T>.This included an interesting bug where the directive/magic preprocessor will accept a left bracket followed by any non-whitespace characters then a right bracket bracket (e.g.,
\[[^\s]+\]) and return those as aSystem.CommandLinedirective, which is exactly what we don't want for F# where[1;2;3;4]is a validList<'t>. To get around that I added some checks to ensure that the magic string parser ignores directives. I also filed dotnet/command-line-api#625 to track future work.Still to figure out: how to consume the helper functions/formatters. E.g.,
a[href: "some-url"]("click me")won't work in F# like it does in C#.