Polish document updates, command connection and completer connections

packages/jupyterlab-lsp/src/adapter_manager.ts

@@ -31,7 +31,7 @@ export class WidgetAdapterManager implements ILSPAdapterManager {
     this.adapterDisposed = new Signal(this);
     this.adapterTypeAdded = new Signal(this);
     this.adapterTypes = [];
-    labShell.currentChanged.connect(this.onLabFocusChanged, this);
+    labShell.currentChanged.connect(this.refreshAdapterFromCurrentWidget, this);
   }
 
   public registerAdapterType(options: IAdapterTypeOptions<IDocumentWidget>) {
@@ -70,9 +70,11 @@ export class WidgetAdapterManager implements ILSPAdapterManager {
         this.connectWidget(extension, widget, type);
       }
     });
+    // possibly update the value of currentAdapter using the current widget
+    this.refreshAdapterFromCurrentWidget();
   }
 
-  protected onLabFocusChanged() {
+  protected refreshAdapterFromCurrentWidget() {
     const current = this.labShell.currentWidget as IDocumentWidget;
     if (!current) {
       return;
@@ -87,8 +89,8 @@ export class WidgetAdapterManager implements ILSPAdapterManager {
     }
 
     if (adapter != null) {
-      this.adapterChanged.emit(adapter);
       this.currentAdapter = adapter;
+      this.adapterChanged.emit(adapter);
     }
   }
 

packages/jupyterlab-lsp/src/adapters/adapter.ts

@@ -82,7 +82,6 @@ export abstract class WidgetAdapter<T extends IDocumentWidget> {
 
   protected app: JupyterFrontEnd;
 
-  public documentsUpdateBegan: Signal<WidgetAdapter<T>, void>;
   public activeEditorChanged: Signal<WidgetAdapter<T>, IEditorChangedData>;
   public update_finished: Promise<void>;
 
@@ -105,7 +104,6 @@ export abstract class WidgetAdapter<T extends IDocumentWidget> {
     this.connection_manager = extension.connection_manager;
     this.adapterConnected = new Signal(this);
     this.activeEditorChanged = new Signal(this);
-    this.documentsUpdateBegan = new Signal(this);
     this.adapters = new Map();
     this.status_message = new StatusMessage();
 
@@ -240,7 +238,6 @@ export abstract class WidgetAdapter<T extends IDocumentWidget> {
    * public for use in tests (but otherwise could be private)
    */
   public update_documents() {
-    this.documentsUpdateBegan.emit();
     return this.virtual_editor.virtual_document.update_manager.update_documents(
       this.editors.map(ce_editor => {
         return {
@@ -520,9 +517,10 @@ export abstract class WidgetAdapter<T extends IDocumentWidget> {
     return adapter;
   }
 
-  private onContentChanged(_slot: any) {
+  private async onContentChanged(_slot: any) {
     // update the virtual documents (sending the updates to LSP is out of scope here)
     this.update_finished = this.update_documents().catch(console.warn);
+    await this.update_finished;
   }
 
   get_position_from_context_menu(): IRootPosition {

packages/jupyterlab-lsp/src/command_manager.ts

@@ -179,7 +179,7 @@ export class ContextCommandManager extends LSPCommandManager {
       }
     }
     if (context == null) {
-      context = this.current_adapter.context_from_active_document();
+      context = this.current_adapter?.context_from_active_document();
     }
     return context;
   }

packages/jupyterlab-lsp/src/features/completion.ts

@@ -85,7 +85,6 @@ export class CompletionLabIntegration implements IFeatureLabIntegration {
     public settings: FeatureSettings<LSPCompletionSettings>,
     private adapterManager: ILSPAdapterManager
   ) {
-    // disconnect old adapter, connect new adapter
     adapterManager.adapterChanged.connect(this.swap_adapter, this);
   }
 
@@ -94,20 +93,26 @@ export class CompletionLabIntegration implements IFeatureLabIntegration {
     adapter: WidgetAdapter<IDocumentWidget>
   ) {
     if (this.current_adapter) {
-      this.current_adapter.activeEditorChanged.disconnect(this.set_connector);
-      this.current_adapter.adapterConnected.disconnect(this.connect_completion);
+      // disconnect signals from the old adapter
+      this.current_adapter.activeEditorChanged.disconnect(this.set_connector, this);
+      this.current_adapter.adapterConnected.disconnect(
+        this.connect_completion,
+        this
+      );
     }
     this.current_adapter = adapter;
+    // connect signals to the new adapter
     this.current_adapter.activeEditorChanged.connect(this.set_connector, this);
     this.current_adapter.adapterConnected.connect(
       this.connect_completion,
       this
     );
+    this.set_connector(adapter, {editor: adapter.activeEditor});
   }
 
   connect_completion(
     adapter: WidgetAdapter<IDocumentWidget>,
-    data: IDocumentConnectionData
+    data?: IDocumentConnectionData
   ) {
     let editor = adapter.activeEditor;
     if (editor == null) {
@@ -136,6 +141,10 @@ export class CompletionLabIntegration implements IFeatureLabIntegration {
     adapter: WidgetAdapter<IDocumentWidget>,
     editor_changed: IEditorChangedData
   ) {
+    if (!this.current_completion_handler) {
+      // workaround for current_completion_handler not being there yet
+      this.connect_completion(adapter);
+    }
     this.set_completion_connector(adapter, editor_changed.editor);
     this.current_completion_handler.editor = editor_changed.editor;
     this.current_completion_handler.connector = this.current_completion_connector;

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

@@ -84,6 +84,7 @@ export class HighlightsCM extends CodeMirrorIntegration {
     }
     let root_position: IRootPosition;
 
+    await this.virtual_editor.virtual_document.update_manager.update_done;
     try {
       root_position = this.virtual_editor
         .getDoc()

packages/jupyterlab-lsp/src/index.ts

@@ -128,19 +128,6 @@ export class LSPExtension implements ILSPExtension {
 
     this.feature_manager = new FeatureManager();
 
-    adapterManager.adapterTypeAdded.connect((manager, type) => {
-      let command_manger = new ContextCommandManager({
-        adapter_manager: adapterManager,
-        app: app,
-        palette: palette,
-        tracker: type.tracker,
-        suffix: type.name,
-        entry_point: type.entrypoint,
-        ...type.context_menu
-      });
-      this.feature_manager.registerCommandManager(command_manger);
-    });
-
     this.setting_registry
       .load(plugin.id)
       .then(settings => {
@@ -158,6 +145,19 @@ export class LSPExtension implements ILSPExtension {
       });
 
     adapterManager.registerExtension(this);
+
+    adapterManager.adapterTypeAdded.connect((manager, type) => {
+      let command_manger = new ContextCommandManager({
+        adapter_manager: adapterManager,
+        app: app,
+        palette: palette,
+        tracker: type.tracker,
+        suffix: type.name,
+        entry_point: type.entrypoint,
+        ...type.context_menu
+      });
+      this.feature_manager.registerCommandManager(command_manger);
+    });
   }
 
   private updateOptions(settings: ISettingRegistry.ISettings) {

packages/jupyterlab-lsp/src/virtual/codemirror_editor.ts

@@ -2,7 +2,7 @@ import * as CodeMirror from 'codemirror';
 import { CodeMirrorEditor } from '@jupyterlab/codemirror';
 import { CodeEditor } from '@jupyterlab/codeeditor';
 import { IEditorName } from '../feature';
-import { IBlockAddedInfo, UpdateManager, VirtualDocument } from './document';
+import { IBlockAddedInfo, ICodeBlockOptions, UpdateManager, VirtualDocument } from './document';
 import { IForeignCodeExtractorsRegistry } from '../extractors/types';
 import { create_console, EditorLogConsole } from './console';
 import { Signal } from '@lumino/signaling';
@@ -84,6 +84,7 @@ export class CodeMirrorVirtualEditor
   change: Signal<IVirtualEditor<CodeMirrorEditor>, IEditorChange>;
 
   editor_to_source_line: Map<CodeEditor.IEditor, number>;
+  private editor_to_source_line_new: Map<CodeEditor.IEditor, number>;
 
   private _proxy: CodeMirrorVirtualEditor;
   protected readonly adapter: WidgetAdapter<IDocumentWidget>;
@@ -113,15 +114,16 @@ export class CodeMirrorVirtualEditor
         }
       }
     });
-    this.adapter.documentsUpdateBegan.connect(() => {
-      // this is not thee most efficient, but probably the most reliable way
-      this.onEditorsUpdated(this.adapter.editors);
-    }, this);
+    // this is not thee most efficient, but probably the most reliable way
+    this.virtual_document.update_manager.update_began.connect(this.onEditorsUpdated, this);
 
     this.virtual_document.update_manager.block_added.connect(
       this.save_block_position,
       this
     );
+    this.virtual_document.update_manager.update_finished.connect(() => {
+      this.editor_to_source_line = this.editor_to_source_line_new;
+    }, this);
 
     this.set_event_handlers();
     return this._proxy;
@@ -163,11 +165,12 @@ export class CodeMirrorVirtualEditor
     return this.getDoc().getCursor('end') as IRootPosition;
   }
 
-  private onEditorsUpdated(editors: Array<CodeEditor.IEditor>): void {
+  private onEditorsUpdated(update_manager: UpdateManager, blocks: ICodeBlockOptions[]): void {
     this.cm_editor_to_ce_editor.clear();
     this.ce_editor_to_cm_editor.clear();
-    this.editor_to_source_line.clear();
-    for (let ce_editor of editors) {
+    this.editor_to_source_line_new = new Map();
+    for (let block of blocks) {
+      let ce_editor = block.ce_editor;
       let cm_editor = (ce_editor as CodeMirrorEditor).editor;
       this.cm_editor_to_ce_editor.set(cm_editor, ce_editor);
       this.ce_editor_to_cm_editor.set(ce_editor, cm_editor);
@@ -178,7 +181,7 @@ export class CodeMirrorVirtualEditor
     update_manager: UpdateManager,
     block_data: IBlockAddedInfo
   ) {
-    this.editor_to_source_line.set(
+    this.editor_to_source_line_new.set(
       block_data.block.ce_editor,
       block_data.virtual_document.last_source_line
     );
@@ -449,9 +452,19 @@ export class CodeMirrorVirtualEditor
 
   get_editor_value(editor: CodeEditor.IEditor): string {
     let codemirror_editor = editor as CodeMirrorEditor;
-    let cm_editor = codemirror_editor.editor;
-
-    return cm_editor.getValue();
+    return codemirror_editor.model.value.text;
+    // A previous implementation was using the underlying
+    // CodeMirror editor instance (as one could expect), i.e:
+    //     let cm_editor = codemirror_editor.editor;
+    //     return cm_editor.getValue();
+    // however, because we are listening to:
+    // this.widget.context.model.contentChanged
+    // it turns out that the model can not be as fast to propagate
+    // the changes to the underlying CodeMirror editor instance yet
+    // so it seems reasonable to use the newer value from the model
+    // to build the next VirtualDocument; it turned out to solve some
+    // of the failures to resolve position within the virtual document
+    // which were due to race conditions.
   }
 
   getWrapperElement(): HTMLElement {

packages/jupyterlab-lsp/src/virtual/document.ts

@@ -816,10 +816,16 @@ export class UpdateManager {
    */
   private document_updated: Signal<UpdateManager, VirtualDocument>;
   public block_added: Signal<UpdateManager, IBlockAddedInfo>;
+  // possibly redundant
+  update_done: Promise<void> = new Promise<void>(() => {});
+  update_began: Signal<UpdateManager, ICodeBlockOptions[]>;
+  update_finished: Signal<UpdateManager, ICodeBlockOptions[]>;
 
   constructor(private virtual_document: VirtualDocument) {
     this.document_updated = new Signal(this);
     this.block_added = new Signal(this);
+    this.update_began = new Signal(this);
+    this.update_finished = new Signal(this);
     this.document_updated.connect(this.on_updated, this);
     // TODO singleton
     this.console = create_console('browser');
@@ -871,7 +877,7 @@ export class UpdateManager {
    * as to avoid an easy trap of ignoring the changes in the virtual documents.
    */
   public async update_documents(blocks: ICodeBlockOptions[]): Promise<void> {
-    return new Promise<void>(async (resolve, reject) => {
+    let update = new Promise<void>(async (resolve, reject) => {
       // defer the update by up to 50 ms (10 retrials * 5 ms break),
       // awaiting for the previous update to complete.
       await until_ready(() => this.can_update(), 10, 5).then(() => {
@@ -880,6 +886,7 @@ export class UpdateManager {
         }
         try {
           this.is_update_in_progress = true;
+          this.update_began.emit(blocks);
 
           this.virtual_document.clear();
 
@@ -894,6 +901,8 @@ export class UpdateManager {
             );
           }
 
+          this.update_finished.emit(blocks);
+
           if (this.virtual_document) {
             this.document_updated.emit(this.virtual_document);
             this.virtual_document.maybe_emit_changed();
@@ -908,6 +917,8 @@ export class UpdateManager {
         }
       });
     });
+    this.update_done = update;
+    return update;
   }
 }