Skip to content

fix(core): track the editor's version and hold per URI it opened - #243

Merged
martin-fleck-at merged 1 commit into
mainfrom
fix/push-at-applied-version
Sep 30, 2026
Merged

martin-fleck-at merged 1 commit into
mainfrom
fix/push-at-applied-version

Conversation

@martin-fleck-at

Copy link
Copy Markdown
Contributor

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.languageClientUris is replaced by languageClientDocuments: Map<LanguageClientUri, LanguageClientDocumentState>. Code that read the set reads languageClientDocuments.keys() instead. This is the one breaking change.
  • LanguageClientDocumentState is exported: { declaredVersion, pushedVersion? } for one URI, from its didOpen to its didClose.
  • clientVersions keeps 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() and closeLanguageClientDocuments() call it before closing by canonical key, so that close ends the editor's hold on every URI.
  • A refused push is now warned from 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 to debug. An override of applyEditToLanguageClient that does not call super no longer gets the warning.
  • With a file open under two URIs, closing one of them no longer fires onDidClose or 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 second of two quick pushes is refused instead of addressed at the version the first moved the editor to.
  • The push after the late echoes goes out a version behind.
  • A refused push logs no warning naming its versions.
  • One tab's change is dropped after the other tab declared a higher version.
  • A push to one tab is refused because it is addressed at the other tab's version.
  • Closing one tab closes the document while the other still holds it.

The remaining new tests pass on main and guard what this change could get wrong. Each goes red when its part of the change is broken:

  • Reading the editor's version per document again instead of per URI: both two-tab addressing tests and the dropped-change test.
  • Not advancing the declared version per URI: eight tests, among them the reopen, late-reply and two-tab tests.
  • Ending the hold when either of two URIs closes: the keep-open test.
  • Addressing a push at the shared declared version: the per-tab addressing test.
  • Reading the URI's state after the applyEdit reply instead of before it: the test where a reply lands after a reopen.
  • Dropping the Math.max of declared and pushed version: the test where the declared version has overtaken the pushed one.
  • Advancing the version after a full replace, which the editor may apply as a no-op without stepping: the no-op full-replace test.
  • Writing the pushed version into the declared one: the late-echo test, whose keystroke after the echoes is then read against the wrong text.
  • Removing the untrackLanguageClientDocuments call from delete(): the test that deletes a document open under two URIs and reopens it.

Fixes #230. Fixes #241.

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
@martin-fleck-at
martin-fleck-at merged commit 2927fe8 into main Sep 30, 2026
7 checks passed
@martin-fleck-at
martin-fleck-at deleted the fix/push-at-applied-version branch September 30, 2026 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant