fix for parameterized notebook closure. (#14324) (#14334)

* Two options

* remove commented line and add the null check

* fix tests: pass isDirty explicitly

* remove commented line

Co-authored-by: chlafreniere <hichise@gmail.com>

Co-authored-by: chlafreniere <hichise@gmail.com>

src/sql/workbench/contrib/notebook/browser/models/notebookInput.ts

@@ -122,6 +122,9 @@ export class NotebookEditorModel extends EditorModel {
 			this._isFirstKernelChange = false;
 			return;
 		}
+		if (!contentChange?.isDirty) {
+			return;
+		}
 		this._lastEditFullReplacement = false;
 		if (contentChange && contentChange.changeType === NotebookChangeType.Saved) {
 			// We send the saved events out, so ignore. Otherwise we double-count this as a change

src/sql/workbench/contrib/notebook/test/electron-browser/notebookEditorModel.test.ts

@@ -191,6 +191,7 @@ suite('Notebook Editor Model', function (): void {
 
 		let contentChange: NotebookContentChange = {
 			changeType: NotebookChangeType.CellsModified,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0
 		};
@@ -215,6 +216,7 @@ suite('Notebook Editor Model', function (): void {
 
 		let contentChange: NotebookContentChange = {
 			changeType: NotebookChangeType.CellsModified,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0
 		};
@@ -226,6 +228,7 @@ suite('Notebook Editor Model', function (): void {
 		newCell.executionCount = 1;
 		contentChange = {
 			changeType: NotebookChangeType.CellExecuted,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0
 		};
@@ -243,6 +246,7 @@ suite('Notebook Editor Model', function (): void {
 		newCell.executionCount = 10;
 		contentChange = {
 			changeType: NotebookChangeType.CellExecuted,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0
 		};
@@ -254,6 +258,7 @@ suite('Notebook Editor Model', function (): void {
 		newCell.executionCount = 15;
 		contentChange = {
 			changeType: NotebookChangeType.CellExecuted,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0
 		};
@@ -265,6 +270,7 @@ suite('Notebook Editor Model', function (): void {
 		newCell.executionCount = 105;
 		contentChange = {
 			changeType: NotebookChangeType.CellExecuted,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0
 		};
@@ -283,6 +289,7 @@ suite('Notebook Editor Model', function (): void {
 
 		let contentChange: NotebookContentChange = {
 			changeType: NotebookChangeType.CellsModified,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0
 		};
@@ -293,6 +300,7 @@ suite('Notebook Editor Model', function (): void {
 
 		contentChange = {
 			changeType: NotebookChangeType.CellOutputCleared,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0
 		};
@@ -317,6 +325,7 @@ suite('Notebook Editor Model', function (): void {
 
 		let contentChange: NotebookContentChange = {
 			changeType: NotebookChangeType.CellsModified,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0
 		};
@@ -327,6 +336,7 @@ suite('Notebook Editor Model', function (): void {
 
 		contentChange = {
 			changeType: NotebookChangeType.CellSourceUpdated,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0,
 			modelContentChangedEvent: {
@@ -363,6 +373,7 @@ suite('Notebook Editor Model', function (): void {
 
 		let contentChange: NotebookContentChange = {
 			changeType: NotebookChangeType.CellsModified,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0
 		};
@@ -373,6 +384,7 @@ suite('Notebook Editor Model', function (): void {
 
 		contentChange = {
 			changeType: NotebookChangeType.CellSourceUpdated,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0,
 			modelContentChangedEvent: {
@@ -407,6 +419,7 @@ suite('Notebook Editor Model', function (): void {
 
 		let contentChange: NotebookContentChange = {
 			changeType: NotebookChangeType.CellsModified,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0
 		};
@@ -419,6 +432,7 @@ suite('Notebook Editor Model', function (): void {
 
 		contentChange = {
 			changeType: NotebookChangeType.CellSourceUpdated,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0,
 			modelContentChangedEvent: undefined
@@ -447,6 +461,7 @@ suite('Notebook Editor Model', function (): void {
 
 		let contentChange: NotebookContentChange = {
 			changeType: NotebookChangeType.CellsModified,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0
 		};
@@ -459,6 +474,7 @@ suite('Notebook Editor Model', function (): void {
 
 		contentChange = {
 			changeType: NotebookChangeType.CellSourceUpdated,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0,
 			modelContentChangedEvent: undefined
@@ -488,6 +504,7 @@ suite('Notebook Editor Model', function (): void {
 
 		let contentChange: NotebookContentChange = {
 			changeType: NotebookChangeType.CellsModified,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0
 		};
@@ -501,6 +518,7 @@ suite('Notebook Editor Model', function (): void {
 
 		contentChange = {
 			changeType: NotebookChangeType.CellSourceUpdated,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0,
 			modelContentChangedEvent: {
@@ -518,6 +536,7 @@ suite('Notebook Editor Model', function (): void {
 
 		contentChange = {
 			changeType: NotebookChangeType.CellSourceUpdated,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0,
 			modelContentChangedEvent: {
@@ -552,6 +571,7 @@ suite('Notebook Editor Model', function (): void {
 
 		let contentChange: NotebookContentChange = {
 			changeType: NotebookChangeType.CellsModified,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0
 		};
@@ -561,6 +581,7 @@ suite('Notebook Editor Model', function (): void {
 
 		contentChange = {
 			changeType: NotebookChangeType.CellSourceUpdated,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0,
 			modelContentChangedEvent: {
@@ -585,6 +606,7 @@ suite('Notebook Editor Model', function (): void {
 
 		contentChange = {
 			changeType: NotebookChangeType.CellSourceUpdated,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0,
 			modelContentChangedEvent: {
@@ -624,6 +646,7 @@ suite('Notebook Editor Model', function (): void {
 
 			contentChange = {
 				changeType: NotebookChangeType.CellsModified,
+				isDirty: true,
 				cells: [cell],
 				cellIndex: 0
 			};
@@ -633,6 +656,7 @@ suite('Notebook Editor Model', function (): void {
 
 		contentChange = {
 			changeType: NotebookChangeType.CellSourceUpdated,
+			isDirty: true,
 			cells: [newCell],
 			modelContentChangedEvent: {
 				changes: [{ range: new Range(1, 1, 1, 1), rangeLength: 0, rangeOffset: 0, text: 'This is a test' }],
@@ -671,6 +695,7 @@ suite('Notebook Editor Model', function (): void {
 
 		let contentChange: NotebookContentChange = {
 			changeType: NotebookChangeType.CellsModified,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0
 		};
@@ -683,6 +708,7 @@ suite('Notebook Editor Model', function (): void {
 
 		contentChange = {
 			changeType: NotebookChangeType.CellOutputUpdated,
+			isDirty: true,
 			cells: [newCell]
 		};
 
@@ -708,6 +734,7 @@ suite('Notebook Editor Model', function (): void {
 
 		let contentChange: NotebookContentChange = {
 			changeType: NotebookChangeType.CellsModified,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0
 		};
@@ -722,6 +749,7 @@ suite('Notebook Editor Model', function (): void {
 
 		contentChange = {
 			changeType: NotebookChangeType.CellOutputUpdated,
+			isDirty: true,
 			cells: [newCell]
 		};
 
@@ -744,6 +772,7 @@ suite('Notebook Editor Model', function (): void {
 
 		contentChange = {
 			changeType: NotebookChangeType.CellOutputUpdated,
+			isDirty: true,
 			cells: [newCell]
 		};
 
@@ -772,6 +801,7 @@ suite('Notebook Editor Model', function (): void {
 
 		let contentChange: NotebookContentChange = {
 			changeType: NotebookChangeType.CellsModified,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0
 		};
@@ -786,6 +816,7 @@ suite('Notebook Editor Model', function (): void {
 
 		contentChange = {
 			changeType: NotebookChangeType.CellOutputUpdated,
+			isDirty: true,
 			cells: [newCell]
 		};
 
@@ -986,6 +1017,7 @@ suite('Notebook Editor Model', function (): void {
 
 		let contentChange: NotebookContentChange = {
 			changeType: NotebookChangeType.CellsModified,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0
 		};
@@ -999,6 +1031,7 @@ suite('Notebook Editor Model', function (): void {
 	function addTextToBeginningOfTextEditorModel(notebookEditorModel: NotebookEditorModel, newCell: ICellModel, textToAdd: string) {
 		let contentChange: NotebookContentChange = {
 			changeType: NotebookChangeType.CellSourceUpdated,
+			isDirty: true,
 			cells: [newCell],
 			cellIndex: 0,
 			modelContentChangedEvent: {

src/sql/workbench/services/notebook/browser/models/notebookModel.ts

@@ -110,6 +110,7 @@ export class NotebookModel extends Disposable implements INotebookModel {
 	private _multiConnectionMode: boolean = false;
 
 	public requestConnectionHandler: (() => Promise<boolean>) | undefined;
+	private _isLoading: boolean = false;
 
 	constructor(
 		private _notebookOptions: INotebookModelOptions,
@@ -385,6 +386,7 @@ export class NotebookModel extends Disposable implements INotebookModel {
 
 	public async loadContents(isTrusted = false, forceLayoutChange = false): Promise<void> {
 		try {
+			this._isLoading = true;
 			this._trustedMode = isTrusted;
 
 			let contents: nb.INotebookContents | undefined;
@@ -421,8 +423,7 @@ export class NotebookModel extends Disposable implements INotebookModel {
 						*/
 						if (cellModel.isInjectedParameter) {
 							hasInjectedCell = true;
-							cellModel.source = cellModel.source.slice(1);
+							cellModel.source = [injectedParametersMsg].concat(cellModel.source.slice(1));
-							cellModel.source = [injectedParametersMsg].concat(cellModel.source);
 						}
 						this.trackMarkdownTelemetry(<nb.ICellContents>c, cellModel);
 						return cellModel;
@@ -444,6 +445,8 @@ export class NotebookModel extends Disposable implements INotebookModel {
 		} catch (error) {
 			this._inErrorState = true;
 			throw error;
+		} finally {
+			this._isLoading = false;
 		}
 	}
 
@@ -1292,7 +1295,7 @@ export class NotebookModel extends Disposable implements INotebookModel {
 			case NotebookChangeType.CellSourceUpdated:
 			case NotebookChangeType.CellInputVisibilityChanged:
 			case NotebookChangeType.CellMetadataUpdated:
-				changeInfo.isDirty = true;
+				changeInfo.isDirty = this._isLoading ? false : true;
 				changeInfo.modelContentChangedEvent = cell.modelContentChangedEvent;
 				break;
 			default: