From 08c8f1a330274cb42964a8fb5813a2e593965b82 Mon Sep 17 00:00:00 2001
From: Don Jayamanne <don.jayamanne@outlook.com>
Date: Mon, 6 Jan 2025 14:58:08 +1100
Subject: [PATCH] Sort cell metadata for notebook cell diffview (#237302)

---
 .../notebook/browser/diff/diffComponents.ts   |  4 +--
 .../notebook/browser/notebook.contribution.ts |  6 ++---
 .../common/model/notebookCellTextModel.ts     | 25 +++++++++++++++++--
 3 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts b/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts
index 3ef8a33b088..a0a5557ddab 100644
--- a/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts
+++ b/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts
@@ -847,7 +847,7 @@ abstract class AbstractElementRenderer extends Disposable {
 					return;
 				}
 
-				const modifiedMetadataSource = getFormattedMetadataJSON(this.notebookEditor.textModel?.transientOptions.transientCellMetadata, this.cell.modified?.metadata || {}, this.cell.modified?.language);
+				const modifiedMetadataSource = getFormattedMetadataJSON(this.notebookEditor.textModel?.transientOptions.transientCellMetadata, this.cell.modified?.metadata || {}, this.cell.modified?.language, true);
 				modifiedMetadataModel.object.textEditorModel.setValue(modifiedMetadataSource);
 			}));
 
@@ -869,7 +869,7 @@ abstract class AbstractElementRenderer extends Disposable {
 			const originalMetadataSource = getFormattedMetadataJSON(this.notebookEditor.textModel?.transientOptions.transientCellMetadata,
 				this.cell.type === 'insert'
 					? this.cell.modified!.metadata || {}
-					: this.cell.original!.metadata || {});
+					: this.cell.original!.metadata || {}, undefined, true);
 			const uri = this.cell.type === 'insert'
 				? this.cell.modified!.uri
 				: this.cell.original!.uri;
diff --git a/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts b/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts
index a09069d24cf..17568757a13 100644
--- a/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts
+++ b/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts
@@ -452,7 +452,7 @@ class CellInfoContentProvider {
 		for (const cell of ref.object.notebook.cells) {
 			if (cell.handle === data.handle) {
 				const cellIndex = ref.object.notebook.cells.indexOf(cell);
-				const metadataSource = getFormattedMetadataJSON(ref.object.notebook.transientOptions.transientCellMetadata, cell.metadata, cell.language);
+				const metadataSource = getFormattedMetadataJSON(ref.object.notebook.transientOptions.transientCellMetadata, cell.metadata, cell.language, true);
 				result = this._modelService.createModel(
 					metadataSource,
 					mode,
@@ -460,9 +460,9 @@ class CellInfoContentProvider {
 				);
 				this._disposables.push(disposables.add(ref.object.notebook.onDidChangeContent(e => {
 					if (result && e.rawEvents.some(event => (event.kind === NotebookCellsChangeType.ChangeCellMetadata || event.kind === NotebookCellsChangeType.ChangeCellLanguage) && event.index === cellIndex)) {
-						const value = getFormattedMetadataJSON(ref.object.notebook.transientOptions.transientCellMetadata, cell.metadata, cell.language);
+						const value = getFormattedMetadataJSON(ref.object.notebook.transientOptions.transientCellMetadata, cell.metadata, cell.language, true);
 						if (result.getValue() !== value) {
-							result.setValue(getFormattedMetadataJSON(ref.object.notebook.transientOptions.transientCellMetadata, cell.metadata, cell.language));
+							result.setValue(value);
 						}
 					}
 				})));
diff --git a/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts b/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts
index 018aa490c9a..ac481402ced 100644
--- a/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts
+++ b/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts
@@ -516,7 +516,7 @@ function computeRunStartTimeAdjustment(oldMetadata: NotebookCellInternalMetadata
 }
 
 
-export function getFormattedMetadataJSON(transientCellMetadata: TransientCellMetadata | undefined, metadata: NotebookCellMetadata, language?: string) {
+export function getFormattedMetadataJSON(transientCellMetadata: TransientCellMetadata | undefined, metadata: NotebookCellMetadata, language?: string, sortKeys?: boolean): string {
 	let filteredMetadata: { [key: string]: any } = {};
 
 	if (transientCellMetadata) {
@@ -541,7 +541,28 @@ export function getFormattedMetadataJSON(transientCellMetadata: TransientCellMet
 	if (language) {
 		obj.language = language;
 	}
-	const metadataSource = toFormattedString(obj, {});
+	const metadataSource = toFormattedString(sortKeys ? sortObjectPropertiesRecursively(obj) : obj, {});
 
 	return metadataSource;
 }
+
+
+/**
+ * Sort the JSON to ensure when diffing, the JSON keys are sorted & matched correctly in diff view.
+ */
+export function sortObjectPropertiesRecursively(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()
+				.reduce<Record<string, any>>((sortedObj, prop) => {
+					sortedObj[prop] = sortObjectPropertiesRecursively(obj[prop]);
+					return sortedObj;
+				}, {}) as any
+		);
+	}
+	return obj;
+}