Skip to content

Commit 6a15ba1

Browse files
committed
Fix highlights, jump to and hover in foreign documents
Prior to this commit the positions would be always resolved with respect to the root document because virtual document was being taken from adapter using `virtualPositionToRootPosition` func which is error prone as the assumption of virtual document == root document is not obvious; this function was removed. To make the fixes work, a workaround for foreign document created in inherited classes always having the usptream class was added; the issue was reported in jupyterlab/jupyterlab#15126
1 parent 640453b commit 6a15ba1

File tree

5 files changed

+45
-39
lines changed

5 files changed

+45
-39
lines changed

packages/jupyterlab-lsp/src/converter.ts

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import type {
1010
} from '@jupyterlab/lsp';
1111
import type * as lsProtocol from 'vscode-languageserver-protocol';
1212

13-
import type { VirtualDocument } from './virtual/document';
13+
import { VirtualDocument } from './virtual/document';
1414

1515
export class PositionConverter {
1616
static lsp_to_cm(position: lsProtocol.Position): IPosition {
@@ -86,18 +86,6 @@ export function rootPositionToVirtualPosition(
8686
return adapter.virtualDocument!.virtualPositionAtDocument(rootAsSource);
8787
}
8888

89-
export function virtualPositionToRootPosition(
90-
adapter: WidgetLSPAdapter<any>,
91-
position: IVirtualPosition
92-
): IRootPosition | null {
93-
if (!adapter.virtualDocument) {
94-
throw Error('Virtual document of adapter disposed!');
95-
}
96-
return (adapter.virtualDocument as VirtualDocument).transformVirtualToRoot(
97-
position
98-
);
99-
}
100-
10189
export function rootPositionToEditorPosition(
10290
adapter: WidgetLSPAdapter<any>,
10391
position: IRootPosition
@@ -126,12 +114,20 @@ export function editorPositionToRootPosition(
126114
export function rangeToEditorRange(
127115
adapter: WidgetLSPAdapter<any>,
128116
range: lsProtocol.Range,
129-
editor: CodeEditor.IEditor | null
117+
editor: CodeEditor.IEditor | null,
118+
virtualDocument: VirtualDocument
130119
): IEditorRange {
131120
let start = PositionConverter.lsp_to_cm(range.start) as IVirtualPosition;
132121
let end = PositionConverter.lsp_to_cm(range.end) as IVirtualPosition;
133122

134-
let startInRoot = virtualPositionToRootPosition(adapter, start);
123+
// because `openForeign()` does not use new this.constructor, we need to workaround it for now:
124+
// const startInRoot = virtualDocument.transformVirtualToRoot(start);
125+
// https://github.com/jupyterlab/jupyterlab/issues/15126
126+
const startInRoot = VirtualDocument.prototype.transformVirtualToRoot.call(
127+
virtualDocument,
128+
start
129+
);
130+
135131
if (!startInRoot) {
136132
throw Error('Could not determine position in root');
137133
}

packages/jupyterlab-lsp/src/features/highlights.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,7 @@ import {
1414
ILSPFeatureManager,
1515
IEditorPosition,
1616
ILSPDocumentConnectionManager,
17-
WidgetLSPAdapter,
18-
VirtualDocument
17+
WidgetLSPAdapter
1918
} from '@jupyterlab/lsp';
2019
import { ISettingRegistry } from '@jupyterlab/settingregistry';
2120
import { LabIcon } from '@jupyterlab/ui-components';
@@ -37,6 +36,7 @@ import { DocumentHighlightKind } from '../lsp';
3736
import { createMarkManager, ISimpleMarkManager } from '../marks';
3837
import { PLUGIN_ID } from '../tokens';
3938
import { BrowserConsole } from '../virtual/console';
39+
import { VirtualDocument } from '../virtual/document';
4040

4141
export const highlightIcon = new LabIcon({
4242
name: 'lsp:highlight',
@@ -160,7 +160,8 @@ export class HighlightsFeature extends Feature {
160160

161161
protected handleHighlight(
162162
items: lsProtocol.DocumentHighlight[] | null,
163-
adapter: WidgetLSPAdapter<any>
163+
adapter: WidgetLSPAdapter<any>,
164+
document: VirtualDocument
164165
) {
165166
this.markManager.clearAllMarks();
166167

@@ -174,7 +175,7 @@ export class HighlightsFeature extends Feature {
174175
>();
175176

176177
for (let item of items) {
177-
let range = rangeToEditorRange(adapter, item.range, null);
178+
let range = rangeToEditorRange(adapter, item.range, null, document);
178179
const editor = range.editor;
179180

180181
let optionsList = highlightsByEditor.get(editor);
@@ -350,7 +351,7 @@ export class HighlightsFeature extends Feature {
350351
return;
351352
}
352353

353-
this.handleHighlight(highlights, adapter);
354+
this.handleHighlight(highlights, adapter, document);
354355
this._lastToken = token;
355356
} catch (e) {
356357
this.console.warn('Could not get highlights:', e);

packages/jupyterlab-lsp/src/features/hover.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import {
1818
isEqual,
1919
ILSPDocumentConnectionManager,
2020
WidgetLSPAdapter,
21-
VirtualDocument,
2221
Document
2322
} from '@jupyterlab/lsp';
2423
import { IRenderMimeRegistry } from '@jupyterlab/rendermime';
@@ -45,6 +44,7 @@ import { createMarkManager, ISimpleMarkManager } from '../marks';
4544
import { PLUGIN_ID } from '../tokens';
4645
import { getModifierState } from '../utils';
4746
import { BrowserConsole } from '../virtual/console';
47+
import { VirtualDocument } from '../virtual/document';
4848

4949
export const hoverIcon = new LabIcon({
5050
name: 'lsp:hover',
@@ -365,7 +365,8 @@ export class HoverFeature extends Feature {
365365
context.adapter,
366366
response,
367367
context.token,
368-
context.editor
368+
context.editor,
369+
virtualDocument
369370
);
370371
return this._addRange(
371372
context.adapter,
@@ -619,7 +620,8 @@ export class HoverFeature extends Feature {
619620
adapter,
620621
response,
621622
token,
622-
editor
623+
editor,
624+
document
623625
);
624626
responseData = {
625627
response: response,
@@ -689,10 +691,11 @@ export class HoverFeature extends Feature {
689691
adapter: WidgetLSPAdapter<any>,
690692
response: lsProtocol.Hover,
691693
token: CodeEditor.IToken,
692-
editor: CodeEditor.IEditor
694+
editor: CodeEditor.IEditor,
695+
document: VirtualDocument
693696
): IEditorRange {
694697
if (typeof response.range !== 'undefined') {
695-
return rangeToEditorRange(adapter, response.range, editor);
698+
return rangeToEditorRange(adapter, response.range, editor, document);
696699
}
697700

698701
const startInEditor = editor.getPositionAt(token.offset);

packages/jupyterlab-lsp/src/features/jump_to.ts

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,13 @@ import {
4848
documentAtRootPosition,
4949
editorAtRootPosition,
5050
rootPositionToVirtualPosition,
51-
rootPositionToEditorPosition,
52-
virtualPositionToRootPosition
51+
rootPositionToEditorPosition
5352
} from '../converter';
5453
import { FeatureSettings, Feature } from '../feature';
5554
import { PLUGIN_ID } from '../tokens';
5655
import { getModifierState, uriToContentsPath, urisEqual } from '../utils';
5756
import { BrowserConsole } from '../virtual/console';
57+
import { VirtualDocument } from '../virtual/document';
5858

5959
export const jumpToIcon = new LabIcon({
6060
name: 'lsp:jump-to',
@@ -245,7 +245,7 @@ export class NavigationFeature extends Feature {
245245
connection.clientRequests['textDocument/definition']
246246
.request(positionParams)
247247
.then(targets => {
248-
this.handleJump(targets, positionParams, adapter)
248+
this.handleJump(targets, positionParams, adapter, document)
249249
.then((result: JumpResult | undefined) => {
250250
if (
251251
result === JumpResult.NoTargetsFound ||
@@ -259,7 +259,7 @@ export class NavigationFeature extends Feature {
259259
})
260260
.then(targets =>
261261
// TODO: explain that we are now presenting references?
262-
this.handleJump(targets, positionParams, adapter)
262+
this.handleJump(targets, positionParams, adapter, document)
263263
)
264264
.catch(this.console.warn);
265265
}
@@ -361,7 +361,8 @@ export class NavigationFeature extends Feature {
361361
async handleJump(
362362
locationData: AnyLocation,
363363
positionParams: lsp.TextDocumentPositionParams,
364-
adapter: WidgetLSPAdapter<any>
364+
adapter: WidgetLSPAdapter<any>,
365+
document: VirtualDocument
365366
) {
366367
const locations = this._harmonizeLocations(locationData);
367368
const targetInfo = await this._chooseTarget(locations);
@@ -382,10 +383,16 @@ export class NavigationFeature extends Feature {
382383

383384
if (urisEqual(uri, positionParams.textDocument.uri)) {
384385
// if in current file, transform from the position within virtual document to the editor position:
385-
const rootPosition = virtualPositionToRootPosition(
386-
adapter,
387-
virtualPosition
388-
);
386+
387+
// because `openForeign()` does not use new this.constructor, we need to workaround it for now:
388+
// const rootPosition = document.transformVirtualToRoot(virtualPosition);
389+
// https://github.com/jupyterlab/jupyterlab/issues/15126
390+
const rootPosition =
391+
VirtualDocument.prototype.transformVirtualToRoot.call(
392+
document,
393+
virtualPosition
394+
);
395+
389396
if (rootPosition === null) {
390397
this.console.warn(
391398
'Could not jump: conversion from virtual position to editor position failed',
@@ -581,7 +588,7 @@ export const JUMP_PLUGIN: JupyterFrontEndPlugin<void> = {
581588
const targets = await connection.clientRequests[
582589
'textDocument/definition'
583590
].request(positionParams);
584-
await feature.handleJump(targets, positionParams, adapter);
591+
await feature.handleJump(targets, positionParams, adapter, document);
585592
},
586593
label: trans.__('Jump to definition'),
587594
icon: jumpToIcon,
@@ -627,7 +634,7 @@ export const JUMP_PLUGIN: JupyterFrontEndPlugin<void> = {
627634
...positionParams,
628635
context: { includeDeclaration: false }
629636
});
630-
await feature.handleJump(targets, positionParams, adapter);
637+
await feature.handleJump(targets, positionParams, adapter, document);
631638
},
632639
label: trans.__('Jump to references'),
633640
icon: jumpToIcon,

packages/jupyterlab-lsp/src/virtual/document.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -184,11 +184,10 @@ export class VirtualDocument extends VirtualDocumentBase {
184184
* @experimental
185185
*/
186186
transformVirtualToRoot(position: IVirtualPosition): IRootPosition | null {
187-
// a method which was previously implemented in CodeMirrorIntegration
188-
// but probably should have been in VirtualDocument all along
189187
let editor = this.virtualLines.get(position.line)!.editor;
190188
let editorPosition = this.transformVirtualToEditor(position);
191-
return this.transformFromEditorToRoot(editor, editorPosition!);
189+
// only root holds the full editor mapping
190+
return this.root.transformFromEditorToRoot(editor, editorPosition!);
192191
}
193192

194193
/**

0 commit comments

Comments
 (0)