When removing a file clear its diagnostics

[MangelMaxime]

Related to ionide/ionide-vscode-fsharp#1768

This PR apply the fix suggested by @Krzysztof-Cieslak on ionide/ionide-vscode-fsharp#1768 (comment)

fsac_clear_diag_when_removing_file.mp4

It works however, if user start to edit the file again. New diagnostics are computed for this file. I believe that we need to remove/clear/unregister something else too.

Example:

fsac_new_diag_generated_after_removing_file.mp4

Editing the file should not generate diagnostics for Log.fs file.

[Booksbaum]

It works however, if user start to edit the file again. New diagnostics are computed for this file. I believe that we need to remove/clear/unregister something else too.

Issue might be: ProjectOptions for that file are still cached -> might still behaves like it's part of project.
https://github.com/fsharp/FsAutoComplete/blob/680a95b9d02c330d5b8a3a459271cc532efb368c/src/FsAutoComplete.Core/State.fs#L152
https://github.com/fsharp/FsAutoComplete/blob/680a95b9d02c330d5b8a3a459271cc532efb368c/src/FsAutoComplete.Core/Commands.fs#L702-L756
-> try Remove Projects options for that file with state.RemoveProjectOptions(file)

---

Editing the file should not generate diagnostics for Log.fs file.

Why not? It's still a F# file inside current workspace -- just without project.
If anything there should be MORE diagnostics: All previous references (to stuff in other files or dependencies) should now be invalid -> more errors
(I know: not how project-less fs-files currently behave -- but would be great to get errors (including maybe a Not part of a project error). Currently dangling fs-files are a bit strange: completion & tooltips (and even CodeFixes) work -- but no diags...)

[MangelMaxime]

Editing the file should not generate diagnostics for Log.fs file.

Why not? It's still a F# file inside current workspace -- just without project. If anything there should be MORE diagnostics: All previous references (to stuff in other files or dependencies) should now be invalid -> more errors (I know: not how project-less fs-files currently behave -- but would be great to get errors (including maybe a Not part of a project error). Currently dangling fs-files are a bit strange: completion & tooltips (and even CodeFixes) work -- but no diags...)

What I mean is it should not generates diagnostics as if the file was still part of the project.

It should consider the file independent.

[MangelMaxime]

Thank you @Booksbaum , your solution seems to be working.

@baronfel I think there still one thing left to clean which is the pipeline hints but that less invasive that the diagnostics.

[baronfel]

The build errors are formatting related, run dotnet run --project build -t Format to handle that part.

The changes look reasonable to me, I'd need to check in Ionide but I think those are computed at the same time codelens are, s if you also send the 'workspace/codeLens/refresh' notification (from the LspClient) to get the editor to re-request those.

[MangelMaxime]

I also get bitten by the formatting issues.

If the formatting is required I think it be easier it was just run by default on the changed files instead of failing later.

In one of my project, I started using Husky.Net which allow to add git hooks and others stuff in your project.

With a Husky task like this one:

{
   "tasks": [
      {
         "name": "dotnet-format",
         "command": "dotnet",
         "args": [ "fantomas", "${staged}" ],
         "include": [ "**/*.fs", "**/*.fsi" ]
      }
   ]
}

and this pre-commit code dotnet husky run --name dotnet-format every time make a commit then all the files that have been staged are formatted before the commit is applied.

The only step required to have that working is to restore the dotnet tools and I think call dotnet husky install once so it can register some stuff. That could be done when restoring the package.

What do you think about this workflow?

[baronfel]

My initial thoughts are

• agree with a pre-commit hook,
• I'm concerned about introducing Husky - we already have Fake (though its use in the repo is diminishing) and I don't have a good idea about which would be used when

Go ahead and add it and we can see how it looks

[MangelMaxime]

It is possible to not use Husky and directly attach to the git hook mechanism.

However, we will need to handle more logic as we will not "benefit" from the Husky task runner. I will have a look at what this involve and if possible show both implementation for comparaison.

[MangelMaxime]

Hello @baronfel

I don't think the failing tests are related to my changes. If yes, don't hesitate to tell me :)

This PR should be ready to be merged. The issues we discussed about the auto formatting will be addressed in another PR.

[baronfel]

I agree on all counts. Going to kick the CI again since I know some of those failures are transient, and then we'll merge.

Thank you for your time and attention on this!

[baronfel]

force-pushed a merge resolution that also applies the fix to the new adaptive LSP server