Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

textDocument/didChange does not number versions according to content changes #411

Closed
castwide opened this issue Sep 4, 2018 · 4 comments

Comments

@castwide
Copy link

castwide commented Sep 4, 2018

I'm working on a language server that uses incremental document syncs. According to the specification, the document's version number should be incremented once for each content change, including when there are multiple changes in a single textDocument/didChange notification. The vscode client conforms in most cases, but occasionally the version number goes out of sync. From my tests, it appears that the most common causes are large copy/pastes, multiple consecutive undo operations, and bulk operations like indenting blocks of code.

Here's a simple reproducible example:

  1. Open a file in an editor. (I opened a Ruby file while using my Ruby language server extension).
  2. The client sends a textDocument/didOpen notification with the version set to 1.
  3. Select 4 lines of code and press tab.
  4. The client sends a textDocument/didChange notification with the following parameters:
{
  "textDocument": {
    "uri": "file:///path/to/file.rb",
    "version": 2
  },
  "contentChanges": [
    {
      "range": {
        "start": {
          "line": 3,
          "character": 0
        },
        "end": {
          "line": 3,
          "character": 0
        }
      },
      "rangeLength": 0,
      "text": "  "
    },
    {
      "range": {
        "start": {
          "line": 2,
          "character": 0
        },
        "end": {
          "line": 2,
          "character": 0
        }
      },
      "rangeLength": 0,
      "text": "  "
    },
    {
      "range": {
        "start": {
          "line": 1,
          "character": 0
        },
        "end": {
          "line": 1,
          "character": 0
        }
      },
      "rangeLength": 0,
      "text": "  "
    },
    {
      "range": {
        "start": {
          "line": 0,
          "character": 0
        },
        "end": {
          "line": 0,
          "character": 0
        }
      },
      "rangeLength": 0,
      "text": "  "
    }
  ]
}

Based on the specification, shouldn't the new version number be 5 instead of 2?

@castwide castwide changed the title textDocument/didChange does not number versions according to content changes. textDocument/didChange does not number versions according to content changes Sep 4, 2018
@dbaeumer
Copy link
Member

dbaeumer commented Sep 4, 2018

I see that the spec is not good formulated here, but the only thing the spec requires is that with every change notification there is a new version number and that the number is increasing. However the number can not be used to indicate the number of internal changes nor must it be the case that the number is consecutive. From the spec:

	/**
	 * The version number of this document (it will increase after each
	 * change, including undo/redo).
	 */
	version: number;

I will add a sentence to the VersionedTextDocumentIdentifier as well.

@dbaeumer dbaeumer closed this as completed Sep 4, 2018
@castwide
Copy link
Author

castwide commented Sep 4, 2018

I may have misunderstood this part of the spec for DidChangeTextDocumentParams:

	/**
	 * The actual content changes. The content changes describe single state changes
	 * to the document. So if there are two content changes c1 and c2 for a document
	 * in state S10 then c1 move the document to S11 and c2 to S12.
	 */
	contentChanges: TextDocumentContentChangeEvent[];

I take it that the referenced document state numbers (S10-S12) are not the same as document version numbers?

@dbaeumer
Copy link
Member

dbaeumer commented Sep 4, 2018

No, they are not. They talk about the state of the document. I could have used S' and S'' which I might consider to do to make this more clear.

@dbaeumer
Copy link
Member

dbaeumer commented Sep 4, 2018

I made that change.

@vscodebot vscodebot bot locked and limited conversation to collaborators Oct 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants