Markdown preview: refresh on file-system change even when document is open#316756
Open
yogeshwaran-c wants to merge 1 commit into
Open
Markdown preview: refresh on file-system change even when document is open#316756yogeshwaran-c wants to merge 1 commit into
yogeshwaran-c wants to merge 1 commit into
Conversation
… open
When the previewed markdown file is overwritten on disk (e.g. a different
file copied over it, an external generator regenerates the file, an editor
agent rewrites it), the preview can keep showing stale content.
The file-system watcher in `MarkdownPreview` was guarded by:
if (!vscode.workspace.textDocuments.some(doc => areUrisEqual(doc.uri, uri))) {
this.refresh();
}
The intent was to avoid a duplicate refresh when the file was also being
tracked as an open `TextDocument` (in which case `onDidChangeTextDocument`
would handle the update). In practice that assumption does not always
hold: the text document model's reload from disk can be skipped or
delayed by etag/stat caching, network/remote FS timing, or
content-equality short-circuits, so `onDidChangeTextDocument` never
fires for the change. The watcher event was the only remaining signal,
and the guard silently dropped it -- leaving the preview frozen on the
pre-overwrite content until the user manually edited or refreshed it.
Drop the guard so the watcher always calls `refresh()`. Any genuine
overlap with `onDidChangeTextDocument` is already coalesced by the
300ms `#throttleTimer` debounce and the `PreviewDocumentVersion`
equality check inside `#updatePreview`, so unconditionally refreshing
here does not introduce duplicate renders.
Fixes microsoft#265277
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to make Markdown previews refresh when the backing file changes on disk, even if the document is already open in VS Code.
Changes:
- Removes the open-document guard from the Markdown preview file-system watcher.
- Adds explanatory comments about why file-system events should trigger refreshes unconditionally.
| // content. Any genuine duplicate refresh that overlaps with `onDidChangeTextDocument` | ||
| // is already coalesced by the `#throttleTimer` debounce and version-equality check in | ||
| // `#updatePreview`, so unconditionally refreshing here is safe. | ||
| this.refresh(); |
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.
Summary
Fixes #265277 — markdown preview keeps showing stale content after the file is overwritten on disk (external generator, file copy, editor agent,
git pull, etc.).Root cause
MarkdownPreviewinstalls aFileSystemWatcherfor the previewed resource but wraps the refresh in a guard:The intent was to avoid a duplicate refresh when the file is also tracked as an open
TextDocument, deferring toonDidChangeTextDocumentin that case. In practice that assumption does not always hold for external overwrites: the text document model's reload from disk can be skipped or delayed by etag/stat caching, network/remote FS timing, or content-equality short-circuits, soonDidChangeTextDocumentnever fires for the change. The file-system event was the only remaining signal, and the guard silently dropped it — leaving the preview frozen on the pre-overwrite content until the user manually edited or invokedMarkdown: Refresh preview.This matches the persistent reports in the issue thread after #300629 landed (the case-insensitive URI fix only addressed the Windows-casing variant; users on Ubuntu/macOS/remote SSH continued to report it for non-casing reasons).
Fix
Drop the open-document guard so the watcher always calls
refresh()when the previewed file changes on disk. The two paths can no longer drop each other's events.Any genuine duplicate refresh that overlaps with
onDidChangeTextDocumentis already coalesced downstream:MarkdownPreview.refresh()debounces via the 300 ms#throttleTimer, so tworefresh()calls within the window collapse into one#updatePreviewinvocation.#updatePreviewearly-returns whenPreviewDocumentVersionmatches the last rendered version, so a redundant invocation against an unchanged in-memory document is a no-op.So unconditionally refreshing here does not introduce duplicate renders.
Test plan
Manual repro on Windows 11 (the original report's platform), confirmed before/after the change:
repro.mdwith some content, open it, then open the markdown preview side-by-side.repro.mdwith a different markdown file (copy a different file with the same name on top of it; or have an external script regenerate it).Markdown: Refresh previewis required.onDidChangeTextDocumentpath is unchanged).tsc -p extensions/markdown-language-features/tsconfig.json --noEmitclean).A unit test isn't really feasible here without a real file-system + watcher harness — the bug is specifically about events that the in-process text model misses.