fs: Map EventKind::Other to Changed instead of None#48695
Closed
lex00 wants to merge 2 commits intozed-industries:mainfrom
Closed
fs: Map EventKind::Other to Changed instead of None#48695lex00 wants to merge 2 commits intozed-industries:mainfrom
lex00 wants to merge 2 commits intozed-industries:mainfrom
Conversation
Contributor
|
Just a small thought: since Ref: https://docs.rs/notify/latest/notify/enum.EventKind.html |
Contributor
Author
|
Thanks @marcocondrache , I'm new here I appreciate it. This makes it more defensive. It looks like handle_event function filters out Access events at line 247-252 before callback , so events never reach path_event_kind() in practice. |
The catch-all arm in the EventKind → PathEventKind mapping returned None for EventKind::Other and EventKind::Any. Three downstream consumers drop events with kind: None: - BackgroundScanner initial scan filters them out - Settings file watcher silently ignores them - LSP didChangeWatchedFiles notifications are never sent EventKind::Other includes OS rescan signals like macOS MustScanSubDirs (FSEvents queue overflow). Map these to Changed so all consumers handle them correctly. Extracts the mapping into a standalone function for testability. Related to zed-industries#38109 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Only map EventKind::Other and EventKind::Any to Changed, keeping Access (and any future unknown variants) as None. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
3234eff to
e726657
Compare
pcfreak30
added a commit
to pcfreak30/zed
that referenced
this pull request
Feb 13, 2026
Applied 5 PRs to fix Zed getting out of sync with filesystem changes: - zed-industries#48691: Include file size in DiskState to fix stale buffer reload - zed-industries#48695: Map EventKind::Other to Changed instead of None - zed-industries#48698: Reload after undo when file changed while dirty - zed-industries#48704: Rescan repositories when window regains focus - zed-industries#47462: Handle removed Linux watch paths
This was referenced Mar 8, 2026
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.
Closes #48694
Related to #38109
Problem
When the OS event queue overflows, the
notifycrate emitsEventKind::Otheras a rescan signal (e.g. macOSMustScanSubDirs, LinuxIN_Q_OVERFLOW). The catch-all infs_watcher.rsmaps this toNone.Three downstream consumers silently drop events with
kind: None:didChangeWatchedFilesnotifications skip them (lsp_store.rs)The original
_ => Nonepredates these consumers. When it was written,process_events()stripped the kind and only used paths, soNonewas harmless. The settings watcher and LSP path were added later and assumedkindwould always be set.This is an uncommon trigger (requires event queue overflow from heavy filesystem activity) and the failure is silent, which makes it hard to link to specific user reports. But the fix is simple and the current behavior is clearly wrong.
Fix
Map the catch-all to
Some(PathEventKind::Changed). "Something changed, re-read from disk" is the correct semantic for bothEventKind::OtherandEventKind::Any.Extracted the mapping into a standalone
path_event_kind()function with unit tests.Release Notes: