Inconsistency with NotebookEdit.updateCellMetadata and updateNotebookMetadata

[Yoyokrazy]

When updating cell metadata for a cell, users call NotebookEdit.updateCellMetadata which provides a function to alter the metadata of a cell. This uses replaceNotebookCellMetadata defined in extHostTypes.ts.

vscode/src/vscode-dts/vscode.d.ts

Lines 3536 to 3542 in 56f11d9

	/**
	* Utility to create an edit that update a cell's metadata.
	*
	* @param index The index of the cell to update.
	* @param newCellMetadata The new metadata for the cell.
	*/
	static updateCellMetadata(index: number, newCellMetadata: { [key: string]: any }): NotebookEdit;

vscode/src/vs/workbench/api/common/extHostTypes.ts

Lines 834 to 836 in 56f11d9

	private replaceNotebookCellMetadata(uri: URI, index: number, cellMetadata: Record<string, any>, metadata?: vscode.WorkspaceEditEntryMetadata): void {
	this._edits.push({ _type: FileEditType.Cell, metadata, uri, edit: { editType: CellEditType.PartialMetadata, index, metadata: cellMetadata } });
	}

When replaceNotebookCellMetadata is called, the CellEditType is marked as PartialMetadata, resulting in the metadata of the cell only being updated with the passed in data, rather than fully replaced. This makes it difficult to determine how to remove fields from a cell's metadata, a change needed for fixing vscode/jupyter#13522. This can be worked around in this situation by setting the field to null.

However, if a user calls NotebookEdit.updateNotebookMetadata, it appears that the functionality is a full replacement of initial metadata. There is a slight inconsistency here between the functionality of the two functions.

Couple initial ideas:

• better document vscode.d.ts to reflect the behavior of updateCellMetadata. Add in a line stating that this is an iterative change, only altering the fields passed into this function
  • * @param newCellMetadata The new metadata for the cell.
  • * @param newCellMetadata The **revised** metadata for the cell.
• revise the functionality of either updateCell or updateNotebook metadata functions to align their behavior. This seems dangerous as it could likely break features leveraging this api.
• deprecate these functions, and rewrite two new ones with clearer functionality and documentation, based on updated needs from the notebook and jupyter teams

[rebornix]

Analyzed all extensions using this API and most of them are already doing the right thing: clone the existing metadata, apply the change, and then call the API to update. However there are a few extensions didn't do so:

• donjayamanne.kusto
  • Fix updateCellMetadata usage. notebookPowerTools/vscode-kusto#79
• ms-dotnettools.dotnet-interactive-vscode/
  • there is one place where the metadata is not updated properly: https://github.com/dotnet/interactive/blob/b0a9d3b1600f3bedcf60c4da8c91b41621ca0988/src/polyglot-notebooks-vscode-common/src/vscodeUtilities.ts#L93
  • PR Fix updateCellMetadata: keep existing metadata on update. dotnet/interactive#3034
• jupyterpowertools. cc @DonJayamanne

[vscodenpa]

This bug has been fixed in the latest release of VS Code Insiders!

@Yoyokrazy, you can help us out by commenting /verified if things are now working as expected.

If things still don't seem right, please ensure you're on version b4952d1 of Insiders (today's or later - you can use Help: About in the command palette to check), and leave a comment letting us know what isn't working as expected.

Happy Coding!

[alexr00]

@rebornix what is the best way to verify this?

[rebornix]

It's a bit complex I would love either @Yoyokrazy or @amunger to help verify.