Skip to content

Support for LSP notebooks experiment #19087

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

Merged
merged 41 commits into from
May 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
2b8c3ff
WIP
debonte Apr 14, 2022
c26e4cc
Setting instead of experiment, fix LspNotebooksExperiment, inject fun…
debonte Apr 21, 2022
0478e8b
API is now being called but func is not saved correctly
debonte Apr 21, 2022
dfa043c
Save func on API object; not getting notebookDocument/did*
debonte Apr 22, 2022
ac28d11
Upgrade to latest LSP
debonte Apr 23, 2022
20f1416
Remove sync property hack
debonte Apr 23, 2022
980372b
Fix usage of LanguageClient.start/stop
debonte Apr 26, 2022
7dce31f
Remove unnecessary doc filters change
debonte Apr 26, 2022
42bb8b3
Determine if experiment is supported via extension versions; telemetr…
debonte Apr 26, 2022
02fbe94
Debug logging in LspNotebooksExperiment
debonte Apr 26, 2022
6c17863
Special handling for config requests on ipynb files
debonte Apr 27, 2022
4062eb4
Post-rebase fixes
debonte Apr 28, 2022
3f32755
Adjust to server startup changes
debonte Apr 28, 2022
4cd555f
Get JupyterExtensionIntegration via container.get<T> since it's creat…
debonte Apr 28, 2022
ebf1251
Call jupyterPythonPathFunction for all URIs, not just ipynb files
debonte Apr 29, 2022
07ed0db
Fix issues in analysisOptions.ts
debonte Apr 29, 2022
4b1870b
Log interpreter path provided by Jupyter
debonte Apr 30, 2022
4a18117
Update to latest LSP
debonte Apr 30, 2022
9b88856
Remove registerProposedFeatures accidentally added back during rebase
debonte May 2, 2022
5915be0
Cleanup for PR
debonte May 2, 2022
6ab13b2
Split LanguageClientMiddleware into Node/Jedi versions
debonte May 5, 2022
d2c1f55
First attempt at handling late Pylance/Jupyter install
debonte May 6, 2022
485db85
Post rebase fixes
debonte May 6, 2022
8375668
Ensure that isInNotebooksExperiment is updated before running createH…
debonte May 6, 2022
4173428
Cleanup remove private _ prefix, ! for undefined checks
debonte May 6, 2022
bbb49b2
Remove need to register func on middleware
debonte May 6, 2022
83421a6
Allow any -dev pylance build
debonte May 6, 2022
e12df27
Adjusted logging
debonte May 6, 2022
4191e8d
Log specific reason that experiment is not enabled; 'was' needs to st…
debonte May 6, 2022
1c4121a
Add back experiment check lost during refactoring
debonte May 6, 2022
aa9d869
localize is failing on kernel switch; reverting to static string
debonte May 6, 2022
e7ff841
Fix Jedi middleware logic
debonte May 7, 2022
60366e0
Current Jupyter releases don't support experiment
debonte May 7, 2022
af38e1e
Added doc comment on setupHidingMiddleware
debonte May 7, 2022
b06e082
Don't enable experiment when using Jedi
debonte May 7, 2022
77c04c5
Remove document filter, we'll rely on selector in Pylance
debonte May 10, 2022
cc2ef35
Apply suggestions from code review
debonte May 10, 2022
4b24ad7
Updates after rebasing on top of LSP upgrade
debonte May 10, 2022
eac854e
Post-rebase fixes
debonte May 17, 2022
3189554
Update Pylance and Jupyter version thresholds
debonte May 17, 2022
bd70302
Update Jupyter version threshold
debonte May 17, 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
9 changes: 9 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -906,6 +906,15 @@
"scope": "machine-overridable",
"type": "string"
},
"python.pylanceLspNotebooksEnabled": {
"type": "boolean",
"default": false,
"description": "Determines if Pylance's experimental LSP notebooks support is used or not.",
"scope": "machine",
"tags": [
"experimental"
]
},
"python.sortImports.args": {
"default": [],
"description": "Arguments passed in. Each argument is a separate item in the array.",
Expand Down
13 changes: 13 additions & 0 deletions src/client/activation/jedi/languageClientMiddleware.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { IServiceContainer } from '../../ioc/types';
import { LanguageClientMiddleware } from '../languageClientMiddleware';
import { LanguageServerType } from '../types';

export class JediLanguageClientMiddleware extends LanguageClientMiddleware {
public constructor(serviceContainer: IServiceContainer, serverVersion?: string) {
super(serviceContainer, LanguageServerType.Jedi, serverVersion);
this.setupHidingMiddleware(serviceContainer);
}
}
5 changes: 3 additions & 2 deletions src/client/activation/jedi/languageServerProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { IInterpreterPathService, Resource } from '../../common/types';
import { PythonEnvironment } from '../../pythonEnvironments/info';
import { captureTelemetry } from '../../telemetry';
import { EventName } from '../../telemetry/constants';
import { LanguageClientMiddleware } from '../languageClientMiddleware';
import { JediLanguageClientMiddleware } from './languageClientMiddleware';
import { ProgressReporting } from '../progress';
import { ILanguageClientFactory, ILanguageServerProxy } from '../types';
import { killPid } from '../../common/process/rawProcessApis';
Expand Down Expand Up @@ -57,7 +57,8 @@ export class JediLanguageServerProxy implements ILanguageServerProxy {
options: LanguageClientOptions,
): Promise<void> {
this.lsVersion =
(options.middleware ? (<LanguageClientMiddleware>options.middleware).serverVersion : undefined) ?? '0.19.3';
(options.middleware ? (<JediLanguageClientMiddleware>options.middleware).serverVersion : undefined) ??
'0.19.3';

this.languageClient = await this.factory.createLanguageClient(resource, interpreter, options);
this.registerHandlers();
Expand Down
13 changes: 4 additions & 9 deletions src/client/activation/jedi/manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,16 @@ import { PythonEnvironment } from '../../pythonEnvironments/info';
import { captureTelemetry } from '../../telemetry';
import { EventName } from '../../telemetry/constants';
import { Commands } from '../commands';
import { LanguageClientMiddleware } from '../languageClientMiddleware';
import {
ILanguageServerAnalysisOptions,
ILanguageServerManager,
ILanguageServerProxy,
LanguageServerType,
} from '../types';
import { JediLanguageClientMiddleware } from './languageClientMiddleware';
import { ILanguageServerAnalysisOptions, ILanguageServerManager, ILanguageServerProxy } from '../types';
import { traceDecoratorError, traceDecoratorVerbose, traceVerbose } from '../../logging';

export class JediLanguageServerManager implements ILanguageServerManager {
private resource!: Resource;

private interpreter: PythonEnvironment | undefined;

private middleware: LanguageClientMiddleware | undefined;
private middleware: JediLanguageClientMiddleware | undefined;

private disposables: IDisposable[] = [];

Expand Down Expand Up @@ -132,7 +127,7 @@ export class JediLanguageServerManager implements ILanguageServerManager {
@traceDecoratorVerbose('Starting language server')
protected async startLanguageServer(): Promise<void> {
const options = await this.analysisOptions.getAnalysisOptions();
this.middleware = new LanguageClientMiddleware(this.serviceContainer, LanguageServerType.Jedi, this.lsVersion);
this.middleware = new JediLanguageClientMiddleware(this.serviceContainer, this.lsVersion);
options.middleware = this.middleware;

// Make sure the middleware is connected if we restart and we we're already connected.
Expand Down
40 changes: 27 additions & 13 deletions src/client/activation/languageClientMiddleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,31 +14,45 @@ import { createHidingMiddleware } from '@vscode/jupyter-lsp-middleware';
export class LanguageClientMiddleware extends LanguageClientMiddlewareBase {
public constructor(serviceContainer: IServiceContainer, serverType: LanguageServerType, serverVersion?: string) {
super(serviceContainer, serverType, sendTelemetryEvent, serverVersion);
}

if (serverType === LanguageServerType.None) {
return;
}

/**
* Creates the HidingMiddleware if needed and sets up code to do so if needed after
* Jupyter is installed.
*
* This method should be called from the constructor of derived classes. It is separated
* from the constructor to allow derived classes to initialize before it is called.
*/
protected setupHidingMiddleware(serviceContainer: IServiceContainer) {
const jupyterDependencyManager = serviceContainer.get<IJupyterExtensionDependencyManager>(
IJupyterExtensionDependencyManager,
);
const disposables = serviceContainer.get<IDisposableRegistry>(IDisposableRegistry) || [];
const extensions = serviceContainer.get<IExtensions>(IExtensions);

// Enable notebook support if jupyter support is installed
if (jupyterDependencyManager && jupyterDependencyManager.isJupyterExtensionInstalled) {
if (this.shouldCreateHidingMiddleware(jupyterDependencyManager)) {
this.notebookAddon = createHidingMiddleware();
}

disposables.push(
extensions?.onDidChange(() => {
if (jupyterDependencyManager) {
if (this.notebookAddon && !jupyterDependencyManager.isJupyterExtensionInstalled) {
this.notebookAddon = undefined;
} else if (!this.notebookAddon && jupyterDependencyManager.isJupyterExtensionInstalled) {
this.notebookAddon = createHidingMiddleware();
}
}
extensions?.onDidChange(async () => {
await this.onExtensionChange(jupyterDependencyManager);
}),
);
}

protected shouldCreateHidingMiddleware(jupyterDependencyManager: IJupyterExtensionDependencyManager): boolean {
return jupyterDependencyManager && jupyterDependencyManager.isJupyterExtensionInstalled;
}

protected async onExtensionChange(jupyterDependencyManager: IJupyterExtensionDependencyManager): Promise<void> {
if (jupyterDependencyManager) {
if (this.notebookAddon && !this.shouldCreateHidingMiddleware(jupyterDependencyManager)) {
this.notebookAddon = undefined;
} else if (!this.notebookAddon && this.shouldCreateHidingMiddleware(jupyterDependencyManager)) {
this.notebookAddon = createHidingMiddleware();
}
}
}
}
9 changes: 8 additions & 1 deletion src/client/activation/languageClientMiddlewareBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,9 @@ export class LanguageClientMiddlewareBase implements Middleware {
const settingDict: LSPObject & { pythonPath: string; _envPYTHONPATH: string } = settings[
i
] as LSPObject & { pythonPath: string; _envPYTHONPATH: string };
settingDict.pythonPath = configService.getSettings(uri).pythonPath;

settingDict.pythonPath =
(await this.getPythonPathOverride(uri)) ?? configService.getSettings(uri).pythonPath;

const env = await envService.getEnvironmentVariables(uri);
const envPYTHONPATH = env.PYTHONPATH;
Expand All @@ -100,6 +102,11 @@ export class LanguageClientMiddlewareBase implements Middleware {
},
};

// eslint-disable-next-line class-methods-use-this
protected async getPythonPathOverride(_uri: Uri | undefined): Promise<string | undefined> {
return undefined;
}

private get connected(): Promise<boolean> {
return this.connectedPromise.promise;
}
Expand Down
8 changes: 7 additions & 1 deletion src/client/activation/node/analysisOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,15 @@ import { IWorkspaceService } from '../../common/application/types';

import { LanguageServerAnalysisOptionsBase } from '../common/analysisOptions';
import { ILanguageServerOutputChannel } from '../types';
import { LspNotebooksExperiment } from './lspNotebooksExperiment';

export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOptionsBase {
// eslint-disable-next-line @typescript-eslint/no-useless-constructor
constructor(lsOutputChannel: ILanguageServerOutputChannel, workspace: IWorkspaceService) {
constructor(
lsOutputChannel: ILanguageServerOutputChannel,
workspace: IWorkspaceService,
private readonly lspNotebooksExperiment: LspNotebooksExperiment,
) {
super(lsOutputChannel, workspace);
}

Expand All @@ -18,6 +23,7 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt
return ({
experimentationSupport: true,
trustedWorkspaceSupport: true,
lspNotebooksSupport: this.lspNotebooksExperiment.isInNotebooksExperiment(),
} as unknown) as LanguageClientOptions;
}
}
61 changes: 61 additions & 0 deletions src/client/activation/node/languageClientMiddleware.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { Uri } from 'vscode';
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 { LanguageServerType } from '../types';

import { LspNotebooksExperiment } from './lspNotebooksExperiment';

export class NodeLanguageClientMiddleware extends LanguageClientMiddleware {
private readonly lspNotebooksExperiment: LspNotebooksExperiment;

public constructor(serviceContainer: IServiceContainer, serverVersion?: string) {
super(serviceContainer, LanguageServerType.Node, serverVersion);

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

protected shouldCreateHidingMiddleware(jupyterDependencyManager: IJupyterExtensionDependencyManager): boolean {
return (
super.shouldCreateHidingMiddleware(jupyterDependencyManager) &&
!this.lspNotebooksExperiment.isInNotebooksExperiment()
);
}

protected async onExtensionChange(jupyterDependencyManager: IJupyterExtensionDependencyManager): Promise<void> {
if (jupyterDependencyManager && jupyterDependencyManager.isJupyterExtensionInstalled) {
await this.lspNotebooksExperiment.onJupyterInstalled();
}

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();
if (!jupyterPythonPathFunction) {
return undefined;
}

const result = await jupyterPythonPathFunction(uri);

if (result) {
traceLog(`Jupyter provided interpreter path override: ${result}`);
}

return result;
}
}
Loading