diff --git a/CHANGELOG.md b/CHANGELOG.md index 85e6e097c..9613bac63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # CHANGELOG +## `@krassowski/jupyterlab-lsp 0.7.1` (unreleased) + +- features + + - users can now choose which columns to display/hide + in the diagnostic panel, using a context menu action ( + [#159](https://github.com/krassowski/jupyterlab-lsp/pull/159) + ) + ## `lsp-ws-connection 0.3.1` - added `sendSaved()` method (textDocument/didSave) ( diff --git a/atest/01_Editor.robot b/atest/01_Editor.robot index 2b926c411..6fd46b8a7 100644 --- a/atest/01_Editor.robot +++ b/atest/01_Editor.robot @@ -112,12 +112,6 @@ Measure Cursor Position ${position} = Wait Until Keyword Succeeds 20 x 0.05s Get Vertical Position ${CM CURSOR} [Return] ${position} -Open Context Menu Over - [Arguments] ${sel} - Wait Until Keyword Succeeds 10 x 0.1 s Mouse Over ${sel} - Wait Until Keyword Succeeds 10 x 0.1 s Click Element ${sel} - Wait Until Keyword Succeeds 10 x 0.1 s Open Context Menu ${sel} - Get Editor Content ${content} Execute JavaScript return document.querySelector('.CodeMirror').CodeMirror.getValue() [Return] ${content} diff --git a/atest/04_Interface/DiagnosticsPanel.robot b/atest/04_Interface/DiagnosticsPanel.robot index 2128bb8d2..033a848b2 100644 --- a/atest/04_Interface/DiagnosticsPanel.robot +++ b/atest/04_Interface/DiagnosticsPanel.robot @@ -5,6 +5,9 @@ Resource ../Keywords.robot *** Variables *** ${EXPECTED_COUNT} 1 ${DIAGNOSTIC} W291 trailing whitespace (pycodestyle) +${DIAGNOSTIC MESSAGE} trailing whitespace +${MENU COLUMNS} xpath://div[contains(@class, 'p-Menu-itemLabel')][contains(text(), "columns")] +${MENU COLUMN MESSAGE} xpath://div[contains(@class, 'p-Menu-itemLabel')][contains(text(), "Message")] *** Test Cases *** Diagnostics Panel Opens @@ -36,6 +39,21 @@ Diagnostics Panel Can Be Restored Wait Until Keyword Succeeds 10 x 1s Should Have Expected Rows Count [Teardown] Clean Up After Working With File Panel.ipynb +Columns Can Be Hidden + [Setup] Gently Reset Workspace + Open Notebook And Panel Panel.ipynb + Wait Until Keyword Succeeds 10 x 1s Element Should Contain ${DIAGNOSTICS PANEL} ${DIAGNOSTIC MESSAGE} + Open Context Menu Over css:.lsp-diagnostics-listing th + Capture Page Screenshot 01-menu-visible.png + Mouse Over ${MENU COLUMNS} + Wait Until Page Contains Element ${MENU COLUMN MESSAGE} timeout=10s + Mouse Over ${MENU COLUMN MESSAGE} + Capture Page Screenshot 02-message-column-visible.png + Click Element ${MENU COLUMN MESSAGE} + Capture Page Screenshot 03-message-column-toggled.png + Wait Until Keyword Succeeds 10 x 1s Element Should Not Contain ${DIAGNOSTICS PANEL} ${DIAGNOSTIC MESSAGE} + [Teardown] Clean Up After Working With File Panel.ipynb + *** Keywords *** Open Notebook And Panel [Arguments] ${notebook} diff --git a/atest/Keywords.robot b/atest/Keywords.robot index ea078aaca..94fe32d44 100644 --- a/atest/Keywords.robot +++ b/atest/Keywords.robot @@ -203,3 +203,9 @@ Enter Cell Editor Wait Until Fully Initialized Wait Until Element Contains ${STATUSBAR} Fully initialized timeout=35s + +Open Context Menu Over + [Arguments] ${sel} + Wait Until Keyword Succeeds 10 x 0.1 s Mouse Over ${sel} + Wait Until Keyword Succeeds 10 x 0.1 s Click Element ${sel} + Wait Until Keyword Succeeds 10 x 0.1 s Open Context Menu ${sel} diff --git a/packages/jupyterlab-lsp/package.json b/packages/jupyterlab-lsp/package.json index 4b6a42cbc..c1d7da38a 100644 --- a/packages/jupyterlab-lsp/package.json +++ b/packages/jupyterlab-lsp/package.json @@ -60,6 +60,7 @@ "@jupyterlab/testutils": "^1.2.2", "@jupyterlab/tooltip": "^1.1", "@phosphor/algorithm": "*", + "@phosphor/widgets": "*", "@types/chai": "^4.1.7", "@types/codemirror": "^0.0.74", "@types/events": "^3.0.0", @@ -93,6 +94,7 @@ "@jupyterlab/testutils": "^1.2.2", "@jupyterlab/tooltip": "^1.1", "@phosphor/algorithm": "*", + "@phosphor/widgets": "*", "codemirror": "*", "react": "*" }, diff --git a/packages/jupyterlab-lsp/src/adapters/codemirror/features/diagnostics.ts b/packages/jupyterlab-lsp/src/adapters/codemirror/features/diagnostics.ts index b8693f605..56c33cebf 100644 --- a/packages/jupyterlab-lsp/src/adapters/codemirror/features/diagnostics.ts +++ b/packages/jupyterlab-lsp/src/adapters/codemirror/features/diagnostics.ts @@ -1,5 +1,6 @@ import * as CodeMirror from 'codemirror'; import * as lsProtocol from 'vscode-languageserver-protocol'; +import { Menu } from '@phosphor/widgets'; import { PositionConverter } from '../../../converter'; import { IVirtualPosition } from '../../../positioning'; import { diagnosticSeverityNames } from '../../../lsp'; @@ -7,6 +8,7 @@ import { DefaultMap } from '../../../utils'; import { CodeMirrorLSPFeature, IFeatureCommand } from '../feature'; import { MainAreaWidget } from '@jupyterlab/apputils'; import { + DIAGNOSTICS_LISTING_CLASS, DiagnosticsDatabase, DiagnosticsListing, IEditorDiagnostic @@ -21,6 +23,7 @@ const default_severity = 2; class DiagnosticsPanel { content: DiagnosticsListing; widget: MainAreaWidget; + is_registered = false; constructor() { this.widget = this.init_widget(); @@ -57,6 +60,8 @@ export const diagnostics_databases = new Map< DiagnosticsDatabase >(); +const CMD_COLUMN_VISIBILITY = 'lsp-set-column-visibility'; + export class Diagnostics extends CodeMirrorLSPFeature { name = 'Diagnostics'; @@ -69,6 +74,44 @@ export class Diagnostics extends CodeMirrorLSPFeature { let panel_widget = diagnostics_panel.widget; + let get_column = (name: string) => { + // TODO: a hashmap in the panel itself? + for (let column of panel_widget.content.columns) { + if (column.name === name) { + return column; + } + } + }; + + if (!diagnostics_panel.is_registered) { + let columns_menu = new Menu({ commands: app.commands }); + app.commands.addCommand(CMD_COLUMN_VISIBILITY, { + execute: args => { + let column = get_column(args['name'] as string); + column.is_visible = !column.is_visible; + panel_widget.update(); + }, + label: args => args['name'] as string, + isToggled: args => { + let column = get_column(args['name'] as string); + return column.is_visible; + } + }); + columns_menu.title.label = 'Panel columns'; + for (let column of panel_widget.content.columns) { + columns_menu.addItem({ + command: CMD_COLUMN_VISIBILITY, + args: { name: column.name } + }); + } + app.contextMenu.addItem({ + selector: '.' + DIAGNOSTICS_LISTING_CLASS + ' th', + submenu: columns_menu, + type: 'submenu' + }); + diagnostics_panel.is_registered = true; + } + if (!panel_widget.isAttached) { app.shell.add(panel_widget, 'main'); } diff --git a/packages/jupyterlab-lsp/src/adapters/codemirror/features/diagnostics_listing.tsx b/packages/jupyterlab-lsp/src/adapters/codemirror/features/diagnostics_listing.tsx index 1d9780b49..323ef7feb 100644 --- a/packages/jupyterlab-lsp/src/adapters/codemirror/features/diagnostics_listing.tsx +++ b/packages/jupyterlab-lsp/src/adapters/codemirror/features/diagnostics_listing.tsx @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { ReactElement } from 'react'; import { VDomModel, VDomRenderer } from '@jupyterlab/apputils'; import * as lsProtocol from 'vscode-languageserver-protocol'; import * as CodeMirror from 'codemirror'; @@ -25,6 +25,8 @@ export interface IEditorDiagnostic { }; } +export const DIAGNOSTICS_LISTING_CLASS = 'lsp-diagnostics-listing'; + export class DiagnosticsDatabase extends Map< VirtualDocument, IEditorDiagnostic[] @@ -108,45 +110,139 @@ interface IDiagnosticsRow { data: IEditorDiagnostic; key: string; document: VirtualDocument; - cell_nr?: number; + /** + * Cell number is the ordinal, 1-based cell identifier displayed to the user. + */ + cell_number?: 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; +interface IListingContext { + db: DiagnosticsDatabase; + editor: VirtualEditor; +} + +interface IColumnOptions { + name: string; + render_cell(data: IDiagnosticsRow, context?: IListingContext): ReactElement; + sort(a: IDiagnosticsRow, b: IDiagnosticsRow): number; + is_available?(context: IListingContext): boolean; +} + +class Column { + public is_visible: boolean; + + constructor(private options: IColumnOptions) { + this.is_visible = true; } -}; + + render_cell(data: IDiagnosticsRow, context: IListingContext) { + return this.options.render_cell(data, context); + } + + sort(a: IDiagnosticsRow, b: IDiagnosticsRow) { + return this.options.sort(a, b); + } + + get name(): string { + return this.options.name; + } + + is_available(context: IListingContext) { + if (typeof this.options.is_available !== 'undefined') { + return this.options.is_available(context); + } + return true; + } + + render_header(listing: DiagnosticsListing): ReactElement { + return ; + } +} + +function SortableTH(props: { name: string; listing: DiagnosticsListing }): any { + const is_sort_key = props.name === props.listing.sort_key; + return ( + props.listing.sort(props.name)} + className={ + is_sort_key + ? 'lsp-sorted ' + + (props.listing.sort_direction === 1 ? 'lsp-descending' : '') + : '' + } + > + {props.name} + {is_sort_key ? : null} + + ); +} export class DiagnosticsListing extends VDomRenderer { sort_key = 'Severity'; sort_direction = 1; + columns = [ + new Column({ + name: 'Virtual Document', + render_cell: (row, context) => ( + + + + ), + sort: (a, b) => a.document.id_path.localeCompare(b.document.id_path), + is_available: context => context.db.size > 1 + }), + new Column({ + name: 'Message', + render_cell: row => { + let message = message_without_code(row.data.diagnostic); + return {message}; + }, + sort: (a, b) => + a.data.diagnostic.message.localeCompare(b.data.diagnostic.message) + }), + new Column({ + name: 'Code', + render_cell: row => {row.data.diagnostic.code}, + sort: (a, b) => + (a.data.diagnostic.code + '').localeCompare( + b.data.diagnostic.source + '' + ) + }), + new Column({ + name: 'Severity', + // TODO: use default diagnostic severity + render_cell: row => ( + {diagnosticSeverityNames[row.data.diagnostic.severity || 1]} + ), + sort: (a, b) => + a.data.diagnostic.severity > b.data.diagnostic.severity ? 1 : -1 + }), + new Column({ + name: 'Source', + render_cell: row => {row.data.diagnostic.source}, + sort: (a, b) => + a.data.diagnostic.source.localeCompare(b.data.diagnostic.source) + }), + new Column({ + name: 'Cell', + render_cell: row => {row.cell_number}, + sort: (a, b) => (a.cell_number > b.cell_number ? 1 : -1), + is_available: context => context.editor.has_cells + }), + new Column({ + name: 'Line', + render_cell: row => {row.data.range.start.line}, + sort: (a, b) => + a.data.range.start.line > b.data.range.start.line ? 1 : -1 + }), + new Column({ + name: 'Ch', + render_cell: row => {row.data.range.start.line}, + sort: (a, b) => (a.data.range.start.ch > b.data.range.start.ch ? 1 : -1) + }) + ]; + constructor(model: DiagnosticsListing.Model) { super(); this.model = model; @@ -168,99 +264,74 @@ export class DiagnosticsListing extends VDomRenderer { 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; + let cell_number: 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; + cell_number = cell_id + 1; } return { data: diagnostic_data, key: virtual_document.uri + ',' + i, document: virtual_document, - cell_nr: cell_nr + cell_number: cell_number, + editor: editor } as IDiagnosticsRow; }); } ); let flattened: IDiagnosticsRow[] = [].concat.apply([], by_document); - let sorter = sorters[this.sort_key]; + let sorted_column = this.columns.filter( + column => column.name === this.sort_key + )[0]; + let sorter = sorted_column.sort.bind(sorted_column); 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 context: IListingContext = { + db: diagnostics_db, + editor: editor + }; - let cm_editor = data.editor; + let columns_to_display = this.columns.filter( + column => column.is_available(context) && column.is_visible + ); + + let elements = sorted.map(row => { + let cm_editor = row.data.editor; - let message = message_without_code(diagnostic); - let severity = diagnostic.severity || 1; + let cells = columns_to_display.map(column => + column.render_cell(row, context) + ); return ( { focus_on(cm_editor.getWrapperElement()); - cm_editor.getDoc().setCursor(data.range.start); + cm_editor.getDoc().setCursor(row.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} + {cells} ); }); - 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} - - ); - }; + let columns_headers = columns_to_display.map(column => + column.render_header(this) + ); return ( - +
- - {documents_count > 1 ? ( - - ) : null} - - - - - {editor.has_cells ? : null} - - - + {columns_headers}{elements}
diff --git a/yarn.lock b/yarn.lock index 7efdf9018..7c86dadae 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1313,10 +1313,10 @@ typestyle "^2.0.1" "@krassowski/jupyterlab-lsp@file:packages/jupyterlab-lsp": - version "0.6.1" + version "0.7.0" dependencies: "@krassowski/jupyterlab_go_to_definition" "^0.7.1" - lsp-ws-connection "*" + lsp-ws-connection "0.3.1" "@krassowski/jupyterlab_go_to_definition@^0.7.1": version "0.7.1" @@ -2168,7 +2168,7 @@ dependencies: "@phosphor/algorithm" "^1.2.0" -"@phosphor/widgets@^1.9.0", "@phosphor/widgets@^1.9.3": +"@phosphor/widgets@*", "@phosphor/widgets@^1.9.0", "@phosphor/widgets@^1.9.3": version "1.9.3" resolved "https://registry.npmjs.org/@phosphor/widgets/-/widgets-1.9.3.tgz#b8b7ad69fd7cc7af8e8c312ebead0e0965a4cefd" integrity sha512-61jsxloDrW/+WWQs8wOgsS5waQ/MSsXBuhONt0o6mtdeL93HVz7CYO5krOoot5owammfF6oX1z0sDaUYIYgcPA== @@ -7346,7 +7346,7 @@ lru-queue@0.1: es5-ext "~0.10.2" "lsp-ws-connection@file:packages/lsp-ws-connection": - version "0.2.0" + version "0.3.1" dependencies: vscode-jsonrpc "^4.1.0-next" vscode-languageclient "^5.2.1"