8000 connected jupyter, got rid of lsp notebook exp checks, interactive no… · microsoft/vscode-python@4b5b66d · GitHub
[go: up one dir, main page]
Skip to content

Commit 4b5b66d

Browse files
heejaechangheejaechang
committed
connected jupyter, got rid of lsp notebook exp checks, interactive notebook check and etc.
1 parent 8f0c3d5 commit 4b5b66d

12 files changed

+72
-147
lines changed

‎src/client/activation/languageClientMiddleware.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ export class LanguageClientMiddleware extends LanguageClientMiddlewareBase {
4242
);
4343
}
4444

45-
protected shouldCreateHidingMiddleware(jupyterDependencyManager: IJupyterExtensionDependencyManager): boolean {
45+
private shouldCreateHidingMiddleware(jupyterDependencyManager: IJupyterExtensionDependencyManager): boolean {
4646
return jupyterDependencyManager && jupyterDependencyManager.isJupyterExtensionInstalled;
4747
}
4848

‎src/client/activation/node/analysisOptions.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import { IExperimentService } from '../../common/types';
1010

1111
import { LanguageServerAnalysisOptionsBase } from '../common/analysisOptions';
1212
import { ILanguageServerOutputChannel } from '../types';
13-
import { LspNotebooksExperiment } from './lspNotebooksExperiment';
1413
import { traceWarn } from '../../logging';
1514

1615
const EDITOR_CONFIG_SECTION = 'editor';
@@ -22,7 +21,6 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt
2221
lsOutputChannel: ILanguageServerOutputChannel,
2322
workspace: IWorkspaceService,
2423
private readonly experimentService: IExperimentService,
25-
private readonly lspNotebooksExperiment: LspNotebooksExperiment,
2624
) {
2725
super(lsOutputChannel, workspace);
2826
}
@@ -36,8 +34,6 @@ export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt
3634
return ({
3735
experimentationSupport: true,
3836
trustedWorkspaceSupport: true,
39-
lspNotebooksSupport: this.lspNotebooksExperiment.isInNotebooksExperiment(),
40-
lspInteractiveWindowSupport: this.lspNotebooksExperiment.isInNotebooksExperimentWithInteractiveWindowSupport(),
4137
autoIndentSupport: await this.isAutoIndentEnabled(),
4238
} as unknown) as LanguageClientOptions;
4339
}

‎src/client/activation/node/languageClientMiddleware.ts

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -36,40 +36,29 @@ export class NodeLanguageClientMiddleware extends LanguageClientMiddleware {
3636
this.jupyterExtensionIntegration = serviceContainer.get<JupyterExtensionIntegration>(
3737
JupyterExtensionIntegration,
3838
);
39-
if (!this.notebookAddon && this.lspNotebooksExperiment.isInNotebooksExperimentWithInteractiveWindowSupport()) {
39+
if (!this.notebookAddon) {
4040
this.notebookAddon = new LspInteractiveWindowMiddlewareAddon(
4141
this.getClient,
4242
this.jupyterExtensionIntegration,
4343
);
4444
}
4545
}
4646

47-
protected shouldCreateHidingMiddleware(jupyterDependencyManager: IJupyterExtensionDependencyManager): boolean {
48-
return (
49-
super.shouldCreateHidingMiddleware(jupyterDependencyManager) &&
50-
!this.lspNotebooksExperiment.isInNotebooksExperiment()
51-
);
52-
}
53-
5447
protected async onExtensionChange(jupyterDependencyManager: IJupyterExtensionDependencyManager): Promise<void> {
5548
if (jupyterDependencyManager && jupyterDependencyManager.isJupyterExtensionInstalled) {
5649
await this.lspNotebooksExperiment.onJupyterInstalled();
5750
}
5851

59-
if (this.lspNotebooksExperiment.isInNotebooksExperimentWithInteractiveWindowSupport()) {
60-
if (!this.notebookAddon) {
61-
this.notebookAddon = new LspInteractiveWindowMiddlewareAddon(
62-
this.getClient,
63-
this.jupyterExtensionIntegration,
64-
);
65-
}
66-
} else {
67-
super.onExtensionChange(jupyterDependencyManager);
52+
if (!this.notebookAddon) {
53+
this.notebookAddon = new LspInteractiveWindowMiddlewareAddon(
54+
this.getClient,
55+
this.jupyterExtensionIntegration,
56+
);
6857
}
6958
}
7059

7160
protected async getPythonPathOverride(uri: Uri | undefined): Promise<string | undefined> {
72-
if (!uri || !this.lspNotebooksExperiment.isInNotebooksExperiment()) {
61+
if (!uri) {
7362
return undefined;
7463
}
7564

‎src/client/activation/node/languageServerProxy.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
LanguageClientOptions,
1010
} from 'vscode-languageclient/node';
1111

12+
import { Extension } from 'vscode';
1213
import { IExperimentService, IExtensions, IInterpreterPathService, Resource } from '../../common/types';
1314
import { IEnvironmentVariablesProvider } from '../../common/variables/types';
1415
import { PythonEnvironment } from '../../pythonEnvironments/info';
@@ -95,7 +96,7 @@ export class NodeLanguageServerProxy implements ILanguageServerProxy {
9596
const extension = await this.getPylanceExtension();
9697
this.lsVersion = extension?.packageJSON.version || '0';
9798

98-
const api = extension?.exports as PylanceApi | undefined;
99+
const api = extension?.exports;
99100
if (api && api.client && api.client.isEnabled()) {
100101
this.pylanceApi = api;
101102
await api.client.start();
@@ -220,8 +221,8 @@ export class NodeLanguageServerProxy implements ILanguageServerProxy {
220221
);
221222
}
222223

223-
private async getPylanceExtension() {
224-
const extension = this.extensions.getExtension(PYLANCE_EXTENSION_ID);
224+
private async getPylanceExtension(): Promise<Extension<PylanceApi> | undefined> {
225+
const extension = this.extensions.getExtension<PylanceApi>(PYLANCE_EXTENSION_ID);
225226
if (!extension) {
226227
return undefined;
227228
}

‎src/client/activation/node/lspNotebooksExperiment.ts

Lines changed: 11 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33
import { inject, injectable } from 'inversify';
4-
import * as semver from 'semver';
54
import { Disposable, extensions } from 'vscode';
65
import { IConfigurationService, IDisposableRegistry } from '../../common/types';
76
import { sendTelemetryEvent } from '../../telemetry';
@@ -10,7 +9,6 @@ import { JUPYTER_EXTENSION_ID, PYLANCE_EXTENSION_ID } from '../../common/constan
109
import { IExtensionSingleActivationService, LanguageServerType } from '../types';
1110
import { traceLog, traceVerbose } from '../../logging';
1211
import { IJupyterExtensionDependencyManager } from '../../common/application/types';
13-
import { ILanguageServerWatcher } from '../../languageServer/types';
1412
import { IServiceContainer } from '../../ioc/types';
1513
import { sleep } from '../../common/utils/async';
1614
import { JupyterExtensionIntegration } from '../../jupyter/jupyterIntegration';
@@ -23,9 +21,7 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService
2321

2422
private isJupyterInstalled = false;
2523

26-
private isInExperiment: boolean | undefined;
27-
28-
private supportsInteractiveWindow: boolean | undefined;
24+
private isUsingPylance: boolean | undefined;
2925

3026
constructor(
3127
@inject(IServiceContainer) private readonly serviceContainer: IServiceContainer,
@@ -50,101 +46,33 @@ export class LspNotebooksExperiment implements IExtensionSingleActivationService
5046
return;
5147
}
5248

53-
if (LspNotebooksExperiment.jupyterSupportsNotebooksExperiment()) {
54-
await this.waitForJupyterToRegisterPythonPathFunction();
55-
this.updateExperimentSupport();
56-
}
49+
await this.waitForJupyterToRegisterPythonPathFunction();
50+
this.updateExperimentSupport();
5751

5852
this.isJupyterInstalled = true;
5953
}
6054

61-
public isInNotebooksExperiment(): boolean {
62-
return this.isInExperiment ?? false;
63-
}
64-
65-
public isInNotebooksExperimentWithInteractiveWindowSupport(): boolean {
66-
return this.supportsInteractiveWindow ?? false;
67-
}
68-
6955
private updateExperimentSupport(): void {
70-
const wasInExperiment = this.isInExperiment;
71-
const isInTreatmentGroup = true;
7256
const languageServerType = this.configurationService.getSettings().languageServer;
7357

74-
this.isInExperiment = false;
58+
this.isUsingPylance = false;
7559
if (languageServerType !== LanguageServerType.Node) {
76-
traceLog(`LSP Notebooks experiment is disabled -- not using Pylance`);
60+
traceLog(`LSP Notebooks is disabled -- not using Pylance`);
7761
} else if (!LspNotebooksExperiment.isJupyterInstalled()) {
78-
traceLog(`LSP Notebooks experiment is disabled -- Jupyter disabled or not installed`);
79-
} else if (!LspNotebooksExperiment.jupyterSupportsNotebooksExperiment()) {
80-
traceLog(`LSP Notebooks experiment is disabled -- Jupyter does not support experiment`);
62+
traceLog(`LSP Notebooks is disabled -- Jupyter disabled or not installed`);
8163
} else if (!LspNotebooksExperiment.isPylanceInstalled()) {
82-
traceLog(`LSP Notebooks experiment is disabled -- Pylance disabled or not installed`);
83-
} else if (!LspNotebooksExperiment.pylanceSupportsNotebooksExperiment()) {
84-
traceLog(`LSP Notebooks experiment is disabled -- Pylance does not support experiment`);
85-
} else if (!isInTreatmentGroup) {
86-
traceLog(`LSP Notebooks experiment is disabled -- not in treatment group`);
87-
// to avoid scorecard SRMs, we're also triggering the telemetry for users who meet
88-
// the criteria to experience LSP notebooks, but may be in the control group.
89-
sendTelemetryEvent(EventName.PYTHON_EXPERIMENTS_LSP_NOTEBOOKS);
64+
traceLog(`LSP Notebooks is disabled -- Pylance disabled or not installed`);
9065
} else {
91-
this.isInExperiment = true;
92-
traceLog(`LSP Notebooks experiment is enabled`);
66+
this.isUsingPylance = true;
67+
traceLog(`LSP Notebooks is enabled`);
9368
sendTelemetryEvent(EventName.PYTHON_EXPERIMENTS_LSP_NOTEBOOKS);
9469
}
9570

96-
this.supportsInteractiveWindow = false;
97-
if (!this.isInExperiment) {
98-
traceLog(`LSP Notebooks interactive window support is disabled -- not in LSP Notebooks experiment`);
99-
} else if (!LspNotebooksExperiment.jupyterSupportsLspInteractiveWindow()) {
100-
traceLog(`LSP Notebooks interactive window support is disabled -- Jupyter is not new enough`);
101-
} else if (!LspNotebooksExperiment.pylanceSupportsLspInteractiveWindow()) {
102-
traceLog(`LSP Notebooks interactive window support is disabled -- Pylance is not new enough`);
71+
if (!this.isUsingPylance) {
72+
traceLog(`LSP Notebooks interactive window support is disabled -- not using Pylance`);
10373
} else {
104-
this.supportsInteractiveWindow = true;
10574
traceLog(`LSP Notebooks interactive window support is enabled`);
10675
}
107-
108-
// Our "in experiment" status can only change from false to true. That's possible if Pylance
109-
// or Jupyter is installed after Python is activated. A true to false transition would require
110-
// either Pylance or Jupyter to be uninstalled or downgraded after Python activated, and that
111-
// would require VS Code to be reloaded before the new extension version could be used.
112-
if (wasInExperiment === false && this.isInExperiment === true) {
113-
const watcher = this.serviceContainer.get<ILanguageServerWatcher>(ILanguageServerWatcher);
114-
if (watcher) {
115-
watcher.restartLanguageServers();
116-
}
117-
}
118-
}
119-
120-
private static jupyterSupportsNotebooksExperiment(): boolean {
121-
const jupyterVersion = extensions.getExtension(JUPYTER_EXTENSION_ID)?.packageJSON.version;
122-
return (
123-
jupyterVersion && (semver.gt(jupyterVersion, '2022.5.1001411044') || semver.patch(jupyterVersion) === 100)
124-
);
125-
}
126-
127-
private static pylanceSupportsNotebooksExperiment(): boolean {
128-
const pylanceVersion = extensions.getExtension(PYLANCE_EXTENSION_ID)?.packageJSON.version;
129-
return (
130-
pylanceVersion &&
131-
(semver.gte(pylanceVersion, '2022.5.3-pre.1') || semver.prerelease(pylanceVersion)?.includes('dev'))
132-
);
133-
}
134-
135-
private static jupyterSupportsLspInteractiveWindow(): boolean {
136-
const jupyterVersion = extensions.getExtension(JUPYTER_EXTENSION_ID)?.packageJSON.version;
137-
return (
138-
jupyterVersion && (semver.gt(jupyterVersion, '2022.7.1002041057') || semver.patch(jupyterVersion) === 100)
139-
);
140-
}
141-
142-
private static pylanceSupportsLspInteractiveWindow(): boolean {
143-
const pylanceVersion = extensions.getExtension(PYLANCE_EXTENSION_ID)?.packageJSON.version;
144-
return (
145-
pylanceVersion &&
146-
(semver.gte(pylanceVersion, '2022.7.51') || semver.prerelease(pylanceVersion)?.includes('dev'))
147-
);
14876
}
14977

15078
private async waitForJupyterToRegisterPythonPathFunction(): Promise<void> {

‎src/client/activation/node/pylanceApi.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,29 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
3+
import {
4+
CancellationToken,
5+
CompletionContext,
6+
CompletionItem,
7+
CompletionList,
8+
Position,
9+
TextDocument,
10+
Uri,
11+
} from 'vscode';
312

413
export interface PylanceApi {
514
client?: {
615
isEnabled(): boolean;
716
start(): Promise<void>;
817
stop(): Promise<void>;
918
};
19+
notebook?: {
20+
registerJupyterPythonPathFunction(func: (uri: Uri) => Promise<string | undefined>): void;
21+
registerGetNotebookUriForTextDocumentUriFunction(func: (textDocumentUri: Uri) => Uri | undefined): void;
22+
getCompletionItems(
23+
document: TextDocument,
24+
position: Position,
25+
context: CompletionContext,
26+
token: CancellationToken,
27+
): Promise<CompletionItem[] | CompletionList | undefined>;
28+
};
1029
}

‎src/client/jupyter/jupyterIntegration.ts

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { dirname } from 'path';
99
import { CancellationToken, Event, Extension, Memento, Uri } from 'vscode';
1010
import type { SemVer } from 'semver';
1111
import { IWorkspaceService } from '../common/application/types';
12-
import { JUPYTER_EXTENSION_ID } from '../common/constants';
12+
import { JUPYTER_EXTENSION_ID, PYLANCE_EXTENSION_ID } from '../common/constants';
1313
import { InterpreterUri, ModuleInstallFlags } from '../common/installer/types';
1414
import {
1515
GLOBAL_MEMENTO,
@@ -34,7 +34,7 @@ import {
3434
} from '../interpreter/contracts';
3535
import { PythonEnvironment } from '../pythonEnvironments/info';
3636
import { IDataViewerDataProvider, IJupyterUriProvider } from './types';
37-
37+
import { PylanceApi } from '../activation/node/pylanceApi';
3838
/**
3939
* This allows Python extension to update Product enum without breaking Jupyter.
4040
* I.e. we have a strict contract, else using numbers (in enums) is bound to break across products.
@@ -184,6 +184,8 @@ type JupyterExtensionApi = {
184184
export class JupyterExtensionIntegration {
185185
private jupyterExtension: Extension<JupyterExtensionApi> | undefined;
186186

187+
private pylanceExtension: Extension<PylanceApi> | undefined;
188+
187189
private jupyterPythonPathFunction: ((uri: Uri) => Promise<string | undefined>) | undefined;
188190

189191
private getNotebookUriForTextDocumentUriFunction: ((textDocumentUri: Uri) => Uri | undefined) | undefined;
@@ -300,6 +302,16 @@ export class JupyterExtensionIntegration {
300302
}
301303

302304
private async getExtensionApi(): Promise<JupyterExtensionApi | undefined> {
305+
if (!this.pylanceExtension) {
306+
const pylanceExtension = this.extensions.getExtension<PylanceApi>(PYLANCE_EXTENSION_ID);
307+
308+
if (pylanceExtension && !pylanceExtension.isActive) {
309+
await pylanceExtension.activate();
310+
}
311+
312+
this.pylanceExtension = pylanceExtension;
313+
}
314+
303315
if (!this.jupyterExtension) {
304316
const jupyterExtension = this.extensions.getExtension<JupyterExtensionApi>(JUPYTER_EXTENSION_ID);
305317
if (!jupyterExtension) {
@@ -316,8 +328,18 @@ export class JupyterExtensionIntegration {
316328
return undefined;
317329
}
318330

331+
private getPylanceApi(): PylanceApi | undefined {
332+
const api = this.pylanceExtension?.exports;
333+
return api && api.notebook && api.client && api.client.isEnabled() ? api : undefined;
334+
}
335+
319336
private registerJupyterPythonPathFunction(func: (uri: Uri) => Promise<string | undefined>) {
320337
this.jupyterPythonPathFunction = func;
338+
339+
const api = this.getPylanceApi();
340+
if (api) {
341+
api.notebook!.registerJupyterPythonPathFunction(func);
342+
}
321343
}
322344

323345
public getJupyterPythonPathFunction(): ((uri: Uri) => Promise<string | undefined>) | undefined {
@@ -326,6 +348,11 @@ export class JupyterExtensionIntegration {
326348

327349
public registerGetNotebookUriForTextDocumentUriFunction(func: (textDocumentUri: Uri) => Uri | undefined): void {
328350
this.getNotebookUriForTextDocumentUriFunction = func;
351+
352+
const api = this.getPylanceApi();
353+
if (api) {
354+
api.notebook!.registerGetNotebookUriForTextDocumentUriFunction(func);
355+
}
329356
}
330357

331358
public getGetNotebookUriForTextDocumentUriFunction(): ((textDocumentUri: Uri) => Uri | undefined) | undefined {

‎src/client/languageServer/pylanceLSExtensionManager.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import { promptForPylanceInstall } from '../activation/common/languageServerChan
55
import { NodeLanguageServerAnalysisOptions } from '../activation/node/analysisOptions';
66
import { NodeLanguageClientFactory } from '../activation/node/languageClientFactory';
77
import { NodeLanguageServerProxy } from '../activation/node/languageServerProxy';
8-
import { LspNotebooksExperiment } from '../activation/node/lspNotebooksExperiment';
98
import { NodeLanguageServerManager } from '../activation/node/manager';
109
import { ILanguageServerOutputChannel } from '../activation/types';
1110
import { IApplicationShell, ICommandManager, IWorkspaceService } from '../common/application/types';
@@ -49,13 +48,11 @@ export class PylanceLSExtensionManager implements IDisposable, ILanguageServerEx
4948
fileSystem: IFileSystem,
5049
private readonly extensions: IExtensions,
5150
readonly applicationShell: IApplicationShell,
52-
lspNotebooksExperiment: LspNotebooksExperiment,
5351
) {
5452
this.analysisOptions = new NodeLanguageServerAnalysisOptions(
5553
outputChannel,
5654
workspaceService,
5755
experimentService,
58-
lspNotebooksExperiment,
5956
);
6057
this.clientFactory = new NodeLanguageClientFactory(fileSystem, extensions);
6158
this.serverProxy = new NodeLanguageServerProxy(

‎src/client/languageServer/watcher.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import { JediLSExtensionManager } from './jediLSExtensionManager';
2727
import { NoneLSExtensionManager } from './noneLSExtensionManager';
2828
import { PylanceLSExtensionManager } from './pylanceLSExtensionManager';
2929
import { ILanguageServerExtensionManager, ILanguageServerWatcher } from './types';
30-
import { LspNotebooksExperiment } from '../activation/node/lspNotebooksExperiment';
3130

3231
@injectable()
3332
/**
@@ -63,7 +62,6 @@ export class LanguageServerWatcher implements IExtensionActivationService, ILang
6362
@inject(IFileSystem) private readonly fileSystem: IFileSystem,
6463
@inject(IExtensions) private readonly extensions: IExtensions,
6564
@inject(IApplicationShell) readonly applicationShell: IApplicationShell,
66-
@inject(LspNotebooksExperiment) private readonly lspNotebooksExperiment: LspNotebooksExperiment,
6765
@inject(IDisposableRegistry) readonly disposables: IDisposableRegistry,
6866
) {
6967
this.workspaceInterpreters = new Map();
@@ -244,7 +242,6 @@ export class LanguageServerWatcher implements IExtensionActivationService, ILang
244242
this.fileSystem,
245243
this.extensions,
246244
this.applicationShell,
247-
this.lspNotebooksExperiment,
248245
);
249246
break;
250247
case LanguageServerType.None:

0 commit comments

Comments
 (0)
0