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

Look up relative paths to the language server in PATH first #198

Merged
merged 1 commit into from
Nov 22, 2023

Conversation

paveloom
Copy link
Contributor

In the case the custom path to the language server is relative, look it up in PATH first. Useful if the name of the language server is swift-mesonlsp instead of Swift-MesonLSP.

@tristan957
Copy link
Contributor

I guess I am trying to understand a little bit more. Could provide some more context? :)

@paveloom
Copy link
Contributor Author

Currently, if one sets a custom language server path in the extension options, and they use a binary name (e.g., swift-mesonlsp), this will not work, as the path will be resolved as file:///swift-mesonlsp:

[Error - 7:18:36 PM] Meson Language Server (Swift-MesonLSP) client: couldn't create connection to server.
Launching server using command /swift-mesonlsp failed. Error: spawn /swift-mesonlsp ENOENT

This is handled here:

if (config["languageServerPath"] !== null && config["languageServerPath"] != "")
return vscode.Uri.from({ scheme: "file", path: config["languageServerPath"] });

That's because vscode.Uri.from expects already resolved paths.

Now, if one doesn't use a custom server path, then the extension will eventually look up PATH for the binary:

const binary = which.sync(server!, { nothrow: true });
if (binary !== null) return vscode.Uri.from({ scheme: "file", path: binary });

I copied this logic to solve the above scenario.

@JCWasmx86
Copy link
Contributor

Useful if the name of the language server is swift-mesonlsp instead of Swift-MesonLSP.

Why would that ever be the case?

@paveloom
Copy link
Contributor Author

paveloom commented Nov 22, 2023

Well, I just have it like that in Nixpkgs. Might change it, though, so as to align with your packages, @JCWasmx86.

@tristan957
Copy link
Contributor

I see. Thanks for the context. Looks like this is good to merge. What do you think @JCWasmx86?

@JCWasmx86
Copy link
Contributor

LGTM

@tristan957 tristan957 merged commit d7594cc into mesonbuild:main Nov 22, 2023
5 checks passed
@paveloom paveloom deleted the resolve-language-server-path branch November 22, 2023 20:55
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.

None yet

3 participants