Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

[Question] How to override a specific lsp config #742

Closed
midnightexigent opened this issue Sep 10, 2021 · 11 comments
Closed

[Question] How to override a specific lsp config #742

midnightexigent opened this issue Sep 10, 2021 · 11 comments

Comments

@midnightexigent
Copy link
Contributor

Hi,

I just found out about this project, very promising stuff

I am trying to customize the lsp config: I install rust-analyzer with the rustup nightly toolchain meaning that the command to run it is rustup run nightly rust-analyzer

Is there a way to configure helix so it runs that command for rust-analyzer?

@midnightexigent midnightexigent changed the title [Question] How do I override a specific lsp config [Question] How to override a specific lsp config Sep 10, 2021
@kirawi
Copy link
Member

kirawi commented Sep 10, 2021

rust-analyzer needs to be present in your PATH environment variable. I don't think rustup adds it for you. If you want to configure LSPs in the future though, that's done through the languages.toml file.

@midnightexigent
Copy link
Contributor Author

Thanks for the reply,

I tried that, but now I am encountering a different issue:

I put this in languages.toml

[[language]]
name = "rust"
language-server = { command = "rustup", args = ["run", "nightly", "rust-analyzer"] }
config = """
{
  "cargo": {
    "allFeatures": true,
  },
  "checkOnSave": {
    "command": "clippy",
  }
}
"""

But helix is not using the config I provided (I am not getting cargo clippy warnings)

@kirawi
Copy link
Member

kirawi commented Sep 10, 2021

Try turning on logging via -v and then look at the log.

@midnightexigent
Copy link
Contributor Author

I tried that, but the config payload is nowhere to be seen in the logs, there is just the capabilities. I also tried with -vvv but didn't get the information in the logs

@kirawi
Copy link
Member

kirawi commented Sep 10, 2021

Seems like it doesn't work because the rustup process ends early.

@midnightexigent
Copy link
Contributor Author

midnightexigent commented Sep 11, 2021

So is it a bug ? Or am I doing something wrong in the config ?

Also, changing back to

language-server = { command = "rust-analyzer" }

is no good either

@kirawi
Copy link
Member

kirawi commented Sep 11, 2021

That's because rust-analyzer is not present in your PATH environment variable; rustup install rust-analyzer doesn't add it. You'll have to install it through some other method, such as a package manager or manually, because it's not yet possible to do with rustup (rust-lang/rustup#2411).

@midnightexigent
Copy link
Contributor Author

Sorry, I meant that I tried this

language-server = { command = "rust-analyzer" }

with rust-analyzer in my PATH but the config wasn't being applied either.

Anyhow, I figured out the problem: the json parsing of the provided config was failing silently due to the trailing commas.

Since we're already in a toml file, shouldn't the config be passed in as a toml::Value rather than a json string ?

@kirawi
Copy link
Member

kirawi commented Sep 11, 2021

No, it has to be JSON according to the LSP spec. As for trailing commas, we should probably be logging such errors https://github.com/serde-rs/json/blob/8604ef948bec5190db6fa23d362bd3368620990c/src/error.rs#L66

serde_json::from_str(language_config.config.as_deref().unwrap_or("")).ok(),

I'll get it done later today.

@midnightexigent
Copy link
Contributor Author

midnightexigent commented Sep 11, 2021

yes, I meant read it as a toml::Value from the config, and the convert it to a serde_json::Value, something like this

let toml_value: toml::Value = toml::from_str(r#"
    [config.cargo]
    allFeatures = true
    [config.checkOnSave]
    command = "clippy"
  "#).unwrap();

let json_value: serde_json::Value = toml_value.try_into().unwrap();

println!("{}", serde_json::to_string_pretty(&json_value).unwrap())

@kirawi
Copy link
Member

kirawi commented Sep 11, 2021

This goes outside of what I know (I haven't worked with the LSP code), but I suspect that it's because converting from JSON to Toml didn't really make sense. This was brought up before in the past before this was implemented, and I guess we decided to stick with JSON.

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants