Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interactive window middleware for Pylance LSP notebooks #19501

Merged
merged 26 commits into from
Jul 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
0f2bbb5
WIP
debonte Jul 7, 2022
7ad4c28
IntelliSense working, hover isn't
debonte Jul 9, 2022
ff68335
Remove unneeded code
debonte Jul 9, 2022
2e45fdb
Send change after open; fix notebookDocument field format
debonte Jul 13, 2022
8ca94ec
Handle input box change/close; include input box in notebook open
debonte Jul 15, 2022
1ae783a
Need to switch to proto objects somehow
debonte Jul 15, 2022
6a15c36
Use proto objects to fix serialization of Range so hover works
debonte Jul 15, 2022
4c49cf4
Cleanup
debonte Jul 16, 2022
c8db11a
Extract helper method for calculating notebook URI
debonte Jul 16, 2022
3347819
Expose API, simplify middleware creation timing
debonte Jul 18, 2022
79151ab
Move middleware .ts file to Pylance/node subdir
debonte Jul 18, 2022
f57161d
Undo package.json changes
debonte Jul 19, 2022
d90d8ad
Cleanup
debonte Jul 19, 2022
a3c814f
Cleanup onExtensionChange logic
debonte Jul 19, 2022
69e02b4
Remove unnecessary notebookDocument/didChange override; simpify maps
debonte Jul 19, 2022
518b813
Add comments on API
debonte Jul 19, 2022
4119f03
Remove remnant of old notebookMetadata type
debonte Jul 19, 2022
60d57cd
Remove knowledge of interactive window scheme
debonte Jul 19, 2022
a8a556e
Basic unit tests passing for first time
debonte Jul 20, 2022
7cdae3c
Fix generated didChange
debonte Jul 21, 2022
25d2f36
Unit tests passing
debonte Jul 21, 2022
95f4137
Simplify TextDocument creation with mock
debonte Jul 21, 2022
1b6707f
Simplify NotebookDocument creation with mock
debonte Jul 21, 2022
b069e0a
Enable feature based on version checks
debonte Jul 21, 2022
74f8e0f
Fix pylance min version; next week's prerel or later
debonte Jul 22, 2022
db7a992
Only use new middleware if Pylance and Jupyter support it
debonte Jul 22, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions src/client/activation/languageClientMiddlewareBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,29 @@ export class LanguageClientMiddlewareBase implements Middleware {
return this.callNext('willSaveWaitUntil', arguments);
}

public async didOpenNotebook() {
return this.callNotebooksNext('didOpen', arguments);
}

public async didSaveNotebook() {
return this.callNotebooksNext('didSave', arguments);
}

public async didChangeNotebook() {
return this.callNotebooksNext('didChange', arguments);
}

public async didCloseNotebook() {
return this.callNotebooksNext('didClose', arguments);
}

notebooks = {
didOpen: this.didOpenNotebook.bind(this),
didSave: this.didSaveNotebook.bind(this),
didChange: this.didChangeNotebook.bind(this),
didClose: this.didCloseNotebook.bind(this),
};

public async provideCompletionItem() {
if (await this.connected) {
return this.callNextAndSendTelemetry(
Expand Down Expand Up @@ -463,6 +486,17 @@ export class LanguageClientMiddlewareBase implements Middleware {
return args[args.length - 1](...args);
}

private callNotebooksNext(funcName: 'didOpen' | 'didSave' | 'didChange' | 'didClose', args: IArguments) {
// This function uses the last argument to call the 'next' item. If we're allowing notebook
// middleware, it calls into the notebook middleware first.
if (this.notebookAddon?.notebooks && (this.notebookAddon.notebooks as any)[funcName]) {
// It would be nice to use args.callee, but not supported in strict mode
return (this.notebookAddon.notebooks as any)[funcName](...args);
}

return args[args.length - 1](...args);
}

private callNextAndSendTelemetry<T extends keyof MiddleWareMethods>(
lspMethod: string,
debounceMilliseconds: number,
Expand Down
1 change: 1 addition & 0 deletions src/client/activation/node/analysisOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt
experimentationSupport: true,
trustedWorkspaceSupport: true,
lspNotebooksSupport: this.lspNotebooksExperiment.isInNotebooksExperiment(),
lspInteractiveWindowSupport: this.lspNotebooksExperiment.isInNotebooksExperimentWithInteractiveWindowSupport(),
} as unknown) as LanguageClientOptions;
}
}
36 changes: 30 additions & 6 deletions src/client/activation/node/languageClientMiddleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@
// Licensed under the MIT License.

import { Uri } from 'vscode';
import { LanguageClient } from 'vscode-languageclient/node';
import { IJupyterExtensionDependencyManager } from '../../common/application/types';
import { IServiceContainer } from '../../ioc/types';
import { JupyterExtensionIntegration } from '../../jupyter/jupyterIntegration';
import { traceLog } from '../../logging';
import { LanguageClientMiddleware } from '../languageClientMiddleware';
import { LspInteractiveWindowMiddlewareAddon } from './lspInteractiveWindowMiddlewareAddon';

import { LanguageServerType } from '../types';

Expand All @@ -15,11 +17,27 @@ import { LspNotebooksExperiment } from './lspNotebooksExperiment';
export class NodeLanguageClientMiddleware extends LanguageClientMiddleware {
private readonly lspNotebooksExperiment: LspNotebooksExperiment;

public constructor(serviceContainer: IServiceContainer, serverVersion?: string) {
private readonly jupyterExtensionIntegration: JupyterExtensionIntegration;

public constructor(
serviceContainer: IServiceContainer,
private getClient: () => LanguageClient | undefined,
serverVersion?: string,
) {
super(serviceContainer, LanguageServerType.Node, serverVersion);

this.lspNotebooksExperiment = serviceContainer.get<LspNotebooksExperiment>(LspNotebooksExperiment);
this.setupHidingMiddleware(serviceContainer);

this.jupyterExtensionIntegration = serviceContainer.get<JupyterExtensionIntegration>(
JupyterExtensionIntegration,
);
if (!this.notebookAddon && this.lspNotebooksExperiment.isInNotebooksExperimentWithInteractiveWindowSupport()) {
this.notebookAddon = new LspInteractiveWindowMiddlewareAddon(
this.getClient,
this.jupyterExtensionIntegration,
);
}
}

protected shouldCreateHidingMiddleware(jupyterDependencyManager: IJupyterExtensionDependencyManager): boolean {
Expand All @@ -34,18 +52,24 @@ export class NodeLanguageClientMiddleware extends LanguageClientMiddleware {
await this.lspNotebooksExperiment.onJupyterInstalled();
}

super.onExtensionChange(jupyterDependencyManager);
if (this.lspNotebooksExperiment.isInNotebooksExperimentWithInteractiveWindowSupport()) {
if (!this.notebookAddon) {
this.notebookAddon = new LspInteractiveWindowMiddlewareAddon(
this.getClient,
this.jupyterExtensionIntegration,
);
}
} else {
super.onExtensionChange(jupyterDependencyManager);
}
}

protected async getPythonPathOverride(uri: Uri | undefined): Promise<string | undefined> {
if (!uri || !this.lspNotebooksExperiment.isInNotebooksExperiment()) {
return undefined;
}

const jupyterExtensionIntegration = this.serviceContainer?.get<JupyterExtensionIntegration>(
JupyterExtensionIntegration,
);
const jupyterPythonPathFunction = jupyterExtensionIntegration?.getJupyterPythonPathFunction();
const jupyterPythonPathFunction = this.jupyterExtensionIntegration.getJupyterPythonPathFunction();
if (!jupyterPythonPathFunction) {
return undefined;
}
Expand Down
187 changes: 187 additions & 0 deletions src/client/activation/node/lspInteractiveWindowMiddlewareAddon.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
import { Disposable, NotebookCell, NotebookDocument, TextDocument, TextDocumentChangeEvent, Uri } from 'vscode';
import { Converter } from 'vscode-languageclient/lib/common/codeConverter';
import {
DidChangeNotebookDocumentNotification,
LanguageClient,
Middleware,
NotebookCellKind,
NotebookDocumentChangeEvent,
} from 'vscode-languageclient/node';
import * as proto from 'vscode-languageserver-protocol';
import { JupyterExtensionIntegration } from '../../jupyter/jupyterIntegration';

type TextContent = Required<Required<Required<proto.NotebookDocumentChangeEvent>['cells']>['textContent']>[0];

/**
* Detects the input box text documents of Interactive Windows and makes them appear to be
* the last cell of their corresponding notebooks.
*/
export class LspInteractiveWindowMiddlewareAddon implements Middleware, Disposable {
constructor(
private readonly getClient: () => LanguageClient | undefined,
private readonly jupyterExtensionIntegration: JupyterExtensionIntegration,
) {
// Make sure a bunch of functions are bound to this. VS code can call them without a this context
this.didOpen = this.didOpen.bind(this);
this.didChange = this.didChange.bind(this);
this.didClose = this.didClose.bind(this);
}

public dispose(): void {
// Nothing to dispose at the moment
}

// Map of document URIs to NotebookDocuments for all known notebooks.
private notebookDocumentMap: Map<string, NotebookDocument> = new Map<string, NotebookDocument>();

// Map of document URIs to TextDocuments that should be linked to a notebook
// whose didOpen we're expecting to see in the future.
private unlinkedTextDocumentMap: Map<string, TextDocument> = new Map<string, TextDocument>();

public async didOpen(document: TextDocument, next: (ev: TextDocument) => Promise<void>): Promise<void> {
const notebookUri = this.getNotebookUriForTextDocumentUri(document.uri);
if (!notebookUri) {
await next(document);
return;
}

const notebookDocument = this.notebookDocumentMap.get(notebookUri.toString());
if (!notebookDocument) {
this.unlinkedTextDocumentMap.set(notebookUri.toString(), document);
return;
}

try {
const result: NotebookDocumentChangeEvent = {
cells: {
structure: {
array: {
start: notebookDocument.cellCount,
deleteCount: 0,
cells: [{ kind: NotebookCellKind.Code, document: document.uri.toString() }],
},
didOpen: [
{
uri: document.uri.toString(),
languageId: document.languageId,
version: document.version,
text: document.getText(),
},
],
didClose: undefined,
},
},
};

await this.getClient()?.sendNotification(DidChangeNotebookDocumentNotification.type, {
notebookDocument: { version: notebookDocument.version, uri: notebookUri.toString() },
change: result,
});
} catch (error) {
this.getClient()?.error('Sending DidChangeNotebookDocumentNotification failed', error);
throw error;
}
}

public async didChange(
event: TextDocumentChangeEvent,
next: (ev: TextDocumentChangeEvent) => Promise<void>,
): Promise<void> {
const notebookUri = this.getNotebookUriForTextDocumentUri(event.document.uri);
if (!notebookUri) {
await next(event);
return;
}

const notebookDocument = this.notebookDocumentMap.get(notebookUri.toString());
if (notebookDocument) {
const client = this.getClient();
if (client) {
client.sendNotification(proto.DidChangeNotebookDocumentNotification.type, {
notebookDocument: { uri: notebookUri.toString(), version: notebookDocument.version },
change: {
cells: {
textContent: [
LspInteractiveWindowMiddlewareAddon._asTextContentChange(
event,
client.code2ProtocolConverter,
),
],
},
},
});
}
}
}

private static _asTextContentChange(event: TextDocumentChangeEvent, c2pConverter: Converter): TextContent {
const params = c2pConverter.asChangeTextDocumentParams(event);
return { document: params.textDocument, changes: params.contentChanges };
}

public async didClose(document: TextDocument, next: (ev: TextDocument) => Promise<void>): Promise<void> {
const notebookUri = this.getNotebookUriForTextDocumentUri(document.uri);
if (!notebookUri) {
await next(document);
return;
}

this.unlinkedTextDocumentMap.delete(notebookUri.toString());
}

public async didOpenNotebook(
notebookDocument: NotebookDocument,
cells: NotebookCell[],
next: (notebookDocument: NotebookDocument, cells: NotebookCell[]) => Promise<void>,
): Promise<void> {
this.notebookDocumentMap.set(notebookDocument.uri.toString(), notebookDocument);

const relatedTextDocument = this.unlinkedTextDocumentMap.get(notebookDocument.uri.toString());
if (relatedTextDocument) {
const newCells = [
...cells,
{
index: notebookDocument.cellCount,
notebook: notebookDocument,
kind: NotebookCellKind.Code,
document: relatedTextDocument,
metadata: {},
outputs: [],
executionSummary: undefined,
},
];

this.unlinkedTextDocumentMap.delete(notebookDocument.uri.toString());

await next(notebookDocument, newCells);
} else {
await next(notebookDocument, cells);
}
}

public async didCloseNotebook(
notebookDocument: NotebookDocument,
cells: NotebookCell[],
next: (notebookDocument: NotebookDocument, cells: NotebookCell[]) => Promise<void>,
): Promise<void> {
this.notebookDocumentMap.delete(notebookDocument.uri.toString());

await next(notebookDocument, cells);
}

notebooks = {
didOpen: this.didOpenNotebook.bind(this),
didClose: this.didCloseNotebook.bind(this),
};

private getNotebookUriForTextDocumentUri(textDocumentUri: Uri): Uri | undefined {
const getNotebookUriFunction = this.jupyterExtensionIntegration.getGetNotebookUriForTextDocumentUriFunction();
if (!getNotebookUriFunction) {
return undefined;
}

return getNotebookUriFunction(textDocumentUri);
}
}
33 changes: 33 additions & 0 deletions src/client/activation/node/lspNotebooksExperiment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService

private isInExperiment: boolean | undefined;

private supportsInteractiveWindow: boolean | undefined;

constructor(
@inject(IServiceContainer) private readonly serviceContainer: IServiceContainer,
@inject(IConfigurationService) private readonly configurationService: IConfigurationService,
Expand Down Expand Up @@ -60,6 +62,10 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService
return this.isInExperiment ?? false;
}

public isInNotebooksExperimentWithInteractiveWindowSupport(): boolean {
return this.supportsInteractiveWindow ?? false;
}

private updateExperimentSupport(): void {
const wasInExperiment = this.isInExperiment;
const isInTreatmentGroup = this.configurationService.getSettings().pylanceLspNotebooksEnabled;
Expand Down Expand Up @@ -87,6 +93,18 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService
sendTelemetryEvent(EventName.PYTHON_EXPERIMENTS_LSP_NOTEBOOKS);
}

this.supportsInteractiveWindow = false;
if (!this.isInExperiment) {
traceLog(`LSP Notebooks interactive window support is disabled -- not in LSP Notebooks experiment`);
} else if (!LspNotebooksExperiment.jupyterSupportsLspInteractiveWindow()) {
traceLog(`LSP Notebooks interactive window support is disabled -- Jupyter is not new enough`);
} else if (!LspNotebooksExperiment.pylanceSupportsLspInteractiveWindow()) {
traceLog(`LSP Notebooks interactive window support is disabled -- Pylance is not new enough`);
} else {
this.supportsInteractiveWindow = true;
traceLog(`LSP Notebooks interactive window support is enabled`);
}

// Our "in experiment" status can only change from false to true. That's possible if Pylance
// or Jupyter is installed after Python is activated. A true to false transition would require
// either Pylance or Jupyter to be uninstalled or downgraded after Python activated, and that
Expand Down Expand Up @@ -114,6 +132,21 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService
);
}

private static jupyterSupportsLspInteractiveWindow(): boolean {
const jupyterVersion = extensions.getExtension(JUPYTER_EXTENSION_ID)?.packageJSON.version;
return (
jupyterVersion && (semver.gt(jupyterVersion, '2022.7.1002041057') || semver.patch(jupyterVersion) === 100)
);
}

private static pylanceSupportsLspInteractiveWindow(): boolean {
const pylanceVersion = extensions.getExtension(PYLANCE_EXTENSION_ID)?.packageJSON.version;
return (
pylanceVersion &&
(semver.gte(pylanceVersion, '2022.7.51') || semver.prerelease(pylanceVersion)?.includes('dev'))
);
}

private async waitForJupyterToRegisterPythonPathFunction(): Promise<void> {
const jupyterExtensionIntegration = this.serviceContainer.get<JupyterExtensionIntegration>(
JupyterExtensionIntegration,
Expand Down
6 changes: 5 additions & 1 deletion src/client/activation/node/manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,11 @@ export class NodeLanguageServerManager implements ILanguageServerManager {
@traceDecoratorVerbose('Starting language server')
protected async startLanguageServer(): Promise<void> {
const options = await this.analysisOptions.getAnalysisOptions();
this.middleware = new NodeLanguageClientMiddleware(this.serviceContainer, this.lsVersion);
this.middleware = new NodeLanguageClientMiddleware(
this.serviceContainer,
() => this.languageServerProxy.languageClient,
this.lsVersion,
);
options.middleware = this.middleware;

// Make sure the middleware is connected if we restart and we we're already connected.
Expand Down
Loading