diff --git a/CHANGELOG.md b/CHANGELOG.md index a51b8d8ad..e00a6f50a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,8 +16,12 @@ - handles characters that need escaping (spaces, non-ASCII characters) more robustly in files and folder names ([#403]) + - moving cells now triggers the document update immediately leading to immediate diagnostics update ([#421]) + - changing cell type to `raw` or `markdown` and then back to `code` properly unbinds/binds event handlers and updates document ([#421]) + - pasted cells are added to the LSP document immediately, without the need for the user to enter them ([#421]) [#403]: https://github.com/krassowski/jupyterlab-lsp/issues/403 +[#421]: https://github.com/krassowski/jupyterlab-lsp/issues/421 ### `@krassowski/jupyterlab_go_to_definition 2.0.0` (???) diff --git a/atest/03_Notebook.robot b/atest/03_Notebook.robot index 17865a775..8507eb5cf 100644 --- a/atest/03_Notebook.robot +++ b/atest/03_Notebook.robot @@ -11,6 +11,39 @@ Python Capture Page Screenshot 01-python.png [Teardown] Clean Up After Working With File Python.ipynb +Conversion Of Cell Types + [Setup] Setup Notebook Python Python.ipynb + ${lsp_entry} = Set Variable Show diagnostics panel + # initial (code) cell + Open Context Menu Over Cell Editor 1 + Capture Page Screenshot 01-initial-code-cell.png + Context Menu Should Contain ${lsp_entry} + Close Context Menu + # raw cell + Lab Command Change to Raw Cell Type + Open Context Menu Over Cell Editor 1 + Capture Page Screenshot 02-as-raw-cell.png + Context Menu Should Not Contain ${lsp_entry} + Close Context Menu + # code cell again + Lab Command Change to Code Cell Type + Open Context Menu Over Cell Editor 1 + Capture Page Screenshot 03-as-code-cell-again.png + Context Menu Should Contain ${lsp_entry} + Close Context Menu + [Teardown] Clean Up After Working With File Python.ipynb + +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 + Enter Cell Editor 1 + Lab Command Move Cells Down + Wait Until Page Does Not Contain Element css:.cm-lsp-diagnostic[title="${diagnostic}"] timeout=35s + [Teardown] Clean Up After Working With File Python.ipynb + Foreign Extractors ${file} = Set Variable Foreign extractors.ipynb Configure JupyterLab Plugin diff --git a/atest/Keywords.robot b/atest/Keywords.robot index 11fbb7ad4..25525f944 100644 --- a/atest/Keywords.robot +++ b/atest/Keywords.robot @@ -279,6 +279,11 @@ Enter Cell Editor Click Element css:.jp-Cell:nth-child(${cell_nr}) .CodeMirror-line:nth-child(${line}) Wait Until Page Contains Element css:.jp-Cell:nth-child(${cell_nr}) .CodeMirror-focused +Open Context Menu Over Cell Editor + [Arguments] ${cell_nr} ${line}=1 + Enter Cell Editor ${cell_nr} line=${line} + Open Context Menu Over css:.jp-Cell:nth-child(${cell_nr}) .CodeMirror-line:nth-child(${line}) + Place Cursor In Cell Editor At [Arguments] ${cell_nr} ${line} ${character} Enter Cell Editor ${cell_nr} ${line} @@ -301,6 +306,19 @@ Open Context Menu Over Wait Until Keyword Succeeds 10 x 0.1 s Mouse Over ${sel} Wait Until Keyword Succeeds 10 x 0.1 s Open Context Menu ${sel} +Context Menu Should Contain + [Arguments] ${label} ${timeout}=10s + ${entry} Set Variable xpath://div[contains(@class, 'lm-Menu-itemLabel')][contains(text(), '${label}')] + Wait Until Page Contains Element ${entry} timeout=${timeout} + +Context Menu Should Not Contain + [Arguments] ${label} ${timeout}=10s + ${entry} Set Variable xpath://div[contains(@class, 'lm-Menu-itemLabel')][contains(text(), '${label}')] + Wait Until Page Does Not Contain Element ${entry} timeout=${timeout} + +Close Context Menu + Press Keys None ESCAPE + Prepare File for Editing [Arguments] ${Language} ${Screenshots} ${file} Set Tags language:${Language.lower()} diff --git a/packages/jupyterlab-lsp/src/adapters/adapter.ts b/packages/jupyterlab-lsp/src/adapters/adapter.ts index d9d4c308d..c5de62291 100644 --- a/packages/jupyterlab-lsp/src/adapters/adapter.ts +++ b/packages/jupyterlab-lsp/src/adapters/adapter.ts @@ -96,6 +96,8 @@ export abstract class WidgetAdapter<T extends IDocumentWidget> { protected app: JupyterFrontEnd; public activeEditorChanged: Signal<WidgetAdapter<T>, IEditorChangedData>; + public editorAdded: Signal<WidgetAdapter<T>, IEditorChangedData>; + public editorRemoved: Signal<WidgetAdapter<T>, IEditorChangedData>; public update_finished: Promise<void>; /** @@ -117,6 +119,8 @@ export abstract class WidgetAdapter<T extends IDocumentWidget> { this.connection_manager = extension.connection_manager; this.adapterConnected = new Signal(this); this.activeEditorChanged = new Signal(this); + this.editorRemoved = new Signal(this); + this.editorAdded = new Signal(this); this.adapters = new Map(); this.status_message = new StatusMessage(); this.isConnected = false; diff --git a/packages/jupyterlab-lsp/src/adapters/notebook/notebook.ts b/packages/jupyterlab-lsp/src/adapters/notebook/notebook.ts index 46d08f182..b1dfdf71a 100644 --- a/packages/jupyterlab-lsp/src/adapters/notebook/notebook.ts +++ b/packages/jupyterlab-lsp/src/adapters/notebook/notebook.ts @@ -1,7 +1,7 @@ import { WidgetAdapter } from '../adapter'; import { Notebook, NotebookPanel } from '@jupyterlab/notebook'; import { until_ready } from '../../utils'; -import { Cell } from '@jupyterlab/cells'; +import { Cell, ICellModel } from '@jupyterlab/cells'; import * as nbformat from '@jupyterlab/nbformat'; import ILanguageInfoMetadata = nbformat.ILanguageInfoMetadata; import { Session } from '@jupyterlab/services'; @@ -17,13 +17,16 @@ import { VirtualDocument } from '../../virtual/document'; export class NotebookAdapter extends WidgetAdapter<NotebookPanel> { editor: Notebook; private ce_editor_to_cell: Map<IEditor, Cell>; + private known_editors_ids: Set<string>; private _language_info: ILanguageInfoMetadata; + private type: nbformat.CellType = 'code'; constructor(extension: LSPExtension, editor_widget: NotebookPanel) { super(extension, editor_widget); this.ce_editor_to_cell = new Map(); this.editor = editor_widget.content; + this.known_editors_ids = new Set(); this.init_once_ready().catch(console.warn); } @@ -136,6 +139,83 @@ export class NotebookAdapter extends WidgetAdapter<NotebookPanel> { ); this.widget.content.activeCellChanged.connect(this.activeCellChanged, this); + this.widget.model.cells.changed.connect(async (cells, change) => { + let cellsAdded: ICellModel[] = []; + let cellsRemoved: ICellModel[] = []; + const type = this.type; + + if (change.type === 'set') { + // handling of conversions is important, because the editors get re-used and their handlers inherited, + // so we need to clear our handlers from editors of e.g. markdown cells which previously were code cells. + let convertedToMarkdownOrRaw = []; + let convertedToCode = []; + + if (change.newValues.length === change.oldValues.length) { + // during conversion the cells should not get deleted nor added + for (let i = 0; i < change.newValues.length; i++) { + if ( + change.oldValues[i].type === type && + change.newValues[i].type !== type + ) { + convertedToMarkdownOrRaw.push(change.newValues[i]); + } else if ( + change.oldValues[i].type !== type && + change.newValues[i].type === type + ) { + convertedToCode.push(change.newValues[i]); + } + } + cellsAdded = convertedToCode; + cellsRemoved = convertedToMarkdownOrRaw; + } + } else if (change.type == 'add') { + cellsAdded = change.newValues.filter( + cellModel => cellModel.type === type + ); + } + // note: editorRemoved is not emitted for removal of cells by change of type 'remove' (but only during cell type conversion) + // because there is no easy way to get the widget associated with the removed cell(s) - because it is no + // longer in the notebook widget list! It would need to be tracked on our side, but it is not necessary + // as (except for a tiny memory leak) it should not impact the functionality in any way + + if ( + cellsRemoved.length || + cellsAdded.length || + change.type === 'move' || + change.type === 'remove' + ) { + // in contrast to the file editor document which can be only changed by the modification of the editor content, + // the notebook document cna also get modified by a change in the number or arrangement of editors themselves; + // for this reason each change has to trigger documents update (so that LSP mirror is in sync). + await this.update_documents(); + } + + for (let cellModel of cellsRemoved) { + let cellWidget = this.widget.content.widgets.find( + cell => cell.model.id === cellModel.id + ); + this.known_editors_ids.delete(cellWidget.editor.uuid); + + // for practical purposes this editor got removed from our consideration; + // it might seem that we should instead look for the editor indicated by + // the oldValues[i] cellModel, but this one got already transferred to the + // markdown cell in newValues[i] + this.editorRemoved.emit({ + editor: cellWidget.editor + }); + } + + for (let cellModel of cellsAdded) { + let cellWidget = this.widget.content.widgets.find( + cell => cell.model.id === cellModel.id + ); + this.known_editors_ids.add(cellWidget.editor.uuid); + + this.editorAdded.emit({ + editor: cellWidget.editor + }); + } + }); } get editors(): CodeEditor.IEditor[] { @@ -178,6 +258,15 @@ export class NotebookAdapter extends WidgetAdapter<NotebookPanel> { } private activeCellChanged(notebook: Notebook, cell: Cell) { + if (cell.model.type !== this.type) { + return; + } + if (!this.known_editors_ids.has(cell.editor.uuid)) { + this.known_editors_ids.add(cell.editor.uuid); + this.editorAdded.emit({ + editor: cell.editor + }); + } this.activeEditorChanged.emit({ editor: cell.editor }); @@ -185,6 +274,13 @@ export class NotebookAdapter extends WidgetAdapter<NotebookPanel> { context_from_active_document(): ICommandContext | null { let cell = this.widget.content.activeCell; + if (cell.model.type !== this.type) { + // context will be sought on all cells to verify if the context menu should be visible, + // thus it is ok to just return null; it seems to stem from the implementation detail + // upstream, i.e. the markdown cells appear to be created by transforming the code cells + // but do not quote me on that. + return null; + } let editor = cell.editor; let ce_cursor = editor.getCursorPosition(); let cm_cursor = PositionConverter.ce_to_cm(ce_cursor) as IEditorPosition; diff --git a/packages/jupyterlab-lsp/src/virtual/codemirror_editor.ts b/packages/jupyterlab-lsp/src/virtual/codemirror_editor.ts index c3087af1f..04821b411 100644 --- a/packages/jupyterlab-lsp/src/virtual/codemirror_editor.ts +++ b/packages/jupyterlab-lsp/src/virtual/codemirror_editor.ts @@ -280,9 +280,15 @@ export class CodeMirrorVirtualEditor }; this._event_wrappers.set([eventName, handler], wrapped_handler); - this.forEveryBlockEditor(cm_editor => { - cm_editor.on(eventName, wrapped_handler); - }); + this.forEveryBlockEditor( + cm_editor => { + cm_editor.on(eventName, wrapped_handler); + }, + true, + cm_editor => { + cm_editor.off(eventName, wrapped_handler); + } + ); } off(eventName: string, handler: CodeMirrorHandler, ...args: any[]): void { @@ -496,28 +502,25 @@ export class CodeMirrorVirtualEditor return 0; } - addEventListener(type: string, listener: EventListenerOrEventListenerObject) { - this.forEveryBlockEditor(cm_editor => { - cm_editor.getWrapperElement().addEventListener(type, listener); - }); - } - forEveryBlockEditor( callback: (cm_editor: CodeMirror.Editor) => any, - monitor_for_new_blocks = true + monitor_for_new_blocks = true, + on_editor_removed_callback: (cm_editor: CodeMirror.Editor) => any = null ) { const editors_with_handlers = new Set<CodeMirror.Editor>(); - // TODO... the need of iterating over all editors is universal. How does the virtual - // editor gets knowledge of the editor instances? From the adapter obviously. + // TODO... the need of iterating over all editors is universal - so this could be + // generalised to the VirtualEditor rather than live in CodeMirrorVirtualEditor; + // How would the VirtualEditor get knowledge of the editor instances? + // From the adapter (obviously). for (let editor of this.adapter.editors) { let cm_editor = (editor as CodeMirrorEditor).editor; editors_with_handlers.add(cm_editor); callback(cm_editor); } if (monitor_for_new_blocks) { - this.adapter.activeEditorChanged.connect( - (adapter, data: IEditorChangedData) => { + this.adapter.editorAdded.connect( + (adapter: WidgetAdapter<IDocumentWidget>, data: IEditorChangedData) => { let { editor } = data; if (editor == null) { return; @@ -528,6 +531,16 @@ export class CodeMirrorVirtualEditor } } ); + this.adapter.editorRemoved.connect( + (adapter, data: IEditorChangedData) => { + let { editor } = data; + if (editor == null) { + return; + } + let cm_editor = (editor as CodeMirrorEditor).editor; + on_editor_removed_callback(cm_editor); + } + ); } }