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

Refactor AdaptiveServerState.forceFindOpenFileOrRead #1208

Conversation

1eyewonder
Copy link
Contributor

@1eyewonder 1eyewonder commented Nov 18, 2023

WHAT

🤖[deprecated] Generated by Copilot at 5501b05

The pull request simplifies the error handling of various functions and methods in the FsAutoComplete project by using option and asyncOption types instead of Result and ResultOrString types. This avoids unnecessary wrapping and unwrapping of results and allows returning more informative error messages. The pull request affects the files src/FsAutoComplete.Core/Commands.fs, src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs, src/FsAutoComplete/LspServers/AdaptiveServerState.fs, src/FsAutoComplete/CodeFixes.fs, src/FsAutoComplete/CodeFixes.fsi, and several files in the src/FsAutoComplete/CodeFixes folder.

🤖[deprecated] Generated by Copilot at 5501b05

We're sailing on the asyncOption tide
We've left the Result types behind
We'll heave away and simplify the code
And make the error messages more kind

📄🛠️🚀

WHY

This function was being used as a Result type and often times being converted to an option type. Since the function either was opened or it wasn't, I felt this refactor expresses the intent more clearly. It think it also allows the higher order functions to "think" more on what better error messages could be, rather than passing the "could not read file" up the pipeline.

HOW

🤖[deprecated] Generated by Copilot at 5501b05

  • Simplify the error handling and avoid wrapping the results in Result or option types by using the asyncResultOption workflow in the AdaptiveFSharpLspServer module (link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link)
  • Change the type of the tryGetFileCheckerOptionsWithLines parameter from a function that returns an Async<Result<IFSACSourceText, _>> to a function that returns an Async<IFSACSourceText option> in the Commands module, and handle the case where the function returns None by returning an error message with the file name (link, link, link, link, link)
  • Change the type of the tryGetFileSource parameter from a function that returns an Async<ResultOrString<IFSACSourceText>> to a function that returns an Async<IFSACSourceText option> in the Commands module, and handle the case where the function returns None by returning an error message with the file name and a flag to indicate whether the error should be fatal or not (link, link, link)
  • Change the type of the GetFileLines alias from a function that returns an Async<ResultOrString<IFSACSourceText>> to a function that returns an Async<IFSACSourceText option> in the CodeFixes module, and handle the case where the function returns None by returning an error message with the file name in the code fixes (link, link, link, link, link, link, link, link)
  • Change the name of the forceFindOpenFileOrRead function to tryForceFindOpenFileOrRead in the AdaptiveServerState module, and handle the case where the function returns None by returning an error message with the file name or using the asyncOption workflow (link, link, link, link, link, link, link, link, link)
  • Change the return type of the GetOpenFileOrRead method from Async<Result<VolatileFile, string>> to Async<VolatileFile option> in the AdaptiveServerState module, and handle the case where the method returns None by returning an error message with the file name or using the asyncOption workflow (link, link, link, link, link, link, link, link, link, link)

Note

I also updated FsToolkit.ErrorHandling to the latest version

@1eyewonder 1eyewonder closed this Dec 2, 2023
let ranges: FSharp.Compiler.Text.Range[] =
match res with
| LocationResponse.Use(_, uses) -> uses |> Array.map (fun u -> u.Range)
let ranges: FSharp.Compiler.Text.Range[] =

Check notice

Code scanning / Ionide.Analyzers.Cli

Detect if type[] is used instead of type array Note

Prefer postfix syntax for arrays.
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.

4 participants