From 1a679c9c7cb6efbaded948817120c6967ef47aa2 Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Sun, 27 Aug 2023 18:23:55 +0100 Subject: [PATCH 01/22] Fix jump to in notebooks, fix diagnostics and jump tests --- atest/01_Editor.robot | 28 ++++++------ atest/03_Notebook.robot | 13 +++--- atest/04_Interface/DiagnosticsPanel.robot | 6 +-- atest/04_Interface/Statusbar.robot | 2 +- atest/05_Features/Diagnostics.robot | 4 +- atest/07_Configuration.robot | 8 ++-- atest/Keywords.resource | 24 ++++++---- atest/Variables.resource | 7 +-- atest/diagnostics.py | 45 +++++++++++++++++++ .../jupyterlab-lsp/src/features/jump_to.ts | 15 ++++++- .../jupyterlab-lsp/src/features/signature.ts | 5 --- 11 files changed, 107 insertions(+), 50 deletions(-) create mode 100644 atest/diagnostics.py diff --git a/atest/01_Editor.robot b/atest/01_Editor.robot index b2d1e3b65..7228572d7 100644 --- a/atest/01_Editor.robot +++ b/atest/01_Editor.robot @@ -17,12 +17,12 @@ Bash CSS ${def} = Set Variable - ... xpath:(//span[contains(@class, 'cm-variable-2')][contains(text(), '--some-var')])[last()] + ... xpath:(//span[contains(@class, 'cm-line')][contains(text(), '--some-var')])[last()] Editor Shows Features for Language CSS example.css Diagnostics=Do not use empty rulesets ... Jump to Definition=${def} Rename=${def} Docker - ${def} = Set Variable xpath://span[contains(@class, 'cm-string')][contains(text(), 'PLANET')] + ${def} = Set Variable xpath://span[contains(@class, 'cm-line')][contains(text(), 'PLANET')] Wait Until Keyword Succeeds 3x 100ms Editor Shows Features for Language Docker Dockerfile ... Diagnostics=Instructions should be written in uppercase letters Jump to Definition=${def} ... Rename=${def} @@ -36,20 +36,20 @@ JSON Editor Shows Features for Language JSON example.json Diagnostics=Duplicate object key JSX - ${def} = Set Variable xpath:(//span[contains(@class, 'cm-variable')][contains(text(), 'hello')])[last()] + ${def} = Set Variable xpath:(//span[contains(@class, 'cm-line')][contains(text(), 'hello')])[last()] Editor Shows Features for Language JSX example.jsx Diagnostics=Expression expected ... Jump to Definition=${def} Rename=${def} # Julia -# ${def} = Set Variable xpath:(//span[contains(@class, 'cm-builtin')][contains(text(), 'add_together')])[last()] +# ${def} = Set Variable xpath:(//span[contains(@class, 'cm-line')][contains(text(), 'add_together')])[last()] # Editor Shows Features for Language Julia example.jl Jump to Definition=${def} Rename=${def} LaTeX [Tags] language:latex - ${def} = Set Variable xpath:(//span[contains(@class, 'cm-atom')][contains(text(), 'foo')])[last()] + ${def} = Set Variable xpath:(//span[contains(@class, 'cm-line')][contains(text(), 'foo')])[last()] Editor Shows Features for Language LaTeX example.tex Jump to Definition=${def} Rename=${def} Less - ${def} = Set Variable xpath:(//span[contains(@class, 'cm-variable-2')][contains(text(), '@width')])[last()] + ${def} = Set Variable xpath:(//span[contains(@class, 'cm-line')][contains(text(), '@width')])[last()] Editor Shows Features for Language Less example.less Diagnostics=Do not use empty rulesets ... Jump to Definition=${def} @@ -57,7 +57,7 @@ Markdown Editor Shows Features for Language Markdown example.md Diagnostics=`Color` is misspelt Python (pylsp) - ${def} = Set Variable xpath:(//span[contains(@class, 'cm-variable')][contains(text(), 'fib')])[last()] + ${def} = Set Variable xpath:(//span[contains(@class, 'cm-line')][contains(text(), 'fib')])[last()] Editor Shows Features for Server ... pylsp ... Python @@ -67,35 +67,35 @@ Python (pylsp) ... Rename=${def} Python (pyright) - ${def} = Set Variable xpath:(//span[contains(@class, 'cm-variable')][contains(text(), 'fib')])[last()] + ${def} = Set Variable xpath:(//div[contains(@class, 'cm-line')][contains(text(), 'fib')])[last()] Editor Shows Features for Server pyright Python example.py Diagnostics=is not defined (Pyright) ... Jump to Definition=${def} R - ${def} = Set Variable xpath:(//span[contains(@class, 'cm-variable')][contains(text(), 'fib')])[last()] + ${def} = Set Variable xpath:(//span[contains(@class, 'cm-line')][contains(text(), 'fib')])[last()] Editor Shows Features for Language R example.R Diagnostics=Put spaces around all infix operators ... Jump to Definition=${def} Robot Framework [Tags] gh:332 ${def} = Set Variable - ... xpath:(//span[contains(@class, 'cm-keyword')][contains(text(), 'Special Log')])[last()] + ... xpath:(//span[contains(@class, 'cm-line')][contains(text(), 'Special Log')])[last()] Editor Shows Features for Language Robot Framework example.robot Diagnostics=Undefined keyword ... Jump to Definition=${def} SCSS ${def} = Set Variable - ... xpath:(//span[contains(@class, 'cm-variable-2')][contains(text(), 'primary-color')])[last()] + ... xpath:(//span[contains(@class, 'cm-line')][contains(text(), 'primary-color')])[last()] Editor Shows Features for Language SCSS example.scss Diagnostics=Do not use empty rulesets ... Jump to Definition=${def} TSX - ${def} = Set Variable xpath:(//span[contains(@class, 'cm-tag')][contains(text(), 'HelloWorld')])[last()] + ${def} = Set Variable xpath:(//span[contains(@class, 'cm-line')][contains(text(), 'HelloWorld')])[last()] Editor Shows Features for Language TSX example.tsx ... Diagnostics='hello' is declared but its value is never read. Jump to Definition=${def} Rename=${def} TypeScript - ${def} = Set Variable xpath:(//span[contains(@class, 'cm-variable')][contains(text(), 'inc')])[last()] + ${def} = Set Variable xpath:(//span[contains(@class, 'cm-line')][contains(text(), 'inc')])[last()] Editor Shows Features for Language TypeScript example.ts Diagnostics=The left-hand side of an arithmetic ... Jump to Definition=${def} Rename=${def} @@ -137,7 +137,7 @@ Editor Shows Features for Language Editor Should Show Diagnostics [Arguments] ${diagnostic} Set Tags feature:diagnostics - Wait Until Page Contains Element css:.cm-lsp-diagnostic[title*="${diagnostic}"] timeout=25s + Wait Until Page Contains Diagnostic [title*="${diagnostic}"] timeout=25s Capture Page Screenshot 01-diagnostics.png Open Diagnostics Panel Capture Page Screenshot 02-diagnostics.png diff --git a/atest/03_Notebook.robot b/atest/03_Notebook.robot index a2e4e87eb..b6325056f 100644 --- a/atest/03_Notebook.robot +++ b/atest/03_Notebook.robot @@ -9,8 +9,7 @@ Test Setup Try to Close All Tabs Python [Setup] Setup Notebook Python Python.ipynb ${diagnostic} = Set Variable W291 trailing whitespace (pycodestyle) - # TODO: no title for diagnostics; we can get the title with JS via `element.cmView.mark.spec.diagnostic.message` but this is not selectable - Wait Until Page Contains Element css:.cm-lintRange[title="${diagnostic}"] timeout=35s + Wait Until Page Contains Diagnostic [title="${diagnostic}"] timeout=35s Capture Page Screenshot 01-python.png [Teardown] Clean Up After Working With File Python.ipynb @@ -40,11 +39,11 @@ Moving Cells Around [Setup] Setup Notebook Python Python.ipynb ${diagnostic} = Set Variable undefined name 'test' (pyflakes) Enter Cell Editor 1 - Lab Command Move Cells Down - Wait Until Page Contains Element css:.cm-lsp-diagnostic[title="${diagnostic}"] timeout=35s + Lab Command Move Cell Down + Wait Until Page Contains Diagnostic [title="${diagnostic}"] timeout=35s Enter Cell Editor 1 - Lab Command Move Cells Down - Wait Until Page Does Not Contain Element css:.cm-lsp-diagnostic[title="${diagnostic}"] timeout=35s + Lab Command Move Cell Down + Wait Until Page Does Not Contain Diagnostic [title="${diagnostic}"] timeout=35s [Teardown] Clean Up After Working With File Python.ipynb Foreign Extractors @@ -61,7 +60,7 @@ Foreign Extractors ... `frob` is misspelt # markdown ... Command terminated with space # latex FOR ${diagnostic} IN @{diagnostics} - Wait Until Page Contains Element css:.cm-lsp-diagnostic[title*\="${diagnostic}"] timeout=35s + Wait Until Page Contains Diagnostic [title*\="${diagnostic}"] timeout=35s END Capture Page Screenshot 11-extracted.png [Teardown] Clean Up After Working with File and Settings ${file} diff --git a/atest/04_Interface/DiagnosticsPanel.robot b/atest/04_Interface/DiagnosticsPanel.robot index b71078775..25fbb3975 100644 --- a/atest/04_Interface/DiagnosticsPanel.robot +++ b/atest/04_Interface/DiagnosticsPanel.robot @@ -26,7 +26,7 @@ Diagnostics Panel Works After Rename [Documentation] Test for #141 bug (diagnostics were not cleared after rename) Rename Jupyter File Panel.ipynb PanelRenamed.ipynb Close Diagnostics Panel - Wait Until Page Contains Element css:.cm-lsp-diagnostic[title*="${DIAGNOSTIC}"] timeout=20s + Wait Until Page Contains Diagnostic [title*="${DIAGNOSTIC}"] timeout=20s Capture Page Screenshot 00-panel-rename.png Open Diagnostics Panel Capture Page Screenshot 01-panel-rename.png @@ -39,7 +39,7 @@ Diagnostics Panel Works After Kernel Restart Lab Command Restart Kernel… Wait For Dialog Accept Default Dialog Option - Wait Until Page Contains Element css:.cm-lsp-diagnostic[title*="${DIAGNOSTIC}"] timeout=20s + Wait Until Page Contains Diagnostic [title*="${DIAGNOSTIC}"] timeout=20s Open Diagnostics Panel Wait Until Keyword Succeeds 10 x 1s Should Have Expected Rows Count ${EXPECTED_COUNT} @@ -135,7 +135,7 @@ Open Notebook And Panel [Arguments] ${notebook} Setup Notebook Python ${notebook} Capture Page Screenshot 00-notebook-and-panel-openeing.png - Wait Until Page Contains Element css:.cm-lsp-diagnostic[title*="${DIAGNOSTIC}"] timeout=20s + Wait Until Page Contains Diagnostic [title*="${DIAGNOSTIC}"] timeout=20s Open Diagnostics Panel Capture Page Screenshot 00-notebook-and-panel-opened.png diff --git a/atest/04_Interface/Statusbar.robot b/atest/04_Interface/Statusbar.robot index 403b6b3c8..0780f6dd4 100644 --- a/atest/04_Interface/Statusbar.robot +++ b/atest/04_Interface/Statusbar.robot @@ -14,7 +14,7 @@ ${HELP_BUTTON} css:.lsp-popover .lsp-help-button *** Test Cases *** Statusbar Popup Opens Setup Notebook Python Python.ipynb - Wait Until Page Contains Element css:.cm-lsp-diagnostic[title*="${DIAGNOSTIC}"] timeout=20s + Wait Until Page Contains Diagnostic [title*="${DIAGNOSTIC}"] timeout=20s Element Should Contain ${STATUSBAR} Fully initialized Click Element ${STATUSBAR} Wait Until Page Contains Element ${POPOVER} timeout=10s diff --git a/atest/05_Features/Diagnostics.robot b/atest/05_Features/Diagnostics.robot index b61517b2d..2a3c139b7 100644 --- a/atest/05_Features/Diagnostics.robot +++ b/atest/05_Features/Diagnostics.robot @@ -11,7 +11,7 @@ Test Tags feature:diagnostics *** Test Cases *** Diagnostics with deprecated tag have strike-through decoration - Wait Until Page Contains Element css:.cm-lsp-diagnostic[title*="is deprecated"] timeout=25s - Wait Until Page Contains Element css:.cm-lsp-diagnostic[title*="Unreachable code"] timeout=5s + Wait Until Page Contains Diagnostic [title*="is deprecated"] timeout=25s + Wait Until Page Contains Diagnostic [title*="Unreachable code"] timeout=5s Page Should Contain Element css:.cm-lsp-diagnostic-tag-Deprecated Page Should Contain Element css:.cm-lsp-diagnostic-tag-Unnecessary diff --git a/atest/07_Configuration.robot b/atest/07_Configuration.robot index ac59ef44c..12e1d7057 100644 --- a/atest/07_Configuration.robot +++ b/atest/07_Configuration.robot @@ -66,8 +66,8 @@ LaTeX *** Keywords *** Settings Should Change Editor Diagnostics [Arguments] ${language} ${file} ${server} ${settings} ${before} ${after} ${save command}=${EMPTY} ${needs reload}=${False} ${setting_key}=serverSettings - ${before diagnostic} = Set Variable ${CSS DIAGNOSTIC}\[title*="${before}"] - ${after diagnostic} = Set Variable ${CSS DIAGNOSTIC}\[title*="${after}"] + ${before diagnostic} = Set Variable [title*="${before}"] + ${after diagnostic} = Set Variable [title*="${after}"] ${tab} = Set Variable ${JLAB XP DOCK TAB}\[contains(., '${file}')] ${close icon} = Set Variable *[contains(@class, 'm-TabBar-tabCloseIcon')] ${save command} = Set Variable If "${save command}" ${save command} Save ${language} File @@ -95,9 +95,9 @@ Settings Should Change Editor Diagnostics IF ${needs reload} Reload After Configuration ${language} ${file} # allow longer after reload - Wait Until Page Contains Element ${after diagnostic} timeout=60s + Wait Until Page Contains Diagnostic ${after diagnostic} timeout=60s ELSE - Wait Until Page Contains Element ${after diagnostic} timeout=30s + Wait Until Page Contains Diagnostic ${after diagnostic} timeout=30s END Capture Page Screenshot 04-configured-diagnostic-found.png [Teardown] Clean Up After Working with File and Settings ${file} diff --git a/atest/Keywords.resource b/atest/Keywords.resource index a11b4b217..a02563c19 100644 --- a/atest/Keywords.resource +++ b/atest/Keywords.resource @@ -9,6 +9,7 @@ Library ./logcheck.py Library ./ports.py Library ./config.py Library ./paths.py +Library ./diagnostics.py *** Keywords *** @@ -334,7 +335,7 @@ Place Cursor In Cell Editor At Enter Cell Editor ${cell_nr} ${line} Execute JavaScript ... var view = document.querySelector('.jp-Cell:nth-child(${cell_nr}) .cm-content').cmView.view; - ... var pos = view.doc.line(${line}).from + ${character}; + ... var pos = view.state.doc.line(${line}).from + ${character}; ... return view.dispatch({selection: {anchor: pos, head: pos}}); Enter File Editor @@ -346,7 +347,7 @@ Place Cursor In File Editor At Enter File Editor Execute JavaScript ... var view = document.querySelector('.jp-FileEditor .cm-content').cmView.view; - ... var pos = view.doc.line(${line}).from + ${character}; + ... var pos = view.state.doc.line(${line}).from + ${character}; ... return view.dispatch({selection: {anchor: pos, head: pos}}); Wait Until Fully Initialized @@ -404,13 +405,16 @@ Set Editor Content [Arguments] ${text} ${css}=${EMPTY} Wait Until Page Contains Element css:${css} .cm-content Execute JavaScript - ... var doc = document.querySelector('${css} .cm-content').cmView.view.doc; - ... return doc.replace(0, doc.length, `${text}`); + ... let view = document.querySelector('${css} .cm-content').cmView.view; + ... view.dispatch({ + ... changes: {from: 0, to: view.state.doc.length, insert: `${text}`} + ... }); Get Editor Content [Arguments] ${css}=${EMPTY} Wait Until Page Contains Element css:${css} .cm-content - ${content} = Execute JavaScript return document.querySelector('${css} .cm-content').cmView.view.doc.toString() + ${content} = Execute JavaScript + ... return document.querySelector('${css} .cm-content').cmView.view.state.doc.toString() RETURN ${content} Configure JupyterLab Plugin @@ -431,8 +435,9 @@ Jump To Definition [Arguments] ${symbol} ${sel} = Set Variable If "${symbol}".startswith(("xpath", "css")) ${symbol} ... xpath:(//span[@role="presentation"][contains(., "${symbol}")])[last()] - Open Context Menu Over ${sel} + Click Element ${sel} ${cursor} = Measure Cursor Position + Open Context Menu Over ${sel} Capture Page Screenshot 02-jump-to-definition-0.png Wait Until Element Is Visible ${MENU JUMP} timeout=5s Mouse Over ${MENU JUMP} @@ -453,8 +458,11 @@ Cursor Should Jump Should Not Be Equal ${original} ${current} Measure Cursor Position - Wait Until Page Contains Element ${CM CURSORS} - ${position} = Wait Until Keyword Succeeds 20 x 0.05s Get Vertical Position ${CM CURSOR} + Wait Until Page Contains Element ${ACTIVE CURSOR} + ${position} = Wait Until Keyword Succeeds 20 x 0.05s Get Vertical Position ${ACTIVE CURSOR} + LOG ${position} + # position = 0 indicates that the element was not active. + Should Not Be Equal ${position} 0 RETURN ${position} Switch To Tab diff --git a/atest/Variables.resource b/atest/Variables.resource index a0bfd2e85..57619f06a 100644 --- a/atest/Variables.resource +++ b/atest/Variables.resource @@ -49,9 +49,8 @@ ${MENU SETTINGS} xpath://div[contains(@class, 'lm-MenuBar-itemLab ${MENU EDITOR THEME} ... xpath://div[contains(@class, 'lm-Menu-itemLabel')][contains(text(), "Text Editor Theme")] ${LAB MENU} css:.lm-Menu -${CM CURSOR} css:.cm-cursor -${CM CURSORS} -... css:.jp-MainAreaWidget:not(.lm-mod-hidden) .cm-cursorLayer:not([style='visibility: hidden']) +${ACTIVE CURSOR} +... css:.jp-MainAreaWidget:not(.lm-mod-hidden) .cm-cursorLayer:not([style='visibility: hidden']) .cm-cursor-primary # settings ${LSP PLUGIN ID} @jupyter-lsp/jupyterlab-lsp:plugin ${COMPLETION PLUGIN ID} @jupyter-lsp/jupyterlab-lsp:completion @@ -62,8 +61,6 @@ ${HOVER PLUGIN ID} @jupyter-lsp/jupyterlab-lsp:hover ${CSS USER SETTINGS} .jp-SettingsRawEditor-user ${JLAB XP CLOSE SETTINGS} ... ${JLAB XP DOCK TAB}\[contains(., 'Settings')]/*[contains(@class, 'm-TabBar-tabCloseIcon')] -# diagnostics -${CSS DIAGNOSTIC} css:.cm-lintRange # log messages @{KNOWN BAD ERRORS} ... pylsp_jsonrpc.endpoint - Failed to handle notification diff --git a/atest/diagnostics.py b/atest/diagnostics.py new file mode 100644 index 000000000..5b1a0e9c1 --- /dev/null +++ b/atest/diagnostics.py @@ -0,0 +1,45 @@ +from functools import partial +from robot.libraries.BuiltIn import BuiltIn +from selenium.common.exceptions import NoSuchElementException +from selenium.webdriver.support.wait import WebDriverWait +from selenium.webdriver.common.by import By +from selenium.webdriver.remote.webdriver import WebDriver +from SeleniumLibrary import SeleniumLibrary + +from robot.utils import timestr_to_secs + +DIAGNOSTIC_CLASS = 'cm-lintRange' + +def page_contains_diagnostic(driver: WebDriver, selector, negate=False): + elements = driver.find_elements(By.CSS_SELECTOR, f'.{DIAGNOSTIC_CLASS}') + if not elements: + return True if negate else False + driver.execute_script(""" + arguments[0].map(el => { + let diagnostic = el.cmView.mark.spec.diagnostic; + el.title = diagnostic.message + " (" + diagnostic.source + ")"; + }); + """, elements) + try: + driver.find_element(By.CSS_SELECTOR, f'.{DIAGNOSTIC_CLASS}{selector}') + except NoSuchElementException: + return True if negate else False + return False if negate else True + + +def wait_until_page_contains_diagnostic(selector, timeout='3s'): + sl: SeleniumLibrary = BuiltIn().get_library_instance("SeleniumLibrary") + wait = WebDriverWait(sl.driver, timestr_to_secs(timeout)) + return wait.until( + partial(page_contains_diagnostic, selector=selector), + f'Diagnostic with selector {selector} not found in {timeout}' + ) + + +def wait_until_page_does_not_contain_diagnostic(selector, timeout='3s'): + sl: SeleniumLibrary = BuiltIn().get_library_instance("SeleniumLibrary") + wait = WebDriverWait(sl.driver, timestr_to_secs(timeout)) + return wait.until( + partial(page_contains_diagnostic, selector=selector, negate=True), + f'Diagnostic with selector {selector} still present after {timeout}' + ) diff --git a/packages/jupyterlab-lsp/src/features/jump_to.ts b/packages/jupyterlab-lsp/src/features/jump_to.ts index e4691cef1..a871647cd 100644 --- a/packages/jupyterlab-lsp/src/features/jump_to.ts +++ b/packages/jupyterlab-lsp/src/features/jump_to.ts @@ -46,6 +46,7 @@ import { ContextAssembler } from '../context'; import { PositionConverter, documentAtRootPosition, + editorAtRootPosition, rootPositionToVirtualPosition, rootPositionToEditorPosition, virtualPositionToRootPosition @@ -367,7 +368,6 @@ export class NavigationFeature extends Feature { ) as IVirtualPosition; if (urisEqual(uri, positionParams.textDocument.uri)) { - let editorIndex = adapter.getEditorIndexAt(virtualPosition); // if in current file, transform from the position within virtual document to the editor position: const rootPosition = virtualPositionToRootPosition( adapter, @@ -385,6 +385,19 @@ export class NavigationFeature extends Feature { rootPosition ); + const editorAccessor = editorAtRootPosition(adapter, rootPosition); + + // TODO: getEditorIndex should work, but does not + // adapter.getEditorIndex(editorAccessor) + await editorAccessor.reveal(); + const editor = editorAccessor.getEditor(); + const editorIndex = adapter.editors.findIndex( + e => e.ceEditor.getEditor() === editor + ); + if (editorIndex === -1) { + return JumpResult.PositioningFailure; + } + this.console.log(`Jumping to ${editorIndex}th editor of ${uri}`); this.console.log('Jump target within editor:', editorPosition); diff --git a/packages/jupyterlab-lsp/src/features/signature.ts b/packages/jupyterlab-lsp/src/features/signature.ts index f9a969c3e..8d6830761 100644 --- a/packages/jupyterlab-lsp/src/features/signature.ts +++ b/packages/jupyterlab-lsp/src/features/signature.ts @@ -544,14 +544,9 @@ export class SignatureFeature extends Feature { return; } - //let editorPosition = - // virtualDocument.transformVirtualToEditor(virtualPosition); - const editorLanguage = this.languageRegistry.findByMIME( editor.model.mimeType ); - // TODO: restore language probing - // let language = cm_editor.getModeAt(editorPosition).name; const language = editorLanguage?.support?.language; let markup = this.getMarkupForSignatureHelp(response, language); From 15267faf5181ea26fc8dbda9838c956ae6e316b4 Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Sun, 27 Aug 2023 22:19:09 +0100 Subject: [PATCH 02/22] Fix context when executed over non-active cell for hover, rename and jump to --- packages/jupyterlab-lsp/src/context.ts | 87 ++++++++++++++----- packages/jupyterlab-lsp/src/features/hover.ts | 32 +++++-- .../jupyterlab-lsp/src/features/jump_to.ts | 16 +++- 3 files changed, 104 insertions(+), 31 deletions(-) diff --git a/packages/jupyterlab-lsp/src/context.ts b/packages/jupyterlab-lsp/src/context.ts index e8e145fc5..f97ff2237 100644 --- a/packages/jupyterlab-lsp/src/context.ts +++ b/packages/jupyterlab-lsp/src/context.ts @@ -1,5 +1,7 @@ +import type { EditorView } from '@codemirror/view'; import { JupyterFrontEnd } from '@jupyterlab/application'; import { CodeEditor } from '@jupyterlab/codeeditor'; +import type { CodeMirrorEditor } from '@jupyterlab/codemirror'; import { IDocumentWidget } from '@jupyterlab/docregistry'; import { ILSPConnection, @@ -16,6 +18,7 @@ import { documentAtRootPosition, rootPositionToVirtualPosition, editorPositionToRootPosition, + editorAtRootPosition, rootPositionToEditorPosition } from './converter'; import { BrowserConsole } from './virtual/console'; @@ -29,22 +32,19 @@ export class ContextAssembler { // no-op } - get isContextMenuOpen(): boolean { - return this.options.app.contextMenu.menu.isAttached; - } - + /** + * Get context from current context menu (or fallback to document context). + */ getContext(): ICommandContext | null { let context: ICommandContext | null = null; - if (this.isContextMenuOpen) { - try { - context = this.getContextFromContextMenu(); - } catch (e) { - this.console.warn( - 'contextMenu is attached, but could not get the context', - e - ); - context = null; - } + try { + context = this.getContextFromContextMenu(); + } catch (e) { + this.console.warn( + 'contextMenu is attached, but could not get the context', + e + ); + context = null; } if (context == null) { try { @@ -74,7 +74,8 @@ export class ContextAssembler { return this._wasOverToken; } const { rootPosition, adapter } = context; - const editor = adapter.activeEditor?.getEditor(); + const editorAccessor = editorAtRootPosition(adapter, rootPosition); + const editor = editorAccessor.getEditor(); if (!editor) { return; } @@ -158,15 +159,9 @@ export class ContextAssembler { positionFromCoordinates( left: number, top: number, - adapter: WidgetLSPAdapter + adapter: WidgetLSPAdapter, + editorAccessor: Document.IEditor | undefined ): IRootPosition | null { - // TODO: this relies on the editor under the cursor being the active editor; - // this may not be the case. Instead we should find editor from DOM and then - // have a magic way of transforming editor instances into CodeEditor and Document.Editor - // a naive way is to iterate all the editors in adapter (=cells) and see which one matches - // but the predicate is expensive and fallible in windowed notebook. - const editorAccessor = adapter.activeEditor; - if (!editorAccessor) { return null; } @@ -220,6 +215,34 @@ export class ContextAssembler { return editorPosition; } + /* + * Attempt to find editor from DOM and then (naively) find `Document.Editor` + * from `CodeEditor` isntances. The naive approach iterates all the editors + * (=cells) in the adapter, which is expensive and prone to fail in windowed notebooks. + * + * This may not be needed once https://github.com/jupyterlab/jupyterlab/pull/14920 + * is finalised an released. + */ + editorFromNode( + adapter: WidgetLSPAdapter, + node: HTMLElement + ): Document.IEditor | undefined { + const cmContent = (node as HTMLElement).closest('.cm-content'); + if (!cmContent) { + return; + } + const cmView = (cmContent as any)?.cmView?.view as EditorView | undefined; + + if (!cmView) { + return; + } + + const editorAccessor = adapter.editors + .map(e => e.ceEditor) + .find(e => (e.getEditor() as CodeMirrorEditor | null)?.editor === cmView); + return editorAccessor; + } + private getContextFromContextMenu(): ICommandContext | null { // Note: could also try using this.app.contextMenu.menu.contentNode position. // Note: could add a guard on this.app.contextMenu.menu.isAttached @@ -250,7 +273,23 @@ export class ContextAssembler { return null; } - const rootPosition = this.positionFromCoordinates(left, top, adapter); + const accessorFromNode = this.editorFromNode(adapter, leafNode); + if (!accessorFromNode) { + // Using `activeEditor` can lead to suprising results in notebook + // as a cell can be opened over a cell diffferent than the active one. + this.console.warn( + 'Editor accessor not found from node, falling back to activeEditor' + ); + } + const editorAccessor = accessorFromNode + ? accessorFromNode + : adapter.activeEditor; + const rootPosition = this.positionFromCoordinates( + left, + top, + adapter, + editorAccessor + ); if (!rootPosition) { return null; diff --git a/packages/jupyterlab-lsp/src/features/hover.ts b/packages/jupyterlab-lsp/src/features/hover.ts index 0fe7f6042..86f205d52 100644 --- a/packages/jupyterlab-lsp/src/features/hover.ts +++ b/packages/jupyterlab-lsp/src/features/hover.ts @@ -37,7 +37,6 @@ import { rootPositionToVirtualPosition, rootPositionToEditorPosition, editorPositionToRootPosition, - editorAtRootPosition, rangeToEditorRange, IEditorRange } from '../converter'; @@ -477,7 +476,6 @@ export class HoverFeature extends Feature { adapter: WidgetLSPAdapter ): Promise { const target = event.target; - // if over an empty space in a line (and not over a token) then not worth checking if ( target == null @@ -502,20 +500,42 @@ export class HoverFeature extends Feature { return false; } + // We cannot just use: + // > const editorAccessor = adapter.activeEditor + // as it relies on the editor under the cursor being the active editor, which is not the case in notebook, + // especially for actions invoked using mouse (hover, rename from context menu). + const accessorFromNode = this.contextAssembler.editorFromNode( + adapter, + target as HTMLElement + ); + if (!accessorFromNode) { + this.console.warn( + 'Editor accessor not found from node, falling back to activeEditor' + ); + } + const editorAccessor = accessorFromNode + ? accessorFromNode + : adapter.activeEditor; + + if (!editorAccessor) { + this.removeRangeHighlight(); + this.console.warn('Could not find editor accessor'); + return false; + } + const rootPosition = this.contextAssembler.positionFromCoordinates( event.clientX, event.clientY, - adapter + adapter, + editorAccessor ); - // happens because mousemove is attached to panel, not individual code cells, - // and because some regions of the editor (between lines) have no characters + // happens because some regions of the editor (between lines) have no characters if (rootPosition == null) { this.removeRangeHighlight(); return false; } - const editorAccessor = editorAtRootPosition(adapter, rootPosition); const editor = editorAccessor.getEditor(); if (!editor) { this.console.warn('Editor not available from accessor'); diff --git a/packages/jupyterlab-lsp/src/features/jump_to.ts b/packages/jupyterlab-lsp/src/features/jump_to.ts index a871647cd..1c0f9d57d 100644 --- a/packages/jupyterlab-lsp/src/features/jump_to.ts +++ b/packages/jupyterlab-lsp/src/features/jump_to.ts @@ -197,10 +197,24 @@ export class NavigationFeature extends Feature { return; } + const accessorFromNode = this.contextAssembler.editorFromNode( + adapter, + event.target as HTMLElement + ); + if (!accessorFromNode) { + this.console.warn( + 'Editor accessor not found from node, falling back to activeEditor' + ); + } + const editorAccessor = accessorFromNode + ? accessorFromNode + : adapter.activeEditor; + const rootPosition = this.contextAssembler.positionFromCoordinates( event.clientX, event.clientY, - adapter + adapter, + editorAccessor ); if (rootPosition == null) { From 7f32a6a04037681a46dc67652d80758bc7745ef2 Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Sun, 27 Aug 2023 22:19:57 +0100 Subject: [PATCH 03/22] Fix tags --- atest/01_Editor.robot | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/atest/01_Editor.robot b/atest/01_Editor.robot index 7228572d7..ac95fab1d 100644 --- a/atest/01_Editor.robot +++ b/atest/01_Editor.robot @@ -45,11 +45,11 @@ JSX LaTeX [Tags] language:latex - ${def} = Set Variable xpath:(//span[contains(@class, 'cm-line')][contains(text(), 'foo')])[last()] + ${def} = Set Variable xpath:(//div[contains(@class, 'cm-line')][contains(text(), 'foo')])[last()] Editor Shows Features for Language LaTeX example.tex Jump to Definition=${def} Rename=${def} Less - ${def} = Set Variable xpath:(//span[contains(@class, 'cm-line')][contains(text(), '@width')])[last()] + ${def} = Set Variable xpath:(//div[contains(@class, 'cm-line')][contains(text(), '@width')])[last()] Editor Shows Features for Language Less example.less Diagnostics=Do not use empty rulesets ... Jump to Definition=${def} @@ -57,7 +57,7 @@ Markdown Editor Shows Features for Language Markdown example.md Diagnostics=`Color` is misspelt Python (pylsp) - ${def} = Set Variable xpath:(//span[contains(@class, 'cm-line')][contains(text(), 'fib')])[last()] + ${def} = Set Variable xpath:(//div[contains(@class, 'cm-line')][contains(text(), 'fib')])[last()] Editor Shows Features for Server ... pylsp ... Python @@ -72,30 +72,30 @@ Python (pyright) ... Jump to Definition=${def} R - ${def} = Set Variable xpath:(//span[contains(@class, 'cm-line')][contains(text(), 'fib')])[last()] + ${def} = Set Variable xpath:(//div[contains(@class, 'cm-line')][contains(text(), 'fib')])[last()] Editor Shows Features for Language R example.R Diagnostics=Put spaces around all infix operators ... Jump to Definition=${def} Robot Framework [Tags] gh:332 ${def} = Set Variable - ... xpath:(//span[contains(@class, 'cm-line')][contains(text(), 'Special Log')])[last()] + ... xpath:(//div[contains(@class, 'cm-line')][contains(text(), 'Special Log')])[last()] Editor Shows Features for Language Robot Framework example.robot Diagnostics=Undefined keyword ... Jump to Definition=${def} SCSS ${def} = Set Variable - ... xpath:(//span[contains(@class, 'cm-line')][contains(text(), 'primary-color')])[last()] + ... xpath:(//div[contains(@class, 'cm-line')][contains(text(), 'primary-color')])[last()] Editor Shows Features for Language SCSS example.scss Diagnostics=Do not use empty rulesets ... Jump to Definition=${def} TSX - ${def} = Set Variable xpath:(//span[contains(@class, 'cm-line')][contains(text(), 'HelloWorld')])[last()] + ${def} = Set Variable xpath:(//div[contains(@class, 'cm-line')][contains(text(), 'HelloWorld')])[last()] Editor Shows Features for Language TSX example.tsx ... Diagnostics='hello' is declared but its value is never read. Jump to Definition=${def} Rename=${def} TypeScript - ${def} = Set Variable xpath:(//span[contains(@class, 'cm-line')][contains(text(), 'inc')])[last()] + ${def} = Set Variable xpath:(//div[contains(@class, 'cm-line')][contains(text(), 'inc')])[last()] Editor Shows Features for Language TypeScript example.ts Diagnostics=The left-hand side of an arithmetic ... Jump to Definition=${def} Rename=${def} From e60e196c2cd16e307bbaf85996db2c26636a45c1 Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Mon, 28 Aug 2023 13:36:41 +0100 Subject: [PATCH 04/22] Fix syntax highlighting test --- atest/05_Features/Syntax_highlighting.robot | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/atest/05_Features/Syntax_highlighting.robot b/atest/05_Features/Syntax_highlighting.robot index e1011cbe3..0c99abb29 100644 --- a/atest/05_Features/Syntax_highlighting.robot +++ b/atest/05_Features/Syntax_highlighting.robot @@ -11,25 +11,25 @@ Test Tags feature:syntax_highlighting *** Test Cases *** Syntax Highlighting Mode Stays Normal In Normal Cells ${mode} = Get Mode Of A Cell 1 - should be equal ${mode['name']} ipython + should be equal ${mode} python Syntax Highlighting Mode Changes In Cells Dominated By Foreign Documents ${mode} = Get Mode Of A Cell 2 - should be equal ${mode['name']} markdown + should be equal ${mode} markdown ${mode} = Get Mode Of A Cell 3 - should be equal ${mode['name']} xml + should be equal ${mode} xml ${mode} = Get Mode Of A Cell 4 - should be equal ${mode['name']} javascript + should be equal ${mode} javascript Highlighing Mode Works For Multiple Documents ${mode} = Get Mode Of A Cell 4 - should be equal ${mode['name']} javascript + should be equal ${mode} javascript ${mode} = Get Mode Of A Cell 6 - should be equal ${mode['name']} javascript + should be equal ${mode} javascript Highlighting Mode Changes Back And Forth After Edits ${mode} = Get Mode Of A Cell 2 - should be equal ${mode['name']} markdown + should be equal ${mode} markdown Enter Cell Editor 2 line=1 Press Keys None BACKSPACE Capture Page Screenshot backapse.png @@ -43,9 +43,9 @@ Highlighting Mode Changes Back And Forth After Edits Get Mode Of A Cell [Arguments] ${cell_number} Click Element css:.jp-Cell:nth-child(${cell_number}) - Wait Until Page Contains Element css:.jp-Cell:nth-child(${cell_number}) .CodeMirror-focused + Wait Until Page Contains Element css:.jp-Cell:nth-child(${cell_number}) .cm-focused ${mode} = Execute JavaScript - ... return document.querySelector('.jp-Cell:nth-child(${cell_number}) .CodeMirror').CodeMirror.getMode() + ... return document.querySelector('.jp-Cell:nth-child(${cell_number}) .cm-content').dataset.language RETURN ${mode} Setup Highlighting Test @@ -54,4 +54,4 @@ Setup Highlighting Test Mode Of A Cell Should Equal [Arguments] ${cell_number} ${expected_mode} ${mode} = Get Mode Of A Cell ${cell_number} - should be equal ${mode['name']} ${expected_mode} + should be equal ${mode} ${expected_mode} From daef0f3f0a3362edc2cb104990070339186f6a54 Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Mon, 28 Aug 2023 13:36:52 +0100 Subject: [PATCH 05/22] Fix jump test --- atest/05_Features/Jump.robot | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/atest/05_Features/Jump.robot b/atest/05_Features/Jump.robot index 13cfc277f..69ddf2cfa 100644 --- a/atest/05_Features/Jump.robot +++ b/atest/05_Features/Jump.robot @@ -25,14 +25,14 @@ Jumps To References With Modifier Click [Setup] Prepare File for Editing Python editor jump_references.py Configure JupyterLab Plugin {"modifierKey": "Accel"} plugin id=${JUMP PLUGIN ID} Wait Until Fully Initialized - ${token} = Select Token Occurrence func type=def + ${token} = Select Token Occurrence func which=1 Click Element ${token} ${original} = Measure Cursor Position Ctrl Click Element ${token} Wait Until Page Contains Choose the jump target ${references_count} = Get Element Count css:.jp-Dialog select option - Should Be True ${references_count} == ${3} - Select From List By Index css:.jp-Dialog select 2 + Should Be True ${references_count} == ${2} + Select From List By Index css:.jp-Dialog select 1 Click Element css:.jp-Dialog-button.jp-mod-accept Wait Until Keyword Succeeds 10 x 1 s Cursor Should Jump ${original} [Teardown] Clean Up After Working With File jump_references.py @@ -40,15 +40,15 @@ Jumps To References With Modifier Click Jumps To References From Context Menu [Setup] Prepare File for Editing Python editor jump_references.py Wait Until Fully Initialized - ${token} = Select Token Occurrence func type=def + ${token} = Select Token Occurrence func which=1 Click Element ${token} ${original} = Measure Cursor Position Open Context Menu Over ${token} Select Menu Entry Jump to references Wait Until Page Contains Choose the jump target ${references_count} = Get Element Count css:.jp-Dialog select option - Should Be True ${references_count} == ${3} - Select From List By Index css:.jp-Dialog select 2 + Should Be True ${references_count} == ${2} + Select From List By Index css:.jp-Dialog select 1 Click Element css:.jp-Dialog-button.jp-mod-accept Wait Until Keyword Succeeds 10 x 1 s Cursor Should Jump ${original} [Teardown] Clean Up After Working With File jump_references.py @@ -92,9 +92,9 @@ Clean Up Folder With Spaces Remove Directory ${NOTEBOOK DIR}${/}${FOLDER WITH SPACE} recursive=True Select Token Occurrence - [Arguments] ${token} ${type}=variable ${which}=last + [Arguments] ${token} ${which}=last() ${sel} = Set Variable - ... xpath:(//span[contains(@class, 'cm-${type}')][contains(text(), '${token}')])[${which}()] + ... xpath:(//div[contains(@class, 'cm-line')]//*[contains(text(), '${token}')])[${which}] RETURN ${sel} Ctrl Click Element From 12ec067068f848b28852e9be6ddafa7bb17cb721 Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Mon, 28 Aug 2023 13:37:16 +0100 Subject: [PATCH 06/22] Apply syntax highlighting on start, fix highlight restoration --- .../src/features/syntax_highlighting.ts | 124 ++++++++++++------ 1 file changed, 87 insertions(+), 37 deletions(-) diff --git a/packages/jupyterlab-lsp/src/features/syntax_highlighting.ts b/packages/jupyterlab-lsp/src/features/syntax_highlighting.ts index b9be71db4..fc3bad706 100644 --- a/packages/jupyterlab-lsp/src/features/syntax_highlighting.ts +++ b/packages/jupyterlab-lsp/src/features/syntax_highlighting.ts @@ -1,4 +1,5 @@ import { EditorView } from '@codemirror/view'; +import type { ViewUpdate } from '@codemirror/view'; import { JupyterFrontEnd, JupyterFrontEndPlugin @@ -16,13 +17,15 @@ import { import { ILSPFeatureManager, ILSPDocumentConnectionManager, - WidgetLSPAdapter + WidgetLSPAdapter, + Document } from '@jupyterlab/lsp'; import { ISettingRegistry } from '@jupyterlab/settingregistry'; import { LabIcon } from '@jupyterlab/ui-components'; import syntaxSvg from '../../style/icons/syntax-highlight.svg'; import { CodeSyntax as LSPSyntaxHighlightingSettings } from '../_syntax_highlighting'; +import { ContextAssembler } from '../context'; import { FeatureSettings, Feature } from '../feature'; import { PLUGIN_ID } from '../tokens'; import { VirtualDocument } from '../virtual/document'; @@ -41,15 +44,17 @@ export class SyntaxHighlightingFeature extends Feature { constructor(protected options: SyntaxHighlightingFeature.IOptions) { super(options); const connectionManager = options.connectionManager; + const contextAssembler = options.contextAssembler; options.editorExtensionRegistry.addExtension({ name: 'lsp:syntaxHighlighting', factory: options => { - const updateListener = EditorView.updateListener.of(viewUpdate => { - if (!viewUpdate.docChanged) { - return; - } + let intialized = false; + const updateHandler = async ( + viewUpdate: ViewUpdate, + awaitUpdate = true + ) => { const adapter = [...connectionManager.adapters.values()].find( adapter => adapter.widget.node.contains(viewUpdate.view.contentDOM) ); @@ -58,9 +63,55 @@ export class SyntaxHighlightingFeature extends Feature { // const editor = adapter.editors.find(e => e.model === options.model); if (adapter) { - this.updateMode(adapter, viewUpdate.view).catch(console.warn); + await adapter.ready; + const accessorFromNode = contextAssembler.editorFromNode( + adapter, + viewUpdate.view.contentDOM + ); + if (!accessorFromNode) { + console.warn( + 'Editor accessor not found from node, falling back to activeEditor' + ); + } + const editorAccessor = accessorFromNode + ? accessorFromNode + : adapter.activeEditor; + + if (!editorAccessor) { + console.warn('No accessor'); + return; + } + await this.updateMode( + adapter, + viewUpdate.view, + editorAccessor, + awaitUpdate + ); + } + }; + + const updateListener = EditorView.updateListener.of( + async viewUpdate => { + if (!viewUpdate.docChanged) { + if (intialized) { + return; + } + // TODO: replace this with a simple Promise.all([editorAccessor.ready, adapter.ready]).then(() => updateMode(options.editor)) + // once JupyterLab 4.1 with improved factory API is out. + // For now we wait 2.5 seconds hoping the adapter will be connected + // and the document will be ready + setTimeout(async () => { + await updateHandler(viewUpdate, false); + }, 2500); + intialized = true; + } + + await updateHandler(viewUpdate); } - }); + ); + + // update the mode at first update even if no changes to ensure the + // correct mode gets applied on load. return EditorExtensionRegistry.createImmutableExtension([ updateListener @@ -90,31 +141,31 @@ export class SyntaxHighlightingFeature extends Feature { return mimetype; } - async updateMode(adapter: WidgetLSPAdapter, view: EditorView) { + async updateMode( + adapter: WidgetLSPAdapter, + view: EditorView, + editorAccessor: Document.IEditor, + awaitUpdate = true + ) { const topDocument = adapter.virtualDocument as VirtualDocument; - const totalArea = view.state.doc.length; - - // TODO no way to map from EditorView to Document.IEditor is blocking here. - // TODO: active editor is not necessairly the editor that triggered the update - const editorAccessor = adapter.activeEditor; - const editor = editorAccessor?.getEditor(); - if ( - !editorAccessor || - !editor || - (editor as CodeMirrorEditor).editor !== view - ) { - // TODO: ideally we would not have to do this (we would have view -> editor map) - return; - } + if (!topDocument) { return; } - await topDocument.updateManager.updateDone; + if (awaitUpdate) { + await topDocument.updateManager.updateDone; + } + + const editor = editorAccessor.getEditor()! as CodeMirrorEditor; + const totalArea = editor.state.doc.length; const overrides = new Map(); for (const map of topDocument.getForeignDocuments(editorAccessor)) { for (const [range, block] of map.entries()) { - const editor = block.editor.getEditor()! as CodeMirrorEditor; + const blockEditor = block.editor.getEditor()! as CodeMirrorEditor; + if (blockEditor != editor) { + continue; + } const coveredArea = editor.getOffsetAt(range.end) - editor.getOffsetAt(range.start); const coverage = coveredArea / totalArea; @@ -137,22 +188,15 @@ export class SyntaxHighlightingFeature extends Feature { } } } - const relevantEditors = new Set( - adapter.editors.map(e => e.ceEditor.getEditor()) - ); - // restore modes on editors which are no longer over the threshold + // restore mode on the editor if it no longer over the threshold // (but only those which belong to this adapter). - for (const [editor, originalMode] of this.originalModes) { - if (!relevantEditors.has(editor)) { - continue; - } - if (overrides.has(editor)) { - continue; - } else { + if (!overrides.has(editor)) { + const originalMode = this.originalModes.get(editor); + if (originalMode) { editor.model.mimeType = originalMode; } } - // add new ovverrides to remember the original mode + // add new overrides to remember the original mode for (const [editor, mode] of overrides) { if (!this.originalModes.has(editor)) { this.originalModes.set(editor, mode); @@ -167,6 +211,7 @@ export namespace SyntaxHighlightingFeature { mimeTypeService: IEditorMimeTypeService; editorExtensionRegistry: IEditorExtensionRegistry; languageRegistry: IEditorLanguageRegistry; + contextAssembler: ContextAssembler; } export const id = PLUGIN_ID + ':syntax_highlighting'; } @@ -199,12 +244,17 @@ export const SYNTAX_HIGHLIGHTING_PLUGIN: JupyterFrontEndPlugin = { if (settings.composite.disable) { return; } + const contextAssembler = new ContextAssembler({ + app, + connectionManager + }); const feature = new SyntaxHighlightingFeature({ settings, connectionManager, editorExtensionRegistry, mimeTypeService: editorServices.mimeTypeService, - languageRegistry + languageRegistry, + contextAssembler }); featureManager.register(feature); // return feature; From e500b57c33eff7b7dad3d9e9b837736560fd1c8f Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Mon, 28 Aug 2023 13:38:32 +0100 Subject: [PATCH 07/22] Remove duplicated context assembler in jump to --- packages/jupyterlab-lsp/src/features/jump_to.ts | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/packages/jupyterlab-lsp/src/features/jump_to.ts b/packages/jupyterlab-lsp/src/features/jump_to.ts index 1c0f9d57d..a56cf242d 100644 --- a/packages/jupyterlab-lsp/src/features/jump_to.ts +++ b/packages/jupyterlab-lsp/src/features/jump_to.ts @@ -554,14 +554,9 @@ export const JUMP_PLUGIN: JupyterFrontEndPlugin = { }); featureManager.register(feature); - const assembler = new ContextAssembler({ - app, - connectionManager - }); - app.commands.addCommand(CommandIDs.jumpToDefinition, { execute: async () => { - const context = assembler.getContext(); + const context = contextAssembler.getContext(); if (!context) { console.warn('Could not get context'); return; @@ -595,7 +590,7 @@ export const JUMP_PLUGIN: JupyterFrontEndPlugin = { label: trans.__('Jump to definition'), icon: jumpToIcon, isEnabled: () => { - const context = assembler.getContext(); + const context = contextAssembler.getContext(); if (!context) { console.warn('Could not get context'); return false; @@ -607,7 +602,7 @@ export const JUMP_PLUGIN: JupyterFrontEndPlugin = { app.commands.addCommand(CommandIDs.jumpToReference, { execute: async () => { - const context = assembler.getContext(); + const context = contextAssembler.getContext(); if (!context) { console.warn('Could not get context'); return; @@ -644,7 +639,7 @@ export const JUMP_PLUGIN: JupyterFrontEndPlugin = { label: trans.__('Jump to references'), icon: jumpToIcon, isEnabled: () => { - const context = assembler.getContext(); + const context = contextAssembler.getContext(); if (!context) { console.warn('Could not get context'); return false; @@ -656,7 +651,7 @@ export const JUMP_PLUGIN: JupyterFrontEndPlugin = { app.commands.addCommand(CommandIDs.jumpBack, { execute: async () => { - const context = assembler.getContext(); + const context = contextAssembler.getContext(); if (!context) { console.warn('Could not get context'); return; @@ -666,7 +661,7 @@ export const JUMP_PLUGIN: JupyterFrontEndPlugin = { label: trans.__('Jump back'), icon: jumpBackIcon, isEnabled: () => { - const context = assembler.getContext(); + const context = contextAssembler.getContext(); if (!context) { console.warn('Could not get context'); return false; From 54dac9a6ce0ff948ad0aa02ba21bbe41cef46b6e Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Mon, 28 Aug 2023 14:30:29 +0100 Subject: [PATCH 08/22] Fix index for `skipInspect` retrieval --- packages/jupyterlab-lsp/src/features/diagnostics/feature.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/jupyterlab-lsp/src/features/diagnostics/feature.ts b/packages/jupyterlab-lsp/src/features/diagnostics/feature.ts index 14ecf4d6c..9fa3a4249 100644 --- a/packages/jupyterlab-lsp/src/features/diagnostics/feature.ts +++ b/packages/jupyterlab-lsp/src/features/diagnostics/feature.ts @@ -434,12 +434,11 @@ export class DiagnosticsFeature extends Feature implements IDiagnosticsFeature { return; } } - if ( // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore TODO document.virtualLines - .get(start.line)! + .get(start.line - 1)! .skipInspect.indexOf(document.idPath) !== -1 ) { this.console.log( From c3f6fd195cdd4a9b4ce25f504e8f6f9c798d5546 Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Mon, 28 Aug 2023 14:35:17 +0100 Subject: [PATCH 09/22] List visible diagnostics when wait fails to aid debugging --- atest/diagnostics.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/atest/diagnostics.py b/atest/diagnostics.py index 5b1a0e9c1..0a8ecf74c 100644 --- a/atest/diagnostics.py +++ b/atest/diagnostics.py @@ -1,6 +1,6 @@ from functools import partial from robot.libraries.BuiltIn import BuiltIn -from selenium.common.exceptions import NoSuchElementException +from selenium.common.exceptions import NoSuchElementException, TimeoutException from selenium.webdriver.support.wait import WebDriverWait from selenium.webdriver.common.by import By from selenium.webdriver.remote.webdriver import WebDriver @@ -30,10 +30,16 @@ def page_contains_diagnostic(driver: WebDriver, selector, negate=False): def wait_until_page_contains_diagnostic(selector, timeout='3s'): sl: SeleniumLibrary = BuiltIn().get_library_instance("SeleniumLibrary") wait = WebDriverWait(sl.driver, timestr_to_secs(timeout)) - return wait.until( - partial(page_contains_diagnostic, selector=selector), - f'Diagnostic with selector {selector} not found in {timeout}' - ) + try: + return wait.until( + partial(page_contains_diagnostic, selector=selector) + ) + except TimeoutException: + elements = sl.driver.find_elements(By.CSS_SELECTOR, f'.{DIAGNOSTIC_CLASS}') + titles = '\n - ' + '\n - '.join([el.get_attribute('title') for el in elements]) + raise TimeoutException( + f'Diagnostic with selector {selector} not found in {timeout}.\nVisible diagnostics are: {titles}' + ) def wait_until_page_does_not_contain_diagnostic(selector, timeout='3s'): From afff86195134b096133d88884d9ee992b212afdd Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Mon, 28 Aug 2023 16:15:32 +0100 Subject: [PATCH 10/22] Remove unused CSS rule --- packages/jupyterlab-lsp/style/highlight.css | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/jupyterlab-lsp/style/highlight.css b/packages/jupyterlab-lsp/style/highlight.css index 2b5dd5e2c..dc32ef159 100644 --- a/packages/jupyterlab-lsp/style/highlight.css +++ b/packages/jupyterlab-lsp/style/highlight.css @@ -3,12 +3,6 @@ @import url('./variables/jupyterlab-dark.css'); @import url('./variables/jupyterlab-light.css'); -.cm-lsp-diagnostic { - text-decoration-line: underline; - /* For Chrome */ - text-decoration-skip-ink: none; -} - .cm-lsp-diagnostic-tag-Unnecessary { /* * LSP: "Clients are allowed to render diagnostics with this tag faded out instead of having an error squiggle." From 88819baba4cf2fb03b89b82c3ab8e2e589796bbf Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Mon, 28 Aug 2023 16:15:43 +0100 Subject: [PATCH 11/22] Fix removing highlights on blur --- packages/jupyterlab-lsp/src/features/highlights.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/jupyterlab-lsp/src/features/highlights.ts b/packages/jupyterlab-lsp/src/features/highlights.ts index 3d5d04d80..558a083fc 100644 --- a/packages/jupyterlab-lsp/src/features/highlights.ts +++ b/packages/jupyterlab-lsp/src/features/highlights.ts @@ -149,8 +149,12 @@ export class HighlightsFeature extends Feature { protected onBlur(view: EditorView) { if (this.settings.composite.removeOnBlur) { - this.markManager.clearEditorMarks(view); - this._lastToken = null; + // Delayed evaluation to avoid error: + // `Error: Calls to EditorView.update are not allowed while an update is in progress` + setTimeout(() => { + this.markManager.clearEditorMarks(view); + this._lastToken = null; + }, 0); } } From 7e899d28effce0c6fa2f587fe273b9ade0d6ba1c Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Mon, 28 Aug 2023 16:16:17 +0100 Subject: [PATCH 12/22] Update selector for highlights in tests --- atest/05_Features/Highlights.robot | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/atest/05_Features/Highlights.robot b/atest/05_Features/Highlights.robot index b5b9e4435..0ad434b21 100644 --- a/atest/05_Features/Highlights.robot +++ b/atest/05_Features/Highlights.robot @@ -8,6 +8,9 @@ Test Teardown Clean Up After Working With File Highlights.ipynb Test Tags feature:highlights +*** Variables *** +${HIGHLIGHT_XPATH_SELECTOR} span[contains(@class, 'cm-lsp-highlight-Read') or contains(@class, 'cm-lsp-highlight-Write') or contains(@class, 'cm-lsp-highlight-Text')] + *** Test Cases *** # cursor is symbolized by pipe (|), for example when # it is at the end of line, after `1` in `test = 1` @@ -64,18 +67,18 @@ Highlights are removed when no cell is focused *** Keywords *** Should Not Highlight Any Tokens - Page Should Not Contain css:.cm-lsp-highlight + Page Should Not Contain xpath://${HIGHLIGHT_XPATH_SELECTOR} Should Highlight Token [Arguments] ${token} ${timeout}=15s ${token_element} Set Variable - ... xpath://span[contains(@class, 'cm-lsp-highlight')][contains(text(), '${token}')] + ... xpath://${HIGHLIGHT_XPATH_SELECTOR}\[contains(text(), '${token}')] Wait Until Page Contains Element ${token_element} timeout=${timeout} Should Not Highlight Token [Arguments] ${token} ${timeout}=15s ${token_element} Set Variable - ... xpath://span[contains(@class, 'cm-lsp-highlight')][contains(text(), '${token}')] + ... xpath://${HIGHLIGHT_XPATH_SELECTOR}\[contains(text(), '${token}')] Wait Until Page Does Not Contain Element ${token_element} timeout=${timeout} Setup Highlights Test From 0baa449aa20a011bde80507074b2c577863a5cc9 Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Mon, 28 Aug 2023 16:53:45 +0100 Subject: [PATCH 13/22] Use the new diagnostic wait to fix configuration tests --- atest/07_Configuration.robot | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atest/07_Configuration.robot b/atest/07_Configuration.robot index 12e1d7057..298c4f4a2 100644 --- a/atest/07_Configuration.robot +++ b/atest/07_Configuration.robot @@ -79,7 +79,7 @@ Settings Should Change Editor Diagnostics Drag and Drop By Offset ${JLAB XP DOCK TAB}\[contains(., 'Diagnostics Panel')] 600 -200 Click Element ${JLAB XP DOCK TAB}\[contains(., 'Launcher')]/${close icon} IF "${before}" - Wait Until Page Contains Element ${before diagnostic} timeout=30s + Wait Until Page Contains Diagnostic ${before diagnostic} timeout=30s END Page Should Not Contain ${after diagnostic} Capture Page Screenshot 01-default-diagnostics-and-settings.png From 1763d75679f60189fcd1a0414083503d744630e0 Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Mon, 28 Aug 2023 16:54:10 +0100 Subject: [PATCH 14/22] Improve (and test) implementation of signature attributes marking - enh: class names will be applied on marks as needed - perf: empty marks won't be added - perf: elements wont be created if not needed --- .../src/features/signature.spec.ts | 41 +++++- .../jupyterlab-lsp/src/features/signature.ts | 122 +++++++++--------- 2 files changed, 102 insertions(+), 61 deletions(-) diff --git a/packages/jupyterlab-lsp/src/features/signature.spec.ts b/packages/jupyterlab-lsp/src/features/signature.spec.ts index f4bf7680b..3203c024a 100644 --- a/packages/jupyterlab-lsp/src/features/signature.spec.ts +++ b/packages/jupyterlab-lsp/src/features/signature.spec.ts @@ -1,13 +1,15 @@ import { python } from '@codemirror/lang-python'; import { Language } from '@codemirror/language'; +import { jupyterHighlightStyle } from '@jupyterlab/codemirror'; +import { tags } from '@lezer/highlight'; import * as lsProtocol from 'vscode-languageserver-protocol'; import { BrowserConsole } from '../virtual/console'; -import { extractLead, signatureToMarkdown } from './signature'; +import { extractLead, signatureToMarkdown, highlightCode } from './signature'; describe('Signature', () => { - describe('extractLead', () => { + describe('extractLead()', () => { it('Extracts standalone one-line paragraph', () => { const split = extractLead( ['This function does foo', '', 'But there are more details'], @@ -56,7 +58,40 @@ describe('Signature', () => { }); }); - describe('SignatureToMarkdown', () => { + describe('highlightCode()', () => { + const pythonLanguage = python().language; + const arithmeticClass = jupyterHighlightStyle.style([ + tags.arithmeticOperator + ]); + const numberClass = jupyterHighlightStyle.style([tags.number]); + + it('marks first parameter', () => { + const result = highlightCode( + '(x: int = 1, y: int = 2) -> None', + { label: 'x: int = 1' }, + pythonLanguage + ); + expect( + result.includes( + `x: int = 1` + ) + ).toBe(true); + }); + it('marks second parameter', () => { + const result = highlightCode( + '(x: int = 1, y: int = 2) -> None', + { label: 'y: int = 2' }, + pythonLanguage + ); + expect( + result.includes( + `y: int = 2` + ) + ).toBe(true); + }); + }); + + describe('signatureToMarkdown()', () => { const MockHighlighter = ( code: string, fragment: lsProtocol.ParameterInformation, diff --git a/packages/jupyterlab-lsp/src/features/signature.ts b/packages/jupyterlab-lsp/src/features/signature.ts index 8d6830761..c511fc9d7 100644 --- a/packages/jupyterlab-lsp/src/features/signature.ts +++ b/packages/jupyterlab-lsp/src/features/signature.ts @@ -168,6 +168,69 @@ export function signatureToMarkdown( return markdown; } +export function highlightCode( + source: string, + parameter: lsProtocol.ParameterInformation, + language: Language | undefined +) { + const pre = document.createElement('pre'); + const code = document.createElement('code'); + pre.appendChild(code); + code.className = + 'cm-s-jupyter' + language ? `language-${language?.name}` : ''; + + const substring: string = + typeof parameter.label === 'string' + ? parameter.label + : source.slice(parameter.label[0], parameter.label[1]); + const start = source.indexOf(substring); + const end = start + substring.length; + if (!language) { + code.innerText = source; + } else { + runMode( + source, + language, + (token: string, className: string, from: number, to: number) => { + let populated = false; + // In CodeMirror6 variables are not necessairly tokenized, + // we need to split them manually + if (from <= end && start <= to) { + const a = Math.max(start, from); + const b = Math.min(to, end); + if (a != b) { + const prefix = source.slice(from, a); + const content = source.slice(a, b); + const suffix = source.slice(b, to); + + const mark = document.createElement('mark'); + if (className) { + mark.className = className; + } + mark.appendChild(document.createTextNode(content)); + code.appendChild(document.createTextNode(prefix)); + code.appendChild(mark); + code.appendChild(document.createTextNode(suffix)); + populated = true; + } + } + if (!populated) { + if (className) { + const element = document.createElement('span'); + element.classList.add(className); + element.textContent = token; + code.appendChild(element); + } else { + code.appendChild(document.createTextNode(token)); + } + } + } + ); + } + + return pre.outerHTML; +} + function extractLastCharacter(changes: ChangeSet): string { // TODO test with pasting, maybe rewrite to retrieve based on cursor position. let last = ''; @@ -372,63 +435,6 @@ export class SignatureFeature extends Feature { }; } - protected highlightCode( - source: string, - parameter: lsProtocol.ParameterInformation, - language: Language | undefined - ) { - const pre = document.createElement('pre'); - const code = document.createElement('code'); - pre.appendChild(code); - code.className = - 'cm-s-jupyter' + language ? `language-${language?.name}` : ''; - - const substring: string = - typeof parameter.label === 'string' - ? parameter.label - : source.slice(parameter.label[0], parameter.label[1]); - const start = source.indexOf(substring); - const end = start + substring.length; - - if (!language) { - code.innerText = source; - } else { - runMode( - source, - language, - (token: string, className: string, from: number, to: number) => { - let element: HTMLElement | Node; - if (className) { - element = document.createElement('span'); - (element as HTMLElement).classList.add(className); - element.textContent = token; - } else { - element = document.createTextNode(token); - } - // In CodeMirror6 variables are not necessairly tokenized, - // we need to split them manually - if (from <= end && start <= to) { - const a = Math.max(start, from); - const b = Math.min(to, end); - const prefix = source.slice(from, a); - const content = source.slice(a, b); - const suffix = source.slice(b, to); - - const mark = document.createElement('mark'); - mark.appendChild(document.createTextNode(content)); - code.appendChild(document.createTextNode(prefix)); - code.appendChild(mark); - code.appendChild(document.createTextNode(suffix)); - } else { - code.appendChild(element); - } - } - ); - } - - return pre.outerHTML; - } - /** * Represent signature as a Markdown element. */ @@ -440,7 +446,7 @@ export class SignatureFeature extends Feature { return signatureToMarkdown( item, language, - this.highlightCode.bind(this), + highlightCode, this.console, activeParameterFallback, this.settings.composite.maxLines From 412d84920a6b0d9a52ae5374675bde1804d2e4e6 Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Mon, 28 Aug 2023 19:39:43 +0100 Subject: [PATCH 15/22] Correct fix for diagnostics silencing (the offset was ok), see: https://github.com/jupyterlab/jupyterlab/issues/15034 --- .../src/features/diagnostics/feature.ts | 4 +- .../jupyterlab-lsp/src/virtual/document.ts | 61 +++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/packages/jupyterlab-lsp/src/features/diagnostics/feature.ts b/packages/jupyterlab-lsp/src/features/diagnostics/feature.ts index 9fa3a4249..e6948e33c 100644 --- a/packages/jupyterlab-lsp/src/features/diagnostics/feature.ts +++ b/packages/jupyterlab-lsp/src/features/diagnostics/feature.ts @@ -438,8 +438,8 @@ export class DiagnosticsFeature extends Feature implements IDiagnosticsFeature { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore TODO document.virtualLines - .get(start.line - 1)! - .skipInspect.indexOf(document.idPath) !== -1 + .get(start.line)! + .skipInspect.includes(document.idPath) ) { this.console.log( 'Ignoring inspections silenced for this document:', diff --git a/packages/jupyterlab-lsp/src/virtual/document.ts b/packages/jupyterlab-lsp/src/virtual/document.ts index 5ccc29455..f42ddae55 100644 --- a/packages/jupyterlab-lsp/src/virtual/document.ts +++ b/packages/jupyterlab-lsp/src/virtual/document.ts @@ -80,6 +80,67 @@ export class VirtualDocument extends VirtualDocumentBase { return { lines, foreignDocumentsMap, skipInspect }; } + appendCodeBlock( + block: Document.ICodeBlockOptions, + editorShift: CodeEditor.IPosition = { line: 0, column: 0 }, + virtualShift?: CodeEditor.IPosition + ): void { + let cellCode = block.value; + let ceEditor = block.ceEditor; + + if (this.isDisposed) { + console.warn('Cannot append code block: document disposed'); + return; + } + let sourceCellLines = cellCode.split('\n'); + let { lines, foreignDocumentsMap, skipInspect } = this.prepareCodeBlock( + block, + editorShift + ); + + for (let i = 0; i < lines.length; i++) { + this.virtualLines.set(this.lastVirtualLine + i, { + skipInspect: skipInspect[i], + editor: ceEditor, + // TODO this is incorrect, wont work if something was extracted + sourceLine: this.lastSourceLine + i + }); + } + for (let i = 0; i < sourceCellLines.length; i++) { + this.sourceLines.set(this.lastSourceLine + i, { + editorLine: i, + editorShift: { + line: editorShift.line - (virtualShift?.line || 0), + column: i === 0 ? editorShift.column - (virtualShift?.column || 0) : 0 + }, + // TODO: move those to a new abstraction layer (DocumentBlock class) + editor: ceEditor, + foreignDocumentsMap, + // TODO this is incorrect, wont work if something was extracted + virtualLine: this.lastVirtualLine + i + }); + } + + this.lastVirtualLine += lines.length; + + // one empty line is necessary to separate code blocks, next 'n' lines are to silence linters; + // the final cell does not get the additional lines (thanks to the use of join, see below) + + this.lineBlocks.push(lines.join('\n') + '\n'); + + // adding the virtual lines for the blank lines + for (let i = 0; i < this.blankLinesBetweenCells; i++) { + this.virtualLines.set(this.lastVirtualLine + i, { + skipInspect: [this.idPath], + editor: ceEditor, + sourceLine: null + }); + } + + this.lastVirtualLine += this.blankLinesBetweenCells; + this.lastSourceLine += sourceCellLines.length; + } + /** * @experimental */ From f4b554d9b7362f198bb011e9f022d67f0785f994 Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Mon, 28 Aug 2023 19:41:08 +0100 Subject: [PATCH 16/22] Attempt to re-enabled diagnostics in foreign test (failed) --- .../features/diagnostics/diagnostics.spec.ts | 33 +++++++++++++------ 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/packages/jupyterlab-lsp/src/features/diagnostics/diagnostics.spec.ts b/packages/jupyterlab-lsp/src/features/diagnostics/diagnostics.spec.ts index 45d6bad2d..83e6ac23f 100644 --- a/packages/jupyterlab-lsp/src/features/diagnostics/diagnostics.spec.ts +++ b/packages/jupyterlab-lsp/src/features/diagnostics/diagnostics.spec.ts @@ -21,7 +21,8 @@ import { NotebookTestEnvironment, codeCell, setNotebookContent, - showAllCells + showAllCells, + MockNotebookAdapter } from '../../testutils'; import { foreignCodeExtractors } from '../../transclusions/ipython/extractors'; @@ -327,7 +328,7 @@ describe('Diagnostics', () => { expect(db.get(document)!.length).toBe(5); }); - it.skip('Works in foreign documents', async () => { + it.skip('works in foreign documents', async () => { setNotebookContent(env.notebook, [ codeCell(['valid = 0', 'code = 1', '# here']), codeCell(['%%python', 'y = 1', 'x']) @@ -337,8 +338,10 @@ describe('Diagnostics', () => { let document = env.adapter.virtualDocument!; console.log(document.foreignDocuments); - expect(document.foreignDocuments.size).toBe(1); - let foreignDocument = document.foreignDocuments.values().next().value; + // expect(document.foreignDocuments.size).toBe(1); + let foreignDocument = [...document.foreignDocuments.values()][ + document.foreignDocuments.size - 1 + ]; let response = { uri: foreignDocument.uri, @@ -356,7 +359,10 @@ describe('Diagnostics', () => { } as lsProtocol.PublishDiagnosticsParams; // test guards against wrongly propagated responses: - await feature.handleDiagnostic(response, foreignDocument, env.adapter); + await feature.handleDiagnostic(response, document, env.adapter); + + await env.adapter.updateDocuments(); + await (env.adapter as MockNotebookAdapter).foreingDocumentOpened.promise; let cmEditors = env.adapter.editors.map( editor => editor.ceEditor.getEditor()! as CodeMirrorEditor @@ -368,12 +374,19 @@ describe('Diagnostics', () => { expect(marksCell1.length).toBe(0); expect(marksCell2.length).toBe(0); + // update the foreignDocument as it may have been re-created in update + foreignDocument = [...document.foreignDocuments.values()][ + document.foreignDocuments.size - 1 + ]; + response.uri = foreignDocument.uri; + // correct propagation - await feature.handleDiagnostic( - response, - env.adapter.virtualDocument!, - env.adapter - ); + await feature.handleDiagnostic(response, foreignDocument, env.adapter); + await framePromise(); + await framePromise(); + + //await env.adapter.updateDocuments(); + //await (env.adapter as MockNotebookAdapter).foreingDocumentOpened.promise; marksCell1 = getDiagnostics(cmEditors[0].state); marksCell2 = getDiagnostics(cmEditors[1].state); From 0c5196d0b955dde880b01b71c50966e304ac10ab Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Mon, 28 Aug 2023 20:27:09 +0100 Subject: [PATCH 17/22] Fix expectations in syntax highlighting tests --- atest/05_Features/Syntax_highlighting.robot | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/atest/05_Features/Syntax_highlighting.robot b/atest/05_Features/Syntax_highlighting.robot index 0c99abb29..d866a47c5 100644 --- a/atest/05_Features/Syntax_highlighting.robot +++ b/atest/05_Features/Syntax_highlighting.robot @@ -17,7 +17,7 @@ Syntax Highlighting Mode Changes In Cells Dominated By Foreign Documents ${mode} = Get Mode Of A Cell 2 should be equal ${mode} markdown ${mode} = Get Mode Of A Cell 3 - should be equal ${mode} xml + should be equal ${mode} html ${mode} = Get Mode Of A Cell 4 should be equal ${mode} javascript @@ -33,7 +33,7 @@ Highlighting Mode Changes Back And Forth After Edits Enter Cell Editor 2 line=1 Press Keys None BACKSPACE Capture Page Screenshot backapse.png - wait until keyword succeeds 5x 2s Mode Of A Cell Should Equal 2 ipython + wait until keyword succeeds 5x 2s Mode Of A Cell Should Equal 2 python Enter Cell Editor 2 line=1 Press Keys None n wait until keyword succeeds 5x 2s Mode Of A Cell Should Equal 2 markdown From 04c58593a59d4fb1ad97ec6b64a338d1892a4a46 Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Mon, 28 Aug 2023 21:13:52 +0100 Subject: [PATCH 18/22] Fix configuration --- packages/jupyterlab-lsp/src/index.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/jupyterlab-lsp/src/index.ts b/packages/jupyterlab-lsp/src/index.ts index cac453cd6..51e482ab4 100644 --- a/packages/jupyterlab-lsp/src/index.ts +++ b/packages/jupyterlab-lsp/src/index.ts @@ -152,9 +152,17 @@ export class LSPExtension { ); // Store the initial server settings, to be sent asynchronously // when the servers are initialized. - const languageServerSettings = (options.language_servers || + let languageServerSettings = (options.language_servers || {}) as TLanguageServerConfigurations; + // Add `configuration` as a copy of `serverSettings` to work with changed name upstream + languageServerSettings = Object.fromEntries( + Object.entries(languageServerSettings).map(([key, value]) => { + value.configuration = value.serverSettings; + return [key, value]; + }) + ); + this._connectionManager.initialConfigurations = languageServerSettings; // TODO: if priorities changed reset connections From 5329958419d1706a56d2038c9c80e6af58e86339 Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Mon, 28 Aug 2023 21:19:05 +0100 Subject: [PATCH 19/22] Try to target more specific part (although spans are not guaranteed) --- atest/05_Features/Hover.robot | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atest/05_Features/Hover.robot b/atest/05_Features/Hover.robot index b1f4ef3fb..99a6f9275 100644 --- a/atest/05_Features/Hover.robot +++ b/atest/05_Features/Hover.robot @@ -84,7 +84,7 @@ Update hover after character deletion Last Occurrence [Arguments] ${symbol} ${sel} = Set Variable If "${symbol}".startswith(("xpath", "css")) ${symbol} - ... xpath:(//div[@class="cm-line"][contains(., "${symbol}")])[last()] + ... xpath:(//div[@class="cm-content"]//*[contains(., "${symbol}")])[last()] RETURN ${sel} Trigger Automatically By Hover From f3d75c0f384603ce123a7936d6edf55825e716a1 Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Mon, 28 Aug 2023 21:42:59 +0100 Subject: [PATCH 20/22] Try text node selectors --- atest/01_Editor.robot | 31 ++++++++++++++++--------------- atest/05_Features/Hover.robot | 2 +- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/atest/01_Editor.robot b/atest/01_Editor.robot index ac95fab1d..8162206e5 100644 --- a/atest/01_Editor.robot +++ b/atest/01_Editor.robot @@ -9,26 +9,27 @@ Test Tags ui:editor aspect:ls:features *** Test Cases *** Bash + ${def} = Set Variable xpath:(//div[contains(@class, 'cm-line')]//text()[contains(., 'HelloWorld')])[last()] Editor Shows Features for Language ... Bash ... example.sh ... Diagnostics=Double quote to prevent globbing and word splitting. - ... Jump to Definition=fib + ... Jump to Definition=${def} CSS ${def} = Set Variable - ... xpath:(//span[contains(@class, 'cm-line')][contains(text(), '--some-var')])[last()] + ... xpath:(//div[contains(@class, 'cm-line')]//text()[contains(., '--some-var')])[last()] Editor Shows Features for Language CSS example.css Diagnostics=Do not use empty rulesets ... Jump to Definition=${def} Rename=${def} Docker - ${def} = Set Variable xpath://span[contains(@class, 'cm-line')][contains(text(), 'PLANET')] + ${def} = Set Variable xpath://div[contains(@class, 'cm-line')]//text()[contains(., 'PLANET')] Wait Until Keyword Succeeds 3x 100ms Editor Shows Features for Language Docker Dockerfile ... Diagnostics=Instructions should be written in uppercase letters Jump to Definition=${def} ... Rename=${def} JS - ${def} = Set Variable xpath:(//span[contains(@class, 'cm-variable')][contains(text(), 'fib')])[last()] + ${def} = Set Variable xpath:(//div[contains(@class, 'cm-line')]//text()[contains(., 'fib')])[last()] Editor Shows Features for Language JS example.js Diagnostics=Expression expected ... Jump to Definition=${def} Rename=${def} @@ -36,20 +37,20 @@ JSON Editor Shows Features for Language JSON example.json Diagnostics=Duplicate object key JSX - ${def} = Set Variable xpath:(//span[contains(@class, 'cm-line')][contains(text(), 'hello')])[last()] + ${def} = Set Variable xpath:(//div[contains(@class, 'cm-line')]//text()[contains(., 'hello')])[last()] Editor Shows Features for Language JSX example.jsx Diagnostics=Expression expected ... Jump to Definition=${def} Rename=${def} # Julia -# ${def} = Set Variable xpath:(//span[contains(@class, 'cm-line')][contains(text(), 'add_together')])[last()] +# ${def} = Set Variable xpath:(//div[contains(@class, 'cm-line')]//text()[contains(., 'add_together')])[last()] # Editor Shows Features for Language Julia example.jl Jump to Definition=${def} Rename=${def} LaTeX [Tags] language:latex - ${def} = Set Variable xpath:(//div[contains(@class, 'cm-line')][contains(text(), 'foo')])[last()] + ${def} = Set Variable xpath:(//div[contains(@class, 'cm-line')]//span[contains(text(), 'foo')])[last()] Editor Shows Features for Language LaTeX example.tex Jump to Definition=${def} Rename=${def} Less - ${def} = Set Variable xpath:(//div[contains(@class, 'cm-line')][contains(text(), '@width')])[last()] + ${def} = Set Variable xpath:(//div[contains(@class, 'cm-line')]//text()[contains(., '@width')])[last()] Editor Shows Features for Language Less example.less Diagnostics=Do not use empty rulesets ... Jump to Definition=${def} @@ -57,7 +58,7 @@ Markdown Editor Shows Features for Language Markdown example.md Diagnostics=`Color` is misspelt Python (pylsp) - ${def} = Set Variable xpath:(//div[contains(@class, 'cm-line')][contains(text(), 'fib')])[last()] + ${def} = Set Variable xpath:(//div[contains(@class, 'cm-line')]//text()[contains(., 'fib')])[last()] Editor Shows Features for Server ... pylsp ... Python @@ -67,35 +68,35 @@ Python (pylsp) ... Rename=${def} Python (pyright) - ${def} = Set Variable xpath:(//div[contains(@class, 'cm-line')][contains(text(), 'fib')])[last()] + ${def} = Set Variable xpath:(//div[contains(@class, 'cm-line')]//text()[contains(., 'fib')])[last()] Editor Shows Features for Server pyright Python example.py Diagnostics=is not defined (Pyright) ... Jump to Definition=${def} R - ${def} = Set Variable xpath:(//div[contains(@class, 'cm-line')][contains(text(), 'fib')])[last()] + ${def} = Set Variable xpath:(//div[contains(@class, 'cm-line')]//text()[contains(., 'fib')])[last()] Editor Shows Features for Language R example.R Diagnostics=Put spaces around all infix operators ... Jump to Definition=${def} Robot Framework [Tags] gh:332 ${def} = Set Variable - ... xpath:(//div[contains(@class, 'cm-line')][contains(text(), 'Special Log')])[last()] + ... xpath:(//div[contains(@class, 'cm-line')]//text()[contains(., 'Special Log')])[last()] Editor Shows Features for Language Robot Framework example.robot Diagnostics=Undefined keyword ... Jump to Definition=${def} SCSS ${def} = Set Variable - ... xpath:(//div[contains(@class, 'cm-line')][contains(text(), 'primary-color')])[last()] + ... xpath:(//div[contains(@class, 'cm-line')]//text()[contains(., 'primary-color')])[last()] Editor Shows Features for Language SCSS example.scss Diagnostics=Do not use empty rulesets ... Jump to Definition=${def} TSX - ${def} = Set Variable xpath:(//div[contains(@class, 'cm-line')][contains(text(), 'HelloWorld')])[last()] + ${def} = Set Variable xpath:(//div[contains(@class, 'cm-line')]//text()[contains(., 'HelloWorld')])[last()] Editor Shows Features for Language TSX example.tsx ... Diagnostics='hello' is declared but its value is never read. Jump to Definition=${def} Rename=${def} TypeScript - ${def} = Set Variable xpath:(//div[contains(@class, 'cm-line')][contains(text(), 'inc')])[last()] + ${def} = Set Variable xpath:(//div[contains(@class, 'cm-line')]//text()[contains(., 'inc')])[last()] Editor Shows Features for Language TypeScript example.ts Diagnostics=The left-hand side of an arithmetic ... Jump to Definition=${def} Rename=${def} diff --git a/atest/05_Features/Hover.robot b/atest/05_Features/Hover.robot index 99a6f9275..6430f3e9d 100644 --- a/atest/05_Features/Hover.robot +++ b/atest/05_Features/Hover.robot @@ -84,7 +84,7 @@ Update hover after character deletion Last Occurrence [Arguments] ${symbol} ${sel} = Set Variable If "${symbol}".startswith(("xpath", "css")) ${symbol} - ... xpath:(//div[@class="cm-content"]//*[contains(., "${symbol}")])[last()] + ... xpath:(//div[@class="cm-content"]//text()[contains(., "${symbol}")])[last()] RETURN ${sel} Trigger Automatically By Hover From e495af8ee7e873dea9d6863f868a0db00482339b Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Mon, 28 Aug 2023 22:01:53 +0100 Subject: [PATCH 21/22] Use upstream FormComponent (hides submit button among others) --- packages/jupyterlab-lsp/src/components/serverSettings.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/jupyterlab-lsp/src/components/serverSettings.tsx b/packages/jupyterlab-lsp/src/components/serverSettings.tsx index 82a78579c..15019a6c4 100644 --- a/packages/jupyterlab-lsp/src/components/serverSettings.tsx +++ b/packages/jupyterlab-lsp/src/components/serverSettings.tsx @@ -4,8 +4,9 @@ import { ISchemaValidator } from '@jupyterlab/settingregistry'; import { TranslationBundle } from '@jupyterlab/translation'; +import { FormComponent } from '@jupyterlab/ui-components'; import { ReadonlyPartialJSONObject } from '@lumino/coreutils'; -import Form, { IChangeEvent } from '@rjsf/core'; +import { IChangeEvent } from '@rjsf/core'; import { FieldProps, ObjectFieldTemplateProps, @@ -139,7 +140,7 @@ export class LanguageServerSettings extends React.Component<
    {validationErrors}
) : null} -
Date: Mon, 28 Aug 2023 22:06:45 +0100 Subject: [PATCH 22/22] Improve/lint diagnostics.py --- atest/diagnostics.py | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/atest/diagnostics.py b/atest/diagnostics.py index 0a8ecf74c..4bd37cd24 100644 --- a/atest/diagnostics.py +++ b/atest/diagnostics.py @@ -31,21 +31,29 @@ def wait_until_page_contains_diagnostic(selector, timeout='3s'): sl: SeleniumLibrary = BuiltIn().get_library_instance("SeleniumLibrary") wait = WebDriverWait(sl.driver, timestr_to_secs(timeout)) try: - return wait.until( - partial(page_contains_diagnostic, selector=selector) - ) + return wait.until( + partial(page_contains_diagnostic, selector=selector) + ) except TimeoutException: - elements = sl.driver.find_elements(By.CSS_SELECTOR, f'.{DIAGNOSTIC_CLASS}') - titles = '\n - ' + '\n - '.join([el.get_attribute('title') for el in elements]) - raise TimeoutException( - f'Diagnostic with selector {selector} not found in {timeout}.\nVisible diagnostics are: {titles}' - ) + elements = sl.driver.find_elements(By.CSS_SELECTOR, f'.{DIAGNOSTIC_CLASS}') + if elements: + titles = ( + '\n - ' + + '\n - '.join([el.get_attribute('title') for el in elements]) + ) + hint = f'Visible diagnostics are: {titles}' + else: + hint = 'No diagnostics were visible.' + raise TimeoutException( + f'Diagnostic with selector {selector} not found in {timeout}.' + f'\n{hint}' + ) def wait_until_page_does_not_contain_diagnostic(selector, timeout='3s'): sl: SeleniumLibrary = BuiltIn().get_library_instance("SeleniumLibrary") wait = WebDriverWait(sl.driver, timestr_to_secs(timeout)) return wait.until( - partial(page_contains_diagnostic, selector=selector, negate=True), - f'Diagnostic with selector {selector} still present after {timeout}' + partial(page_contains_diagnostic, selector=selector, negate=True), + f'Diagnostic with selector {selector} still present after {timeout}' )