-
Notifications
You must be signed in to change notification settings - Fork 0
feat(rs): implement watch #56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughA new file-watching utility module is introduced that monitors a specified file for changes, detects write completion events, copies updated file contents to the system clipboard, and reports the byte count transferred. Supporting dependencies Changes
Sequence DiagramsequenceDiagram
participant User
participant Main as Application
participant Watcher as File System Watcher
participant File as File System
participant Clipboard as System Clipboard
User->>Main: Execute watch command with file path
Main->>Watcher: Initialize non-recursive watcher
activate Watcher
loop Monitor Events
File->>Watcher: File modification event
Watcher->>Watcher: Check for Close Write event
alt Close Write Detected
Watcher->>File: Read file contents
File-->>Watcher: Return file data
Watcher->>Clipboard: Copy contents to clipboard
Clipboard-->>Watcher: Confirm copy
Watcher->>Main: Report byte count
Main->>User: Print byte count transferred
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This replaces "profiling watch" #51
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@rs/Cargo.toml`:
- Around line 21-22: The Cargo.toml dependency choices need verification:
confirm notify = "8.2.0" is acceptable and run a dependency audit for
cli-clipboard = "0.4.0" because it is older and may pull risky transitive crates
(e.g., x11-clipboard, wl-clipboard-rs and xcb versions); run `cargo audit` on
the resolved tree, inspect the cli-clipboard dependency chain for any xcb <
1.6.0 or other RustSec advisories, and if any vulnerable/unmaintained transitive
crates are present either update cli-clipboard to a newer maintained alternative
or replace it with a maintained clipboard crate and update Cargo.toml
accordingly.
In `@rs/src/modules/watch.rs`:
- Around line 36-37: Replace the panic on clipboard set failure with propagated
error handling: change the call to ctx.set_contents(contents).expect("Failed to
copy to clipboard!") so it returns the error to the caller using the ? operator
(or map/into to convert the clipboard error into the function's error type)
inside the function that calls it (e.g., the function containing
ctx.set_contents). Ensure the surrounding function signature returns a Result so
the ? can be used and convert the clipboard error to your crate's error type if
necessary.
- Around line 26-27: Replace the panic-causing .expect() call when creating the
clipboard context with error propagation: change the
ClipboardContext::new().expect(...) usage so the constructor's error is returned
via the function's anyhow::Result (e.g., using the ? operator and optionally
.context(...) from anyhow to add a helpful message). Update the binding for ctx
to use the propagated-result form (referencing ClipboardContext::new and the
local ctx variable) so failures bubble up instead of panicking.
- Around line 32-39: The current watcher matches
EventKind::Access(AccessKind::Close(AccessMode::Write)) which is Linux-only;
replace that arm to match EventKind::Modify(ModifyKind::Data(_)) so the handler
runs cross-platform, keep using fs::read_to_string(&file) and
ctx.set_contents(...) to load and copy the updated file; additionally implement
a simple debounce (e.g., track last-handled timestamp per file and ignore events
within a short window) around the Modify handler to avoid duplicate clipboard
writes when multiple Modify events fire for a single save.
🧹 Nitpick comments (1)
rs/src/modules/watch.rs (1)
41-41: Consider propagating watcher errors or at least logging with more context.Silently printing watcher errors may hide critical issues (e.g., watched file deleted, permissions changed). Consider whether some errors should terminate the watch loop or be logged at a higher severity.
♻️ Suggested improvement
- Err(e) => println!("Error: {:?}", e), + Err(e) => eprintln!("Watcher error: {}", e),
| notify = "8.2.0" | ||
| cli-clipboard = "0.4.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if the specified versions exist on crates.io API
echo "=== Checking notify 8.2.0 ==="
curl -s "https://crates.io/api/v1/crates/notify" | jq '.crate.versions[] | select(.num == "8.2.0") | {version: .num, created_at: .created_at}' 2>/dev/null || echo "Failed to query notify"
echo ""
echo "=== Checking cli-clipboard 0.4.0 ==="
curl -s "https://crates.io/api/v1/crates/cli-clipboard" | jq '.crate.versions[] | select(.num == "0.4.0") | {version: .num, created_at: .created_at}' 2>/dev/null || echo "Failed to query cli-clipboard"
echo ""
echo "=== Listing latest versions of notify ==="
curl -s "https://crates.io/api/v1/crates/notify" | jq '.versions[0:5] | .[] | {version: .num, created_at: .created_at}' 2>/dev/null
echo ""
echo "=== Listing latest versions of cli-clipboard ==="
curl -s "https://crates.io/api/v1/crates/cli-clipboard" | jq '.versions[0:5] | .[] | {version: .num, created_at: .created_at}' 2>/dev/nullRepository: SomeMWDev/mwutil
Length of output: 998
🌐 Web query:
notify crate 8.2.0 Rust security vulnerabilities CVE
💡 Result:
I couldn’t find any published RustSec advisories (RUSTSEC-*) or CVE/GHSA entries for the Rust crate notify (including v8.2.0) in the places that normally track and export Rust crate vulns: the RustSec Advisory Database and its OSV export. [1][2]
If you want to double-check in your own project, run:
cargo install cargo-audit
cargo auditRustSec is the primary upstream source for Rust crate advisories, and it is exported to OSV (which many scanners consume). [1][2]
Sources
[1] https://rustsec.org/
[2] https://osv.dev/vulnerability/RUSTSEC-2024-0434
🌐 Web query:
cli-clipboard crate 0.4.0 Rust security vulnerabilities
💡 Result:
As of January 25, 2026, I did not find any RustSec/OSV advisory filed specifically against the cli-clipboard crate (including v0.4.0).
However, cli-clipboard v0.4.0 (released 2022-12-13) depends on several platform backends—most notably x11-clipboard = 0.7 and wl-clipboard-rs = 0.7 on Unix/Linux, plus clipboard-win, objc, objc-foundation, and objc_id on Windows/macOS. [1]
Relevant related advisories / risk notes:
- The upstream crate that
cli-clipboardforked from (clipboard/ “rust-clipboard”) has a RustSec INFO Unmaintained advisory: RUSTSEC-2022-0056. This advisory is for theclipboardcrate (notcli-clipboard), but it signals elevated supply-chain/maintenance risk in that lineage. [2] - There is a RustSec INFO Unsound advisory for
xcb(RUSTSEC-2025-0051, patched inxcb >= 1.6.0). If your resolved dependency tree (possibly via X11-related crates) includesxcb< 1.6.0, that’s a concrete issue to address. [3]
Recommended next step: run cargo audit on your project’s Cargo.lock to see whether your exact resolved versions pull in any vulnerable transitive dependencies (especially around X11/XCB). [3]
Sources: [1] [2] [3]
Verify notify 8.2.0 is acceptable; flag that cli-clipboard 0.4.0 is outdated with potential transitive dependency risks.
Both versions exist on crates.io. notify 8.2.0 (released August 2025) has no known security advisories. However, cli-clipboard 0.4.0 was released December 2022 and is over three years old. While it has no direct RustSec advisory, its dependencies (x11-clipboard, wl-clipboard-rs) and the maintenance status of its upstream source (rust-clipboard has RUSTSEC-2022-0056 for being unmaintained) present supply-chain risk. Run cargo audit on the resolved dependency tree to ensure transitive dependencies don't include vulnerable versions (notably xcb < 1.6.0).
🤖 Prompt for AI Agents
In `@rs/Cargo.toml` around lines 21 - 22, The Cargo.toml dependency choices need
verification: confirm notify = "8.2.0" is acceptable and run a dependency audit
for cli-clipboard = "0.4.0" because it is older and may pull risky transitive
crates (e.g., x11-clipboard, wl-clipboard-rs and xcb versions); run `cargo
audit` on the resolved tree, inspect the cli-clipboard dependency chain for any
xcb < 1.6.0 or other RustSec advisories, and if any vulnerable/unmaintained
transitive crates are present either update cli-clipboard to a newer maintained
alternative or replace it with a maintained clipboard crate and update
Cargo.toml accordingly.
| let mut ctx = ClipboardContext::new() | ||
| .expect("Failed to create clipboard context!"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace .expect() with proper error propagation.
Using .expect() causes a panic on failure, which provides a poor user experience. Since the function already returns anyhow::Result, propagate the error instead.
🔧 Suggested fix
- let mut ctx = ClipboardContext::new()
- .expect("Failed to create clipboard context!");
+ let mut ctx = ClipboardContext::new()
+ .map_err(|e| anyhow::anyhow!("Failed to create clipboard context: {}", e))?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let mut ctx = ClipboardContext::new() | |
| .expect("Failed to create clipboard context!"); | |
| let mut ctx = ClipboardContext::new() | |
| .map_err(|e| anyhow::anyhow!("Failed to create clipboard context: {}", e))?; |
🤖 Prompt for AI Agents
In `@rs/src/modules/watch.rs` around lines 26 - 27, Replace the panic-causing
.expect() call when creating the clipboard context with error propagation:
change the ClipboardContext::new().expect(...) usage so the constructor's error
is returned via the function's anyhow::Result (e.g., using the ? operator and
optionally .context(...) from anyhow to add a helpful message). Update the
binding for ctx to use the propagated-result form (referencing
ClipboardContext::new and the local ctx variable) so failures bubble up instead
of panicking.
| if let EventKind::Access(AccessKind::Close(AccessMode::Write)) = event.kind { | ||
| let contents = fs::read_to_string(&file) | ||
| .context("Failed to read updated file!")?; | ||
| let len = contents.len(); | ||
| ctx.set_contents(contents) | ||
| .expect("Failed to copy to clipboard!"); | ||
| println!("Copied {} bytes to clipboard!", len); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
notify crate CloseWrite event cross-platform support macOS Windows
💡 Result:
In the Rust notify crate, a CloseWrite / CLOSE_WRITE-style event (i.e., “file opened for writing was closed”, exposed historically as Op::CLOSE_WRITE and in newer APIs via EventKind::Access(...) close semantics) is Linux-only because it relies on inotify’s IN_CLOSE_WRITE capability. It is not available on macOS or Windows, so you can’t depend on it for cross-platform behavior. [1][2]
What to do cross-platform instead (macOS + Windows + Linux):
- Treat “write completed” as a debounced burst of
Modify(Data)/Writeevents (i.e., “no more writes for N ms ⇒ consider complete”). [3] - Also handle atomic-save patterns used by many editors/apps (write temp file → rename over target) by watching for rename/move-to events, not just writes. [1]
Sources: [1] [2] [3]
🌐 Web query:
notify crate Rust EventKind::Modify ModifyKind::Data event types
💡 Result:
In notify (v8.2.0), “data modification” events are represented as:
EventKind::Modify(ModifyKind::Data(data_change))[1][2]
Where data_change: DataChange can be one of: [3]
DataChange::Any(unknown data-change type)DataChange::Size(file size changed)DataChange::Content(file contents changed)DataChange::Other(known but not otherwise representable)
ModifyKind overall is: Any | Data(DataChange) | Metadata(MetadataKind) | Name(RenameMode) | Other. [2]
Sources: [1] [2] [3]
CloseWrite events are Linux-specific and won't trigger on macOS or Windows.
The EventKind::Access(AccessKind::Close(AccessMode::Write)) event relies on inotify's IN_CLOSE_WRITE capability, which is unavailable on macOS (FSEvents) and Windows (ReadDirectoryChangesW). For cross-platform support, match on EventKind::Modify(ModifyKind::Data(_)) events instead.
🔧 Cross-platform event handling
+use notify::event::ModifyKind;
+
for res in rx {
match res {
Ok(event) => {
- if let EventKind::Access(AccessKind::Close(AccessMode::Write)) = event.kind {
+ let should_copy = matches!(
+ event.kind,
+ EventKind::Access(AccessKind::Close(AccessMode::Write))
+ | EventKind::Modify(ModifyKind::Data(_))
+ );
+ if should_copy {
let contents = fs::read_to_string(&file)
.context("Failed to read updated file!")?;Note: Matching on Modify events may trigger frequently (e.g., multiple times per save). Add debouncing to avoid redundant clipboard operations.
🤖 Prompt for AI Agents
In `@rs/src/modules/watch.rs` around lines 32 - 39, The current watcher matches
EventKind::Access(AccessKind::Close(AccessMode::Write)) which is Linux-only;
replace that arm to match EventKind::Modify(ModifyKind::Data(_)) so the handler
runs cross-platform, keep using fs::read_to_string(&file) and
ctx.set_contents(...) to load and copy the updated file; additionally implement
a simple debounce (e.g., track last-handled timestamp per file and ignore events
within a short window) around the Modify handler to avoid duplicate clipboard
writes when multiple Modify events fire for a single save.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please open an issue for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
@SomeMWDev, I'll open an issue for tracking the cross-platform event handling improvement.
🏁 Script executed:
#!/bin/bash
# Create a GitHub issue for the cross-platform watch event handling
gh issue create \
--title "Make watch module event handling cross-platform" \
--body "## Problem
The watch module currently uses \`EventKind::Access(AccessKind::Close(AccessMode::Write))\` to detect file changes, which relies on inotify's \`IN_CLOSE_WRITE\` capability. This event type is Linux-only and won't trigger on macOS (FSEvents) or Windows (ReadDirectoryChangesW).
## Proposed Solution
Replace the current event matching with \`EventKind::Modify(ModifyKind::Data(_))\` for cross-platform support. Consider adding debouncing to handle multiple Modify events that may fire for a single save operation.
## References
- Pull Request: https://github.com/SomeMWDev/mwutil/pull/56
- Review Comment: https://github.com/SomeMWDev/mwutil/pull/56#discussion_r2726025520
- Requested by: `@SomeMWDev`
## Affected File
\`rs/src/modules/watch.rs\` (lines 32-39)" \
--assignee SomeMWDev
Length of output: 45
@SomeMWDev, I've created issue #57 to track the cross-platform event handling improvement for the watch module. The issue has been assigned to you and includes references to this PR and review comment.
| ctx.set_contents(contents) | ||
| .expect("Failed to copy to clipboard!"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Propagate clipboard error instead of panicking.
Same issue as the clipboard initialization—use ? operator with error conversion.
🔧 Suggested fix
- ctx.set_contents(contents)
- .expect("Failed to copy to clipboard!");
+ ctx.set_contents(contents)
+ .map_err(|e| anyhow::anyhow!("Failed to copy to clipboard: {}", e))?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ctx.set_contents(contents) | |
| .expect("Failed to copy to clipboard!"); | |
| ctx.set_contents(contents) | |
| .map_err(|e| anyhow::anyhow!("Failed to copy to clipboard: {}", e))?; |
🤖 Prompt for AI Agents
In `@rs/src/modules/watch.rs` around lines 36 - 37, Replace the panic on clipboard
set failure with propagated error handling: change the call to
ctx.set_contents(contents).expect("Failed to copy to clipboard!") so it returns
the error to the caller using the ? operator (or map/into to convert the
clipboard error into the function's error type) inside the function that calls
it (e.g., the function containing ctx.set_contents). Ensure the surrounding
function signature returns a Result so the ? can be used and convert the
clipboard error to your crate's error type if necessary.
This replaces "profiling watch"
#51
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.