Merge pull request #919 from krassowski/overrides-fix

Fix `overrides.json` not being picked up in defaults

Author: krassowski

CHANGELOG.md

@@ -1,5 +1,17 @@
 ## Changelog
 
+### `@jupyter-lsp/jupyterlab-lsp 4.0.1` (unreleased)
+
+- bug fixes
+  - overrides from `overrides.json` are now picked up again ([#919])
+
+### `jupyter-lsp 2.0.1` (unreleased)
+
+- bug fixes:
+  - pyright schema now includes required `python.` prefixes ([#919])
+
+[#919]: https://github.com/jupyter-lsp/jupyterlab-lsp/pull/919
+
 ### `@jupyter-lsp/jupyterlab-lsp 4.0.0` (2023-03-15)
 
 Requires JupyterLab `>=3.6.0,<4.0.0a0` and Python 3.8 or newer.
@@ -9,6 +21,7 @@ Requires JupyterLab `>=3.6.0,<4.0.0a0` and Python 3.8 or newer.
   - implement settings UI using native JupyterLab 3.3 UI ([#778])
   - add option to show hover tooltip automatically ([#864], thanks @yamaton)
   - implement eliding for long paths/files in completer ([#893])
+  - allow to pass default workspace config (`serverSettings`) in specs ([#856])
 - bug fixes:
   - use correct websocket URL if configured as different from base URL ([#820], thanks @MikeSem)
   - clean up all completer styles when completer feature is disabled ([#829]).
@@ -50,6 +63,7 @@ Requires JupyterLab `>=3.6.0,<4.0.0a0` and Python 3.8 or newer.
 - performance:
   - `entry_point` discovery is deferred until server has started, improving
     `jupyter_server` startup time ([#852])
+- allow to pass default workspace config (`serverSettings`) in specs ([#856])
 
 [#724]: https://github.com/jupyter-lsp/jupyterlab-lsp/pull/724
 [#738]: https://github.com/jupyter-lsp/jupyterlab-lsp/pull/738
@@ -63,6 +77,7 @@ Requires JupyterLab `>=3.6.0,<4.0.0a0` and Python 3.8 or newer.
 [#833]: https://github.com/jupyter-lsp/jupyterlab-lsp/pull/833
 [#836]: https://github.com/jupyter-lsp/jupyterlab-lsp/pull/836
 [#852]: https://github.com/jupyter-lsp/jupyterlab-lsp/pull/852
+[#856]: https://github.com/jupyter-lsp/jupyterlab-lsp/pull/856
 [#860]: https://github.com/jupyter-lsp/jupyterlab-lsp/pull/860
 [#864]: https://github.com/jupyter-lsp/jupyterlab-lsp/pull/864
 [#867]: https://github.com/jupyter-lsp/jupyterlab-lsp/pull/867

atest/07_Configuration.robot

@@ -16,9 +16,18 @@ Python
     ...    undefined name 'foo' (pyflakes)
     ...    undefined name 'foo' (flake8)
 
-Python (programmatically)
+Python (server-side via overrides.json)
+    [Documentation]    same as "Python" but changing the defaults in server specification via `overrides.json`
+    Settings Should Change Editor Diagnostics    Python    style.py    pylsp-with-override-json
+    ...    settings=100
+    ...    before=undefined name 'foo' (pyflakes)
+    ...    after=undefined name 'foo' (flake8)
+    ...    setting_key=priority
+    ...    needs reload=${True}
+
+Python (server-side via spec)
     [Documentation]    same as "Python" but changing the defaults in server specification via `workspace_configuration`
-    Settings Should Change Editor Diagnostics    Python    style.py    pylsp-with-config-override
+    Settings Should Change Editor Diagnostics    Python    style.py    pylsp-with-override-spec
     ...    settings=100
     ...    before=undefined name 'foo' (pyflakes)
     ...    after=undefined name 'foo' (flake8)

atest/Keywords.resource

@@ -8,6 +8,7 @@ Resource    Variables.resource
 Library     ./logcheck.py
 Library     ./ports.py
 Library     ./config.py
+Library     ./paths.py
 
 
 *** Keywords ***
@@ -41,13 +42,14 @@ Create Notebok Server Config
     Set Environment Variable
     ...    name=JUPYTER_CONFIG_DIR
     ...    value=${NOTEBOOK DIR}
+    ${jupyterlab_dir} =    Get JupyterLab Path
     ${extra_node_roots} =    Create List    ${ROOT}
     ${port} =    Get Unused Port
     Set Global Variable    ${PORT}    ${port}
     Set Global Variable    ${URL}    http://localhost:${PORT}${BASE URL}
     Copy File    ${FIXTURES}${/}${JPSERVER CONF JSON}    ${conf}
     Copy File    ${FIXTURES}${/}${JPSERVER CONF PY}    ${NOTEBOOK DIR}${/}${JPSERVER CONF PY}
-    Copy File    ${FIXTURES}${/}overrides.json    ${NOTEBOOK DIR}${/}overrides.json
+    Copy File    ${FIXTURES}${/}overrides.json    ${jupyterlab_dir}${/}settings${/}overrides.json
     Update Jupyter Config    ${conf}    ServerApp
     ...    base_url=${BASE URL}
     ...    port=${PORT}

atest/fixtures/jupyter_server_config.py

@@ -7,12 +7,16 @@
 
 
 c.LanguageServerManager.language_servers.update({
-    "pylsp-with-config-override": {
+    "pylsp-with-override-spec": {
         **pylsp_base["pylsp"],
-        "display_name": "pylsp (with-config-override)",
+        "display_name": "pylsp (with override via server spec)",
         "workspace_configuration": {
           "pylsp.plugins.flake8.enabled": True,
           "pylsp.plugins.pyflakes.enabled": False
         }
+    },
+    "pylsp-with-override-json": {
+        **pylsp_base["pylsp"],
+        "display_name": "pylsp (with override via overrides.json)"
     }
 })

atest/fixtures/overrides.json

@@ -1,5 +1,15 @@
 {
-    "@jupyterlab/statusbar-extension:plugin": {
-        "visible": true
+  "@jupyterlab/statusbar-extension:plugin": {
+    "visible": true
+  },
+  "@jupyter-lsp/jupyterlab-lsp:plugin": {
+    "language_servers": {
+      "pylsp-with-override-json": {
+        "serverSettings": {
+          "pylsp.plugins.flake8.enabled": true,
+          "pylsp.plugins.pyflakes.enabled": false
+        }
+      }
     }
+  }
 }

atest/paths.py

@@ -0,0 +1,8 @@
+"""work with jupyter paths"""
+
+from jupyterlab.commands import get_app_dir
+
+
+def get_jupyterlab_path():
+    """Get JupyterLab Application Directory path"""
+    return get_app_dir()

packages/jupyterlab-lsp/src/settings.ts

@@ -241,9 +241,15 @@ export class SettingsSchemaManager {
 
     const defaults: Record<string, any> = {};
     const knownServersConfig: Record<string, any> = {};
+    // `sharedDefaults` may be empty as we do not define/receive custom
+    // per-property defaults in schema as of the day of writing.
     const sharedDefaults = getDefaults(
       schema.properties!.language_servers.properties
     );
+    const defaultsOverrides = schema.properties!.language_servers.default as
+      | Record<string, any>
+      | undefined;
+
     for (let [serverKey, serverSpec] of languageServerManager.specs.entries()) {
       if ((serverKey as string) === '') {
         this.console.warn(
@@ -324,6 +330,26 @@ export class SettingsSchemaManager {
           configSchema.properties[key].default = value;
         }
       }
+      // add server-speficic default overrides from overrides.json (and pre-defined in schema)
+      const serverDefaultsOverrides =
+        defaultsOverrides && defaultsOverrides.hasOwnProperty(serverKey)
+          ? defaultsOverrides[serverKey]
+          : {};
+      if (serverDefaultsOverrides.serverSettings) {
+        for (const [key, value] of Object.entries(
+          serverDefaultsOverrides.serverSettings
+        )) {
+          if (!configSchema.properties.hasOwnProperty(key)) {
+            this.console.warn(
+              '`overrides.json` includes an override for key not in schema',
+              key,
+              serverKey
+            );
+            continue;
+          }
+          configSchema.properties[key].default = value;
+        }
+      }
 
       const defaultMap = getDefaults(configSchema.properties);
 
@@ -332,6 +358,7 @@ export class SettingsSchemaManager {
       knownServersConfig[serverKey] = baseSchemaCopy;
       defaults[serverKey] = {
         ...sharedDefaults,
+        ...serverDefaultsOverrides,
         serverSettings: defaultMap
       };
     }