vscode icon indicating copy to clipboard operation
vscode copied to clipboard

notebookDocument/didChange event's `notebook.version` contains previous notebook version (which means 0 is sent on the first edit)

Open rehmsen opened this issue 1 year ago • 1 comments

Does this issue occur when all extensions are disabled?: Yes/No

  • VS Code Version: 1.92.2
  • OS Version: OS X

Steps to Reproduce:

  1. Open a notebook document, like extensions/vscode-api-tests/testWorkspace/test.ipynb
  2. Inspect the LSP notebookDocument/didOpen event, which contains notebook.version: 0 - this is correct and expected.
  3. Make a change like adding a space to a cell.
  4. Inspect the LSP notebookDocument/didChange event, which still contains notebook.version: 0 - this is a protocol violation. The spec says:
        /**
     * The version number of this document (it will increase after each
     * change, including undo/redo).
     */
    version: [integer](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#integer);
    

Because of the "didChange", I believe the LSP message should come after the change, and have the up to date version (which would be 1 after the first change).

I dug in a bit to understand why this happens, and the version increment seems to happen here: https://github.com/microsoft/vscode/blob/2a0e70d6c6226ef5021285b22cfbe07c929d64bc/src/vs/workbench/services/workingCopy/common/storedFileWorkingCopy.ts#L734

This is called from the listener installed here: https://github.com/microsoft/vscode/blob/2a0e70d6c6226ef5021285b22cfbe07c929d64bc/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts#L161

The LSP event is sent from a listener installed earlier, here: https://github.com/microsoft/vscode/blob/2a0e70d6c6226ef5021285b22cfbe07c929d64bc/src/vs/workbench/api/browser/mainThreadDocuments.ts#L96-L98

See how in line 97, the event has a separate version, which is indeed an already incremented version (in my case it is 2, not 1, which is a bit surprising). In extHostDocuments, that version is still accessible from the event, but then not sent on to _onDidChangeDocument in line 159ff: https://github.com/microsoft/vscode/blob/2a0e70d6c6226ef5021285b22cfbe07c929d64bc/src/vs/workbench/api/common/extHostDocuments.ts#L143-L171

I checked in the same location for a non-notebook document, and the event.version was equal to the document.version (it was also 2 for the first edit).

Do you agree this is a protocol violation for notebooks? If so, do you have an idea for how to best fix it?

rehmsen avatar Aug 28 '24 10:08 rehmsen