Remove notifyChangesToDocument as this is no longer necessary (#14237)

Author: DonJayamanne

src/client/datascience/jupyter/kernels/cellExecution.ts

@@ -27,7 +27,6 @@ import {
 } from '../../notebook/helpers/helpers';
 import { MultiCancellationTokenSource } from '../../notebook/helpers/multiCancellationToken';
 import { NotebookEditor } from '../../notebook/notebookEditor';
-import { INotebookContentProvider } from '../../notebook/types';
 import {
     IDataScienceErrorHandler,
     IJupyterSession,
@@ -41,7 +40,6 @@ const vscodeNotebookEnums = require('vscode') as typeof import('vscode-proposed'
 
 export class CellExecutionFactory {
     constructor(
-        private readonly contentProvider: INotebookContentProvider,
         private readonly errorHandler: IDataScienceErrorHandler,
         private readonly editorProvider: INotebookEditorProvider,
         private readonly appShell: IApplicationShell,
@@ -53,7 +51,6 @@ export class CellExecutionFactory {
         return CellExecution.fromCell(
             this.vscNotebook.notebookEditors.find((e) => e.document === cell.notebook)!,
             cell,
-            this.contentProvider,
             this.errorHandler,
             this.editorProvider,
             this.appShell
@@ -104,7 +101,6 @@ export class CellExecution {
     private constructor(
         public readonly editor: VSCNotebookEditor,
         public readonly cell: NotebookCell,
-        private readonly contentProvider: INotebookContentProvider,
         private readonly errorHandler: IDataScienceErrorHandler,
         private readonly editorProvider: INotebookEditorProvider,
         private readonly applicationService: IApplicationShell
@@ -116,12 +112,11 @@ export class CellExecution {
     public static fromCell(
         editor: VSCNotebookEditor,
         cell: NotebookCell,
-        contentProvider: INotebookContentProvider,
         errorHandler: IDataScienceErrorHandler,
         editorProvider: INotebookEditorProvider,
         appService: IApplicationShell
     ) {
-        return new CellExecution(editor, cell, contentProvider, errorHandler, editorProvider, appService);
+        return new CellExecution(editor, cell, errorHandler, editorProvider, appService);
     }
 
     public async start(kernelPromise: Promise<IKernel>, notebook: INotebook) {
@@ -136,8 +131,6 @@ export class CellExecution {
             });
         });
         this.stopWatch.reset();
-        // Changes to metadata must be saved in ipynb, hence mark doc has dirty.
-        this.contentProvider.notifyChangesToDocument(this.cell.notebook);
         this.notifyCellExecution();
 
         // Begin the request that will modify our cell.
@@ -182,13 +175,10 @@ export class CellExecution {
             })
         );
         await updateCellWithErrorStatus(this.editor, this.cell, error);
-        this.contentProvider.notifyChangesToDocument(this.cell.notebook);
         this.errorHandler.handleError((error as unknown) as Error).ignoreErrors();
 
         this._completed = true;
         this._result.resolve(this.cell.metadata.runState);
-        // Changes to metadata must be saved in ipynb, hence mark doc has dirty.
-        this.contentProvider.notifyChangesToDocument(this.cell.notebook);
     }
 
     private async completedSuccessfully() {
@@ -222,8 +212,6 @@ export class CellExecution {
 
         this._completed = true;
         this._result.resolve(this.cell.metadata.runState);
-        // Changes to metadata must be saved in ipynb, hence mark doc has dirty.
-        this.contentProvider.notifyChangesToDocument(this.cell.notebook);
     }
 
     /**
@@ -258,8 +246,6 @@ export class CellExecution {
         );
         this._completed = true;
         this._result.resolve(this.cell.metadata.runState);
-        // Changes to metadata must be saved in ipynb, hence mark doc has dirty.
-        this.contentProvider.notifyChangesToDocument(this.cell.notebook);
     }
 
     /**
@@ -273,7 +259,6 @@ export class CellExecution {
                 runState: vscodeNotebookEnums.NotebookCellRunState.Running
             })
         );
-        this.contentProvider.notifyChangesToDocument(this.cell.notebook);
     }
 
     private sendPerceivedCellExecute() {
@@ -371,8 +356,6 @@ export class CellExecution {
         // tslint:disable-next-line:no-require-imports
         const jupyterLab = require('@jupyterlab/services') as typeof import('@jupyterlab/services');
 
-        // Keep track of we need to send an update to VS code or not.
-        let shouldUpdate = true;
         try {
             if (jupyterLab.KernelMessage.isExecuteResultMsg(msg)) {
                 await this.handleExecuteResult(msg as KernelMessage.IExecuteResultMsg, clearState);
@@ -382,28 +365,23 @@ export class CellExecution {
                 // Status is handled by the result promise. While it is running we are active. Otherwise we're stopped.
                 // So ignore status messages.
                 const statusMsg = msg as KernelMessage.IStatusMsg;
-                shouldUpdate = false;
                 this.handleStatusMessage(statusMsg, clearState);
             } else if (jupyterLab.KernelMessage.isStreamMsg(msg)) {
                 await this.handleStreamMessage(msg as KernelMessage.IStreamMsg, clearState);
             } else if (jupyterLab.KernelMessage.isDisplayDataMsg(msg)) {
                 await this.handleDisplayData(msg as KernelMessage.IDisplayDataMsg, clearState);
-            } else if (jupyterLab.KernelMessage.isUpdateDisplayDataMsg(msg)) {
-                // No new data to update UI, hence do not send updates.
-                shouldUpdate = false;
             } else if (jupyterLab.KernelMessage.isClearOutputMsg(msg)) {
                 await this.handleClearOutput(msg as KernelMessage.IClearOutputMsg, clearState);
             } else if (jupyterLab.KernelMessage.isErrorMsg(msg)) {
                 await this.handleError(msg as KernelMessage.IErrorMsg, clearState);
+            } else if (jupyterLab.KernelMessage.isUpdateDisplayDataMsg(msg)) {
+                // Noop.
             } else if (jupyterLab.KernelMessage.isCommOpenMsg(msg)) {
-                // No new data to update UI, hence do not send updates.
-                shouldUpdate = false;
+                // Noop.
             } else if (jupyterLab.KernelMessage.isCommMsgMsg(msg)) {
-                // No new data to update UI, hence do not send updates.
-                shouldUpdate = false;
+                // Noop.
             } else if (jupyterLab.KernelMessage.isCommCloseMsg(msg)) {
-                // No new data to update UI, hence do not send updates.
-                shouldUpdate = false;
+                // Noop.
             } else {
                 traceWarning(`Unknown message ${msg.header.msg_type} : hasData=${'data' in msg.content}`);
             }
@@ -412,11 +390,6 @@ export class CellExecution {
             if ('execution_count' in msg.content && typeof msg.content.execution_count === 'number') {
                 updateCellExecutionCount(this.editor, this.cell, msg.content.execution_count).then(noop, noop);
             }
-
-            // Show our update if any new output.
-            if (shouldUpdate) {
-                this.contentProvider.notifyChangesToDocument(this.cell.notebook);
-            }
         } catch (err) {
             // If not a restart error, then tell the subscriber
             this.completedWithErrors(err).then(noop, noop);
@@ -588,9 +561,6 @@ export class CellExecution {
             if ('execution_count' in msg.content && typeof msg.content.execution_count === 'number') {
                 await updateCellExecutionCount(this.editor, this.cell, msg.content.execution_count);
             }
-
-            // Send this event.
-            this.contentProvider.notifyChangesToDocument(this.cell.notebook);
         }
     }
 }

src/client/datascience/jupyter/kernels/kernel.ts

@@ -24,7 +24,6 @@ import { createDeferred, Deferred } from '../../../common/utils/async';
 import { noop } from '../../../common/utils/misc';
 import { IInterpreterService } from '../../../interpreter/contracts';
 import { CodeSnippets } from '../../constants';
-import { INotebookContentProvider } from '../../notebook/types';
 import { getDefaultNotebookContent, updateNotebookMetadata } from '../../notebookStorage/baseModel';
 import {
     IDataScienceErrorHandler,
@@ -82,7 +81,6 @@ export class Kernel implements IKernel {
         commandManager: ICommandManager,
         interpreterService: IInterpreterService,
         private readonly errorHandler: IDataScienceErrorHandler,
-        contentProvider: INotebookContentProvider,
         editorProvider: INotebookEditorProvider,
         private readonly kernelProvider: IKernelProvider,
         private readonly kernelSelectionUsage: IKernelSelectionUsage,
@@ -94,7 +92,6 @@ export class Kernel implements IKernel {
             commandManager,
             interpreterService,
             errorHandler,
-            contentProvider,
             editorProvider,
             kernelSelectionUsage,
             appShell,

src/client/datascience/jupyter/kernels/kernelExecution.ts

@@ -13,7 +13,6 @@ import { captureTelemetry } from '../../../telemetry';
 import { Commands, Telemetry, VSCodeNativeTelemetry } from '../../constants';
 import { handleUpdateDisplayDataMessage } from '../../notebook/helpers/executionHelpers';
 import { MultiCancellationTokenSource } from '../../notebook/helpers/multiCancellationToken';
-import { INotebookContentProvider } from '../../notebook/types';
 import { IDataScienceErrorHandler, INotebook, INotebookEditorProvider } from '../../types';
 import { CellExecution, CellExecutionFactory } from './cellExecution';
 import type { IKernel, IKernelProvider, IKernelSelectionUsage } from './types';
@@ -40,19 +39,12 @@ export class KernelExecution implements IDisposable {
         private readonly commandManager: ICommandManager,
         private readonly interpreterService: IInterpreterService,
         errorHandler: IDataScienceErrorHandler,
-        private readonly contentProvider: INotebookContentProvider,
         editorProvider: INotebookEditorProvider,
         readonly kernelSelectionUsage: IKernelSelectionUsage,
         readonly appShell: IApplicationShell,
         readonly vscNotebook: IVSCodeNotebook
     ) {
-        this.executionFactory = new CellExecutionFactory(
-            this.contentProvider,
-            errorHandler,
-            editorProvider,
-            appShell,
-            vscNotebook
-        );
+        this.executionFactory = new CellExecutionFactory(errorHandler, editorProvider, appShell, vscNotebook);
     }
 
     @captureTelemetry(Telemetry.ExecuteNativeCell, undefined, true)
@@ -174,9 +166,7 @@ export class KernelExecution implements IDisposable {
         const jupyterLab = require('@jupyterlab/services') as typeof import('@jupyterlab/services');
         const editor = this.vscNotebook.notebookEditors.find((e) => e.document === document);
         if (jupyterLab.KernelMessage.isUpdateDisplayDataMsg(msg) && editor) {
-            if (await handleUpdateDisplayDataMessage(msg, editor)) {
-                this.contentProvider.notifyChangesToDocument(document);
-            }
+            await handleUpdateDisplayDataMessage(msg, editor);
         }
     }
 

src/client/datascience/jupyter/kernels/kernelProvider.ts

@@ -10,7 +10,6 @@ import { IApplicationShell, ICommandManager, IVSCodeNotebook } from '../../../co
 import { traceInfo, traceWarning } from '../../../common/logger';
 import { IAsyncDisposableRegistry, IConfigurationService, IDisposableRegistry } from '../../../common/types';
 import { IInterpreterService } from '../../../interpreter/contracts';
-import { INotebookContentProvider } from '../../notebook/types';
 import { IDataScienceErrorHandler, INotebookEditorProvider, INotebookProvider } from '../../types';
 import { Kernel } from './kernel';
 import { KernelSelector } from './kernelSelector';
@@ -27,7 +26,6 @@ export class KernelProvider implements IKernelProvider {
         @inject(ICommandManager) private readonly commandManager: ICommandManager,
         @inject(IInterpreterService) private readonly interpreterService: IInterpreterService,
         @inject(IDataScienceErrorHandler) private readonly errorHandler: IDataScienceErrorHandler,
-        @inject(INotebookContentProvider) private readonly contentProvider: INotebookContentProvider,
         @inject(INotebookEditorProvider) private readonly editorProvider: INotebookEditorProvider,
         @inject(KernelSelector) private readonly kernelSelectionUsage: IKernelSelectionUsage,
         @inject(IApplicationShell) private readonly appShell: IApplicationShell,
@@ -54,7 +52,6 @@ export class KernelProvider implements IKernelProvider {
             this.commandManager,
             this.interpreterService,
             this.errorHandler,
-            this.contentProvider,
             this.editorProvider,
             this,
             this.kernelSelectionUsage,

src/client/datascience/notebook/contentProvider.ts

@@ -7,6 +7,7 @@ import { inject, injectable } from 'inversify';
 import { CancellationToken, EventEmitter, Uri } from 'vscode';
 import type {
     NotebookCommunication,
+    NotebookContentProvider as VSCNotebookContentProvider,
     NotebookData,
     NotebookDocument,
     NotebookDocumentBackup,
@@ -22,10 +23,8 @@ import { INotebookStorageProvider } from '../notebookStorage/notebookStorageProv
 import { VSCodeNotebookModel } from '../notebookStorage/vscNotebookModel';
 import { notebookModelToVSCNotebookData } from './helpers/helpers';
 import { NotebookEditorCompatibilitySupport } from './notebookEditorCompatibilitySupport';
-import { INotebookContentProvider } from './types';
 // tslint:disable-next-line: no-var-requires no-require-imports
 const vscodeNotebookEnums = require('vscode') as typeof import('vscode-proposed');
-
 /**
  * This class is responsible for reading a notebook file (ipynb or other files) and returning VS Code with the NotebookData.
  * Its up to extension authors to read the files and return it in a format that VSCode understands.
@@ -35,7 +34,7 @@ const vscodeNotebookEnums = require('vscode') as typeof import('vscode-proposed'
  * When saving, VSC will provide their model and we need to take that and merge it with an existing ipynb json (if any, to preserve metadata).
  */
 @injectable()
-export class NotebookContentProvider implements INotebookContentProvider {
+export class NotebookContentProvider implements VSCNotebookContentProvider {
     private notebookChanged = new EventEmitter<NotebookDocumentContentChangeEvent>();
     public get onDidChangeNotebook() {
         return this.notebookChanged.event;
@@ -45,9 +44,6 @@ export class NotebookContentProvider implements INotebookContentProvider {
         @inject(NotebookEditorCompatibilitySupport)
         private readonly compatibilitySupport: NotebookEditorCompatibilitySupport
     ) {}
-    public notifyChangesToDocument(_document: NotebookDocument) {
-        // this.notebookChanged.fire({ document });
-    }
     public async resolveNotebook(_document: NotebookDocument, _webview: NotebookCommunication): Promise<void> {
         // Later
     }

src/client/datascience/notebook/helpers/executionHelpers.ts

@@ -21,14 +21,14 @@ const vscodeNotebookEnums = require('vscode') as typeof import('vscode-proposed'
 export async function handleUpdateDisplayDataMessage(
     msg: KernelMessage.IUpdateDisplayDataMsg,
     editor: NotebookEditor
-): Promise<boolean> {
+): Promise<void> {
     const document = editor.document;
-    let updated = false;
     // Find any cells that have this same display_id
     for (const cell of document.cells) {
         if (cell.cellKind !== vscodeNotebookEnums.CellKind.Code) {
-            return false;
+            continue;
         }
+        let updated = false;
 
         const outputs = createIOutputFromCellOutputs(cell.outputs);
         const changedOutputs = outputs.map((output) => {
@@ -51,15 +51,10 @@ export async function handleUpdateDisplayDataMessage(
             }
         });
 
-        if (!updated) {
-            continue;
+        if (updated) {
+            await updateCellOutput(editor, cell, changedOutputs);
         }
-
-        await updateCellOutput(editor, cell, changedOutputs);
-        updated = true;
     }
-
-    return updated;
 }
 
 /**
@@ -87,7 +82,7 @@ export async function updateCellExecutionCount(
     editor: NotebookEditor,
     cell: NotebookCell,
     executionCount: number
-): Promise<boolean> {
+): Promise<void> {
     if (cell.metadata.executionOrder !== executionCount && executionCount) {
         const cellIndex = editor.document.cells.indexOf(cell);
         await editor.edit((edit) =>
@@ -96,9 +91,7 @@ export async function updateCellExecutionCount(
                 executionOrder: executionCount
             })
         );
-        return true;
     }
-    return false;
 }
 
 /**

src/client/datascience/notebook/integration.ts

@@ -3,6 +3,7 @@
 'use strict';
 import { inject, injectable } from 'inversify';
 import { ConfigurationTarget } from 'vscode';
+import { NotebookContentProvider as VSCNotebookContentProvider } from '../../../../types/vscode-proposed';
 import { IExtensionSingleActivationService } from '../../activation/types';
 import {
     IApplicationEnvironment,
@@ -33,7 +34,7 @@ export class NotebookIntegration implements IExtensionSingleActivationService {
         @inject(IVSCodeNotebook) private readonly vscNotebook: IVSCodeNotebook,
         @inject(IExperimentsManager) private readonly experiment: IExperimentsManager,
         @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry,
-        @inject(INotebookContentProvider) private readonly notebookContentProvider: INotebookContentProvider,
+        @inject(INotebookContentProvider) private readonly notebookContentProvider: VSCNotebookContentProvider,
         @inject(VSCodeKernelPickerProvider) private readonly kernelProvider: VSCodeKernelPickerProvider,
         @inject(IApplicationEnvironment) private readonly env: IApplicationEnvironment,
         @inject(IApplicationShell) private readonly shell: IApplicationShell,

src/client/datascience/notebook/serviceRegistry.ts

@@ -3,6 +3,7 @@
 
 'use strict';
 
+import { NotebookContentProvider as VSCNotebookContentProvider } from '../../../../types/vscode-proposed';
 import { IExtensionSingleActivationService } from '../../activation/types';
 import { IInterpreterStatusbarVisibilityFilter } from '../../interpreter/contracts';
 import { IServiceManager } from '../../ioc/types';
@@ -17,7 +18,7 @@ import { NotebookSurveyBanner, NotebookSurveyDataLogger } from './survey';
 import { INotebookContentProvider } from './types';
 
 export function registerTypes(serviceManager: IServiceManager) {
-    serviceManager.addSingleton<INotebookContentProvider>(INotebookContentProvider, NotebookContentProvider);
+    serviceManager.addSingleton<VSCNotebookContentProvider>(INotebookContentProvider, NotebookContentProvider);
     serviceManager.addSingleton<IExtensionSingleActivationService>(
         IExtensionSingleActivationService,
         NotebookIntegration

src/client/datascience/notebook/types.ts

@@ -3,14 +3,4 @@
 
 'use strict';
 
-import type { NotebookContentProvider as VSCodeNotebookContentProvider, NotebookDocument } from 'vscode-proposed';
-
 export const INotebookContentProvider = Symbol('INotebookContentProvider');
-export interface INotebookContentProvider extends VSCodeNotebookContentProvider {
-    /**
-     * Notify VS Code that document has changed.
-     * The change is not something that can be undone by using the `undo`.
-     * E.g. updating execution count of a cell, or making a notebook readonly, or updating kernel info in ipynb metadata.
-     */
-    notifyChangesToDocument(document: NotebookDocument): void;
-}