diff --git a/README.md b/README.md index 8dd816224..7b4c9ed40 100644 --- a/README.md +++ b/README.md @@ -28,9 +28,9 @@ over the underlined code to see a more detailed message ![inspections](https://raw.githubusercontent.com/jupyter-lsp/jupyterlab-lsp/master/examples/screenshots/inspections.png) -### Jump to Definition +### Jump to Definition and References -Use the context menu entry, or Alt + :computer_mouse: to jump to definitions (you can change it to Ctrl/⌘ in settings); use Alt + o to jump back +Use the context menu entry, or Alt + :computer_mouse: to jump to definitions/references (you can change it to Ctrl/⌘ in settings); use Alt + o to jump back. ![jump](https://raw.githubusercontent.com/jupyter-lsp/jupyterlab-lsp/master/examples/screenshots/jump_to_definition.png) diff --git a/atest/04_Interface/DiagnosticsPanel.robot b/atest/04_Interface/DiagnosticsPanel.robot index 3657b47a1..721f7775b 100644 --- a/atest/04_Interface/DiagnosticsPanel.robot +++ b/atest/04_Interface/DiagnosticsPanel.robot @@ -12,7 +12,6 @@ ${DIAGNOSTIC MESSAGE} trailing whitespace ${DIAGNOSTIC MESSAGE R} Closing curly-braces should always be on their own line ${R CELL} %%R\n{} ${MENU COLUMNS} xpath://div[contains(@class, 'lm-Menu-itemLabel')][contains(text(), "columns")] -${LAB MENU} css:.lm-Menu *** Test Cases *** Diagnostics Panel Opens @@ -119,28 +118,6 @@ Open Context Menu Over W291 Table Cell Should Equal W291 row=-1 column=2 Open Context Menu Over css:.lsp-diagnostics-listing tbody > tr:last-child -Expand Menu Entry - [Arguments] ${label} - ${entry} = Set Variable xpath://div[contains(@class, 'lm-Menu-itemLabel')][contains(text(), "${label}")] - Wait Until Page Contains Element ${entry} timeout=10s - ${menus before} = Get Element Count ${LAB MENU} - Mouse Over ${entry} - ${expected menus} = Evaluate ${menus before} + 1 - Wait Until Keyword Succeeds 10 x 1s Menus Count Equal ${expected menus} - -Menus Count Equal - [Arguments] ${count} - ${menus count} = Get Element Count ${LAB MENU} - Should Be Equal ${menus count} ${count} - -Select Menu Entry - [Arguments] ${label} - ${entry} Set Variable xpath://div[contains(@class, 'lm-Menu-itemLabel')][contains(text(), '${label}')] - Wait Until Page Contains Element ${entry} timeout=10s - Mouse Over ${entry} - Click Element ${entry} - Wait Until Page Does Not Contain Element ${entry} timeout=10s - Open Notebook And Panel [Arguments] ${notebook} Setup Notebook Python ${notebook} diff --git a/atest/05_Features/Jump.robot b/atest/05_Features/Jump.robot index 3b2b4214e..8de249436 100644 --- a/atest/05_Features/Jump.robot +++ b/atest/05_Features/Jump.robot @@ -4,31 +4,60 @@ Force Tags feature:jump-to-definition gh:403 Resource ../Keywords.robot *** Variables *** -${FOLDER WITH SPACE} a földer +${FOLDER WITH SPACE} a föl@der *** Test Cases *** -Python Jumps between Files +Python Jumps Between Files Copy Files to Folder With Spaces jump_a.py jump_b.py - ${def} = Set Variable a_function_definition Open ${FOLDER WITH SPACE}/jump_b.py in ${MENU EDITOR} Wait Until Fully Initialized - ${sel} = Set Variable xpath:(//span[contains(@class, 'cm-variable')][contains(text(), '${def}')])[last()] + ${sel} = Select Token Occurrence a_function_definition Jump To Definition ${sel} Wait Until Page Contains ANOTHER_CONSTANT Capture Page Screenshot 10-jumped.png Clean Up After Working With File jump_b.py +Jumps To References With Modifier Click + [Setup] Prepare File for Editing Python editor jump_references.py + Configure JupyterLab Plugin {"modifierKey": "Accel"} plugin id=${JUMP PLUGIN ID} + Wait Until Fully Initialized + ${token} = Select Token Occurrence func type=def + Click Element ${token} + ${original} = Measure Cursor Position + Ctrl Click Element ${token} + Wait Until Page Contains Choose the jump target + ${references_count} = Get Element Count css:.jp-Dialog select option + Should Be True ${references_count} == ${3} + Select From List By Index css:.jp-Dialog select 2 + Click Element css:.jp-Dialog-button.jp-mod-accept + Wait Until Keyword Succeeds 10 x 1 s Cursor Should Jump ${original} + Clean Up After Working With File jump_references.py + +Jumps To References From Context Menu + [Setup] Prepare File for Editing Python editor jump_references.py + Wait Until Fully Initialized + ${token} = Select Token Occurrence func type=def + Click Element ${token} + ${original} = Measure Cursor Position + Open Context Menu Over ${token} + Select Menu Entry Jump to references + Wait Until Page Contains Choose the jump target + ${references_count} = Get Element Count css:.jp-Dialog select option + Should Be True ${references_count} == ${3} + Select From List By Index css:.jp-Dialog select 2 + Click Element css:.jp-Dialog-button.jp-mod-accept + Wait Until Keyword Succeeds 10 x 1 s Cursor Should Jump ${original} + Clean Up After Working With File jump_references.py + Ctrl Click And Jumping Back Works [Setup] Prepare File for Editing Python editor jump.py Configure JupyterLab Plugin {"modifierKey": "Accel"} plugin id=${JUMP PLUGIN ID} Wait Until Fully Initialized - ${usage} = Set Variable a_variable - ${sel} = Set Variable xpath:(//span[contains(@class, 'cm-variable')][contains(text(), '${usage}')])[last()] + ${sel} = Select Token Occurrence a_variable Click Element ${sel} ${original} = Measure Cursor Position Capture Page Screenshot 01-ready-to-jump.png - ${key} = Evaluate 'COMMAND' if platform.system() == 'Darwin' else 'CTRL' platform - Click Element ${sel} modifier=${key} + Ctrl Click Element ${sel} Capture Page Screenshot 02-jumped.png Wait Until Keyword Succeeds 10 x 1 s Cursor Should Jump ${original} ${new} = Measure Cursor Position @@ -47,3 +76,16 @@ Copy Files to Folder With Spaces FOR ${file} IN @{files} Copy File examples${/}${file} ${NOTEBOOK DIR}${/}${FOLDER WITH SPACE}${/}${file} END + +Select Token Occurrence + [Arguments] ${token} ${type}=variable ${which}=last + [Return] xpath:(//span[contains(@class, 'cm-${type}')][contains(text(), '${token}')])[${which}()] + +Ctrl Click Element + [Arguments] ${element} + ${key} = Evaluate 'COMMAND' if platform.system() == 'Darwin' else 'CTRL' platform + Click Element ${element} modifier=${key} + +Should Have Expected Count + [Arguments] ${expected_count} + ${count} = Count Diagnostics In Panel diff --git a/atest/Keywords.robot b/atest/Keywords.robot index ec941d6a2..a89466b99 100644 --- a/atest/Keywords.robot +++ b/atest/Keywords.robot @@ -434,3 +434,25 @@ Restart Kernel Lab Command Restart Kernel… Wait For Dialog Accept Default Dialog Option + +Expand Menu Entry + [Arguments] ${label} + ${entry} = Set Variable xpath://div[contains(@class, 'lm-Menu-itemLabel')][contains(text(), "${label}")] + Wait Until Page Contains Element ${entry} timeout=10s + ${menus before} = Get Element Count ${LAB MENU} + Mouse Over ${entry} + ${expected menus} = Evaluate ${menus before} + 1 + Wait Until Keyword Succeeds 10 x 1s Menus Count Equal ${expected menus} + +Menus Count Equal + [Arguments] ${count} + ${menus count} = Get Element Count ${LAB MENU} + Should Be Equal ${menus count} ${count} + +Select Menu Entry + [Arguments] ${label} + ${entry} Set Variable xpath://div[contains(@class, 'lm-Menu-itemLabel')][contains(text(), '${label}')] + Wait Until Page Contains Element ${entry} timeout=10s + Mouse Over ${entry} + Click Element ${entry} + Wait Until Page Does Not Contain Element ${entry} timeout=10s \ No newline at end of file diff --git a/atest/Variables.robot b/atest/Variables.robot index 7aec5bc14..eb2603666 100644 --- a/atest/Variables.robot +++ b/atest/Variables.robot @@ -40,6 +40,7 @@ ${MENU EDITOR} xpath://div[contains(@class, 'lm-Menu-itemLabel')][contains(., ${MENU JUMP} xpath://div[contains(@class, 'lm-Menu-itemLabel')][contains(text(), "Jump to definition")] ${MENU SETTINGS} xpath://div[contains(@class, 'lm-MenuBar-itemLabel')][contains(text(), "Settings")] ${MENU EDITOR THEME} xpath://div[contains(@class, 'lm-Menu-itemLabel')][contains(text(), "Text Editor Theme")] +${LAB MENU} css:.lm-Menu ${CM CURSOR} css:.CodeMirror-cursor ${CM CURSORS} css:.jp-MainAreaWidget:not(.lm-mod-hidden) .CodeMirror-cursors:not([style='visibility: hidden']) # settings diff --git a/atest/examples/jump_references.py b/atest/examples/jump_references.py new file mode 100644 index 000000000..a8f65418a --- /dev/null +++ b/atest/examples/jump_references.py @@ -0,0 +1,6 @@ +def func(): + pass + + +x = func() +y = func() \ No newline at end of file diff --git a/examples/screenshots/jump_to_definition.png b/examples/screenshots/jump_to_definition.png index 9f1bc769a..daf59f7ae 100644 Binary files a/examples/screenshots/jump_to_definition.png and b/examples/screenshots/jump_to_definition.png differ diff --git a/packages/jupyterlab-lsp/src/connection.ts b/packages/jupyterlab-lsp/src/connection.ts index 7e7609284..ef17d576e 100644 --- a/packages/jupyterlab-lsp/src/connection.ts +++ b/packages/jupyterlab-lsp/src/connection.ts @@ -129,10 +129,10 @@ export interface IClientResult { [Method.ClientRequest.DEFINITION]: AnyLocation; [Method.ClientRequest.DOCUMENT_HIGHLIGHT]: lsp.DocumentHighlight[]; [Method.ClientRequest.DOCUMENT_SYMBOL]: lsp.DocumentSymbol[]; - [Method.ClientRequest.HOVER]: lsp.Hover; + [Method.ClientRequest.HOVER]: lsp.Hover | null; [Method.ClientRequest.IMPLEMENTATION]: AnyLocation; [Method.ClientRequest.INITIALIZE]: lsp.InitializeResult; - [Method.ClientRequest.REFERENCES]: Location[]; + [Method.ClientRequest.REFERENCES]: lsp.Location[] | null; [Method.ClientRequest.RENAME]: lsp.WorkspaceEdit; [Method.ClientRequest.SIGNATURE_HELP]: lsp.SignatureHelp; [Method.ClientRequest.TYPE_DEFINITION]: AnyLocation; diff --git a/packages/jupyterlab-lsp/src/editor_integration/codemirror.ts b/packages/jupyterlab-lsp/src/editor_integration/codemirror.ts index 5b209bc1b..26123f55c 100644 --- a/packages/jupyterlab-lsp/src/editor_integration/codemirror.ts +++ b/packages/jupyterlab-lsp/src/editor_integration/codemirror.ts @@ -98,6 +98,7 @@ export abstract class CodeMirrorIntegration protected virtual_document: VirtualDocument; protected connection: LSPConnection; + /** @deprecated: use `setStatusMessage()` instead */ protected status_message: StatusMessage; protected adapter: WidgetAdapter; protected console: ILSPLogConsole; @@ -128,6 +129,17 @@ export abstract class CodeMirrorIntegration this.is_registered = false; } + /** + * Set the text message and (optionally) the timeout to remove it. + * @param message + * @param timeout - number of ms to until the message is cleaned; + * -1 if the message should stay up indefinitely; + * defaults to 3000ms (3 seconds) + */ + setStatusMessage(message: string, timeout?: number): void { + this.status_message.set(message, timeout); + } + register(): void { // register editor handlers for (let [event_name, handler] of this.editor_handlers) { diff --git a/packages/jupyterlab-lsp/src/features/highlights.ts b/packages/jupyterlab-lsp/src/features/highlights.ts index d9d553a1f..5e8363bfa 100644 --- a/packages/jupyterlab-lsp/src/features/highlights.ts +++ b/packages/jupyterlab-lsp/src/features/highlights.ts @@ -4,17 +4,15 @@ import { } from '@jupyterlab/application'; import { CodeEditor } from '@jupyterlab/codeeditor'; import { ISettingRegistry } from '@jupyterlab/settingregistry'; -import { ITranslator, TranslationBundle } from '@jupyterlab/translation'; import { LabIcon } from '@jupyterlab/ui-components'; import { Debouncer } from '@lumino/polling'; import type * as CodeMirror from 'codemirror'; import type * as lsProtocol from 'vscode-languageserver-protocol'; -import highlightTypeSvg from '../../style/icons/highlight-type.svg'; import highlightSvg from '../../style/icons/highlight.svg'; import { CodeHighlights as LSPHighlightsSettings } from '../_highlights'; import { CodeMirrorIntegration } from '../editor_integration/codemirror'; -import { FeatureSettings, IFeatureCommand } from '../feature'; +import { FeatureSettings } from '../feature'; import { DocumentHighlightKind } from '../lsp'; import { IRootPosition, IVirtualPosition } from '../positioning'; import { ILSPFeatureManager, PLUGIN_ID } from '../tokens'; @@ -25,32 +23,6 @@ export const highlightIcon = new LabIcon({ svgstr: highlightSvg }); -export const highlightTypeIcon = new LabIcon({ - name: 'lsp:highlight-type', - svgstr: highlightTypeSvg -}); - -const COMMANDS = (trans: TranslationBundle): IFeatureCommand[] => [ - { - id: 'highlight-references', - execute: ({ connection, virtual_position, document }) => - connection?.getReferences(virtual_position, document.document_info), - is_enabled: ({ connection }) => - connection ? connection.isReferencesSupported() : false, - label: trans.__('Highlight references'), - icon: highlightIcon - }, - { - id: 'highlight-type-definition', - execute: ({ connection, virtual_position, document }) => - connection?.getTypeDefinition(virtual_position, document.document_info), - is_enabled: ({ connection }) => - connection ? connection.isTypeDefinitionSupported() : false, - label: trans.__('Highlight type definition'), - icon: highlightTypeIcon - } -]; - export class HighlightsCM extends CodeMirrorIntegration { protected highlight_markers: CodeMirror.TextMarker[] = []; private debounced_get_highlight: Debouncer< @@ -240,24 +212,21 @@ const FEATURE_ID = PLUGIN_ID + ':highlights'; export const HIGHLIGHTS_PLUGIN: JupyterFrontEndPlugin = { id: FEATURE_ID, - requires: [ILSPFeatureManager, ISettingRegistry, ITranslator], + requires: [ILSPFeatureManager, ISettingRegistry], autoStart: true, activate: ( app: JupyterFrontEnd, featureManager: ILSPFeatureManager, - settingRegistry: ISettingRegistry, - translator: ITranslator + settingRegistry: ISettingRegistry ) => { const settings = new FeatureSettings(settingRegistry, FEATURE_ID); - const trans = translator.load('jupyterlab_lsp'); featureManager.register({ feature: { editorIntegrationFactory: new Map([['CodeMirrorEditor', HighlightsCM]]), id: FEATURE_ID, name: 'LSP Highlights', - settings: settings, - commands: COMMANDS(trans) + settings: settings } }); } diff --git a/packages/jupyterlab-lsp/src/features/hover.ts b/packages/jupyterlab-lsp/src/features/hover.ts index 769303fcb..a9acdbaea 100644 --- a/packages/jupyterlab-lsp/src/features/hover.ts +++ b/packages/jupyterlab-lsp/src/features/hover.ts @@ -23,7 +23,12 @@ import { IEditorIntegrationOptions, IFeatureLabIntegration } from '../feature'; -import { IRootPosition, IVirtualPosition, is_equal } from '../positioning'; +import { + IRootPosition, + IVirtualPosition, + ProtocolCoordinates, + is_equal +} from '../positioning'; import { ILSPFeatureManager, PLUGIN_ID } from '../tokens'; import { getModifierState } from '../utils'; import { VirtualDocument } from '../virtual/document'; @@ -122,10 +127,12 @@ export class HoverCM extends CodeMirrorIntegration { private virtual_position: IVirtualPosition; protected cache: ResponseCache; - private debounced_get_hover: Throttler>; + private debounced_get_hover: Throttler< + Promise + >; private tooltip: FreeTooltip; private _previousHoverRequest: Promise< - Promise + Promise > | null = null; constructor(options: IEditorIntegrationOptions) { @@ -154,13 +161,7 @@ export class HoverCM extends CodeMirrorIntegration { return false; } let range = cache_item.response.range!; - return ( - line >= range.start.line && - line <= range.end.line && - // need to be non-overlapping see https://github.com/jupyter-lsp/jupyterlab-lsp/issues/628 - (line != range.start.line || ch > range.start.character) && - (line != range.end.line || ch <= range.end.character) - ); + return ProtocolCoordinates.isWithinRange({ line, character: ch }, range); }); if (matching_items.length > 1) { this.console.warn( @@ -250,10 +251,13 @@ export class HoverCM extends CodeMirrorIntegration { } protected create_throttler() { - return new Throttler>(this.on_hover, { - limit: this.settings.composite.throttlerDelay, - edge: 'trailing' - }); + return new Throttler>( + this.on_hover, + { + limit: this.settings.composite.throttlerDelay, + edge: 'trailing' + } + ); } afterChange(change: IEditorChange, root_position: IRootPosition) { diff --git a/packages/jupyterlab-lsp/src/features/jump_to.ts b/packages/jupyterlab-lsp/src/features/jump_to.ts index 47d3799ba..544a377cf 100644 --- a/packages/jupyterlab-lsp/src/features/jump_to.ts +++ b/packages/jupyterlab-lsp/src/features/jump_to.ts @@ -2,6 +2,7 @@ import { JupyterFrontEnd, JupyterFrontEndPlugin } from '@jupyterlab/application'; +import { InputDialog } from '@jupyterlab/apputils'; import { CodeMirrorEditor } from '@jupyterlab/codemirror'; import { URLExt } from '@jupyterlab/coreutils'; import { IDocumentManager } from '@jupyterlab/docmanager'; @@ -16,6 +17,7 @@ import { NotebookJumper } from '@krassowski/code-jumpers'; import { AnyLocation } from 'lsp-ws-connection/lib/types'; +import type * as lsp from 'vscode-languageserver-protocol'; import jumpToSvg from '../../style/icons/jump-to.svg'; import { CodeJump as LSPJumpSettings, ModifierKey } from '../_jump_to'; @@ -27,9 +29,10 @@ import { IFeatureCommand, IFeatureLabIntegration } from '../feature'; -import { IVirtualPosition } from '../positioning'; +import { IVirtualPosition, ProtocolCoordinates } from '../positioning'; import { ILSPAdapterManager, ILSPFeatureManager, PLUGIN_ID } from '../tokens'; import { getModifierState, uri_to_contents_path, uris_equal } from '../utils'; +import { CodeMirrorVirtualEditor } from '../virtual/codemirror_editor'; export const jumpToIcon = new LabIcon({ name: 'lsp:jump-to', @@ -45,6 +48,15 @@ const FEATURE_ID = PLUGIN_ID + ':jump_to'; let trans: TranslationBundle; +const enum JumpResult { + NoTargetsFound = 1, + PositioningFailure = 2, + PathResolutionFailure = 3, + AssumeSuccess = 4, + UnspecifiedFailure = 5, + AlreadyAtTarget = 6 +} + export class CMJumpToDefinition extends CodeMirrorIntegration { get jumper() { return (this.feature.labIntegration as JumperLabIntegration).jumper; @@ -61,94 +73,177 @@ export class CMJumpToDefinition extends CodeMirrorIntegration { register() { this.editor_handlers.set( 'mousedown', - (virtual_editor, event: MouseEvent) => { - const { button } = event; - if (button === 0 && getModifierState(event, this.modifierKey)) { - let root_position = this.position_from_mouse(event); - if (root_position == null) { - this.console.warn( - 'Could not retrieve root position from mouse event to jump to definition' - ); - return; - } - let document = - virtual_editor.document_at_root_position(root_position); - let virtual_position = - virtual_editor.root_position_to_virtual_position(root_position); - - this.connection - .getDefinition(virtual_position, document.document_info, false) - .then(targets => { - this.handle_jump(targets, document.document_info.uri).catch( - this.console.warn - ); - }) - .catch(this.console.warn); - event.preventDefault(); - event.stopPropagation(); - } - } + this._jumpToDefinitionOrRefernce.bind(this) ); super.register(); } - get_uri_and_range(location_or_locations: AnyLocation) { - if (location_or_locations == null) { - return undefined; + private _jumpToDefinitionOrRefernce( + virtual_editor: CodeMirrorVirtualEditor, + event: MouseEvent + ) { + const { button } = event; + const shouldJump = + button === 0 && getModifierState(event, this.modifierKey); + if (!shouldJump) { + return; } - // some language servers appear to return a single object - const locations = Array.isArray(location_or_locations) - ? location_or_locations - : [location_or_locations]; - - // TODO: implement selector for multiple locations - // (like when there are multiple definitions or usages) - // could use the showHints() or completion frontend as a reference - if (locations.length === 0) { - return undefined; + let root_position = this.position_from_mouse(event); + if (root_position == null) { + this.console.warn( + 'Could not retrieve root position from mouse event to jump to definition/reference' + ); + return; } + let document = virtual_editor.document_at_root_position(root_position); + let virtual_position = + virtual_editor.root_position_to_virtual_position(root_position); + + const positionParams: lsp.TextDocumentPositionParams = { + textDocument: { + uri: document.document_info.uri + }, + position: { + line: virtual_position.line, + character: virtual_position.ch + } + }; + + this.connection.clientRequests['textDocument/definition'] + .request(positionParams) + .then(targets => { + this.handleJump(targets, positionParams) + .then((result: JumpResult | undefined) => { + if ( + result === JumpResult.NoTargetsFound || + result === JumpResult.AlreadyAtTarget + ) { + // definition was not found, or we are in definition already, suggest references + this.connection.clientRequests['textDocument/references'] + .request({ + ...positionParams, + context: { includeDeclaration: false } + }) + .then(targets => + // TODO: explain that we are now presenting references? + this.handleJump(targets, positionParams) + ) + .catch(this.console.warn); + } + }) + .catch(this.console.warn); + }) + .catch(this.console.warn); - this.console.log( - 'Will jump to the first of suggested locations:', - locations - ); + event.preventDefault(); + event.stopPropagation(); + } - const location_or_link = locations[0]; + private _harmonizeLocations(locationData: AnyLocation): lsp.Location[] { + if (locationData == null) { + return []; + } - if ('targetUri' in location_or_link) { - return { - uri: location_or_link.targetUri, - range: location_or_link.targetRange - }; - } else if ('uri' in location_or_link) { - return { - uri: location_or_link.uri, - range: location_or_link.range + const locationsList = Array.isArray(locationData) + ? locationData + : [locationData]; + + return (locationsList as (lsp.Location | lsp.LocationLink)[]) + .map((locationOrLink): lsp.Location | undefined => { + if ('targetUri' in locationOrLink) { + return { + uri: locationOrLink.targetUri, + range: locationOrLink.targetRange + }; + } else if ('uri' in locationOrLink) { + return { + uri: locationOrLink.uri, + range: locationOrLink.range + }; + } else { + this.console.warn( + 'Returned jump location is incorrect (no uri or targetUri):', + locationOrLink + ); + return undefined; + } + }) + .filter((location): location is lsp.Location => location != null); + } + + private async _chooseTarget(locations: lsp.Location[]) { + if (locations.length > 1) { + const choices = locations.map(location => { + // TODO: extract the line, the line above and below, and show it + const path = this._resolvePath(location.uri) || location.uri; + return path + ', line: ' + location.range.start.line; + }); + + // TODO: use selector with preview, basically needes the ui-component + // from jupyterlab-citation-manager; let's try to move it to JupyterLab core + // (and re-implement command palette with it) + // the preview should use this.jumper.document_manager.services.contents + + let getItemOptions = { + title: trans.__('Choose the jump target'), + okLabel: trans.__('Jump'), + items: choices }; - } else { - this.console.warn( - 'Returned jump location is incorrect (no uri or targetUri):', - location_or_link + // TODO: use showHints() or completion-like widget instead? + const choice = await InputDialog.getItem(getItemOptions).catch( + this.console.warn ); - return undefined; + if (!choice || choice.value == null) { + this.console.warn('No choice selected for jump location selection'); + return; + } + const choiceIndex = choices.indexOf(choice.value); + if (choiceIndex === -1) { + this.console.error( + 'Choice selection error: please report this as a bug:', + choices, + choice + ); + return; + } + return locations[choiceIndex]; + } else { + return locations[0]; } } - async handle_jump(location_or_locations: AnyLocation, document_uri: string) { - const target_info = this.get_uri_and_range(location_or_locations); + private _resolvePath(uri: string): string | null { + let contentsPath = uri_to_contents_path(uri); - if (!target_info) { - this.status_message.set(trans.__('No jump targets found'), 2 * 1000); - return; + if (contentsPath == null) { + if (uri.startsWith('file://')) { + contentsPath = decodeURIComponent(uri.slice(7)); + } else { + contentsPath = decodeURIComponent(uri); + } + } + return contentsPath; + } + + async handleJump( + locationData: AnyLocation, + positionParams: lsp.TextDocumentPositionParams + ) { + const locations = this._harmonizeLocations(locationData); + const targetInfo = await this._chooseTarget(locations); + + if (!targetInfo) { + this.setStatusMessage(trans.__('No jump targets found'), 2 * 1000); + return JumpResult.NoTargetsFound; } - let { uri, range } = target_info; + let { uri, range } = targetInfo; let virtual_position = PositionConverter.lsp_to_cm( range.start ) as IVirtualPosition; - if (uris_equal(uri, document_uri)) { + if (uris_equal(uri, positionParams.textDocument.uri)) { let editor_index = this.adapter.get_editor_index_at(virtual_position); // if in current file, transform from the position within virtual document to the editor position: let editor_position = @@ -158,21 +253,32 @@ export class CMJumpToDefinition extends CodeMirrorIntegration { 'Could not jump: conversion from virtual position to editor position failed', virtual_position ); - return; + return JumpResult.PositioningFailure; } let editor_position_ce = PositionConverter.cm_to_ce(editor_position); this.console.log(`Jumping to ${editor_index}th editor of ${uri}`); this.console.log('Jump target within editor:', editor_position_ce); - let contents_path = this.adapter.widget.context.path; + let contentsPath = this.adapter.widget.context.path; + + const didUserChooseThis = locations.length > 1; + + // note: we already know that URIs are equal, so just check the position range + if ( + !didUserChooseThis && + ProtocolCoordinates.isWithinRange(positionParams.position, range) + ) { + return JumpResult.AlreadyAtTarget; + } this.jumper.global_jump({ line: editor_position_ce.line, column: editor_position.ch, editor_index: editor_index, is_symlink: false, - contents_path: contents_path + contents_path: contentsPath }); + return JumpResult.AssumeSuccess; } else { // otherwise there is no virtual document and we expect the returned position to be source position: let source_position_ce = PositionConverter.cm_to_ce(virtual_position); @@ -190,39 +296,33 @@ export class CMJumpToDefinition extends CodeMirrorIntegration { // with different OSes but also with JupyterHub and other platforms. // can it be resolved vs our guessed server root? - let contents_path = uri_to_contents_path(uri); - - if (contents_path == null && uri.startsWith('file://')) { - contents_path = decodeURI(uri.slice(7)); - } + const contentsPath = this._resolvePath(uri); - if (contents_path === null) { + if (contentsPath === null) { this.console.warn('contents_path could not be resolved'); - return; + return JumpResult.PathResolutionFailure; } try { - await this.jumper.document_manager.services.contents.get( - contents_path, - { - content: false - } - ); + await this.jumper.document_manager.services.contents.get(contentsPath, { + content: false + }); this.jumper.global_jump({ - contents_path, + contents_path: contentsPath, ...jump_data, is_symlink: false }); - return; + return JumpResult.AssumeSuccess; } catch (err) { this.console.warn(err); } this.jumper.global_jump({ - contents_path: URLExt.join('.lsp_symlink', contents_path), + contents_path: URLExt.join('.lsp_symlink', contentsPath), ...jump_data, is_symlink: true }); + return JumpResult.AssumeSuccess; } } } @@ -271,19 +371,65 @@ const COMMANDS = (trans: TranslationBundle): IFeatureCommand[] => [ { id: 'jump-to-definition', execute: async ({ connection, virtual_position, document, features }) => { - const jump_feature = features.get(FEATURE_ID) as CMJumpToDefinition; - const targets = await connection?.getDefinition( - virtual_position, - document.document_info, - false - ); - await jump_feature.handle_jump(targets, document.document_info.uri); + const jumpFeature = features.get(FEATURE_ID) as CMJumpToDefinition; + if (!connection) { + jumpFeature.setStatusMessage( + trans.__('Connection not found for jump'), + 2 * 1000 + ); + return; + } + + const positionParams: lsp.TextDocumentPositionParams = { + textDocument: { + uri: document.document_info.uri + }, + position: { + line: virtual_position.line, + character: virtual_position.ch + } + }; + const targets = await connection.clientRequests[ + 'textDocument/definition' + ].request(positionParams); + await jumpFeature.handleJump(targets, positionParams); }, is_enabled: ({ connection }) => - connection ? connection.isDefinitionSupported() : false, + connection ? connection.provides('definitionProvider') : false, label: trans.__('Jump to definition'), icon: jumpToIcon }, + { + id: 'jump-to-reference', + execute: async ({ connection, virtual_position, document, features }) => { + const jumpFeature = features.get(FEATURE_ID) as CMJumpToDefinition; + if (!connection) { + jumpFeature.setStatusMessage( + trans.__('Connection not found for jump'), + 2 * 1000 + ); + return; + } + + const positionParams: lsp.TextDocumentPositionParams = { + textDocument: { + uri: document.document_info.uri + }, + position: { + line: virtual_position.line, + character: virtual_position.ch + } + }; + const targets = await connection.clientRequests[ + 'textDocument/references' + ].request({ ...positionParams, context: { includeDeclaration: false } }); + await jumpFeature.handleJump(targets, positionParams); + }, + is_enabled: ({ connection }) => + connection ? connection.provides('referencesProvider') : false, + label: trans.__('Jump to references'), + icon: jumpToIcon + }, { id: 'jump-back', execute: async ({ connection, virtual_position, document, features }) => { @@ -291,7 +437,10 @@ const COMMANDS = (trans: TranslationBundle): IFeatureCommand[] => [ jump_feature.jumper.global_jump_back(); }, is_enabled: ({ connection }) => - connection ? connection.isDefinitionSupported() : false, + connection + ? connection.provides('definitionProvider') || + connection.provides('referencesProvider') + : false, label: trans.__('Jump back'), icon: jumpBackIcon, // do not attach to any of the context menus diff --git a/packages/jupyterlab-lsp/src/positioning.ts b/packages/jupyterlab-lsp/src/positioning.ts index cdbc36964..c54d55679 100644 --- a/packages/jupyterlab-lsp/src/positioning.ts +++ b/packages/jupyterlab-lsp/src/positioning.ts @@ -1,5 +1,6 @@ import { CodeEditor } from '@jupyterlab/codeeditor'; import type * as CodeMirror from 'codemirror'; +import type * as lsp from 'vscode-languageserver-protocol'; /** * is_* attributes are there only to enforce strict interface type checking @@ -68,3 +69,19 @@ export function offset_at_position( export class PositionError extends Error { // no-op } + +export namespace ProtocolCoordinates { + export function isWithinRange( + position: lsp.Position, + range: lsp.Range + ): boolean { + const { line, character } = position; + return ( + line >= range.start.line && + line <= range.end.line && + // need to be non-overlapping see https://github.com/jupyter-lsp/jupyterlab-lsp/issues/628 + (line != range.start.line || character > range.start.character) && + (line != range.end.line || character <= range.end.character) + ); + } +} diff --git a/packages/jupyterlab-lsp/src/utils.ts b/packages/jupyterlab-lsp/src/utils.ts index 412f6e2c3..5ea3b85a9 100644 --- a/packages/jupyterlab-lsp/src/utils.ts +++ b/packages/jupyterlab-lsp/src/utils.ts @@ -155,7 +155,8 @@ export function uri_to_contents_path(child: string, parent?: string) { return null; } if (child.startsWith(parent)) { - return decodeURI(child.replace(parent, '')); + // 'decodeURIComponent' is needed over 'decodeURI' for '@' in TS/JS paths + return decodeURIComponent(child.replace(parent, '')); } return null; } diff --git a/packages/jupyterlab-lsp/style/icons/README.md b/packages/jupyterlab-lsp/style/icons/README.md index 89e3a607a..8c293d04d 100644 --- a/packages/jupyterlab-lsp/style/icons/README.md +++ b/packages/jupyterlab-lsp/style/icons/README.md @@ -9,7 +9,6 @@ respective license holders: - [rename.svg](./rename.svg) by Austin Andrews (Templarian) - [hover.svg](./hover.svg) by Austin Andrews (Templarian) - [completion.svg](./completion.svg): based on `format-list-bulleted-type` by Austin Andrews (Templarian) -- [highlight-type.svg](./highlight-type.svg) a derivative of `marker` by Google and `alpha-t` by GreenTurtwig Following icons are derivative works of the `code-tags-check icon` by Simran (XT3000) and: diff --git a/packages/jupyterlab-lsp/style/icons/highlight-type.svg b/packages/jupyterlab-lsp/style/icons/highlight-type.svg deleted file mode 100644 index b47de42dc..000000000 --- a/packages/jupyterlab-lsp/style/icons/highlight-type.svg +++ /dev/null @@ -1,5 +0,0 @@ - - - diff --git a/packages/lsp-ws-connection/src/ws-connection.ts b/packages/lsp-ws-connection/src/ws-connection.ts index 9876a14f1..19eac3eb4 100644 --- a/packages/lsp-ws-connection/src/ws-connection.ts +++ b/packages/lsp-ws-connection/src/ws-connection.ts @@ -247,6 +247,9 @@ export class LspWsConnection ); } + /** + * @deprecated + */ public async getHoverTooltip( location: IPosition, documentInfo: IDocumentInfo, @@ -278,6 +281,9 @@ export class LspWsConnection return hover; } + /** + * @deprecated + */ public async getCompletion( location: IPosition, token: ITokenInfo, @@ -378,6 +384,7 @@ export class LspWsConnection /** * Request the locations of all matching document symbols + * @deprecated */ public async getDocumentHighlights( location: IPosition, @@ -410,6 +417,7 @@ export class LspWsConnection /** * Request a link to the definition of the current symbol. The results will not be displayed * unless they are within the same file URI + * @deprecated */ public async getDefinition( location: IPosition, @@ -445,6 +453,7 @@ export class LspWsConnection /** * Request a link to the type definition of the current symbol. The results will not be displayed * unless they are within the same file URI + * @deprecated */ public async getTypeDefinition( location: IPosition, @@ -480,6 +489,7 @@ export class LspWsConnection /** * Request a link to the implementation of the current symbol. The results will not be displayed * unless they are within the same file URI + * @deprecated */ public getImplementation(location: IPosition, documentInfo: IDocumentInfo) { if (!this.isReady || !this.isImplementationSupported()) { @@ -507,6 +517,7 @@ export class LspWsConnection /** * Request a link to all references to the current symbol. The results will not be displayed * unless they are within the same file URI + * @deprecated */ public async getReferences( location: IPosition, @@ -544,6 +555,7 @@ export class LspWsConnection /** * The characters that trigger completion automatically. + * @deprecated */ public getLanguageCompletionCharacters(): string[] { return this.serverCapabilities?.completionProvider?.triggerCharacters || []; @@ -551,6 +563,7 @@ export class LspWsConnection /** * The characters that trigger signature help automatically. + * @deprecated */ public getLanguageSignatureCharacters(): string[] { return ( @@ -560,6 +573,7 @@ export class LspWsConnection /** * Does the server support go to definition? + * @deprecated */ public isDefinitionSupported() { return !!this.serverCapabilities?.definitionProvider; @@ -567,6 +581,7 @@ export class LspWsConnection /** * Does the server support go to type definition? + * @deprecated */ public isTypeDefinitionSupported() { return !!this.serverCapabilities?.typeDefinitionProvider; @@ -574,6 +589,7 @@ export class LspWsConnection /** * Does the server support go to implementation? + * @deprecated */ public isImplementationSupported() { return !!this.serverCapabilities?.implementationProvider; @@ -581,6 +597,7 @@ export class LspWsConnection /** * Does the server support find all references? + * @deprecated */ public isReferencesSupported() { return !!this.serverCapabilities?.referencesProvider;