fix(core): track the editor's version and hold per URI it opened - #243
Merged
Merged
Conversation
Pushes ahead of their echo - Address a line-keyed push at the version the previous applied push produced, so a write within the editor's echo latency lands as an edit instead of being refused and replaced in full - Keep that version apart from the declared one so the staleness guard still accepts the late echo and consumes its pending push - Leave it unchanged after a full replace, which may apply as a no-op - Drop a reply that lands after a reopen, so a stale version never addresses the new buffer - Warn with the addressed and last-declared versions when the editor refuses a push; the model service's retry note moves to debug A file open under a symlink and its real path - Track the declared version, pushed version and hold per editor URI in one LanguageClientDocumentState, replacing languageClientUris - Check a change against its own URI's version, so one tab's edits are not dropped because the other tab counted higher - Keep the document open until the editor closes its last URI - Close every editor URI when the document is deleted, so a reopen recreates it Fixes #230 Fixes #241
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When the server writes a document the editor has open, it pushes the change as a line-keyed edit addressed at the editor's version, so the editor refuses an edit its buffer has outrun. That version only moved when the editor's echo arrived. A second write sent before the first write's echo was therefore addressed one version behind, refused, and replaced in full, which costs the editor its cursor and undo grouping and leaves it on older text until the replace lands. This change records the version an applied push moves the editor to and addresses the next push there, so a quick second write lands as an edit (#230).
Tracking that version per editor URI exposed that the store kept all of the editor's state per document. A file open under a symlink and its real path is one document but two editor buffers, each counting its own versions, and they shared one slot. One tab's changes were dropped once the other tab had counted higher, pushes to one tab were addressed at the other tab's version, and closing either tab released the document while the other was still open (#241). The editor's state is now kept per URI: each URI has its own declared version, pushed version and hold, and the document stays open until the editor closes its last URI.
What changes for adopters
DocumentTrackingRecord.languageClientUrisis replaced bylanguageClientDocuments: Map<LanguageClientUri, LanguageClientDocumentState>. Code that read the set readslanguageClientDocuments.keys()instead. This is the one breaking change.LanguageClientDocumentStateis exported:{ declaredVersion, pushedVersion? }for one URI, from itsdidOpento itsdidClose.clientVersionskeeps a language-client entry, the latest version any of its URIs declared, but the store no longer checks or addresses the editor through it.languageClientVersion(uri, targetUri?)takes the URI a push goes to. An override with the old one-argument signature still compiles; it is passed the target URI and ignores it.untrackLanguageClientDocuments(key)is a new protected method.delete()andcloseLanguageClientDocuments()call it before closing by canonical key, so that close ends the editor's hold on every URI.applyEditToLanguageClient, naming the version it was addressed at and the one the editor last declared. The model service's note that it retries with a full replace moved todebug. An override ofapplyEditToLanguageClientthat does not callsuperno longer gets the warning.onDidCloseor releases the document; the last close does.How I know it works
Against
main's store, six of the new tests fail for the reason they name:The remaining new tests pass on
mainand guard what this change could get wrong. Each goes red when its part of the change is broken:applyEditreply instead of before it: the test where a reply lands after a reopen.Math.maxof declared and pushed version: the test where the declared version has overtaken the pushed one.untrackLanguageClientDocumentscall fromdelete(): the test that deletes a document open under two URIs and reopens it.Fixes #230. Fixes #241.