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

Implement LSP workspace/configuration and workspace/didChangeConfiguration #1684

Merged
merged 4 commits into from Feb 28, 2022

Conversation

Triton171
Copy link
Contributor

This partially fixes #1268, LSP configuration should now work with all servers (tested with texlab). Even though we don't yet allow for changing settings on the fly, providing the workspace/didChangeConfiguration capability has some advantages:

  • Some servers might continuously request the configuration if the capability isn't defined, in case any settings change.
  • The client receives the configuration even if it doesn't request it by itself, because we send a notification immediately after initialization. This is (probably) not required by the spec but Neovim does it as well.

Comment on lines 838 to 862
let doc = self.editor.documents().find(|doc| {
if let Some(server) = doc.language_server() {
if server.id() != server_id {
return false;
}
// The server may request the config for a specific document.
// Currently, the configs should all be the same but we might
// suport per-document configuration in the future.
if let Some(scope) = &item.scope_uri {
if Some(scope) != doc.url().as_ref() {
return false;
}
}
true
} else {
false
}
})?;
let mut config = doc.language_config()?.config.as_ref()?;
if let Some(section) = item.section.as_ref() {
for part in section.split('.') {
config = config.get(part)?;
}
}
Some(config)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd check scope uri first, look up the doc like this:

let path = params.uri.to_file_path().unwrap();
let doc = self.editor.document_by_path_mut(&path);

Comment on lines 557 to 564
let config = self.editor.documents().find_map(|doc| {
if doc.language_server().map(|server| server.id()) == Some(server_id) {
doc.language_config()
.and_then(|config| config.config.clone())
} else {
None
}
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We register language servers by language_config.scope:

helix/helix-lsp/src/lib.rs

Lines 309 to 314 in 5cfdb6d

let config = match &language_config.language_server {
Some(config) => config,
None => return Err(Error::LspNotDefined),
};
match self.inner.entry(language_config.scope.clone()) {

I'd add a reverse lookup method that can scan over (scope, server) in language_servers.iter() return scope where server.id() == id

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just saw that the LSP Client struct stores the config. I added a getter for it, that makes it a lot simpler.

Comment on lines 113 to 115
pub fn config(&self) -> &Option<Value> {
&self.config
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub fn config(&self) -> &Option<Value> {
&self.config
}
pub fn config(&self) -> Option<&Value> {
self.config.as_ref()
}

Comment on lines 557 to 558
let config = language_server.config();
if let Some(config) = config {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let config = language_server.config();
if let Some(config) = config {
if let Some(config) = language_server.config() {

Comment on lines 836 to 840
None => self.editor.documents().find(|doc| {
doc.language_server()
.map(|server| server.id() == server_id)
.unwrap_or(false)
})?,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be simplifiable now too by just using language_server.config()?

@David-Else
Copy link
Contributor

David-Else commented Apr 7, 2022

I am trying to setup the HTML language server, but it won't accept the "html.format.enable": true, option:

[[language]]
name = "html"
scope = "text.html.basic"
injection-regex = "html"
file-types = ["html"]
roots = []
language-server = { command = "vscode-html-language-server", args = ["--stdio"] }
auto-format = true
config = """
{
  "settings": {
    "html.format.enable": true,
  }
}
"""
indent = { tab-width = 2, unit = "  " }

It is documented here

I can't find any examples of using config = in the languages.toml file, I just copied it from #1898, am I making a mistake?

2022-04-07T18:22:39.972 helix_lsp::client [INFO] Using custom LSP config: "{\n  \"settings\": {\n    \"html.format.enable\": true,\n  }\n}\n"
2022-04-07T18:22:39.972 helix_lsp::transport [INFO] -> {"jsonrpc":"2.0","method":"initialize","params":{"capabilities":{"textDocument":{"codeAction":{"codeActionLiteralSupport":{"codeActionKind":{"valueSet":["","quickfix","refactor","refactor.extract","refactor.inline","refactor.rewrite","source","source.organizeImports"]}}},"completion":{"completionItem":{"snippetSupport":false},"completionItemKind":{}},"hover":{"contentFormat":["markdown"]},"rename":{"dynamicRegistration":false,"honorsChangeAnnotations":false,"prepareSupport":false}},"window":{"workDoneProgress":true},"workspace":{"configuration":true,"didChangeConfiguration":{"dynamicRegistration":false},"workspaceFolders":true}},"initializationOptions":"{\n  \"settings\": {\n    \"html.format.enable\": true,\n  }\n}\n","processId":597749,"rootPath":"/home/david/Downloads","rootUri":"file:///home/david/Downloads","workspaceFolders":[{"name":"Downloads","uri":"file:///home/david/Downloads"}]},"id":0}
2022-04-07T18:22:40.464 helix_lsp::transport [INFO] <- {"jsonrpc":"2.0","id":0,"result":{"capabilities":{"textDocumentSync":2,"hoverProvider":true,"documentHighlightProvider":true,"documentRangeFormattingProvider":false,"documentFormattingProvider":false,"documentLinkProvider":{"resolveProvider":false},"documentSymbolProvider":true,"definitionProvider":true,"signatureHelpProvider":{"triggerCharacters":["("]},"referencesProvider":true,"colorProvider":{},"foldingRangeProvider":true,"selectionRangeProvider":true,"renameProvider":true,"linkedEditingRangeProvider":true}}}
2022-04-07T18:22:40.464 helix_lsp::transport [INFO] <- {"capabilities":{"colorProvider":{},"definitionProvider":true,"documentFormattingProvider":false,"documentHighlightProvider":true,"documentLinkProvider":{"resolveProvider":false},"documentRangeFormattingProvider":false,"documentSymbolProvider":true,"foldingRangeProvider":true,"hoverProvider":true,"linkedEditingRangeProvider":true,"referencesProvider":true,"renameProvider":true,"selectionRangeProvider":true,"signatureHelpProvider":{"triggerCharacters":["("]},"textDocumentSync":2}}
2022-04-07T18:22:40.464 helix_lsp::transport [INFO] -> {"jsonrpc":"2.0","method":"initialized","params":{}}
2022-04-07T18:22:40.465 helix_lsp::transport [INFO] -> {"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":"{\n  \"settings\": {\n    \"html.format.enable\": true,\n  }\n}\n"}}
2022-04-07T18:22:40.465 helix_lsp::transport [INFO] -> {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"languageId":"basic","text":"<!DOCTYPE html>\n\n\n <html>\n             <body>\n        <h1>My First Heading</h1>\n\n    <p>My first paragraph.</p>\n  </body>\n</html>\n","uri":"file:///home/david/Downloads/hello.html","version":0}}}
2022-04-07T18:22:40.466 helix_lsp::transport [INFO] <- {"jsonrpc":"2.0","id":0,"method":"client/registerCapability","params":{"registrations":[{"id":"42866e65-0524-4f2d-bd8a-d84033de89b4","method":"workspace/didChangeWorkspaceFolders","registerOptions":{}}]}}
2022-04-07T18:22:40.466 helix_lsp [WARN] unhandled lsp request: client/registerCapability
2022-04-07T18:22:40.466 helix_term::application [ERROR] Method not found client/registerCapability
2022-04-07T18:22:43.025 helix_lsp::transport [INFO] -> {"jsonrpc":"2.0","method":"textDocument/didChange","params":{"contentChanges":[{"range":{"end":{"character":0,"line":3},"start":{"character":0,"line":3}},"text":" "}],"textDocument":{"uri":"file:///home/david/Downloads/hello.html","version":1}}}
2022-04-07T18:22:48.649 helix_lsp::transport [INFO] -> {"jsonrpc":"2.0","method":"shutdown","params":null,"id":1}
2022-04-07T18:22:48.650 helix_lsp::transport [INFO] <- {"jsonrpc":"2.0","id":1,"result":null}
2022-04-07T18:22:48.650 helix_lsp::transport [INFO] <- null
2022-04-07T18:22:48.650 helix_lsp::transport [INFO] -> {"jsonrpc":"2.0","method":"exit","params":null}

Neovim settings are https://github.com/neovim/nvim-lspconfig/blob/master/lua/lspconfig/server_configurations/html.lua

@Triton171
Copy link
Contributor Author

I think the issue is that you have to provide the config in the toml format. Helix will then convert it to json before sending it to the Language Server. For you, the correct snippet should be:

config = { settings = { "html.format.enable" = true } }

This should probably be documented somewhere, it can be a bit unexpected.

@David-Else
Copy link
Contributor

@Triton171 Cheers! I am afraid it doesn't work, but I think that is because what is actually needed is https://microsoft.github.io/language-server-protocol/specification#initialize

Looking at the Neovim Wiki https://github.com/neovim/nvim-lspconfig/wiki/Understanding-setup-%7B%7D I see:

init_options

init_options corresponds to initializationOptions in the initialize request. These options may overlap with settings depending on the server, and less frequently need to be configured by the user.

The HTML server is Neovim uses:

    init_options = {
      provideFormatter = true,
      embeddedLanguages = { css = true, javascript = true },
      configurationSection = { 'html', 'css', 'javascript' },
    },

Is there a way to do this in Helix?

@sudormrfbin
Copy link
Member

sudormrfbin commented Jul 8, 2022

@David-Else The contents of the config key is what's send as initialization options, so I think it's a problem with how the setting is send. This might work (note the lack of quotes around the html.format.enable key, which makes toml treat it as a tested table):

[[language]]
name = "html"
config = { html.format.enable = true }

In Neovim only the contents of the settings table in nvim-lspconfig is sent to the server, i.e. settings in Neovim == config in Helix (I'm not sure about this now, since it seems helix sources both the workspace configuration and initialization options from the config key -- maybe someone more familiar with the LSP side of the editor can clear that up).

The HTML server is Neovim uses:

    init_options = {
      provideFormatter = true,
      embeddedLanguages = { css = true, javascript = true },
      configurationSection = { 'html', 'css', 'javascript' },
    },

Is there a way to do this in Helix?

[[langauge]]
name = "html"

[language.config]
provideFormatter = true
embeddedLanguages = { css = true, javascript = true }
configurationSection = { "html", "css", "javascript" }

might work.

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.

Implementing workspace/configuration
4 participants