Merge pull request #520 from krassowski/completion/model-and-sorting

Sorted completions, improve completions marking and filtering and doc panel
jupyter-lsp · Feb 13, 2021 · 5bf9427 · 5bf9427
2 parents f080ad0 + 9dc9f81
commit 5bf9427
Show file tree

Hide file tree

Showing 11 changed files with 503 additions and 159 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,20 @@
 ## CHANGELOG
 
+### `@krassowski/jupyterlab-lsp 3.4.0` (unreleased)
+
+- features:
+
+  - the priority of the completions from kernel can now be changed by switching new `kernelCompletionsFirst` setting ([#520])
+  - completer panel will now always render markdown documentation if available ([#520])
+    - the implementation re-renders the panel as it is the best we can do until [jupyterlab#9663](https://github.com/jupyterlab/jupyterlab/pull/9663) is merged
+  - the completer now uses `filterText` and `sortText` if available to better filter and sort completions ([#520])
+
+- bug fixes:
+  - completer documentation will now consistently show up after filtering the completion items ([#520])
+  - completions containing HTML-like syntax will be displayed properly (an upstream issue) ([#520])
+
+[#520]: https://github.com/krassowski/jupyterlab-lsp/pull/520
+
 ### `@krassowski/jupyterlab-lsp 3.3.1` (2020-02-07)
 
 - bug fixes:

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -227,7 +227,7 @@ python scripts/atest.py --suite "05_Features.Completion"
 ##### Run a single test
 
 ```bash
-python scripts/atest.py --test "Works With Kernel Running"
+python scripts/atest.py --test "Works When Kernel Is Idle"
 ```
 
 ##### Run test with a tag

diff --git a/atest/05_Features/Completion.robot b/atest/05_Features/Completion.robot
@@ -22,13 +22,31 @@ Works When Kernel Is Idle
     Completer Should Suggest    TabError
     # this comes from LSP:
     Completer Should Suggest    test
-    # this comes from kernel; sometimes the kernel response may come a bit later
-    Wait Until Keyword Succeeds    20x    0.5s    Completer Should Suggest    %%timeit
+    # this comes from kernel
+    Completer Should Suggest    %%timeit
     Press Keys    None    ENTER
     Capture Page Screenshot    03-completion-confirmed.png
     ${content} =    Get Cell Editor Content    1
     Should Contain    ${content}    TabError
 
+Can Prioritize Kernel Completions
+    Configure JupyterLab Plugin    {"kernelCompletionsFirst": true, "kernelResponseTimeout": -1}    plugin id=${COMPLETION PLUGIN ID}
+    Enter Cell Editor    1    line=2
+    Trigger Completer
+    Completer Should Suggest    %%timeit
+    ${lsp_position} =    Get Completion Item Vertical Position    test
+    ${kernel_position} =    Get Completion Item Vertical Position    %%timeit
+    Should Be True    ${kernel_position} < ${lsp_position}
+
+Can Prioritize LSP Completions
+    Configure JupyterLab Plugin    {"kernelCompletionsFirst": false, "kernelResponseTimeout": -1}    plugin id=${COMPLETION PLUGIN ID}
+    Enter Cell Editor    1    line=2
+    Trigger Completer
+    Completer Should Suggest    %%timeit
+    ${lsp_position} =    Get Completion Item Vertical Position    test
+    ${kernel_position} =    Get Completion Item Vertical Position    %%timeit
+    Should Be True    ${kernel_position} > ${lsp_position}
+
 Invalidates On Cell Change
     Enter Cell Editor    1    line=2
     Press Keys    None    TAB
@@ -233,10 +251,10 @@ Completes Correctly With R Double And Triple Colon
     Place Cursor In File Editor At    2    7
     Wait Until Fully Initialized
     Trigger Completer
-    Completer Should Suggest    assertCondition
-    Select Completer Suggestion    assertCondition
-    Wait Until Keyword Succeeds    40x    0.5s    File Editor Line Should Equal    1    tools::assertCondition
-    # tripple colont
+    Completer Should Suggest    .print.via.format
+    Select Completer Suggestion    .print.via.format
+    Wait Until Keyword Succeeds    40x    0.5s    File Editor Line Should Equal    1    tools::.print.via.format
+    # tripple colon
     Place Cursor In File Editor At    4    11
     Trigger Completer
     Completer Should Suggest    .packageName
@@ -254,9 +272,13 @@ Completes Large Namespaces
 
 Shows Documentation With CompletionItem Resolve
     [Setup]    Prepare File for Editing    R    completion    completion.R
-    Place Cursor In File Editor At    8    12
+    Place Cursor In File Editor At    8    7
     Wait Until Fully Initialized
     Trigger Completer
+    Completer Should Suggest    print.data.frame
+    Completer Should Include Documentation    Print a data frame.
+    # should remain visible after typing:
+    Press Keys    None    efa
     Completer Should Suggest    print.default
     Completer Should Include Documentation    the default method of the
     [Teardown]    Clean Up After Working With File    completion.R
@@ -297,6 +319,11 @@ Completer Should Suggest
     Wait Until Page Contains Element    ${COMPLETER_BOX} .jp-Completer-item[data-value="${text}"]    timeout=${timeout}
     Capture Page Screenshot    ${text.replace(' ', '_')}.png
 
+Get Completion Item Vertical Position
+    [Arguments]    ${text}
+    ${position} =    Get Vertical Position    ${COMPLETER_BOX} .jp-Completer-item[data-value="${text}"]
+    [Return]    ${position}
+
 Completer Should Include Icon
     [Arguments]    ${icon}
     Wait Until Page Contains Element    ${COMPLETER_BOX} svg[data-icon="${icon}"]    timeout=10s

diff --git a/atest/examples/completion.R b/atest/examples/completion.R
@@ -1,8 +1,8 @@
-# `tools::<tab>` → select `assertCondition` → `tools::assertCondition`
+# `tools::<tab>` → select `.print.via.format` → `tools::.print.via.format`
 tools::
 # `datasets:::<tab>` → select `.packageName` → `datasets:::.packageName`
 datasets:::
 # `base:::<tab>` → works
 base:::
-# `print.defaul<tab>` → shows documentation for `print.default`
-print.defaul
+# `print.d<tab>` → shows documentation for `print.data.frame` → press `efa` → shows documentation for `print.default`
+print.d
diff --git a/packages/jupyterlab-lsp/schema/completion.json b/packages/jupyterlab-lsp/schema/completion.json
@@ -42,7 +42,12 @@
       "title": "Completer theme",
       "type": ["string", "null"],
       "default": "vscode",
-      "description": "The identifier of a completer theme with icons which indicate the kind of completion. Set to null to disable icons."
+      "description": "The identifier of a completer theme with icons which indicate the kind of completion. Set to null to disable icons. Search for 'completer themes' in the command palette for a command displaying available themes."
+    },
+    "kernelCompletionsFirst": {
+      "title": "Prioritize completions from kernel",
+      "default": false,
+      "description": "In case of ties when sorting completions, should the kernel completions receive higher priority than the language server completions?"
     },
     "typesMap": {
       "title": "Mapping of custom kernel types to valid completion kind names",

diff --git a/packages/jupyterlab-lsp/src/features/completion/completion.ts b/packages/jupyterlab-lsp/src/features/completion/completion.ts
@@ -7,7 +7,7 @@ import * as CodeMirror from 'codemirror';
 import { CodeMirrorIntegration } from '../../editor_integration/codemirror';
 import { JupyterFrontEnd } from '@jupyterlab/application';
 import { IEditorChangedData, WidgetAdapter } from '../../adapters/adapter';
-import { LazyCompletionItem, LSPConnector } from './completion_handler';
+import { LSPConnector } from './completion_handler';
 import { CompletionHandler, ICompletionManager } from '@jupyterlab/completer';
 import { CodeEditor } from '@jupyterlab/codeeditor';
 import { IDocumentWidget } from '@jupyterlab/docregistry';
@@ -21,6 +21,8 @@ import { NotebookAdapter } from '../../adapters/notebook/notebook';
 import { ILSPCompletionThemeManager } from '@krassowski/completion-theme/lib/types';
 import { LSPCompletionRenderer } from './renderer';
 import { IRenderMimeRegistry } from '@jupyterlab/rendermime';
+import { LSPCompleterModel } from './model';
+import { LazyCompletionItem } from './item';
 
 const DOC_PANEL_SELECTOR = '.jp-Completer-docpanel';
 const DOC_PANEL_PLACEHOLDER_CLASS = 'lsp-completer-placeholder';
@@ -117,6 +119,10 @@ export class CompletionLabIntegration implements IFeatureLabIntegration {
     item: LazyCompletionItem
   ) {
     if (!item.supportsResolution()) {
+      if (item.isDocumentationMarkdown) {
+        // TODO: remove once https://github.com/jupyterlab/jupyterlab/pull/9663 is merged and released
+        this.refresh_doc_panel(item);
+      }
       return;
     }
 
@@ -187,10 +193,18 @@ export class CompletionLabIntegration implements IFeatureLabIntegration {
       },
       this.renderer
     ) as CompletionHandler;
+    let completer = this.completer;
+    completer.addClass('lsp-completer');
+    completer.model = new LSPCompleterModel();
+  }
+
+  get completer() {
+    return this.current_completion_handler.completer;
   }
 
   invoke_completer(kind: ExtendedCompletionTriggerKind) {
     let command: string;
+
     this.current_completion_connector.trigger_kind = kind;
 
     if (this.adapterManager.currentAdapter instanceof NotebookAdapter) {

diff --git a/packages/jupyterlab-lsp/src/features/completion/completion_handler.ts b/packages/jupyterlab-lsp/src/features/completion/completion_handler.ts
@@ -33,147 +33,9 @@ import {
 import { LabIcon } from '@jupyterlab/ui-components';
 import { ILSPLogConsole } from '../../tokens';
 import { CompletionLabIntegration } from './completion';
+import { LazyCompletionItem } from './item';
 import ICompletionItemsResponseType = CompletionHandler.ICompletionItemsResponseType;
 
-export class LazyCompletionItem implements CompletionHandler.ICompletionItem {
-  private _documentation: string;
-  private _is_documentation_markdown: boolean;
-  private _requested_resolution: boolean;
-  private _resolved: boolean;
-  /**
-   * Self-reference to make sure that the instance for will remain accessible
-   * after any copy operation (whether via spread syntax or Object.assign)
-   * performed by the JupyterLab completer internals.
-   */
-  public self: LazyCompletionItem;
-
-  get isDocumentationMarkdown(): boolean {
-    return this._is_documentation_markdown;
-  }
-
-  constructor(
-    /**
-     * User facing completion.
-     * If insertText is not set, this will be inserted.
-     */
-    public label: string,
-    /**
-     * Type of this completion item.
-     */
-    public type: string,
-    /**
-     * LabIcon object for icon to be rendered with completion type.
-     */
-    public icon: LabIcon,
-    private match: lsProtocol.CompletionItem,
-    private connector: LSPConnector,
-    private uri: string
-  ) {
-    this._setDocumentation(match.documentation);
-    this._requested_resolution = false;
-    this._resolved = false;
-    this.self = this;
-  }
-
-  private _setDocumentation(documentation: string | lsProtocol.MarkupContent) {
-    if (lsProtocol.MarkupContent.is(documentation)) {
-      this._documentation = documentation.value;
-      this._is_documentation_markdown = documentation.kind === 'markdown';
-    } else {
-      this._documentation = documentation;
-      this._is_documentation_markdown = false;
-    }
-  }
-
-  /**
-   * Completion to be inserted.
-   */
-  get insertText(): string {
-    return this.match.insertText || this.match.label;
-  }
-
-  public supportsResolution() {
-    const connection = this.connector.get_connection(this.uri);
-
-    return connection.isCompletionResolveProvider();
-  }
-
-  public needsResolution(): boolean {
-    if (this.documentation) {
-      return false;
-    }
-
-    if (this._resolved) {
-      return false;
-    }
-
-    if (this._requested_resolution) {
-      return false;
-    }
-
-    return this.supportsResolution();
-  }
-
-  public isResolved() {
-    return this._resolved;
-  }
-
-  public fetchDocumentation(): void {
-    if (!this.needsResolution()) {
-      return;
-    }
-
-    const connection = this.connector.get_connection(this.uri);
-
-    this._requested_resolution = true;
-
-    connection
-      .getCompletionResolve(this.match)
-      .then(resolvedCompletionItem => {
-        this.connector.lab_integration.set_doc_panel_placeholder(false);
-        if (resolvedCompletionItem === null) {
-          return;
-        }
-        this._setDocumentation(resolvedCompletionItem.documentation);
-        this._resolved = true;
-        this.connector.lab_integration.refresh_doc_panel(this);
-      })
-      .catch(e => {
-        this.connector.lab_integration.set_doc_panel_placeholder(false);
-        console.warn(e);
-      });
-  }
-
-  /**
-   * A human-readable string with additional information
-   * about this item, like type or symbol information.
-   */
-  get documentation(): string {
-    if (!this.connector.should_show_documentation) {
-      return null;
-    }
-    if (this._documentation) {
-      return this._documentation;
-    }
-    return null;
-  }
-
-  /**
-   * Indicates if the item is deprecated.
-   */
-  get deprecated(): boolean {
-    if (this.match.deprecated) {
-      return this.match.deprecated;
-    }
-    return (
-      this.match.tags &&
-      this.match.tags.some(
-        tag => tag == lsProtocol.CompletionItemTag.Deprecated
-      )
-    );
-  }
-}
-
 /**
  * A LSP connector for completion handlers.
  */
@@ -195,6 +57,10 @@ export class LSPConnector
   lab_integration: CompletionLabIntegration;
   items: CompletionHandler.ICompletionItems;
 
+  get kernel_completions_first(): boolean {
+    return this.options.settings.composite.kernelCompletionsFirst;
+  }
+
   protected get suppress_auto_invoke_in(): string[] {
     return this.options.settings.composite.suppressInvokeIn;
   }
@@ -449,7 +315,6 @@ export class LSPConnector
     lspCompletionItems.forEach(match => {
       let kind = match.kind ? CompletionItemKind[match.kind] : '';
       let completionItem = new LazyCompletionItem(
-        match.label,
         kind,
         this.icon_for(kind),
         match,
@@ -520,15 +385,17 @@ export class LSPConnector
           label: item.text as string,
           insertText: item.text as string,
           type: item.type as string,
-          icon: this.icon_for(item.type as string)
+          icon: this.icon_for(item.type as string),
+          sortText: this.kernel_completions_first ? 'a' : 'z'
         };
       });
     } else {
       items = reply.matches.map(match => {
         return {
           label: match,
           insertText: match,
-          icon: this.icon_for('Kernel')
+          icon: this.icon_for('Kernel'),
+          sortText: this.kernel_completions_first ? 'a' : 'z'
         };
       });
     }