Add Expert LSP support for Elixir#9398
Conversation
Wire up Expert as a built-in LSP server for Warp's editor. For now, Warp only uses Expert when `start_expert` is already available on `PATH`. It does not auto-install Expert or bundle a runtime. Expert is becoming the main Elixir language server under the EEF, so this was preferred over ElixirLS. This adds: * Elixir language detection for `.ex`, `.exs`, `.eex`, `.heex`, `.leex`, `.neex`, plus `mix.exs`, `mix.lock`, and `.formatter.exs` * `LSPServerType::Expert`, which runs `start_expert --stdio` * Workspace detection through `mix.exs`, `mix.lock`, or `.formatter.exs` * An install message that points users to the Expert releases page Tree-sitter highlighting is already covered by arborium' `lang-elixir` feature, so there are no crate or language wiring changes needed.
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @manusajith on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
I'm starting a first review of this pull request. I requested changes on this pull request and posted feedback. Comment I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I requested changes on this pull request and posted feedback. Comment Powered by Oz |
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR adds Elixir language detection and wires the new Expert LSP server type through the LSP server registry.
Concerns
- Expert is suggested for Mix repos even when
start_expertis not installed, but this PR does not provide an auto-install implementation. Existing setup/settings flows treat that state as installable and will route users to an install action that always fails.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
Only suggest Expert when start_expert is already on PATH.
|
I'm checking this implementation PR for association with a likely matching ready issue. Powered by Oz |
|
/oz-review |
There was a problem hiding this comment.
Overview
This PR adds Elixir/Expert language detection, server configuration, and candidate wiring for Warp's LSP support.
Concerns
- Expert is configured to find and launch only
start_expert, but the public releases linked by the install guidance provide the released executable (expertor platform-specificexpert_*) rather thanstart_expert, so users following the documented install path will not be detected or started. - Template extensions are mapped to the same
LanguageId::Elixirvalue, which means Warp sendselixiras the LSPlanguageIdfor EEx/HEEx-style buffers instead of template-specific identifiers. - Supplemental security pass: no security-specific issues found in the changed lines.
Verdict
Found: 0 critical, 2 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| LSPServerType::Pyright => "pyright-langserver", | ||
| LSPServerType::TypeScriptLanguageServer => "typescript-language-server", | ||
| LSPServerType::Clangd => "clangd", | ||
| LSPServerType::Expert => "start_expert", |
There was a problem hiding this comment.
expert or a platform-specific expert_*, while start_expert is only present in source-built plain releases. Since detection and spawning use binary_name(), users following the linked releases install path will be reported as not installed and unable to start Expert; support the release binary or both names before enabling this.
| // compile_commands.json is present, clangd will use the correct language | ||
| // regardless of the languageId we send. | ||
| "h" | "H" | "hh" | "hpp" | "hxx" => Some(Self::Cpp), | ||
| "ex" | "exs" | "eex" | "heex" | "leex" | "neex" => Some(Self::Elixir), |
There was a problem hiding this comment.
.eex/.heex/.leex/.neex to LanguageId::Elixir makes didOpen send languageId elixir for template files. Expert/editor configs treat EEx/HEEx as distinct filetypes (eelixir/heex), so template buffers can be parsed as plain Elixir; add distinct language IDs or otherwise send the template-specific identifiers.
The Expert release binaries are named `expert_<platform>` (renamed to `expert` on PATH); `start_expert` only exists in source-built plain releases. Spawning and detection both go through binary_name, so users following the documented install path were never matched. Templates also need distinct LSP language IDs: EEx and HEEx are treated as their own filetypes by Expert and other Elixir LSPs, so sending `elixir` for `.eex` / `.heex` buffers means template syntax is parsed as plain Elixir.
1 participant
Description
Wire up Expert as a built-in LSP server for Warp's editor.
For now, Warp only uses Expert when
start_expertis already available onPATH. It does not auto-install Expert or bundle a runtime.Expert is becoming the main Elixir language server under the EEF, so this was preferred over ElixirLS.
This adds:
.ex,.exs,.eex,.heex,.leex,.neex, plusmix.exs,mix.lock, and.formatter.exsLSPServerType::Expert, which runsstart_expert --stdiomix.exs,mix.lock, or.formatter.exsTree-sitter highlighting is already covered by arborium's
lang-elixirfeature, so there are no crate or language wiring changes needed.Testing
crates/lsp/src/config_tests.rscovering:.ex,.exs,.eex,.heex,.leex,.neexcargo test -p lsp,cargo fmt -- --checkServer API dependencies
N/A
Agent Mode
Changelog Entries for Stable
CHANGELOG-IMPROVEMENT: Added built-in support for the Expert language server for Elixir in the editor (install via the Expert releases page so that
start_expertis on yourPATH).