Skip to content

Commit 87b0f93

Browse files
krassowskikrassowski
authored
Merge pull request #433 from krassowski/add-highlights-debounce
Add highlights debounce
2 parents 69b3dfb + 2aba360 commit 87b0f93

File tree

7 files changed

+236
-22
lines changed

7 files changed

+236
-22
lines changed

‎CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
## CHANGELOG
22

3+
### `@krassowski/jupyterlab-lsp 2.1.2` (2020-12-XX)
4+
5+
- bug fixes
6+
- improved performance of completion and highlights by minimising the number of highlight requests and GUI redraws (token checking, debouncing, acting on a single response only) ([#433])
7+
- highlights now update after cell focus/blur events even if those do not trigger cursor movement ([#433])
8+
9+
[#433]: https://github.com/krassowski/jupyterlab-lsp/issues/433
10+
311
### `@krassowski/jupyterlab-lsp 2.1.1` (2020-12-15)
412

513
- bug fixes

‎atest/05_Features/Highlights.robot

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
*** Settings ***
2+
Suite Setup Setup Suite For Screenshots highlights
3+
Test Setup Setup Highlights Test
4+
Test Teardown Clean Up After Working With File Highlights.ipynb
5+
Force Tags feature:highlights
6+
Resource ../Keywords.robot
7+
8+
*** Test Cases ***
9+
# cursor is symbolized by pipe (|), for example when
10+
# it is at the end of line, after `1` in `test = 1`
11+
# it is presented as: `test = 1|`
12+
Highlights work at the start of a token
13+
Enter Cell Editor 1 line=1
14+
Press Keys None END # cursor to the end of first line (`test = 1|`)
15+
Should Not Highlight Any Tokens
16+
Press Keys None HOME # cursor before the token (`|test = 1`)
17+
Should Highlight Token test
18+
Should Not Highlight Token gist
19+
20+
Highlights work at the end of a token
21+
Enter Cell Editor 1 line=1
22+
Press Keys None END # cursor to the end of first line (`test = 1|`)
23+
Press Keys None DOWN # cursor to the end of the token in second line (`test`)
24+
Should Highlight Token test
25+
Should Not Highlight Token gist
26+
27+
Highlights are changed when moving cursor between cells
28+
[Documentation] GH431
29+
Enter Cell Editor 1 line=2
30+
Press Keys None END # cursor after the token in second line (`test|`)
31+
Should Highlight Token test
32+
Should Not Highlight Token gist
33+
Press Keys None DOWN # cursor to next cell, which is empty
34+
Should Not Highlight Any Tokens
35+
Press Keys None DOWN # cursor to third cell (`|gist = 1`)
36+
Should Highlight Token gist
37+
Press Keys None DOWN # cursor to third cell, second line (`|test `)
38+
Should Highlight Token test
39+
40+
Highlights are added after typing
41+
Enter Cell Editor 1 line=2
42+
Should Highlight Token test
43+
Press Keys None a
44+
Should Highlight Token testa
45+
46+
*** Keywords ***
47+
Should Not Highlight Any Tokens
48+
Page Should Not Contain css:.cm-lsp-highlight
49+
50+
Should Highlight Token
51+
[Arguments] ${token} ${timeout}=10s
52+
${token_element} Set Variable xpath://span[contains(@class, 'cm-lsp-highlight')][contains(text(), '${token}')]
53+
Wait Until Page Contains Element ${token_element} timeout=${timeout}
54+
55+
Should Not Highlight Token
56+
[Arguments] ${token} ${timeout}=10s
57+
${token_element} Set Variable xpath://span[contains(@class, 'cm-lsp-highlight')][contains(text(), '${token}')]
58+
Wait Until Page Does Not Contain Element ${token_element} timeout=${timeout}
59+
60+
Setup Highlights Test
61+
Setup Notebook Python Highlights.ipynb

‎atest/examples/Highlights.ipynb

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
{
2+
"cells": [
3+
{
4+
"cell_type": "code",
5+
"execution_count": null,
6+
"metadata": {},
7+
"outputs": [],
8+
"source": [
9+
"test = 1\n",
10+
"test"
11+
]
12+
},
13+
{
14+
"cell_type": "code",
15+
"execution_count": null,
16+
"metadata": {},
17+
"outputs": [],
18+
"source": []
19+
},
20+
{
21+
"cell_type": "code",
22+
"execution_count": null,
23+
"metadata": {},
24+
"outputs": [],
25+
"source": [
26+
"gist = 1\n",
27+
"test\n",
28+
"gist\n",
29+
"test"
30+
]
31+
}
32+
],
33+
"metadata": {
34+
"kernelspec": {
35+
"display_name": "Python 3",
36+
"language": "python",
37+
"name": "python3"
38+
},
39+
"language_info": {
40+
"codemirror_mode": {
41+
"name": "ipython",
42+
"version": 3
43+
},
44+
"file_extension": ".py",
45+
"mimetype": "text/x-python",
46+
"name": "python",
47+
"nbconvert_exporter": "python",
48+
"pygments_lexer": "ipython3",
49+
"version": "3.7.5"
50+
}
51+
},
52+
"nbformat": 4,
53+
"nbformat_minor": 4
54+
}

‎packages/jupyterlab-lsp/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,14 @@
2929
},
3030
"scripts": {
3131
"build": "jlpm build:schema && tsc -b",
32-
"build:schema": "jlpm build:schema-backend && jlpm build:schema-completion && jlpm build:schema-hover && jlpm build:schema-diagnostics && jlpm build:schema-syntax_highlighting && jlpm build:schema-jump_to",
32+
"build:schema": "jlpm build:schema-backend && jlpm build:schema-completion && jlpm build:schema-hover && jlpm build:schema-diagnostics && jlpm build:schema-syntax_highlighting && jlpm build:schema-jump_to && jlpm build:schema-highlights",
3333
"build:schema-backend": "json2ts ../../py_src/jupyter_lsp/schema/schema.json --unreachableDefinitions | prettier --stdin-filepath _schema.d.ts > src/_schema.d.ts",
3434
"build:schema-completion": "json2ts schema/completion.json | prettier --stdin-filepath _completion.d.ts > src/_completion.d.ts",
3535
"build:schema-hover": "json2ts schema/hover.json | prettier --stdin-filepath _hover.d.ts > src/_hover.d.ts",
3636
"build:schema-jump_to": "json2ts schema/jump_to.json | prettier --stdin-filepath _jump_to.d.ts > src/_jump_to.d.ts",
3737
"build:schema-diagnostics": "json2ts schema/diagnostics.json | prettier --stdin-filepath _diagnostics.d.ts > src/_diagnostics.d.ts",
3838
"build:schema-syntax_highlighting": "json2ts schema/syntax_highlighting.json | prettier --stdin-filepath _syntax_highlighting.d.ts > src/_syntax_highlighting.d.ts",
39+
"build:schema-highlights": "json2ts schema/highlights.json | prettier --stdin-filepath _highlights.d.ts > src/_highlights.d.ts",
3940
"bundle": "npm pack .",
4041
"clean": "rimraf lib",
4142
"lab:link": "jupyter labextension link . --no-build",

‎packages/jupyterlab-lsp/schema/highlights.json

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
{
2+
"jupyter.lab.setting-icon": "lsp:highlight",
3+
"jupyter.lab.setting-icon-label": "Language integration",
4+
"title": "Code Highlights",
5+
"description": "LSP highlights of the variable under the cursor settings.",
6+
"type": "object",
7+
"properties": {
8+
"debouncerDelay": {
9+
"title": "Debouncer delay",
10+
"type": "number",
11+
"default": 250,
12+
"description": "Number of milliseconds to delay sending out the highlights request to the language server; you can get better responsiveness adjusting this value, but setting it to zero can actually slow it down as the server might get overwhelmed when moving the cursor."
13+
}
14+
},
15+
"jupyter.lab.shortcuts": []
16+
}

‎packages/jupyterlab-lsp/schema/hover.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
"title": "Throttler delay",
1717
"type": "number",
1818
"default": 50,
19-
"description": "Number of milliseconds to delay sending out the request hover request to the language server; you can get better responsiveness adjusting this value, but setting it to zero can actually slow it down as the server might get overwhelmed when moving the mose over the code."
19+
"description": "Number of milliseconds to delay sending out the hover request to the language server; you can get better responsiveness adjusting this value, but setting it to zero can actually slow it down as the server might get overwhelmed when moving the mouse over the code."
2020
},
2121
"cacheSize": {
2222
"title": "Cache size",

‎packages/jupyterlab-lsp/src/features/highlights.ts

Lines changed: 94 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@ import * as CodeMirror from 'codemirror';
22
import * as lsProtocol from 'vscode-languageserver-protocol';
33
import { DocumentHighlightKind } from '../lsp';
44
import { VirtualDocument } from '../virtual/document';
5-
import { IRootPosition } from '../positioning';
6-
import { uris_equal } from '../utils';
5+
import { IRootPosition, IVirtualPosition } from '../positioning';
76
import { FeatureSettings, IFeatureCommand } from '../feature';
87
import { CodeMirrorIntegration } from '../editor_integration/codemirror';
98
import {
@@ -15,6 +14,9 @@ import { ISettingRegistry } from '@jupyterlab/settingregistry';
1514
import { LabIcon } from '@jupyterlab/ui-components';
1615
import highlightSvg from '../../style/icons/highlight.svg';
1716
import highlightTypeSvg from '../../style/icons/highlight-type.svg';
17+
import { Debouncer } from '@lumino/polling';
18+
import { CodeHighlights as LSPHighlightsSettings } from '../_highlights';
19+
import { CodeEditor } from '@jupyterlab/codeeditor';
1820

1921
export const highlightIcon = new LabIcon({
2022
name: 'lsp:highlight',
@@ -47,9 +49,24 @@ const COMMANDS: IFeatureCommand[] = [
4749

4850
export class HighlightsCM extends CodeMirrorIntegration {
4951
protected highlight_markers: CodeMirror.TextMarker[] = [];
52+
private debounced_get_highlight: Debouncer<lsProtocol.DocumentHighlight[]>;
53+
private virtual_position: IVirtualPosition;
54+
private sent_version: number;
55+
private last_token: CodeEditor.IToken;
56+
57+
get settings() {
58+
return super.settings as FeatureSettings<LSPHighlightsSettings>;
59+
}
5060

5161
register(): void {
62+
this.debounced_get_highlight = this.create_debouncer();
63+
64+
this.settings.changed.connect(() => {
65+
this.debounced_get_highlight = this.create_debouncer();
66+
});
5267
this.editor_handlers.set('cursorActivity', this.onCursorActivity);
68+
this.editor_handlers.set('blur', this.onCursorActivity);
69+
this.editor_handlers.set('focus', this.onCursorActivity);
5370
super.register();
5471
}
5572

@@ -67,13 +84,7 @@ export class HighlightsCM extends CodeMirrorIntegration {
6784
this.highlight_markers = [];
6885
}
6986

70-
protected handleHighlight = (
71-
items: lsProtocol.DocumentHighlight[],
72-
documentUri: string
73-
) => {
74-
if (!uris_equal(documentUri, this.virtual_document.document_info.uri)) {
75-
return;
76-
}
87+
protected handleHighlight = (items: lsProtocol.DocumentHighlight[]) => {
7788
this.clear_markers();
7889

7990
if (!items) {
@@ -93,6 +104,22 @@ export class HighlightsCM extends CodeMirrorIntegration {
93104
}
94105
};
95106

107+
protected create_debouncer() {
108+
return new Debouncer<lsProtocol.DocumentHighlight[]>(
109+
this.on_cursor_activity,
110+
this.settings.composite.debouncerDelay
111+
);
112+
}
113+
114+
protected on_cursor_activity = async () => {
115+
this.sent_version = this.virtual_document.document_info.version;
116+
return await this.connection.getDocumentHighlights(
117+
this.virtual_position,
118+
this.virtual_document.document_info,
119+
false
120+
);
121+
};
122+
96123
protected onCursorActivity = async () => {
97124
if (!this.virtual_editor?.virtual_document?.document_info) {
98125
return;
@@ -109,6 +136,22 @@ export class HighlightsCM extends CodeMirrorIntegration {
109136
return;
110137
}
111138

139+
const token = this.virtual_editor.get_token_at(root_position);
140+
141+
// if token has not changed, no need to update highlight, unless it is an empty token
142+
// which would indicate that the cursor is at the first character
143+
if (
144+
this.last_token &&
145+
token.value === this.last_token.value &&
146+
token.value !== ''
147+
) {
148+
console.log(
149+
'LSP: not requesting highlights (token did not change)',
150+
token
151+
);
152+
return;
153+
}
154+
112155
let document: VirtualDocument;
113156
try {
114157
document = this.virtual_editor.document_at_root_position(root_position);
@@ -122,21 +165,52 @@ export class HighlightsCM extends CodeMirrorIntegration {
122165
if (document !== this.virtual_document) {
123166
return;
124167
}
168+
125169
try {
126170
let virtual_position = this.virtual_editor.root_position_to_virtual_position(
127171
root_position
128172
);
129-
const highlights = await this.connection.getDocumentHighlights(
130-
virtual_position,
131-
this.virtual_document.document_info,
132-
false
133-
);
134-
if (!this.virtual_document.isDisposed) {
135-
this.handleHighlight(
136-
highlights,
137-
this.virtual_document.document_info.uri
138-
);
139-
}
173+
174+
this.virtual_position = virtual_position;
175+
176+
Promise.all([
177+
// request the highlights as soon as possible
178+
this.debounced_get_highlight.invoke(),
179+
// and in the meantime remove the old markers
180+
async () => {
181+
this.clear_markers();
182+
this.last_token = null;
183+
}
184+
])
185+
.then(([highlights]) => {
186+
// in the time the response returned the document might have been closed - check that
187+
if (this.virtual_document.isDisposed) {
188+
return;
189+
}
190+
191+
let version_after = this.virtual_document.document_info.version;
192+
193+
/// if document was updated since (e.g. user pressed delete - token change, but position did not)
194+
if (version_after !== this.sent_version) {
195+
console.log(
196+
'LSP: skipping highlights response delayed by ' +
197+
(version_after - this.sent_version) +
198+
' document versions'
199+
);
200+
return;
201+
}
202+
// if cursor position changed (e.g. user moved cursor up - position has changed, but document version did not)
203+
if (virtual_position !== this.virtual_position) {
204+
console.log(
205+
'LSP: skipping highlights response: cursor moved since it was requested'
206+
);
207+
return;
208+
}
209+
210+
this.handleHighlight(highlights);
211+
this.last_token = token;
212+
})
213+
.catch(console.warn);
140214
} catch (e) {
141215
console.warn('Could not get highlights:', e);
142216
}

0 commit comments

Comments
 (0)