Skip to content

Commit

Permalink
Backport PR jupyterlab#15262: Fix connection loop issue with standalo…
Browse files Browse the repository at this point in the history 
…ne foreign document in LSP
  • Loading branch information
@trungleduc @meeseeksmachine
trungleduc authored and meeseeksmachine committed Jan 24, 2024
1 parent 9d4a361 commit 08644e6
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 16 deletions.
46 changes: 36 additions & 10 deletions packages/lsp/src/virtual/document.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -461,13 +461,16 @@ export class VirtualDocument implements IDisposable {
* Clear the virtual document and all related stuffs
*/
clear(): void {
this.unusedStandaloneDocuments.clear();

for (let document of this.foreignDocuments.values()) {
document.clear();
if (document.standalone) {
let set = this.unusedStandaloneDocuments.get(document.language);
set.push(document);
}
}

// TODO - deep clear (assure that there is no memory leak)
this.unusedStandaloneDocuments.clear();

this.virtualLines.clear();
this.sourceLines.clear();
this.lastVirtualLine = 0;
Expand Down Expand Up @@ -739,7 +742,7 @@ export class VirtualDocument implements IDisposable {
);
continue;
}
let foreignDocument = this.chooseForeignDocument(extractor);
let foreignDocument = this._chooseForeignDocument(extractor);
foreignDocumentsMap.set(result.range, {
virtualLine: foreignDocument.lastVirtualLine,
virtualDocument: foreignDocument,
Expand Down Expand Up @@ -888,6 +891,19 @@ export class VirtualDocument implements IDisposable {
} as ISourcePosition;
}

/**
* Compute the position in root document from the position of
* a virtual document.
*/
transformVirtualToRoot(position: IVirtualPosition): IRootPosition | null {
const editor = this.virtualLines.get(position.line)?.editor;
const editorPosition = this.transformVirtualToEditor(position);
if (!editor || !editorPosition) {
return null;
}
return this.root.transformFromEditorToRoot(editor, editorPosition);
}

/**
* Get the corresponding editor of the virtual line.
*/
Expand Down Expand Up @@ -965,7 +981,7 @@ export class VirtualDocument implements IDisposable {
/**
* Get the foreign document that can be opened with the input extractor.
*/
private chooseForeignDocument(
private _chooseForeignDocument(
extractor: IForeignCodeExtractor
): VirtualDocument {
let foreignDocument: VirtualDocument;
Expand All @@ -976,11 +992,18 @@ export class VirtualDocument implements IDisposable {
} else {
// if (previous document does not exists) or (extractor produces standalone documents
// and no old standalone document could be reused): create a new document
foreignDocument = this.openForeign(
extractor.language,
extractor.standalone,
extractor.fileExtension
let unusedStandalone = this.unusedStandaloneDocuments.get(
extractor.language
);
if (extractor.standalone && unusedStandalone.length > 0) {
foreignDocument = unusedStandalone.pop()!;
} else {
foreignDocument = this.openForeign(
extractor.language,
extractor.standalone,
extractor.fileExtension
);
}
}
return foreignDocument;
}
Expand All @@ -997,13 +1020,16 @@ export class VirtualDocument implements IDisposable {
standalone: boolean,
fileExtension: string
): VirtualDocument {
let document = new VirtualDocument({
let document = new (this.constructor as new (
...args: ConstructorParameters<typeof VirtualDocument>
) => VirtualDocument)({
...this.options,
parent: this,
standalone: standalone,
fileExtension: fileExtension,
language: language
});

const context: Document.IForeignContext = {
foreignDocument: document,
parentHost: this
Expand Down
81 changes: 75 additions & 6 deletions packages/lsp/test/document.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ describe('@jupyterlab/lsp', () => {
let extractorManager: ILSPCodeExtractorsManager;
let markdownCellExtractor: TextForeignCodeExtractor;
let rawCellExtractor: TextForeignCodeExtractor;
let standaloneCellExtractor: TextForeignCodeExtractor;
beforeAll(() => {
extractorManager = new CodeExtractorsManager();

Expand All @@ -70,6 +71,13 @@ describe('@jupyterlab/lsp', () => {
cellType: ['raw']
});
extractorManager.register(rawCellExtractor, null);
standaloneCellExtractor = new TextForeignCodeExtractor({
language: 'standalone-text',
isStandalone: true,
file_extension: 'txt',
cellType: ['standalone-raw']
});
extractorManager.register(standaloneCellExtractor, null);
});
beforeEach(() => {
document = new VirtualDocument({
Expand Down Expand Up @@ -225,18 +233,41 @@ describe('@jupyterlab/lsp', () => {
expect(foreignDocumentsMap.size).toEqual(1);
});
});
describe('#chooseForeignDocument', () => {
describe('#_chooseForeignDocument', () => {
it('should select the foreign document for markdown cell', () => {
const md: VirtualDocument = document['chooseForeignDocument'](
const md: VirtualDocument = document['_chooseForeignDocument'](
markdownCellExtractor
);
expect(md.uri).toBe('test.ipynb.python-markdown.md');
});
it('should select the foreign document for raw cell', () => {
const md: VirtualDocument =
document['chooseForeignDocument'](rawCellExtractor);
document['_chooseForeignDocument'](rawCellExtractor);
expect(md.uri).toBe('test.ipynb.python-text.txt');
});
it('should use unused virtual document if available', () => {
document.appendCodeBlock({
value: 'test line in raw 1\ntest line in raw 2',
ceEditor: {} as Document.IEditor,
type: 'standalone-raw'
});
document.clear();
expect(
document['unusedStandaloneDocuments'].get(
standaloneCellExtractor.language
).length
).toEqual(1);

const openForeignSpy = jest.spyOn(document as any, 'openForeign');
document['_chooseForeignDocument'](standaloneCellExtractor);

expect(
document['unusedStandaloneDocuments'].get(
standaloneCellExtractor.language
).length
).toEqual(0);
expect(openForeignSpy).toHaveBeenCalledTimes(0);
});
});
describe('#closeExpiredDocuments', () => {
it('should close expired foreign documents', async () => {
Expand Down Expand Up @@ -282,14 +313,14 @@ describe('@jupyterlab/lsp', () => {
it('should emit the `foreignDocumentClosed` signal', () => {
const cb = jest.fn();
document.foreignDocumentClosed.connect(cb);
const md: VirtualDocument = document['chooseForeignDocument'](
const md: VirtualDocument = document['_chooseForeignDocument'](
markdownCellExtractor
);
document.closeForeign(md);
expect(cb).toHaveBeenCalled();
});
it('should close correctly foreign documents', () => {
const md: VirtualDocument = document['chooseForeignDocument'](
const md: VirtualDocument = document['_chooseForeignDocument'](
markdownCellExtractor
);
md.closeAllForeignDocuments = jest.fn();
Expand All @@ -312,7 +343,7 @@ describe('@jupyterlab/lsp', () => {
);
});
it('should get the markdown content of the document', () => {
const md = document['chooseForeignDocument'](markdownCellExtractor);
const md = document['_chooseForeignDocument'](markdownCellExtractor);

expect(md.value).toContain(
'test line in markdown 1\ntest line in markdown 2'
Expand All @@ -339,5 +370,43 @@ describe('@jupyterlab/lsp', () => {
expect(position).toEqual({ ch: 2, line: 0 });
});
});
describe('#transformVirtualToRoot', () => {
it('should return the position in root', async () => {
await document.updateManager.updateDocuments([
{
value: 'new line',
ceEditor: {} as Document.IEditor,
type: 'code'
}
]);
const position = document.transformVirtualToRoot({
isVirtual: true,
ch: 2,
line: 0
});
expect(position).toEqual({ ch: 2, line: 0 });
});
});
describe('#clear', () => {
it('should clear everything', () => {
document.clear();
expect(document['unusedStandaloneDocuments'].size).toEqual(0);
expect(document['virtualLines'].size).toEqual(0);
expect(document['virtualLines'].size).toEqual(0);
expect(document['sourceLines'].size).toEqual(0);
expect(document['lastVirtualLine']).toEqual(0);
expect(document['lastSourceLine']).toEqual(0);
expect(document['lineBlocks']).toEqual([]);
});
it('should keep unused document', () => {
document.appendCodeBlock({
value: 'test line in raw 1\ntest line in raw 2',
ceEditor: {} as Document.IEditor,
type: 'standalone-raw'
});
document.clear();
expect(document['unusedStandaloneDocuments'].size).toEqual(1);
});
});
});
});

0 comments on commit 08644e6

Please sign in to comment.