diff --git a/atest/05_Features/Completion.robot b/atest/05_Features/Completion.robot index 6d2dc82c2..9113784ad 100644 --- a/atest/05_Features/Completion.robot +++ b/atest/05_Features/Completion.robot @@ -58,11 +58,54 @@ Autocompletes If Only One Option User Can Select Lowercase After Starting Uppercase [Tags] language:python Setup Notebook Python Completion.ipynb - Enter Cell Editor 4 line=1 + # `from time import Tim` → `from time import time` + 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 4 from time import time + 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 + # `dispdata` → `display_tabledata` + 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 + # `display` → `display_table` + 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 ` → `from statistics import` + 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` → `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 @@ -79,7 +122,7 @@ Triggers Completer On Dot *** Keywords *** Get Cell Editor Content [Arguments] ${cell_nr} - ${content} Execute JavaScript return document.querySelector('.jp-CodeCell:nth-child(${cell_nr}) .CodeMirror').CodeMirror.getValue() + ${content} Execute JavaScript return document.querySelector('.jp-Cell:nth-child(${cell_nr}) .CodeMirror').CodeMirror.getValue() [Return] ${content} Cell Editor Should Equal @@ -87,6 +130,12 @@ Cell Editor Should Equal ${content} = Get Cell Editor Content ${cell} Should Be Equal ${content} ${value} +Select Completer Suggestion + [Arguments] ${text} + ${suggestion} = Set Variable css:.jp-Completer-item[data-value="${text}"] + Mouse Over ${suggestion} + Click Element ${suggestion} code + Completer Should Suggest [Arguments] ${text} Page Should Contain Element ${COMPLETER_BOX} .jp-Completer-item[data-value="${text}"] diff --git a/atest/Keywords.robot b/atest/Keywords.robot index ea078aaca..6d8ad7041 100644 --- a/atest/Keywords.robot +++ b/atest/Keywords.robot @@ -198,8 +198,13 @@ Gently Reset Workspace 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 + Click Element css:.jp-Cell:nth-child(${cell_nr}) .CodeMirror-line:nth-child(${line}) + Wait Until Page Contains Element css:.jp-Cell:nth-child(${cell_nr}) .CodeMirror-focused + +Place Cursor In Cell Editor At + [Arguments] ${cell_nr} ${line} ${character} + Enter Cell Editor ${cell_nr} ${line} + 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 diff --git a/atest/examples/Completion.ipynb b/atest/examples/Completion.ipynb index dd7a18dab..2aca6d4d8 100644 --- a/atest/examples/Completion.ipynb +++ b/atest/examples/Completion.ipynb @@ -28,6 +28,13 @@ "list." ] }, + { + "cell_type": "markdown", + "metadata": {}, + "source": [ + "`from time import Tim` → `from time import time`" + ] + }, { "cell_type": "code", "execution_count": null, @@ -36,6 +43,87 @@ "source": [ "from time import Tim" ] + }, + { + "cell_type": "markdown", + "metadata": {}, + "source": [ + "Setup (not a test case)" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "def display_table():\n", + " pass" + ] + }, + { + "cell_type": "markdown", + "metadata": {}, + "source": [ + "`dispdata` → `display_tabledata`" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "dispdata" + ] + }, + { + "cell_type": "markdown", + "metadata": {}, + "source": [ + "`display` → `display_table`" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "display" + ] + }, + { + "cell_type": "markdown", + "metadata": {}, + "source": [ + "`from statistics ` → `from statistics import`" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "from statistics " + ] + }, + { + "cell_type": "markdown", + "metadata": {}, + "source": [ + "`import os.pat` → select `pathsep` → `import os.pathsep` (tests whether kernel prefixes are removed)" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "import os.pat" + ] } ], "metadata": { diff --git a/packages/jupyterlab-lsp/src/adapters/jupyterlab/components/completion.ts b/packages/jupyterlab-lsp/src/adapters/jupyterlab/components/completion.ts index 4a111286b..6dfccb02d 100644 --- a/packages/jupyterlab-lsp/src/adapters/jupyterlab/components/completion.ts +++ b/packages/jupyterlab-lsp/src/adapters/jupyterlab/components/completion.ts @@ -111,6 +111,7 @@ export class LSPConnector extends DataConnector< const start = editor.getPositionAt(token.offset); const end = editor.getPositionAt(token.offset + token.value.length); + let position_in_token = cursor.column - start.column - 1; const typed_character = token.value[cursor.column - start.column - 1]; let start_in_root = this.transform_from_editor_to_root(start); @@ -150,7 +151,8 @@ export class LSPConnector extends DataConnector< virtual_start, virtual_end, virtual_cursor, - document + document, + position_in_token ) ]).then(([kernel, lsp]) => this.merge_replies(kernel, lsp, this._editor) @@ -163,7 +165,8 @@ export class LSPConnector extends DataConnector< virtual_start, virtual_end, virtual_cursor, - document + document, + position_in_token ).catch(e => { console.warn('LSP: hint failed', e); return this.fallback_connector.fetch(request); @@ -180,7 +183,8 @@ export class LSPConnector extends DataConnector< start: IVirtualPosition, end: IVirtualPosition, cursor: IVirtualPosition, - document: VirtualDocument + document: VirtualDocument, + position_in_token: number ): Promise { let connection = this._connections.get(document.id_path); @@ -190,7 +194,7 @@ export class LSPConnector extends DataConnector< // to the matches... // Suggested in https://github.com/jupyterlab/jupyterlab/issues/7044, TODO PR - console.log(token); + console.log('[LSP][Completer] Token:', token); let completion_items: lsProtocol.CompletionItem[] = []; await connection @@ -208,6 +212,8 @@ export class LSPConnector extends DataConnector< completion_items = items || []; }); + let prefix = token.value.slice(0, position_in_token + 1); + let matches: Array = []; const types: Array = []; let all_non_prefixed = true; @@ -219,10 +225,22 @@ export class LSPConnector extends DataConnector< // kind: 3 // label: "mean(data)" // sortText: "amean" + + // TODO: add support for match.textEdit let text = match.insertText ? match.insertText : match.label; - if (text.toLowerCase().startsWith(token.value.toLowerCase())) { + if (text.toLowerCase().startsWith(prefix.toLowerCase())) { all_non_prefixed = false; + if (prefix !== token.value) { + if (text.toLowerCase().startsWith(token.value.toLowerCase())) { + // given a completion insert text "display_table" and two test cases: + // dispdata → display_tabledata + // display → display_table + // we have to adjust the prefix for the latter (otherwise we would get display_tablelay), + // as we are constrained NOT to replace after the prefix (which would be "disp" otherwise) + prefix = token.value; + } + } } matches.push(text); @@ -243,7 +261,7 @@ export class LSPConnector extends DataConnector< // text = token.value + text; // but it did not work for "from statistics " and lead to "from statisticsimport" (no space) start: token.offset + (all_non_prefixed ? 1 : 0), - end: token.offset + token.value.length, + end: token.offset + prefix.length, matches: matches, metadata: { _jupyter_types_experimental: types @@ -266,7 +284,7 @@ export class LSPConnector extends DataConnector< } else if (lsp.matches.length === 0) { return kernel; } - console.log('merging LSP and kernel completions:', lsp, kernel); + console.log('[LSP][Completer] Merging completions:', lsp, kernel); // Populate the result with a copy of the lsp matches. const matches = lsp.matches.slice(); @@ -285,8 +303,10 @@ export class LSPConnector extends DataConnector< if (lsp.start > kernel.start) { const cursor = editor.getCursorPosition(); const line = editor.getLine(cursor.line); - prefix = line.substring(cursor.column - 1, cursor.column); - console.log('will remove prefix from kernel response:', prefix); + prefix = line.substring(kernel.start, lsp.start); + console.log('[LSP][Completer] Removing kernel prefix: ', prefix); + } else if (lsp.start < kernel.start) { + console.warn('[LSP][Completer] Kernel start > LSP start'); } let remove_prefix = (value: string) => {