search

dense-analysis / ale

Check syntax in Vim/Neovim asynchronously and fix files, with Language Server Protocol (LSP) support
BSD 2-Clause "Simplified" License
13.55k stars 1.43k forks source link

A Few bugs with Metals integration #4220

Open frioux opened 2 years ago

frioux commented 2 years ago

I can submit a PR if people agree on what's required. Here are the issues:

  1. ale_linters/scala/metals.vim tries to use a binary called metals-vim, but that's not what the binary is called (anymore at least.) It's just metals.
  2. The root search code looks for build.sc, build.sbt, .bloop, and .metals using ale#path#ResolveLocalPath, but it looks like that function is only able to look for files, and .bloop and .metals are directories, so I think it's just the wrong function. What's the more correct function to use?
ckipp01 commented 2 years ago

👋🏼 One of the Metals maintainers here. Just a couple notes on this

ale_linters/scala/metals.vim tries to use a binary called metals-vim, but that's not what the binary is called (anymore at least.) It's just metals.

This is legacy, we used to have a metals-vim that was able to be installed with Coursier, but we've since moved away from that approach and instead clients should utilize InitializationOptions to ensure everything is set up correctly. You can find the Metals InitializationOptions here.

The root search code looks for build.sc, build.sbt, .bloop, and .metals using ale#path#ResolveLocalPath, but it looks like that function is only able to look for files, and .bloop and .metals are directories, so I think it's just the wrong function. What's the more correct function to use?

Imo, it doesn't make any sense to have .bloop/ and .metals/ in here since when using Metals those are only created after the root dir has been recognized and passed into Metals, and then Metals creates them.

zoonfafer commented 2 years ago

Thanks @frioux for reporting and thanks @ckipp01 for chiming in! I created the ale integration for metals but I find it lacking. I love the progress on nvim-metals but at the same time I don't feel like abandoning ale... so here I am!

@frioux, I basically followed @ckipp01's suggestions (actually I looked at https://github.com/scalameta/nvim-metals/blob/e6cd8ff487b0140863e683b2ea4cf7f0c14bc504/lua/metals/config.lua#L310 for inspirations) for what to look for. The default executable is also now just metals, but you can actually further configure it with:

let g:ale_scala_metals_executable = 'my_metals'

Would be great if I can borrow your eyes 👀 @ #4273

frioux commented 2 years ago

Sounds great! Glad to see progress on this!