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

Add highlights debounce #433

Merged
merged 6 commits into from
Dec 31, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
## CHANGELOG

### `@krassowski/jupyterlab-lsp 2.1.2` (2020-12-XX)

- bug fixes
- 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])
- highlights now update after cell focus/blur events even if those do not trigger cursor movement ([#433])

[#433]: https://github.com/krassowski/jupyterlab-lsp/issues/433

### `@krassowski/jupyterlab-lsp 2.1.1` (2020-12-15)

- bug fixes
Expand Down
61 changes: 61 additions & 0 deletions atest/05_Features/Highlights.robot
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
*** Settings ***
Suite Setup Setup Suite For Screenshots highlights
Test Setup Setup Highlights Test
Test Teardown Clean Up After Working With File Highlights.ipynb
Force Tags feature:highlights
Resource ../Keywords.robot

*** Test Cases ***
# cursor is symbolized by pipe (|), for example when
# it is at the end of line, after `1` in `test = 1`
# it is presented as: `test = 1|`
Highlights work at the start of a token
Enter Cell Editor 1 line=1
Press Keys None END # cursor to the end of first line (`test = 1|`)
Should Not Highlight Any Tokens
Press Keys None HOME # cursor before the token (`|test = 1`)
Should Highlight Token test
Should Not Highlight Token gist

Highlights work at the end of a token
Enter Cell Editor 1 line=1
Press Keys None END # cursor to the end of first line (`test = 1|`)
Press Keys None DOWN # cursor to the end of the token in second line (`test`)
Should Highlight Token test
Should Not Highlight Token gist

Highlights are changed when moving cursor between cells
[Documentation] GH431
Enter Cell Editor 1 line=2
Press Keys None END # cursor after the token in second line (`test|`)
Should Highlight Token test
Should Not Highlight Token gist
Press Keys None DOWN # cursor to next cell, which is empty
Should Not Highlight Any Tokens
Press Keys None DOWN # cursor to third cell (`|gist = 1`)
Should Highlight Token gist
Press Keys None DOWN # cursor to third cell, second line (`|test `)
Should Highlight Token test

Highlights are added after typing
Enter Cell Editor 1 line=2
Should Highlight Token test
Press Keys None a
Should Highlight Token testa

*** Keywords ***
Should Not Highlight Any Tokens
Page Should Not Contain css:.cm-lsp-highlight

Should Highlight Token
[Arguments] ${token} ${timeout}=10s
${token_element} Set Variable xpath://span[contains(@class, 'cm-lsp-highlight')][contains(text(), '${token}')]
Wait Until Page Contains Element ${token_element} timeout=${timeout}

Should Not Highlight Token
[Arguments] ${token} ${timeout}=10s
${token_element} Set Variable xpath://span[contains(@class, 'cm-lsp-highlight')][contains(text(), '${token}')]
Wait Until Page Does Not Contain Element ${token_element} timeout=${timeout}

Setup Highlights Test
Setup Notebook Python Highlights.ipynb
54 changes: 54 additions & 0 deletions atest/examples/Highlights.ipynb
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
{
"cells": [
{
"cell_type": "code",
"execution_count": null,
"metadata": {},
"outputs": [],
"source": [
"test = 1\n",
"test"
]
},
{
"cell_type": "code",
"execution_count": null,
"metadata": {},
"outputs": [],
"source": []
},
{
"cell_type": "code",
"execution_count": null,
"metadata": {},
"outputs": [],
"source": [
"gist = 1\n",
"test\n",
"gist\n",
"test"
]
}
],
"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.7.5"
}
},
"nbformat": 4,
"nbformat_minor": 4
}
3 changes: 2 additions & 1 deletion packages/jupyterlab-lsp/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,14 @@
},
"scripts": {
"build": "jlpm build:schema && tsc -b",
"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",
"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",
"build:schema-backend": "json2ts ../../py_src/jupyter_lsp/schema/schema.json --unreachableDefinitions | prettier --stdin-filepath _schema.d.ts > src/_schema.d.ts",
"build:schema-completion": "json2ts schema/completion.json | prettier --stdin-filepath _completion.d.ts > src/_completion.d.ts",
"build:schema-hover": "json2ts schema/hover.json | prettier --stdin-filepath _hover.d.ts > src/_hover.d.ts",
"build:schema-jump_to": "json2ts schema/jump_to.json | prettier --stdin-filepath _jump_to.d.ts > src/_jump_to.d.ts",
"build:schema-diagnostics": "json2ts schema/diagnostics.json | prettier --stdin-filepath _diagnostics.d.ts > src/_diagnostics.d.ts",
"build:schema-syntax_highlighting": "json2ts schema/syntax_highlighting.json | prettier --stdin-filepath _syntax_highlighting.d.ts > src/_syntax_highlighting.d.ts",
"build:schema-highlights": "json2ts schema/highlights.json | prettier --stdin-filepath _highlights.d.ts > src/_highlights.d.ts",
"bundle": "npm pack .",
"clean": "rimraf lib",
"lab:link": "jupyter labextension link . --no-build",
Expand Down
16 changes: 16 additions & 0 deletions packages/jupyterlab-lsp/schema/highlights.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"jupyter.lab.setting-icon": "lsp:highlight",
"jupyter.lab.setting-icon-label": "Language integration",
"title": "Code Highlights",
"description": "LSP highlights of the variable under the cursor settings.",
"type": "object",
"properties": {
"debouncerDelay": {
"title": "Debouncer delay",
"type": "number",
"default": 250,
"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."
}
},
"jupyter.lab.shortcuts": []
}
2 changes: 1 addition & 1 deletion packages/jupyterlab-lsp/schema/hover.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"title": "Throttler delay",
"type": "number",
"default": 50,
"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."
"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."
},
"cacheSize": {
"title": "Cache size",
Expand Down
114 changes: 94 additions & 20 deletions packages/jupyterlab-lsp/src/features/highlights.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ import * as CodeMirror from 'codemirror';
import * as lsProtocol from 'vscode-languageserver-protocol';
import { DocumentHighlightKind } from '../lsp';
import { VirtualDocument } from '../virtual/document';
import { IRootPosition } from '../positioning';
import { uris_equal } from '../utils';
import { IRootPosition, IVirtualPosition } from '../positioning';
import { FeatureSettings, IFeatureCommand } from '../feature';
import { CodeMirrorIntegration } from '../editor_integration/codemirror';
import {
Expand All @@ -15,6 +14,9 @@ import { ISettingRegistry } from '@jupyterlab/settingregistry';
import { LabIcon } from '@jupyterlab/ui-components';
import highlightSvg from '../../style/icons/highlight.svg';
import highlightTypeSvg from '../../style/icons/highlight-type.svg';
import { Debouncer } from '@lumino/polling';
import { CodeHighlights as LSPHighlightsSettings } from '../_highlights';
import { CodeEditor } from '@jupyterlab/codeeditor';

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

export class HighlightsCM extends CodeMirrorIntegration {
protected highlight_markers: CodeMirror.TextMarker[] = [];
private debounced_get_highlight: Debouncer<lsProtocol.DocumentHighlight[]>;
private virtual_position: IVirtualPosition;
private sent_version: number;
private last_token: CodeEditor.IToken;

get settings() {
return super.settings as FeatureSettings<LSPHighlightsSettings>;
}

register(): void {
this.debounced_get_highlight = this.create_debouncer();

this.settings.changed.connect(() => {
this.debounced_get_highlight = this.create_debouncer();
});
this.editor_handlers.set('cursorActivity', this.onCursorActivity);
this.editor_handlers.set('blur', this.onCursorActivity);
this.editor_handlers.set('focus', this.onCursorActivity);
super.register();
}

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

protected handleHighlight = (
items: lsProtocol.DocumentHighlight[],
documentUri: string
) => {
if (!uris_equal(documentUri, this.virtual_document.document_info.uri)) {
return;
}
protected handleHighlight = (items: lsProtocol.DocumentHighlight[]) => {
this.clear_markers();

if (!items) {
Expand All @@ -93,6 +104,22 @@ export class HighlightsCM extends CodeMirrorIntegration {
}
};

protected create_debouncer() {
return new Debouncer<lsProtocol.DocumentHighlight[]>(
this.on_cursor_activity,
this.settings.composite.debouncerDelay
);
}

protected on_cursor_activity = async () => {
this.sent_version = this.virtual_document.document_info.version;
return await this.connection.getDocumentHighlights(
this.virtual_position,
this.virtual_document.document_info,
false
);
};

protected onCursorActivity = async () => {
if (!this.virtual_editor?.virtual_document?.document_info) {
return;
Expand All @@ -109,6 +136,22 @@ export class HighlightsCM extends CodeMirrorIntegration {
return;
}

const token = this.virtual_editor.get_token_at(root_position);

// if token has not changed, no need to update highlight, unless it is an empty token
// which would indicate that the cursor is at the first character
if (
this.last_token &&
token.value === this.last_token.value &&
token.value !== ''
) {
console.log(
'LSP: not requesting highlights (token did not change)',
token
);
return;
}

let document: VirtualDocument;
try {
document = this.virtual_editor.document_at_root_position(root_position);
Expand All @@ -122,21 +165,52 @@ export class HighlightsCM extends CodeMirrorIntegration {
if (document !== this.virtual_document) {
return;
}

try {
let virtual_position = this.virtual_editor.root_position_to_virtual_position(
root_position
);
const highlights = await this.connection.getDocumentHighlights(
virtual_position,
this.virtual_document.document_info,
false
);
if (!this.virtual_document.isDisposed) {
this.handleHighlight(
highlights,
this.virtual_document.document_info.uri
);
}

this.virtual_position = virtual_position;

Promise.all([
// request the highlights as soon as possible
this.debounced_get_highlight.invoke(),
// and in the meantime remove the old markers
async () => {
this.clear_markers();
this.last_token = null;
}
])
.then(([highlights]) => {
// in the time the response returned the document might have been closed - check that
if (this.virtual_document.isDisposed) {
return;
}

let version_after = this.virtual_document.document_info.version;

/// if document was updated since (e.g. user pressed delete - token change, but position did not)
if (version_after !== this.sent_version) {
console.log(
'LSP: skipping highlights response delayed by ' +
(version_after - this.sent_version) +
' document versions'
);
return;
}
// if cursor position changed (e.g. user moved cursor up - position has changed, but document version did not)
if (virtual_position !== this.virtual_position) {
console.log(
'LSP: skipping highlights response: cursor moved since it was requested'
);
return;
}

this.handleHighlight(highlights);
this.last_token = token;
})
.catch(console.warn);
} catch (e) {
console.warn('Could not get highlights:', e);
}
Expand Down