Skip to content

Update WatchFilter API#3

Merged
MaggieShan merged 1 commit into
warpdotdev/notify-8.0.0from
maggs/update-watchfilter-api
May 21, 2026
Merged

Update WatchFilter API#3
MaggieShan merged 1 commit into
warpdotdev/notify-8.0.0from
maggs/update-watchfilter-api

Conversation

@MaggieShan
Copy link
Copy Markdown

@MaggieShan MaggieShan commented May 21, 2026

Description

  • Updates the WatchFilter API to support more granular filter fns - should_emit_event and should_watch_directory
  • This is to fix a linux/inotify-specific bug where the filter was being used at watcher registration time rather than at event emission - so filtering out a directory would silently drop all of the child events from the directory

Oz plan: https://staging.warp.dev/drive/notebook/Linux-inotify-dual-predicate-watch-filter-Warp-caller-updates-5HQUBSdF5ZqO5e8GxJmuAe

Testing

Copy link
Copy Markdown
Member

@kevinyang372 kevinyang372 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Comment thread notify/src/inotify.rs
)?;
watch_self = false;
.into_iter();
while let Some(entry_result) = iter.next() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooc why do we need to rewrite the for iterator into while let?

My understanding is we should just change should_watch in the original filter to should_watch_directory right?

Copy link
Copy Markdown
Author

@MaggieShan MaggieShan May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iiuc the previous filter_map and filter would evaluate the entry/directory after it's already been explored, whereas WalkDir::skip_current_dir ensures that we're skipping entries before exploring them

I believe we can similarly use WalkDir::filter_entry to be slightly more ergonomic but I don't have a strong opinion

Comment thread notify/src/inotify.rs
}
}

/// return `DirEntry` when it is a directory
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above but I think we should be able to keep the original logic here

@MaggieShan MaggieShan merged commit 91b7198 into warpdotdev/notify-8.0.0 May 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants