Merge pull request #142 from krassowski/tests/diagnostics_panel

Add commands to palette, add diagnostics panel tests

CHANGELOG.md

@@ -20,6 +20,9 @@
     in code (on click); accessible from the context menu (
     [#127](https://github.com/krassowski/jupyterlab-lsp/pull/127)
     )
+  - all commands are now accessible from the command palette (
+    [#142](https://github.com/krassowski/jupyterlab-lsp/pull/142)
+    )
 - bugfixes
   - diagnostics in foreign documents are now correctly updated (
     [133fd3d](https://github.com/krassowski/jupyterlab-lsp/pull/129/commits/133fd3d71401c7e5affc0a8637ee157de65bef62)

atest/01_Editor.robot

@@ -12,6 +12,8 @@ ${CM CURSOR}      css:.CodeMirror-cursor
 ${CM CURSORS}     css:.CodeMirror-cursors:not([style='visibility: hidden'])
 ${DIALOG WINDOW}    css:.jp-Dialog
 ${DIALOG INPUT}    css:.jp-Input-Dialog input
+${DIAGNOSTICS PANEL}    css:#lsp-diagnostics-panel
+${DIAGNOSTIC PANEL CLOSE}    css:.p-DockPanel-tabBar .p-TabBar-tab[data-id="lsp-diagnostics-panel"] .p-TabBar-tabCloseIcon
 
 *** Test Cases ***
 Bash
@@ -106,6 +108,13 @@ Editor Should Show Diagnostics
     Set Tags    feature:diagnostics
     Wait Until Page Contains Element    css:.cm-lsp-diagnostic[title*="${diagnostic}"]    timeout=20s
     Capture Page Screenshot    01-diagnostics.png
+    Lab Command    Show Diagnostics Panel
+    Wait Until Page Contains Element    ${DIAGNOSTICS PANEL}    timeout=20s
+    Capture Page Screenshot    02-diagnostics.png
+    ${count} =    Get Element Count    css:.lsp-diagnostics-listing tr
+    SHOULD BE TRUE    ${count} >= 1
+    Mouse Over    ${DIAGNOSTIC PANEL CLOSE}
+    Click Element    ${DIAGNOSTIC PANEL CLOSE}
 
 Editor Should Jump To Definition
     [Arguments]    ${symbol}

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

@@ -363,6 +363,9 @@ export abstract class JupyterLabWidgetAdapter
   }
 
   get_position_from_context_menu(): IRootPosition {
+    // Note: could also try using this.app.contextMenu.menu.contentNode position.
+    // Note: could add a guard on this.app.contextMenu.menu.isAttached
+
     // get the first node as it gives the most accurate approximation
     let leaf_node = this.app.contextMenuHitTest(() => true);
 
@@ -378,6 +381,7 @@ export abstract class JupyterLabWidgetAdapter
       top = event.clientY;
       event.stopPropagation();
     }
+
     return this.virtual_editor.coordsChar(
       {
         left: left,
@@ -387,8 +391,7 @@ export abstract class JupyterLabWidgetAdapter
     ) as IRootPosition;
   }
 
-  get_context_from_context_menu(): ICommandContext {
-    let root_position = this.get_position_from_context_menu();
+  get_context(root_position: IRootPosition): ICommandContext {
     let document = this.virtual_editor.document_at_root_position(root_position);
     let virtual_position = this.virtual_editor.root_position_to_virtual_position(
       root_position
@@ -404,6 +407,11 @@ export abstract class JupyterLabWidgetAdapter
     };
   }
 
+  get_context_from_context_menu(): ICommandContext {
+    let root_position = this.get_position_from_context_menu();
+    return this.get_context(root_position);
+  }
+
   public create_tooltip(
     markup: lsProtocol.MarkupContent,
     cm_editor: CodeMirror.Editor,

packages/jupyterlab-lsp/src/command_manager.spec.ts

@@ -1,13 +1,13 @@
 import { expect } from 'chai';
-import { ContextMenuCommandManager } from './command_manager';
+import { ContextCommandManager, ICommandContext } from './command_manager';
 import {
   CommandEntryPoint,
   IFeatureCommand
 } from './adapters/codemirror/feature';
 import { JupyterLabWidgetAdapter } from './adapters/jupyterlab/jl_adapter';
 
 describe('ContextMenuCommandManager', () => {
-  class ManagerImplementation extends ContextMenuCommandManager {
+  class ManagerImplementation extends ContextCommandManager {
     public get_rank(command: IFeatureCommand): number {
       return super.get_rank(command);
     }
@@ -18,6 +18,10 @@ describe('ContextMenuCommandManager', () => {
     get current_adapter(): JupyterLabWidgetAdapter {
       return undefined;
     }
+
+    context_from_active_document(): ICommandContext {
+      return undefined;
+    }
   }
   let manager: ManagerImplementation;
 
@@ -34,22 +38,22 @@ describe('ContextMenuCommandManager', () => {
 
   describe('#get_rank()', () => {
     it('uses in-group (relative) positioning by default', () => {
-      manager = new ManagerImplementation(null, null, null, 0, 5);
+      manager = new ManagerImplementation(null, null, null, null, 0, 5);
       let rank = manager.get_rank(base_command);
       expect(rank).to.equal(0);
 
       rank = manager.get_rank({ ...base_command, rank: 1 });
       expect(rank).to.equal(1 / 5);
 
-      manager = new ManagerImplementation(null, null, null, 1, 5);
+      manager = new ManagerImplementation(null, null, null, null, 1, 5);
 
       rank = manager.get_rank({ ...base_command, rank: 1 });
       expect(rank).to.equal(1 + 1 / 5);
     });
   });
 
   it('respects is_rank_relative value', () => {
-    manager = new ManagerImplementation(null, null, null, 0, 5);
+    manager = new ManagerImplementation(null, null, null, null, 0, 5);
 
     let rank = manager.get_rank({
       ...base_command,

packages/jupyterlab-lsp/src/command_manager.ts

@@ -1,5 +1,5 @@
 import { JupyterFrontEnd } from '@jupyterlab/application';
-import { IWidgetTracker } from '@jupyterlab/apputils';
+import { ICommandPalette, IWidgetTracker } from '@jupyterlab/apputils';
 import { JupyterLabWidgetAdapter } from './adapters/jupyterlab/jl_adapter';
 import {
   CommandEntryPoint,
@@ -12,8 +12,13 @@ import { NotebookAdapter } from './adapters/jupyterlab/notebook';
 import { INotebookTracker } from '@jupyterlab/notebook';
 import { VirtualDocument } from './virtual/document';
 import { LSPConnection } from './connection';
-import { IRootPosition, IVirtualPosition } from './positioning';
+import {
+  IEditorPosition,
+  IRootPosition,
+  IVirtualPosition
+} from './positioning';
 import { VirtualEditor } from './virtual/editor';
+import { PositionConverter } from './converter';
 
 export const file_editor_adapters: Map<string, FileEditorAdapter> = new Map();
 export const notebook_adapters: Map<string, NotebookAdapter> = new Map();
@@ -30,6 +35,7 @@ function is_context_menu_over_token(adapter: JupyterLabWidgetAdapter) {
 abstract class LSPCommandManager {
   protected constructor(
     protected app: JupyterFrontEnd,
+    protected palette: ICommandPalette,
     protected tracker: IWidgetTracker,
     protected suffix: string
   ) {}
@@ -40,10 +46,13 @@ abstract class LSPCommandManager {
   abstract execute(command: IFeatureCommand): void;
   abstract is_enabled(command: IFeatureCommand): boolean;
   abstract is_visible(command: IFeatureCommand): boolean;
+  add_to_palette: boolean = true;
+  category: string = 'Language Server Protocol';
 
   add(commands: Array<IFeatureCommand>) {
     for (let cmd of commands) {
-      this.app.commands.addCommand(this.create_id(cmd), {
+      let id = this.create_id(cmd);
+      this.app.commands.addCommand(id, {
         execute: () => this.execute(cmd),
         isEnabled: () => this.is_enabled(cmd),
         isVisible: () => this.is_visible(cmd),
@@ -53,6 +62,10 @@ abstract class LSPCommandManager {
       if (this.should_attach(cmd)) {
         this.attach_command(cmd);
       }
+
+      if (this.add_to_palette) {
+        this.palette.addItem({ command: id, category: this.category });
+      }
     }
   }
 
@@ -68,17 +81,22 @@ abstract class LSPCommandManager {
   }
 }
 
-export abstract class ContextMenuCommandManager extends LSPCommandManager {
+/**
+ * Contextual commands, with the context retrieved from the ContextMenu
+ * position (if open) or from the cursor in the current widget.
+ */
+export abstract class ContextCommandManager extends LSPCommandManager {
   abstract selector: string;
 
   constructor(
     app: JupyterFrontEnd,
+    palette: ICommandPalette,
     tracker: IWidgetTracker,
     suffix: string,
     protected rank_group?: number,
     protected rank_group_size?: number
   ) {
-    super(app, tracker, suffix);
+    super(app, palette, tracker, suffix);
   }
 
   attach_command(command: IFeatureCommand): void {
@@ -98,23 +116,63 @@ export abstract class ContextMenuCommandManager extends LSPCommandManager {
   }
 
   execute(command: IFeatureCommand): void {
-    let context = this.current_adapter.get_context_from_context_menu();
-    command.execute(context);
+    let context = this.get_context();
+    if (context) {
+      command.execute(context);
+    }
+  }
+
+  protected get is_context_menu_open(): boolean {
+    return this.app.contextMenu.menu.isAttached;
+  }
+
+  protected get is_widget_current(): boolean {
+    // is the current widget of given type (notebook/editor)
+    // also the currently used widget in the entire app?
+    return (
+      this.tracker.currentWidget !== null &&
+      this.tracker.currentWidget === this.app.shell.currentWidget
+    );
   }
 
   is_enabled() {
-    return is_context_menu_over_token(this.current_adapter);
+    if (this.is_context_menu_open) {
+      return is_context_menu_over_token(this.current_adapter);
+    } else {
+      return this.is_widget_current;
+    }
+  }
+
+  get_context(): ICommandContext | null {
+    let context: ICommandContext = null;
+    if (this.is_context_menu_open) {
+      try {
+        context = this.current_adapter.get_context_from_context_menu();
+      } catch (e) {
+        console.warn(
+          'contextMenu is attached, but could not get the context',
+          e
+        );
+        context = null;
+      }
+    }
+    if (context === null) {
+      context = this.context_from_active_document();
+    }
+    return context;
   }
 
   is_visible(command: IFeatureCommand): boolean {
     try {
-      let context = this.current_adapter.get_context_from_context_menu();
+      let context = this.get_context();
       return (
+        context !== null &&
         this.current_adapter &&
         context.connection &&
         command.is_enabled(context)
       );
     } catch (e) {
+      console.warn('is_visible failed', e);
       return false;
     }
   }
@@ -135,9 +193,11 @@ export abstract class ContextMenuCommandManager extends LSPCommandManager {
       return typeof command.rank !== 'undefined' ? command.rank : Infinity;
     }
   }
+
+  abstract context_from_active_document(): ICommandContext;
 }
 
-export class NotebookCommandManager extends ContextMenuCommandManager {
+export class NotebookCommandManager extends ContextCommandManager {
   protected tracker: INotebookTracker;
   selector = '.jp-Notebook .jp-CodeCell .jp-Editor';
   entry_point = CommandEntryPoint.CellContextMenu;
@@ -146,9 +206,29 @@ export class NotebookCommandManager extends ContextMenuCommandManager {
     let notebook = this.tracker.currentWidget;
     return notebook_adapters.get(notebook.id);
   }
+
+  context_from_active_document(): ICommandContext {
+    if (!this.is_widget_current) {
+      return null;
+    }
+    let notebook = this.tracker.currentWidget;
+    let cell = notebook.content.activeCell;
+    let editor = cell.editor;
+    let ce_cursor = editor.getCursorPosition();
+    let cm_cursor = PositionConverter.ce_to_cm(ce_cursor) as IEditorPosition;
+
+    let virtual_editor = this.current_adapter.virtual_editor;
+
+    let root_position = virtual_editor.transform_from_notebook_to_root(
+      cell,
+      cm_cursor
+    );
+
+    return this.current_adapter.get_context(root_position);
+  }
 }
 
-export class FileEditorCommandManager extends ContextMenuCommandManager {
+export class FileEditorCommandManager extends ContextCommandManager {
   protected tracker: IEditorTracker;
   selector = '.jp-FileEditor';
   entry_point = CommandEntryPoint.FileEditorContextMenu;
@@ -157,6 +237,16 @@ export class FileEditorCommandManager extends ContextMenuCommandManager {
     let fileEditor = this.tracker.currentWidget.content;
     return file_editor_adapters.get(fileEditor.id);
   }
+
+  context_from_active_document(): ICommandContext {
+    if (!this.is_widget_current) {
+      return null;
+    }
+    let editor = this.tracker.currentWidget.content.editor;
+    let ce_cursor = editor.getCursorPosition();
+    let root_position = PositionConverter.ce_to_cm(ce_cursor) as IRootPosition;
+    return this.current_adapter.get_context(root_position);
+  }
 }
 
 export interface ICommandContext {

packages/jupyterlab-lsp/src/index.ts

@@ -109,7 +109,6 @@ const plugin: JupyterFrontEndPlugin<void> = {
       let adapter = null;
       if (notebookTracker.has(current)) {
         let id = (current as NotebookPanel).id;
-        console.warn(id);
         adapter = notebook_adapters.get(id);
       } else if (fileEditorTracker.has(current)) {
         let id = (current as IDocumentWidget<FileEditor>).content.id;
@@ -162,6 +161,7 @@ const plugin: JupyterFrontEndPlugin<void> = {
 
     let command_manager = new FileEditorCommandManager(
       app,
+      palette,
       fileEditorTracker,
       'file_editor'
     );
@@ -189,6 +189,7 @@ const plugin: JupyterFrontEndPlugin<void> = {
     // TODO: PR bumping rank of clear all outputs instead?
     let notebook_command_manager = new NotebookCommandManager(
       app,
+      palette,
       notebookTracker,
       'notebook',
       // adding a very small number (epsilon) places the group just after 10th entry

packages/jupyterlab-lsp/src/virtual/editors/notebook.ts

@@ -41,7 +41,9 @@ class DocDispatcher implements CodeMirror.Doc {
   getCursor(start?: string): CodeMirror.Position {
     let cell = this.virtual_editor.notebook.activeCell;
     let active_editor = cell.editor as CodeMirrorEditor;
-    let cursor = active_editor.editor.getDoc().getCursor(start);
+    let cursor = active_editor.editor
+      .getDoc()
+      .getCursor(start) as IEditorPosition;
     return this.virtual_editor.transform_from_notebook_to_root(cell, cursor);
   }
 }
@@ -94,15 +96,15 @@ export class VirtualEditorForNotebook extends VirtualEditor {
 
   transform_from_notebook_to_root(
     cell: Cell,
-    position: CodeMirror.Position
+    position: IEditorPosition
   ): IRootPosition {
     // TODO: if cell is not known, refresh
     let shift = this.cell_to_corresponding_source_line.get(cell);
     if (shift === undefined) {
       throw Error('Cell not found in cell_line_map');
     }
     return {
-      ...position,
+      ...(position as CodeMirror.Position),
       line: position.line + shift
     } as IRootPosition;
   }
@@ -197,7 +199,7 @@ export class VirtualEditorForNotebook extends VirtualEditor {
         continue;
       }
 
-      return this.transform_from_notebook_to_root(cell, pos);
+      return this.transform_from_notebook_to_root(cell, pos as IEditorPosition);
     }
   }