Skip to content

Commit 9cb6603

Browse files
DonJayamanneDonJayamanne
authored
Hook up commands to notebook API and add tests (#11910)
1 parent 06af191 commit 9cb6603

24 files changed

Lines changed: 1201 additions & 218 deletions

‎.vscode/launch.json‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
// Enable this to turn on redux logging during debugging
2525
"XVSC_PYTHON_FORCE_LOGGING": "1",
2626
// Enable this to try out new experiments locally
27-
"XVSC_PYTHON_LOAD_EXPERIMENTS_FROM_FILE" : "1",
27+
"XVSC_PYTHON_LOAD_EXPERIMENTS_FROM_FILE": "1",
2828
// Enable this to log telemetry to the output during debugging
2929
"XVSC_PYTHON_LOG_TELEMETRY": "1",
3030
// Enable this to log debugger output. Directory must exist ahead of time
@@ -182,6 +182,33 @@
182182
"<node_internals>/**"
183183
]
184184
},
185+
{
186+
"name": "Tests (DataScience, *.ds.test.ts)",
187+
"type": "extensionHost",
188+
"request": "launch",
189+
"runtimeExecutable": "${execPath}",
190+
"args": [
191+
"${workspaceFolder}/src/test",
192+
"--disable-extensions",
193+
"--enable-proposed-api",
194+
"--extensionDevelopmentPath=${workspaceFolder}",
195+
"--extensionTestsPath=${workspaceFolder}/out/test"
196+
],
197+
"env": {
198+
"VSC_PYTHON_CI_TEST_GREP": "", // Modify this to run a subset of the single workspace tests
199+
"VSC_PYTHON_LOAD_EXPERIMENTS_FROM_FILE": "true",
200+
"TEST_FILES_SUFFIX": "ds.test"
201+
},
202+
"stopOnEntry": false,
203+
"sourceMaps": true,
204+
"outFiles": [
205+
"${workspaceFolder}/out/**/*.js"
206+
],
207+
"preLaunchTask": "Compile",
208+
"skipFiles": [
209+
"<node_internals>/**"
210+
]
211+
},
185212
{
186213
"name": "Tests (Multiroot, VS Code, *.test.ts)",
187214
"type": "extensionHost",

‎build/ci/templates/test_phases.yml‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,17 @@ steps:
416416
env:
417417
DISPLAY: :10
418418
419+
# Run the single workspace tests in VS Code Insiders.
420+
- script: |
421+
npm run testDataScience
422+
displayName: 'Run DataScience Tests in VSCode Insiders'
423+
condition: and(succeeded(), contains(variables['TestsToRun'], 'testDataScience'))
424+
env:
425+
DISPLAY: :10
426+
VSC_PYTHON_CI_TEST_VSC_CHANNEL: 'insiders'
427+
VSC_PYTHON_LOAD_EXPERIMENTS_FROM_FILE: 'true'
428+
TEST_FILES_SUFFIX: 'ds.test'
429+
419430
# Upload the test results to Azure DevOps to facilitate test reporting in their UX.
420431
- task: PublishTestResults@2
421432
displayName: 'Publish single workspace tests results'

‎build/ci/vscode-python-pr-validation.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ stages:
4545
'Single Workspace':
4646
TestsToRun: 'testSingleWorkspace'
4747
NeedsPythonTestReqs: true
48+
'DataScience':
49+
TestsToRun: 'testDataScience'
50+
NeedsPythonTestReqs: true
4851
'Smoke':
4952
TestsToRun: 'testSmoke'
5053
NeedsPythonTestReqs: true

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2983,6 +2983,7 @@
29832983
"test:cover:report": "nyc --nycrc-path build/.nycrc report --reporter=text --reporter=html --reporter=text-summary --reporter=cobertura",
29842984
"testDebugger": "node ./out/test/testBootstrap.js ./out/test/debuggerTest.js",
29852985
"testSingleWorkspace": "node ./out/test/testBootstrap.js ./out/test/standardTest.js",
2986+
"testDataScience": "cross-env VSC_PYTHON_CI_TEST_VSC_CHANNEL=insiders TEST_FILES_SUFFIX=ds.test node ./out/test/testBootstrap.js ./out/test/standardTest.js",
29862987
"testMultiWorkspace": "node ./out/test/testBootstrap.js ./out/test/multiRootTest.js",
29872988
"testPerformance": "node ./out/test/testBootstrap.js ./out/test/performanceTest.js",
29882989
"testSmoke": "node ./out/test/smokeTest.js",

‎src/client/common/application/commands.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,10 @@ export interface ICommandNameArgumentTypeMapping extends ICommandNameWithoutArgu
9090
['python._loadLanguageServerExtension']: {}[];
9191
['python.SelectAndInsertDebugConfiguration']: [TextDocument, Position, CancellationToken];
9292
['vscode.open']: [Uri];
93+
['notebook.cell.execute']: [];
94+
['notebook.cell.insertCodeCellBelow']: [];
95+
['notebook.undo']: [];
96+
['notebook.redo']: [];
9397
['python.viewLanguageServerOutput']: [];
9498
['vscode.open']: [Uri];
9599
['workbench.action.files.saveAs']: [Uri];

‎src/client/common/application/notebook.ts‎

Lines changed: 81 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@ import { inject, injectable } from 'inversify';
55
import {
66
Disposable,
77
Event,
8+
EventEmitter,
89
GlobPattern,
910
notebook,
1011
NotebookContentProvider,
1112
NotebookDocument,
12-
NotebookDocumentChangeEvent,
1313
NotebookEditor,
1414
NotebookKernel,
1515
NotebookOutputRenderer,
@@ -18,40 +18,45 @@ import {
1818
window
1919
} from 'vscode';
2020
import { UseProposedApi } from '../constants';
21-
import { IVSCodeNotebook } from './types';
21+
import { IDisposableRegistry } from '../types';
22+
import {
23+
IVSCodeNotebook,
24+
NotebookCellLanguageChangeEvent,
25+
NotebookCellMoveEvent,
26+
NotebookCellOutputsChangeEvent,
27+
NotebookCellsChangeEvent
28+
} from './types';
2229

2330
@injectable()
2431
export class VSCodeNotebook implements IVSCodeNotebook {
25-
constructor(@inject(UseProposedApi) private readonly useProposedApi: boolean) {}
26-
public registerNotebookContentProvider(notebookType: string, provider: NotebookContentProvider): Disposable {
27-
return notebook.registerNotebookContentProvider(notebookType, provider);
28-
}
29-
public registerNotebookKernel(id: string, selectors: GlobPattern[], kernel: NotebookKernel): Disposable {
30-
return notebook.registerNotebookKernel(id, selectors, kernel);
31-
}
32-
public registerNotebookOutputRenderer(
33-
id: string,
34-
outputSelector: NotebookOutputSelector,
35-
renderer: NotebookOutputRenderer
36-
): Disposable {
37-
return notebook.registerNotebookOutputRenderer(id, outputSelector, renderer);
38-
}
32+
private readonly _onDidChangeNotebookDocument = new EventEmitter<
33+
| NotebookCellsChangeEvent
34+
| NotebookCellMoveEvent
35+
| NotebookCellOutputsChangeEvent
36+
| NotebookCellLanguageChangeEvent
37+
>();
3938
public get onDidOpenNotebookDocument(): Event<NotebookDocument> {
4039
return notebook.onDidOpenNotebookDocument;
4140
}
4241
public get onDidCloseNotebookDocument(): Event<NotebookDocument> {
4342
return notebook.onDidCloseNotebookDocument;
4443
}
45-
46-
public get onDidChangeNotebookDocument(): Event<NotebookDocumentChangeEvent> {
47-
return notebook.onDidChangeNotebookDocument;
44+
public get notebookEditors() {
45+
return notebook.visibleNotebookEditors;
4846
}
49-
public isCell(textDocument: TextDocument) {
50-
return (
51-
textDocument.fileName.toLowerCase().includes('.ipynb') &&
52-
textDocument.uri.query.includes('notebook') &&
53-
textDocument.uri.query.includes('cell')
54-
);
47+
public get onDidChangeNotebookDocument(): Event<
48+
| NotebookCellsChangeEvent
49+
| NotebookCellMoveEvent
50+
| NotebookCellOutputsChangeEvent
51+
| NotebookCellLanguageChangeEvent
52+
> {
53+
// Temporarily disabled as API is not yet
54+
// Bogus if, to satisyf compiler and ensure addEventHandlers method is used.
55+
if (process.env.SOME_BOGUS_ENV_VAR) {
56+
this.addEventHandlers();
57+
}
58+
59+
return this._onDidChangeNotebookDocument.event;
5560
}
5661
public get activeNotebookEditor(): NotebookEditor | undefined {
5762
if (!this.useProposedApi) {
@@ -66,6 +71,57 @@ export class VSCodeNotebook implements IVSCodeNotebook {
6671
if (window.activeTextEditor && !this.isCell(window.activeTextEditor.document)) {
6772
return;
6873
}
74+
// Temporary until VSC API stabilizes.
75+
if (Array.isArray(notebook.visibleNotebookEditors)) {
76+
return notebook.visibleNotebookEditors.find((item) => item.active && item.visible);
77+
}
6978
return notebook.activeNotebookEditor;
7079
}
80+
private addedEventHandlers?: boolean;
81+
constructor(
82+
@inject(UseProposedApi) private readonly useProposedApi: boolean,
83+
@inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry
84+
) {}
85+
public registerNotebookContentProvider(notebookType: string, provider: NotebookContentProvider): Disposable {
86+
return notebook.registerNotebookContentProvider(notebookType, provider);
87+
}
88+
public registerNotebookKernel(id: string, selectors: GlobPattern[], kernel: NotebookKernel): Disposable {
89+
return notebook.registerNotebookKernel(id, selectors, kernel);
90+
}
91+
public registerNotebookOutputRenderer(
92+
id: string,
93+
outputSelector: NotebookOutputSelector,
94+
renderer: NotebookOutputRenderer
95+
): Disposable {
96+
return notebook.registerNotebookOutputRenderer(id, outputSelector, renderer);
97+
}
98+
public isCell(textDocument: TextDocument) {
99+
return (
100+
(textDocument.uri.fsPath.toLowerCase().includes('.ipynb') &&
101+
textDocument.uri.query.includes('notebook') &&
102+
textDocument.uri.query.includes('cell')) ||
103+
textDocument.uri.scheme.includes('vscode-notebook-cell')
104+
);
105+
}
106+
private addEventHandlers() {
107+
if (this.addedEventHandlers) {
108+
return;
109+
}
110+
this.disposables.push(
111+
...[
112+
notebook.onDidChangeCellLanguage((e) =>
113+
this._onDidChangeNotebookDocument.fire({ ...e, type: 'changeCellLanguage' })
114+
),
115+
notebook.onDidChangeCellOutputs((e) =>
116+
this._onDidChangeNotebookDocument.fire({ ...e, type: 'changeCellOutputs' })
117+
),
118+
notebook.onDidChangeNotebookCells((e) =>
119+
this._onDidChangeNotebookDocument.fire({ ...e, type: 'changeCells' })
120+
),
121+
notebook.onDidMoveNotebookCell((e) =>
122+
this._onDidChangeNotebookDocument.fire({ ...e, type: 'moveCell' })
123+
)
124+
]
125+
);
126+
}
71127
}

‎src/client/common/application/types.ts‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,12 @@ import {
2424
InputBoxOptions,
2525
MessageItem,
2626
MessageOptions,
27+
NotebookCellLanguageChangeEvent as VSCNotebookCellLanguageChangeEvent,
28+
NotebookCellMoveEvent as VSCNotebookCellMoveEvent,
29+
NotebookCellOutputsChangeEvent as VSCNotebookCellOutputsChangeEvent,
30+
NotebookCellsChangeEvent as VSCNotebookCellsChangeEvent,
2731
NotebookContentProvider,
2832
NotebookDocument,
29-
NotebookDocumentChangeEvent,
3033
NotebookEditor,
3134
NotebookKernel,
3235
NotebookOutputRenderer,
@@ -1450,12 +1453,23 @@ export interface IClipboard {
14501453
writeText(value: string): Promise<void>;
14511454
}
14521455

1456+
export type NotebookCellsChangeEvent = { type: 'changeCells' } & VSCNotebookCellsChangeEvent;
1457+
export type NotebookCellMoveEvent = { type: 'moveCell' } & VSCNotebookCellMoveEvent;
1458+
export type NotebookCellOutputsChangeEvent = { type: 'changeCellOutputs' } & VSCNotebookCellOutputsChangeEvent;
1459+
export type NotebookCellLanguageChangeEvent = { type: 'changeCellLanguage' } & VSCNotebookCellLanguageChangeEvent;
1460+
14531461
export const IVSCodeNotebook = Symbol('IVSCodeNotebook');
14541462
export interface IVSCodeNotebook {
14551463
readonly onDidOpenNotebookDocument: Event<NotebookDocument>;
14561464
readonly onDidCloseNotebookDocument: Event<NotebookDocument>;
14571465

1458-
readonly onDidChangeNotebookDocument: Event<NotebookDocumentChangeEvent>;
1466+
readonly onDidChangeNotebookDocument: Event<
1467+
| NotebookCellsChangeEvent
1468+
| NotebookCellMoveEvent
1469+
| NotebookCellOutputsChangeEvent
1470+
| NotebookCellLanguageChangeEvent
1471+
>;
1472+
readonly notebookEditors: Readonly<NotebookEditor[]>;
14591473
readonly activeNotebookEditor: NotebookEditor | undefined;
14601474
/**
14611475
* Whether the current document is aCell.

‎src/client/common/experiments.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ export class ExperimentsManager implements IExperimentsManager {
154154
// User cannot belong to NotebookExperiment if they are not using Insiders.
155155
if (
156156
(experiment.name === NativeNotebook.experiment || experiment.name === NativeNotebook.control) &&
157-
this.appEnvironment.channel !== 'insiders'
157+
this.appEnvironment.channel === 'stable'
158158
) {
159159
continue;
160160
}

‎src/client/common/types.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ export const IDocumentSymbolProvider = Symbol('IDocumentSymbolProvider');
2828
export interface IDocumentSymbolProvider extends DocumentSymbolProvider {}
2929
export const IsWindows = Symbol('IS_WINDOWS');
3030
export const IDisposableRegistry = Symbol('IDisposableRegistry');
31-
export type IDisposableRegistry = { push(disposable: Disposable): void };
31+
export type IDisposableRegistry = { push(...items: Disposable[]): void };
3232
export const IMemento = Symbol('IGlobalMemento');
3333
export const GLOBAL_MEMENTO = Symbol('IGlobalMemento');
3434
export const WORKSPACE_MEMENTO = Symbol('IWorkspaceMemento');

‎src/client/datascience/context/activeEditorContext.ts‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,10 @@ import { inject, injectable } from 'inversify';
77
import { TextEditor } from 'vscode';
88
import { IExtensionSingleActivationService } from '../../activation/types';
99
import { ICommandManager, IDocumentManager, IVSCodeNotebook } from '../../common/application/types';
10-
import { PYTHON_LANGUAGE, UseProposedApi } from '../../common/constants';
10+
import { PYTHON_LANGUAGE } from '../../common/constants';
1111
import { ContextKey } from '../../common/contextKey';
12-
import { IDisposable, IDisposableRegistry } from '../../common/types';
12+
import { NativeNotebook } from '../../common/experimentGroups';
13+
import { IDisposable, IDisposableRegistry, IExperimentsManager } from '../../common/types';
1314
import { EditorContexts } from '../constants';
1415
import { IInteractiveWindow, IInteractiveWindowProvider, INotebookEditor, INotebookEditorProvider } from '../types';
1516

@@ -22,6 +23,7 @@ export class ActiveEditorContextService implements IExtensionSingleActivationSer
2223
private pythonOrInteractiveContext: ContextKey;
2324
private pythonOrNativeContext: ContextKey;
2425
private pythonOrInteractiveOrNativeContext: ContextKey;
26+
private hasNativeNotebookCells: ContextKey;
2527
private isPythonFileActive: boolean = false;
2628
constructor(
2729
@inject(IInteractiveWindowProvider) private readonly interactiveProvider: IInteractiveWindowProvider,
@@ -30,7 +32,7 @@ export class ActiveEditorContextService implements IExtensionSingleActivationSer
3032
@inject(ICommandManager) private readonly commandManager: ICommandManager,
3133
@inject(IDisposableRegistry) disposables: IDisposableRegistry,
3234
@inject(IVSCodeNotebook) private readonly vscodeNotebook: IVSCodeNotebook,
33-
@inject(UseProposedApi) private readonly useProposedApi: boolean
35+
@inject(IExperimentsManager) private readonly experiments: IExperimentsManager
3436
) {
3537
disposables.push(this);
3638
this.nativeContext = new ContextKey(EditorContexts.IsNativeActive, this.commandManager);
@@ -48,6 +50,7 @@ export class ActiveEditorContextService implements IExtensionSingleActivationSer
4850
EditorContexts.IsPythonOrInteractiveOrNativeActive,
4951
this.commandManager
5052
);
53+
this.hasNativeNotebookCells = new ContextKey(EditorContexts.HaveNativeCells, this.commandManager);
5154
}
5255
public dispose() {
5356
this.disposables.forEach((item) => item.dispose());
@@ -69,17 +72,26 @@ export class ActiveEditorContextService implements IExtensionSingleActivationSer
6972
if (this.docManager.activeTextEditor?.document.languageId === PYTHON_LANGUAGE) {
7073
this.onDidChangeActiveTextEditor(this.docManager.activeTextEditor);
7174
}
72-
if (this.useProposedApi) {
75+
if (this.experiments.inExperiment(NativeNotebook.experiment)) {
7376
this.vscodeNotebook.onDidChangeNotebookDocument(this.onDidChangeVSCodeNotebook, this, this.disposables);
7477
this.vscodeNotebook.onDidCloseNotebookDocument(this.onDidChangeVSCodeNotebook, this, this.disposables);
7578
this.vscodeNotebook.onDidOpenNotebookDocument(this.onDidChangeVSCodeNotebook, this, this.disposables);
7679
this.docManager.onDidChangeActiveTextEditor(this.onDidChangeVSCodeNotebook, this, this.disposables);
7780
}
7881
}
7982

83+
private udpateNativeNotebookCellContext() {
84+
if (!this.experiments.inExperiment(NativeNotebook.experiment)) {
85+
return;
86+
}
87+
this.hasNativeNotebookCells
88+
.set((this.vscodeNotebook.activeNotebookEditor?.document?.cells?.length || 0) >= 0)
89+
.ignoreErrors();
90+
}
8091
private onDidChangeVSCodeNotebook() {
8192
this.isPythonFileActive = !this.vscodeNotebook.activeNotebookEditor;
8293
this.nativeContext.set(!!this.vscodeNotebook.activeNotebookEditor).ignoreErrors();
94+
this.udpateNativeNotebookCellContext();
8395
this.updateMergedContexts();
8496
}
8597
private onDidChangeActiveInteractiveWindow(e?: IInteractiveWindow) {
@@ -93,6 +105,7 @@ export class ActiveEditorContextService implements IExtensionSingleActivationSer
93105
private onDidChangeActiveTextEditor(e?: TextEditor) {
94106
this.isPythonFileActive =
95107
e?.document.languageId === PYTHON_LANGUAGE && !this.vscodeNotebook.activeNotebookEditor;
108+
this.udpateNativeNotebookCellContext();
96109
this.updateMergedContexts();
97110
}
98111
private updateMergedContexts() {

0 commit comments

Comments
 (0)