python-lsp / python-lsp-server

Fork of the python-language-server project, maintained by the Spyder IDE team and the community
MIT License
1.76k stars 186 forks source link

Support `initializationOptions` to configure the server #459

Closed tkrabel-db closed 8 months ago

tkrabel-db commented 9 months ago

What changes are proposed in this pull request?

Fixes #195.

The language server supports passing initializationOptions with the initialize message, but actually doesn't do anything with it. I think this was an oversight. This PR supports overriding configuration settings using the initializationOptions field.

How is this tested?

krassowski commented 9 months ago

Some context:

tkrabel-db commented 9 months ago

@krassowski thanks for the context.

I have a clear use case in mind: enabling rope_autoimport without a config file. From the code and https://github.com/python-lsp/python-lsp-server/issues/195 I see that workspace/didChangeConfiguration is the supported way to achieve that, and I am happy to go with that approach.

I feel the discussion around initializationOptions is unfortunate. There is no consensus around how initializationOptionsshould be used, but many developers find it very useful to configure the server on initialize. I think so too.

As more and more language servers will run in cloud VMs and not on a local machine, these servers need to be configurable dynamically. workspace/configure is a valid general approach to handle server configuration at runtime, but using that to enable/disable some features is overkill, wasteful and will lead to bad user behaviour.

tkrabel-db commented 9 months ago

Anyway, I moved my comment to #195, i.e. here

tkrabel-db commented 8 months ago

@ccordoba12 thanks for supporting this feature! I addressed you comment and also refactored the unit test to use the new test util functions