fix ws signature failed

CHANGELOG.md

@@ -2,9 +2,12 @@
 
 ## `lsp-ws-connection 0.3.1`
 
-- added sendSaved() method (textDocument/didSave) (
+- added `sendSaved()` method (textDocument/didSave) (
   [#147](https://github.com/krassowski/jupyterlab-lsp/pull/147)
   )
+- fixed `getSignatureHelp()` off-by-one error (
+  [#140](https://github.com/krassowski/jupyterlab-lsp/pull/140)
+  )
 
 ## `@krassowski/jupyterlab-lsp 0.7.0-rc.0`
 
@@ -38,6 +41,8 @@
     exctracted R documents (improved foreign code extractor) (
     [#148](https://github.com/krassowski/jupyterlab-lsp/pull/148)
     )
+  - console logs can now easily be redirected to a floating console
+    windows for debugging of the browser tests (see CONTRIBUTING.md)
 
 - bugfixes
   - diagnostics in foreign documents are now correctly updated (
@@ -62,6 +67,9 @@
     [#95](https://github.com/krassowski/jupyterlab-lsp/pull/95),
     [#147](https://github.com/krassowski/jupyterlab-lsp/pull/147),
     )
+  - signature feature is now correctly working in notebooks (
+    [#140](https://github.com/krassowski/jupyterlab-lsp/pull/140)
+    )
 
 ## `lsp-ws-connection 0.3.0`
 

CONTRIBUTING.md

@@ -185,6 +185,10 @@ atest/
 
   and re-run the tests.
 
+- To display logs on the screenshots, write logs with `virtual_editor.console.log` method,
+  and change `create_console('browser')` to `create_console('floating')` in `VirtualEditor`
+  constructor (please feel free to add a config option for this).
+
 ### Formatting
 
 Minimal code style is enforced with `pytest-flake8` during unit testing. If installed,

atest/05_Features/Completion.robot

@@ -3,14 +3,13 @@ Suite Setup       Setup Suite For Screenshots    completion
 Resource          ../Keywords.robot
 
 *** Variables ***
-${STATUSBAR}      css:div.lsp-statusbar-item
 ${COMPLETER_BOX}    css:.jp-Completer.jp-HoverBox
 
 *** Test Cases ***
 Works With Kernel Running
     [Documentation]    The suggestions from kernel and LSP should get integrated.
     Setup Notebook    Python    Completion.ipynb
-    Wait Until Element Contains    ${STATUSBAR}    Fully initialized    timeout=20s
+    Wait Until Fully Initialized
     Enter Cell Editor    1    line=2
     Capture Page Screenshot    01-entered-cell.png
     Trigger Completer
@@ -29,6 +28,7 @@ Works With Kernel Running
 
 Works When Kernel Is Shut Down
     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
@@ -46,6 +46,7 @@ Autocompletes If Only One Option
     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
@@ -54,17 +55,22 @@ User Can Select Lowercase After Starting Uppercase
     Setup Notebook    Python    Completion.ipynb
     Enter Cell Editor    4    line=1
     Trigger Completer
-    Completer Should Suggest  time
+    Completer Should Suggest    time
     Press Keys    None    ENTER
     Wait Until Keyword Succeeds    40x    0.5s    Cell Editor Should Equal    4    from time import time
     [Teardown]    Clean Up After Working With File    Completion.ipynb
 
-*** Keywords ***
-Enter Cell Editor
-    [Arguments]    ${cell_nr}    ${line}=1
-    Click Element    css:.jp-CodeCell:nth-child(${cell_nr}) .CodeMirror-line:nth-child(${line})
-    Wait Until Page Contains Element    css:.jp-CodeCell:nth-child(${cell_nr}) .CodeMirror-focused
+Triggers Completer On Dot
+    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
     [Arguments]    ${cell_nr}
     ${content}    Execute JavaScript    return document.querySelector('.jp-CodeCell:nth-child(${cell_nr}) .CodeMirror').CodeMirror.getValue()
@@ -85,4 +91,4 @@ Completer Should Not Suggest
 
 Trigger Completer
     Press Keys    None    TAB
-    Wait Until Page Contains Element    ${COMPLETER_BOX}    timeout=15s
+    Wait Until Page Contains Element    ${COMPLETER_BOX}    timeout=35s

atest/05_Features/Signature.robot

@@ -0,0 +1,18 @@
+*** Settings ***
+Suite Setup       Setup Suite For Screenshots    completion
+Resource          ../Keywords.robot
+
+*** Variables ***
+${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    (
+    Capture Page Screenshot    02-signature-shown.png
+    Wait Until Keyword Succeeds    20x    0.5s    Page Should Contain Element    ${SIGNATURE_BOX}
+    Element Should Contain    ${SIGNATURE_BOX}    Important docstring of abc()
+    [Teardown]    Clean Up After Working With File    Signature.ipynb

atest/Keywords.robot

@@ -78,7 +78,9 @@ Ensure All Kernels Are Shut Down
     Enter Command Name    Shut Down All Kernels
     ${els} =    Get WebElements    ${CMD PALETTE ITEM ACTIVE}
     Run Keyword If    ${els.__len__()}    Click Element    ${CMD PALETTE ITEM ACTIVE}
-    Run Keyword If    ${els.__len__()}    Click Element    css:.jp-mod-accept.jp-mod-warn
+    ${accept} =    Set Variable    css:.jp-mod-accept.jp-mod-warn
+    Run Keyword If    ${els.__len__()}    Wait Until Page Contains Element    ${accept}
+    Run Keyword If    ${els.__len__()}    Click Element    ${accept}
 
 Open Command Palette
     Press Keys    id:main    ${ACCEL}+SHIFT+c
@@ -189,3 +191,11 @@ Wait For Dialog
 
 Gently Reset Workspace
     Lab Command    Close All Tabs
+
+Enter Cell Editor
+    [Arguments]    ${cell_nr}    ${line}=1
+    Click Element    css:.jp-CodeCell:nth-child(${cell_nr}) .CodeMirror-line:nth-child(${line})
+    Wait Until Page Contains Element    css:.jp-CodeCell:nth-child(${cell_nr}) .CodeMirror-focused
+
+Wait Until Fully Initialized
+    Wait Until Element Contains    ${STATUSBAR}    Fully initialized    timeout=35s

atest/Variables.robot

@@ -21,3 +21,4 @@ ${DIAGNOSTIC PANEL CLOSE}    css:.p-DockPanel-tabBar .p-TabBar-tab[data-id="lsp-
 ${DIALOG WINDOW}    css:.jp-Dialog
 ${DIALOG INPUT}    css:.jp-Input-Dialog input
 ${DIALOG ACCEPT}    css:button.jp-Dialog-button.jp-mod-accept
+${STATUSBAR}      css:div.lsp-statusbar-item

atest/examples/Signature.ipynb

@@ -0,0 +1,53 @@
+{
+ "cells": [
+  {
+   "cell_type": "code",
+   "execution_count": null,
+   "metadata": {
+    "collapsed": false,
+    "jupyter": {
+     "outputs_hidden": false
+    }
+   },
+   "outputs": [],
+   "source": [
+    "def abc(x=1):\n",
+    "    \"\"\"Important docstring of abc()\"\"\"\n",
+    "    pass\n",
+    "\n",
+    "\n",
+    "abc"
+   ]
+  },
+  {
+   "cell_type": "code",
+   "execution_count": null,
+   "metadata": {},
+   "outputs": [],
+   "source": [
+    "list"
+   ]
+  }
+ ],
+ "metadata": {
+  "kernelspec": {
+   "display_name": "Python 3",
+   "language": "python",
+   "name": "python3"
+  },
+  "language_info": {
+   "codemirror_mode": {
+    "name": "ipython",
+    "version": 3
+   },
+   "file_extension": ".py",
+   "mimetype": "text/x-python",
+   "name": "python",
+   "nbconvert_exporter": "python",
+   "pygments_lexer": "ipython3",
+   "version": "3.8.0"
+  }
+ },
+ "nbformat": 4,
+ "nbformat_minor": 4
+}

packages/jupyterlab-lsp/package.json

@@ -39,7 +39,7 @@
   },
   "dependencies": {
     "@krassowski/jupyterlab_go_to_definition": "^0.7.1",
-    "lsp-ws-connection": "*"
+    "lsp-ws-connection": "0.3.1"
   },
   "devDependencies": {
     "@babel/preset-env": "^7.4.3",

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

@@ -26,7 +26,11 @@ export class CodeMirrorAdapter {
     for (let feature of features) {
       feature.register();
       if (!feature.is_registered) {
-        console.warn('The feature ', feature, 'was not registered properly');
+        this.editor.console.warn(
+          'The feature ',
+          feature,
+          'was not registered properly'
+        );
       }
       this.features.set(feature.name, feature);
     }
@@ -63,8 +67,8 @@ export class CodeMirrorAdapter {
       }
       return true;
     } catch (e) {
-      console.log('updateAfterChange failure');
-      console.error(e);
+      this.editor.console.log('updateAfterChange failure');
+      this.editor.console.error(e);
     }
     this.invalidateLastChange();
   }

packages/jupyterlab-lsp/src/adapters/codemirror/features/completion.ts

@@ -25,6 +25,10 @@ export class Completion extends CodeMirrorLSPFeature {
     //  requires an up-to-date virtual document on the LSP side, so we need to wait for sync.
     let last_character = this.extract_last_character(change);
     if (this.completionCharacters.indexOf(last_character) > -1) {
+      this.virtual_editor.console.log(
+        'Will invoke completer after',
+        last_character
+      );
       this.jupyterlab_components.invoke_completer(
         CompletionTriggerKind.TriggerCharacter
       );

packages/jupyterlab-lsp/src/adapters/codemirror/features/signature.ts

@@ -76,6 +76,7 @@ export class Signature extends CodeMirrorLSPFeature {
   private handleSignature(response: lsProtocol.SignatureHelp) {
     this.jupyterlab_components.remove_tooltip();
 
+    this.virtual_editor.console.log('Signature received', response);
     if (!this.signature_character || !response || !response.signatures.length) {
       return;
     }
@@ -88,11 +89,19 @@ export class Signature extends CodeMirrorLSPFeature {
     let language = this.get_language_at(editor_position, cm_editor);
     let markup = this.get_markup_for_signature_help(response, language);
 
-    this.jupyterlab_components.create_tooltip(
+    this.virtual_editor.console.log(
+      'Signature will be shown',
+      language,
+      markup,
+      root_position
+    );
+
+    let tooltip = this.jupyterlab_components.create_tooltip(
       markup,
       cm_editor,
       editor_position
     );
+    tooltip.addClass('lsp-signature-help');
   }
 
   get signatureCharacters() {
@@ -115,6 +124,10 @@ export class Signature extends CodeMirrorLSPFeature {
       let virtual_position = this.virtual_editor.root_position_to_virtual_position(
         root_position
       );
+      this.virtual_editor.console.log(
+        'Signature will be requested for',
+        virtual_position
+      );
       this.connection.getSignatureHelp(virtual_position);
     }
   }

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

@@ -224,7 +224,7 @@ export abstract class JupyterLabWidgetAdapter
 
     await this.virtual_editor.update_documents().then(() => {
       // refresh the document on the LSP server
-      this.document_changed(virtual_document, true);
+      this.document_changed(virtual_document, virtual_document, true);
       console.log(
         'LSP: virtual document(s) for',
         this.document_path,
@@ -243,7 +243,11 @@ export abstract class JupyterLabWidgetAdapter
     });
   }
 
-  document_changed(virtual_document: VirtualDocument, is_init = false) {
+  document_changed(
+    virtual_document: VirtualDocument,
+    document: VirtualDocument,
+    is_init = false
+  ) {
     // TODO only send the difference, using connection.sendSelectiveChange()
     let connection = this.connection_manager.connections.get(
       virtual_document.id_path

packages/jupyterlab-lsp/src/virtual/console.ts

@@ -0,0 +1,62 @@
+import '../../style/console.css';
+
+export abstract class EditorLogConsole {
+  abstract log(...args: any[]): void;
+  abstract warn(...args: any[]): void;
+  abstract error(...args: any[]): void;
+}
+
+export class FloatingConsole extends EditorLogConsole {
+  // likely to be replaced by JupyterLab console: https://github.com/jupyterlab/jupyterlab/pull/6833#issuecomment-543016425
+  element: HTMLElement;
+
+  constructor() {
+    super();
+    this.element = document.createElement('ul');
+    this.element.className = 'lsp-floating-console';
+    document.body.appendChild(this.element);
+  }
+
+  append(text: string, kind = 'log') {
+    let entry = document.createElement('li');
+    entry.innerHTML = '<span class="lsp-kind">' + kind + '</span>' + text;
+    this.element.appendChild(entry);
+    this.element.scrollTop = this.element.scrollHeight;
+  }
+
+  private to_string(args: any[]) {
+    return args
+      .map(arg => '<span class="lsp-code">' + JSON.stringify(arg) + '</span>')
+      .join(', ');
+  }
+
+  log(...args: any[]) {
+    this.append(this.to_string(args), 'log');
+  }
+  warn(...args: any[]) {
+    this.append(this.to_string(args), 'warn');
+  }
+  error(...args: any[]) {
+    this.append(this.to_string(args), 'error');
+  }
+}
+
+export class BrowserConsole extends EditorLogConsole {
+  log(...args: any[]) {
+    console.log('LSP: ', ...args);
+  }
+  warn(...args: any[]) {
+    console.warn('LSP: ', ...args);
+  }
+  error(...args: any[]) {
+    console.error('LSP: ', ...args);
+  }
+}
+
+export function create_console(kind: 'browser' | 'floating'): EditorLogConsole {
+  if (kind === 'browser') {
+    return new BrowserConsole();
+  } else {
+    return new FloatingConsole();
+  }
+}