Skip to content
This repository has been archived by the owner on Dec 29, 2022. It is now read-only.

Allow enablement of plugins in InitializeParams.initializationOptions #1026

Closed
mickaelistria opened this issue Aug 30, 2018 · 6 comments
Closed

Comments

@mickaelistria
Copy link

Several other LS, include VSCode JSon for instance, so allow usage of InitializeParams.initializationOptions to configure the initial behavior of the server.
I see RLS enables multiple options with DidChangeConfiguration. It would be nice of the same options could be passed as InitializeParams.initializationOptions as well for default enablement.

@alexheretic
Copy link
Member

You can avoid the double initial build by awaiting the first didChangeConfiguration, see #711.

@mickaelistria
Copy link
Author

The goal isn't to avoid the double initial build, but instead of allowing sending a default configuration as initialize argument very early and to avoid sending a didChangeConfiguration when there was actually no real change in configuration (as it's what we consider being the default).
Semantically, didChangeConfiguration means that the user or the IDE decided to change configuration of the running LS for whatever reason, while what we want to do is to send the initializationOptions at startup (to make them default for this session) to configure how the LS will behave from very first step. This is a subtle difference which I think make it worth allowing a the same settings in initializeOptions as in didChangeConfiguration.

@Xanewok
Copy link
Member

Xanewok commented Aug 30, 2018

I'm convinced it might be worth it to support passing initial config (same as we pass via didChangeConfiguration) via something like initializeOptions.config key or similar.

I stumbled upon a similar problem when trying to integrate RLS in Nuclide (atom-based) - the architecture didn't allow to easily pass initial config; the server was spawned in a generic remote LSP service factory, so getting back to that concrete RLS instance connection is hacky and unergonomic. I think that spawning it with actual initial config seems like a more principled approach, especially since we have designated initializeOptions to do that already.

@Xanewok
Copy link
Member

Xanewok commented Nov 28, 2018

This would be useful to other clients and I think it's a good first issue to tackle.
Some information that might be helpful if someone wants to tackle this:

The server logic executed as part of the initialization is here:

fn init<O: Output>(&self, init_options: &InitializationOptions, out: &O) {

The idea is to support optional Config value (defined in

pub struct Config {
)
in the init_options and try to initialize the config with the passed values instead, possibly taking into account also inferring useful defaults (logic defined in
pub fn infer_defaults(&mut self, project_dir: &Path) -> CargoResult<()> {
).

@mickaelistria
Copy link
Author

Thank you!

@norru
Copy link

norru commented Dec 1, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants