Delete unwanted code generating/accessing metadata

src/interactive-window/editor-integration/cellFactory.ts

@@ -5,11 +5,10 @@ import { NotebookCellData, NotebookCellKind, NotebookDocument, Range, TextDocume
 import { CellMatcher } from './cellMatcher';
 import { ICellRange, IJupyterSettings } from '../../platform/common/types';
 import { noop } from '../../platform/common/utils/misc';
-import { createJupyterCellFromVSCNotebookCell } from '../../kernels/execution/helpers';
-import { appendLineFeed, parseForComments, generateMarkdownFromCodeLines } from '../../platform/common/utils';
+import { parseForComments, generateMarkdownFromCodeLines } from '../../platform/common/utils';
 import { splitLines } from '../../platform/common/helpers';
 import { isSysInfoCell } from '../systemInfoCell';
-import { getCellMetadata } from '../../platform/common/utils/jupyter';
+import { getCellMetadata } from '../../platform/common/utils';
 
 export function uncommentMagicCommands(line: string): string {
     // Uncomment lines that are shell assignments (starting with #!),
@@ -128,10 +127,7 @@ export function generateCellsFromDocument(document: TextDocument, settings?: IJu
     );
 }
 
-export function generateCellsFromNotebookDocument(
-    notebookDocument: NotebookDocument,
-    magicCommandsAsComments: boolean
-): NotebookCellData[] {
+export function generateCellsFromNotebookDocument(notebookDocument: NotebookDocument): NotebookCellData[] {
     return notebookDocument
         .getCells()
         .filter((cell) => !isSysInfoCell(cell))
@@ -141,11 +137,6 @@ export function generateCellsFromNotebookDocument(
             if (cell.metadata.interactiveWindowCellMarker !== undefined) {
                 code.unshift(cell.metadata.interactiveWindowCellMarker + '\n');
             }
-            const data = createJupyterCellFromVSCNotebookCell(cell);
-            data.source =
-                cell.kind === NotebookCellKind.Code
-                    ? appendLineFeed(code, '\n', magicCommandsAsComments ? uncommentMagicCommands : undefined)
-                    : appendLineFeed(code);
             const cellData = new NotebookCellData(
                 cell.kind,
                 code.join('\n'),

src/interactive-window/interactiveWindow.ts

@@ -584,16 +584,12 @@ export class InteractiveWindow implements IInteractiveWindow {
         if (!this.notebookDocument) {
             throw new Error('no notebook to export.');
         }
-        const { magicCommandsAsComments } = this.configuration.getSettings(this.owningResource);
-        const cells = generateCellsFromNotebookDocument(this.notebookDocument, magicCommandsAsComments);
-
-        // Should be an array of cells
-        if (cells) {
-            // Bring up the export file dialog box
-            const uri = await new ExportDialog().showDialog(ExportFormat.ipynb, this.owningResource);
-            if (uri) {
-                await this.jupyterExporter?.exportToFile(cells, getFilePath(uri));
-            }
+        const cells = generateCellsFromNotebookDocument(this.notebookDocument);
+
+        // Bring up the export file dialog box
+        const uri = await new ExportDialog().showDialog(ExportFormat.ipynb, this.owningResource);
+        if (uri) {
+            await this.jupyterExporter?.exportToFile(cells, getFilePath(uri));
         }
     }
 

src/kernels/execution/cellExecution.ts

@@ -38,7 +38,7 @@ import { isKernelSessionDead } from '../kernel';
 import { ICellExecution } from './types';
 import { KernelError } from '../errors/kernelError';
 import { getCachedSysPrefix } from '../../platform/interpreter/helpers';
-import { getCellMetadata } from '../../platform/common/utils/jupyter';
+import { getCellMetadata } from '../../platform/common/utils';
 
 /**
  * Factory for CellExecution objects.

src/kernels/execution/cellExecutionQueue.ts

@@ -4,13 +4,14 @@
 import { CancellationToken, Disposable, EventEmitter, NotebookCell } from 'vscode';
 import { traceError, traceVerbose, traceWarning } from '../../platform/logging';
 import { noop } from '../../platform/common/utils/misc';
-import { createJupyterCellFromVSCNotebookCell, traceCellMessage } from './helpers';
+import { traceCellMessage } from './helpers';
 import { CellExecutionFactory } from './cellExecution';
 import { IKernelSession, KernelConnectionMetadata, ResumeCellExecutionInformation } from '../../kernels/types';
 import { Resource } from '../../platform/common/types';
 import { ICellExecution, ICodeExecution } from './types';
 import { CodeExecution } from './codeExecution';
 import { once } from '../../platform/common/utils/events';
+import { getCellMetadata } from '../../platform/common/utils';
 
 /**
  * A queue responsible for execution of cells.
@@ -253,7 +254,7 @@ export class CellExecutionQueue implements Disposable {
             // I.e. if the cell has the tag `raises-exception`, then ignore exceptions
             if (
                 itemToExecute.type === 'cell' &&
-                createJupyterCellFromVSCNotebookCell(itemToExecute.cell).metadata?.tags?.includes('raises-exception')
+                getCellMetadata(itemToExecute.cell).metadata?.tags?.includes('raises-exception')
             ) {
                 cellErrorsAllowed = true;
             }

src/kernels/execution/helpers.ts

@@ -2,14 +2,7 @@
 // Licensed under the MIT License.
 
 import type * as nbformat from '@jupyterlab/nbformat';
-import {
-    NotebookCellOutput,
-    NotebookCellOutputItem,
-    NotebookCell,
-    NotebookCellData,
-    NotebookCellKind,
-    NotebookCellExecutionState
-} from 'vscode';
+import { NotebookCellOutput, NotebookCellOutputItem, NotebookCell, NotebookCellExecutionState } from 'vscode';
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import type { KernelMessage } from '@jupyterlab/services';
 import fastDeepEqual from 'fast-deep-equal';
@@ -39,61 +32,6 @@ export enum CellOutputMimeTypes {
     stdout = 'application/vnd.code.notebook.stdout'
 }
 
-export function createJupyterCellFromVSCNotebookCell(
-    vscCell: NotebookCell | NotebookCellData
-): nbformat.IRawCell | nbformat.IMarkdownCell | nbformat.ICodeCell {
-    let cell: nbformat.IRawCell | nbformat.IMarkdownCell | nbformat.ICodeCell;
-    if (vscCell.kind === NotebookCellKind.Markup) {
-        cell = createMarkdownCellFromNotebookCell(vscCell);
-    } else if (
-        ('document' in vscCell && vscCell.document.languageId === 'raw') ||
-        ('languageId' in vscCell && vscCell.languageId === 'raw')
-    ) {
-        cell = createRawCellFromNotebookCell(vscCell);
-    } else {
-        cell = createCodeCellFromNotebookCell(vscCell);
-    }
-    return cell;
-}
-
-function createRawCellFromNotebookCell(cell: NotebookCell | NotebookCellData): nbformat.IRawCell {
-    const cellMetadata = cell.metadata?.custom as CellMetadata | undefined;
-    const rawCell: nbformat.IRawCell = {
-        cell_type: 'raw',
-        source: splitMultilineString('document' in cell ? cell.document.getText() : cell.value),
-        metadata: cellMetadata?.metadata || {} // This cannot be empty.
-    };
-    if (cellMetadata?.attachments) {
-        rawCell.attachments = cellMetadata.attachments;
-    }
-    return rawCell;
-}
-
-function createCodeCellFromNotebookCell(cell: NotebookCell | NotebookCellData): nbformat.ICodeCell {
-    const cellMetadata = cell.metadata?.custom as CellMetadata | undefined;
-    const code = 'document' in cell ? cell.document.getText() : cell.value;
-    const codeCell: nbformat.ICodeCell = {
-        cell_type: 'code',
-        execution_count: cell.executionSummary?.executionOrder ?? null,
-        source: splitMultilineString(code),
-        outputs: (cell.outputs || []).map(translateCellDisplayOutput),
-        metadata: cellMetadata?.metadata || {} // This cannot be empty.
-    };
-    return codeCell;
-}
-
-function createMarkdownCellFromNotebookCell(cell: NotebookCell | NotebookCellData): nbformat.IMarkdownCell {
-    const cellMetadata = cell.metadata?.custom as CellMetadata | undefined;
-    const markdownCell: nbformat.IMarkdownCell = {
-        cell_type: 'markdown',
-        source: splitMultilineString('document' in cell ? cell.document.getText() : cell.value),
-        metadata: cellMetadata?.metadata || {} // This cannot be empty.
-    };
-    if (cellMetadata?.attachments) {
-        markdownCell.attachments = cellMetadata.attachments;
-    }
-    return markdownCell;
-}
 const orderOfMimeTypes = [
     'application/vnd.*',
     'application/vdom.*',
@@ -316,20 +254,6 @@ type JupyterOutput =
     | nbformat.IStream
     | nbformat.IError;
 
-/**
- * Metadata we store in VS Code cells.
- * This contains the original metadata from the Jupyuter cells.
- */
-type CellMetadata = {
-    /**
-     * Stores attachments for cells.
-     */
-    attachments?: nbformat.IAttachments;
-    /**
-     * Stores cell metadata.
-     */
-    metadata?: Partial<nbformat.ICellMetadata>;
-};
 /**
  * Metadata we store in VS Code cell output items.
  * This contains the original metadata from the Jupyuter Outputs.

src/platform/common/utils.ts

@@ -4,7 +4,17 @@
 import { SemVer, parse } from 'semver';
 import type * as nbformat from '@jupyterlab/nbformat';
 import * as uriPath from '../../platform/vscode-path/resources';
-import { NotebookData, NotebookDocument, NotebookEdit, TextDocument, Uri, WorkspaceEdit, workspace } from 'vscode';
+import {
+    NotebookData,
+    NotebookDocument,
+    NotebookEdit,
+    TextDocument,
+    Uri,
+    WorkspaceEdit,
+    workspace,
+    type NotebookCell,
+    type NotebookCellData
+} from 'vscode';
 import {
     InteractiveWindowView,
     jupyterLanguageToMonacoLanguageMapping,
@@ -190,11 +200,14 @@ export async function updateNotebookMetadata(document: NotebookDocument, metadat
 
     docMetadata.custom = docMetadata.custom || {};
     docMetadata.custom.metadata = metadata;
+
     edit.set(document.uri, [
-        NotebookEdit.updateNotebookMetadata({
-            ...(document.metadata || {}),
-            custom: docMetadata.custom
-        })
+        NotebookEdit.updateNotebookMetadata(
+            sortObjectPropertiesRecursively({
+                ...(document.metadata || {}),
+                custom: docMetadata.custom
+            })
+        )
     ]);
     await workspace.applyEdit(edit);
 }
@@ -410,3 +423,53 @@ export function parseSemVer(versionString: string): SemVer | undefined {
         return parse(`${major}.${minor}.${build}`, true) ?? undefined;
     }
 }
+
+type JupyterCellMetadata = Pick<nbformat.IRawCell, 'id' | 'metadata' | 'attachments'> &
+    Pick<nbformat.IMarkdownCell, 'id' | 'attachments'> &
+    Pick<nbformat.ICodeCell, 'id' | 'metadata' | 'attachments'> &
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    Record<string, any>;
+
+export function getCellMetadata(cell: NotebookCell | NotebookCellData): JupyterCellMetadata {
+    const metadata: JupyterCellMetadata = cell.metadata?.custom || {};
+    const cellMetadata = metadata as nbformat.IRawCell;
+    // metadata property is never optional.
+    cellMetadata.metadata = cellMetadata.metadata || {};
+
+    return metadata;
+}
+
+// function useCustomMetadata() {
+//     if (extensions.getExtension('vscode.ipynb')?.exports.dropCustomMetadata) {
+//         return false;
+//     }
+//     return true;
+// }
+
+/**
+ * Sort the JSON to minimize unnecessary SCM changes.
+ * Jupyter notbeooks/labs sorts the JSON keys in alphabetical order.
+ * https://github.com/microsoft/vscode/issues/208137
+ */
+export function sortObjectPropertiesRecursively<T>(obj: T): T {
+    return doSortObjectPropertiesRecursively(obj) as T;
+}
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+function doSortObjectPropertiesRecursively(obj: any): any {
+    if (Array.isArray(obj)) {
+        return obj.map(sortObjectPropertiesRecursively);
+    }
+    if (obj !== undefined && obj !== null && typeof obj === 'object' && Object.keys(obj).length > 0) {
+        return (
+            Object.keys(obj)
+                .sort()
+                // eslint-disable-next-line @typescript-eslint/no-explicit-any
+                .reduce<Record<string, any>>((sortedObj, prop) => {
+                    sortedObj[prop] = sortObjectPropertiesRecursively(obj[prop]);
+                    return sortedObj;
+                    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+                }, {}) as any
+        );
+    }
+    return obj;
+}

src/test/datascience/plotViewer/plotViewer.vscode.test.ts

@@ -14,7 +14,6 @@ import {
     runAllCellsInActiveNotebook,
     waitForExecutionCompletedSuccessfully
 } from '../notebook/helper.node';
-import { createJupyterCellFromVSCNotebookCell } from '../../../kernels/execution/helpers';
 import { window } from 'vscode';
 import { captureScreenShot } from '../../common';
 
@@ -69,8 +68,7 @@ plt.show()`,
         await waitForCondition(async () => plotCell?.outputs.length >= 1, 10000, 'Plot output not generated');
         // Sometimes on CI we end up with >1 output, and the test fails, but we're expecting just one.
         if (plotCell.outputs.length === 0) {
-            const jupyterCell = createJupyterCellFromVSCNotebookCell(plotCell);
-            traceInfo(`Plot cell has ${plotCell.outputs.length} outputs, Cell JSON = ${JSON.stringify(jupyterCell)}`);
+            traceInfo(`Plot cell has ${plotCell.outputs.length} outputs`);
         }
         assert.isAtLeast(plotCell.outputs.length, 1, 'Plot cell output incorrect count');