From 133fd3d71401c7e5affc0a8637ee157de65bef62 Mon Sep 17 00:00:00 2001 From: krassowski Date: Sun, 5 Jan 2020 16:38:11 +0000 Subject: [PATCH 1/3] Bugfix: changes in foreign documents were not sent to the server before --- packages/jupyterlab-lsp/src/adapters/jupyterlab/jl_adapter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jupyterlab-lsp/src/adapters/jupyterlab/jl_adapter.ts b/packages/jupyterlab-lsp/src/adapters/jupyterlab/jl_adapter.ts index c4aa8848d..af02ddb86 100644 --- a/packages/jupyterlab-lsp/src/adapters/jupyterlab/jl_adapter.ts +++ b/packages/jupyterlab-lsp/src/adapters/jupyterlab/jl_adapter.ts @@ -225,7 +225,7 @@ export abstract class JupyterLabWidgetAdapter await this.connect(virtual_document).catch(console.warn); virtual_document.foreign_document_opened.connect((host, context) => { - this.connect(context.foreign_document).catch(console.warn); + this.connect_document(context.foreign_document).catch(console.warn); }); } From b77f19e7c141a2fa3c25bbc23aa49be9fd2f08d5 Mon Sep 17 00:00:00 2001 From: krassowski Date: Sun, 5 Jan 2020 20:39:11 +0000 Subject: [PATCH 2/3] Implement diagnostics panel, closes #42 --- .../src/adapters/codemirror/feature.spec.ts | 70 +---- .../src/adapters/codemirror/feature.ts | 2 +- .../codemirror/features/diagnostics.spec.ts | 139 ++++++++- .../codemirror/features/diagnostics.ts | 159 ++++++++-- .../features/diagnostics_listing.tsx | 286 ++++++++++++++++++ .../codemirror/features/highlights.ts | 12 +- .../adapters/codemirror/features/rename.ts | 4 +- .../src/adapters/codemirror/testutils.ts | 83 ++++- .../jupyterlab/components/statusbar.tsx | 20 +- .../src/adapters/jupyterlab/jl_adapter.ts | 3 +- .../jupyterlab-lsp/src/command_manager.ts | 15 +- .../jupyterlab-lsp/src/connection_manager.ts | 44 +-- .../jupyterlab-lsp/src/virtual/document.ts | 23 +- .../jupyterlab-lsp/src/virtual/editor.spec.ts | 13 +- .../style/diagnostics_listing.css | 80 +++++ 15 files changed, 794 insertions(+), 159 deletions(-) create mode 100644 packages/jupyterlab-lsp/src/adapters/codemirror/features/diagnostics_listing.tsx create mode 100644 packages/jupyterlab-lsp/style/diagnostics_listing.css diff --git a/packages/jupyterlab-lsp/src/adapters/codemirror/feature.spec.ts b/packages/jupyterlab-lsp/src/adapters/codemirror/feature.spec.ts index 59b3f65c0..8911e9e45 100644 --- a/packages/jupyterlab-lsp/src/adapters/codemirror/feature.spec.ts +++ b/packages/jupyterlab-lsp/src/adapters/codemirror/feature.spec.ts @@ -7,67 +7,20 @@ import { } from '../jupyterlab/jl_adapter'; import { CodeMirrorLSPFeature } from './feature'; import { + code_cell, FeatureTestEnvironment, FileEditorFeatureTestEnvironment, - NotebookFeatureTestEnvironment + getCellsJSON, + NotebookFeatureTestEnvironment, + python_notebook_metadata, + showAllCells, + synchronize_content } from './testutils'; import * as lsProtocol from 'vscode-languageserver-protocol'; import { nbformat } from '@jupyterlab/coreutils'; import { language_specific_overrides } from '../../magics/defaults'; import { foreign_code_extractors } from '../../extractors/defaults'; -import { Notebook, NotebookModel } from '@jupyterlab/notebook'; -import { ICellModel } from '@jupyterlab/cells'; - -function code_cell( - source: string[] | string, - metadata: object = { trusted: false } -) { - return { - cell_type: 'code', - source: source, - metadata: metadata, - execution_count: null, - outputs: [] - } as nbformat.ICodeCell; -} - -const python_notebook_metadata = { - kernelspec: { - display_name: 'Python [default]', - language: 'python', - name: 'python3' - }, - language_info: { - codemirror_mode: { - name: 'ipython', - version: 3 - }, - file_extension: '.py', - mimetype: 'text/x-python', - name: 'python', - nbconvert_exporter: 'python', - pygments_lexer: 'ipython3', - version: '3.5.2' - }, - orig_nbformat: 4.1 -} as nbformat.INotebookMetadata; - -function showAllCells(notebook: Notebook) { - notebook.show(); - // iterate over every cell to activate the editors - for (let i = 0; i < notebook.model.cells.length; i++) { - notebook.activeCellIndex = i; - notebook.activeCell.show(); - } -} - -function getCellsJSON(notebook: Notebook) { - let cells: Array = []; - for (let i = 0; i < notebook.model.cells.length; i++) { - cells.push(notebook.model.cells.get(i)); - } - return cells.map(cell => cell.toJSON()); -} +import { NotebookModel } from '@jupyterlab/notebook'; const js_fib_code = `function fib(n) { return n<2?n:fib(n-1)+fib(n-2); @@ -273,13 +226,8 @@ describe('Feature', () => { environment.dispose(); }); - async function synchronizeContent() { - await environment.virtual_editor.update_documents(); - try { - await adapter.updateAfterChange(); - } catch (e) { - console.warn(e); - } + function synchronizeContent() { + synchronize_content(environment, adapter); } it('applies edit across cells', async () => { diff --git a/packages/jupyterlab-lsp/src/adapters/codemirror/feature.ts b/packages/jupyterlab-lsp/src/adapters/codemirror/feature.ts index 9c546c854..a9803c06b 100644 --- a/packages/jupyterlab-lsp/src/adapters/codemirror/feature.ts +++ b/packages/jupyterlab-lsp/src/adapters/codemirror/feature.ts @@ -112,7 +112,7 @@ export class CodeMirrorLSPFeature implements ILSPFeature { public is_registered: boolean; protected readonly editor_handlers: Map; protected readonly connection_handlers: Map; - protected readonly wrapper_handlers: Map; + protected readonly wrapper_handlers: Map; protected wrapper: HTMLElement; constructor( diff --git a/packages/jupyterlab-lsp/src/adapters/codemirror/features/diagnostics.spec.ts b/packages/jupyterlab-lsp/src/adapters/codemirror/features/diagnostics.spec.ts index 0220bb556..e359e6476 100644 --- a/packages/jupyterlab-lsp/src/adapters/codemirror/features/diagnostics.spec.ts +++ b/packages/jupyterlab-lsp/src/adapters/codemirror/features/diagnostics.spec.ts @@ -1,19 +1,29 @@ import { expect } from 'chai'; import { TextMarker } from 'codemirror'; -import { Diagnostics } from './diagnostics'; -import { FileEditorFeatureTestEnvironment } from '../testutils'; +import { Diagnostics, diagnostics_panel } from './diagnostics'; +import { + code_cell, + FileEditorFeatureTestEnvironment, + NotebookFeatureTestEnvironment, + set_notebook_content, + showAllCells +} from '../testutils'; +import { CodeMirrorEditor } from '@jupyterlab/codemirror'; describe('Diagnostics', () => { - let env: FileEditorFeatureTestEnvironment; + let feature: Diagnostics; - beforeEach(() => (env = new FileEditorFeatureTestEnvironment())); - afterEach(() => env.dispose()); + describe('FileEditor integration', () => { + let env: FileEditorFeatureTestEnvironment; - describe('Works with VirtualFileEditor', () => { - let feature: Diagnostics; - - beforeEach(() => (feature = env.init_feature(Diagnostics))); - afterEach(() => env.dispose_feature(feature)); + beforeEach(() => { + env = new FileEditorFeatureTestEnvironment(); + feature = env.init_feature(Diagnostics); + }); + afterEach(() => { + env.dispose(); + env.dispose_feature(feature); + }); it('calls parent register()', () => { feature.register(); @@ -30,7 +40,7 @@ describe('Diagnostics', () => { expect(markers.length).to.equal(0); feature.handleDiagnostic({ - uri: 'file://foo/dummy.py', + uri: env.path(), diagnostics: [ { range: { @@ -46,4 +56,111 @@ describe('Diagnostics', () => { expect(marks.length).to.equal(1); }); }); + + describe('Notebook integration', () => { + let env: NotebookFeatureTestEnvironment; + + beforeEach(() => { + env = new NotebookFeatureTestEnvironment(); + feature = env.init_feature(Diagnostics); + }); + afterEach(() => { + env.dispose(); + env.dispose_feature(feature); + }); + + it('renders inspections across cells', async () => { + set_notebook_content(env.notebook, [ + code_cell(['x =1\n', 'test']), + code_cell([' ']) + ]); + showAllCells(env.notebook); + await env.virtual_editor.update_documents(); + + let document = env.virtual_editor.virtual_document; + let uri = env.virtual_editor.virtual_document.uri; + + feature.handleDiagnostic({ + uri: uri, + diagnostics: [ + { + source: 'pyflakes', + range: { + start: { line: 1, character: 0 }, + end: { line: 1, character: 5 } + }, + message: "undefined name 'test'", + severity: 1 + }, + { + source: 'pycodestyle', + range: { + start: { line: 0, character: 3 }, + end: { line: 0, character: 5 } + }, + message: 'E225 missing whitespace around operator', + code: 'E225', + severity: 2 + }, + { + source: 'pycodestyle', + range: { + start: { line: 4, character: 0 }, + end: { line: 4, character: 5 } + }, + message: 'W391 blank line at end of file', + code: 'W391', + severity: 2 + }, + { + source: 'pycodestyle', + range: { + start: { line: 4, character: 0 }, + end: { line: 4, character: 5 } + }, + message: 'W293 blank line contains whitespace', + code: 'W293', + severity: 2 + }, + { + source: 'mypy', + range: { + start: { line: 1, character: 0 }, + end: { line: 1, character: 4 } + }, + message: "Name 'test' is not defined", + severity: 1 + } + ] + }); + + let cm_editors = env.virtual_editor.notebook.widgets.map( + cell => (cell.editor as CodeMirrorEditor).editor + ); + let marks_cell_1 = cm_editors[0].getDoc().getAllMarks(); + // test from mypy, test from pyflakes, whitespace around operator from pycodestyle + expect(marks_cell_1.length).to.equal(3); + + // W391 and W293 should get merged into a single diagnostic (same range) + let marks_cell_2 = cm_editors[1].getDoc().getAllMarks(); + expect(marks_cell_2.length).to.equal(1); + + let mark_cell_2 = marks_cell_2[0]; + let merged_mark_title = (mark_cell_2 as any).title; + expect(merged_mark_title).to.contain('W391'); + expect(merged_mark_title).to.contain('W293'); + // should be separated by new line + expect(merged_mark_title).to.contain('\n'); + + expect(feature.diagnostics_db.size).to.equal(1); + expect(feature.diagnostics_db.get(document).length).to.equal(5); + + feature.switchDiagnosticsPanelSource(); + diagnostics_panel.widget.content.update(); + // the panel should contain all 5 diagnostics + let db = diagnostics_panel.content.model.diagnostics; + expect(db.size).to.equal(1); + expect(db.get(document).length).to.equal(5); + }); + }); }); diff --git a/packages/jupyterlab-lsp/src/adapters/codemirror/features/diagnostics.ts b/packages/jupyterlab-lsp/src/adapters/codemirror/features/diagnostics.ts index e8bf5d036..bf6b5a1ed 100644 --- a/packages/jupyterlab-lsp/src/adapters/codemirror/features/diagnostics.ts +++ b/packages/jupyterlab-lsp/src/adapters/codemirror/features/diagnostics.ts @@ -1,34 +1,97 @@ import * as CodeMirror from 'codemirror'; import * as lsProtocol from 'vscode-languageserver-protocol'; import { PositionConverter } from '../../../converter'; -import { IEditorPosition, IVirtualPosition } from '../../../positioning'; +import { IVirtualPosition } from '../../../positioning'; import { diagnosticSeverityNames } from '../../../lsp'; import { DefaultMap } from '../../../utils'; -import { CodeMirrorLSPFeature } from '../feature'; +import { CodeMirrorLSPFeature, IFeatureCommand } from '../feature'; +import { MainAreaWidget } from '@jupyterlab/apputils'; +import { + DiagnosticsDatabase, + DiagnosticsListing, + IEditorDiagnostic +} from './diagnostics_listing'; +import { collect_documents, VirtualDocument } from '../../../virtual/document'; +import { DocumentConnectionManager } from '../../../connection_manager'; +import { VirtualEditor } from '../../../virtual/editor'; // TODO: settings const default_severity = 2; -/** - * Diagnostic which is localized at a specific editor (cell) within a notebook - * (if used in the context of a FileEditor, then there is just a single editor) - */ -export interface IEditorDiagnostic { - diagnostic: lsProtocol.Diagnostic; - editor: CodeMirror.Editor; - range: { - start: IEditorPosition; - end: IEditorPosition; - }; +class DiagnosticsPanel { + content: DiagnosticsListing; + widget: MainAreaWidget; + + constructor() { + this.widget = this.init_widget(); + this.widget.content.disposed.connect(() => { + // immortal widget (or mild memory leak) TODO: rewrite this + this.widget.dispose(); + this.widget = this.init_widget(); + }); + } + + init_widget() { + this.content = new DiagnosticsListing(new DiagnosticsListing.Model()); + this.content.model.diagnostics = new DiagnosticsDatabase(); + this.content.addClass('lsp-diagnostics-panel-content'); + const widget = new MainAreaWidget({ content: this.content }); + widget.id = 'lsp-diagnostics-panel'; + widget.title.label = 'Diagnostics Panel'; + widget.title.closable = true; + return widget; + } + + update() { + // if not attached, do not bother to update + if (!this.widget.isAttached) { + return; + } + this.widget.content.update(); + } } +export const diagnostics_panel = new DiagnosticsPanel(); +export const diagnostics_databases = new Map< + VirtualEditor, + DiagnosticsDatabase +>(); + export class Diagnostics extends CodeMirrorLSPFeature { + static commands: Array = [ + { + id: 'show-diagnostics-panel', + execute: ({ app, features }) => { + let diagnostics_feature = features.get('Diagnostics') as Diagnostics; + diagnostics_feature.switchDiagnosticsPanelSource(); + + let panel_widget = diagnostics_panel.widget; + + if (!panel_widget.isAttached) { + app.shell.add(panel_widget, 'main'); + } + app.shell.activateById(panel_widget.id); + }, + is_enabled: () => true, + label: 'Show diagnostics panel', + rank: 10 + } + ]; + register(): void { this.connection_handlers.set( 'diagnostic', this.handleDiagnostic.bind(this) ); + this.wrapper_handlers.set( + 'focusin', + this.switchDiagnosticsPanelSource.bind(this) + ); this.unique_editor_ids = new DefaultMap(() => this.unique_editor_ids.size); + if (!diagnostics_databases.has(this.virtual_editor)) { + diagnostics_databases.set(this.virtual_editor, new DiagnosticsDatabase()); + } + this.diagnostics_db = diagnostics_databases.get(this.virtual_editor); super.register(); } @@ -40,8 +103,21 @@ export class Diagnostics extends CodeMirrorLSPFeature { * One can use VirtualEditorForNotebook.find_cell_by_editor() to find * the corresponding cell in notebook. * Can be used to implement a Panel showing diagnostics list. + * + * Maps virtual_document.uri to IEditorDiagnostic[]. */ - public diagnostics_db: IEditorDiagnostic[]; + public diagnostics_db: DiagnosticsDatabase; + + switchDiagnosticsPanelSource() { + if ( + diagnostics_panel.content.model.virtual_editor === this.virtual_editor + ) { + return; + } + diagnostics_panel.content.model.diagnostics = this.diagnostics_db; + diagnostics_panel.content.model.virtual_editor = this.virtual_editor; + diagnostics_panel.update(); + } protected collapse_overlapping_diagnostics( diagnostics: lsProtocol.Diagnostic[] @@ -95,7 +171,23 @@ export class Diagnostics extends CodeMirrorLSPFeature { public handleDiagnostic(response: lsProtocol.PublishDiagnosticsParams) { /* TODO: gutters */ try { - let diagnostics_db: IEditorDiagnostic[] = []; + let diagnostics_list: IEditorDiagnostic[] = []; + + let documents = [...collect_documents(this.virtual_document)]; + + let documents_by_uri = new Map( + documents.map(document => { + let key = DocumentConnectionManager.solve_uris(document, '').document; + return [key, document]; + }) + ); + let accept_uris = new Set(documents_by_uri.keys()); + if (!accept_uris.has(response.uri)) { + console.log( + `Ignoring too broadly propagated response ${response.uri}; accepting: ${accept_uris}` + ); + return; + } // Note: no deep equal for Sets or Maps in JS const markers_to_retain: Set = new Set(); @@ -125,17 +217,24 @@ export class Diagnostics extends CodeMirrorLSPFeature { ); return; } - // assuming that we got a response for this document - let start_in_root = this.transform_virtual_position_to_root_position( - start - ); - let document = this.virtual_editor.document_at_root_position( - start_in_root - ); - - // TODO why do I get signals from the other connection in the first place? - // A: because each virtual document adds listeners AND if the extracted content - // is kept in the host document, it remains in the same editor. + + let document: VirtualDocument; + try { + // assuming that we got a response for this document + let start_in_root = this.transform_virtual_position_to_root_position( + start + ); + document = this.virtual_editor.document_at_root_position( + start_in_root + ); + } catch (e) { + console.log(e, diagnostics); + return; + } + + // Note: the guard is important because: + // - each virtual document adds listeners + // - if the extracted content is kept in the host document, it remains in the same editor. if (this.virtual_document !== document) { console.log( `Ignoring inspections from ${response.uri}`, @@ -196,7 +295,7 @@ export class Diagnostics extends CodeMirrorLSPFeature { editor: this.unique_editor_ids.get(cm_editor) }); for (let diagnostic of diagnostics) { - diagnostics_db.push({ + diagnostics_list.push({ diagnostic, editor: cm_editor, range: range_in_editor @@ -233,7 +332,11 @@ export class Diagnostics extends CodeMirrorLSPFeature { // remove the markers which were not included in the new message this.remove_unused_diagnostic_markers(markers_to_retain); - this.diagnostics_db = diagnostics_db; + this.diagnostics_db.set( + documents_by_uri.get(response.uri), + diagnostics_list + ); + diagnostics_panel.update(); } catch (e) { console.warn(e); } diff --git a/packages/jupyterlab-lsp/src/adapters/codemirror/features/diagnostics_listing.tsx b/packages/jupyterlab-lsp/src/adapters/codemirror/features/diagnostics_listing.tsx new file mode 100644 index 000000000..0a8540154 --- /dev/null +++ b/packages/jupyterlab-lsp/src/adapters/codemirror/features/diagnostics_listing.tsx @@ -0,0 +1,286 @@ +import React from 'react'; +import { VDomModel, VDomRenderer } from '@jupyterlab/apputils'; +import * as lsProtocol from 'vscode-languageserver-protocol'; +import * as CodeMirror from 'codemirror'; +import { IEditorPosition } from '../../../positioning'; +import { VirtualEditor } from '../../../virtual/editor'; +import { VirtualEditorForNotebook } from '../../../virtual/editors/notebook'; +import { VirtualDocument } from '../../../virtual/document'; + +import '../../../../style/diagnostics_listing.css'; +import { Cell } from '@jupyterlab/cells'; +import { diagnosticSeverityNames } from '../../../lsp'; + +/** + * Diagnostic which is localized at a specific editor (cell) within a notebook + * (if used in the context of a FileEditor, then there is just a single editor) + */ +export interface IEditorDiagnostic { + diagnostic: lsProtocol.Diagnostic; + editor: CodeMirror.Editor; + range: { + start: IEditorPosition; + end: IEditorPosition; + }; +} + +export class DiagnosticsDatabase extends Map< + VirtualDocument, + IEditorDiagnostic[] +> { + get all(): Array { + return [].concat.apply([], this.values()); + } +} + +function focus_on(node: HTMLElement) { + if (!node) { + return; + } + node.scrollIntoView(); + node.focus(); +} + +function DocumentLocator(props: { + document: VirtualDocument; + editor: VirtualEditor; +}) { + let { document, editor } = props; + let ancestry = document.ancestry; + let target_cell: Cell = null; + let breadcrumbs: any = ancestry.map(document => { + if (!document.parent) { + let path = document.path; + if ( + !editor.has_lsp_supported_file && + path.endsWith(document.file_extension) + ) { + path = path.slice(0, -document.file_extension.length - 1); + } + return {path}; + } + if (!document.virtual_lines.size) { + return Empty document; + } + try { + if (editor.has_cells) { + let first_line = document.virtual_lines.get(0); + let last_line = document.virtual_lines.get( + document.last_virtual_line - 1 + ); + let notebook_editor = editor as VirtualEditorForNotebook; + let { cell_id: first_cell, cell } = notebook_editor.find_cell_by_editor( + first_line.editor + ); + let { cell_id: last_cell } = notebook_editor.find_cell_by_editor( + last_line.editor + ); + target_cell = cell; + + let cell_locator = + first_cell === last_cell + ? `cell ${first_cell + 1}` + : `cells: ${first_cell + 1}-${last_cell + 1}`; + + return ( + + {document.language} ({cell_locator}) + + ); + } + } catch (e) { + console.warn('LSP: could not display document cell location', e); + } + return {document.language}; + }); + return ( +
focus_on(target_cell ? target_cell.node : null)} + > + {breadcrumbs} +
+ ); +} + +interface IDiagnosticsRow { + data: IEditorDiagnostic; + key: string; + document: VirtualDocument; + cell_nr?: number; +} + +const sorters: Record< + string, + (a: IDiagnosticsRow, b: IDiagnosticsRow) => number +> = { + 'Virtual Document': (a: IDiagnosticsRow, b: IDiagnosticsRow) => { + return a.document.id_path.localeCompare(b.document.id_path); + }, + Message: (a: IDiagnosticsRow, b: IDiagnosticsRow) => { + return a.data.diagnostic.message.localeCompare(b.data.diagnostic.message); + }, + Source: (a: IDiagnosticsRow, b: IDiagnosticsRow) => { + return a.data.diagnostic.source.localeCompare(b.data.diagnostic.source); + }, + Code: (a: IDiagnosticsRow, b: IDiagnosticsRow) => { + return (a.data.diagnostic.code + '').localeCompare( + b.data.diagnostic.source + '' + ); + }, + Severity: (a: IDiagnosticsRow, b: IDiagnosticsRow) => { + return a.data.diagnostic.severity > b.data.diagnostic.severity ? 1 : -1; + }, + Line: (a: IDiagnosticsRow, b: IDiagnosticsRow) => { + return a.data.range.start.line > b.data.range.start.line ? 1 : -1; + }, + Ch: (a: IDiagnosticsRow, b: IDiagnosticsRow) => { + return a.data.range.start.ch > b.data.range.start.ch ? 1 : -1; + }, + Cell: (a: IDiagnosticsRow, b: IDiagnosticsRow) => { + return a.cell_nr > b.cell_nr ? 1 : -1; + } +}; + +export class DiagnosticsListing extends VDomRenderer { + sort_key = 'Severity'; + sort_direction = 1; + + constructor(model: DiagnosticsListing.Model) { + super(); + this.model = model; + } + + sort(key: string) { + if (key === this.sort_key) { + this.sort_direction = this.sort_direction * -1; + } else { + this.sort_key = key; + this.sort_direction = 1; + } + this.update(); + } + + render() { + let diagnostics_db = this.model.diagnostics; + const editor = this.model.virtual_editor; + if (!diagnostics_db || typeof editor === 'undefined') { + return
No issues detected, great job!
; + } + let documents_count = diagnostics_db.size; + + let by_document = Array.from(diagnostics_db).map( + ([virtual_document, diagnostics]) => { + return diagnostics.map((diagnostic_data, i) => { + let cell_nr: number = null; + if (editor.has_cells) { + let notebook_editor = editor as VirtualEditorForNotebook; + let { cell_id } = notebook_editor.find_cell_by_editor( + diagnostic_data.editor + ); + cell_nr = cell_id + 1; + } + return { + data: diagnostic_data, + key: virtual_document.uri + ',' + i, + document: virtual_document, + cell_nr: cell_nr + } as IDiagnosticsRow; + }); + } + ); + let flattened: IDiagnosticsRow[] = [].concat.apply([], by_document); + + let sorter = sorters[this.sort_key]; + let sorted = flattened.sort((a, b) => sorter(a, b) * this.sort_direction); + + let elements = sorted.map(({ data, key, document, cell_nr }) => { + let diagnostic = data.diagnostic; + + let cm_editor = data.editor; + + let message = diagnostic.message; + let code = '' + diagnostic.code; + if (message.startsWith(code)) { + message = message.slice(code.length); + } + let severity = diagnostic.severity || 1; + + return ( + { + focus_on(cm_editor.getWrapperElement()); + cm_editor.getDoc().setCursor(data.range.start); + cm_editor.focus(); + }} + > + {documents_count > 1 ? ( + + + + ) : null} + {message} + {diagnostic.code} + {diagnosticSeverityNames[severity]} + {diagnostic.source} + {editor.has_cells ? {cell_nr} : null} + {data.range.start.line} + {data.range.start.ch} + + ); + }); + + let SortableTH = (props: { name: string }): any => { + const is_sort_key = props.name === this.sort_key; + return ( + this.sort(props.name)} + className={ + is_sort_key + ? 'lsp-sorted ' + + (this.sort_direction === 1 ? 'lsp-descending' : '') + : '' + } + > + {props.name} + {is_sort_key ? : null} + + ); + }; + + return ( + + + + {documents_count > 1 ? ( + + ) : null} + + + + + {editor.has_cells ? : null} + + + + + {elements} +
+ ); + } +} + +export namespace DiagnosticsListing { + /** + * A VDomModel for the LSP of current file editor/notebook. + */ + export class Model extends VDomModel { + diagnostics: DiagnosticsDatabase; + virtual_editor: VirtualEditor; + + constructor() { + super(); + } + } +} diff --git a/packages/jupyterlab-lsp/src/adapters/codemirror/features/highlights.ts b/packages/jupyterlab-lsp/src/adapters/codemirror/features/highlights.ts index 1c3118792..03dbe994d 100644 --- a/packages/jupyterlab-lsp/src/adapters/codemirror/features/highlights.ts +++ b/packages/jupyterlab-lsp/src/adapters/codemirror/features/highlights.ts @@ -83,9 +83,13 @@ export class Highlights extends CodeMirrorLSPFeature { if (document !== this.virtual_document) { return; } - let virtual_position = this.virtual_editor.root_position_to_virtual_position( - root_position - ); - this.connection.getDocumentHighlights(virtual_position); + try { + let virtual_position = this.virtual_editor.root_position_to_virtual_position( + root_position + ); + this.connection.getDocumentHighlights(virtual_position); + } catch (e) { + console.warn('Could not get highlights:', e); + } } } diff --git a/packages/jupyterlab-lsp/src/adapters/codemirror/features/rename.ts b/packages/jupyterlab-lsp/src/adapters/codemirror/features/rename.ts index fdab6a393..efd2e3aed 100644 --- a/packages/jupyterlab-lsp/src/adapters/codemirror/features/rename.ts +++ b/packages/jupyterlab-lsp/src/adapters/codemirror/features/rename.ts @@ -107,7 +107,9 @@ function ux_workaround_for_rope_limitation( if (!has_index_error) { return null; } - let dire_python_errors = (diagnostics_feature.diagnostics_db || []).filter( + let dire_python_errors = ( + diagnostics_feature.diagnostics_db.all || [] + ).filter( diagnostic => diagnostic.diagnostic.message.includes('invalid syntax') || diagnostic.diagnostic.message.includes('SyntaxError') || diff --git a/packages/jupyterlab-lsp/src/adapters/codemirror/testutils.ts b/packages/jupyterlab-lsp/src/adapters/codemirror/testutils.ts index 5e98f3573..be8da2cb3 100644 --- a/packages/jupyterlab-lsp/src/adapters/codemirror/testutils.ts +++ b/packages/jupyterlab-lsp/src/adapters/codemirror/testutils.ts @@ -13,11 +13,14 @@ import { StatusMessage } from '../jupyterlab/jl_adapter'; import { VirtualEditorForNotebook } from '../../virtual/editors/notebook'; -import { Notebook } from '@jupyterlab/notebook'; +import { Notebook, NotebookModel } from '@jupyterlab/notebook'; import { NBTestUtils } from '@jupyterlab/testutils'; import { IOverridesRegistry } from '../../magics/overrides'; import { IForeignCodeExtractorsRegistry } from '../../extractors/types'; +import { nbformat } from '@jupyterlab/coreutils'; +import { ICellModel } from '@jupyterlab/cells'; import createNotebook = NBTestUtils.createNotebook; +import { CodeMirrorAdapter } from './cm_adapter'; interface IFeatureTestEnvironment { host: HTMLElement; @@ -145,6 +148,7 @@ export class FileEditorFeatureTestEnvironment extends FeatureTestEnvironment { export class NotebookFeatureTestEnvironment extends FeatureTestEnvironment { public notebook: Notebook; + virtual_editor: VirtualEditorForNotebook; public wrapper: HTMLElement; constructor( @@ -172,3 +176,80 @@ export class NotebookFeatureTestEnvironment extends FeatureTestEnvironment { ); } } + +export function code_cell( + source: string[] | string, + metadata: object = { trusted: false } +) { + return { + cell_type: 'code', + source: source, + metadata: metadata, + execution_count: null, + outputs: [] + } as nbformat.ICodeCell; +} + +export function set_notebook_content( + notebook: Notebook, + cells: nbformat.ICodeCell[], + metadata = python_notebook_metadata +) { + let test_notebook = { + cells: cells, + metadata: metadata + } as nbformat.INotebookContent; + + notebook.model = new NotebookModel(); + notebook.model.fromJSON(test_notebook); +} + +export const python_notebook_metadata = { + kernelspec: { + display_name: 'Python [default]', + language: 'python', + name: 'python3' + }, + language_info: { + codemirror_mode: { + name: 'ipython', + version: 3 + }, + file_extension: '.py', + mimetype: 'text/x-python', + name: 'python', + nbconvert_exporter: 'python', + pygments_lexer: 'ipython3', + version: '3.5.2' + }, + orig_nbformat: 4.1 +} as nbformat.INotebookMetadata; + +export function showAllCells(notebook: Notebook) { + notebook.show(); + // iterate over every cell to activate the editors + for (let i = 0; i < notebook.model.cells.length; i++) { + notebook.activeCellIndex = i; + notebook.activeCell.show(); + } +} + +export function getCellsJSON(notebook: Notebook) { + let cells: Array = []; + for (let i = 0; i < notebook.model.cells.length; i++) { + cells.push(notebook.model.cells.get(i)); + } + return cells.map(cell => cell.toJSON()); +} + +export async function synchronize_content( + environment: FeatureTestEnvironment, + adapter: CodeMirrorAdapter +) { + await environment.virtual_editor.update_documents(); + try { + await adapter.updateAfterChange(); + } catch (e) { + console.warn(e); + } +} diff --git a/packages/jupyterlab-lsp/src/adapters/jupyterlab/components/statusbar.tsx b/packages/jupyterlab-lsp/src/adapters/jupyterlab/components/statusbar.tsx index 16d47a8a2..a4abe1abd 100644 --- a/packages/jupyterlab-lsp/src/adapters/jupyterlab/components/statusbar.tsx +++ b/packages/jupyterlab-lsp/src/adapters/jupyterlab/components/statusbar.tsx @@ -4,22 +4,22 @@ import React from 'react'; -import { VDomRenderer, VDomModel } from '@jupyterlab/apputils'; +import { VDomModel, VDomRenderer } from '@jupyterlab/apputils'; import '../../../../style/statusbar.css'; import * as SCHEMA from '../../../_schema'; import { + GroupItem, interactiveItem, Popup, showPopup, - TextItem, - GroupItem + TextItem } from '@jupyterlab/statusbar'; import { DefaultIconReact } from '@jupyterlab/ui-components'; import { JupyterLabWidgetAdapter } from '../jl_adapter'; -import { VirtualDocument } from '../../../virtual/document'; +import { collect_documents, VirtualDocument } from '../../../virtual/document'; import { LSPConnection } from '../../../connection'; import { PageConfig } from '@jupyterlab/coreutils'; @@ -286,18 +286,6 @@ export interface IStatus { status: StatusCode; } -function collect_documents( - virtual_document: VirtualDocument -): Set { - let collected = new Set(); - collected.add(virtual_document); - for (let foreign of virtual_document.foreign_documents.values()) { - let foreign_languages = collect_documents(foreign); - foreign_languages.forEach(collected.add, collected); - } - return collected; -} - function collect_languages(virtual_document: VirtualDocument): Set { let documents = collect_documents(virtual_document); return new Set( diff --git a/packages/jupyterlab-lsp/src/adapters/jupyterlab/jl_adapter.ts b/packages/jupyterlab-lsp/src/adapters/jupyterlab/jl_adapter.ts index af02ddb86..3cf68465e 100644 --- a/packages/jupyterlab-lsp/src/adapters/jupyterlab/jl_adapter.ts +++ b/packages/jupyterlab-lsp/src/adapters/jupyterlab/jl_adapter.ts @@ -399,7 +399,8 @@ export abstract class JupyterLabWidgetAdapter virtual_position, root_position, features: this.get_features(document), - editor: this.virtual_editor + editor: this.virtual_editor, + app: this.app }; } diff --git a/packages/jupyterlab-lsp/src/command_manager.ts b/packages/jupyterlab-lsp/src/command_manager.ts index 804ea637b..676582be7 100644 --- a/packages/jupyterlab-lsp/src/command_manager.ts +++ b/packages/jupyterlab-lsp/src/command_manager.ts @@ -107,10 +107,16 @@ export abstract class ContextMenuCommandManager extends LSPCommandManager { } is_visible(command: IFeatureCommand): boolean { - let context = this.current_adapter.get_context_from_context_menu(); - return ( - this.current_adapter && context.connection && command.is_enabled(context) - ); + try { + let context = this.current_adapter.get_context_from_context_menu(); + return ( + this.current_adapter && + context.connection && + command.is_enabled(context) + ); + } catch (e) { + return false; + } } protected get_rank(command: IFeatureCommand): number { @@ -154,6 +160,7 @@ export class FileEditorCommandManager extends ContextMenuCommandManager { } export interface ICommandContext { + app: JupyterFrontEnd; document: VirtualDocument; connection: LSPConnection; virtual_position: IVirtualPosition; diff --git a/packages/jupyterlab-lsp/src/connection_manager.ts b/packages/jupyterlab-lsp/src/connection_manager.ts index 0be4e4293..f3086e589 100644 --- a/packages/jupyterlab-lsp/src/connection_manager.ts +++ b/packages/jupyterlab-lsp/src/connection_manager.ts @@ -86,27 +86,13 @@ export class DocumentConnectionManager { this.documents_changed.emit(this.documents); } - protected solve_uris(virtual_document: VirtualDocument, language: string) { - const wsBase = PageConfig.getBaseUrl().replace(/^http/, 'ws'); - const rootUri = PageConfig.getOption('rootUri'); - const virtualDocumentsUri = PageConfig.getOption('virtualDocumentsUri'); - - const baseUri = virtual_document.has_lsp_supported_file - ? rootUri - : virtualDocumentsUri; - - return { - base: baseUri, - document: URLExt.join(baseUri, virtual_document.uri), - server: URLExt.join('ws://jupyter-lsp', language), - socket: URLExt.join(wsBase, 'lsp', language) - }; - } - private connect_socket(options: ISocketConnectionOptions): LSPConnection { let { virtual_document, language } = options; - const uris = this.solve_uris(virtual_document, language); + const uris = DocumentConnectionManager.solve_uris( + virtual_document, + language + ); let socket = new WebSocket(uris.socket); @@ -233,3 +219,25 @@ export class DocumentConnectionManager { this.connections.clear(); } } + +export namespace DocumentConnectionManager { + export function solve_uris( + virtual_document: VirtualDocument, + language: string + ) { + const wsBase = PageConfig.getBaseUrl().replace(/^http/, 'ws'); + const rootUri = PageConfig.getOption('rootUri'); + const virtualDocumentsUri = PageConfig.getOption('virtualDocumentsUri'); + + const baseUri = virtual_document.has_lsp_supported_file + ? rootUri + : virtualDocumentsUri; + + return { + base: baseUri, + document: URLExt.join(baseUri, virtual_document.uri), + server: URLExt.join('ws://jupyter-lsp', language), + socket: URLExt.join(wsBase, 'lsp', language) + }; + } +} diff --git a/packages/jupyterlab-lsp/src/virtual/document.ts b/packages/jupyterlab-lsp/src/virtual/document.ts index 36c844c57..d49e0ae79 100644 --- a/packages/jupyterlab-lsp/src/virtual/document.ts +++ b/packages/jupyterlab-lsp/src/virtual/document.ts @@ -143,9 +143,9 @@ export class VirtualDocument { overrides_registry: IOverridesRegistry, foreign_code_extractors: IForeignCodeExtractorsRegistry, standalone: boolean, - protected file_extension: string, + public file_extension: string, public has_lsp_supported_file: boolean, - private readonly parent?: VirtualDocument + readonly parent?: VirtualDocument ) { this.language = language; let overrides = @@ -570,6 +570,13 @@ export class VirtualDocument { : this.language; } + get ancestry(): Array { + if (!this.parent) { + return [this]; + } + return this.parent.ancestry.concat([this]); + } + get id_path(): VirtualDocument.id_path { if (!this.parent) { return this.virtual_id; @@ -657,3 +664,15 @@ export namespace VirtualDocument { */ export type virtual_id = string; } + +export function collect_documents( + virtual_document: VirtualDocument +): Set { + let collected = new Set(); + collected.add(virtual_document); + for (let foreign of virtual_document.foreign_documents.values()) { + let foreign_languages = collect_documents(foreign); + foreign_languages.forEach(collected.add, collected); + } + return collected; +} diff --git a/packages/jupyterlab-lsp/src/virtual/editor.spec.ts b/packages/jupyterlab-lsp/src/virtual/editor.spec.ts index eedde892d..a59f8c5cb 100644 --- a/packages/jupyterlab-lsp/src/virtual/editor.spec.ts +++ b/packages/jupyterlab-lsp/src/virtual/editor.spec.ts @@ -9,7 +9,6 @@ import { import * as CodeMirror from 'codemirror'; import { PageConfig } from '@jupyterlab/coreutils'; import { DocumentConnectionManager } from '../connection_manager'; -import { VirtualDocument } from './document'; class VirtualEditorImplementation extends VirtualEditor { private cm_editor: CodeMirror.Editor; @@ -75,16 +74,8 @@ describe('VirtualEditor', () => { const virtualDocumentsUri = PageConfig.getOption('virtualDocumentsUri'); expect(rootUri).to.be.not.equal(virtualDocumentsUri); - class DummyConnectionManager extends DocumentConnectionManager { - public get_solved_uris(document: VirtualDocument, language: string) { - return this.solve_uris(document, language); - } - } - - const manager = new DummyConnectionManager(); - let document = editor.virtual_document; - let uris = manager.get_solved_uris(document, 'python'); + let uris = DocumentConnectionManager.solve_uris(document, 'python'); expect(uris.base.startsWith(virtualDocumentsUri)).to.be.equal(true); let editor_with_plain_file = new VirtualEditorImplementation( @@ -96,7 +87,7 @@ describe('VirtualEditor', () => { true ); document = editor_with_plain_file.virtual_document; - uris = manager.get_solved_uris(document, 'python'); + uris = DocumentConnectionManager.solve_uris(document, 'python'); expect(uris.base.startsWith(virtualDocumentsUri)).to.be.equal(false); }); }); diff --git a/packages/jupyterlab-lsp/style/diagnostics_listing.css b/packages/jupyterlab-lsp/style/diagnostics_listing.css new file mode 100644 index 000000000..68121ddfe --- /dev/null +++ b/packages/jupyterlab-lsp/style/diagnostics_listing.css @@ -0,0 +1,80 @@ +.lsp-diagnostics-panel-content { + overflow-y: auto; +} + +.lsp-diagnostics-listing { + color: var(--jp-ui-font-color1); + background: var(--jp-layout-color1); + font-size: var(--jp-ui-font-size1); + border-spacing: 0; + width: 100%; +} + +.lsp-diagnostics-listing thead { + box-shadow: var(--jp-toolbar-box-shadow); +} + +.lsp-diagnostics-listing tbody { + overflow-y: auto; + overflow-x: hidden; +} + +.lsp-diagnostics-listing th { + padding: 4px 12px; + font-weight: 500; + text-align: left; + border-bottom: var(--jp-border-width) solid var(--jp-border-color1); + background: var(--jp-layout-color1); + position: sticky; + top: 0; + z-index: 2; + white-space: nowrap; + user-select: none; +} + +.lsp-diagnostics-listing th:not(:first-child) { + border-left: var(--jp-border-width) solid var(--jp-border-color2); +} + +.lsp-diagnostics-listing th:hover { + background: var(--jp-layout-color2); +} + +.lsp-document-locator span:not(:last-child)::after { + content: ' > '; +} + +.lsp-diagnostics-listing td:first-child,.lsp-diagnostics-listing td:last-child { + padding: 4px 12px; +} + +.lsp-diagnostics-listing tbody td:not(:first-child):not(:last-child) { + padding: 4px 2px; +} + +.lsp-diagnostics-listing tbody tr:hover { + background: var(--jp-layout-color2); +} + +.lsp-sort-caret +{ + height: 16px; + width: 16px; + background-size: 18px; + background-image: 18px; + background-repeat: no-repeat; + background-position: center; + display: inline-block; + position: relative; + top: 3px; +} + +.lsp-sorted .lsp-caret { + background-image: var(--jp-icon-caretup); + margin-top: -4px; + left: 3px; +} + +.lsp-sorted.lsp-descending .lsp-caret { + background-image: var(--jp-icon-caretdown); +} From 2927fb053f449eafe49863b28df5f5deca750e40 Mon Sep 17 00:00:00 2001 From: krassowski Date: Sun, 5 Jan 2020 23:50:45 +0000 Subject: [PATCH 3/3] Add changelog entry for diagnostics panel, lint --- CHANGELOG.md | 5 ++ .../src/adapters/codemirror/feature.spec.ts | 4 +- .../style/diagnostics_listing.css | 82 +++++++++---------- 3 files changed, 48 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a27f303e..26f257024 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,11 @@ - add 'rename' function for notebooks, using shadow filesystem ( [#115](https://github.com/krassowski/jupyterlab-lsp/pull/115) ) +- add a widget panel with diagnostics (inspections), allowing to + sort and explore diagnostics, and to go-to the respective location + in code (on click); accessible from the context menu ( + [#129](https://github.com/krassowski/jupyterlab-lsp/pull/129) + ) ## `lsp-ws-connection` (unreleased) diff --git a/packages/jupyterlab-lsp/src/adapters/codemirror/feature.spec.ts b/packages/jupyterlab-lsp/src/adapters/codemirror/feature.spec.ts index 8911e9e45..8d476c37a 100644 --- a/packages/jupyterlab-lsp/src/adapters/codemirror/feature.spec.ts +++ b/packages/jupyterlab-lsp/src/adapters/codemirror/feature.spec.ts @@ -226,8 +226,8 @@ describe('Feature', () => { environment.dispose(); }); - function synchronizeContent() { - synchronize_content(environment, adapter); + async function synchronizeContent() { + await synchronize_content(environment, adapter); } it('applies edit across cells', async () => { diff --git a/packages/jupyterlab-lsp/style/diagnostics_listing.css b/packages/jupyterlab-lsp/style/diagnostics_listing.css index 68121ddfe..2a167ee69 100644 --- a/packages/jupyterlab-lsp/style/diagnostics_listing.css +++ b/packages/jupyterlab-lsp/style/diagnostics_listing.css @@ -1,80 +1,80 @@ .lsp-diagnostics-panel-content { - overflow-y: auto; + overflow-y: auto; } .lsp-diagnostics-listing { - color: var(--jp-ui-font-color1); - background: var(--jp-layout-color1); - font-size: var(--jp-ui-font-size1); - border-spacing: 0; - width: 100%; + color: var(--jp-ui-font-color1); + background: var(--jp-layout-color1); + font-size: var(--jp-ui-font-size1); + border-spacing: 0; + width: 100%; } .lsp-diagnostics-listing thead { - box-shadow: var(--jp-toolbar-box-shadow); + box-shadow: var(--jp-toolbar-box-shadow); } .lsp-diagnostics-listing tbody { - overflow-y: auto; - overflow-x: hidden; + overflow-y: auto; + overflow-x: hidden; } .lsp-diagnostics-listing th { - padding: 4px 12px; - font-weight: 500; - text-align: left; - border-bottom: var(--jp-border-width) solid var(--jp-border-color1); - background: var(--jp-layout-color1); - position: sticky; - top: 0; - z-index: 2; - white-space: nowrap; - user-select: none; + padding: 4px 12px; + font-weight: 500; + text-align: left; + border-bottom: var(--jp-border-width) solid var(--jp-border-color1); + background: var(--jp-layout-color1); + position: sticky; + top: 0; + z-index: 2; + white-space: nowrap; + user-select: none; } .lsp-diagnostics-listing th:not(:first-child) { - border-left: var(--jp-border-width) solid var(--jp-border-color2); + border-left: var(--jp-border-width) solid var(--jp-border-color2); } .lsp-diagnostics-listing th:hover { - background: var(--jp-layout-color2); + background: var(--jp-layout-color2); } .lsp-document-locator span:not(:last-child)::after { - content: ' > '; + content: ' > '; } -.lsp-diagnostics-listing td:first-child,.lsp-diagnostics-listing td:last-child { - padding: 4px 12px; +.lsp-diagnostics-listing td:first-child, +.lsp-diagnostics-listing td:last-child { + padding: 4px 12px; } .lsp-diagnostics-listing tbody td:not(:first-child):not(:last-child) { - padding: 4px 2px; + padding: 4px 2px; } .lsp-diagnostics-listing tbody tr:hover { - background: var(--jp-layout-color2); + background: var(--jp-layout-color2); } -.lsp-sort-caret -{ - height: 16px; - width: 16px; - background-size: 18px; - background-image: 18px; - background-repeat: no-repeat; - background-position: center; - display: inline-block; - position: relative; - top: 3px; +.lsp-sort-caret { + height: 16px; + width: 16px; + background-size: 18px; + background-image: 18px; + background-repeat: no-repeat; + background-position: center; + display: inline-block; + position: relative; + top: 3px; } .lsp-sorted .lsp-caret { - background-image: var(--jp-icon-caretup); - margin-top: -4px; - left: 3px; + background-image: var(--jp-icon-caretup); + margin-top: -4px; + left: 3px; } .lsp-sorted.lsp-descending .lsp-caret { - background-image: var(--jp-icon-caretdown); + background-image: var(--jp-icon-caretdown); }