Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor LSPConnection, ConnectionManager #165

Merged
merged 67 commits into from
Feb 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
67 commits
Select commit Hold shift + click to select a range
a3a5ac5
(wip) only create one connection manager per app, one socket per lang…
bollwyvl Jan 16, 2020
afc45ac
more work on lazy loading
bollwyvl Jan 16, 2020
312c9e5
make highlight uri-aware
bollwyvl Jan 16, 2020
568234c
make hover uri-aware
bollwyvl Jan 16, 2020
48e0a94
mostly get highlighting to not duplicate in wrong editors
bollwyvl Jan 18, 2020
99752d3
Merge remote-tracking branch 'upstream/master' into add-one-socket-pe…
bollwyvl Jan 20, 2020
d8fb97f
start working on getting the unit tests back up
bollwyvl Jan 20, 2020
8b39f9d
fix connection test
bollwyvl Jan 20, 2020
e11a63f
add rename to lsp interface
bollwyvl Jan 20, 2020
ac82ff0
hide non-essential logging behind DEBUG
bollwyvl Jan 20, 2020
41c5b0c
wait for kernel before initializing notebook adapter to reduce log ou…
bollwyvl Jan 20, 2020
e5ec536
hide more logging
bollwyvl Jan 20, 2020
1ff5b32
squash react log errors
bollwyvl Jan 20, 2020
83c1dd8
squash more errors with checks on widget lifecycle
bollwyvl Jan 20, 2020
95eacdf
more log squashing
bollwyvl Jan 20, 2020
291e525
walk back expectation of kernel readiness, change log squashing approach
bollwyvl Jan 20, 2020
bd6442b
use solve_uris for document_info
bollwyvl Jan 20, 2020
32b6a64
rework document closing behavior
bollwyvl Jan 20, 2020
93c5067
linting
bollwyvl Jan 20, 2020
4ac60a6
handle some open doc issues when closing tabs
bollwyvl Jan 20, 2020
548998b
change connection constructor for searchability
bollwyvl Jan 20, 2020
273c9b6
remove todo on bash, since they are randomized now
bollwyvl Jan 20, 2020
74ebb52
work on behavior of foreign docs, diagnostics and singleton connectio…
bollwyvl Jan 20, 2020
b6df0b7
pyflakes and mypy can both claim the error
bollwyvl Jan 20, 2020
bbb50d4
start working on unit tests
bollwyvl Jan 20, 2020
505c8a6
more work on unit tests (down to 5 failing)
bollwyvl Jan 21, 2020
c6f9f36
temporarily ignore jest test fails on azure
bollwyvl Jan 21, 2020
69559b1
try linking in hot lsp-ws
bollwyvl Jan 21, 2020
79095e6
try entrypoints instead of pkg_resources
bollwyvl Jan 21, 2020
d9f2605
roll back zipp stuff
bollwyvl Jan 22, 2020
9c225e6
activeCell can be null
bollwyvl Jan 22, 2020
a995e11
fix up unit tests through PageConfig, other means
bollwyvl Jan 22, 2020
71fc0e7
uncomment unit tests
bollwyvl Jan 22, 2020
4d5b112
trailing quote
bollwyvl Jan 22, 2020
70d2d39
hide some more warnings from unready connections
bollwyvl Jan 22, 2020
5ee1427
refactor goto to use promises
bollwyvl Jan 22, 2020
949407d
refactor signature to use promises
bollwyvl Jan 22, 2020
1e03f43
more work on promises for signature, hover
bollwyvl Jan 22, 2020
792749e
rework on hover, completion
bollwyvl Jan 22, 2020
763aa6d
typeofpocalypse
bollwyvl Jan 22, 2020
c64556f
handle malformed end ranges
bollwyvl Jan 22, 2020
2be2b68
work on type defs
bollwyvl Jan 22, 2020
4450a0b
more initialization check into notebook setup
bollwyvl Jan 22, 2020
74991ec
restore verbose logging, remove DEBUG
bollwyvl Jan 22, 2020
2d470a1
it would appear server_root no longer does anything
bollwyvl Jan 23, 2020
bbbb4f9
substantially increase ready check interval (50 -> 200)
bollwyvl Jan 23, 2020
f0d38a1
don't react to document changes until connection is ready
bollwyvl Jan 23, 2020
3249d42
Merge remote-tracking branch 'upstream/master' into add-one-socket-pe…
bollwyvl Jan 23, 2020
29be129
some looking at memory profiling
bollwyvl Jan 24, 2020
5763fd4
Merge remote-tracking branch 'upstream/master' into add-one-socket-pe…
bollwyvl Jan 29, 2020
bcb4e65
significant memory leak investigation underway
bollwyvl Jan 30, 2020
2145c42
clear up some more memory leaks
bollwyvl Feb 2, 2020
55bb0d3
clobber debug stuff again
bollwyvl Feb 2, 2020
d384809
handle late cleanup of document vs update
bollwyvl Feb 2, 2020
e2a869c
rework screenshot strategy
bollwyvl Feb 2, 2020
f2a79d6
missing quote
bollwyvl Feb 2, 2020
3fa15ee
rework document closing (connection doesn't go away)
bollwyvl Feb 2, 2020
8ae1b9e
fix completion regression
bollwyvl Feb 2, 2020
36aa419
some cleanup of completion, document updating
bollwyvl Feb 2, 2020
832c7b5
hoist pathChanged concern to plugin level
bollwyvl Feb 2, 2020
87173e1
just bump the max listeners to something ridiculous
bollwyvl Feb 2, 2020
452969b
remove highlight and hover connection_handlers
bollwyvl Feb 2, 2020
33b1b04
restore py38/win tests
bollwyvl Feb 3, 2020
9e7337f
update CHANGELOG
bollwyvl Feb 4, 2020
02569d3
some docs and todos on connection manager
bollwyvl Feb 4, 2020
7e3bc34
merge master
bollwyvl Feb 8, 2020
3d45458
linting
bollwyvl Feb 8, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,4 @@ junit.xml
coverage/
.vscode/
_schema.d.ts
.virtual_documents/
22 changes: 22 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,27 @@
# CHANGELOG

## `@krassowski/jupyterlab-lsp 0.8.0` (unreleased)

- features

- opens a maximum of one WebSocket per language server ([#165][])
- lazy-loads language server protocol machinery ([#165][])
- waits much longer for slow-starting language servers ([#165][])
- cleans up documents, handlers, events, and signals more aggressively ([#165][])
- ignores malformed diagnostic ranges, enabling markdown support ([#165][])
- passes tests on Python 3.8 on Windows ([#165][])

## `lsp-ws-connection 0.4.0` (unreleased)

- breaking changes

- no longer assumes one document per connection ([#165][])
- requires documents be opened explicitly ([#165][])
- use of the `eventEmitter` pattern mostly deprecated in favor of `Promise`s
([#165][])

[#165]: https://github.com/krassowski/jupyterlab-lsp/pull/165

## `jupyter-lsp 0.7.0`

- bugfixes
Expand Down
6 changes: 2 additions & 4 deletions atest/01_Editor.robot
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@ ${CM CURSORS} css:.CodeMirror-cursors:not([style='visibility: hidden'])

*** Test Cases ***
Bash
[Documentation] TODO: figure out why the first server is extra flaky
Wait Until Keyword Succeeds 6x 5s Editor Shows Features for Language Bash example.sh Diagnostics=Failed to parse expression
... Jump to Definition=fib
Editor Shows Features for Language Bash example.sh Diagnostics=Failed to parse expression Jump to Definition=fib

CSS
${def} = Set Variable xpath:(//span[contains(@class, 'cm-variable-2')][contains(text(), '--some-var')])[last()]
Expand Down Expand Up @@ -67,7 +65,7 @@ Editor Shows Features for Language
Set Tags language:${Language.lower()}
Set Screenshot Directory ${OUTPUT DIR}${/}screenshots${/}editor${/}${Language.lower()}
Copy File examples${/}${file} ${OUTPUT DIR}${/}home${/}${file}
Lab Command Close All Tabs
Try to Close All Tabs
Open ${file} in ${MENU EDITOR}
Capture Page Screenshot 00-opened.png
FOR ${f} IN @{features}
Expand Down
8 changes: 4 additions & 4 deletions atest/03_Notebook.robot
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
*** Settings ***
Suite Setup Setup Suite For Screenshots notebook
Test Setup Try to Close All Tabs
Resource Keywords.robot

*** Test Cases ***
Python
[Setup] Gently Reset Workspace
Setup Notebook Python Python.ipynb
Capture Page Screenshot 01-python.png
${diagnostic} = Set Variable W291 trailing whitespace (pycodestyle)
Expand All @@ -13,11 +13,11 @@ Python
Clean Up After Working With File Python.ipynb

Foregin Extractors
[Setup] Gently Reset Workspace
Setup Notebook Python Foreign extractors.ipynb
@{diagnostics} = Create List Failed to parse expression undefined name 'valid' (pyflakes) Trailing whitespace is superfluous. (lintr)
# if mypy and pyflakes will fight over `(N|n)ame 'valid'`, just hope for the best
@{diagnostics} = Create List Failed to parse expression ame 'valid' Trailing whitespace is superfluous. (lintr)
FOR ${diagnostic} IN @{diagnostics}
Wait Until Page Contains Element css:.cm-lsp-diagnostic[title="${diagnostic}"] timeout=35s
Wait Until Page Contains Element css:.cm-lsp-diagnostic[title*\="${diagnostic}"] timeout=35s
Capture Page Screenshot 0x-${diagnostic}.png
END
Clean Up After Working With File Foreign Extractors.ipynb
38 changes: 5 additions & 33 deletions atest/05_Features/Completion.robot
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
*** Settings ***
Suite Setup Setup Suite For Screenshots completion
Test Setup Setup Notebook Python Completion.ipynb
Test Teardown Clean Up After Working With File Completion.ipynb
Force Tags feature:completion
Resource ../Keywords.robot

Expand All @@ -9,9 +11,6 @@ ${COMPLETER_BOX} css:.jp-Completer.jp-HoverBox
*** Test Cases ***
Works With Kernel Running
[Documentation] The suggestions from kernel and LSP should get integrated.
[Tags] language:python
Setup Notebook Python Completion.ipynb
Wait Until Fully Initialized
Enter Cell Editor 1 line=2
Capture Page Screenshot 01-entered-cell.png
Trigger Completer
Expand All @@ -26,12 +25,8 @@ Works With Kernel Running
Capture Page Screenshot 03-completion-confirmed.png
${content} = Get Cell Editor Content 1
Should Contain ${content} TabError
[Teardown] Clean Up After Working With File Completion.ipynb

Works When Kernel Is Shut Down
[Tags] language:python
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 @@ -43,82 +38,58 @@ Works When Kernel Is Shut Down
Completer Should Suggest test
# this comes from kernel:
Completer Should Not Suggest %%timeit
[Teardown] Clean Up After Working With File Completion.ipynb

Autocompletes If Only One Option
[Tags] language:python
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

User Can Select Lowercase After Starting Uppercase
[Tags] language:python
Setup Notebook Python Completion.ipynb
# `from time import Tim<tab>` → `from time import time`
Wait Until Fully Initialized
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 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
# `disp<tab>data` → `display_table<cursor>data`
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
# `disp<tab>lay` → `display_table<cursor>`
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 <tab>` → `from statistics import<cursor>`
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<tab>` → `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
[Tags] language:python
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
Expand All @@ -139,11 +110,12 @@ Select Completer Suggestion

Completer Should Suggest
[Arguments] ${text}
Page Should Contain Element ${COMPLETER_BOX} .jp-Completer-item[data-value="${text}"]
Wait Until Page Contains Element ${COMPLETER_BOX} .jp-Completer-item[data-value="${text}"]
Capture Page Screenshot ${text.replace(' ', '_')}.png

Completer Should Not Suggest
[Arguments] ${text}
Page Should Not Contain Element ${COMPLETER_BOX} .jp-Completer-item[data-value="${text}"]
Wait Until Page Does Not Contain Element ${COMPLETER_BOX} .jp-Completer-item[data-value="${text}"]

Trigger Completer
Press Keys None TAB
Expand Down
1 change: 0 additions & 1 deletion atest/05_Features/Signature.robot
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ ${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 (
Expand Down
20 changes: 15 additions & 5 deletions atest/Keywords.robot
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,17 @@ Open JupyterLab
Close JupyterLab
Close All Browsers

Reset Application State
Close All Tabs
Accept Default Dialog Option
Lab Command Close All Tabs
Accept Default Dialog Option

Try to Close All Tabs
Wait Until Keyword Succeeds 5x 50ms Close All Tabs

Reset Application State
Try to Close All Tabs
Accept Default Dialog Option
Ensure All Kernels Are Shut Down
Lab Command Reset Application State
Wait For Splash
Expand Down Expand Up @@ -172,11 +180,13 @@ Clean Up After Working With File
Setup Notebook
[Arguments] ${Language} ${file}
Set Tags language:${Language.lower()}
Set Screenshot Directory ${OUTPUT DIR}${/}screenshots${/}notebook${/}${file.lower()}
Set Screenshot Directory ${OUTPUT DIR}${/}screenshots${/}notebook${/}${TEST NAME.replace(' ', '_')}
Copy File examples${/}${file} ${OUTPUT DIR}${/}home${/}${file}
Lab Command Close All Tabs
Try to Close All Tabs
Open ${file} in ${MENU NOTEBOOK}
Capture Page Screenshot 00-opened.png
Wait Until Fully Initialized
Capture Page Screenshot 01-initialized.png

Open Diagnostics Panel
Lab Command Show Diagnostics Panel
Expand All @@ -194,7 +204,7 @@ Wait For Dialog
Wait Until Page Contains Element ${DIALOG WINDOW} timeout=180s

Gently Reset Workspace
Lab Command Close All Tabs
Try to Close All Tabs

Enter Cell Editor
[Arguments] ${cell_nr} ${line}=1
Expand All @@ -207,7 +217,7 @@ Place Cursor In Cell Editor At
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
Wait Until Element Contains ${STATUSBAR} Fully initialized timeout=60s
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seemed to reduce flake a fair amount: on a fresh browser load, the lazy load does make the first connection slower to start, so this time isn't getting rolled up into the aggressive up-front animation waiting.


Open Context Menu Over
[Arguments] ${sel}
Expand Down
3 changes: 3 additions & 0 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ variables:
THIRD_PARTY_LABEXTENSIONS: >-
@krassowski/jupyterlab_go_to_definition

LINKED_EXTENSIONS: >-
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should have done this ages ago

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not testing the as-built tarball, but what can you do? that would be an advantage of killing the intermediate layer, as the entire module would be:

export import { ConsoleLogger, listen, MessageConnection } from 'vscode-ws-jsonrpc';

...plus 10 files of boilerplate, and would only need to be updated once an upstream release (if then).

packages/lsp-ws-connection

jobs:
- template: ci/job.lint.yml
- template: ci/job.test.yml
Expand Down
4 changes: 4 additions & 0 deletions ci/job.test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ parameters:
spec: '>=3.8,<3.9.0a0'
lab: '>=1.2.4,<1.3.0a0'
env_update: conda env update -n jupyterlab-lsp --file env-test.yml --quiet
lab_link: jupyter labextension link --no-build $(LINKED_EXTENSIONS)
lab_ext: jupyter labextension install --no-build $(THIRD_PARTY_LABEXTENSIONS) $(FIRST_PARTY_LABEXTENSIONS)

jobs:
Expand Down Expand Up @@ -79,6 +80,9 @@ jobs:
- script: ${{ platform.activate }} jupyterlab-lsp && python scripts/utest.py --test-run-title="Pytest ${{ platform.name }}${{ python.name }}"
displayName: run python tests

- script: ${{ platform.activate }} jupyterlab-lsp && ${{ parameters.lab_link }} || ${{ parameters.lab_link }} || ${{ parameters.lab_link }}
displayName: install support packages

- script: ${{ platform.activate }} jupyterlab-lsp && ${{ parameters.lab_ext }} || ${{ parameters.lab_ext }} || ${{ parameters.lab_ext }}
displayName: install labextensions

Expand Down
51 changes: 36 additions & 15 deletions packages/jupyterlab-lsp/src/adapters/codemirror/cm_adapter.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,24 @@
import * as CodeMirror from 'codemirror';
import { until_ready } from '../../utils';
import { CodeMirrorHandler, VirtualEditor } from '../../virtual/editor';
import { VirtualEditor } from '../../virtual/editor';
import { VirtualDocument } from '../../virtual/document';
import { IRootPosition } from '../../positioning';
import { ILSPFeature } from './feature';
import { IJupyterLabComponentsManager } from '../jupyterlab/jl_adapter';

export class CodeMirrorAdapter {
features: Map<string, ILSPFeature>;
isDisposed = false;

private last_change: CodeMirror.EditorChange;
private doc_change_handler: CodeMirrorHandler;

constructor(
protected editor: VirtualEditor,
protected virtual_document: VirtualDocument,
protected jupyterlab_components: IJupyterLabComponentsManager,
features = new Array<ILSPFeature>()
) {
this.doc_change_handler = this.saveChange.bind(this);
this.editor.on('change', this.doc_change_handler);
this.editor.on('change', this.saveChange);

this.features = new Map();

Expand All @@ -38,19 +37,28 @@ export class CodeMirrorAdapter {

public async updateAfterChange() {
this.jupyterlab_components.remove_tooltip();
await until_ready(() => this.last_change != null, 30, 22).catch(() => {
this.invalidateLastChange();
throw Error(

try {
await until_ready(() => this.last_change != null, 30, 22);
} catch (err) {
console.log(
'No change obtained from CodeMirror editor within the expected time of 0.66s'
);
});
return;
}

let change: CodeMirror.EditorChange = this.last_change;

let root_position: IRootPosition;

try {
const root_position = this.editor
.getDoc()
.getCursor('end') as IRootPosition;
root_position = this.editor.getDoc().getCursor('end') as IRootPosition;
} catch (err) {
console.log('LSP: Root positon not found');
return;
}

try {
let document = this.editor.document_at_root_position(root_position);

if (this.virtual_document !== document) {
Expand All @@ -77,14 +85,27 @@ export class CodeMirrorAdapter {
this.last_change = null;
}

public saveChange(doc: CodeMirror.Doc, change: CodeMirror.EditorChange) {
public saveChange = (
doc: CodeMirror.Doc,
change: CodeMirror.EditorChange
) => {
this.last_change = change;
}
};

public remove() {
public dispose() {
if (this.isDisposed) {
return;
}
for (let feature of this.features.values()) {
feature.remove();
}
this.editor.off('change', this.doc_change_handler);
this.features.clear();
this.editor.off('change', this.saveChange);

// just to be sure
this.editor = null;

// actually disposed
this.isDisposed = true;
}
}
Loading