Skip to content

Commit

Permalink
Merge branch 'master' into jupyterlabgh-13746-update-json-schema
Browse files Browse the repository at this point in the history
  • Loading branch information
bollwyvl committed Feb 8, 2023
2 parents 9130cf9 + c599501 commit fd85fa3
Show file tree
Hide file tree
Showing 9 changed files with 53 additions and 71 deletions.
4 changes: 4 additions & 0 deletions .github/workflows/linuxjs-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ jobs:
name: JS
strategy:
matrix:
# Fix for https://github.com/jupyterlab/jupyterlab/issues/13903
include:
- group: js-debugger
python-version: '3.11'
group:
[
js-application,
Expand Down
2 changes: 1 addition & 1 deletion jupyterlab/handlers/error_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
# Copyright (c) Jupyter Development Team.
# Distributed under the terms of the Modified BSD License.

from jupyter_server.base.handlers import JupyterHandler
from jupyter_server.extension.handler import ExtensionHandlerMixin
from jupyterlab_server.server import JupyterHandler
from tornado import web

TEMPLATE = """
Expand Down
2 changes: 1 addition & 1 deletion packages/fileeditor/src/fileeditorlspadapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ export class FileEditorAdapter extends WidgetLSPAdapter<

// connect the document, but do not open it as the adapter will handle this
// after registering all features
await this.connectDocument(this.virtualDocument, false);
await this.connectDocument(this.virtualDocument!, false);

this.editor.model.mimeTypeChanged.connect(this.reloadConnection, this);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/lsp-extension/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,8 @@ export class RunningLanguageServer implements IRunningSessions.IRunningItem {
shutdown(): void {
for (const [key, value] of this._manager.connections.entries()) {
if (this._connection.has(value)) {
const document = this._manager.documents.get(key)!;
this._manager.unregisterDocument(document);
const { uri } = this._manager.documents.get(key)!;
this._manager.unregisterDocument(uri);
}
}
this._manager.disconnect(this._serverIdentifier as TLanguageServerId);
Expand Down
78 changes: 30 additions & 48 deletions packages/lsp/src/adapters/adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ export abstract class WidgetLSPAdapter<T extends IDocumentWidget>
/**
* Internal virtual document of the adapter.
*/
get virtualDocument(): VirtualDocument {
get virtualDocument(): VirtualDocument | null {
return this._virtualDocument;
}

Expand All @@ -261,14 +261,8 @@ export abstract class WidgetLSPAdapter<T extends IDocumentWidget>
return;
}
this._isDisposed = true;

this.virtualDocument?.dispose();

this.widget.context.saveState.disconnect(this.onSaveState, this);
this.connectionManager.closed.disconnect(this.onConnectionClosed, this);
this.widget.disposed.disconnect(this.dispose, this);
this.virtualDocument.changed.disconnect(this.documentChanged);
this.disconnect();
this._virtualDocument = null;
this._disposed.emit();
Signal.clearData(this);
}
Expand All @@ -277,8 +271,12 @@ export abstract class WidgetLSPAdapter<T extends IDocumentWidget>
* Disconnect virtual document from the language server.
*/
disconnect(): void {
this.connectionManager.unregisterDocument(this.virtualDocument);
this.widget.context.model.contentChanged.disconnect(this._onContentChanged);
const uri = this.virtualDocument?.uri;
const { model } = this.widget.context;
if (uri) {
this.connectionManager.unregisterDocument(uri);
}
model.contentChanged.disconnect(this._onContentChanged, this);

// pretend that all editors were removed to trigger the disconnection of even handlers
// they will be connected again on new connection
Expand All @@ -288,7 +286,7 @@ export abstract class WidgetLSPAdapter<T extends IDocumentWidget>
});
}

this.virtualDocument.dispose();
this.virtualDocument?.dispose();
}

/**
Expand All @@ -299,7 +297,7 @@ export abstract class WidgetLSPAdapter<T extends IDocumentWidget>
console.warn('Cannot update documents: adapter disposed');
return Promise.reject('Cannot update documents: adapter disposed');
}
return this.virtualDocument.updateManager.updateDocuments(this.editors);
return this.virtualDocument!.updateManager.updateDocuments(this.editors);
}

/**
Expand Down Expand Up @@ -364,7 +362,7 @@ export abstract class WidgetLSPAdapter<T extends IDocumentWidget>
// but also reloads the connection; used during file rename (or when it was moved)
protected reloadConnection(): void {
// ignore premature calls (before the editor was initialized)
if (this.virtualDocument == null) {
if (this.virtualDocument === null) {
return;
}

Expand All @@ -389,7 +387,7 @@ export abstract class WidgetLSPAdapter<T extends IDocumentWidget>
state: DocumentRegistry.SaveState
): void {
// ignore premature calls (before the editor was initialized)
if (this.virtualDocument == null) {
if (this.virtualDocument === null) {
return;
}

Expand Down Expand Up @@ -522,19 +520,10 @@ export abstract class WidgetLSPAdapter<T extends IDocumentWidget>
* Create the virtual document using current path and language.
*/
protected initVirtual(): void {
let virtualDocument = this.createVirtualDocument();
if (virtualDocument == null) {
console.error(
'Could not initialize a VirtualDocument for adapter: ',
this
);
return;
}
if (this._virtualDocument) {
this._virtualDocument.dispose();
}
this._virtualDocument = virtualDocument;
this._connectContentChangedSignal();
const { model } = this.widget.context;
this._virtualDocument?.dispose();
this._virtualDocument = this.createVirtualDocument();
model.contentChanged.connect(this._onContentChanged, this);
}

/**
Expand Down Expand Up @@ -599,7 +588,7 @@ export abstract class WidgetLSPAdapter<T extends IDocumentWidget>

private _isConnected: boolean;
private _updateFinished: Promise<void>;
private _virtualDocument: VirtualDocument;
private _virtualDocument: VirtualDocument | null = null;

/**
* Callback called when a foreign document is closed,
Expand Down Expand Up @@ -671,28 +660,21 @@ export abstract class WidgetLSPAdapter<T extends IDocumentWidget>
}

/**
* Connect the change signal in order to update all virtual documents after a change.
* Handle content changes and update all virtual documents after a change.
*
* Update to the state of a notebook may be done without a notice on the CodeMirror level,
* e.g. when a cell is deleted. Therefore a JupyterLab-specific signals are watched instead.
* #### Notes
* Update to the state of a notebook may be done without a notice on the
* CodeMirror level, e.g. when a cell is deleted. Therefore a
* JupyterLab-specific signal is watched instead.
*
* While by not using the change event of CodeMirror editors we loose an easy way to send selective,
* (range) updates this can be still implemented by comparison of before/after states of the
* virtual documents, which is even more resilient and -obviously - editor-independent.
*/
private _connectContentChangedSignal(): void {
this.widget.context.model.contentChanged.connect(
this._onContentChanged,
this
);
}

/**
* Callback called when the content of the document have changed.
*/
private async _onContentChanged(_slot: any) {
// update the virtual documents (sending the updates to LSP is out of scope here)

* While by not using the change event of CodeMirror editors we lose an easy
* way to send selective (range) updates this can be still implemented by
* comparison of before/after states of the virtual documents, which is
* more resilient and editor-independent.
*/
private async _onContentChanged(_: unknown) {
// Update the virtual documents.
// Sending the updates to LSP is out of scope here.
const promise = this.updateDocuments();
if (!promise) {
console.warn('Could not update documents');
Expand Down
19 changes: 9 additions & 10 deletions packages/lsp/src/connection_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ export class DocumentConnectionManager
context: Document.IForeignContext
): void {
const { foreignDocument } = context;
this.unregisterDocument(foreignDocument, false);
this.unregisterDocument(foreignDocument.uri, false);
this.disconnectDocumentSignals(foreignDocument);
}

Expand All @@ -210,7 +210,9 @@ export class DocumentConnectionManager
): void {
this.adapters.set(path, adapter);
adapter.disposed.connect(() => {
this.documents.delete(adapter.virtualDocument.uri);
if (adapter.virtualDocument) {
this.documents.delete(adapter.virtualDocument.uri);
}
this.adapters.delete(path);
});
}
Expand Down Expand Up @@ -402,15 +404,12 @@ export class DocumentConnectionManager
}

/**
* Disconnect the signals of requested virtual document.
* Disconnect the signals of requested virtual document uri.
*/
unregisterDocument(
virtualDocument: VirtualDocument,
emit: boolean = true
): void {
const connection = this.connections.get(virtualDocument.uri);
unregisterDocument(uri: string, emit: boolean = true): void {
const connection = this.connections.get(uri);
if (connection) {
this.connections.delete(virtualDocument.uri);
this.connections.delete(uri);
const allConnection = new Set(this.connections.values());

if (!allConnection.has(connection)) {
Expand Down Expand Up @@ -653,7 +652,7 @@ namespace Private {
capabilities: ClientCapabilities
): Promise<LSPConnection> {
let connection = _connections.get(languageServerId);
if (connection == null) {
if (!connection) {
const socket = new WebSocket(uris.socket);
const connection = new LSPConnection({
languageId: language,
Expand Down
4 changes: 2 additions & 2 deletions packages/lsp/src/tokens.ts
Original file line number Diff line number Diff line change
Expand Up @@ -450,9 +450,9 @@ export interface ILSPDocumentConnectionManager {
disconnect(languageId: TLanguageServerId): void;

/**
* Disconnect the signals of requested virtual document.
* Disconnect the signals of requested virtual document uri.
*/
unregisterDocument(virtualDocument: VirtualDocument): void;
unregisterDocument(uri: string): void;

/**
* Register a widget adapter.
Expand Down
7 changes: 2 additions & 5 deletions packages/lsp/src/virtual/document.ts
Original file line number Diff line number Diff line change
Expand Up @@ -425,10 +425,7 @@ export class VirtualDocument implements IDisposable {
* Get the root document of current virtual document.
*/
get root(): VirtualDocument {
if (this.parent == null) {
return this;
}
return this.parent.root;
return this.parent ? this.parent.root : this;
}

/**
Expand Down Expand Up @@ -489,7 +486,7 @@ export class VirtualDocument implements IDisposable {
documentAtSourcePosition(position: ISourcePosition): VirtualDocument {
let sourceLine = this.sourceLines.get(position.line);

if (sourceLine == null) {
if (!sourceLine) {
return this;
}

Expand Down
4 changes: 2 additions & 2 deletions packages/notebook/src/notebooklspadapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ export class NotebookAdapter extends WidgetLSPAdapter<NotebookPanel> {

// connect the document, but do not open it as the adapter will handle this
// after registering all features
this.connectDocument(this.virtualDocument, false).catch(console.warn);
this.connectDocument(this.virtualDocument!, false).catch(console.warn);

this.widget.context.sessionContext.kernelChanged.connect(
this.onKernelChanged,
Expand Down Expand Up @@ -427,7 +427,7 @@ export class NotebookAdapter extends WidgetLSPAdapter<NotebookPanel> {
* @param pos - Position in the virtual document.
*/
private _getCellAt(pos: IVirtualPosition): Cell {
let editor = this.virtualDocument.getEditorAtVirtualLine(pos);
let editor = this.virtualDocument!.getEditorAtVirtualLine(pos);
return this._editorToCell.get(editor)!;
}

Expand Down

0 comments on commit fd85fa3

Please sign in to comment.