Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/15415.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Updated to used the latest Notebook (proposed) API.
1 change: 1 addition & 0 deletions news/3 Code Health/15573.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix failing smoke tests on CI.
6 changes: 3 additions & 3 deletions package-lock.json

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

4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
"theme": "dark"
},
"engines": {
"vscode": "^1.51.0"
"vscode": "^1.54.0"
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@brettcannon We don't want anyone on any older version of VS Code to get this new update.
The API changes are not backwards compatible.
Hence the restriction to ensure only those with 1.54 get this update.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ensure only those with 1.54 get this update

@DonJayamanne then should this be "1.54.0" instead of "^1.54.0"?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes, thanks @joyceerhl , done

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

VSCode didn't like that in the engines setting, i think its not supported, leaving as ^.

},
"keywords": [
"python",
Expand Down Expand Up @@ -2082,7 +2082,7 @@
"@types/tmp": "0.0.33",
"@types/untildify": "^3.0.0",
"@types/uuid": "^3.4.3",
"@types/vscode": "~1.51.0",
"@types/vscode": "~1.53.0",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is this different than the version of the engine?

Copy link
Copy Markdown
Author

@DonJayamanne DonJayamanne Mar 9, 2021

Choose a reason for hiding this comment

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

None of the API we are using for Notebooks is in stable, its all proposed API.
Hence there's no need to change this to 1.54

"@types/webpack-bundle-analyzer": "^2.13.0",
"@types/winreg": "^1.2.30",
"@types/xml2js": "^0.4.2",
Expand Down
14 changes: 11 additions & 3 deletions src/client/common/application/notebook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type {
NotebookEditor,
NotebookKernel,
NotebookKernelProvider,
window,
} from 'vscode-proposed';
import { UseProposedApi } from '../constants';
import { IDisposableRegistry } from '../types';
Expand All @@ -34,7 +35,7 @@ export class VSCodeNotebook implements IVSCodeNotebook {
}
public get onDidChangeActiveNotebookEditor(): Event<NotebookEditor | undefined> {
return this.canUseNotebookApi
? this.notebook.onDidChangeActiveNotebookEditor
? this.window.onDidChangeActiveNotebookEditor
: new EventEmitter<NotebookEditor | undefined>().event;
}
public get onDidOpenNotebookDocument(): Event<NotebookDocument> {
Expand All @@ -56,7 +57,7 @@ export class VSCodeNotebook implements IVSCodeNotebook {
return this.canUseNotebookApi ? this.notebook.notebookDocuments : [];
}
public get notebookEditors() {
return this.canUseNotebookApi ? this.notebook.visibleNotebookEditors : [];
return this.canUseNotebookApi ? this.window.visibleNotebookEditors : [];
}
public get onDidChangeNotebookDocument(): Event<NotebookCellChangedEvent> {
return this.canUseNotebookApi
Expand All @@ -67,17 +68,24 @@ export class VSCodeNotebook implements IVSCodeNotebook {
if (!this.useProposedApi) {
return;
}
return this.notebook.activeNotebookEditor;
return this.window.activeNotebookEditor;
}
private get notebook() {
if (!this._notebook) {
this._notebook = require('vscode').notebook;
}
return this._notebook!;
}
private get window() {
if (!this._window) {
this._window = require('vscode').window;
}
return this._window!;
}
private readonly _onDidChangeNotebookDocument = new EventEmitter<NotebookCellChangedEvent>();
private addedEventHandlers?: boolean;
private _notebook?: typeof notebook;
private _window?: typeof window;
private readonly canUseNotebookApi?: boolean;
private readonly handledCellChanges = new WeakSet<VSCNotebookCellsChangeEvent>();
constructor(
Expand Down
12 changes: 11 additions & 1 deletion src/client/jupyter/languageserver/notebookConcatDocument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { isEqual } from 'lodash';
import { NotebookConcatTextDocument, NotebookCell, NotebookDocument } from 'vscode-proposed';
import { IVSCodeNotebook } from '../../common/application/types';
import { IDisposable } from '../../common/types';
import { PYTHON_LANGUAGE } from '../../common/constants';

export const NotebookConcatPrefix = '_NotebookConcat_';

Expand All @@ -44,7 +45,16 @@ export class NotebookConcatDocument implements TextDocument, IDisposable {
}

public get languageId(): string {
return this.notebook.languages[0];
// eslint-disable-next-line global-require
const { NotebookCellKind } = require('vscode');
// Return Python if we have python cells.
if (this.notebook.cells.some((item) => item.language.toLowerCase() === PYTHON_LANGUAGE.toLowerCase())) {
return PYTHON_LANGUAGE;
}
// Return the language of the first available cell, else assume its a Python notebook.
// The latter is not possible, except for the case where we have all markdown cells,
// in which case the language server will never kick in.
return this.notebook.cells.find((item) => item.cellKind === NotebookCellKind.Code)?.language || PYTHON_LANGUAGE;
}

public get version(): number {
Expand Down
6 changes: 3 additions & 3 deletions src/test/insiders/languageServer.insiders.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,12 @@ suite('Insiders Test: Language Server', () => {
expect(locations![0].uri.fsPath).to.contain(path.basename(notebookDefinitions));

// Insert a new cell
const activeEditor = vscode.notebook.activeNotebookEditor;
const activeEditor = vscode.window.activeNotebookEditor;
expect(activeEditor).not.to.be.equal(undefined, 'Active editor not found in notebook');
await activeEditor!.edit((edit) => {
edit.replaceCells(0, 0, [
{
cellKind: vscode.CellKind.Code,
cellKind: vscode.NotebookCellKind.Code,
language: PYTHON_LANGUAGE,
source: 'x = 4',
metadata: {
Expand All @@ -125,7 +125,7 @@ suite('Insiders Test: Language Server', () => {
edit.replaceCells(0, 1, []);
edit.replaceCells(1, 0, [
{
cellKind: vscode.CellKind.Code,
cellKind: vscode.NotebookCellKind.Code,
language: PYTHON_LANGUAGE,
source: 'x = 4',
metadata: {
Expand Down
2 changes: 2 additions & 0 deletions src/test/mockClasses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ export class MockOutputChannel implements vscode.OutputChannel {
}

export class MockStatusBarItem implements vscode.StatusBarItem {
backgroundColor: vscode.ThemeColor | undefined;
accessibilityInformation?: vscode.AccessibilityInformation | undefined;
public alignment!: vscode.StatusBarAlignment;
public priority!: number;
public text!: string;
Expand Down
30 changes: 28 additions & 2 deletions src/test/mocks/vsc/extHostedTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { vscUri } from './uri';
import { generateUuid } from './uuid';

export namespace vscMockExtHostedTypes {
export enum CellKind {
export enum NotebookCellKind {
Markdown = 1,
Code = 2,
}
Expand Down Expand Up @@ -544,6 +544,32 @@ export namespace vscMockExtHostedTypes {
}

export class WorkspaceEdit implements vscode.WorkspaceEdit {
appendNotebookCellOutput(
_uri: vscode.Uri,
_index: number,
_outputs: vscode.NotebookCellOutput[],
_metadata?: vscode.WorkspaceEditEntryMetadata,
): void {
// Noop
}
replaceNotebookCellOutputItems(
_uri: vscode.Uri,
_index: number,
_outputId: string,
_items: vscode.NotebookCellOutputItem[],
_metadata?: vscode.WorkspaceEditEntryMetadata,
): void {
// Noop
}
appendNotebookCellOutputItems(
_uri: vscode.Uri,
_index: number,
_outputId: string,
_items: vscode.NotebookCellOutputItem[],
_metadata?: vscode.WorkspaceEditEntryMetadata,
): void {
// Noop
}
replaceNotebookMetadata(_uri: vscode.Uri, _value: vscode.NotebookDocumentMetadata): void {
//
}
Expand All @@ -560,7 +586,7 @@ export namespace vscMockExtHostedTypes {
replaceNotebookCellOutput(
_uri: vscode.Uri,
_index: number,
_outputs: vscode.CellOutput[],
_outputs: vscode.NotebookCellOutput[],
_metadata?: vscode.WorkspaceEditEntryMetadata,
): void {
// Noop.
Expand Down
2 changes: 1 addition & 1 deletion src/test/smoke/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export async function enableJedi(enable: boolean | undefined): Promise<void> {
export async function openNotebook(file: string): Promise<vscode.NotebookEditor> {
await verifyExtensionIsAvailable(JUPYTER_EXTENSION_ID);
await vscode.commands.executeCommand('vscode.openWith', vscode.Uri.file(file), 'jupyter-notebook');
const notebook = vscode.notebook.activeNotebookEditor;
const notebook = vscode.window.activeNotebookEditor;
assert(notebook, 'Notebook did not open');
return notebook;
}
Expand Down
6 changes: 3 additions & 3 deletions src/test/smoke/datascience.smoke.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import * as vscode from 'vscode';
import { JUPYTER_EXTENSION_ID } from '../../client/common/constants';
import { openFile, setAutoSaveDelayInWorkspaceRoot, waitForCondition } from '../common';
import { EXTENSION_ROOT_DIR_FOR_TESTS, IS_SMOKE_TEST } from '../constants';
import { noop, sleep } from '../core';
import { sleep } from '../core';
import { closeActiveWindows, initialize, initializeTest } from '../initialize';
import { verifyExtensionIsAvailable } from './common';

Expand Down Expand Up @@ -75,8 +75,8 @@ suite('Smoke Test: Datascience', () => {
if (await fs.pathExists(outputFile)) {
await fs.unlink(outputFile);
}
// Ignore exceptions (as native editor closes the document as soon as its opened);
await openFile(file).catch(noop);

await vscode.commands.executeCommand('jupyter.opennotebook', vscode.Uri.file(file));

// Wait for 15 seconds for notebook to launch.
// Unfortunately there's no way to know for sure it has completely loaded.
Expand Down
2 changes: 1 addition & 1 deletion src/test/vscode-mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ mockedVSCode.QuickInputButtons = vscodeMocks.vscMockExtHostedTypes.QuickInputBut
mockedVSCode.FileType = vscodeMocks.vscMock.FileType;
mockedVSCode.UIKind = vscodeMocks.vscMock.UIKind;
mockedVSCode.FileSystemError = vscodeMocks.vscMockExtHostedTypes.FileSystemError;
(mockedVSCode as any).CellKind = vscodeMocks.vscMockExtHostedTypes.CellKind;
(mockedVSCode as any).NotebookCellKind = vscodeMocks.vscMockExtHostedTypes.NotebookCellKind;
(mockedVSCode as any).CellOutputKind = vscodeMocks.vscMockExtHostedTypes.CellOutputKind;
(mockedVSCode as any).NotebookCellRunState = vscodeMocks.vscMockExtHostedTypes.NotebookCellRunState;

Expand Down
Loading