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.

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

6/2 API Changes #6089

Merged
merged 17 commits into from
Jun 3, 2021
Merged
Show file tree
Hide file tree
Changes from 16 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
78 changes: 39 additions & 39 deletions package.json

Large diffs are not rendered by default.

11 changes: 6 additions & 5 deletions src/client/common/application/notebook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import {
NotebookEditorSelectionChangeEvent,
NotebookExecuteHandler,
NotebookRendererScript,
window
window,
workspace
} from 'vscode';
import { UseVSCodeNotebookEditorApi } from '../constants';
import { IDisposableRegistry } from '../types';
Expand All @@ -31,7 +32,7 @@ export class VSCodeNotebook implements IVSCodeNotebook {
public readonly onDidSaveNotebookDocument: Event<NotebookDocument>;
public readonly onDidChangeNotebookDocument: Event<NotebookCellChangedEvent>;
public get notebookDocuments(): ReadonlyArray<NotebookDocument> {
return this.canUseNotebookApi ? notebooks.notebookDocuments : [];
return this.canUseNotebookApi ? workspace.notebookDocuments : [];
}
public get notebookEditors() {
return this.canUseNotebookApi ? window.visibleNotebookEditors : [];
Expand Down Expand Up @@ -60,8 +61,8 @@ export class VSCodeNotebook implements IVSCodeNotebook {
this.canUseNotebookApi = true;
this.onDidChangeNotebookEditorSelection = window.onDidChangeNotebookEditorSelection;
this.onDidChangeActiveNotebookEditor = window.onDidChangeActiveNotebookEditor;
this.onDidOpenNotebookDocument = notebooks.onDidOpenNotebookDocument;
this.onDidCloseNotebookDocument = notebooks.onDidCloseNotebookDocument;
this.onDidOpenNotebookDocument = workspace.onDidOpenNotebookDocument;
this.onDidCloseNotebookDocument = workspace.onDidCloseNotebookDocument;
this.onDidChangeVisibleNotebookEditors = window.onDidChangeVisibleNotebookEditors;
this.onDidSaveNotebookDocument = notebooks.onDidSaveNotebookDocument;
this.onDidChangeNotebookDocument = this._onDidChangeNotebookDocument.event;
Expand All @@ -86,7 +87,7 @@ export class VSCodeNotebook implements IVSCodeNotebook {
transientDocumentMetadata?: { [x: string]: boolean | undefined } | undefined;
}
): Disposable {
return notebooks.registerNotebookContentProvider(notebookType, provider, options);
return workspace.registerNotebookContentProvider(notebookType, provider, options);
}
public createNotebookController(
id: string,
Expand Down
30 changes: 17 additions & 13 deletions src/client/datascience/jupyter/kernels/cellExecution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,8 @@ export class CellExecution {
`Cell Exec contents ${this.cell.document.getText().substring(0, 50)}...`
);
if (!this.canExecuteCell()) {
this.execution?.end({});
// End state is bool | undefined not optional. Undefined == not success or failure
Copy link
Member Author

Choose a reason for hiding this comment

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

The new tristate here for success is a bit funky so I called it out with a comment.

this.execution?.end(undefined);
this.execution = undefined;
return;
}
Expand All @@ -199,7 +200,7 @@ export class CellExecution {

this.startTime = new Date().getTime();
CellExecution.activeNotebookCellExecution.set(this.cell.notebook, this.execution);
this.execution?.start({ startTime: this.startTime });
this.execution?.start(this.startTime);
await Promise.all([this.initPromise, this.execution?.clearOutput()]);
this.stopWatch.reset();

Expand Down Expand Up @@ -289,13 +290,15 @@ export class CellExecution {
}
private endCellTask(success: 'success' | 'failed' | 'cancelled') {
if (this.isEmptyCodeCell) {
this.execution?.end({});
// Undefined for not success or failures
this.execution?.end(undefined);
} else if (success === 'success' || success === 'failed') {
this.endTime = new Date().getTime();
this.execution?.end({ endTime: this.endTime, success: success === 'success' });
this.execution?.end(success === 'success', this.endTime);
} else {
// Cell was cancelled.
this.execution?.end({});
// Undefined for not success or failures
this.execution?.end(undefined);
}
if (CellExecution.activeNotebookCellExecution.get(this.cell.notebook) === this.execution) {
CellExecution.activeNotebookCellExecution.set(this.cell.notebook, undefined);
Expand Down Expand Up @@ -323,7 +326,7 @@ export class CellExecution {
// Create a temporary task.
this.previousResultsToRestore = { ...(this.cell.executionSummary || {}) };
this.temporaryExecution = this.controller.createNotebookCellExecution(this.cell);
this.temporaryExecution?.start({});
this.temporaryExecution?.start();
if (this.previousResultsToRestore?.executionOrder && this.execution) {
this.execution.executionOrder = this.previousResultsToRestore.executionOrder;
}
Expand All @@ -337,12 +340,13 @@ export class CellExecution {
if (this.previousResultsToRestore.executionOrder) {
this.temporaryExecution.executionOrder = this.previousResultsToRestore.executionOrder;
}
this.temporaryExecution.end({
endTime: this.previousResultsToRestore.endTime,
success: this.previousResultsToRestore.success
});
this.temporaryExecution.end(
this.previousResultsToRestore.success,
this.previousResultsToRestore.timing?.endTime
);
} else {
this.temporaryExecution?.end({});
// Undefined for not success or failure
this.temporaryExecution?.end(undefined);
}
this.previousResultsToRestore = undefined;
this.temporaryExecution = undefined;
Expand Down Expand Up @@ -719,7 +723,7 @@ export class CellExecution {
name: msg.content.name,
text: formatStreamText(concatMultilineString(`${existingOutputText}${newContent}`))
});
promise = task?.replaceOutputItems(output.items, existingOutputToAppendTo.id);
promise = task?.replaceOutputItems(output.items, existingOutputToAppendTo);
} else if (clearOutput) {
// Replace the current outputs with a single new output.
const output = cellOutputToVSCCellOutput({
Expand Down Expand Up @@ -857,7 +861,7 @@ export class CellExecution {
// This message could have come from a background thread.
// In such circumstances, create a temporary task & use that to update the output (only cell execution tasks can update cell output).
const task = this.execution || this.createTemporaryTask();
const promise = task?.replaceOutputItems(newOutput.items, outputToBeUpdated.id);
const promise = task?.replaceOutputItems(newOutput.items, outputToBeUpdated);
this.endTemporaryTask();
// await on the promise at the end, we want to minimize UI flickers.
// The way we update output of other cells is to use an existing task or a temporary task.
Expand Down
4 changes: 2 additions & 2 deletions src/client/datascience/jupyter/kernels/kernelExecution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

'use strict';

import { notebooks, NotebookCell, NotebookCellKind, NotebookController, NotebookDocument } from 'vscode';
import { NotebookCell, NotebookCellKind, NotebookController, NotebookDocument, workspace } from 'vscode';
import { ServerStatus } from '../../../../datascience-ui/interactive-common/mainState';
import { IApplicationShell } from '../../../common/application/types';
import { traceInfo, traceWarning } from '../../../common/logger';
Expand Down Expand Up @@ -133,7 +133,7 @@ export class KernelExecution implements IDisposable {
);

// If the document is closed (user or on CI), then just stop handling the UI updates & cancel cell execution queue.
notebooks.onDidCloseNotebookDocument(
workspace.onDidCloseNotebookDocument(
async (e: NotebookDocument) => {
if (e === document) {
if (!newCellExecutionQueue.failed || !newCellExecutionQueue.isEmpty) {
Expand Down
17 changes: 8 additions & 9 deletions src/client/datascience/notebook/contentProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,15 @@ export class NotebookContentProvider implements VSCNotebookContentProvider {
if (!this.compatibilitySupport.canOpenWithVSCodeNotebookEditor(uri)) {
// If not supported, return a notebook with error displayed.
// We cannot, not display a notebook.
const cellData = new NotebookCellData(
Copy link
Member Author

Choose a reason for hiding this comment

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

It's funny I thought that typescript had some syntactic sugar for this, like a C# field initializer syntax, but didn't see it.

NotebookCellKind.Markup,
`# ${DataScience.usingPreviewNotebookWithOtherNotebookWarning()}`,
MARKDOWN_LANGUAGE
);
cellData.outputs = [];
cellData.metadata = {};
return {
cells: [
new NotebookCellData(
NotebookCellKind.Markup,
`# ${DataScience.usingPreviewNotebookWithOtherNotebookWarning()}`,
MARKDOWN_LANGUAGE,
[],
{}
)
],
cells: [cellData],
metadata: {}
};
}
Expand Down
7 changes: 4 additions & 3 deletions src/client/datascience/notebook/helpers/executionHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,9 @@ export async function updateCellCode(cell: NotebookCell, text: string) {
export async function addNewCellAfter(cell: NotebookCell, text: string) {
await chainWithPendingUpdates(cell.notebook, (edit) => {
traceCellMessage(cell, 'Create new cell after current');
edit.replaceNotebookCells(cell.notebook.uri, new NotebookRange(cell.index + 1, cell.index + 1), [
new NotebookCellData(NotebookCellKind.Code, text, cell.document.languageId, [], cell.metadata || {})
]);
const cellData = new NotebookCellData(NotebookCellKind.Code, text, cell.document.languageId);
cellData.outputs = [];
cellData.metadata = cell.metadata || {};
IanMatthewHuff marked this conversation as resolved.
Show resolved Hide resolved
edit.replaceNotebookCells(cell.notebook.uri, new NotebookRange(cell.index + 1, cell.index + 1), [cellData]);
});
}
33 changes: 19 additions & 14 deletions src/client/datascience/notebook/helpers/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -304,9 +304,10 @@ function createCodeCellFromNotebookCell(cell: NotebookCell): nbformat.ICodeCell
}

function createNotebookCellDataFromRawCell(cell: nbformat.IRawCell): NotebookCellData {
return new NotebookCellData(NotebookCellKind.Code, concatMultilineString(cell.source), 'raw', [], {
custom: getNotebookCellMetadata(cell)
});
const cellData = new NotebookCellData(NotebookCellKind.Code, concatMultilineString(cell.source), 'raw');
cellData.outputs = [];
cellData.metadata = { custom: getNotebookCellMetadata(cell) };
return cellData;
}
function createMarkdownCellFromNotebookCell(cell: NotebookCell): nbformat.IMarkdownCell {
const cellMetadata = cell.metadata.custom as CellMetadata | undefined;
Expand All @@ -321,9 +322,14 @@ function createMarkdownCellFromNotebookCell(cell: NotebookCell): nbformat.IMarkd
return markdownCell;
}
function createNotebookCellDataFromMarkdownCell(cell: nbformat.IMarkdownCell): NotebookCellData {
return new NotebookCellData(NotebookCellKind.Markup, concatMultilineString(cell.source), MARKDOWN_LANGUAGE, [], {
custom: getNotebookCellMetadata(cell)
});
const cellData = new NotebookCellData(
NotebookCellKind.Markup,
concatMultilineString(cell.source),
MARKDOWN_LANGUAGE
);
cellData.outputs = [];
cellData.metadata = { custom: getNotebookCellMetadata(cell) };
return cellData;
}
function createNotebookCellDataFromCodeCell(cell: nbformat.ICodeCell, cellLanguage: string): NotebookCellData {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand All @@ -336,14 +342,13 @@ function createNotebookCellDataFromCodeCell(cell: nbformat.ICodeCell, cellLangua
const executionSummary: NotebookCellExecutionSummary = hasExecutionCount
? { executionOrder: cell.execution_count as number }
: {};
return new NotebookCellData(
NotebookCellKind.Code,
source,
cellLanguage,
outputs,
{ custom: getNotebookCellMetadata(cell) },
executionSummary
);

const cellData = new NotebookCellData(NotebookCellKind.Code, source, cellLanguage);

cellData.outputs = outputs;
cellData.metadata = { custom: getNotebookCellMetadata(cell) };
cellData.executionSummary = executionSummary;
return cellData;
}
const orderOfMimeTypes = [
'application/vnd.*',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ export class NoKernelsNotebookController implements Disposable {
traceback: errorMessage.split('\n')
});
task.appendOutput(errorOutput).then(noop, noop);
task.end();
task.end(undefined);
this.errorHandler.handleError(new KernelSpecNotFoundError(getNotebookMetadata(notebook))).catch(noop);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export class VSCodeNotebookController implements Disposable {
this.controller.interruptHandler = this.handleInterrupt.bind(this);
this.controller.description = getDescriptionOfKernelConnection(kernelConnection);
this.controller.detail = getDetailOfKernelConnection(kernelConnection, this.pathUtils);
this.controller.hasExecutionOrder = true;
this.controller.supportsExecutionOrder = true;
this.controller.supportedLanguages = this.languageService.getSupportedLanguages(kernelConnection);
// Hook up to see when this NotebookController is selected by the UI
this.controller.onDidChangeNotebookAssociation(this.onDidChangeNotebookAssociation, this, this.disposables);
Expand Down
2 changes: 1 addition & 1 deletion src/client/datascience/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export function translateCellToNative(
executionSummary: {
executionOrder: cell.data.execution_count as number,
success: true,
endTime: 0
timing: { startTime: 0, endTime: 0 }
},
outputs: [],
kind: NotebookCellKind.Code,
Expand Down
84 changes: 42 additions & 42 deletions src/test/datascience/notebook/contentProviderMain.vscode.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,27 +69,25 @@ suite('DataScience - VSCode Notebook ContentProvider', () => {

assert.isOk(notebook);

assert.deepEqual(notebook.cells, [
new NotebookCellData(
NotebookCellKind.Code,
'print(1)',
PYTHON_LANGUAGE,
[],
{
custom: {
metadata: {}
}
},
{
executionOrder: 10
}
),
new NotebookCellData(NotebookCellKind.Markup, '# HEAD', MARKDOWN_LANGUAGE, [], {
custom: {
metadata: {}
}
})
]);
const codeCellData = new NotebookCellData(NotebookCellKind.Code, 'print(1)', PYTHON_LANGUAGE);

codeCellData.outputs = [];
codeCellData.metadata = {
custom: {
metadata: {}
}
};
codeCellData.executionSummary = { executionOrder: 10 };

const markdownCellData = new NotebookCellData(NotebookCellKind.Markup, '# HEAD', MARKDOWN_LANGUAGE);
markdownCellData.outputs = [];
markdownCellData.metadata = {
custom: {
metadata: {}
}
};

assert.deepEqual(notebook.cells, [codeCellData, markdownCellData]);
});

test('Return notebook with csharp language', async () => {
Expand Down Expand Up @@ -121,27 +119,29 @@ suite('DataScience - VSCode Notebook ContentProvider', () => {

assert.isOk(notebook);

assert.deepEqual(notebook.cells, [
new NotebookCellData(
NotebookCellKind.Code,
'Console.WriteLine("1")',
'csharp',
[],
{
custom: {
metadata: {}
}
},
{
executionOrder: 10
}
),
new NotebookCellData(NotebookCellKind.Markup, '# HEAD', MARKDOWN_LANGUAGE, [], {
custom: {
metadata: {}
}
})
]);
const codeCellData = new NotebookCellData(NotebookCellKind.Code, 'Console.WriteLine("1")', 'csharp');

codeCellData.outputs = [];
codeCellData.metadata = {
custom: {
metadata: {}
}
};

codeCellData.executionSummary = {
executionOrder: 10
};

const markdownCellData = new NotebookCellData(NotebookCellKind.Markup, '# HEAD', MARKDOWN_LANGUAGE);

markdownCellData.outputs = [];
markdownCellData.metadata = {
custom: {
metadata: {}
}
};

assert.deepEqual(notebook.cells, [codeCellData, markdownCellData]);
});
test('Verify mime types and order', () => {
// https://github.com/microsoft/vscode-python/issues/11880
Expand Down
18 changes: 10 additions & 8 deletions src/test/datascience/notebook/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,12 @@ export async function insertMarkdownCell(source: string, options?: { index?: num
throw new Error('No active editor');
}
const startNumber = options?.index ?? activeEditor.document.cellCount;
await chainWithPendingUpdates(activeEditor.document, (edit) =>
edit.replaceNotebookCells(activeEditor.document.uri, new NotebookRange(startNumber, startNumber), [
new NotebookCellData(NotebookCellKind.Markup, source, MARKDOWN_LANGUAGE, [], {})
])
);
await chainWithPendingUpdates(activeEditor.document, (edit) => {
const cellData = new NotebookCellData(NotebookCellKind.Markup, source, MARKDOWN_LANGUAGE);
cellData.outputs = [];
cellData.metadata = {};
edit.replaceNotebookCells(activeEditor.document.uri, new NotebookRange(startNumber, startNumber), [cellData]);
});
return activeEditor.document.cellAt(startNumber)!;
}
export async function insertCodeCell(source: string, options?: { language?: string; index?: number }) {
Expand All @@ -102,9 +103,10 @@ export async function insertCodeCell(source: string, options?: { language?: stri
}
const startNumber = options?.index ?? activeEditor.document.cellCount;
const edit = new WorkspaceEdit();
edit.replaceNotebookCells(activeEditor.document.uri, new NotebookRange(startNumber, startNumber), [
new NotebookCellData(NotebookCellKind.Code, source, options?.language || PYTHON_LANGUAGE, [], {})
]);
const cellData = new NotebookCellData(NotebookCellKind.Code, source, options?.language || PYTHON_LANGUAGE);
cellData.outputs = [];
cellData.metadata = {};
edit.replaceNotebookCells(activeEditor.document.uri, new NotebookRange(startNumber, startNumber), [cellData]);
await workspace.applyEdit(edit);

return activeEditor.document.cellAt(startNumber)!;
Expand Down
Loading