move dirty state to NotebookEditorModel.

src/vs/workbench/api/browser/mainThreadNotebook.ts

@@ -21,6 +21,7 @@ import { INotebookCellStatusBarService } from 'vs/workbench/contrib/notebook/com
 import { ACCESSIBLE_NOTEBOOK_DISPLAY_ORDER, CellEditType, CellKind, DisplayOrderKey, ICellEditOperation, ICellRange, IEditor, IMainCellDto, INotebookDocumentFilter, NotebookCellMetadata, NotebookCellOutputsSplice, NotebookCellsChangeType, NotebookDocumentMetadata, NOTEBOOK_DISPLAY_ORDER, TransientMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon';
 import { IMainNotebookController, INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService';
 import { IEditorService } from 'vs/workbench/services/editor/common/editorService';
+import { IWorkingCopyService } from 'vs/workbench/services/workingCopy/common/workingCopyService';
 import { ExtHostContext, ExtHostNotebookShape, IExtHostContext, INotebookCellStatusBarEntryDto, INotebookDocumentsAndEditorsDelta, INotebookModelAddedData, MainContext, MainThreadNotebookShape, NotebookEditorRevealType, NotebookExtensionDescription } from '../common/extHost.protocol';
 
 class DocumentAndEditorState {
@@ -147,7 +148,8 @@ export class MainThreadNotebooks extends Disposable implements MainThreadNoteboo
 		@IEditorService private readonly editorService: IEditorService,
 		@IAccessibilityService private readonly accessibilityService: IAccessibilityService,
 		@ILogService private readonly logService: ILogService,
-		@INotebookCellStatusBarService private readonly cellStatusBarService: INotebookCellStatusBarService
+		@INotebookCellStatusBarService private readonly cellStatusBarService: INotebookCellStatusBarService,
+		@IWorkingCopyService private readonly _workingCopyService: IWorkingCopyService,
 	) {
 		super();
 		this._proxy = extHostContext.getProxy(ExtHostContext.ExtHostNotebook);
@@ -295,7 +297,15 @@ export class MainThreadNotebooks extends Disposable implements MainThreadNoteboo
 									}
 									: e
 							);
-					this._proxy.$acceptModelChanged(textModel.uri, data, textModel.isDirty);
+
+					/**
+					 * TODO@rebornix, @jrieken
+					 * When a document is modified, it will trigger onDidChangeContent events.
+					 * The first event listener is this one, which doesn't know if the text model is dirty or not. It can ask `workingCopyService` but get the wrong result
+					 * The second event listener is `NotebookEditorModel`, which will then set `isDirty` to `true`.
+					 * Since `e.transient` decides if the model should be dirty or not, we will use the same logic here.
+					 */
+					this._proxy.$acceptModelChanged(textModel.uri, data, !e.transient);
 					this._proxy.$acceptDocumentPropertiesChanged(textModel.uri, { metadata: null });
 				}));
 
@@ -611,9 +621,11 @@ export class MainThreadNotebooks extends Disposable implements MainThreadNoteboo
 
 		if (textModel) {
 			textModel.handleUnknownEdit(label, () => {
-				return this._proxy.$undoNotebook(textModel.viewType, textModel.uri, editId, textModel.isDirty);
+				const isDirty = this._workingCopyService.isDirty(textModel.uri.with({ scheme: Schemas.vscodeNotebook }));
+				return this._proxy.$undoNotebook(textModel.viewType, textModel.uri, editId, isDirty);
 			}, () => {
-				return this._proxy.$redoNotebook(textModel.viewType, textModel.uri, editId, textModel.isDirty);
+				const isDirty = this._workingCopyService.isDirty(textModel.uri.with({ scheme: Schemas.vscodeNotebook }));
+				return this._proxy.$redoNotebook(textModel.viewType, textModel.uri, editId, isDirty);
 			});
 		}
 	}

src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts

@@ -112,15 +112,10 @@ class DelayedEmitter {
 	}
 
 	emit(data: {
-		triggerDirty: { value: boolean } | undefined,
 		contentChange: { value: NotebookTextModelChangedEvent } | undefined,
 	}) {
 		this._increaseVersion();
 
-		if (data.triggerDirty) {
-			this._textModel.setDirty(data.triggerDirty.value);
-		}
-
 		if (data.contentChange) {
 			this._onDidChangeContent.fire(
 				{
@@ -163,10 +158,6 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
 	private _operationManager: NotebookOperationManager;
 	private _eventEmitter: DelayedEmitter;
 
-	private _dirty = false;
-	protected readonly _onDidChangeDirty = this._register(new Emitter<void>());
-	readonly onDidChangeDirty = this._onDidChangeDirty.event;
-
 	constructor(
 		readonly viewType: string,
 		readonly supportBackup: boolean,
@@ -201,17 +192,6 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
 		return this._versionId;
 	}
 
-	get isDirty() {
-		return this._dirty;
-	}
-
-	setDirty(newState: boolean) {
-		if (this._dirty !== newState) {
-			this._dirty = newState;
-			this._onDidChangeDirty.fire();
-		}
-	}
-
 	createCellTextModel(
 		source: string,
 		language: string,
@@ -238,9 +218,8 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
 			this._mapping.set(mainCells[i].handle, mainCells[i]);
 			const dirtyStateListener = mainCells[i].onDidChangeContent(() => {
 				this._eventEmitter.emit({
-					triggerDirty: { value: true },
 					contentChange: {
-						value: { kind: NotebookCellsChangeType.ChangeCellContent, versionId: this.versionId, synchronous: true }
+						value: { kind: NotebookCellsChangeType.ChangeCellContent, versionId: this.versionId, synchronous: true, transient: false }
 					}
 				});
 			});
@@ -334,8 +313,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
 			);
 			const dirtyStateListener = cell.onDidChangeContent(() => {
 				this._eventEmitter.emit({
-					triggerDirty: { value: true },
-					contentChange: { value: { kind: NotebookCellsChangeType.ChangeCellContent, versionId: this.versionId, synchronous: true } }
+					contentChange: { value: { kind: NotebookCellsChangeType.ChangeCellContent, versionId: this.versionId, synchronous: true, transient: false } }
 				});
 			});
 			this._cellListeners.set(cell.handle, dirtyStateListener);
@@ -364,13 +342,13 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
 
 		// should be deferred
 		this._eventEmitter.emit({
-			triggerDirty: { value: true },
 			contentChange: {
 				value: {
 					kind: NotebookCellsChangeType.ModelChange,
 					versionId: this._versionId,
 					changes: diffs,
-					synchronous
+					synchronous,
+					transient: false
 				}
 			}
 		});
@@ -388,11 +366,30 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
 				redo();
 			},
 		});
-		this.setDirty(true);
+
+		this._eventEmitter.emit({
+			contentChange: {
+				value: {
+					kind: NotebookCellsChangeType.Unknown,
+					transient: false,
+					synchronous: true,
+					versionId: this._versionId,
+				}
+			}
+		});
 	}
 
 	handleUnknownChange() {
-		this.setDirty(true);
+		this._eventEmitter.emit({
+			contentChange: {
+				value: {
+					kind: NotebookCellsChangeType.Unknown,
+					transient: false,
+					synchronous: true,
+					versionId: this._versionId,
+				}
+			}
+		});
 	}
 
 	createSnapshot(preserveBOM?: boolean): ITextSnapshot {
@@ -417,8 +414,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
 	updateNotebookMetadata(metadata: NotebookDocumentMetadata) {
 		this.metadata = metadata;
 		this._eventEmitter.emit({
-			triggerDirty: { value: true },
-			contentChange: { value: { kind: NotebookCellsChangeType.ChangeDocumentMetadata, versionId: this.versionId, metadata: this.metadata, synchronous: true } }
+			contentChange: { value: { kind: NotebookCellsChangeType.ChangeDocumentMetadata, versionId: this.versionId, metadata: this.metadata, synchronous: true, transient: false } }
 		});
 	}
 
@@ -427,8 +423,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
 			this._mapping.set(cells[i].handle, cells[i]);
 			const dirtyStateListener = cells[i].onDidChangeContent(() => {
 				this._eventEmitter.emit({
-					triggerDirty: { value: true },
-					contentChange: { value: { kind: NotebookCellsChangeType.ChangeCellContent, versionId: this.versionId, synchronous: true } }
+					contentChange: { value: { kind: NotebookCellsChangeType.ChangeCellContent, versionId: this.versionId, synchronous: true, transient: false } }
 				});
 			});
 
@@ -437,7 +432,6 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
 
 		this._cells.splice(index, 0, ...cells);
 		this._eventEmitter.emit({
-			triggerDirty: { value: true },
 			contentChange: {
 				value: {
 					kind: NotebookCellsChangeType.ModelChange,
@@ -448,7 +442,8 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
 							cells
 						]],
 					synchronous,
-					endSelections: endSelections
+					endSelections: endSelections,
+					transient: false
 				}
 			}
 		});
@@ -464,8 +459,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
 		}
 		this._cells.splice(index, count);
 		this._eventEmitter.emit({
-			triggerDirty: { value: true },
-			contentChange: { value: { kind: NotebookCellsChangeType.ModelChange, versionId: this._versionId, changes: [[index, count, []]], synchronous, endSelections } }
+			contentChange: { value: { kind: NotebookCellsChangeType.ModelChange, versionId: this._versionId, changes: [[index, count, []]], synchronous, endSelections, transient: false } }
 		});
 	}
 
@@ -513,9 +507,8 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
 		}
 
 		this._eventEmitter.emit({
-			triggerDirty: { value: triggerDirtyChange },
 			contentChange: {
-				value: { kind: NotebookCellsChangeType.ChangeCellMetadata, versionId: this._versionId, index: this._cells.indexOf(cell), metadata: cell.metadata, synchronous: true }
+				value: { kind: NotebookCellsChangeType.ChangeCellMetadata, versionId: this._versionId, index: this._cells.indexOf(cell), metadata: cell.metadata, synchronous: true, transient: !triggerDirtyChange }
 			}
 		});
 	}
@@ -526,8 +519,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
 			cell.language = languageId;
 
 			this._eventEmitter.emit({
-				triggerDirty: { value: true },
-				contentChange: { value: { kind: NotebookCellsChangeType.ChangeLanguage, versionId: this._versionId, index: this._cells.indexOf(cell), language: languageId, synchronous: true } }
+				contentChange: { value: { kind: NotebookCellsChangeType.ChangeLanguage, versionId: this._versionId, index: this._cells.indexOf(cell), language: languageId, synchronous: true, transient: false } }
 			});
 		}
 	}
@@ -539,7 +531,6 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
 			cell.spliceNotebookCellOutputs(splices);
 
 			this._eventEmitter.emit({
-				triggerDirty: { value: !this.transientOptions.transientOutputs },
 				contentChange: {
 					value: {
 						kind: NotebookCellsChangeType.Output,
@@ -602,8 +593,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
 			const cells = this._cells.splice(index, length);
 			this._cells.splice(newIdx, 0, ...cells);
 			this._eventEmitter.emit({
-				triggerDirty: { value: true },
-				contentChange: { value: { kind: NotebookCellsChangeType.Move, versionId: this._versionId, index, length, newIdx, cells, synchronous, endSelections } }
+				contentChange: { value: { kind: NotebookCellsChangeType.Move, versionId: this._versionId, index, length, newIdx, cells, synchronous, endSelections, transient: false } }
 			});
 		}
 

src/vs/workbench/contrib/notebook/common/notebookCommon.ts

@@ -348,7 +348,8 @@ export enum NotebookCellsChangeType {
 	ChangeCellMetadata = 7,
 	Output = 8,
 	ChangeCellContent = 9,
-	ChangeDocumentMetadata = 10
+	ChangeDocumentMetadata = 10,
+	Unknown = 11
 }
 
 export interface NotebookCellsInitializeEvent<T> {
@@ -405,8 +406,13 @@ export interface NotebookDocumentChangeMetadataEvent {
 	readonly metadata: NotebookDocumentMetadata | undefined;
 }
 
-export type NotebookCellsChangedEventDto = NotebookCellsInitializeEvent<IMainCellDto> | NotebookDocumentChangeMetadataEvent | NotebookCellContentChangeEvent | NotebookCellsModelChangedEvent<IMainCellDto> | NotebookCellsModelMoveEvent<IMainCellDto> | NotebookOutputChangedEvent | NotebookCellsChangeLanguageEvent | NotebookCellsChangeMetadataEvent;
-export type NotebookTextModelChangedEvent = (NotebookCellsInitializeEvent<ICell> | NotebookDocumentChangeMetadataEvent | NotebookCellContentChangeEvent | NotebookCellsModelChangedEvent<ICell> | NotebookCellsModelMoveEvent<ICell> | NotebookOutputChangedEvent | NotebookCellsChangeLanguageEvent | NotebookCellsChangeMetadataEvent) & { synchronous: boolean; endSelections?: number[] };
+export interface NotebookDocumentUnknownChangeEvent {
+	readonly kind: NotebookCellsChangeType.Unknown;
+	readonly versionId: number;
+}
+
+export type NotebookCellsChangedEventDto = NotebookCellsInitializeEvent<IMainCellDto> | NotebookDocumentChangeMetadataEvent | NotebookCellContentChangeEvent | NotebookCellsModelChangedEvent<IMainCellDto> | NotebookCellsModelMoveEvent<IMainCellDto> | NotebookOutputChangedEvent | NotebookCellsChangeLanguageEvent | NotebookCellsChangeMetadataEvent | NotebookDocumentUnknownChangeEvent;
+export type NotebookTextModelChangedEvent = (NotebookCellsInitializeEvent<ICell> | NotebookDocumentChangeMetadataEvent | NotebookCellContentChangeEvent | NotebookCellsModelChangedEvent<ICell> | NotebookCellsModelMoveEvent<ICell> | NotebookOutputChangedEvent | NotebookCellsChangeLanguageEvent | NotebookCellsChangeMetadataEvent | NotebookDocumentUnknownChangeEvent) & { synchronous: boolean; endSelections?: number[]; transient: boolean };
 
 export const enum CellEditType {
 	Replace = 1,

src/vs/workbench/contrib/notebook/common/notebookEditorModel.ts

@@ -41,6 +41,8 @@ export class NotebookEditorModel extends EditorModel implements INotebookEditorM
 	private readonly _name: string;
 	private readonly _workingCopyResource: URI;
 
+	private _dirty = false;
+
 	constructor(
 		readonly resource: URI,
 		readonly viewType: string,
@@ -80,6 +82,13 @@ export class NotebookEditorModel extends EditorModel implements INotebookEditorM
 		return this._notebook;
 	}
 
+	setDirty(newState: boolean) {
+		if (this._dirty !== newState) {
+			this._dirty = newState;
+			this._onDidChangeDirty.fire();
+		}
+	}
+
 	async backup(): Promise<IWorkingCopyBackup<NotebookDocumentBackupData>> {
 		if (this._notebook.supportBackup) {
 			const tokenSource = new CancellationTokenSource();
@@ -116,7 +125,7 @@ export class NotebookEditorModel extends EditorModel implements INotebookEditorM
 		const newStats = await this._resolveStats(this.resource);
 		this._lastResolvedFileStat = newStats;
 
-		this._notebook.setDirty(false);
+		this.setDirty(false);
 		this._onDidChangeDirty.fire();
 	}
 
@@ -150,19 +159,19 @@ export class NotebookEditorModel extends EditorModel implements INotebookEditorM
 			if (e.kind !== NotebookCellsChangeType.Initialize) {
 				this._onDidChangeContent.fire();
 			}
-		}));
 
-		this._register(this._notebook.onDidChangeDirty(() => {
-			this._onDidChangeDirty.fire();
+			if (!e.transient) {
+				this.setDirty(true);
+			}
 		}));
 
 		if (forceReloadFromDisk) {
-			this._notebook.setDirty(false);
+			this.setDirty(false);
 		}
 
 		if (backupId) {
 			await this._backupFileService.discardBackup(this._workingCopyResource);
-			this._notebook.setDirty(true);
+			this.setDirty(true);
 		}
 
 		return this;
@@ -173,7 +182,7 @@ export class NotebookEditorModel extends EditorModel implements INotebookEditorM
 	}
 
 	isDirty() {
-		return this._notebook?.isDirty;
+		return this._dirty;
 	}
 
 	isUntitled() {
@@ -225,7 +234,7 @@ export class NotebookEditorModel extends EditorModel implements INotebookEditorM
 		await this._notebookService.save(this.notebook.viewType, this.notebook.uri, tokenSource.token);
 		const newStats = await this._resolveStats(this.resource);
 		this._lastResolvedFileStat = newStats;
-		this._notebook.setDirty(false);
+		this.setDirty(false);
 		return true;
 	}
 
@@ -245,7 +254,7 @@ export class NotebookEditorModel extends EditorModel implements INotebookEditorM
 		await this._notebookService.saveAs(this.notebook.viewType, this.notebook.uri, targetResource, tokenSource.token);
 		const newStats = await this._resolveStats(this.resource);
 		this._lastResolvedFileStat = newStats;
-		this._notebook.setDirty(false);
+		this.setDirty(false);
 		return true;
 	}
 

src/vs/workbench/contrib/notebook/common/notebookEditorModelResolverService.ts

@@ -81,8 +81,8 @@ export class NotebookModelResolverService implements INotebookEditorModelResolve
 	private static _autoReferenceDirtyModel(model: INotebookEditorModel, ref: () => IDisposable) {
 
 		const references = new DisposableStore();
-		const listener = model.notebook.onDidChangeDirty(() => {
-			if (model.notebook.isDirty) {
+		const listener = model.onDidChangeDirty(() => {
+			if (model.isDirty()) {
 				references.add(ref());
 			} else {
 				references.clear();