Set execution_count to null when clearning notebook outputs (#231806)

* Set execution_count to null when clearning notebook outputs

* Fix tests
This commit is contained in:
Don Jayamanne
2024-10-21 15:56:16 +11:00
committed by GitHub
parent 05ff1fda3c
commit 27d99fbee7
4 changed files with 28 additions and 13 deletions

View File

@@ -62,6 +62,6 @@ export interface CellMetadata {
/**
* The code cell's prompt number. Will be null if the cell has not been run.
*/
execution_count?: number;
execution_count?: number | null;
}

View File

@@ -110,7 +110,7 @@ function trackAndUpdateCellMetadata(notebook: NotebookDocument, updates: { cell:
updates.forEach(({ cell, metadata }) => {
const newMetadata = { ...cell.metadata, ...metadata };
if (!metadata.execution_count && newMetadata.execution_count) {
delete newMetadata.execution_count;
newMetadata.execution_count = null;
}
if (!metadata.attachments && newMetadata.attachments) {
delete newMetadata.attachments;
@@ -150,8 +150,15 @@ function onDidChangeNotebookCells(e: NotebookDocumentChangeEventEx) {
metadata.execution_count = e.executionSummary.executionOrder;
metadataUpdated = true;
} else if (!e.executionSummary && !e.metadata && e.outputs?.length === 0 && currentMetadata.execution_count) {
// Clear all.
delete metadata.execution_count;
// Clear all (user hit clear all).
// NOTE: At this point we're updating the `execution_count` in metadata to `null`.
// Thus this is a change in metadata, which we will need to update in the model.
metadata.execution_count = null;
metadataUpdated = true;
} else if ((!e.executionSummary || (!e.executionSummary?.executionOrder && !e.executionSummary?.success && !e.executionSummary?.timing))
&& !e.metadata && !e.outputs && currentMetadata.execution_count) {
// This is a result of the previous cell being cleared.
metadata.execution_count = null;
metadataUpdated = true;
}

View File

@@ -52,10 +52,13 @@ export function getCellMetadata(options: { cell: NotebookCell | NotebookCellData
if ('cell' in options) {
const cell = options.cell;
const metadata = {
execution_count: null,
// it contains the cell id, and the cell metadata, along with other nb cell metadata
...(cell.metadata ?? {})
};
} satisfies CellMetadata;
if (cell.kind === NotebookCellKindMarkup) {
delete (metadata as any).execution_count;
}
return metadata;
} else {
const cell = options;
@@ -64,7 +67,7 @@ export function getCellMetadata(options: { cell: NotebookCell | NotebookCellData
...(cell.metadata ?? {})
};
return metadata;
return metadata as CellMetadata;
}
}

View File

@@ -129,7 +129,7 @@ suite(`Notebook Model Store Sync`, () => {
assert.strictEqual(editsApplied.length, 1);
assert.strictEqual(cellMetadataUpdates.length, 1);
const newMetadata = cellMetadataUpdates[0].newCellMetadata;
assert.deepStrictEqual(newMetadata, { metadata: {} });
assert.deepStrictEqual(newMetadata, { execution_count: null, metadata: {} });
});
test('Add cell id if nbformat is 4.5', async () => {
sinon.stub(notebook, 'metadata').get(() => ({ nbformat: 4, nbformat_minor: 5 }));
@@ -160,7 +160,8 @@ suite(`Notebook Model Store Sync`, () => {
assert.strictEqual(editsApplied.length, 1);
assert.strictEqual(cellMetadataUpdates.length, 1);
const newMetadata = cellMetadataUpdates[0].newCellMetadata || {};
assert.strictEqual(Object.keys(newMetadata).length, 2);
assert.strictEqual(Object.keys(newMetadata).length, 3);
assert.deepStrictEqual(newMetadata.execution_count, null);
assert.deepStrictEqual(newMetadata.metadata, {});
assert.ok(newMetadata.id);
});
@@ -195,7 +196,8 @@ suite(`Notebook Model Store Sync`, () => {
assert.strictEqual(editsApplied.length, 1);
assert.strictEqual(cellMetadataUpdates.length, 1);
const newMetadata = cellMetadataUpdates[0].newCellMetadata || {};
assert.strictEqual(Object.keys(newMetadata).length, 2);
assert.strictEqual(Object.keys(newMetadata).length, 3);
assert.deepStrictEqual(newMetadata.execution_count, null);
assert.deepStrictEqual(newMetadata.metadata, {});
assert.strictEqual(newMetadata.id, '1234');
});
@@ -276,7 +278,8 @@ suite(`Notebook Model Store Sync`, () => {
assert.strictEqual(editsApplied.length, 1);
assert.strictEqual(cellMetadataUpdates.length, 1);
const newMetadata = cellMetadataUpdates[0].newCellMetadata || {};
assert.strictEqual(Object.keys(newMetadata).length, 2);
assert.strictEqual(Object.keys(newMetadata).length, 3);
assert.deepStrictEqual(newMetadata.execution_count, null);
assert.deepStrictEqual(newMetadata.metadata, { collapsed: true, scrolled: true, vscode: { languageId: 'javascript' } });
assert.strictEqual(newMetadata.id, '1234');
});
@@ -369,7 +372,8 @@ suite(`Notebook Model Store Sync`, () => {
assert.strictEqual(editsApplied.length, 1);
assert.strictEqual(cellMetadataUpdates.length, 1);
const newMetadata = cellMetadataUpdates[0].newCellMetadata || {};
assert.strictEqual(Object.keys(newMetadata).length, 2);
assert.strictEqual(Object.keys(newMetadata).length, 3);
assert.deepStrictEqual(newMetadata.execution_count, null);
assert.deepStrictEqual(newMetadata.metadata, { collapsed: true, scrolled: true });
assert.strictEqual(newMetadata.id, '1234');
});
@@ -419,7 +423,8 @@ suite(`Notebook Model Store Sync`, () => {
assert.strictEqual(editsApplied.length, 1);
assert.strictEqual(cellMetadataUpdates.length, 1);
const newMetadata = cellMetadataUpdates[0].newCellMetadata || {};
assert.strictEqual(Object.keys(newMetadata).length, 2);
assert.strictEqual(Object.keys(newMetadata).length, 3);
assert.deepStrictEqual(newMetadata.execution_count, null);
assert.deepStrictEqual(newMetadata.metadata, { collapsed: true, scrolled: true, vscode: { languageId: 'powershell' } });
assert.strictEqual(newMetadata.id, '1234');
});