Update NotebookModel with changes from VS Code Notebook UI (#12048)

For #10496

Update our model when user edits VS Code cells

Author: DonJayamanne

.vscode/launch.json

@@ -195,14 +195,16 @@
             "request": "launch",
             "runtimeExecutable": "${execPath}",
             "args": [
-                "${workspaceFolder}/src/test",
+                "${workspaceFolder}/src/test/datascience",
                 "--disable-extensions",
                 "--enable-proposed-api",
                 "--extensionDevelopmentPath=${workspaceFolder}",
                 "--extensionTestsPath=${workspaceFolder}/out/test"
             ],
             "env": {
                 "VSC_PYTHON_CI_TEST_GREP": "", // Modify this to run a subset of the single workspace tests
+                "VSC_PYTHON_CI_TEST_INVERT_GREP": "", // Initialize this to invert the grep (exclude tests with value defined in grep).
+                
                 "VSC_PYTHON_LOAD_EXPERIMENTS_FROM_FILE": "true",
                 "TEST_FILES_SUFFIX": "ds.test"
             },

.vscode/tasks.json

@@ -33,7 +33,10 @@
                 "kind": "build",
                 "isDefault": true
             },
-            "problemMatcher": []
+            "problemMatcher": [
+                "$tsc-watch",
+                "$ts-checker-webpack-watch"
+            ]
         },
         {
             "label": "Run Unit Tests",
@@ -48,7 +51,9 @@
             "label": "Inject DS WebBrowser UI",
             "type": "shell",
             "command": "node",
-            "args": ["build/debug/replaceWithWebBrowserPanel.js"],
+            "args": [
+                "build/debug/replaceWithWebBrowserPanel.js"
+            ],
             "problemMatcher": []
         }
     ]

package.json

@@ -2998,7 +2998,7 @@
         "test:cover:report": "nyc --nycrc-path build/.nycrc  report --reporter=text --reporter=html --reporter=text-summary --reporter=cobertura",
         "testDebugger": "node ./out/test/testBootstrap.js ./out/test/debuggerTest.js",
         "testSingleWorkspace": "node ./out/test/testBootstrap.js ./out/test/standardTest.js",
-        "testDataScience": "cross-env VSC_PYTHON_CI_TEST_VSC_CHANNEL=insiders TEST_FILES_SUFFIX=ds.test node ./out/test/testBootstrap.js ./out/test/standardTest.js",
+        "testDataScience": "cross-env CODE_TESTS_WORKSPACE=src/test/datascience VSC_PYTHON_CI_TEST_VSC_CHANNEL=insiders TEST_FILES_SUFFIX=ds.test node ./out/test/testBootstrap.js ./out/test/standardTest.js",
         "testMultiWorkspace": "node ./out/test/testBootstrap.js ./out/test/multiRootTest.js",
         "testPerformance": "node ./out/test/testBootstrap.js ./out/test/performanceTest.js",
         "testSmoke": "node ./out/test/smokeTest.js",

src/client/common/application/notebook.ts

@@ -5,6 +5,7 @@ import { inject, injectable } from 'inversify';
 import { Disposable, Event, EventEmitter, GlobPattern, TextDocument, window } from 'vscode';
 import type {
     notebook,
+    NotebookCellsChangeEvent as VSCNotebookCellsChangeEvent,
     NotebookContentProvider,
     NotebookDocument,
     NotebookEditor,
@@ -17,19 +18,15 @@ import { IDisposableRegistry } from '../types';
 import {
     IVSCodeNotebook,
     NotebookCellLanguageChangeEvent,
-    NotebookCellMoveEvent,
     NotebookCellOutputsChangeEvent,
     NotebookCellsChangeEvent
 } from './types';
 
 @injectable()
 export class VSCodeNotebook implements IVSCodeNotebook {
-    private readonly _onDidChangeNotebookDocument = new EventEmitter<
-        | NotebookCellsChangeEvent
-        | NotebookCellMoveEvent
-        | NotebookCellOutputsChangeEvent
-        | NotebookCellLanguageChangeEvent
-    >();
+    public get onDidChangeActiveNotebookEditor(): Event<NotebookEditor | undefined> {
+        return this.notebook.onDidChangeActiveNotebookEditor;
+    }
     public get onDidOpenNotebookDocument(): Event<NotebookDocument> {
         return this.notebook.onDidOpenNotebookDocument;
     }
@@ -40,17 +37,8 @@ export class VSCodeNotebook implements IVSCodeNotebook {
         return this.notebook.visibleNotebookEditors;
     }
     public get onDidChangeNotebookDocument(): Event<
-        | NotebookCellsChangeEvent
-        | NotebookCellMoveEvent
-        | NotebookCellOutputsChangeEvent
-        | NotebookCellLanguageChangeEvent
+        NotebookCellsChangeEvent | NotebookCellOutputsChangeEvent | NotebookCellLanguageChangeEvent
     > {
-        // Temporarily disabled as API is not yet
-        // Bogus if, to satisyf compiler and ensure addEventHandlers method is used.
-        if (process.env.SOME_BOGUS_ENV_VAR) {
-            this.addEventHandlers();
-        }
-
         return this._onDidChangeNotebookDocument.event;
     }
     public get activeNotebookEditor(): NotebookEditor | undefined {
@@ -72,19 +60,27 @@ export class VSCodeNotebook implements IVSCodeNotebook {
         }
         return this.notebook.activeNotebookEditor;
     }
-    private addedEventHandlers?: boolean;
-    private _notebook?: typeof notebook;
     private get notebook() {
         if (!this._notebook) {
             // tslint:disable-next-line: no-require-imports
             this._notebook = require('vscode').notebook;
         }
         return this._notebook!;
     }
+    private readonly _onDidChangeNotebookDocument = new EventEmitter<
+        NotebookCellsChangeEvent | NotebookCellOutputsChangeEvent | NotebookCellLanguageChangeEvent
+    >();
+    private addedEventHandlers?: boolean;
+    private _notebook?: typeof notebook;
+    private readonly handledCellChanges = new WeakSet<VSCNotebookCellsChangeEvent>();
     constructor(
         @inject(UseProposedApi) private readonly useProposedApi: boolean,
         @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry
-    ) {}
+    ) {
+        if (this.useProposedApi) {
+            this.addEventHandlers();
+        }
+    }
     public registerNotebookContentProvider(notebookType: string, provider: NotebookContentProvider): Disposable {
         return this.notebook.registerNotebookContentProvider(notebookType, provider);
     }
@@ -110,6 +106,7 @@ export class VSCodeNotebook implements IVSCodeNotebook {
         if (this.addedEventHandlers) {
             return;
         }
+        this.addedEventHandlers = true;
         this.disposables.push(
             ...[
                 this.notebook.onDidChangeCellLanguage((e) =>
@@ -118,12 +115,13 @@ export class VSCodeNotebook implements IVSCodeNotebook {
                 this.notebook.onDidChangeCellOutputs((e) =>
                     this._onDidChangeNotebookDocument.fire({ ...e, type: 'changeCellOutputs' })
                 ),
-                this.notebook.onDidChangeNotebookCells((e) =>
-                    this._onDidChangeNotebookDocument.fire({ ...e, type: 'changeCells' })
-                ),
-                this.notebook.onDidMoveNotebookCell((e) =>
-                    this._onDidChangeNotebookDocument.fire({ ...e, type: 'moveCell' })
-                )
+                this.notebook.onDidChangeNotebookCells((e) => {
+                    if (this.handledCellChanges.has(e)) {
+                        return;
+                    }
+                    this.handledCellChanges.add(e);
+                    this._onDidChangeNotebookDocument.fire({ ...e, type: 'changeCells' });
+                })
             ]
         );
     }

src/client/common/application/types.ts

@@ -60,7 +60,6 @@ import {
 } from 'vscode';
 import type {
     NotebookCellLanguageChangeEvent as VSCNotebookCellLanguageChangeEvent,
-    NotebookCellMoveEvent as VSCNotebookCellMoveEvent,
     NotebookCellOutputsChangeEvent as VSCNotebookCellOutputsChangeEvent,
     NotebookCellsChangeEvent as VSCNotebookCellsChangeEvent,
     NotebookContentProvider,
@@ -1456,20 +1455,16 @@ export interface IClipboard {
 }
 
 export type NotebookCellsChangeEvent = { type: 'changeCells' } & VSCNotebookCellsChangeEvent;
-export type NotebookCellMoveEvent = { type: 'moveCell' } & VSCNotebookCellMoveEvent;
 export type NotebookCellOutputsChangeEvent = { type: 'changeCellOutputs' } & VSCNotebookCellOutputsChangeEvent;
 export type NotebookCellLanguageChangeEvent = { type: 'changeCellLanguage' } & VSCNotebookCellLanguageChangeEvent;
 
 export const IVSCodeNotebook = Symbol('IVSCodeNotebook');
 export interface IVSCodeNotebook {
     readonly onDidOpenNotebookDocument: Event<NotebookDocument>;
     readonly onDidCloseNotebookDocument: Event<NotebookDocument>;
-
+    readonly onDidChangeActiveNotebookEditor: Event<NotebookEditor | undefined>;
     readonly onDidChangeNotebookDocument: Event<
-        | NotebookCellsChangeEvent
-        | NotebookCellMoveEvent
-        | NotebookCellOutputsChangeEvent
-        | NotebookCellLanguageChangeEvent
+        NotebookCellsChangeEvent | NotebookCellOutputsChangeEvent | NotebookCellLanguageChangeEvent
     >;
     readonly notebookEditors: Readonly<NotebookEditor[]>;
     readonly activeNotebookEditor: NotebookEditor | undefined;

src/client/datascience/interactive-common/interactiveWindowTypes.ts

@@ -363,6 +363,11 @@ export interface INotebookModelModifyChange extends INotebookModelChange {
     newCells: ICell[];
     oldCells: ICell[];
 }
+export interface INotebookModelCellExecutionCountChange extends INotebookModelChange {
+    kind: 'updateCellExecutionCount';
+    cellId: string;
+    executionCount?: number;
+}
 
 export interface INotebookModelClearChange extends INotebookModelChange {
     kind: 'clear';
@@ -478,7 +483,8 @@ export type NotebookModelChange =
     | INotebookModelAddChange
     | INotebookModelEditChange
     | INotebookModelVersionChange
-    | INotebookModelChangeTypeChange;
+    | INotebookModelChangeTypeChange
+    | INotebookModelCellExecutionCountChange;
 
 export interface IRunByLine {
     cell: ICell;

src/client/datascience/interactive-ipynb/nativeEditorStorage.ts

@@ -208,6 +208,9 @@ export class NativeEditorNotebookModel implements INotebookModel {
             case 'swap':
                 changed = this.swapCells(change.firstCellId, change.secondCellId);
                 break;
+            case 'updateCellExecutionCount':
+                changed = this.updateCellExecutionCount(change.cellId, change.executionCount);
+                break;
             case 'version':
                 changed = this.updateVersionInfo(change.interpreter, change.kernelSpec);
                 break;
@@ -325,6 +328,15 @@ export class NativeEditorNotebookModel implements INotebookModel {
         return false;
     }
 
+    private updateCellExecutionCount(cellId: string, executionCount?: number) {
+        const index = this.cells.findIndex((v) => v.id === cellId);
+        if (index >= 0) {
+            this._state.cells[index].data.execution_count = (executionCount || 0) > 0 ? executionCount : null;
+            return true;
+        }
+        return false;
+    }
+
     private modifyCells(cells: ICell[]): boolean {
         // Update these cells in our list
         cells.forEach((c) => {

src/client/datascience/notebook/autoSaveService.ts

@@ -0,0 +1,18 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+'use strict';
+
+/**
+ * When executing cells, VSCode doesn't perform a save operation.
+ * However in Jupyter Lab (with auto save on), even when you execute cells auto save kicks in.
+ * Solution:
+ * - If notebook isn't saved after a cell is executed, then we can save manually.
+ * - We need to take the auto save delay into account (if any, for throttling).
+ * - Save only if user has executed (if execution count changes or cell output changes).
+ * - When adding/deleting/editing cells, VSC automatically performs a save.
+ *
+ * Hence, we only need to handle auto save when executing cell.s
+ */
+// @injectable()
+// export class AutoSaveService implements IExtensionSingleActivationService {}