Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add commands to palette, add diagnostics panel tests #142

Merged
merged 2 commits into from
Jan 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
9 changes: 9 additions & 0 deletions atest/01_Editor.robot
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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}
Expand Down
12 changes: 10 additions & 2 deletions packages/jupyterlab-lsp/src/adapters/jupyterlab/jl_adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -378,6 +381,7 @@ export abstract class JupyterLabWidgetAdapter
top = event.clientY;
event.stopPropagation();
}

return this.virtual_editor.coordsChar(
{
left: left,
Expand All @@ -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
Expand All @@ -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,
Expand Down
14 changes: 9 additions & 5 deletions packages/jupyterlab-lsp/src/command_manager.spec.ts
Original file line number Diff line number Diff line change
@@ -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);
}
Expand All @@ -18,6 +18,10 @@ describe('ContextMenuCommandManager', () => {
get current_adapter(): JupyterLabWidgetAdapter {
return undefined;
}

context_from_active_document(): ICommandContext {
return undefined;
}
}
let manager: ManagerImplementation;

Expand All @@ -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,
Expand Down
112 changes: 101 additions & 11 deletions packages/jupyterlab-lsp/src/command_manager.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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();
Expand All @@ -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
) {}
Expand All @@ -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),
Expand All @@ -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 });
}
}
}

Expand All @@ -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 {
Expand All @@ -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;
}
}
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion packages/jupyterlab-lsp/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -162,6 +161,7 @@ const plugin: JupyterFrontEndPlugin<void> = {

let command_manager = new FileEditorCommandManager(
app,
palette,
fileEditorTracker,
'file_editor'
);
Expand Down Expand Up @@ -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
Expand Down
10 changes: 6 additions & 4 deletions packages/jupyterlab-lsp/src/virtual/editors/notebook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
}
}

Expand Down