Merge pull request #671 from jupyter-lsp/improve-signature

Make signature persist, highlight active parameter, show on top
jupyter-lsp · Oct 10, 2021 · 8cfd4f4 · 8cfd4f4
2 parents 0cff6b9 + 18b01a8
commit 8cfd4f4
Show file tree

Hide file tree

Showing 24 changed files with 744 additions and 266 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,18 @@
 ## Changelog
 
+### `@krassowski/jupyterlab-lsp 3.9.0` (unreleased)
+
+- features:
+  - signature help box will now persist while typing the arguments of a function ([#671])
+  - the currently active argument will be highlighted in the signature help box
+  - if the documentation exceeds a user-configurable number of lines the signature
+    help box will only display the first line of the documentation and the following
+    lines will be collapsed into an expandable details section.
+  - the signature box is now displayed above the current line
+  - the signature box takes up less space
+
+[#671]: https://github.com/krassowski/jupyterlab-lsp/pull/671
+
 ### `@krassowski/jupyterlab-lsp 3.8.1` (2021-08-02)
 
 - bug fixes:

diff --git a/atest/05_Features/Signature.robot b/atest/05_Features/Signature.robot
@@ -2,26 +2,51 @@
 Suite Setup       Setup Suite For Screenshots    signature
 Force Tags        feature:signature
 Resource          ../Keywords.robot
+Test Setup        Setup Notebook    Python    Signature.ipynb
+Test Teardown     Clean Up After Working With File    Signature.ipynb
 
 *** Variables ***
 ${SIGNATURE_BOX}    css:.lsp-signature-help
+${SIGNATURE_HIGHLIGHTED_ARG}    css:.lsp-signature-help mark
 
 *** Test Cases ***
 Triggers Signature Help After A Keystroke
-    Setup Notebook    Python    Signature.ipynb
     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}
+    Wait Until Keyword Succeeds    10x    0.5s    Element Should Contain    ${SIGNATURE_BOX}    Important docstring of abc()
+    Element Should Contain    ${SIGNATURE_HIGHLIGHTED_ARG}    x
+    # should remain visible after typing an argument
+    Press Keys    None    x=2,
     Element Should Contain    ${SIGNATURE_BOX}    Important docstring of abc()
-    [Teardown]    Clean Up After Working With File    Signature.ipynb
+    # and should switch highlight to y
+    Wait Until Keyword Succeeds    20x    0.5s    Element Should Contain    ${SIGNATURE_HIGHLIGHTED_ARG}    y
+    Press Keys    None    LEFT
+    # should switch back to x
+    Wait Until Keyword Succeeds    20x    0.5s    Element Should Contain    ${SIGNATURE_HIGHLIGHTED_ARG}    x
+    # should close on closing bracket
+    Press Keys    None    )
+    Wait Until Keyword Succeeds    20x    0.5s    Page Should Not Contain Element    ${SIGNATURE_BOX}
+
+Should Close After Moving Cursor Prior To Start
+    Enter Cell Editor    1    line=6
+    Press Keys    None    (
+    Wait Until Keyword Succeeds    20x    0.5s    Page Should Contain Element    ${SIGNATURE_BOX}
+    Press Keys    None    LEFT
+    Wait Until Keyword Succeeds    20x    0.5s    Page Should Not Contain Element    ${SIGNATURE_BOX}
+
+Should Close After Executing The Cell
+    Enter Cell Editor    1    line=6
+    Press Keys    None    (
+    Wait Until Keyword Succeeds    20x    0.5s    Page Should Contain Element    ${SIGNATURE_BOX}
+    Press Keys    None    SHIFT+ENTER
+    Wait Until Keyword Succeeds    20x    0.5s    Page Should Not Contain Element    ${SIGNATURE_BOX}
 
 Invalidates On Cell Change
-    Setup Notebook    Python    Signature.ipynb
     Enter Cell Editor    1    line=6
     Press Keys    None    (
+    Wait Until Keyword Succeeds    20x    0.5s    Page Should Contain Element    ${SIGNATURE_BOX}
     Enter Cell Editor    2
-    # just to increase chances of caching this on CI (which is slow)
-    Sleep    5s
-    Page Should Not Contain Element    ${SIGNATURE_BOX}
+    Wait Until Keyword Succeeds    20x    0.5s    Page Should Not Contain Element    ${SIGNATURE_BOX}
diff --git a/atest/examples/Signature.ipynb b/atest/examples/Signature.ipynb
@@ -11,7 +11,7 @@
    },
    "outputs": [],
    "source": [
-    "def abc(x=1):\n",
+    "def abc(x=1, y=2):\n",
     "    \"\"\"Important docstring of abc()\"\"\"\n",
     "    pass\n",
     "\n",
@@ -31,7 +31,7 @@
  ],
  "metadata": {
   "kernelspec": {
-   "display_name": "Python 3",
+   "display_name": "Python 3 (ipykernel)",
    "language": "python",
    "name": "python3"
   },

diff --git a/docs/Configuring.ipynb b/docs/Configuring.ipynb
@@ -46,9 +46,8 @@
     "and we know about them. You can disable auto-detection behavior by configuring\n",
     "[autodetect](#autodetect).\n",
     "\n",
-    "If you did not find an implementation for the language server you need on\n",
-    "the list of [known language servers](./Language%20Servers.html), continue\n",
-    "reading!\n",
+    "If you did not find an implementation for the language server you need on the\n",
+    "list of [known language servers](./Language%20Servers.html), continue reading!\n",
     "\n",
     "> Please consider [contributing your language server spec](./Contributing.html)\n",
     "> to `jupyter-lsp`!"
@@ -113,8 +112,8 @@
     "The documentation of `display_name` along many other properties is available in\n",
     "the [schema][]. Please note that some of the properties defined in the schema\n",
     "are intended for future use: we would like to use them to enrich the user\n",
-    "experience but we prioritized other features for now. We welcome any help\n",
-    "in creating the user interface making use of these properties.\n",
+    "experience but we prioritized other features for now. We welcome any help in\n",
+    "creating the user interface making use of these properties.\n",
     "\n",
     "[schema]:\n",
     "  https://github.com/krassowski/jupyterlab-lsp/blob/master/python_packages/jupyter_lsp/jupyter_lsp/schema/schema.json\n",

diff --git a/packages/jupyterlab-lsp/package.json b/packages/jupyterlab-lsp/package.json
@@ -33,7 +33,7 @@
     "build:labextension:dev": "jupyter labextension build --development True .",
     "build:lib": "tsc",
     "build:prod": "jlpm run build:lib && jlpm run build:labextension",
-    "build:schema": "jlpm build:schema-backend && jlpm build:schema-completion && jlpm build:schema-hover && jlpm build:schema-diagnostics && jlpm build:schema-syntax_highlighting && jlpm build:schema-jump_to && jlpm build:schema-highlights && jlpm build:schema-plugin",
+    "build:schema": "jlpm build:schema-backend && jlpm build:schema-completion && jlpm build:schema-hover && jlpm build:schema-diagnostics && jlpm build:schema-syntax_highlighting && jlpm build:schema-jump_to && jlpm build:schema-signature && jlpm build:schema-highlights && jlpm build:schema-plugin",
     "build:schema-backend": "json2ts ../../python_packages/jupyter_lsp/jupyter_lsp/schema/schema.json --unreachableDefinitions | prettier --stdin-filepath _schema.d.ts > src/_schema.d.ts",
     "build:schema-plugin": "json2ts schema/plugin.json | prettier --stdin-filepath _plugin.d.ts > src/_plugin.d.ts",
     "build:schema-completion": "json2ts schema/completion.json | prettier --stdin-filepath _completion.d.ts > src/_completion.d.ts",
@@ -42,6 +42,7 @@
     "build:schema-jump_to": "json2ts schema/jump_to.json | prettier --stdin-filepath _jump_to.d.ts > src/_jump_to.d.ts",
     "build:schema-syntax_highlighting": "json2ts schema/syntax_highlighting.json | prettier --stdin-filepath _syntax_highlighting.d.ts > src/_syntax_highlighting.d.ts",
     "build:schema-highlights": "json2ts schema/highlights.json | prettier --stdin-filepath _highlights.d.ts > src/_highlights.d.ts",
+    "build:schema-signature": "json2ts schema/signature.json | prettier --stdin-filepath _signature.d.ts > src/_signature.d.ts",
     "bundle": "npm pack .",
     "clean": "jlpm run clean:lib",
     "clean:all": "jlpm run clean:lib && jlpm run clean:labextension",
@@ -88,7 +89,7 @@
     "@lumino/widgets": "^1.16.1",
     "@retrolab/application": "^0.2.0",
     "@types/chai": "^4.1.7",
-    "@types/codemirror": "^0.0.74",
+    "@types/codemirror": "^5.6.20",
     "@types/events": "^3.0.0",
     "@types/jest": "^23.3.11",
     "@types/lodash.mergewith": "^4.6.1",

diff --git a/packages/jupyterlab-lsp/schema/signature.json b/packages/jupyterlab-lsp/schema/signature.json
@@ -0,0 +1,25 @@
+{
+  "jupyter.lab.setting-icon": "lsp:hover",
+  "jupyter.lab.setting-icon-label": "Language integration",
+  "title": "Code Signature",
+  "description": "LSP Code Signature Help settings.",
+  "type": "object",
+  "properties": {
+    "closeCharacters": {
+      "title": "Close characters",
+      "type": "array",
+      "items": {
+        "type": "string"
+      },
+      "default": [")", ";"],
+      "description": "An array of characters which should close the signature help. The signature help may be shown again after typing a character from this list if the server requests it to open."
+    },
+    "maxLines": {
+      "title": "Number of lines to show without collapsing",
+      "type": "number",
+      "default": 12,
+      "description": "The number of documentation lines to show without collapsing into the details section."
+    }
+  },
+  "jupyter.lab.shortcuts": []
+}
diff --git a/packages/jupyterlab-lsp/src/components/free_tooltip.ts b/packages/jupyterlab-lsp/src/components/free_tooltip.ts
@@ -4,7 +4,11 @@
 import { HoverBox } from '@jupyterlab/apputils';
 import { CodeEditor } from '@jupyterlab/codeeditor';
 import { IDocumentWidget } from '@jupyterlab/docregistry';
-import { IRenderMimeRegistry } from '@jupyterlab/rendermime';
+import {
+  IRenderMime,
+  MimeModel,
+  IRenderMimeRegistry
+} from '@jupyterlab/rendermime';
 import { Tooltip } from '@jupyterlab/tooltip';
 import { Widget } from '@lumino/widgets';
 import * as lsProtocol from 'vscode-languageserver-protocol';
@@ -16,15 +20,25 @@ import { IEditorPosition } from '../positioning';
 const MIN_HEIGHT = 20;
 const MAX_HEIGHT = 250;
 
+const CLASS_NAME = 'lsp-tooltip';
+
 interface IFreeTooltipOptions extends Tooltip.IOptions {
   /**
    * Position at which the tooltip should be placed, or null (default) to use the current cursor position.
    */
   position: CodeEditor.IPosition | null;
   /**
-   * Should the tooltip be placed at the end of the line indicated by position?
+   * HoverBox privilege.
+   */
+  privilege?: 'above' | 'below' | 'forceAbove' | 'forceBelow';
+  /**
+   * Alignment with respect to the current token.
+   */
+  alignment?: 'start' | 'end' | null;
+  /**
+   * default: true; ESC will always hide
    */
-  moveToLineEnd: boolean;
+  hideOnKeyPress?: boolean;
 }
 
 /**
@@ -33,13 +47,45 @@ interface IFreeTooltipOptions extends Tooltip.IOptions {
 // eslint-disable-next-line @typescript-eslint/ban-ts-comment
 // @ts-ignore
 export class FreeTooltip extends Tooltip {
-  position: CodeEditor.IPosition | null;
-  movetoLineEnd: boolean;
-
-  constructor(options: IFreeTooltipOptions) {
+  constructor(protected options: IFreeTooltipOptions) {
     super(options);
-    this.position = options.position;
-    this.movetoLineEnd = options.moveToLineEnd;
+    this._setGeometry();
+    // TODO: remove once https://github.com/jupyterlab/jupyterlab/pull/11010 is merged & released
+    const model = new MimeModel({ data: options.bundle });
+    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+    // @ts-ignore
+    const content: IRenderMime.IRenderer = this._content;
+    content
+      .renderModel(model)
+      .then(() => this._setGeometry())
+      .catch(console.warn);
+  }
+
+  handleEvent(event: Event): void {
+    if (this.isHidden || this.isDisposed) {
+      return;
+    }
+
+    const { node } = this;
+    const target = event.target as HTMLElement;
+
+    switch (event.type) {
+      case 'keydown': {
+        const keyCode = (event as KeyboardEvent).keyCode;
+        // ESC or Backspace cancel anyways
+        if (
+          node.contains(target) ||
+          (!this.options.hideOnKeyPress && keyCode != 27 && keyCode != 8)
+        ) {
+          return;
+        }
+        this.dispose();
+        break;
+      }
+      default:
+        super.handleEvent(event);
+        break;
+    }
   }
 
   // eslint-disable-next-line @typescript-eslint/ban-ts-comment
@@ -50,7 +96,9 @@ export class FreeTooltip extends Tooltip {
     // @ts-ignore
     const editor = this._editor as CodeEditor.IEditor;
     const cursor: CodeEditor.IPosition =
-      this.position == null ? editor.getCursorPosition() : this.position;
+      this.options.position == null
+        ? editor.getCursorPosition()
+        : this.options.position;
 
     const end = editor.getOffsetAt(cursor);
     const line = editor.getLine(cursor.line);
@@ -61,13 +109,25 @@ export class FreeTooltip extends Tooltip {
 
     let position: CodeEditor.IPosition;
 
-    if (this.movetoLineEnd) {
-      const tokens = line.substring(0, end).split(/\W+/);
-      const last = tokens[tokens.length - 1];
-      const start = last ? end - last.length : end;
-      position = editor.getPositionAt(start);
-    } else {
-      position = cursor;
+    switch (this.options.alignment) {
+      case 'start': {
+        const tokens = line.substring(0, end).split(/\W+/);
+        const last = tokens[tokens.length - 1];
+        const start = last ? end - last.length : end;
+        position = editor.getPositionAt(start);
+        break;
+      }
+      case 'end': {
+        const tokens = line.substring(0, end).split(/\W+/);
+        const last = tokens[tokens.length - 1];
+        const start = last ? end - last.length : end;
+        position = editor.getPositionAt(start);
+        break;
+      }
+      default: {
+        position = cursor;
+        break;
+      }
     }
 
     if (!position) {
@@ -86,52 +146,73 @@ export class FreeTooltip extends Tooltip {
       minHeight: MIN_HEIGHT,
       node: this.node,
       offset: { horizontal: -1 * paddingLeft },
-      privilege: 'below',
+      privilege: this.options.privilege || 'below',
       style: style
     });
   }
 }
 
 export namespace EditorTooltip {
   export interface IOptions {
+    id?: string;
     markup: lsProtocol.MarkupContent;
     ce_editor: CodeEditor.IEditor;
     position: IEditorPosition;
     adapter: WidgetAdapter<IDocumentWidget>;
     className?: string;
+    tooltip?: Partial<IFreeTooltipOptions>;
   }
 }
 
 export class EditorTooltipManager {
   private currentTooltip: FreeTooltip = null;
+  private currentOptions: EditorTooltip.IOptions | null;
 
   constructor(private rendermime_registry: IRenderMimeRegistry) {}
 
   create(options: EditorTooltip.IOptions): FreeTooltip {
     this.remove();
+    this.currentOptions = options;
     let { markup, position, adapter } = options;
     let widget = adapter.widget;
     const bundle =
       markup.kind === 'plaintext'
         ? { 'text/plain': markup.value }
         : { 'text/markdown': markup.value };
     const tooltip = new FreeTooltip({
+      ...(options.tooltip || {}),
       anchor: widget.content,
       bundle: bundle,
       editor: options.ce_editor,
       rendermime: this.rendermime_registry,
-      position: PositionConverter.cm_to_ce(position),
-      moveToLineEnd: false
+      position: PositionConverter.cm_to_ce(position)
     });
+    tooltip.addClass(CLASS_NAME);
     tooltip.addClass(options.className);
     Widget.attach(tooltip, document.body);
     this.currentTooltip = tooltip;
     return tooltip;
   }
 
+  get position(): IEditorPosition {
+    return this.currentOptions.position;
+  }
+
+  isShown(id?: string): boolean {
+    if (id && this.currentOptions && this.currentOptions?.id !== id) {
+      return false;
+    }
+    return (
+      this.currentTooltip &&
+      !this.currentTooltip.isDisposed &&
+      this.currentTooltip.isVisible
+    );
+  }
+
   remove() {
     if (this.currentTooltip !== null) {
       this.currentTooltip.dispose();
+      this.currentTooltip = null;
     }
   }
 }
diff --git a/packages/jupyterlab-lsp/src/connection.ts b/packages/jupyterlab-lsp/src/connection.ts
@@ -188,6 +188,7 @@ class ClientRequestHandler<
     protected emitter: LSPConnection
   ) {}
   request(params: IClientRequestParams[T]): Promise<IClientResult[T]> {
+    // TODO check if is ready?
     this.emitter.log(MessageKind.client_requested, {
       method: this.method,
       message: params