Replies: 6 comments 5 replies
-
For nvim-jdtls I ended up going with 1), although I initially was also opposed to doing that. You can have a look at the change here In the end it wasn't as much logic as I had thought. |
Beta Was this translation helpful? Give feedback.
-
Thanks for linking that @mfussenegger! That's really helpful. That's also the route I'm planning on taking as well now when I get some time. |
Beta Was this translation helpful? Give feedback.
-
So I wanted to do a quick update on this as it's been quite some time since I've had time to dig back into nvim-metals. This weekend I've been able to put some effort into the following: No longer requiring nvim/nvim-lspconfigThe reason for me doing this is twofold. Firstly, there are plans to remove the ability to manage installations from nvim/nvim-lspconfig. Having this automated is quite nice in my opinion, so I'd like to continue having this feature in the future. More importantly, if you use nvim-metals, you need to override a bunch of handlers, add in if has('nvim-0.5')
augroup lsp
au!
au FileType scala lua require('metals').initialize_or_attach({})
augroup end
endif The Fully managed installation and update processI'm currently working on this at the moment. One of the important parts of the Metals plugins so far, is that we wanted an easy way to install and update Metals. In the VS Code extension, coc-metals, and the sublime plugin it was decided to just include an executable I'd honestly love to just leave the installation up to a package manager, coursier, but I don't want the user to have to deal with the full
They can use Since it's not, I'm currently leaning towards including the executable since it will make the process much more polished. Plus it allows for more fine grained control over things like being able to set a specific Java version for Metals which may be different then what is being used if you just use the bootstrapped Expect in the next couple weeks to see all of this land in master with some other cool updates. EDIT: After playing around some more, part of me is starting to believe that sort of a hybrid approach may be best. Basically, the user needs to have |
Beta Was this translation helpful? Give feedback.
-
Thank you for starting looking into this, something I had planned to do. It's unclear to me that lspconfig adds any value and having an opinionated way of doing things tailored to our case makes things much simpler. Same with a lot of the handlers and central configuration stuff. The client should register automatically for the diagnostic stuff, and for the textDocument/defintion etc. requests it's simpler to use something like I proposed in neovim/neovim#12277 (comment) Maybe I missed recent development but I don't think the nvim-lsp has improved much in that sense, there's still discussion how to configure different UIs and custom workflows like with codeActions. Those calls are user initiated requests and the callback is just to make an RPC async, we should pass the callback in the request, and then composing calls with FP combinators to model a workflow. Nvim-lsp has had very powerful building blocks for a long time, but then there is the expectation that it's going to provide a full package with allowing all possible configuration that complicates things with many changes impacting the adoption IMO. |
Beta Was this translation helpful? Give feedback.
-
This post will serve as a migration guide for anyone after #37 is merged. Prior to this, a setup would be quite involved with overriding a bunch of handlers and init options. For those of you that have been using nvim-metals up until this point, the following steps will apply to you.
local lspconfig = require'lspconfig'
local metals = require'metals'
local setup = require'metals.setup'
local M = {}
M.on_attach = function()
" Note that both of these are for external plugins
require'completion'.on_attach()
setup.auto_commands()
end
lspconfig.metals.setup{
on_attach = M.on_attach;
root_dir = metals.root_pattern("build.sbt", "build.sc", ".git");
init_options = {
-- If you set this, make sure to have the `metals#status()` function
-- in your statusline, or you won't see any status messages
statusBarProvider = "on";
inputBoxProvider = true;
quickPickProvider = true;
executeClientCommandProvider = true;
decorationProvider = true;
didFocusProvider = true;
};
handlers = {
["textDocument/hover"] = metals['textDocument/hover'];
["metals/status"] = metals['metals/status'];
["metals/inputBox"] = metals['metals/inputBox'];
["metals/quickPick"] = metals['metals/quickPick'];
["metals/executeClientCommand"] = metals["metals/executeClientCommand"];
["metals/publishDecorations"] = metals["metals/publishDecorations"];
["metals/didFocusTextDocument"] = metals["metals/didFocusTextDocument"];
};
} The same exact settings from above can now just be replaced with the following :lua << EOF
metals_config = {}
metals_config.handlers = {}
metals_config.init_options = {}
metals_config.on_attach = function()
require'completion'.on_attach();
end
metals_config.init_options.statusBarProvider = 'on'
EOF
if has('nvim-0.5')
augroup lsp
au!
au FileType scala lua require('metals').initialize_or_attach(metals_config)
augroup end
endif
|
Beta Was this translation helpful? Give feedback.
-
Seeing that the decisions on how this will all be done have now been made, I'm going to go ahead and lock this. |
Beta Was this translation helpful? Give feedback.
-
Question
As a follow-up to https://github.com/ckipp01/nvim-metals/pull/12#issuecomment-628661919, I think the setup needs to be addressed sooner rather than later.
Currently everything needs to be defined. For example, if you want to enable
metals/status
, then you need to do two things:statusBarProvider
to true in yourinit_options
metals/status
Then for every other thing that we are adding, we are having to do the same thing.
When I originally started this plugin I envisioned not have a "do this one thing to get started" like there is in coc-metals, but rather instructions in how to tie everything together to hopefully utilize a bunch of other great plugins. However, I'm quickly realizing that we are going to end up creating quite a few callbacks, a lot of the
init_option
need to be set since the configuration innvim-lsp
is the very base just for Metals to work correctly for a user out of the box withoutnvim-metals
. All that to say, I'm leaning more towards a simple way to add innvim-metals
, and have it do the necessary configuration without having a user add in 30 lines (apart from mappings of course). I think you also had a good point @landerlo about adding a new feature. Ideally a new feature can be added without the user having to adjust their setup.Approaches
nvim-lsp
at all. However, this would be the most work, and I'm not sure I want this. Just adding it here as an approachSearch terms
setup
Beta Was this translation helpful? Give feedback.
All reactions