Repository: python-lsp/python-lsp-server

Issue:
Maybe use initializationOptions as additional source of settings
This came up in [this issue](https://github.com/kak-lsp/kak-lsp/issues/611) with the kak-lsp client.
Before the workaround for that issue, kak-lsp sent server configuration via the `initialize` request (in `initializationOptions`).
It only sent `workspace/didChangeConfiguration` when the configuration actually changed.
This caused problems because pylsp ignores `initializationOptions`. The workaround is to re-send options with `workspace/didChangeConfiguration`.

I wonder if it's a good idea to use `initializationOptions` as additional settings source.
I see two possible variants:
1. Always include `initializationOptions` but let `workspace/didChangeConfiguration` override its settings
2. As soon as `workspace/didChangeConfiguration` arrives, forget the `initializationOptions`

Annoyingly, option 2 would not have fixed the kak-lsp issue, because kak-lsp used to an empty 
settings object in `workspace/didChangeConfiguration` (but that's an issue on our side).

It's probably also fine to leave it; LSP is quite underspecified about how these requests interact. I heard that both are deprecated in favor of `workspace/configuration`..

Discussion:
krassowski: Some context:
- as per spec initialization options are different from configuration options; however there is no agreement as to how they are different; there was an attempt to standardize it in https://github.com/microsoft/language-server-protocol/commit/545cd6958ace603cd254e8e376e4fb2840bb1223 after discussion in https://github.com/microsoft/language-server-protocol/issues/567 but it was reverted due to lack of consensus
- this was previously discussed in https://github.com/python-lsp/python-lsp-server/issues/195 so it is not an omission - we should follow the discussion in there
- one way that these two could be use used differently, and are used differently by _some_ language server is following https://github.com/microsoft/language-server-protocol/commit/545cd6958ace603cd254e8e376e4fb2840bb1223: initialization options could include `pythonVersion: string` which is required during initialization but configuration options would have `showWarnings: boolean` which can be adjusted on runtime
tkrabel-db: @krassowski thanks for the context.

I have a clear use case in mind: enabling [rope_autoimport](https://github.com/python-lsp/python-lsp-server/blob/develop/pylsp/plugins/rope_autoimport.py) 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 `initializationOptions`should 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. 
- It's wasteful because you will end up turning off features that might have been enabled already on initialize, consuming resources unnecessarily (think e.g. rope_autoimport, which needs to index python modules; that problem becomes worse when the language server loves in an ephemeral environment like a cloud VM)
-  It will lead to bad user behaviour since the client will already send messages and potentially receive responses from features that the used didn't even want to use in the first place (e.g. getting ruff diagnostics instead of flake8 ones). 
- It is overkill because it will require extra logic to make sure the server is responsive while requesting `workspace/configuration` and while applying it (something that would be a no-brainer with `initializeOptions` as the server hasn't send a response to that yet, so the client keeps waiting)


tkrabel-db: Anyway, I moved my comment [here](https://github.com/python-lsp/python-lsp-server/issues/195#issuecomment-1761338131)
tkrabel-db: @ccordoba12 thanks for supporting this feature! I addressed you comment and also refactored the unit test to use the new test util functions

Patch:
diff --git a/pylsp/config/config.py b/pylsp/config/config.py
index b458069..454ee4b 100644
--- a/pylsp/config/config.py
+++ b/pylsp/config/config.py
@@ -98,6 +98,10 @@ class Config:
                 self._plugin_settings, plugin_conf
             )
 
+        self._plugin_settings = _utils.merge_dicts(
+            self._plugin_settings, self._init_opts.get("pylsp", {})
+        )
+
         self._update_disabled_plugins()
 
     @property


Test patch:
diff --git a/test/test_configuration.py b/test/test_configuration.py
new file mode 100644
index 0000000..91da421
--- /dev/null
+++ b/test/test_configuration.py
@@ -0,0 +1,53 @@
+# Copyright 2021- Python Language Server Contributors.
+
+from unittest.mock import patch
+
+from test.test_utils import send_initialize_request
+from test.test_notebook_document import wait_for_condition
+
+import pytest
+
+from pylsp import IS_WIN
+
+
+INITIALIZATION_OPTIONS = {
+    "pylsp": {
+        "plugins": {
+            "flake8": {"enabled": True},
+            "pycodestyle": {"enabled": False},
+            "pyflakes": {"enabled": False},
+        },
+    }
+}
+
+
+@pytest.mark.skipif(IS_WIN, reason="Flaky on Windows")
+def test_set_flake8_using_init_opts(client_server_pair):
+    client, server = client_server_pair
+    send_initialize_request(client, INITIALIZATION_OPTIONS)
+    for key, value in INITIALIZATION_OPTIONS["pylsp"]["plugins"].items():
+        assert server.workspace._config.settings().get("plugins").get(key).get(
+            "enabled"
+        ) == value.get("enabled")
+
+
+@pytest.mark.skipif(IS_WIN, reason="Flaky on Windows")
+def test_set_flake8_using_workspace_did_change_configuration(client_server_pair):
+    client, server = client_server_pair
+    send_initialize_request(client, None)
+    assert (
+        server.workspace._config.settings().get("plugins").get("flake8").get("enabled")
+        is False
+    )
+
+    with patch.object(server.workspace, "update_config") as mock_update_config:
+        client._endpoint.notify(
+            "workspace/didChangeConfiguration",
+            {"settings": INITIALIZATION_OPTIONS},
+        )
+        wait_for_condition(lambda: mock_update_config.call_count >= 1)
+
+        for key, value in INITIALIZATION_OPTIONS["pylsp"]["plugins"].items():
+            assert server.workspace._config.settings().get("plugins").get(key).get(
+                "enabled"
+            ) == value.get("enabled")
diff --git a/test/test_utils.py b/test/test_utils.py
index fb4a8b8..8b518d7 100644
--- a/test/test_utils.py
+++ b/test/test_utils.py
@@ -6,7 +6,7 @@ import os
 import sys
 from threading import Thread
 import time
-from typing import List
+from typing import Any, Dict, List
 from unittest import mock
 
 from flaky import flaky
@@ -62,12 +62,13 @@ def notebook_with_python_cells(cells: List[str]):
     }
 
 
-def send_initialize_request(client):
+def send_initialize_request(client, initialization_options: Dict[str, Any] = None):
     return client._endpoint.request(
         "initialize",
         {
             "processId": 1234,
             "rootPath": os.path.dirname(__file__),
+            "initializationOptions": initialization_options,
         },
     ).result(timeout=CALL_TIMEOUT_IN_SECONDS)