Changing cell text does not include the cell change in the change event

[rchiodo]

Testing #145555

1. Create the extension
2. Listen to onDidChangeNotebookDocument
3. Run the extension
4. Create a new notebook
5. Add a single character to a cell.

Output of event is this:

Change event fired for file:///d%3A/Source/Testing_3/python3.ipynb => {
 "notebook": {
  "uri": {
   "$mid": 1,
   "fsPath": "d:\\Source\\Testing_3\\python3.ipynb",
   "_sep": 1,
   "external": "file:///d%3A/Source/Testing_3/python3.ipynb",
   "path": "/d:/Source/Testing_3/python3.ipynb",
   "scheme": "file"
  },
  "version": 40,
  "notebookType": "jupyter-notebook",
  "isDirty": true,
  "isUntitled": false,
  "isClosed": false,
  "metadata": {
   "custom": {
    "cells": [],
    "metadata": {
     "interpreter": {
      "hash": "8d06b578fee6feaf7c36758381f71ee046b0b7658f0309c0328fc2e4c7ac3eb9"
     },
     "kernelspec": {
      "display_name": "Python 3",
      "language": "python",
      "name": "python3"
     },
     "language_info": {
      "codemirror_mode": {
       "name": "ipython",
       "version": 3
      },
      "file_extension": ".py",
      "mimetype": "text/x-python",
      "name": "python",
      "nbconvert_exporter": "python",
      "pygments_lexer": "ipython3",
      "version": "3.10.1"
     }
    },
    "nbformat": 4,
    "nbformat_minor": 4
   },
   "indentAmount": " "
  },
  "cellCount": 4
 },
 "cellChanges": [],
 "contentChanges": []
}

I expected there to be 'cellChanges'?

[jrieken]

I expected there to be 'cellChanges'?

Fair question. Till now the thinking is that cells are always and only represented as TextDocuments and that text document events fire for it. I believe I still like that or, in other words, I don't want to duplicate text events for cells/notebooks.

Another question however is why the notebook change event has fired at all. When typing inside a cell the notebook shouldn't change

[jrieken]

Treating the fact that the event fires as you type in a cell as bug

[jrieken]

Another question however is why the notebook change event has fired at all. When typing inside a cell the notebook shouldn't change

Looping in @dbaeumer for that and what LSP is doing? The NotebookDocumentContentCellChange type could have another property document: TextDocument | undefined with similar semantics: when set something in the document has changed, its value or language. That would be consistent with other properties of the cell and for detailed change events of the cell document and still resort to the corresponding text document event. What needs to be guaranteed for that is that the text document is already updated at that point - this should be the case but more by chance than dependency...

[dbaeumer]

In LSP I support two mode:

• document only: this means they get normal text change events for cells. This is the same as listening to text model changes in the VS Code API.
• notebook (this is what @rchiodo is asking for): then in LSP I include the textual changes into the NotebookDocumentChangeEvent. To implement list I currently listen to both onDidChangeNotebookDocument and text document changes and fold them. Having this in the VS Code API first class would help me as well.

[jrieken]

To implement list I currently listen to both onDidChangeNotebookDocument and text document changes and fold them. Having this in the VS Code API first class would help me as well.

Folding them cannot be done because it is two IPC messages on the wire: one that updates the cell text document and one that updates the notebook document. But I can do the document property hint to signal that the cell document has changed is some way (either content or language)

[jrieken]

Pushed the contrary to what I said here. There is now NotebookDocumentContentCellChange#document which is set whenever something in the document has changed

[rchiodo]

Maybe I'm doing something wrong but I don't see the new change?

I get this:

I believe that should include a 'document' change?

[rchiodo]

My listening code is this:

  vscode.workspace.onDidChangeNotebookDocument(
    (e: vscode.NotebookDocumentChangeEvent) => {
      console.log(
        `Change event fired for ${e.notebook.uri} => ${JSON.stringify(
          e,
          undefined,
          " "
        )}`
      );
    }
  );

[jrieken]

😱 I see the same...

[jrieken]

Pushed a fix. This is a good sample of why unit tests aren't always enough - I simply didn't implement the renderer side of this and only relied on extension host side unit tests 🙃

[jrieken]

Fixed in main, created #146181 to cherry-pick the change into the 1.66 release branch. @rchiodo please review. Thanks