Skip to content

Commit 55cf9aa

Browse files
authored
Stable sorting of notebook cell diff info (microsoft#255005)
1 parent af0a171 commit 55cf9aa

File tree

2 files changed

+17
-3
lines changed

2 files changed

+17
-3
lines changed

src/vs/workbench/contrib/chat/browser/chatEditing/notebook/notebookCellChanges.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ export function countChanges(changes: ICellDiffInfo[]): number {
8383
}
8484

8585
export function sortCellChanges(changes: ICellDiffInfo[]): ICellDiffInfo[] {
86+
const indexes = new Map<ICellDiffInfo, number>();
87+
changes.forEach((c, i) => indexes.set(c, i));
8688
return [...changes].sort((a, b) => {
8789
// For unchanged and modified, use modifiedCellIndex
8890
if ((a.type === 'unchanged' || a.type === 'modified') &&
@@ -101,10 +103,22 @@ export function sortCellChanges(changes: ICellDiffInfo[]): ICellDiffInfo[] {
101103
}
102104

103105
if (a.type === 'delete' && b.type === 'insert') {
104-
return -1;
106+
// If the deleted cell comes before the inserted cell, we want the delete to come first
107+
// As this means the cell was deleted before it was inserted
108+
// We would like to see the deleted cell first in the list
109+
// Else in the UI it would look weird to see an inserted cell before a deleted cell,
110+
// When the users operation was to first delete the cell and then insert a new one
111+
// I.e. this is merely just a simple way to ensure we have a stable sort.
112+
return indexes.get(a)! - indexes.get(b)!;
105113
}
106114
if (a.type === 'insert' && b.type === 'delete') {
107-
return 1;
115+
// If the deleted cell comes before the inserted cell, we want the delete to come first
116+
// As this means the cell was deleted before it was inserted
117+
// We would like to see the deleted cell first in the list
118+
// Else in the UI it would look weird to see an inserted cell before a deleted cell,
119+
// When the users operation was to first delete the cell and then insert a new one
120+
// I.e. this is merely just a simple way to ensure we have a stable sort.
121+
return indexes.get(a)! - indexes.get(b)!;
108122
}
109123

110124
if ((a.type === 'delete' && b.type !== 'insert') || (a.type !== 'insert' && b.type === 'delete')) {

src/vs/workbench/contrib/chat/test/browser/chatEditingModifiedNotebookEntry.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -680,7 +680,7 @@ suite('ChatEditingModifiedNotebookEntry', function () {
680680
]);
681681
});
682682

683-
test.skip('Revert first deleted with multiple cells', async function () {
683+
test('Revert first deleted with multiple cells', async function () {
684684
const cellsDiffInfo: ICellDiffInfo[] = [
685685
{
686686
diff, keep, undo, type: 'insert', originalModel: createOriginalModel('null'), originalCellIndex: undefined,

0 commit comments

Comments
 (0)