Merge pull request #165 from bollwyvl/add-one-socket-per-lang

Refactor LSPConnection, ConnectionManager

Author: krassowski

.gitignore

@@ -114,3 +114,4 @@ junit.xml
 coverage/
 .vscode/
 _schema.d.ts
+.virtual_documents/

CHANGELOG.md

@@ -1,5 +1,27 @@
 # CHANGELOG
 
+## `@krassowski/jupyterlab-lsp 0.8.0` (unreleased)
+
+- features
+
+  - opens a maximum of one WebSocket per language server ([#165][])
+  - lazy-loads language server protocol machinery ([#165][])
+  - waits much longer for slow-starting language servers ([#165][])
+  - cleans up documents, handlers, events, and signals more aggressively ([#165][])
+  - ignores malformed diagnostic ranges, enabling markdown support ([#165][])
+  - passes tests on Python 3.8 on Windows ([#165][])
+
+## `lsp-ws-connection 0.4.0` (unreleased)
+
+- breaking changes
+
+  - no longer assumes one document per connection ([#165][])
+  - requires documents be opened explicitly ([#165][])
+  - use of the `eventEmitter` pattern mostly deprecated in favor of `Promise`s
+    ([#165][])
+
+[#165]: https://github.com/krassowski/jupyterlab-lsp/pull/165
+
 ## `jupyter-lsp 0.7.0`
 
 - bugfixes

atest/01_Editor.robot

@@ -11,9 +11,7 @@ ${CM CURSORS}     css:.CodeMirror-cursors:not([style='visibility: hidden'])
 
 *** Test Cases ***
 Bash
-    [Documentation]    TODO: figure out why the first server is extra flaky
-    Wait Until Keyword Succeeds    6x    5s    Editor Shows Features for Language    Bash    example.sh    Diagnostics=Failed to parse expression
-    ...    Jump to Definition=fib
+    Editor Shows Features for Language    Bash    example.sh    Diagnostics=Failed to parse expression    Jump to Definition=fib
 
 CSS
     ${def} =    Set Variable    xpath:(//span[contains(@class, 'cm-variable-2')][contains(text(), '--some-var')])[last()]
@@ -67,7 +65,7 @@ Editor Shows Features for Language
     Set Tags    language:${Language.lower()}
     Set Screenshot Directory    ${OUTPUT DIR}${/}screenshots${/}editor${/}${Language.lower()}
     Copy File    examples${/}${file}    ${OUTPUT DIR}${/}home${/}${file}
-    Lab Command    Close All Tabs
+    Try to Close All Tabs
     Open ${file} in ${MENU EDITOR}
     Capture Page Screenshot    00-opened.png
     FOR    ${f}    IN    @{features}

atest/03_Notebook.robot

@@ -1,10 +1,10 @@
 *** Settings ***
 Suite Setup       Setup Suite For Screenshots    notebook
+Test Setup        Try to Close All Tabs
 Resource          Keywords.robot
 
 *** Test Cases ***
 Python
-    [Setup]    Gently Reset Workspace
     Setup Notebook    Python    Python.ipynb
     Capture Page Screenshot    01-python.png
     ${diagnostic} =    Set Variable    W291 trailing whitespace (pycodestyle)
@@ -13,11 +13,11 @@ Python
     Clean Up After Working With File    Python.ipynb
 
 Foregin Extractors
-    [Setup]    Gently Reset Workspace
     Setup Notebook    Python    Foreign extractors.ipynb
-    @{diagnostics} =    Create List    Failed to parse expression    undefined name 'valid' (pyflakes)    Trailing whitespace is superfluous. (lintr)
+    # if mypy and pyflakes will fight over `(N|n)ame 'valid'`, just hope for the best
+    @{diagnostics} =    Create List    Failed to parse expression    ame 'valid'    Trailing whitespace is superfluous. (lintr)
     FOR    ${diagnostic}    IN    @{diagnostics}
-        Wait Until Page Contains Element    css:.cm-lsp-diagnostic[title="${diagnostic}"]    timeout=35s
+        Wait Until Page Contains Element    css:.cm-lsp-diagnostic[title*\="${diagnostic}"]    timeout=35s
         Capture Page Screenshot    0x-${diagnostic}.png
     END
     Clean Up After Working With File    Foreign Extractors.ipynb

atest/05_Features/Completion.robot

@@ -1,5 +1,7 @@
 *** Settings ***
 Suite Setup       Setup Suite For Screenshots    completion
+Test Setup        Setup Notebook    Python    Completion.ipynb
+Test Teardown     Clean Up After Working With File    Completion.ipynb
 Force Tags        feature:completion
 Resource          ../Keywords.robot
 
@@ -9,9 +11,6 @@ ${COMPLETER_BOX}    css:.jp-Completer.jp-HoverBox
 *** Test Cases ***
 Works With Kernel Running
     [Documentation]    The suggestions from kernel and LSP should get integrated.
-    [Tags]    language:python
-    Setup Notebook    Python    Completion.ipynb
-    Wait Until Fully Initialized
     Enter Cell Editor    1    line=2
     Capture Page Screenshot    01-entered-cell.png
     Trigger Completer
@@ -26,12 +25,8 @@ Works With Kernel Running
     Capture Page Screenshot    03-completion-confirmed.png
     ${content} =    Get Cell Editor Content    1
     Should Contain    ${content}    TabError
-    [Teardown]    Clean Up After Working With File    Completion.ipynb
 
 Works When Kernel Is Shut Down
-    [Tags]    language:python
-    Setup Notebook    Python    Completion.ipynb
-    Wait Until Fully Initialized
     Lab Command    Shut Down All Kernels…
     Capture Page Screenshot    01-shutting-kernels.png
     Accept Default Dialog Option
@@ -43,82 +38,58 @@ Works When Kernel Is Shut Down
     Completer Should Suggest    test
     # this comes from kernel:
     Completer Should Not Suggest    %%timeit
-    [Teardown]    Clean Up After Working With File    Completion.ipynb
 
 Autocompletes If Only One Option
-    [Tags]    language:python
-    Setup Notebook    Python    Completion.ipynb
     Enter Cell Editor    3    line=1
     Press Keys    None    cle
     Wait Until Fully Initialized
     Press Keys    None    TAB
     Wait Until Keyword Succeeds    40x    0.5s    Cell Editor Should Equal    3    list.clear
-    [Teardown]    Clean Up After Working With File    Completion.ipynb
 
 User Can Select Lowercase After Starting Uppercase
-    [Tags]    language:python
-    Setup Notebook    Python    Completion.ipynb
     # `from time import Tim<tab>` → `from time import time`
-    Wait Until Fully Initialized
     Enter Cell Editor    5    line=1
     Trigger Completer
     Completer Should Suggest    time
     Press Keys    None    ENTER
     Wait Until Keyword Succeeds    40x    0.5s    Cell Editor Should Equal    5    from time import time
-    [Teardown]    Clean Up After Working With File    Completion.ipynb
 
 Mid Token Completions Do Not Overwrite
-    [Tags]    language:python
-    Setup Notebook    Python    Completion.ipynb
     # `disp<tab>data` → `display_table<cursor>data`
     Place Cursor In Cell Editor At    9    line=1    character=4
     Capture Page Screenshot    01-cursor-placed.png
-    Wait Until Fully Initialized
     Press Keys    None    TAB
     Capture Page Screenshot    02-completed.png
     Wait Until Keyword Succeeds    40x    0.5s    Cell Editor Should Equal    9    display_tabledata
     # `disp<tab>lay` → `display_table<cursor>`
     Place Cursor In Cell Editor At    11    line=1    character=4
     Press Keys    None    TAB
     Wait Until Keyword Succeeds    40x    0.5s    Cell Editor Should Equal    11    display_table
-    [Teardown]    Clean Up After Working With File    Completion.ipynb
 
 Completion Works For Tokens Separated By Space
-    [Tags]    language:python
-    Setup Notebook    Python    Completion.ipynb
     # `from statistics <tab>` → `from statistics import<cursor>`
     Enter Cell Editor    13    line=1
-    Wait Until Fully Initialized
     Trigger Completer
     Completer Should Suggest    import
     Press Keys    None    ENTER
     Wait Until Keyword Succeeds    40x    0.5s    Cell Editor Should Equal    13    from statistics import
-    [Teardown]    Clean Up After Working With File    Completion.ipynb
 
 Kernel And LSP Completions Merge Prefix Conflicts Are Resolved
     [Documentation]    Reconciliate Python kernel returning prefixed completions and LSP (pyls) not-prefixed ones
-    [Tags]    language:python
     # For more details see: https://github.com/krassowski/jupyterlab-lsp/issues/30#issuecomment-576003987
     # `import os.pat<tab>` → `import os.pathsep`
-    Setup Notebook    Python    Completion.ipynb
     Enter Cell Editor    15    line=1
-    Wait Until Fully Initialized
     Trigger Completer
     Completer Should Suggest    pathsep
     Select Completer Suggestion    pathsep
     Wait Until Keyword Succeeds    40x    0.5s    Cell Editor Should Equal    15    import os.pathsep
-    [Teardown]    Clean Up After Working With File    Completion.ipynb
 
 Triggers Completer On Dot
-    [Tags]    language:python
-    Setup Notebook    Python    Completion.ipynb
     Enter Cell Editor    2    line=1
-    Wait Until Fully Initialized
     Press Keys    None    .
     Wait Until Keyword Succeeds    10x    0.5s    Cell Editor Should Equal    2    list.
     Wait Until Page Contains Element    ${COMPLETER_BOX}    timeout=35s
     Completer Should Suggest    append
-    [Teardown]    Clean Up After Working With File    Completion.ipynb
 
 *** Keywords ***
 Get Cell Editor Content
@@ -139,11 +110,12 @@ Select Completer Suggestion
 
 Completer Should Suggest
     [Arguments]    ${text}
-    Page Should Contain Element    ${COMPLETER_BOX} .jp-Completer-item[data-value="${text}"]
+    Wait Until Page Contains Element    ${COMPLETER_BOX} .jp-Completer-item[data-value="${text}"]
+    Capture Page Screenshot    ${text.replace(' ', '_')}.png
 
 Completer Should Not Suggest
     [Arguments]    ${text}
-    Page Should Not Contain Element    ${COMPLETER_BOX} .jp-Completer-item[data-value="${text}"]
+    Wait Until Page Does Not Contain Element    ${COMPLETER_BOX} .jp-Completer-item[data-value="${text}"]
 
 Trigger Completer
     Press Keys    None    TAB

atest/05_Features/Signature.robot

@@ -9,7 +9,6 @@ ${SIGNATURE_BOX}    css:.lsp-signature-help
 *** Test Cases ***
 Triggers Signature Help After A Keystroke
     Setup Notebook    Python    Signature.ipynb
-    Wait Until Fully Initialized
     Enter Cell Editor    1    line=6
     Capture Page Screenshot    01-entered-cell.png
     Press Keys    None    (

atest/Keywords.robot

@@ -66,9 +66,17 @@ Open JupyterLab
 Close JupyterLab
     Close All Browsers
 
-Reset Application State
+Close All Tabs
+    Accept Default Dialog Option
     Lab Command    Close All Tabs
     Accept Default Dialog Option
+
+Try to Close All Tabs
+    Wait Until Keyword Succeeds    5x    50ms    Close All Tabs
+
+Reset Application State
+    Try to Close All Tabs
+    Accept Default Dialog Option
     Ensure All Kernels Are Shut Down
     Lab Command    Reset Application State
     Wait For Splash
@@ -172,11 +180,13 @@ Clean Up After Working With File
 Setup Notebook
     [Arguments]    ${Language}    ${file}
     Set Tags    language:${Language.lower()}
-    Set Screenshot Directory    ${OUTPUT DIR}${/}screenshots${/}notebook${/}${file.lower()}
+    Set Screenshot Directory    ${OUTPUT DIR}${/}screenshots${/}notebook${/}${TEST NAME.replace(' ', '_')}
     Copy File    examples${/}${file}    ${OUTPUT DIR}${/}home${/}${file}
-    Lab Command    Close All Tabs
+    Try to Close All Tabs
     Open ${file} in ${MENU NOTEBOOK}
     Capture Page Screenshot    00-opened.png
+    Wait Until Fully Initialized
+    Capture Page Screenshot    01-initialized.png
 
 Open Diagnostics Panel
     Lab Command    Show Diagnostics Panel
@@ -194,7 +204,7 @@ Wait For Dialog
     Wait Until Page Contains Element    ${DIALOG WINDOW}    timeout=180s
 
 Gently Reset Workspace
-    Lab Command    Close All Tabs
+    Try to Close All Tabs
 
 Enter Cell Editor
     [Arguments]    ${cell_nr}    ${line}=1
@@ -207,7 +217,7 @@ Place Cursor In Cell Editor At
     Execute JavaScript    return document.querySelector('.jp-Cell:nth-child(${cell_nr}) .CodeMirror').CodeMirror.setCursor({line: ${line} - 1, ch: ${character}})
 
 Wait Until Fully Initialized
-    Wait Until Element Contains    ${STATUSBAR}    Fully initialized    timeout=35s
+    Wait Until Element Contains    ${STATUSBAR}    Fully initialized    timeout=60s
 
 Open Context Menu Over
     [Arguments]    ${sel}

azure-pipelines.yml

@@ -22,6 +22,9 @@ variables:
   THIRD_PARTY_LABEXTENSIONS: >-
     @krassowski/jupyterlab_go_to_definition
 
+  LINKED_EXTENSIONS: >-
+    packages/lsp-ws-connection
+
 jobs:
   - template: ci/job.lint.yml
   - template: ci/job.test.yml

ci/job.test.yml

@@ -20,6 +20,7 @@ parameters:
       spec: '>=3.8,<3.9.0a0'
       lab: '>=1.2.4,<1.3.0a0'
   env_update: conda env update -n jupyterlab-lsp --file env-test.yml --quiet
+  lab_link: jupyter labextension link --no-build $(LINKED_EXTENSIONS)
   lab_ext: jupyter labextension install --no-build $(THIRD_PARTY_LABEXTENSIONS) $(FIRST_PARTY_LABEXTENSIONS)
 
 jobs:
@@ -79,6 +80,9 @@ jobs:
               - script: ${{ platform.activate }} jupyterlab-lsp && python scripts/utest.py --test-run-title="Pytest ${{ platform.name }}${{ python.name }}"
                 displayName: run python tests
 
+              - script: ${{ platform.activate }} jupyterlab-lsp && ${{ parameters.lab_link }} || ${{ parameters.lab_link }} || ${{ parameters.lab_link }}
+                displayName: install support packages
+
               - script: ${{ platform.activate }} jupyterlab-lsp && ${{ parameters.lab_ext }} || ${{ parameters.lab_ext }} || ${{ parameters.lab_ext }}
                 displayName: install labextensions
 

packages/jupyterlab-lsp/src/adapters/codemirror/cm_adapter.ts

@@ -1,25 +1,24 @@
 import * as CodeMirror from 'codemirror';
 import { until_ready } from '../../utils';
-import { CodeMirrorHandler, VirtualEditor } from '../../virtual/editor';
+import { VirtualEditor } from '../../virtual/editor';
 import { VirtualDocument } from '../../virtual/document';
 import { IRootPosition } from '../../positioning';
 import { ILSPFeature } from './feature';
 import { IJupyterLabComponentsManager } from '../jupyterlab/jl_adapter';
 
 export class CodeMirrorAdapter {
   features: Map<string, ILSPFeature>;
+  isDisposed = false;
 
   private last_change: CodeMirror.EditorChange;
-  private doc_change_handler: CodeMirrorHandler;
 
   constructor(
     protected editor: VirtualEditor,
     protected virtual_document: VirtualDocument,
     protected jupyterlab_components: IJupyterLabComponentsManager,
     features = new Array<ILSPFeature>()
   ) {
-    this.doc_change_handler = this.saveChange.bind(this);
-    this.editor.on('change', this.doc_change_handler);
+    this.editor.on('change', this.saveChange);
 
     this.features = new Map();
 
@@ -38,19 +37,28 @@ export class CodeMirrorAdapter {
 
   public async updateAfterChange() {
     this.jupyterlab_components.remove_tooltip();
-    await until_ready(() => this.last_change != null, 30, 22).catch(() => {
-      this.invalidateLastChange();
-      throw Error(
+
+    try {
+      await until_ready(() => this.last_change != null, 30, 22);
+    } catch (err) {
+      console.log(
         'No change obtained from CodeMirror editor within the expected time of 0.66s'
       );
-    });
+      return;
+    }
+
     let change: CodeMirror.EditorChange = this.last_change;
 
+    let root_position: IRootPosition;
+
     try {
-      const root_position = this.editor
-        .getDoc()
-        .getCursor('end') as IRootPosition;
+      root_position = this.editor.getDoc().getCursor('end') as IRootPosition;
+    } catch (err) {
+      console.log('LSP: Root positon not found');
+      return;
+    }
 
+    try {
       let document = this.editor.document_at_root_position(root_position);
 
       if (this.virtual_document !== document) {
@@ -77,14 +85,27 @@ export class CodeMirrorAdapter {
     this.last_change = null;
   }
 
-  public saveChange(doc: CodeMirror.Doc, change: CodeMirror.EditorChange) {
+  public saveChange = (
+    doc: CodeMirror.Doc,
+    change: CodeMirror.EditorChange
+  ) => {
     this.last_change = change;
-  }
+  };
 
-  public remove() {
+  public dispose() {
+    if (this.isDisposed) {
+      return;
+    }
     for (let feature of this.features.values()) {
       feature.remove();
     }
-    this.editor.off('change', this.doc_change_handler);
+    this.features.clear();
+    this.editor.off('change', this.saveChange);
+
+    // just to be sure
+    this.editor = null;
+
+    // actually disposed
+    this.isDisposed = true;
   }
 }