Skip to content

Commit

Permalink
Merge pull request #140 from yunair/fix_ws_signature
Browse files Browse the repository at this point in the history
fix ws signature failed
  • Loading branch information
krassowski authored Jan 12, 2020
2 parents 5a21fe6 + 8266290 commit ce963da
Show file tree
Hide file tree
Showing 16 changed files with 238 additions and 22 deletions.
10 changes: 9 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`

Expand Down Expand Up @@ -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 (
Expand All @@ -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`

Expand Down
4 changes: 4 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
24 changes: 15 additions & 9 deletions atest/05_Features/Completion.robot
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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()
Expand All @@ -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
18 changes: 18 additions & 0 deletions atest/05_Features/Signature.robot
Original file line number Diff line number Diff line change
@@ -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
12 changes: 11 additions & 1 deletion atest/Keywords.robot
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
1 change: 1 addition & 0 deletions atest/Variables.robot
Original file line number Diff line number Diff line change
Expand Up @@ -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
53 changes: 53 additions & 0 deletions atest/examples/Signature.ipynb
Original file line number Diff line number Diff line change
@@ -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
}
2 changes: 1 addition & 1 deletion packages/jupyterlab-lsp/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
10 changes: 7 additions & 3 deletions packages/jupyterlab-lsp/src/adapters/codemirror/cm_adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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() {
Expand All @@ -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);
}
}
Expand Down
8 changes: 6 additions & 2 deletions packages/jupyterlab-lsp/src/adapters/jupyterlab/jl_adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down
62 changes: 62 additions & 0 deletions packages/jupyterlab-lsp/src/virtual/console.ts
Original file line number Diff line number Diff line change
@@ -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();
}
}
Loading

0 comments on commit ce963da

Please sign in to comment.