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.

Sign up for GitHub

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

Add setting for supporting pylance handling notebook intellisense #8140

Merged
merged 1 commit into from
Nov 2, 2021
Merged

Add setting for supporting pylance handling notebook intellisense #8140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions news/3 Code Health/8096.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add setting to allow usage of experimental pylance intellisense in notebooks.
6 changes: 3 additions & 3 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 7 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1859,6 +1859,12 @@
"default": true,
"description": "Append a new empty cell to an interactive window file on running the currently last cell.",
"scope": "resource"
},
"jupyter.pylanceHandlesNotebooks": {
"type": "boolean",
"default": false,
"description": "Determines if pylance's experimental notebook support is used or not",
"scope": "machine"
}
}
},
Expand Down Expand Up @@ -1974,7 +1980,7 @@
"@jupyterlab/services": "^6.1.17",
"@lumino/widgets": "^1.28.0",
"@nteract/messaging": "^7.0.0",
"@vscode/jupyter-lsp-middleware": "^0.2.17",
"@vscode/jupyter-lsp-middleware": "^0.2.18",
"ansi-to-html": "^0.6.7",
"arch": "^2.1.0",
"bootstrap": "^4.3.1",
Expand Down
1 change: 1 addition & 0 deletions src/client/common/configSettings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ export class JupyterSettings implements IWatchableJupyterSettings {
public verboseLogging: boolean = false;
public showVariableViewWhenDebugging: boolean = true;
public newCellOnRunLast: boolean = true;
public pylanceHandlesNotebooks: boolean = false;

public variableTooltipFields: IVariableTooltipFields = {
python: {
Expand Down
1 change: 1 addition & 0 deletions src/client/common/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ export interface IJupyterSettings {
readonly variableTooltipFields: IVariableTooltipFields;
readonly showVariableViewWhenDebugging: boolean;
readonly newCellOnRunLast: boolean;
readonly pylanceHandlesNotebooks?: boolean;
}

export interface IVariableTooltipFields {
Expand Down
30 changes: 27 additions & 3 deletions src/client/datascience/notebook/intellisense/intellisenseProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
// Licensed under the MIT License.
'use strict';
import { inject, injectable } from 'inversify';
import { NotebookDocument, Uri } from 'vscode';
import { ConfigurationChangeEvent, NotebookDocument, Uri } from 'vscode';
import { IExtensionSyncActivationService } from '../../../activation/types';
import { IVSCodeNotebook, IWorkspaceService } from '../../../common/application/types';
import { IDisposableRegistry } from '../../../common/types';
import { IConfigurationService, IDisposableRegistry } from '../../../common/types';
import { IInterpreterService } from '../../../interpreter/contracts';
import { PythonEnvironment } from '../../../pythonEnvironments/info';
import { getInterpreterId } from '../../../pythonEnvironments/info/interpreter';
Expand Down Expand Up @@ -34,7 +34,8 @@ export class IntellisenseProvider implements IExtensionSyncActivationService {
@inject(IVSCodeNotebook) private readonly notebooks: IVSCodeNotebook,
@inject(IInterpreterService) private readonly interpreterService: IInterpreterService,
@inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService,
@inject(IInteractiveWindowProvider) private readonly interactiveWindowProvider: IInteractiveWindowProvider
@inject(IInteractiveWindowProvider) private readonly interactiveWindowProvider: IInteractiveWindowProvider,
@inject(IConfigurationService) private readonly configService: IConfigurationService
) {}
public activate() {
// Sign up for kernel change events on notebooks
Expand All @@ -50,6 +51,9 @@ export class IntellisenseProvider implements IExtensionSyncActivationService {
// can compare during intellisense operations.
this.getActiveInterpreterSync(undefined);
this.interpreterService.onDidChangeInterpreter(this.handleInterpreterChange, this, this.disposables);

// If we change the language server type, we need to restart
this.workspaceService.onDidChangeConfiguration(this.onDidChangeConfiguration, this, this.disposables);
}

private handleInterpreterChange() {
Expand Down Expand Up @@ -157,12 +161,18 @@ export class IntellisenseProvider implements IExtensionSyncActivationService {
private async ensureLanguageServer(interpreter: PythonEnvironment | undefined, notebook: NotebookDocument) {
// We should have one language server per active interpreter.

// Check the setting to determine if we let pylance handle notebook intellisense or not
const middlewareType = this.configService.getSettings(notebook.uri).pylanceHandlesNotebooks
? 'pylance'
: 'jupyter';

// See if we already have one for this interpreter or not
const id = interpreter ? getInterpreterId(interpreter) : undefined;
if (id && !this.servers.has(id) && interpreter) {
// We don't already have one. Create a new one for this interpreter.
// The logic for whether or not
const languageServerPromise = LanguageServer.createLanguageServer(
middlewareType,
interpreter,
this.shouldAllowIntellisense.bind(this)
).then((l) => {
Expand All @@ -175,4 +185,18 @@ export class IntellisenseProvider implements IExtensionSyncActivationService {

return id ? this.servers.get(id) : undefined;
}

private onDidChangeConfiguration(event: ConfigurationChangeEvent) {
if (
event.affectsConfiguration('jupyter.pylanceHandlesNotebooks') ||
event.affectsConfiguration('python.languageServer')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably doesn't matter, but feasibly someone could change from "Pylance" to "Default" or vice versa, in which case the config changes from VS Code's view, but under the hood, the Python extension does nothing.

I doubt it really matters, but thought I'd mention.

) {
// Dispose all servers and start over for each open notebook
this.servers.forEach((p) => p.then((s) => s?.dispose()));
this.servers.clear();

// For all currently open notebooks, launch their language server
this.notebooks.notebookDocuments.forEach((n) => this.openedNotebook(n).ignoreErrors());
}
}
}
28 changes: 18 additions & 10 deletions src/client/datascience/notebook/intellisense/languageServer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import * as path from 'path';
import * as fs from 'fs-extra';
import { FileBasedCancellationStrategy } from './fileBasedCancellationStrategy';
import { NOTEBOOK_SELECTOR, PYTHON_LANGUAGE } from '../../../common/constants';
import { createNotebookMiddleware, NotebookMiddleware } from '@vscode/jupyter-lsp-middleware';
import { createNotebookMiddleware, createPylanceMiddleware, NotebookMiddleware } from '@vscode/jupyter-lsp-middleware';
import { traceInfo } from '../../../common/logger';
import { PythonEnvironment } from '../../../pythonEnvironments/info';
import { sleep } from '../../../common/utils/async';
Expand Down Expand Up @@ -144,6 +144,7 @@ export class LanguageServer implements Disposable {
}

public static async createLanguageServer(
middlewareType: 'pylance' | 'jupyter',
interpreter: PythonEnvironment,
shouldAllowIntellisense: (uri: Uri, interpreterId: string, interpreterPath: string) => boolean
): Promise<LanguageServer | undefined> {
Expand All @@ -153,15 +154,22 @@ export class LanguageServer implements Disposable {
let languageClient: LanguageClient | undefined;
const outputChannel = window.createOutputChannel(`${interpreter.displayName || 'notebook'}-languageserver`);
const interpreterId = getInterpreterId(interpreter);
const middleware = createNotebookMiddleware(
notebookApi,
() => languageClient,
() => noop, // Don't trace output. Slows things down too much
NOTEBOOK_SELECTOR,
/.*\.(ipynb|interactive)/m,
interpreter.path,
(uri) => shouldAllowIntellisense(uri, interpreterId, interpreter.path)
);
const middleware =
middlewareType == 'jupyter'
? createNotebookMiddleware(
notebookApi,
() => languageClient,
() => noop, // Don't trace output. Slows things down too much
NOTEBOOK_SELECTOR,
/.*\.(ipynb|interactive)/m,
interpreter.path,
(uri) => shouldAllowIntellisense(uri, interpreterId, interpreter.path)
)
: createPylanceMiddleware(
() => languageClient,
interpreter.path,
(uri) => shouldAllowIntellisense(uri, interpreterId, interpreter.path)
);

// Client options should be the same for all servers we support.
const clientOptions: LanguageClientOptions = {
Expand Down