diff --git a/CHANGELOG.md b/CHANGELOG.md index 56cd3de16..0e74e755f 100644 --- a/CHANGELOG.md +++ b/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` diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9ff33ac9b..8164f4ccb 100644 --- a/CONTRIBUTING.md +++ b/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, diff --git a/atest/05_Features/Completion.robot b/atest/05_Features/Completion.robot index fcd76a3e8..de8b5a9a0 100644 --- a/atest/05_Features/Completion.robot +++ b/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 diff --git a/atest/05_Features/Signature.robot b/atest/05_Features/Signature.robot new file mode 100644 index 000000000..71aecc1b7 --- /dev/null +++ b/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 diff --git a/atest/Keywords.robot b/atest/Keywords.robot index c94fa9334..1f1ffd456 100644 --- a/atest/Keywords.robot +++ b/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 diff --git a/atest/Variables.robot b/atest/Variables.robot index ce1ff16ac..81d680e82 100644 --- a/atest/Variables.robot +++ b/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 diff --git a/atest/examples/Signature.ipynb b/atest/examples/Signature.ipynb new file mode 100644 index 000000000..402fdd383 --- /dev/null +++ b/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 +} diff --git a/packages/jupyterlab-lsp/package.json b/packages/jupyterlab-lsp/package.json index 70c77dd90..f804620af 100644 --- a/packages/jupyterlab-lsp/package.json +++ b/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", diff --git a/packages/jupyterlab-lsp/src/adapters/codemirror/cm_adapter.ts b/packages/jupyterlab-lsp/src/adapters/codemirror/cm_adapter.ts index f9f6b46d3..292f87cd3 100644 --- a/packages/jupyterlab-lsp/src/adapters/codemirror/cm_adapter.ts +++ b/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(); } diff --git a/packages/jupyterlab-lsp/src/adapters/codemirror/features/completion.ts b/packages/jupyterlab-lsp/src/adapters/codemirror/features/completion.ts index 309e84444..6a96c3d07 100644 --- a/packages/jupyterlab-lsp/src/adapters/codemirror/features/completion.ts +++ b/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 ); diff --git a/packages/jupyterlab-lsp/src/adapters/codemirror/features/signature.ts b/packages/jupyterlab-lsp/src/adapters/codemirror/features/signature.ts index a6c328359..173da15c2 100644 --- a/packages/jupyterlab-lsp/src/adapters/codemirror/features/signature.ts +++ b/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); } } diff --git a/packages/jupyterlab-lsp/src/adapters/jupyterlab/jl_adapter.ts b/packages/jupyterlab-lsp/src/adapters/jupyterlab/jl_adapter.ts index c506687bc..40851d9f0 100644 --- a/packages/jupyterlab-lsp/src/adapters/jupyterlab/jl_adapter.ts +++ b/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 diff --git a/packages/jupyterlab-lsp/src/virtual/console.ts b/packages/jupyterlab-lsp/src/virtual/console.ts new file mode 100644 index 000000000..0f2f884ac --- /dev/null +++ b/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 = '' + kind + '' + text; + this.element.appendChild(entry); + this.element.scrollTop = this.element.scrollHeight; + } + + private to_string(args: any[]) { + return args + .map(arg => '' + JSON.stringify(arg) + '') + .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(); + } +} diff --git a/packages/jupyterlab-lsp/src/virtual/editor.ts b/packages/jupyterlab-lsp/src/virtual/editor.ts index eb6506ea2..cb7d91699 100644 --- a/packages/jupyterlab-lsp/src/virtual/editor.ts +++ b/packages/jupyterlab-lsp/src/virtual/editor.ts @@ -10,6 +10,7 @@ import { } from '../positioning'; import { until_ready } from '../utils'; import { Signal } from '@phosphor/signaling'; +import { EditorLogConsole, create_console } from './console'; export type CodeMirrorHandler = (instance: any, ...args: any[]) => void; type WrappedHandler = (instance: CodeMirror.Editor, ...args: any[]) => void; @@ -33,6 +34,7 @@ export abstract class VirtualEditor implements CodeMirror.Editor { * Whether the editor reflects an interface with multiple cells (such as a notebook) */ has_cells: boolean; + console: EditorLogConsole; public constructor( protected language: () => string, @@ -45,6 +47,7 @@ export abstract class VirtualEditor implements CodeMirror.Editor { this.create_virtual_document(); this.documents_updated = new Signal(this); this.documents_updated.connect(this.on_updated.bind(this)); + this.console = create_console('browser'); } create_virtual_document() { @@ -67,7 +70,7 @@ export abstract class VirtualEditor implements CodeMirror.Editor { try { root_document.close_expired_documents(); } catch (e) { - console.warn('LSP: Failed to close expired documents'); + this.console.warn('LSP: Failed to close expired documents'); } } @@ -102,6 +105,7 @@ export abstract class VirtualEditor implements CodeMirror.Editor { * @param fn - the callback to execute in update lock */ public async with_update_lock(fn: Function) { + this.console.log('Will enter update lock with', fn); await until_ready(() => this.can_update(), 12, 10).then(() => { try { this.update_lock = true; @@ -129,7 +133,7 @@ export abstract class VirtualEditor implements CodeMirror.Editor { this.virtual_document.maybe_emit_changed(); resolve(); } catch (e) { - console.warn('Documents update failed:', e); + this.console.warn('Documents update failed:', e); reject(e); } finally { this.is_update_in_progress = false; @@ -189,7 +193,7 @@ export abstract class VirtualEditor implements CodeMirror.Editor { try { return handler(this, ...args); } catch (error) { - console.warn( + this.console.warn( 'Wrapped handler (which should accept a CodeMirror Editor instance) failed', { error, instance, args, this: this } ); diff --git a/packages/jupyterlab-lsp/style/console.css b/packages/jupyterlab-lsp/style/console.css new file mode 100644 index 000000000..39035c010 --- /dev/null +++ b/packages/jupyterlab-lsp/style/console.css @@ -0,0 +1,25 @@ +.lsp-floating-console { + position: absolute; + width: 600px; + height: 350px; + right: 0; + bottom: 0; + background: rgba(255, 255, 255, 0.8); + border: 3px solid rgba(204, 204, 204, 0.7); + font-size: 12px; + overflow-y: auto; + margin: 0; + list-style-type: none; + padding: 5px; +} + +.lsp-code { + font-family: monospace; + background: #eee; + border-radius: 2px; + padding: 2px 5px; +} + +.lsp-kind { + padding: 2px 5px; +} diff --git a/packages/lsp-ws-connection/src/ws-connection.ts b/packages/lsp-ws-connection/src/ws-connection.ts index 3729581e3..422307002 100644 --- a/packages/lsp-ws-connection/src/ws-connection.ts +++ b/packages/lsp-ws-connection/src/ws-connection.ts @@ -323,7 +323,7 @@ export class LspWsConnection extends events.EventEmitter const code = this.documentInfo.documentText(); const lines = code.split('\n'); - const typedCharacter = lines[location.line][location.ch]; + const typedCharacter = lines[location.line][location.ch - 1]; const triggers = this.serverCapabilities?.signatureHelpProvider?.triggerCharacters || [];