Merge pull request #485 from krassowski/various-fixes

Various fixes

Author: krassowski

CHANGELOG.md

@@ -1,5 +1,20 @@
 ## CHANGELOG
 
+### `@krassowski/jupyterlab-lsp 3.1.1` (unreleased)
+
+- bug fixes:
+
+  - diagnostics panel works after kernel restart properly ([#485])
+  - workaround was added to enable `jedi-language-server` diagnostics ([#485])
+
+### `jupyter-lsp 1.1.1` (unreleased)
+
+- bug fixes:
+
+  - `PythonModuleSpec` no longer raises exception when the server module does not exist ([#485])
+
+[#485]: https://github.com/krassowski/jupyterlab-lsp/pull/485
+
 ### `@krassowski/jupyterlab-lsp 3.1.0` (2021-01-17)
 
 - features

atest/04_Interface/DiagnosticsPanel.robot

@@ -28,6 +28,16 @@ Diagnostics Panel Works After Rename
     Wait Until Keyword Succeeds    10 x    1s    Should Have Expected Rows Count    ${EXPECTED_COUNT}
     Clean Up After Working With File    PanelRenamed.ipynb
 
+Diagnostics Panel Works After Kernel Restart
+    [Documentation]    Test for #475 bug
+    Close Diagnostics Panel
+    Lab Command    Restart Kernel…
+    Wait For Dialog
+    Accept Default Dialog Option
+    Wait Until Page Contains Element    css:.cm-lsp-diagnostic[title*="${DIAGNOSTIC}"]    timeout=20s
+    Open Diagnostics Panel
+    Wait Until Keyword Succeeds    10 x    1s    Should Have Expected Rows Count    ${EXPECTED_COUNT}
+
 Diagnostics Panel Can Be Restored
     Close Diagnostics Panel
     Open Diagnostics Panel

atest/05_Features/Completion.robot

@@ -223,7 +223,7 @@ Completes Large Namespaces
     [Setup]    Prepare File for Editing    R    completion    completion.R
     Place Cursor In File Editor At    6    7
     Wait Until Fully Initialized
-    Wait Until Keyword Succeeds    3x    2s    Trigger Completer    timeout=45s
+    Wait Until Keyword Succeeds    3x    2s    Trigger Completer    timeout=90s
     Completer Should Suggest    abs    timeout=30s
     [Teardown]    Clean Up After Working With File    completion.R
 

packages/jupyterlab-lsp/schema/syntax_highlighting.json

@@ -9,7 +9,7 @@
       "title": "Threshold of foreign code coverage for changing the mode in an editor",
       "type": "number",
       "default": 0.5,
-      "description": "If a code editor includes a code fragment in another language (for example a %%markdown magic in IPython) with appropriate foreign code extractor defined , and the extend of this code (coverage of the editor) passes the threshold, the syntax highlighting (i.e. the mode) will change to provide highlighting for the language of the foreign code."
+      "description": "If a code editor includes a code fragment in another language (for example a %%markdown magic in IPython) with appropriate foreign code extractor defined, and the extend of this code (coverage of the editor) passes the threshold, the syntax highlighting (i.e. the mode) will change to provide highlighting for the language of the foreign code."
     }
   },
   "jupyter.lab.shortcuts": []

packages/jupyterlab-lsp/src/adapters/adapter.ts

@@ -167,20 +167,8 @@ export abstract class WidgetAdapter<T extends IDocumentWidget> {
     this.widget.context.saveState.disconnect(this.on_save_state, this);
     this.connection_manager.closed.disconnect(this.on_connection_closed, this);
     this.widget.disposed.disconnect(this.dispose, this);
-    this.widget.context.model.contentChanged.disconnect(
-      this.onContentChanged,
-      this
-    );
 
-    for (let adapter of this.adapters.values()) {
-      adapter.dispose();
-    }
-    this.adapters.clear();
-
-    this.connection_manager.disconnect_document_signals(
-      this.virtual_editor.virtual_document
-    );
-    this.virtual_editor.dispose();
+    this.disconnect();
 
     // just to be sure
     this.virtual_editor = null;
@@ -224,6 +212,23 @@ export abstract class WidgetAdapter<T extends IDocumentWidget> {
 
   abstract get language_file_extension(): string;
 
+  disconnect() {
+    this.connection_manager.unregister_document(
+      this.virtual_editor.virtual_document
+    );
+    this.widget.context.model.contentChanged.disconnect(
+      this.onContentChanged,
+      this
+    );
+
+    for (let adapter of this.adapters.values()) {
+      adapter.dispose();
+    }
+    this.adapters.clear();
+
+    this.virtual_editor.dispose();
+  }
+
   // equivalent to triggering didClose and didOpen, as per syncing specification,
   // but also reloads the connection; used during file rename (or when it was moved)
   protected reload_connection() {
@@ -233,16 +238,12 @@ export abstract class WidgetAdapter<T extends IDocumentWidget> {
     }
 
     // disconnect all existing connections (and dispose adapters)
-    this.connection_manager.unregister_document(
-      this.virtual_editor.virtual_document
-    );
+    this.disconnect();
 
     // recreate virtual document using current path and language
     // as virtual editor assumes it gets the virtual document at init,
     // just dispose virtual editor (which disposes virtual document too)
     // and re-initialize both virtual editor and document
-    this.virtual_editor.dispose();
-
     this.init_virtual();
 
     // reconnect

packages/jupyterlab-lsp/src/components/statusbar.tsx

@@ -489,7 +489,7 @@ export namespace LSPStatus {
           server => server.spec.languages.indexOf(language) !== -1
         );
         if (servers.length > 1) {
-          console.warn('More than one server per language for' + language);
+          console.warn('More than one server per language for', language);
         }
         if (servers.length === 0) {
           continue;

packages/jupyterlab-lsp/src/connection_manager.ts

@@ -306,7 +306,7 @@ export class DocumentConnectionManager {
         );
       } catch {
         this.console.warn(
-          `LSP: Connection to ${virtual_document.uri} timed out after ${firstTimeoutSeconds} seconds, will continue retrying for another ${secondTimeoutMinutes} minutes`
+          `Connection to ${virtual_document.uri} timed out after ${firstTimeoutSeconds} seconds, will continue retrying for another ${secondTimeoutMinutes} minutes`
         );
         try {
           await until_ready(
@@ -316,7 +316,7 @@ export class DocumentConnectionManager {
           );
         } catch {
           this.console.warn(
-            `LSP: Connection to ${virtual_document.uri} timed out again after ${secondTimeoutMinutes} minutes, giving up`
+            `Connection to ${virtual_document.uri} timed out again after ${secondTimeoutMinutes} minutes, giving up`
           );
           return;
         }

packages/jupyterlab-lsp/src/features/diagnostics/diagnostics.ts

@@ -275,11 +275,10 @@ export class DiagnosticsCM extends CodeMirrorIntegration {
     this.connection_handlers.set('diagnostic', this.handleDiagnostic);
     this.wrapper_handlers.set('focusin', this.switchDiagnosticsPanelSource);
     this.unique_editor_ids = new DefaultMap(() => this.unique_editor_ids.size);
-    if (!diagnostics_databases.has(this.virtual_editor)) {
-      diagnostics_databases.set(this.virtual_editor, new DiagnosticsDatabase());
-    }
-    this.diagnostics_db = diagnostics_databases.get(this.virtual_editor);
     this.settings.changed.connect(this.refreshDiagnostics, this);
+    this.adapter.adapterConnected.connect(() =>
+      this.switchDiagnosticsPanelSource()
+    );
     super.register();
   }
 
@@ -294,11 +293,18 @@ export class DiagnosticsCM extends CodeMirrorIntegration {
    *
    * Maps virtual_document.uri to IEditorDiagnostic[].
    */
-  public diagnostics_db: DiagnosticsDatabase;
+  public get diagnostics_db(): DiagnosticsDatabase {
+    // Note that virtual_editor can change at runtime (kernel restart)
+    if (!diagnostics_databases.has(this.virtual_editor)) {
+      diagnostics_databases.set(this.virtual_editor, new DiagnosticsDatabase());
+    }
+    return diagnostics_databases.get(this.virtual_editor);
+  }
 
   switchDiagnosticsPanelSource = () => {
     if (
-      diagnostics_panel.content.model.virtual_editor === this.virtual_editor
+      diagnostics_panel.content.model.virtual_editor === this.virtual_editor &&
+      diagnostics_panel.content.model.diagnostics == this.diagnostics_db
     ) {
       return;
     }
@@ -378,6 +384,12 @@ export class DiagnosticsCM extends CodeMirrorIntegration {
       let code = diagnostic.code;
       if (
         typeof code !== 'undefined' &&
+        // pygls servers return code null if value is missing (rather than undefined)
+        // which is a departure from the LSP specs: https://microsoft.github.io/language-server-protocol/specification#diagnostic
+        // there is an open issue: https://github.com/openlawlibrary/pygls/issues/124
+        // and PR: https://github.com/openlawlibrary/pygls/pull/132
+        // this also affects hover tooltips.
+        code !== null &&
         ignoredDiagnosticsCodes.has(code.toString())
       ) {
         return false;
@@ -588,7 +600,9 @@ export class DiagnosticsCM extends CodeMirrorIntegration {
   };
 
   public refreshDiagnostics() {
-    this.setDiagnostics(this.last_response);
+    if (this.last_response) {
+      this.setDiagnostics(this.last_response);
+    }
     diagnostics_panel.update();
   }
 

python_packages/jupyter_lsp/jupyter_lsp/specs/utils.py

@@ -93,6 +93,9 @@ class PythonModuleSpec(SpecBase):
     def __call__(self, mgr: LanguageServerManagerAPI) -> KeyedLanguageServerSpecs:
         spec = __import__("importlib").util.find_spec(self.python_module)
 
+        if not spec:
+            return {}
+
         if not spec.origin:  # pragma: no cover
             return {}
 

python_packages/jupyter_lsp/jupyter_lsp/tests/test_detect.py

@@ -1,6 +1,7 @@
 import shutil
 
 from jupyter_lsp.specs.r_languageserver import RLanguageServer
+from jupyter_lsp.specs.utils import PythonModuleSpec
 
 
 def test_no_detect(manager):
@@ -28,3 +29,13 @@ class NonInstalledRServer(RLanguageServer):
 
     non_installed_server = NonInstalledRServer()
     assert non_installed_server.is_installed(cmd=existing_runner) is False
+
+
+def test_missing_python_module_spec():
+    """Prevent failure in module detection raising error"""
+
+    class NonInstalledPythonServer(PythonModuleSpec):
+        python_module = "not_installed_python_module"
+
+    not_installed_server = NonInstalledPythonServer()
+    assert not_installed_server(mgr=None) == {}