Skip to content
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

Correct IDE diagnostics and lifetimes for editing with #r package references #10160

Merged
merged 13 commits into from
Sep 20, 2020

Conversation

dsyme
Copy link
Contributor

@dsyme dsyme commented Sep 19, 2020

This extends #10159 with some basic improvements to correct how and when we create DependencyProvider

  1. Only create DependencyProvider once per project, and one overall for scripts

  2. Don't register endless chains of dynamic resolution handlers for the compiler and IDE cases

  3. Gives diagnostics and failures in the IDE

    image

  4. Gives diagnostics and failures in F# compilation of scripts

    image

  5. Gives diagnostics and failures in F# Interactive execuation of scripts

    image

  6. Give tooltips (this was a feature lost on VS2017 or VS2019 for #r references in scripts, and easy to resurrect and augment to give information for package references). Note how the text shows the exact package versions and DLLs caused by the reference

    image

    image

    image

Note that script editing is still always .NET Framework in this release, sadly - there are obviously many scripts where this doesn't matter but it's something to be aware of - we will be able to correct this somehow in the next release, it's too important not to.

image

@dsyme dsyme changed the base branch from main to release/dev16.8 September 19, 2020 23:18
@dsyme dsyme changed the title Further perf improvements for https://github.com/dotnet/fsharp/pull/10159/ Further perf improvements for script editing with #r package references Sep 19, 2020
@dsyme dsyme changed the base branch from release/dev16.8 to main September 19, 2020 23:22
@dsyme dsyme changed the title Further perf improvements for script editing with #r package references Correct IDE diagnostics for script editing with #r package references Sep 20, 2020
@dsyme dsyme changed the title Correct IDE diagnostics for script editing with #r package references Correct IDE diagnostics and lifetimes for editing with #r package references Sep 20, 2020
@cartermp
Copy link
Contributor

Surface area needs update

@cartermp
Copy link
Contributor

This looks good subject to CI passing.

@dsyme
Copy link
Contributor Author

dsyme commented Sep 20, 2020

Note to self - tests needs adjusting

  X FSharp.Compiler.Scripting.DependencyManager.UnitTests.DependencyManagerInteractiveTests.SmokeTest - #r nuget package not found [1s 925ms]
  X FSharp.Compiler.Scripting.DependencyManager.UnitTests.DependencyManagerInteractiveTests.Verify that Dispose on DependencyProvider unhooks ResolvingUnmanagedDll event handler [2ms]

@dsyme
Copy link
Contributor Author

dsyme commented Sep 20, 2020

OK, the tests are adjusted for the diagnostics updates.

@dsyme
Copy link
Contributor Author

dsyme commented Sep 20, 2020

OK, green and ready

@@ -757,6 +761,9 @@ type internal FsiCommandLineOptions(fsi: FsiEvaluationSessionHostConfig, argv: s
stopProcessingRecovery e range0; failwithf "Error creating evaluation session: %A" e
inputFilesAcc

// We need a dependency provider with native resolution. Managed resolution is handled by generated `#r`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is slightly optimistic. But I don't know a scenario where this is a wrong statement. I'm guessing we'll find them.

Copy link
Contributor

@cartermp cartermp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks and feels good enough in VS as well.

@cartermp cartermp merged commit aa4eeb6 into dotnet:main Sep 20, 2020
nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Feb 23, 2021
…erences (dotnet#10160)

Co-authored-by: Don Syme <donsyme@fastmail.com>
nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Jan 26, 2022
…erences (dotnet#10160)

Co-authored-by: Don Syme <donsyme@fastmail.com>
nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Jan 26, 2022
…erences (dotnet#10160)

Co-authored-by: Don Syme <donsyme@fastmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants