Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot pass the settings of rust-analyer on Neovim over Neovim LSP #15

Closed
YuanYuYuan opened this issue Feb 7, 2023 · 15 comments
Closed
Labels
usability This issue limits the usability of lsp mux

Comments

@YuanYuYuan
Copy link
Contributor

YuanYuYuan commented Feb 7, 2023

Hi, @pr2502 , I really appreciate your work on ra-multiplex. It seems that neovim cannot pass the settings into the rust-analyzer while using ra-multiplex. Let me give you an example. You can run luafile ./config.lua in neovimn to check it.

./config.lua

require('lspconfig').rust_analyzer.setup {
    cmd = { "ra-multiplex" },
    settings = {
        ["rust-analyzer"] = {
            check = {
                overrideCommand = {
                    "cargo",
                    "clippy",
                    "--message-format=json-diagnostic-rendered-ansi",
                    "--fix",
                    "--allow-dirty"
                }
            }
        }
    }
}

If I comment out the line cmd = { "ra-multiplex" }, then neovim would run cargo clippy, which is exactly what I want. Do you have any thoughts about addressing this issue? Thanks.

@pr2502
Copy link
Owner

pr2502 commented Feb 7, 2023

Hi, can you please enable debug logging in ra-multiplex (log_filters = "debug" in ~/.config/ra-multiplex/config.toml), try it again and attach the logged messages here, thanks!

@YuanYuYuan
Copy link
Contributor Author

Thanks for your rapid reply!

image

I'm afraid that nothing interesting was found.

@pr2502
Copy link
Owner

pr2502 commented Feb 7, 2023

I can't reproduce your neovim setup, you'll have to produce some logs yourself. I've found there is :LspLog command in the neovim lsp plugin, maybe you could log those while not using ra-multiplex and while using it to see the difference. My guess is either the command itself or the response to it comes as some notification without an ID and ra-multiplex can't forward those.

@YuanYuYuan
Copy link
Contributor Author

Yes, I had checked LspLog at the beginning, but nothing special was found. So I think I need to intercept the incoming setting JSON message from the client to the ra-multiplex wrapper. Or it might be the problem at neovim-lsp like it does not send the message at all.

@JeanMertz
Copy link

I don't have anything useful to report (yet), but here's a "me too" comment, in that I have the exact same setup (NeoVim, lspconfig, and wanting to run clippy instead of check.

Unfortunately, ra-multiplex does not seem to honour the provided configuration currently. If/when I find some more time, I'll see if I can dig up some additional logs, but at first glance, nothing relevant is being logged currently.

@IndianBoy42
Copy link

IndianBoy42 commented Jun 7, 2023

Remember to add vim.lsp.set_log_level(vim.log.levels.DEBUG) and RUST_LOG=trace

https://pastebin.com/ALJ4TnBg

https://pastebin.com/xfB3qWyF

Could it be the response to no request about a workspace/configuration message send from rust-analyzer, that doesn't go through to the neovim lsp client, which thus doesn't respond with any configuration.

You can see neovim receiving it and then shortly after responding with the full settings table when I disable ra-multiplex

https://pastebin.com/Ajez5j4M

is response to no request workspace/configuration being dropped expected behaviour? but then how do other clients work? configuration should also be sent through didChangedConfiguration so I don't think workspace/configuration being dropped should be a problem.

currently you only log::trace messages received from the server, so I am not sure

@IndianBoy42
Copy link

https://github.com/rust-lang/rust-analyzer/blob/9c03aa1ac2e67051db83a85baf3cfee902e4dd84/crates/rust-analyzer/src/handlers/notification.rs#L150

I'm not sure how (or if?) it worked with coc or any other client but ignoring workspace/configuration request from server to client will not work.

didChangeConfiguration (although it contains all the config data for legacy sake) is not actually used to set the configuration. it is a notification to tell the server to ask the client for the configuration.

This is spec compliant and what the maintainers of lsp want long term (microsoft/language-server-protocol#972)

@jefftt
Copy link

jefftt commented Jun 17, 2023

FWIW i got it to work by hardcoding the initializationOptions parameter in the proxied InitializeRequest. Not ideal but it gets the job done while we figure out a real solution. Can see the diff on my branch

@IndianBoy42
Copy link

Oh I forgot to link it but I fixed it by handling workspace/configuration requests from the server

@jefftt
Copy link

jefftt commented Jun 19, 2023

ah great! yes that's much better :)

@pr2502
Copy link
Owner

pr2502 commented Oct 25, 2023

We need to handle https://microsoft.github.io/language-server-protocol/specifications/specification-3-15/#workspace_configuration in some way.

#26 is an option but I think we should be more deliberate about how we handle each possible server request and fix this issue handling the workspace/configuration request only.

@pr2502 pr2502 added the usability This issue limits the usability of lsp mux label Oct 25, 2023
@pr2502
Copy link
Owner

pr2502 commented Mar 17, 2024

This should be fixed with #60, I'll try to recreate a neovim setup and confirm myself but if someone can verify on their own setup it'd be very helpful^^

@YuanYuYuan
Copy link
Contributor Author

YuanYuYuan commented Mar 19, 2024

Hi @pr2502! Thanks for your update. The following configuration works for me. Saying, we can either change the check behavior or modify the feature flags now.
Hooray! 😄

require'lspconfig'.rust_analyzer.setup {
  cmd = vim.lsp.rpc.connect("127.0.0.1", 27631),
  init_options = {
    lspMux = {
      version = "1",
      method = "connect",
      server = "rust-analyzer",
    },
  },
  settings = {
    ["rust-analyzer"] = {
      check = {
        overrideCommand = {
            "cargo",
            "clippy",
            "--message-format=json",
        }
      },
      cargo = {
        features = "all",
      }
    }
  }
}

BTW, I notice that if I need to revert the modification or change the overrideCommand back to the default "cargo check", we need to specify the value explicitly to override the previous status of ra-multiplex (if we don't shutdown the server).

@WhyNotHugo
Copy link
Contributor

WhyNotHugo commented Mar 19, 2024

I can confirm that @YuanYuYuan's solution works on 189397d.

@pr2502
Copy link
Owner

pr2502 commented Mar 19, 2024

Great! I'll close this now then^^

@pr2502 pr2502 closed this as completed Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
usability This issue limits the usability of lsp mux
Projects
None yet
Development

No branches or pull requests

6 participants