scalameta / metals

Scala language server with rich IDE features 🚀
https://scalameta.org/metals/
Apache License 2.0
2.06k stars 320 forks source link

Document existing editor clients and instructions on how to integrate a new one #217

Closed olafurpg closed 6 years ago

olafurpg commented 6 years ago

It would be good to document an overview of clients that have been used with metals.

A clients.md in the repo should be enough to begin with.

rossabaker commented 6 years ago

I just published lsp-scala with some instructions and an experience report.

Big :+1: to this ticket. Letting people know this can work for any of these editors is going to be good for all of these editors.

gvolpe commented 6 years ago

I got it working on NeoVim and wrote the instructions I followed in the Readme file: https://github.com/gvolpe/vim-setup

Metals rocks!!! :ok_hand:

olafurpg commented 6 years ago

Awesome @gvolpe ! You might be interested in the new overview (https://github.com/scalameta/metals/blob/master/docs/overview.md) document that explains each features and what configuration is required to make it work.

I just remembered one undocumented clien requirement: metals expects to be notified by the client when files matching the patterns **/*.compilerconfig and **/*.semanticdb change via workspace/didChangeWatchedFiles (https://microsoft.github.io/language-server-protocol/specification#workspace_didChangeWatchedFiles) This might explain why definition/references didn't work in emacs @rossabaker

gabro commented 6 years ago

@olafurpg I don't know if it makes sense to include it in the scope of this issue, but a "client implementors vademecum" would definitely be useful.

There are non-obvious things that are expected from the client that are currently undocumented, namely:

In the meantime, I can recommend:

/cc @gvolpe @rossabaker

olafurpg commented 6 years ago

That's a great idea @gabro !

We're in the process of moving the *.compilerconfig files generated by metalsSetup to .metals/buildinfo/ and using .properties file extension. See #229 Once that migration is complete we should be able to write up more permanent documentation for editor client requirements with respect to file watching and configuration.

ivanopagano commented 6 years ago

Using with Sublime

I managed to run the server with the Sublime 3 editor using the "officially available" extension.

Details are here

The instructions are specific to my setup and as such will not necessarily work for anyone else. They can be considered as the results of ongoing experimentation with sublime lsp integration.

olafurpg commented 6 years ago

Thanks for the writeup @ivanopagano !

For scalafix linting, you need a .scalafix.conf file enabling a couple rules. There should appear errors in the buffer with the source "scalafix". Compilation errors from sbt/presentation-compiler have source "scalac".

For code actions, you need to have an "unused import" warning in the file and move your cursor above the range of the warning. A lightbulb should appear then to "Remove unused imports". This is still just a proof-of-concept testing the whole refactoring pipeline. We have a lot more nice scalafix refactorings in the roadmap :)

olafurpg commented 6 years ago

PSA. We just merged #229 which is one of the last editor client affecting changes we plan before the v0.1 release. Note that this changes the location of the *.compilerconfig files so metalsSetup (*: prefix is no longer needed) will need to be re-run.

Next change in the line is #230

ivanopagano commented 6 years ago

@olafurpg what is not clear to me is if scalafix is enabled implicitly by parts of the metals plugin, or if I need to add the plugin myself to the project...

I tried creating the rules file but I can't make it work it seems...

gabro commented 6 years ago

@olafurpg what is not clear to me is if scalafix is enabled implicitly by parts of the metals plugin, or if I need to add the plugin myself to the project...

scalafix comes with metals. It only requires the semanticdb setup, so you need to enable that and then compile.

olafurpg commented 6 years ago

Do the vim/sublime/emacs LSP clients support workspace/didChangeWatchedFiles? I am wondering if the server should implement it's own file watching for clients that communicate in initialize that they don't support file watching.

gvolpe commented 6 years ago

@olafurpg for vim is not yet supported: https://github.com/autozimu/LanguageClient-neovim/issues/347 But I guess this should be a responsibility of the client, not the server.

rossabaker commented 6 years ago

didChangeWatchedFiles is currently unsupported in both lsp-mode and lsp-scala.

Is the server registering for which files to watch? I can't find anything in metals pertaining to DidChangeWatchedFilesRegistrationOptions. I think that would permit a generic client implementation vs. more metals-aware glue code.

gabro commented 6 years ago

Is the server registering for which files to watch?

No, so far this is a task delegated to clients.

Given the current state of file watching in clients, we may need to work around this, although I'm not excited about it. Technically speaking metals could not even be running on the same machine as the editor.

olafurpg commented 6 years ago

The server is not using DidChangeWatchedFilesRegistrationOptions to watch those files, it's defined currently in each editor client plugin https://github.com/scalameta/metals/blob/5c0827aaafab5548c4ecc71c707c9f30732bc074/vscode-extension/src/extension.ts#L48-L49

I agree it would be great to delegate this entirely to the client, so using DidChangeWatchedFilesRegistrationOptions seems like the correct long-term solution.

gabro commented 6 years ago

If we want to tackle this, I would definitely prefer client's file watching if available (i.e. didChangeWatchedFiles is included in the WorkspaceClientCapabilities) and fallback to server file watching if not.

Also: file watching is hard.

olafurpg commented 6 years ago

I opened #256 adding docs for integrating new editor clients with Metals based on our previous discussions. Please consider contributing if you are affected by problems where metals is not compliant with LSP, for example #255 or #216. I am happy to give guidance on how to best approach those.

olafurpg commented 6 years ago

Please comment on the PR if I missed something!

olafurpg commented 6 years ago

Somewhat related, I opened #257 to document how to integrate a new build tool with metals.