fsprojects / fsharp-language-server

Other
219 stars 37 forks source link

Upgrade vscode and fix windows #92

Closed MangelMaxime closed 2 years ago

MangelMaxime commented 3 years ago

I am trying to work on some issues/improvements I would like to propose to fsharp-language-server and encountered some problem for running the project.

Here is a list of the issues and solution I followed to fix them:

When running npm install I got:

Detected VS Code engine version: ^1.24.0
Error installing vscode.d.ts: Error: Request returned status code: 404
Details: <!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Error</title>
</head>
<body>
<pre>Cannot GET /api/releases/stable</pre>
</body>
</html>

A solution is proposed on this issue https://github.com/microsoft/vscode/issues/119822#issuecomment-806489621 linking to a migration guide.

I followed the migration guide but didn't installed the vscode-test extensions because I don't think this project is running debugging inside of the VSCode.

After that I got an error from the TypeScript compiler

TS2304: Cannot find name 'unknown'.

In order, to fix it I upgraded the TypeScript compiler to latest version as unkown has been introduced in version 3 and the project was using version 2.

After that I got an error from the TypeScript compiler

TS2300: Duplicate identifier 'IteratorResult'.

This time I upgraded @types/node to the latest version.

When running the extension via the Extension debug task, I had the error Couldn't start client F# Language Server. The problem, was that on windows the command name as used in findInPath should be dotnet.exe and not just dotnet.

I added a platform detection so choose the right name to follow the actual code. But personally, I would just change the code to

serverOptions = { 
-   command: findInPath(commandName), 
+   command: "dotnet", 
    args: ['run', '--project', 'src/FSharpLanguageServer'], 
    transport: TransportKind.stdio,
    options: { cwd: context.extensionPath }
}

because the findInPath doesn't really provide anything. We don't add an error message or anything currently to help debug the issue.

Edit: If you want me to change the code to remove findInPath please tell me, I will update the PR or create a new one.

georgewfraser commented 3 years ago

Hey @MangelMaxime , can you try rebasing on latest master to see if this fixes the tests?

MangelMaxime commented 3 years ago

Sure will do

MangelMaxime commented 3 years ago

The CI is indeed fixed.

I don't know why the one test is still failing locally for me. The one about CSharp reference but it is not a problem to me if the CI validate the build.

faldor20 commented 2 years ago

Closing because this is no longer relevant. We are way past this version. Thanks for your work regardless :)