-
Notifications
You must be signed in to change notification settings - Fork 156
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update FSAC to use FCS and FSharp.Core from 9.0.100, as well as updated Analyzer SDK #1324
Conversation
…conditionally-compile away ta call with a new API in .NET 9
@@ -413,7 +413,12 @@ and AdaptiveCancellableTask<'a>(cancel: unit -> unit, real: Task<'a>) = | |||
real | |||
else | |||
cachedTcs <- new TaskCompletionSource<'a>() | |||
#if NET8_0 | |||
cachedTcs.TrySetFromTask real | |||
#if NET9_0_OR_GREATER | |||
cachedTcs.TrySetFromTask real |> ignore<bool> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we have to call this from within the current TrySetFromTask
as it needs to be a completed task.
df9652f
to
becea2a
Compare
becea2a
to
9791109
Compare
real.ContinueWith(fun (task: Task<_>) -> | ||
real.ContinueWith (fun (task: Task<_>) -> | ||
#if NET9_0_OR_GREATER | ||
tcs.TrySetFromTask(task) |> ignore<bool> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh fascinating - so the runtime is generally getting APIs that are better/correct/easier to use?
@@ -111,7 +111,8 @@ let private getRangesAndPlacement input pos = | |||
members | |||
|> List.tryPick (fun m -> | |||
match m with | |||
| SynMemberDefn.AutoProperty(accessibility = None; ident = ident; trivia = trivia) as a when | |||
// TODO: Need to handle setting get or set accessibility separately |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making a comment here to make a link to this todo for an issue - we need to be able to handle the other AST cases when the getter or setter have different visibilities.
@@ -303,8 +303,8 @@ let formattingTests state = | |||
| Result.Error e -> failwithf "Error while formatting %s: %A" sourceFile e | |||
| Core.Result.Error errors -> failwithf "Errors while parsing script %s: %A" sourceFile errors | |||
}) | |||
|
|||
testList | |||
// Skip until https://github.com/dotnet/sdk/issues/44838 is fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be the first week of December FWIW
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Brilliant - thank you for digging into the test failures and getting us to green!
…ed Analyzer SDK (ionide#1324) Co-authored-by: Chet Husk <[email protected]>
…ed Analyzer SDK (ionide#1324) Co-authored-by: Chet Husk <[email protected]>
Fixes #1323