Unify initialization process for all editors #680
Conversation
Right now, our LSP only requests settings when client is VSCode or NeoVim. To make matters worse, it requests *different* settings for these clients: - for VSCode, it requests VSCode-specific about files and extensions, - for NeoVim, it requests settings in scope `"Lua"`, and uses them as overrides for `.emmyrc`. This PR aims to unify this behavior and make it easier for users to override settings without having to create `.emmyrc.json`. This is especially handy when you want to override a setting for a single project, but don't want to change project's `.emmyrc.json`. For example, switching documentation highlighting on per-project basis will become easier (see discussion in EmmyLuaLs#665). # Changes - When client is VSCode, LSP will request settings under `"emmylua"` scope, as well as VSCode-specific settings mentioned above. I'll add relevant settings to the plugin settings page in a separate PR when this is merged; until then, these changes will not affect user experience. - When client is NeoVim, LSP will request settings under `"emmylua"` scope. If there's no such scope, it will fall back to using the `"Lua"` scope. Currently, both LuaLs and EmmyLua use the same scope `"Lua"`. It works in NeoVim because options for every LSP server are independent. In VSCode, however, they all share the same namespace, meaning that we can't reuse `"Lua"`. I'd like to keep things consistent, hence this rename. I'll send a PR to update nvim-lspconfig's documentation for EmmyLua when this is merged. - For any other client, LSP will request settings under `"emmylua"` scope. Currently, there are no clients that provide this scope (EmmyLua2 for Intellij doesn't provide any config scopes), so it shouldn't break anything, but it will provide future client implementations with a better way to override LSP configuration. # Tests Tested this with VSCode and NeoVim.
| let main_workspace_folder = workspace_folders.get(0); | ||
| let client = &context.client; | ||
| let scope_uri = main_workspace_folder.map(|p| file_path_to_uri(p).unwrap()); |
There was a problem hiding this comment.
Note: this rolls back a change made in previous commit. This is because NeoVim doesn't require workspace root to run EmmyLua. In this case, workspace_folders will be empty, but we should still request configs from the client.
Let me know if I've missed something and I shouldn't change this.
…c.json We've recently updated EmmyLua to make initialization consistent across various editors (see EmmyLuaLs/emmylua-analyzer-rust#680). This PR updates the documentation to reflect the changes.
crates/emmylua_ls/src/handlers/initialized/client_config/mod.rs
Outdated
Show resolved
Hide resolved
| ClientId::Neovim => get_client_config_neovim(context, &mut config).await, | ||
| _ => Some(()), | ||
| ClientId::Neovim => { | ||
| get_client_config_default(context, &mut config, Some(&["Lua", "emmylua"])).await |
There was a problem hiding this comment.
I think since LuaLS has already become the user preference for Neovim, there’s no need to request emmyLua anymore. Also, “Lua” is capitalized, while “emmylua” is lowercase, which doesn’t look as harmonious.
There was a problem hiding this comment.
I want to make it similar to VSCode. See PR description for detailed explanation:
Currently, both LuaLs and EmmyLua use the same scope "Lua". It works in NeoVim because options for every LSP server are independent. In VSCode, however, they all share the same namespace, meaning that we can't reuse "Lua". I'd like to keep things consistent, hence this rename.
| context: &ServerContextSnapshot, | ||
| config: &mut ClientConfig, | ||
| ) -> Option<()> { | ||
| get_client_config_default(context, config, None).await; |
There was a problem hiding this comment.
Why request configuration? The existing configuration in EmmyLua is unrelated to the language server itself; it’s mainly for color settings.
There was a problem hiding this comment.
I want to provide a way to change non-project-related settings (like which inlay hints to display, whether to render markdown in comments or not, etc.) from VSCode interface. This approach repeats what rust-analyzer and several other LSPs do.
Current WIP on VSCode settings:
Plus, I want to provide a standard way for all editors to change EmmyLua's config.
There was a problem hiding this comment.
Manual maintenance is very difficult. LuaLS generates its package.json, so if this is to be implemented, I hope it can be generated automatically.
There was a problem hiding this comment.
With schema export already present, this shouldn't be hard to implement. I'll add it when sending PR to the VSCode extension.
2 participants
Right now, our LSP only requests settings when client is VSCode or NeoVim. To make matters worse, it requests different settings for these clients:
"Lua", and uses them as overrides for.emmyrc.This PR aims to unify this behavior and make it easier for users to override settings without having to create
.emmyrc.json. This is especially handy when you want to override a setting for a single project, but don't want to change project's.emmyrc.json. For example, switching documentation highlighting on per-project basis will become easier (see discussion in #665).Changes
When client is VSCode, LSP will request settings under
"emmylua"scope, as well as VSCode-specific settings mentioned above.I'll add relevant settings to the plugin settings page in a separate PR when this is merged; until then, these changes will not affect user experience.
When client is NeoVim, LSP will request settings under
"emmylua"scope. If there's no such scope, it will fall back to using the"Lua"scope.Currently, both LuaLs and EmmyLua use the same scope
"Lua". It works in NeoVim because options for every LSP server are independent. In VSCode, however, they all share the same namespace, meaning that we can't reuse"Lua". I'd like to keep things consistent, hence this rename.I'll send a PR to update nvim-lspconfig's documentation for EmmyLua when this is merged.
For any other client, LSP will request settings under
"emmylua"scope.Currently, there are no clients that provide this scope (EmmyLua2 for Intellij doesn't provide any config scopes), so it shouldn't break anything, but it will provide future client implementations with a better way to override LSP configuration.
Tests
Tested this with VSCode and NeoVim.